googlefonts / glyphsLib

A bridge from Glyphs source files (.glyphs) to UFOs
Apache License 2.0
178 stars 51 forks source link

Fix color component layers #974

Closed simoncozens closed 5 months ago

simoncozens commented 5 months ago

This is a tiny fix for #973. In components.py we gather a list of color layers and take the index to get the new component name, like so:

https://github.com/googlefonts/glyphsLib/blob/ed02363c5e2d53294fc7a49992df2da5006f5f2e/Lib/glyphsLib/builder/components.py#L59-L68

We do this because we do the same operation in _to_ufo_color_palette_layers to rename the glyphs:

https://github.com/googlefonts/glyphsLib/blob/ed02363c5e2d53294fc7a49992df2da5006f5f2e/Lib/glyphsLib/builder/color_layers.py#L184-L194

However, the list which we take the index from there is actually built here:

https://github.com/googlefonts/glyphsLib/blob/ed02363c5e2d53294fc7a49992df2da5006f5f2e/Lib/glyphsLib/builder/glyph.py#L257-L262

So in components.py we need to make sure we are building the same list by also testing for l.associatedMasterId == layer.associatedMasterId.

anthrotype commented 5 months ago

would be nice if we could also add that test file of yours to the test suite. Maybe just parsing it and converting to_designspace and asserting that the component names are as expected with the correct .color{i} suffix, perhaps inside tests/builder/components_test.py. Could you please add that?