Smart components

[simoncozens]

Fixes #91. Does not round trip!

[simoncozens]

Hmm, the smart component tests work but now other tests are failing. Bah.

[simoncozens]

Urgh. This works with the tests but not real life files. The decomposed layers need to be positioned. It is also very slow and (with all the decomposition warnings) quite noisy.

[simoncozens]

OK, I'm done here and my fonts work.

[simoncozens]

Regression test fails are mainly because we now decompose smart components instead of just treating them as dumb components, plus something that looks like an incompatible path.

[anthrotype]

LGTM! Thanks so much Simon for implementing this, it's been on top of the feature requests for literally years...

I left some comments.

[khaledhosny]

Glyphs’ smart components support extrapolation, is it supported in this PR as well?

[anthrotype]

Glyphs’ smart components support extrapolation, is it supported in this PR as well?

maybe it's just a matter of setting extrapolate=True when constructing VariationModel..

[anthrotype]

I wonder if we should support round-tripping smart components as such (i.e. without instantiating them to "regular" components) when going through glyphs=>UFO=>glyphs. Maybe we should enable the smart component instantiation when minimal=True or minimize glypsh diffs=False?

[simoncozens]

I wonder if we should support round-tripping smart components as such (i.e. without instantiating them to "regular" components) when going through glyphs=>UFO=>glyphs. Maybe we should enable the smart component instantiation when minimal=True or minimize glypsh diffs=False?

I think this is a good idea, but for later - once we've tidied up the minimal/minimize_glyphs_diffs mess. Perhaps the answer is to replace all those flags with a for_compilation flag (we can bikeshed the name) - I think that's the cleanest way of signalling that we don't want a complete representation of the font, we just want the data required to produce a TTF.

[anthrotype]

for VariationModel(extrapolate=True) you want at least fonttools >= 4.37, you may as well bump to current latest both setup.py and requirements.txt

[anthrotype]

extrapolate=True

note that the current implementation has some limitations fonttools/fonttools#2843 which are being addressed by fonttools/fonttools#2846 (unreleased)

[anthrotype]

maybe we could first release a new fonttools with that extrapolation patch, and then bump the requirements in here before merging and releasing glyphsLib

[anthrotype]

@simoncozens could we add a test that extrapolation works as intended? I know the CI will fail until fonttools gets updated but it'd still good to verify that we do something equivalent to what Glyphs.app would produce with the same data.

[justvanrossum]

Regarding future round-tripping, it would be nice to converge to something along these lines: https://github.com/BlackFoundryCom/variable-components-in-ufo.

[simoncozens]

Regarding future round-tripping, it would be nice to converge to something along these lines:
https://github.com/BlackFoundryCom/variable-components-in-ufo.

Oh, hrm. If that's going to be the future, then ufo2ft needs to handle it. Which means all this decomposition code needs to move there instead of here.

[justvanrossum]

Could be optional. Also relevant to this WIP: https://github.com/harfbuzz/boring-expansion-spec/blob/glyf1/glyf1.md

[simoncozens]

Well, with glyf1, we're talking about relatively far future. :-)

I would prefer to stick with what we have for now and not make the perfect the enemy of the good, especially since it looks like the UFO stuff isn't quite defined/standardized yet. We can move it around when all those things are sorted out.

[anthrotype]

i'm ok to merge this first, try it out a bit on real world fonts, and then later we work on translating between Glyphs.app and this proposed variable ufo components and implmenent that in ufo2ft.

[anthrotype]

but thanks @justvanrossum for reminding us of that. Maybe we should file an issue somewhere to track support for these variable components in the UFO build pipeline

[simoncozens]

OK. Will work on the extrapolation stuff if you could cut a fonttools release.

[justvanrossum]

I don't suggest to change anything now, I'd just like you to know that the UFO stuff is being worked in, just in case round-tripping is at some point desirable. Or just conversion of .glyphs to ufo+variable-components, which I may want for prototype-ish reasons sooner than glyf1 will be a viable format.

[simoncozens]

Actually before you do the fonttools release I would need this:

diff --git a/Lib/fontTools/varLib/models.py b/Lib/fontTools/varLib/models.py
index a7e020b00..db7c98008 100644
--- a/Lib/fontTools/varLib/models.py
+++ b/Lib/fontTools/varLib/models.py
@@ -43,7 +43,7 @@ def subList(truth, lst):
     return [l for l, t in zip(lst, truth) if t]


-def normalizeValue(v, triple):
+def normalizeValue(v, triple, extrapolate=False):
     """Normalizes value based on a min/default/max triple.

       >>> normalizeValue(400, (100, 400, 900))
@@ -59,7 +59,8 @@ def normalizeValue(v, triple):
             f"Invalid axis values, must be minimum, default, maximum: "
             f"{lower:3.3f}, {default:3.3f}, {upper:3.3f}"
         )
-    v = max(min(v, upper), lower)
+    if not extrapolate:
+        v = max(min(v, upper), lower)
     if v == default:
         v = 0.0
     elif v < default:

[simoncozens]

Possibly. Or maybe with the extrapolation stuff in VariationModel there's a smart way to do it. This is new to me and it's not clear to me how it works out what the default is without normalised locations.

[simoncozens]

@anthrotype Please check my requirements-dev fiddling before merging; it seems like if we don't pin the version there, we get a conflict between the requirements.txt and requirements-dev.txt.