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

Missing .notdef in sparse masters still causing some trouble #501

Closed
justvanrossum opened this issue May 20, 2021 · 17 comments · Fixed by #773
Closed

Missing .notdef in sparse masters still causing some trouble #501

justvanrossum opened this issue May 20, 2021 · 17 comments · Fixed by #773

Comments

@justvanrossum
Copy link
Contributor

I see #381 and #387, but this problem still exists if the .notdef glyph otherwise participates in interpolation.

I understand that .notdef needs to be present in the TTF masters that are sent to varLib, and I'm not sure what the solution is. But it's a bit unfortunate that the input sources have to provide .notdef for all intermediate masters.

(I know this is nothing major, but perhaps at least worth noting/knowing.)

@anthrotype
Copy link
Member

did we not change varLib to interpret an empty (no contours) glyph from a non-default master (when the corresponding glyph in the default master is not empty) to "not participate"? I think you did that change.
Which means, we could have ufo2ft add an empty .notdef glyph to masters that don't have one, right? Pls send PR if that works

@justvanrossum
Copy link
Contributor Author

Yes, but that's only for the outline if I recall correctly: it would still create a HVAR intermediate, which is not desirable in this case. I'll double check that.

@justvanrossum
Copy link
Contributor Author

justvanrossum commented May 20, 2021

Indeed, the change you're referring to only affects gvar:

https://github.com/fonttools/fonttools/blob/0c4adad88d1eacbd5aa6d150ce70b39a0382b7a4/Lib/fontTools/varLib/__init__.py#L241-L247

There's a (potential) use case for building deltas for the advance width and not the outline. But perhaps that's silly, and the gvar behavior should be extended to HVAR.

@anthrotype
Copy link
Member

anthrotype commented Aug 1, 2023

I was just caught by this issue. There's a font (Rubik) that has a .notdef glyph with rounded corners (matching the overall design), and it uses some sparse layers (via Glyphs' brace layers) which are of course missing a .notdef glyph, thus ufo2ft uses the default master's .notdef for the sparse layers whose glyph set is missing a .notdef. However the default notdef contains some cubic curves and ufo2ft inserts it too late in the outlineCompiler, after the preProcessor (which among other things converts curves to quadratic) has finished, which means that the sparse master TTFs produced will have .notdef with cubic flags whereas the full masters have been preprocessed with cu2qu (the TTGlyphPoint pen silently passes those through, yeah... it's by design -- we only added an explicit check in fontBuilder); now yesterday I added a check in outlineCompiler to prevent cubic flags from sneaking in the font involuntarily (376b042) and this currently makes the Rubik's build fail because the sparse master TTFs' .notdef do contain cubic flags.

Ideally for sparse layers we want to have the .notdef "not participate" in the gvar while still be present (to ensure gid=0 is .notdef) and I can make that change by inserting an empty glyph instead of a copy of the default master's .notdef glyph (as we do now), but it will still participate in the HVAR computation in varLib, which may also be undesirable, as @justvanrossum noted above.

I think we can change varLib to treat the .notdef as "sparse" (omit from VF interpolation) when: 1) its outlines are empty (like we do for gvar), 2) and its advance and left sidebearing are both 0xFFFF (which is unlikely/impossible? for a real world font).

WDYT?

@anthrotype
Copy link
Member

anthrotype commented Aug 1, 2023

treat the .notdef as "sparse" (omit from VF HVAR interpolation) when: 1) its outlines are empty (like we do for gvar), 2) and its advance and left sidebearing are both 0xFFFF (which is unlikely/impossible? for a real world font).

@behdad WDYT of this for enabling support for sparse hmtx/HVAR?

@behdad
Copy link
Collaborator

behdad commented Aug 1, 2023

treat the .notdef as "sparse" (omit from VF HVAR interpolation) when: 1) its outlines are empty (like we do for gvar), 2) and its advance and left sidebearing are both 0xFFFF (which is unlikely/impossible? for a real world font).

@behdad WDYT of this for enabling support for sparse hmtx/HVAR?

I'm fine with that. Or let's go to 0x10000 so it fails to serialize if accidentally leaks?

@behdad
Copy link
Collaborator

behdad commented Aug 1, 2023

treat the .notdef as "sparse" (omit from VF HVAR interpolation) when: 1) its outlines are empty (like we do for gvar), 2) and its advance and left sidebearing are both 0xFFFF (which is unlikely/impossible? for a real world font).

@behdad WDYT of this for enabling support for sparse hmtx/HVAR?

I'm fine with that. Or let's go to 0x10000 so it fails to serialize if accidentally leaks?

Oh. We're going the binary. Nevermind.

For advance it should be 0xFFFF; sidebearing is signed, so 0xFFFF is -1. For that, take either 0x7FFF or 0x8000.

@anthrotype
Copy link
Member

i'd be fine with that but sometimes you actually want to serialize the master ttfs to disk.. -o ttf-interpolatable does that

@behdad
Copy link
Collaborator

behdad commented Aug 1, 2023

Maybe 0x8000 for both?

@anthrotype
Copy link
Member

i think I prefer 0xFFFF for advance and 0x8000 for lsb, to make the chance of this occurring even more remote. I think it's very unlikely that these values would ever occur intentionally in a font source, not just because they are huge (64 or 32 times the average UPEM) but also because an empty glyph such as these ones usually would have a left sidebearing of 0

@behdad
Copy link
Collaborator

behdad commented Aug 1, 2023

Fine with me. Do note that some versions of the OT spec before had advance as a signed value. At any rate, this is fine.

@anthrotype
Copy link
Member

anthrotype commented Aug 1, 2023

do we want to make the metrics sparseness (advance/lsb set to sentinels) conditions be independent of the outline sparseness (i.e. empty) condition? Is it ever desirable to have the outlines interpolate without the glyph horizontal/vertical metrics or viceversa?

@behdad
Copy link
Collaborator

behdad commented Aug 1, 2023

That sounds rare. Do what feels natural in the code.

@behdad
Copy link
Collaborator

behdad commented Aug 1, 2023

We don't generate lsb deltas, so that one is moot.

Even filtering out advance of zero might be fine.

@anthrotype
Copy link
Member

Even filtering out advance of zero might be fine.

but those might potentially be set like that with the intention of participating in the interpolation, e.g. advance that is 0 only in some non-default locations of the variation space

@justvanrossum
Copy link
Contributor Author

justvanrossum commented Aug 2, 2023

I just thought of an alternative solution. Since the problem is that the intermediate otf/ttf master must contain .notdef, how about treating .notdef as a regular glyph, and temporarily rename it (say, something like, __notdef_fontmake_temp__), and rename it back upon merging the final VF? Then .notdef won't need special treatment for sparse master.

(And use an empty placeholder .notdef to satisfy the constraint)

@anthrotype
Copy link
Member

but the change I'm proposing in varLib is not just for .notdef, it would allow to define sparse hmtx/vmtx for any glyph that you may want to include in the sparse master for other reasons. e.g. it's a base glyph used in a composite glyph where one wants to variate the component offsets without needing to add extra intermediate master for the base glyphs as well; or the glyph may even be referenced in GPOS rules

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