Skip to content
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

don't drop implied oncurves for interpolatable TTFs when building a VF #817

Merged
merged 3 commits into from
Feb 6, 2024

Conversation

anthrotype
Copy link
Member

this should only be done at the end in the final VF, otherwise masters may end up with incompatible glyphs...

@anthrotype
Copy link
Member Author

anthrotype commented Feb 6, 2024

Added a test with a reproducer, which fails as expected.

https://github.com/googlefonts/ufo2ft/actions/runs/7804773235/job/21287495551?pr=817#step:5:217

WARNING  fontTools.varLib:__init__.py:1069 Failed to drop implied oncurves for 'o': Incompatible flags for glyph at master index 1: expected array('B', [1, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0]), found array('B', [1, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0])
INFO     fontTools.varLib:__init__.py:1170 Dropped 0 on-curve points from simple glyphs in the 'glyf' table
[...]
WARNING  fontTools.varLib:__init__.py:335 glyph o has incompatible masters; skipping
``

this should only be done at the end in the final VF, otherwise masters end up with incompatible glyphs...
@behdad
Copy link
Collaborator

behdad commented Feb 6, 2024

Does varLib do it?

@anthrotype
Copy link
Member Author

this issue might have been noticed earlier if varLib errored instead of just warning: fonttools/fonttools#2572

@anthrotype
Copy link
Member Author

Does varLib do it?

yes, we do it either in the TTGlyphPen (for the static fonts) or in varLib (for the variable ones)

@anthrotype
Copy link
Member Author

anthrotype commented Feb 6, 2024

this is a regression from #801 when we moved things around. The previous code was pop'ing the the dropImpliedOnCurves parameter from the kwargs before making the outline compiler for the VF's interpolatable masters and then passing it on to varLib.build

see

dropImpliedOnCurves = kwargs.pop("dropImpliedOnCurves")

@anthrotype anthrotype merged commit 0a6868f into main Feb 6, 2024
9 checks passed
@anthrotype anthrotype deleted the fix-drop-implied-on-curves branch February 6, 2024 19:13
anthrotype added a commit to googlefonts/fontmake that referenced this pull request Feb 6, 2024
Fixes a bug when --drop-implied-oncurves option was set
googlefonts/ufo2ft#817
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.

2 participants