Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 31, 2025

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Sep 6, 2024

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 to DejaVu Sans. Thus font fallback can easily be tested by starting with cmr10, and pulling additional characters from DejaVu 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 ttconv extension 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

@QuLogic
Copy link
Member Author

QuLogic commented Sep 6, 2024

FWIW, #20866 does appear to fix the EPS result.

@QuLogic
Copy link
Member Author

QuLogic commented Oct 16, 2024

Comparing 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.

@QuLogic
Copy link
Member Author

QuLogic commented Dec 3, 2024

I modified a couple other tests to use the same fonts/text, and removed a couple tests that seem to duplicate the others.

@ksunden
Copy link
Member

ksunden commented Dec 4, 2024

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.

@QuLogic
Copy link
Member Author

QuLogic commented Dec 6, 2024

We are still checking Noto Sans CJK and WenQuanYi in test_font_manager.py.

I don't think CJK characters are sufficiently different from semi-Latin script (unlike e.g., Devanagari, etc.) in these cases.

@QuLogic QuLogic force-pushed the better-fallback-tests branch 2 times, most recently from 6844ee8 to 47fb508 Compare December 18, 2024 01:00
Working on matplotlib#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 to `DejaVu Sans`. Thus font fallback can easily be
tested by starting with `cmr10`, and pulling additional characters from
`DejaVu 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.
- 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.
@QuLogic QuLogic force-pushed the better-fallback-tests branch from 47fb508 to 921e49f Compare December 19, 2024 09:41
@tacaswell tacaswell merged commit 7285029 into matplotlib:main Jan 31, 2025
39 checks passed
@tacaswell tacaswell added this to the v3.11.0 milestone Jan 31, 2025
@QuLogic QuLogic deleted the better-fallback-tests branch January 31, 2025 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants