Insert empty lookups for interpolation

[madig]

See https://github.com/fonttools/fonttools/pull/3073.

I need to think about what information to provide to feature writers again. Currently, I pass a dict of feature writer name to list of lookups it wrote to the writer, to mimic what's done on the default source. Problem: that might be too little information. Lookups also need to be registered in feature blocks, so I'd need to pass these in as well (is the lookup registered for kern or dist?). Maybe I need to approach this differently and instead walk the fea AST before it gets compiled, and insert new empty AST nodes? But then I again need to know precisely what to insert where. Or combine the appraoches to see which writer authored which lookup, and then walk the default AST and copy the nodes I need and clear out their statements, so I can copy them to the other sources if need be?

For information, we want to implementing the bottom part of this diagram:

[belluzj]

@madig From the variable name "defaultGeneratedLookups", I think you're not implementing the diagram but something else.

What it sounds like you're doing:

• generate the feature for the default master
• generate features for the other masters, knowing what the default yielded; while generating insert empty lookups for alignment

What you should be doing instead (= what the diagram shows):

• generate features for all masters, collect the generated lookups by all masters
• figure out the union of all the lookups generated by any of the masters
• go over the features of all masters again, and add empty lookups to align them to the union above

The advantage of doing it like in the diagram is that e.g. for kerning, a corner of the designspace could have more kerning than the default, e.g. only the Black needs to kern punctuation against numbers for whatever design reason that doesn't work otherwise in the Black.

To implement that, I think it might make sense that each feature writer has a new method called e.g. "alignLookups" or "postCompile", like so:

class BaseFeatureCompiler:
    def compile(self) -> Tuple[TTFont, GeneratedLookups]: # <-- added return type
        ... # as-is
        return self.ttFont, self.generatedLookups

    def postCompile(self, allLookups: GeneratedLookups) -> TTFont:
        # Insert blank lookups for anything that is in allLookups but isn't in self.generatedLookups
        return self.ttFont

and the main feature compilation loop can be something like:

for featureWriter in featureWriters:
    allLookups = GeneratedLookups()
    for master in masters:
         font, lookups = featureCompiler.compile()
         allLookups.union(lookups)
    allLookups.align() # Not sure how you're going to implement the type GeneratedLookups
    for master in masters:
         featureCompiler.postCompile(allLookups)

The advantage of having each feature writer do its own postCompile is that each feature writer will know best how and where to insert the empty lookups.

I'm not sure what the type should be for GeneratedLookups, let's discuss. Possibly each FeatureWriter could have its own GeneratedLookup type, e.g. for the kern-splitting you can identify a lookup by its target script and direction maybe, so that's the data that feature will use to identify and align its lookups.

[madig]

I assume that just inserting lookups is not enough, you have to wire them into the feature block. Actually, lemme test that.

[anthrotype]

the union of all the lookups generated by any of the masters

I'm not sure it's correct to take the union; the default master should have priority, just like we do with sparse glyph sets, in the sense that if something is not present in the default it should also be omitted in non-default; conversely, if something happens to be present in a non-default but is not in the default, it's as if it does not exist at all.

I assume that just inserting lookups is not enough

no that's not enough. varLib merge expects not only the LookupList to have same lenght, but also the ScriptList and FeatureList; when aadding a lookup you also need to register it under a given feature and script/langsys

[belluzj]

I'm not sure it's correct to take the union; the default master should have priority, just like we do with sparse glyph sets, in the sense that if something is not present in the default it should also be omitted in non-default; conversely, if something happens to be present in a non-default but is not in the default, it's as if it does not exist at all.

What about the argument that maybe the Regular doesn't need kerning but a corner of the designspace does? Does it mean that the Regular should have kerning for everything even if the values are 0? (because with the new kern splitter by script, if there's isn't kerning between two scripts, no lookup gets generated)

[anthrotype]

Fair enough. Then if we take the union of all the lookups generated by any masters, we'll have to assume zero default value for those pairs that are only present in a non-default master.

[madig]

To summarise the secret chat:

1. For each feature writer, collect the generated lookups (and their registration in the feature block) and possibly their contents (i.e. all contained pairs for the kern feature writer)
2. Go over the generated features again, and
   1. In the default source, insert lookups with, e.g. for the kern writer, the union of all pairs generated by other sources within the same lookup, ...
   2. In all other sources, insert just an empty lookup, ...
   3. ... such that all sources have the same lookups in the same order
   4. Hook the lookups in the feature blocks such that all feature blocks for all sources are the same (registration of script, language, lookup order)

[belluzj]

@anthrotype we figured with Nikolaus that we could add optional per-UFO preWrite() and global preMerge() steps to the FeatureWriters so that only the KernFeatureWriter really needs changing to implement these extra pre-steps, and the last step remains compile() for all FeatureWriters.

We added a sketch of the InterpolatableFeatureCompiler that drives the whole thing.

What do you think?

[belluzj]

If, having looked more into this, we find that it's too complicated to be worthwhile as a temporary fix until we have variable variable compilation, we may as well switch to productionizing variable feature compilation instead.

[madig]

Abandoning this as too hacky/complicated.