[builder] add support for "Replace Feature" custom parameter

[anthrotype]

Second take
#222

/cc @punchcutter

[belluzj]

I think the middle (.*) should be made non-greedy to avoid consuming an unrelated feature, e.g. if the following is possible:

feature liga {
} liga;

feature calt {
} calt;

feature liga {
} liga;

(I don't know if it is possible)

[anthrotype]

good point, but also bear in mind this is not meant to work with arbitrary UFO features.fea, but only those that where composed by glyphsLib ifself by concatenating the GSFont's features.

is it possible from the Glyphs.app's features panel to add multiple features with the same tag? I can't try right now.
If so, we need to support such case.

[anthrotype]

also, it's not clear what the "Replace Feature" custom parameter should do in this case.
I made it only replace the first occurrence of a feature block with a given tag (count=1).

[belluzj]

Just to make sure that I understand, is this a master or instance parameter?

[anthrotype]

also good question. the set_custom_parameter function is generic (by design) and doesn't know whether it's been called for setting font's, master's or instance's custom parameters.

I expect this "Replace Feature" custom parameter to be useful only for instances. But I don't mind if it happens to work for masters as well.

We don't check anywhere whether a custom parameter is valid in the context of font, masters or instances.

[belluzj]

I agree a bit with @schriftgestalt's comment on #222 that the regex could be avoided if the custom parameter was applied using the GSFeature objects. However with the current code, that would only be possible if the customParameter is on the master, right?

I have a slightly unrelated question for the work on the round-trip: I saw that the code in interpolation.py does not only rely on MutatorMath to copy data from the sources to the instances, but does it itself. Is this in order to implement this kind of customParameters, that are Glyphs-specific? From a higher level, is it "the plan" to always have glyphsLib handle the generation of UFO masters + UFO instances, or would it be possible to only generate UFO masters + designspace and have another designspace tool generate the UFO instances? As far as I know, the designspace document cannot specify tricky things like replacing a feature in a specific instance, like this custom parameter can do if it is applied to an instance.

?

I will try to add several features with the same tag in Glyphs.

[anthrotype]

We can't apply it using GSFeature objects because by the time we do the replacement the instances are UFOs, interpolated using MutatorMath.

The instance UFOs features are copied by MutatorMath from the base master UFO features, and they are the same as the global GSFont's features.

I saw that the code in interpolation.py does not only rely on MutatorMath to copy data from the sources to the instances, but does it itself

do you mean, the apply_instance_data function? Yes, all the things that are Glyphs.app-specific like the custom parameters have to be applied to the generated UFOs after MutatorMath has completed. Even if we have a different designspace tool to generate the UFO instances, we will always need to fix up the generated UFOs with Glyphs.app-only instance data.

We need this in to build some Indic Noto fonts, where the global features contain dist feature with some contextual rules which apply to the Regular instance only, then the other instances have "Replace Feature" custom parameter that overrides the default dist with one that applies for that particular instance.

I believe regex does the job for now. We can always refine later.

[anthrotype]

the designspace document cannot specify tricky things like replacing a feature in a specific instance, like this custom parameter can do

yes, that's why we need to add support for having a lib element in designspace document, where we an store private data to disk, and apply it at a later time. Currently we are holding this in memory while mutatormath runs and then passing it to apply_instance_data after it completes.

[schriftgestalt]

‘Replace Feature’ parameters are useful only in instances.

[anthrotype]

‘Replace Feature’ parameters are useful only in instances.

yes, exactly my point

[belluzj]

add support for having a lib element in designspace document

By the way, I have started using designSpaceDocument in my roundtrip branch (by copy-pasting the code) and I added a quick-and-dirty lib element to designspace instances, in which I will probably store this kind of custom parameters when I start testing on the Noto fonts. I have also opened the PR in fonttools to migrate the code.

[anthrotype]

I have also opened the PR in fonttools to migrate the code.

yes, thanks very much for that. I'll review it soon 👍