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

incorrect glyph variation deltas for sparse masters #400

Closed
anthrotype opened this issue Aug 15, 2023 · 6 comments · Fixed by #403
Closed

incorrect glyph variation deltas for sparse masters #400

anthrotype opened this issue Aug 15, 2023 · 6 comments · Fixed by #403
Assignees

Comments

@anthrotype
Copy link
Member

anthrotype commented Aug 15, 2023

fontc currently creates a single VariationModel that is initialized with the union of all the glyph locations and then uses this same model for computing the deltas of each glyph. But sparse glyphs may not contain sources for all the locations, and in those cases we need to create a sub-model like fontTools.varLib does, where the locations and regions that do not apply are omitted and the min/max partition and weights of the remaining regions are recomputed for that particular glyph's constellation of locations.
If we don't do that, we may end up associating the wrong region min/max to given glyph's locations, leading to unexpected interpolations that look weird and do not match fontTools.
Besides, fontTools keeps a caches of these submodels since it's common for several glyphs to define sources for the same set of locations, maybe we should do the same as well.

@anthrotype anthrotype self-assigned this Aug 15, 2023
@rsheeter
Copy link
Contributor

rsheeter commented Aug 15, 2023

Lets try to get a minimal reproduction. I did specifically intend to avoid the submodel business when porting.

@behdad
Copy link

behdad commented Aug 16, 2023

Lets try to get a minimal reproduction. I did specifically intend to avoid the submodel business when porting.

lol. There's a reason submodel business is there. :)

Here's a simple example: A font with Regular, Bold, and Black masters at 400, 700, 1000. In this model the support for 1000 master starts going up from 700. However, if a glyph is missing at 700, then the support for 1000 should start going up at 400. Otherwise you get no emboldening between 400 and 700.

@behdad
Copy link

behdad commented Aug 16, 2023

Here's a simple example: A font with Regular, Bold, and Black masters at 400, 700, 1000. In this model the support for 1000 master starts going up from 700. However, if a glyph is missing at 700, then the support for 1000 should start going up at 400. Otherwise you get no emboldening between 400 and 700.

A way to see this:

% ./fonttools varLib.models -l 0 .5 1
Sorted locations:
[{}, {'A': 0.5}, {'A': 1.0}]
Supports:
[{}, {'A': (0, 0.5, 1.0)}, {'A': (0.5, 1.0, 1.0)}]

% ./fonttools varLib.models -l 0 1
Sorted locations:
[{}, {'A': 1.0}]
Supports:
[{}, {'A': (0, 1.0, 1.0)}]

@behdad
Copy link

behdad commented Aug 16, 2023

You can also visualize that:

% ./fonttools varLib.plot 0 1 .5
% ./fonttools varLib.plot 0 1

@anthrotype
Copy link
Member Author

I made a test variable font with a single weight axis between 400 (default) and 900, and with only two glyphs "I" and "i". The "I" only has two masters 400 and 900, whereas the "i" has three masters 400, 700 and 900. I deformed the 700 intermediate master to make it more visible when transitioning through it with the sliders.
The designspace + UFOs are in this zip file (I drew the font with Glyphs.app and exported to DS+ufo with fontmake):
NewFont.designspace.zip

You can see from the video below how in the first row that was built with fontc from main branch, the "I" does not change between 400 and 700 but only starts to variate from 700 to 900, whereas the "i" gets variated correctly throughout.
The following rows contains the same font built respectively with my PR #403 and fontmake, which match and display the expected behavior with the "I" variating from 400 to 900, not just from 700 onwards.

Screen.Recording.2023-08-17.at.13.32.55.mov

It's also clear from the ttx diff that the fontc-main built font has an incorrect tent for wght axis, instead of (0.0, 1.0, 1.0) it has (0.6, 1.0, 1.0):

ttx -q -o - -t gvar "/Users/clupo/Desktop/NewFontVF-fontmake.ttf" 2>/dev/null
ttx -q -o - -t gvar "build/NewFontVF-fontc-main.ttf" 2>/dev/null
--- /dev/fd/63  2023-08-17 13:34:17
+++ /dev/fd/62  2023-08-17 13:34:17
@@ -6,7 +6,7 @@
     <reserved value="0"/>
     <glyphVariations glyph="I">
       <tuple>
-        <coord axis="wght" value="1.0"/>
+        <coord axis="wght" min="0.6" value="1.0" max="1.0"/>
         <delta pt="1" x="-60" y="0"/>
         <delta pt="3" x="60" y="0"/>
       </tuple>

I'll add this to my PR as a test.

@anthrotype
Copy link
Member Author

A reason we had not realised this maybe is because for .glyphs input, fontc currently cannot handle intermediate glyph layers (aka "brace" layers) yet: #62
So for .glyphs source we currently ignore these intermediate layers (that would require a specific model), and build the VF with a single model for all glyphs (which by definition contain one layer per "master").
Whereas for .designspace input, fontc does support loading glyphs from sparse UFO layers (e.g. designspace source with layer attribute) or more generally from UFO masters containing different glyph sets.

anthrotype added a commit that referenced this issue Aug 17, 2023
Intended to reproduce the issue described at #400, whereby we currently use the same VariationModel for all glyphs whether or not they contain sources for all the locations or only some, leading to incorrect region definitions
anthrotype added a commit that referenced this issue Aug 17, 2023
Intended to reproduce the issue described at #400, whereby we currently use the same VariationModel for all glyphs whether or not they contain sources for all the locations or only some, leading to incorrect region definitions
anthrotype added a commit that referenced this issue Aug 17, 2023
Intended to reproduce the issue described at #400, whereby we currently use the same VariationModel for all glyphs whether or not they contain sources for all the locations or only some, leading to incorrect region definitions
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 a pull request may close this issue.

3 participants