gftools icon indicating copy to clipboard operation
gftools copied to clipboard

[builder] Calling UFOBuilder directly doesn't set minimal=True and thus prevents color layer processing

Open yanone opened this issue 2 years ago • 5 comments

Since the ninja builder came about, gftools relies on its own routine to generate designspace objects from Glyphs sources, rather than having fontmake do that.

fontmake however calls UFOBuilder() with minimal=True, which the ninja builder doesn't, and the effect of that is that

  1. color layers aren't processed, which rely on minimal=True
  2. possibly the UFO builder is overall slower than it should for one-way font generation

So minimal=True needs to be set explicitly (which is set to False by default) to ensure correct color layer handling in glyphsLib.

I'll attach a PR in a second.

yanone avatar Sep 29 '23 13:09 yanone

actually, the gftools ninja builder is still calling fontmake -o ufo -g input.glyphs command to convert from .glyphs to DS+ufo (see the glyphs2ufo ninja build rule). It seems to be using the UFOBuilder just to get a minimal designspace file in order to obtain the paths of the master UFOs that will be generated when the glyphs2ufo build rule is actually run..

@simoncozens can you change glyphsLib to add APIs to obtain these pieces of info without you having to hack into the bowels of UFOBuilder (e.g. like overriding methods with lambda: True to make them noop, etc.)?

anthrotype avatar Sep 29 '23 14:09 anthrotype

can you change glyphsLib to add APIs to obtain these pieces of info without you having to hack into the bowels of UFOBuild

No, I don't think I can. For example, if we skip the line

            list(self.masters)  # Make sure that the UFOs are built

then to_designspace_sources fails because self._sources is not filled in; that happens in to_ufo_font_attributes. glyphsLib.builders really isn't designed to work in a compartmentalised way, and I don't really think it's worth the effort of completely rearchitecturing it right now. I think under the circumstances hacking into the bowels of overriding methods is the least bad approach.

simoncozens avatar Oct 01 '23 08:10 simoncozens

color layers aren't processed, which rely on minimal=True

That's kind of weird. minimal=True should do less work than minimal=False.

simoncozens avatar Oct 12 '23 11:10 simoncozens

color layers aren't processed, which rely on minimal=True

That's kind of weird. minimal=True should do less work than minimal=False.

In case of color fonts, we are piggybacking to mean give me a UFO I can build with ufo2ft and get color tables, since UFO officially has no support whatsoever for color fonts and we are using private ufo2ft lib keys that (I thought) would not be appropriate for a general purpose UFO.

khaledhosny avatar Oct 15 '23 19:10 khaledhosny

perhaps it is mis-named, but minimal=True option in glyphsLib effectively means "get me UFOs+DS suitable for building an OpenType font without worrying about saving additional metadata that's not relevant for compilation but only for roundtripping back to .glyphs". Or conversely, minimal=False means "try to preserve as much as you can of the original source file so that I can do some edits elsewhere and then convert it back to the original format". Thus, color layers don't get translated to ufo2ft-compatible format in minimal=False mode

anthrotype avatar Oct 16 '23 08:10 anthrotype