search

googlefonts / fontc

Where in we pursue oxidizing (context: https://github.com/googlefonts/oxidize) fontmake.
Apache License 2.0
62 stars 10 forks source link

[fontbe] Fix incorrect feature deltas w/ sparse glyph sources #779

Closed anthrotype closed 1 month ago

anthrotype commented 2 months ago

This fixes 'incorrect feature deltas when sparse' https://github.com/googlefonts/fontc/issues/407

The global StaticMetadata.variation_model now only includes 'full' masters (i.e. defining kerning) for both UFOs+DS and .glyphs input (previously this was the case only for the latter).

When we call resolve_variable_metric we make new sub-model when locations != global_model.locations(), or just reuse the global model when locations are equal -- which is currently basically always true for the common source formats (DS and glyphs) in the case of kerning, as the latter is meant to be "dense" and can only be specified for the "full" or globally-defined masters. However, we don't preclude the possibility that in the future other (or newer) source formats may define sparse kerning data, and our fontbe is ready for that.

I added a test file in #776, and verified locally that this patch fixes the issue #407, but I haven't find the time to make an actual unit test yet. If I don't make it in time, you should be able connect the dots...

EDIT: I have added a test case #817

anthrotype commented 1 month ago

the same test that failed in #817 passed with this PR branch

anthrotype commented 1 month ago

given that Colin already LGTM'ed this PR before I had added the test, I'll just go on and merge it.