| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
To actually trigger this workflow, I'll push similar commits similar to #20549.. Also, there's a lot of print/debug statements throughout, I'll remove them once this moves ahead.. |
Sorry, something went wrong.
|
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 |
Sorry, something went wrong.
On Linux and Mac. Add a font_manager test that loads the font and uses it. This could be useful in testing the multi-font support (matplotlib#20740 etc). I couldn't get the font installed on Windows. There is a Chocolatey installer that installs all of the Noto fonts, which takes a really long time to run.
|
@aitikgupta Do these have a review team looking at them? |
Sorry, something went wrong.
|
@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) |
Sorry, something went wrong.
|
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. We can 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. |
Sorry, something went wrong.
|
(Unfortunately, I will not have the bandwidth to review this in the coming days.) |
Sorry, something went wrong.
|
Hi @aitikgupta we were all discussing this PR yesterday - did you have an opportunity to rebase and address the review comments? |
Sorry, something went wrong.
|
Hey @jklymak, I'm currently out sick. Will get back to this as soon as I can, probably a week or two. Apologies! |
Sorry, something went wrong.
|
No rush, get better! |
Sorry, something went wrong.
|
Hey @QuLogic, addressed all the comments (added comments for further discussions), and brought the PR up to speed! |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
The new code is 2-space indent instead of 4. |
Sorry, something went wrong.
| Get the `.FT2Font` for *font_prop*, clear its buffer, and set its size. | ||
| """ | ||
| font = get_font(findfont(font_prop)) | ||
| fname = find_fonts_by_props(font_prop) |
There was a problem hiding this comment.
fname should be renamed to filename_dict or at least filenames (plural) for clarity.
Given that get_font only uses .values(), I would suggest making it clearer at this stage too (filenames = list(find_fonts_by_props(font_prop).values()); font = get_font(filenames)) (and modify get_font to take a list instead of a dict). This would also make it easier for third-parties to directly call get_font with a list of font paths rather having to invent keys to create a dict, if they don't already have a dict at hand.
Sorry, something went wrong.
There was a problem hiding this comment.
I do not remember why we used a dictionary instead of a list
Sorry, something went wrong.
|
As a meta comment, this seems like a pretty big change to squeeze in right before a 3.6 release, versus getting it in main 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. |
Sorry, something went wrong.
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...) |
Sorry, something went wrong.
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. |
Sorry, something went wrong.
|
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 fallback paths 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.) |
Sorry, something went wrong.
That will require documenting ft2font 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. |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
There was a problem hiding this comment.
Should be good, though needs the rebase for added/removed files.
Sorry, something went wrong.
Co-authored-by: Aitik Gupta <aitikgupta@gmail.com> Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
|
Squashed this all to 1 commit: (dd310) ~/s/p/m/matplotlib fallback-revamp↑·1↓·63|…3⚑5 ✔ 17:45:01 $ git describe v3.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 |
Sorry, something went wrong.
|
Thank you @aitikgupta ! This would not have been possible without your work. |
Sorry, something went wrong.
|
Super glad this finally made it! Thanks everyone for pushing this 🙇🏼♂️ |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
PR Summary
This PR modifies the internal structure of FT2Font (the interface between fonts and Matplotlib) in favor of implementing Font Fallback for Matplotlib, and allow Agg backend to use the new codepath.
It builds on the previous PR: #20549, which was the 'first-step', i.e., "parsing multiple families".. this PR implements using those families for font fallback.
This would help us in multi-language support, for example (Previous / After):
^the fonts are chosen such that the difference is visually noticeable.
A flowchart explaining the text rendering algorithm with font fallback:

Here's the script:
Fixes #18883, #15260
PR Checklist