Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Improve fallback font export tests#28784
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
FWIW,#20866 does appear to fix the EPS result. |
0129d60 to4f6cf0eCompareComparing in evince, the type 3 PDF glyphs are slightly cropped at the bottom vs the type42 file. But when@ksunden checked on macOS Preview, the whole glyphs changed thickness somewhat, so I'm going to chalk this up to renderer differences right now. |
4f6cf0e to1cff265Compare1cff265 to9638129CompareI modified a couple other tests to use the same fonts/text, and removed a couple tests that seem to duplicate the others. |
Are we missing the CJK font checks now with the new test string? (It has a lot of accented characters, but not as many characters that require specific fonts) If the goal of the test is purely to test if fallback works, I gues this is probably enough, but do we also need to test the specific fallback for CJK characters? Because that feels like a specific usecase that is representative of real world usage. |
We are still checking Noto Sans CJK and WenQuanYi in I don't think CJK characters are sufficiently different from semi-Latin script (unlike e.g., Devanagari, etc.) in these cases. |
6844ee8 to47fb508CompareWorking onmatplotlib#28765, I realized that we don't need an external font totest font fallback, as `cmr10`, and related, fonts have fairly lowcoverage compared to `DejaVu Sans`. Thus font fallback can easily betested by starting with `cmr10`, and pulling additional characters from`DejaVu Sans`.In addition, this change increases unique characters in the figure to491, which triggers some code paths for tables that are normally limitedto 256 characters.
- matplotlib.tests.test_backend_pdf::test_embed_fonts is the same as `test_multi_font_type3` and `test_multi_font_type42`- matplotlib.tests.test_ft2font::test_font_fallback_chinese is the same as `test_fallback_smoke` except with Chinese fonts.
47fb508 to921e49fCompare7285029 intomatplotlib:mainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
PR summary
Working on#28765, I realized that we don't need an external font to test font fallback, as
cmr10, and related, fonts have fairly low coverage compared toDejaVu Sans. Thus font fallback can easily be tested by starting withcmr10, and pulling additional characters fromDejaVu Sans.In addition, this change increases unique characters in the figure to 491, which triggers some code paths for tables that are normally limited to 256 characters.
The latter may be a bit overkill, but it shows that EPS is broken for a large set of unique characters, at least when viewed with evince. In that case, about half the bottom set of accented characters is missing, without any warnings produce. I have not yet determined the cause of this problem, but it looks like both type 3 and 42 use the
ttconvextension to subset, though for large character sets, type 3 is forced to 42.It is possible that#20866 might be helpful here, but it would also be helpful if someone could confirm with a different EPS viewer.
PR checklist