Add '--expand-features-to-instances' arg to fontmake UFO generation
UFOs can use relative include statements to import feature files, which is very helpful in the case of a family that has many UFO sources, but just one feature file required. Or, it’s helpful if there are many features, and several complex ones, so they are split into multiple files (example: a Recursive UFO source and its features directory).
Currently, the Builder just brings the include statement directly into the instance UFOs, but then the relative paths are broken, so the fonts don’t build.
fontmake: Error: Compiling UFO failed: <features>:1:8: The following feature file should be included but cannot be found: ./features/features.fea
However, it is very easy to support by adding the arg --expand-features-to-instances to the UFO instantiation step, as I have done in this PR.
I’ve tested this on a UFO-based family (with includes) and a Glyphs-based family (with normal Glyphs-based features), and it seems to work well in both cases. The features are built into the fonts, as expected.
I’m not aware of any downsides or hidden issues with adding this, but they might exist. If any are known, I would be very interested in learning about such cases.
Please let me know if you have any questions or suggestions here! Thanks so much.
Oh, I’ve also just realized that the current config option for this seems to activately prevent a build from working.
After I tested the above solution on a config that included expandFeaturesToInstances: true, the build failed with this:
[229/1410] buildTTF
FAILED: /var/folders/nb/mc9zt2n930vd_jkbg0kn2d680000gn/T/tmp8ertvdij
/Users/stephennixon/venv/bin/python -m gftools.builder.jobrunner fontmake --output-path /var/folders/nb/mc9zt2n930vd_jkbg0kn2d680000gn/T/tmp8ertvdij -o ttf -u sources/ufo/instance_ufos/FamilynameExtraCompressed-Hairline.ufo.json --filter ... --filter FlattenComponentsFilter --filter DecomposeTransformedComponentsFilter --expand-features-to-instances
Command failed:
fontmake --output-path /var/folders/nb/mc9zt2n930vd_jkbg0kn2d680000gn/T/tmp8ertvdij -o ttf -u sources/ufo/instance_ufos/FamilynameExtraCompressed-Hairline.ufo.json --filter ... --filter FlattenComponentsFilter --filter DecomposeTransformedComponentsFilter --expand-features-to-instances
usage: fontmake [-h] [--version] [-g GLYPHS | -u UFO [UFO ...] | -m
DESIGNSPACE] [--glyph-data GLYPHDATA] [-o FORMAT [FORMAT ...]]
# etc etc etc
fontmake: error: "--expand-features-to-instances" option invalid for UFO source
With that arg removed from the static font step, the build does complete.
~However, the next challenge is adding the config option to the instantiateUfo process. I need to figure out how to access the config at that stage (assuming I can, somehow... I may not yet properly understand the logic here).~
Okay! I’ve figured out how to access and pass that config option to the instantiateUfo step, and I’ve made it default to true, as is written in the builder docs. I’ve tested the config without the option, and with the option set to true and then set to false. It acted as expected, in each case. It inserts that arg into the UFO instantiation, making those features work in the final build:
▶ gftools builder 'Familyname/config.yaml'
[1/1410] instantiateUfo
fontmake -i Familyname Extra Compressed Extra Bold -o ufo -m sources/ufo/FamilynameRoman.designspace --ufo-structure=json --expand-features-to-instances --output-dir sources/ufo/instance_ufos
[2/1410] instantiateUfo
fontmake -i Familyname Extra Compressed Thin -o ufo -m sources/ufo/FamilynameRoman.designspace --ufo-structure=json --expand-features-to-instances --output-dir sources/ufo/instance_ufos
Thanks, @m4rc1e! I believe it’s now formatted as desired. It seems to need approval to run the workflow to verify that, though.
Okay, thanks for taking a look and for suggesting that update! Fair enough. I’ll try to make those adjustments soon.
@m4rc1e Oh wait, isn’t it already there in the schema? It’s also described in the docs. This PR merely makes it work as already described.
https://github.com/googlefonts/gftools/blob/d23ad2110b68b06327d6d8f81b4eb50f5912fa7e/Lib/gftools/builder/schema.py#L102-L104
https://github.com/googlefonts/gftools/blob/d23ad2110b68b06327d6d8f81b4eb50f5912fa7e/docs/gftools-builder/README.md#L170-L172
In the past, @anthrotype has suggested that this option should be the default:
We should make that the default...
The builder seems like a place it might make sense to do that, and that’s why I figured it was documented as such. Or, do you think it’s important to keep it non-default? I don’t think it would be a breaking chance to make it default, as I don’t think it blocks features that are already flattened into each source.