error exporting UFO when a GSGlyph contain multiple layers with the same name

[anthrotype]

when building master UFOs, if a Glyphs's glyph object has more than one supplementary layers with identical name, the builder will attempt to call ufoLib2.Layer.newGlyph method twice for the same layer and glyph name, resulting in errors like this

[...]
  File "glyphsLib/builder/builders.py", line 222, in masters
    ufo_glyph = ufo_layer.newGlyph(glyph.name)
  File "ufoLib2/objects/layer.py", line 120, in newGlyph
    raise KeyError("glyph %r already exists" % name)
KeyError: "glyph '_part.spirale' already exists"

[anthrotype]

i am not sure how we should fix this. If it is a common thing to have multiple layers within the same glyph with the same name, then we should support it somehow without losing data. Otherwise, I'd be inclined to simply warn and only save the last duplicate glyph layer.

By the way, before when we were using defcon instead of ufoLib2, we were silently taking the last duplicate-name glyph layer.

There is in fact a difference between defcon's and ufoLib2's newGlyph methods: the former overwrites any existing glyph, whereas the latter raises an error...

[schriftgestalt]

I wouldn’t say it is common but it is possible. e.g. with interpolating 'iColor XX' or 'Color XX' layers. And a 'Color XX' layer might show up twice inside a master.
So possible setups:

Regular 
    Color 1
    Color 2
Bold
    Color 1
    Color 2

or

Regular 
    Color 1
    Color 2
    Color 1

[madig]

I'm inclined to warn and take the last layer.

@schriftgestalt the former case is supported because different masters are different UFOs. I guess it's the latter case that blows up. Maybe layer names can be made unique for Glyphs 3?

[anthrotype]

interpolating 'iColor XX' or 'Color XX' layers. And a 'Color XX' layer might show up twice inside a master

is this some special type of layer naming scheme that we should be aware of?

[anthrotype]

replying to myself, yes iColorlayers is for sbix color fonts and are documented here:
https://glyphsapp.com/tutorials/creating-an-apple-color-font

whereas Color XX is for COLR/CPAL or OT-SVG color fonts:
https://glyphsapp.com/tutorials/creating-a-microsoft-color-font
https://glyphsapp.com/tutorials/creating-an-svg-color-font

Especially considering that want to support building color fonts (googlefonts/fontmake#30), we do want to make sure we export such layers correctly.

Given that UFO layers must have unique names, glyphsLib should make them unique using some suffix when exporting and strip the suffix upon importing.

[anthrotype]

the problem here is that the uniqueness of layers in UFO is in their name (layercontents.plist says layer names must be unique), whereas in Glyphs.app it is given by their ID (which is like a uuid). This allows it to have layers with the same name (but different uuid).
Any suffix scheme will be prone to ambiguities when going back and forth between Glyphs <-> UFO, because layer names may contain any characters.

[belluzj]

(same error on Noto Sans Georgian from https://github.com/googlefonts/noto-source/blob/809ac0b/src/NotoSansGeorgian-MM.glyphs)

[anthrotype]

@moyogo was working on a PR
#570

[anthrotype]

Fixed with #570

[belluzj]

Thanks @moyogo and @anthrotype! (happy new year!)

[anthrotype]

'Color XX' layer might show up twice inside a master

but why? If the two supplementary layers (both linked to the same master) bear the same "Color XX", couldn't (should) they be flattened into one? What is the purpose of having two with the same name (color)?

/cc @schriftgestalt

[anthrotype]

oh.. the order is important. Now I understood your example above.

Regular 
    Color 1
    Color 2
    Color 1

Hm not sure what to do. Could Glyphs.app be chaged so that it ignores any extra characters that appear after the "Color XX" string (where XX are digits)?

[anthrotype]

At least the suffix should be removed when the name starts with "Color" or "iColor".

I guess we can do that.

[anthrotype]

i'll open a separate issue for this