New instantiator

[madig]

Build on top of the test rework to add an instantiator to (eventually) replace MutatorMath.

Todo:

• Make it user-selectable to switch between MutatorMath and the new instantiator.
• add handling of slnt axis (like for wght and wdth) which translates 1:1 to post.italicAngle, with clamping.
• Check that data copied to instances from the Instantiator object is independent (don't repeat the hours of fun we had with the ghost kerning bug in varLib.merger)
• How to handle glyphs blacklisted by public.skipExportGlyphs that do not interpolate? Write out as empty outlines and warn, but don't error out.

Deferred:

• Do we test for rounding of anchor and component coordinates? (I think this is more a test of fontMath, so no)
• More elegant handling of UFO filename generation for the Instantiator code path. Refactor Designspace loading to fill in instance filepaths? Needs access to global computed paths, etc.
• Switch from defcon to ufoLib2 completely

[behdad]

This is great, thanks! Why is it in fontmake though, shouldn't it be in ufo2ft or somewhere lower?

[madig]

@anthrotype and me want to test drive it in fontmake first (I use it for Cantarell already) and maintaining an external lib is extra admin. Fontmake should be the ideal temporary home, as it is a self-contained app hat should not be used as a library.

The thinking is, we want to merge ufoLib2 into fontTools.ufoLib.objects one day, reimplement fontMath on those objects and then we figure out where to stick this code. I think.

[madig]

I need some more tests that test rounding on vs. off (maybe even against cutting an instance from a VF to ensure that static instances and VF instances are as close as possible if not identical?) and rules. Anything else? How to test axis mappings more?

Also:

• Support SourceDescriptor.mutedGlyphNames?
• Support SourceDescriptor.muteKerning?
• What to do with a glyph's lib? I currently drop it because there's no useful interpolating data afaik, but maybe I'm wrong? What about public.verticalOrigin and public.postscript.hints? fontMath doesn't interpolate that.

Edit: just realized that maybe the skipping of empty source glyphs exists to support mutedGlyphNames?

[anthrotype]

this is not a full review

[anthrotype]

about those "muted" attributes for source kerning, info and glyphs, what are they used for? I don't know how widely used they are in the wild, but varLib doesn't read them when generating varfonts.

[anthrotype]

What about public.verticalOrigin and public.postscript.hints?

maybe once public.verticalOrigin is upgraded to a proper item in the next version of UFO, we can add support for that too
unified-font-object/ufo-spec#95

Not sure about postscript hints. Were they expected to be interpolated in the mutatormath workflow? Can be done later, I guess

[madig]

(Idea to self: fill in PANOSE weight field from wght axis maybe)

Edit: the weights listed in https://monotype.github.io/panose/pan2.htm#_Toc380547249 differ from the weight class, so there seems to be no 1:1 correlation of OS/2 weight class and PANOSE weight, so maybe don't do it.

[madig]

Regarding "muted" attributes, shall we remove them to match what varLib does? Makes sense from that standpoint. Plus, they can be added back in I guess.

[madig]

I somehow managed to kill instantiator.py while rebasing. Hope I didn't loose anything.

[madig]

I am wondering: swapGlyphNames works by, well, swapping glyphs and some metadata elsewhere.

1. Should we move instead of swap? I.e. when a rule says S -> S.closed, should the content of S.closed be moved into S and the original S.closed be deleted instead of swapping the glyph? I imagine that outline compatibility after swapping isn't guaranteed anymore anyway.

2. What about feature files?

[madig]

Not sure I missed something by reducing the component remapping to one loop.

[madig]

I think I only need some rounding tests now. Or is there anything else missing?

Edit: oh yeah, refactor generate_instance some to reduce complexity, e.g. by breaking it up?

[madig]

Do we need more sanity checking in the Instantiator constructor? Do it in fontmake instead? As in:

1. The designspace must be rectangular or whatever the term is
2. Groups in all masters must be the same
3. Glyphs must be compatible (outlines, components, anchors). fontMath does not guarantee to blow up on anchor errors iirc.
4. Anything else?

[madig]

The following code

def test_interpolation_against_vf(data_dir):
    designspace = designspaceLib.DesignSpaceDocument.fromfile(
        data_dir / "MutatorSans" / "MutatorSans-weight-only.designspace"
    )
    designspace.rules = []
    designspace.loadSourceFonts(ufoLib2.Font.open)
    designspace.instances[0].location = {"weight": 123.456}

    # Generate VF and VF instance.
    vf_designspace = ufo2ft.compileInterpolatableTTFsFromDS(designspace, inplace=False)
    varfont, _, _ = fontTools.varLib.build(vf_designspace)
    varinstance = fontTools.varLib.mutator.instantiateVariableFont(
        varfont, {"wght": 123.456}
    )

    # varfont.save("/tmp/vf.ttf")
    varinstance.save("/tmp/vfinstance.ttf")

    # Generate instance.
    generator = fontmake.instantiator.Instantiator.from_designspace(
        designspace, round_geometry=True
    )
    instance_ufo = generator.generate_instance(designspace.instances[0])
    instance = ufo2ft.compileTTF(instance_ufo, removeOverlaps=False)

    instance.save("/tmp/instance.ttf")

Does not write identical TTFs. Some diffs in e.g. head and some other misc. tables. The glyf table contains some off-by-off rounding diffs, but mostly diffs seemingly related to cu2qu conversion (more points in curvy glyphs), additional overlap="1" point attributes in the VF-cut instance and different component flags. At least the kerning values seem the same. Maybe cutting an instance just isn't the same.

Edit: although... setting or commenting out fontMath.mathFunctions.setRoundIntegerFunction(fontTools.misc.fixedTools.otRound) in instantiator.py seems to make no difference. Maybe this isn't working the way I think it should?

[madig]

Todo: add handling of slnt axis which translates 1:1 to post.italicAngle, with clamping.

[madig]

TestSubset.glyphs or glyphsLib's handling of it is broken. The axis definition comes out as

<axes>
    <axis tag="wght" name="Weight" minimum="400" maximum="400" default="400">
      <map input="400" output="700"/>
    </axis>
  </axes>

Which trips varLib.

[anthrotype]

what do you mean "not rely on instance filenames"? how does fontmake know where to save the instances if requested to generate them?

[madig]

My concern is what happens if filename is not specified.

[madig]

Note to self: explore Python libraries like Dask and https://pypi.org/project/joblib/ to generate instances in parallel for maximum room heating.

[madig]

Interesting performance benefit of Instantiator: Fontmaking NotoSansHebrew-MM.designspace instances takes ~60 seconds on my Ryzen 1700, doing it with Instantiator takes ~20 seconds. Don't know why, but I like it.

[madig]

Deepcopying the default source's lib is much slower than iterating over it and assigning key and value to the new dict, but I suppose we have to to make the data independent?

[madig]

Designspace loading needs to be refactored. I'd like to sanity-check instance filenames so the code doesn't overwrite sources with instances because the filenames are the same (this has happened to me before) and make sure all instances actually have filenames so I don't have to make one up on the spot when saving instances. Fail fast and early. The _load_designspace_sources static method looks like the right place, except it doesn't know the instance_dir.

[madig]

Idea: with the optimization detailed above, the -M switch could be used to apply rules to masters that can't interpolate yet. We currently have a project that is still heavily worked on where interpolation isn't feasible yet, but that has a bracket layer for dollar. The team would like to generate the static masters with the dollar substitution rule applied so they don't have to resort to cmap hacks.

[madig]

Idea: pare down core MutatorMath code and bundle it up for use in instantiator.py, so users can use either varLib or MutatorMath math, but the UFO assembly code stays the same. This would make it possible to support sparse masters/source layers for MutatorMath.

[madig]

Hm. Copying things is tricky. If the Designspace object passed into Instantiator.from_designspace has no loaded fonts, we don't need to copy the fonts because we load them ourselves anyway; if there are loaded fonts, we need to deepcopy everything everywhere because a cursory glance at fontMath suggests it is not rigorous about maintaining independent copies of the data it handles. Maybe deepcopy the passed in Designspace object and then call loadSourceFonts?

Also, to be fully immutable, we need to turn all dicts and lists we see into types.MappingProxyType and tuple... go to that length? I'd leave it trying to be independent from the passed in Designspace object.

Edit: decided that we don't care about independence from the input, but the generated output should be independent.

[anthrotype]

haven't finished reviewing, but looking good so far, will continue later or tomorrow

[anthrotype]

added some more comments

[anthrotype]

nice work @madig 👏