GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Remove ttconv and implement Type-42 embedding using fontTools by jkseppan · Pull Request #20866 · matplotlib/matplotlib Conversation
|
|
Now this works with almost all TrueType fonts on my Mac, with the exception of CFF fonts (#20870) and |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
de16cf0 to
daee4e5
Compare
016713a to
7cea2d4
Compare
There was an error while loading. Please reload this page.
| if fontfile.endswith('ttc'): | ||
| # fix this once we support loading more fonts from a collection | ||
| # https://github.com/matplotlib/matplotlib/issues/3135#issuecomment-571085541 | ||
| options.font_number = 0 |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I think we test a ttc in CI already, so it could be used to test this line?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| The Type-42 formatted font | ||
| """ | ||
| version, breakpoints = _version_and_breakpoints(font.get('loca'), fontdata) | ||
| post, name = font['post'], font['name'] |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Are these tables guaranteed to exist?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Both post and name are required: https://docs.fileformat.com/font/ttf/
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| tuple | ||
| ((v1, v2), breakpoints) where v1 is the major version number, | ||
| v2 is the minor version number and breakpoints is a sorted list | ||
| of offsets into fontdata; if loca is not available, just the table | ||
| boundaries |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
For multiple return, you should list the entries separately, like Parameters.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Wow, I missed that this came in. 💯 on dropping an extension module! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Dropping to draft until the comments and rebase are addressed. Anyone should feel free to move back to active... |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Ping @jkseppan? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
|
||
| Returns | ||
| ------- | ||
| fontTools.ttLib.ttFont.TTFont |
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Long term, it could make sense to link to the documentation? (Although fontTools is not in the intersphinx_mapping yet.)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
We want to try and do mpl 3.7 feature freeze around Jan 1 so I think it is unlikely we will have the bandwidth to get this rebased and reviewed by then, pushing to 3.8. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
7cea2d4 to
a1b41fe
Compare
|
Do we have someone who can pick this up? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
7f2b2a2 to
1393702
Compare
|
FYI, I think I'm okay with this as it stands, but as I did some followup cleanup work, I'm not sure if I should approve it myself. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I am still open to the possibility to add fontTools to the intersphinx-stuff and provide a proper link, but I am also open to do that myself in another PR.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Btw, do we (i.e. someone with a Mac(?)) know if #20866 (comment) still holds? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Maybe @greglucas or @ksunden can have a look at it? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Using the example from the linked issue: # this needs the patch above import matplotlib from pathlib import Path matplotlib.use('ps') from matplotlib import pyplot as plt # the following font is in CFF format on my Mac font = Path('/System/Library/Fonts/AppleSDGothicNeo.ttc') fig = plt.figure() plt.axis((0,10,0,10)) plt.text(3,3,'Glib jocks quiz nymph to vex dwarf.', font=font) matplotlib.rc('ps', fonttype=42) plt.savefig('fmt42.ps') matplotlib.rc('ps', fonttype=3) plt.savefig('fmt3.ps') The type42 is completely blank for me (no axes either). The type3 shows up as expected. I don't get an error though 🤷 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
That example fails for me on main. Trying to open the and with this branch I get |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Okay, looks like the answer is that this has not fixed #20866 (comment), however that is broken on main anyway, so I'd lean towards not blocking for merging this if we are otherwise satisfied. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
The issue also appears to be font specific, dejavu being embedded as type42 works OK. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Split _backend_pdf_ps.get_glyphs_subset into two functions: get_glyphs_subset returns the font object (which needs to be closed after using) and font_as_file serializes this object into a BytesIO file-like object.
Move the test in lib/matplotlib/tests/test_ttconv.py to lib/matplotlib/tests/test_backend_pdf.py. Actually it has not been a test of ttconv for a while, since type-3 conversion has not been using ttconv.
Fonttools cannot subset these and drops them; avoid the warning.
Also don't end up in an infinite loop if there is a larger gap between breakpoints than we would like.
FT2Font throws RuntimeError for some fonts
Not the subsetted one, since in some cases fontTools.subset does not produce a good name table.
De-wrap a few lines now that we allow longer lines.
1393702 to
45a1654
Compare
This was missed in matplotlib#20866
This was missed in matplotlib#20866
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
replace ttconv for ps/pdf
There was an error while loading. Please reload this page.