FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Support scale in ttf composite glyphs by jkseppan · Pull Request #18081 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

Support scale in ttf composite glyphs - #18081

Closed
jkseppan wants to merge 7 commits into
matplotlib:masterfrom
jkseppan:ttconv-scale
ConversationCommits (7)ChecksFiles changed
Closed

Support scale in ttf composite glyphs#18081
jkseppan wants to merge 7 commits into
matplotlib:masterfrom
jkseppan:ttconv-scale

Conversation

jkseppan commented Jul 27, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes #17197

I added a test using Free Serif, which has plenty of composite characters such as arrows drawn by rotating other arrows.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/next_api_changes/* if API changed in a backward-incompatible way

jkseppan mentioned this pull request Jul 27, 2020
Closed
jkseppan marked this pull request as ready for review July 27, 2020 13:36

jkseppan commented Jul 27, 2020

Copy link
Copy Markdown
Member Author

Interestingly, the test reveals that the Agg backend doesn't handle the font quite right.

jkseppan commented Jul 27, 2020

Copy link
Copy Markdown
Member Author

Correct output, screenshot from a PDF viewer.

jkseppan requested a review from QuLogic July 27, 2020 13:43
anntzer reviewed Jul 27, 2020
View reviewed changes
Comment thread setupext.py Outdated
jkseppan force-pushed the ttconv-scale branch 2 times, most recently from e1d4421 to 8293741 Compare July 27, 2020 16:08

anntzer commented Jul 27, 2020

Copy link
Copy Markdown
Contributor

I'll trust you on the ttf parsing, but otherwise the patch looks reasonable.

jkseppan closed this Jul 27, 2020
jkseppan deleted the ttconv-scale branch July 27, 2020 16:47
jkseppan restored the ttconv-scale branch July 27, 2020 16:47

jkseppan commented Jul 27, 2020

Copy link
Copy Markdown
Member Author

Oops, made a typo with a push and accidentally closed this.

jkseppan reopened this Jul 27, 2020

jkseppan commented Jul 27, 2020

Copy link
Copy Markdown
Member Author

If someone wants to check the code against documentation, composite glyphs are described at https://docs.microsoft.com/en-us/typography/opentype/spec/glyf#composite-glyph-description and 2.14 fixed-point format at https://docs.microsoft.com/en-us/typography/opentype/otspec181/otff#data-types (search for F2DOT14).

jkseppan requested a review from anntzer July 29, 2020 11:25
anntzer previously approved these changes Jul 29, 2020
View reviewed changes
QuLogic reviewed Aug 1, 2020
View reviewed changes
Comment thread extern/ttconv/pprdrv_tt.cpp Outdated
Comment thread extern/ttconv/pprdrv_tt.cpp
Comment thread extern/ttconv/truetype.h Outdated
QuLogic reviewed Aug 1, 2020
View reviewed changes
Comment thread lib/matplotlib/tests/test_ttconv.py Outdated

QuLogic commented Aug 1, 2020

Copy link
Copy Markdown
Member

Interestingly, the test reveals that the Agg backend doesn't handle the font quite right.

If I run outside the test framework, it appears to be okay?

jkseppan commented Aug 1, 2020

Copy link
Copy Markdown
Member Author

Interestingly, the test reveals that the Agg backend doesn't handle the font quite right.

If I run outside the test framework, it appears to be okay?

Yes, I couldn't reproduce this. It might have been some problem with an intermediate version, as I might have forgotten to compile the extension when running some test.

QuLogic commented Aug 1, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Interestingly, the test reveals that the Agg backend doesn't handle the font quite right.

If I run outside the test framework, it appears to be okay?

Yes, I couldn't reproduce this. It might have been some problem with an intermediate version, as I might have forgotten to compile the extension when running some test.

To clarify, I can reproduce if running in the test suite, by removing the explicit extension from the comparison settings.

This is because tests disable hinting with plt.rcParams['text.hinting'] = 'none'; if you run the script outside the test suite, but add that, then you get the weird streaks. Any setting other than 'force_autohint' or 'auto' (same thing to FreeType, latter is what we call our default) will do it, so it seems to be something wrong in the font hinting itself.

jkseppan commented Aug 1, 2020

Copy link
Copy Markdown
Member Author

The test failures are because of unrelated Pandas deprecations; apparently Pandas 1.1.0 is now available

QuLogic commented Aug 1, 2020

Copy link
Copy Markdown
Member

This is already fixed on master; you should rebase if you're having trouble with it.

jkseppan added 3 commits August 1, 2020 11:58
Support scale in ttf composite glyphs
00fe329 
Fixes matplotlib#17197
Add a test
be60f79 
The GNU Free Serif font has a wide variety of characters with which
we can exercise the linear transforms used in TTF composite glyphs.
Stay within the allowed line length
b9c956e
jkseppan added 2 commits August 1, 2020 11:58
Trim trailing zeroes in ttconv output
e78d502 
These could increase the file length in case of fonts with
many composite glyphs.
Make F2DOT14 a typedef of Fixed
d0779be
jkseppan mentioned this pull request Aug 1, 2020
Closed
jklymak requested a review from QuLogic August 1, 2020 17:45

QuLogic commented Aug 4, 2020

Copy link
Copy Markdown
Member

It looks like you may have accidentally reverted some things in that last rebase?

QuLogic added this to the v3.4.0 milestone Aug 4, 2020
jkseppan and others added 2 commits August 5, 2020 09:44
Simplify test decorator
f7799e3 
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Use sizeof instead of strlen+1
527c969 
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>

jkseppan commented Aug 5, 2020

Copy link
Copy Markdown
Member Author

It looks like you may have accidentally reverted some things in that last rebase?

You're right, it looks like I lost the suggestions committed via the Github web UI. I committed those again.

anntzer commented Aug 5, 2020

Copy link
Copy Markdown
Contributor

See #18181 for an alternate solution, though (which can reasonably avoid adding a new font file in the repo, and in any case seems more robust in the long term, as it can Type3-subset any font that FreeType can outline-decompose).

anntzer dismissed their stale review August 5, 2020 10:24

Still approving, but perhaps make a decision wrt #18181 first.

jkseppan closed this Aug 8, 2020
QuLogic mentioned this pull request Aug 14, 2020
Closed

QuLogic commented Oct 3, 2025

Copy link
Copy Markdown
Member

This branch was also in the main repo; I've now deleted it, as I assume that it's out of sync with this one on your fork that's been rebased.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

anntzer anntzer left review comments
QuLogic Awaiting requested review from QuLogic

Assignees

No one assigned

Labels

backend: pdf topic: text/fonts

Projects

None yet

Milestone

v3.4.0

Development

Successfully merging this pull request may close these issues.

Missing character upon savefig() with Free Serif font

3 participants

Back | FazBrowse Home | New Git URL