To actually trigger this workflow, I'll push similar commits similar to#20549..
Note: this is only tested against Agg backend (since agg is pretty straightforward in terms of font usage)

Also, there's a lot of print/debug statements throughout, I'll remove them once this moves ahead..

Note: There's some structural changes to how FT2Font is handling stuff in#20804, which builds on this PR..

EDIT: I've merged the FT2Font changes together for both PRs

@aitikgupta Do these have a review team looking at them?

@jklymak I don't think anybody is actively reviewing these font handling PRs at the moment.. but they're ready for review and not drafts (I'll resolve the conflicts)

Going ahead and removing the test, since font distributions for different Linux versions across CI are different.. and thus their kerning information is different -- different visual outputs.

Wecan test it with a self-shipped CJK font, which is guaranteed to be present across all platforms.. but that'd be another problem because their file sizes are huge.

(Unfortunately, I will not have the bandwidth to review this in the coming days.)

Hi@aitikgupta we were all discussing this PR yesterday - did you have an opportunity to rebase and address the review comments?

Hey@jklymak, I'm currently out sick. Will get back to this as soon as I can, probably a week or two. Apologies!

No rush, get better!

Hey@QuLogic, addressed all the comments (added comments for further discussions), and brought the PR up to speed!

The general design looks reasonable and a skim through the actual implementation only revealed the small issues listed above, which should be fixed, but once that's done I am happy with merging this as is and iterating over it later.

The new code is 2-space indent instead of 4.

As a meta comment, this seems like a pretty big change to squeeze in right before a 3.6 release, versus getting it inmain and letting the devs live with it, and anyone else who is testing our dev tree, for a few months. However, maybe I am misunderstanding its potential to break existing users who don't typically need the fallback.

right before a 3.6 release

Talking about that: what is the time line? I sort of expected 3.5.3 to be released "soon" and then probably a few months until 3.6? (Really OT, but still...)

As a meta comment, this seems like a pretty big change to squeeze in right before a 3.6 release, ...

Fair, however you do not pass a list of fallbacks then there should be no behavior change, we hammer these code paths in the tests, and I do not think of the core devs do enough mixed glyph work (I could be wrong about that) to meaning stress it.

An additional point I just thought about: the actual low-level API (including passing a list of fallback FT2Font to the FT2Font constructor) should probably be marked as provisional. While I haven't actually checked, I believe that the current API may be slightly inefficient, because (if I understood it correctly) we will always immediately parse all font files (a costly operation) in the fallback list, regardless of whether we actually need them or not, whereas we could imagine an API where we only store a list of fallbackpaths that get parsed one at a time when needed. (This is also relevant for mplcairo, which doesn't use FT2Font at all and simply gets the paths of the fonts to do its own interfacing with freetype.)
I certainly don't want to hold up merging by requesting a large API design change at the last minute, but marking the low-level parts as provisional would certainly simplify possible improvements in the future.

An additional point I just thought about: the actual low-level API (including passing a list of fallback FT2Font to the FT2Font constructor) should probably be marked as provisional.

That will require documentingft2font at all, but a reasonable request. The keyword already starts with_ so I think documenting it at provisional and/or private is consistent with the original intent.

I wasn't actually asking for FT2Font to be documented (a blurb in the changelog without linking API entries would have worked too), but sure, that's always a good thing to add.

Squashed this all to 1 commit:

(dd310) ~/s/p/m/matplotlib fallback-revamp↑·1↓·63|…3⚑5✔ 17:45:01 $ git describev3.5.2-2641-g85bacdccc5(dd310) ~/s/p/m/matplotlib fallback-revamp↑·1↓·63|…3⚑5✔ 17:45:05 $ git diff 14251eb(dd310) ~/s/p/m/matplotlib fallback-revamp↑·1↓·63|…3⚑5

Thank you@aitikgupta ! This would not have been possible without your work.

Super glad this finally made it! Thanks everyone for pushing this 🙇🏼‍♂️