New problems with duplicate Unicode for color glyphs

[yanone]

The duplicate Unicode issue still appears for Blaka Ink, a COLRv1 font with gradients.

Observations:

• The tests that were recently added are clearly insufficient in catching this.
• I'm not 100% sure yet, but I believe that the unicodes must be muted in two places: here in addition to here.
• This condition as triggered from here currently prevents the processing of color layers for my font (minimal=False).

I can't easily fix this because I don't understand the minimal architecture. But I'm happy to help with a PR if I'm told what to do.

[yanone]

I would start by adding

    color0.attributes["color"] = 1
    color1.attributes["color"] = 1

to the new test here.

It already starts throwing other errors before even checking for the unicodes.

[anthrotype]

clearly insufficient

well, not clearly enough to me by just reading this issue. Could you provide steps to reproduce the issue?

[anthrotype]

I don't understand the minimal architecture.

fontmake sets minimal=True because it's only interested in the minimal pieces of data required for compilation, not caring about other metadata desirable when roundtripping Glyphs<=>UFO

[yanone]

Take Blaka Ink (linked at the top), and try to generate a font from it.

It stalls with fontmake: Error: In 'instance_ufo/BlakaInk-Regular.ufo': Compiling UFO failed: cannot map 'A.color0' to U+0041; already mapped to 'A', which is the same issue of https://github.com/googlefonts/glyphsLib/issues/937 still persisting.

I already pulled out all the relevant points in the code where something could go wrong.

[anthrotype]

https://github.com/googlefonts/glyphsLib/pull/939 was supposed to fix an issue with COLRv0 ("color palettes" fonts as Glyphs.app calls them). So you're saying the duplicate unicode issue also affects COLRv1, good. Will take a look

[yanone]

Then the test should be duplicated, once for COLRv0 and once for COLRv1. But given the absence of processing of any type of color glyphs through the minimal switch, even the COLRv0 must fail in real world examples, I believe.

[anthrotype]

Then the test should be duplicated, once for COLRv0 and once for COLRv1

sounds good, can you work on a PR?

You're correct, we need to set unicodes to empty list right after we call to_ufo_glyph() to populate the COLRv1 layer glyph

[anthrotype]

I don't know what you mean with your references to minimal={True,False}, fontmake sets that correctly to True so the color layers are processed. Are you doing something else?

[yanone]

Are you doing something else?

Idk, I'm using gftools builder. Maybe it sets the variable. Let me investigate this.

I'll start a PR after lunch.

[yanone]

I don't understand this.

gftools builder doesn't set minimal to False anywhere, and in fact fontmake doesn't even have an interface for that. It calls glyphsLib.to_designspace() with minimal=True explicitly, as you said, so it overwrites the default minimal=False in the method definition of glyphsLib.to_designspace(), which in turn hands over the incoming parameter directly to UFOBuilder().

In my case, both to_designspace() and UFOBuilder() receive a minimal=False value and thus fail the color handling.

I'm running gftools builder sources/blakaink.yaml in the Blaka repo

Can you please try to reproduce this? I feel like there's a ghost in my computer.

[yanone]

Update: There's a new ninja builder in gftools that I overlooked which calls UFOBuilder() directly with all the default arguments, so minimal comes as False here.

Let's keep this issue open anyway please because I want to make sure that COLRv1 fonts are tested correctly in glyphsLib.

[anthrotype]

great so now we have a new fontmake-but-not-quite to bug fix

[yanone]

Turns out that the unicodes have to be explicitly muted for color layers as well, so I was hunting two different issues in two different packages, both leading up to the same error.

PR https://github.com/googlefonts/glyphsLib/pull/947 has the fix and a new test.