-
-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Support TTC fonts by converting them to TTF in font cache #23293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Nice one! I cannot really review the content, but do you know how cff-parts of the ttc behaves, e.g. #20870? (Also, out of curiosity but maybe also usefulness, it may be nice to have some info-level logging of the conversion process.) |
I guess that directly handling TTC is "better", but if the option is to not be able to handle TTC, this is for sure a very worthwhile approach! I think it can be worth a user release note as users now can do things that they couldn't do earlier (and may not realize that otherwise). |
Haven't checked the technical details of the approach, but if that can be made to work this may be one of the easier ways forward (we can always remove that later once "real" ttc support comes in, similarly to @oscargus' point above). |
As far as I can tell, the cff-tables are correctly extracted. The created TTF file, e.g. for Apple SD Gothic Neo, is valid in the font preview and font forge without warnings. However, the example from #20870 still doesn't work, probably failing in the pdf/ps backend.
Good point, I will add that. |
51cd34c
to
8dc3d5a
Compare
8dc3d5a
to
93618e8
Compare
Fonts in a TrueType collection file (TTC) can now be added and used. Internally, | ||
the embedded TTF fonts are extracted and stored in the matplotlib cache | ||
directory. Users upgrading to this version need to rebuild the font cache for | ||
this feature to become effective. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe write how that is done? (I do not really recall myself...)
(Also one may consider having a public method that is directly importable from matplotlib
, but that is another question...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean rebuilding the font cache? I think the simplest way to trigger a rebuild is to delete the folder printed by
python -c "import matplotlib; print(matplotlib.get_cachedir())"
Maybe there is a more user-friendly, programmatic way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding the question about the public API, I'm not sure if we want to expose this. It's not directly useful for the end user. If someone wants to use a custom TTC font, they would call the font manager's addfont()
with the path to the TTC file. This will automatically extract the TTF and add the created files to the internal list of fonts.
If a user were to use the _split_ttc()
function, they would need to use the returned paths and add them to the font manager themself. I don't think that's what we want users to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, rebuilding the font cache. I seem to recall that there is a function to rebuild the cache without removing it, but cannot find it now... Googling gives matplotlib.font_manager._rebuild()
(and quite a few questions), but that is not a current approach.
Anyway, some sort of info about how to do this (maybe it is in the docs to link to) I would think is useful since probably quite a few users like to do that.
Fix typos Co-authored-by: Oscar Gustafsson <oscar.gustafsson@gmail.com>
I was just looking at how to write this functionality! This is very brute force, we do currently support ttc, but only pick out the first font.
we have a version number on the font cache for exactly this reason (well in case we change the format, but close enough). |
tacaswell@b143647 is a stack of changes that will find all of the fonts in a ttc and passes all but 1 test (which I think identifies that there is a bigger set of bugs in the way fonts + mathtext are being used....they are just not raising). This is on top of #20740 so won't cleanly cherry-pick to main. As @anntzer predicted, the hard part was not the c++ work, but in threading through the information that we have to identify fonts by The code is very ambiguous about treating On a bit more consideration, adding public API to re-generate the font cache and install it in the correct global state would be good, however I am not sure that it is trivial to do reliably because we make a We could replace those with free functions that then call the right methods, but I'm sure at least one of our users is trying to walk back up to the instance from the method, but that is probably an acceptable thing to break with no warning. @sauerburger Sorry for high-jacking you PR with only marginally related side-tracks. |
FWIW in mplcairo I specifically recognize when you pass a path that ends with |
I thought of doing |
@sauerburger is this waiting on us, or on you? I've moved to draft, but feel free to move back if we should review. |
PR Summary
This PR adds support for all ttf fonts bundeled in ttc files. In
FontManager.addfont()
, if a font ends in.ttc
, the new functions_read_ttc()
and_dump_ttf()
are triggered to extract all TTF fonts from the collection. The extracted fonts are stored in the font cache directory with the.ttf
extension and added to the list of ttf fonts.The details of the implantation can be changed (for example where to put the parsing code). Do you think the overall strategy of parsing TTC, dumping TTF to the font cache is a good idea?
Why is it useful
Caveats
PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).