Rename backgrounds of layers with the same name

Lib/glyphsLib/builder/builders.py

@@ -24,7 +24,14 @@
 from fontTools import designspaceLib
 
 from glyphsLib import classes, util
-from .constants import PUBLIC_PREFIX, FONT_CUSTOM_PARAM_PREFIX, GLYPHLIB_PREFIX
+from .constants import (
+    FONT_CUSTOM_PARAM_PREFIX,
+    FOREGROUND_LAYER_ID_KEY,
+    GLYPHLIB_PREFIX,
+    LAYER_ID_KEY,
+    LAYER_ORIGINAL_NAME_KEY,
+    PUBLIC_PREFIX,
+)
 from .axes import WEIGHT_AXIS_DEF, WIDTH_AXIS_DEF, find_base_style, class_to_value
 
 GLYPH_ORDER_KEY = PUBLIC_PREFIX + "glyphOrder"
@@ -180,8 +187,7 @@ def _is_vertical(self):
 
     @property
     def masters(self):
-        """Get an iterator over master UFOs that match the given family_name.
-        """
+        """Get an iterator over master UFOs that match the given family_name."""
         if self._sources:
             for source in self._sources.values():
                 yield source.font
@@ -216,6 +222,10 @@ def masters(self):
                 ufo_glyph = ufo_layer.newGlyph(glyph.name)
                 self.to_ufo_glyph(ufo_glyph, layer, glyph)
 
+        # Prepare layers with the same name in Glyphs as layers UFO must have
+        # unique names.
+        self._prepare_layers_with_same_name_to_ufo()
+
         # And sublayers (brace, bracket, ...) second.
         for glyph, layer in supplementary_layer_data:
             if (
@@ -517,6 +527,63 @@ def _copy_bracket_layers_to_ufo_glyphs(self, bracket_layer_map):
                     # parent's kerning.
                     _expand_kerning_to_brackets(glyph_name, ufo_glyph_name, ufo_font)
 
+    def _prepare_layers_with_same_name_to_ufo(self):
+        for master in self.font.masters:
+            ufo_font = self._sources[master.id].font
+
+            # Collect layers in the order we first see them
+            layers_by_name = dict()
+            for glyph in self.font.glyphs:
+                for layer in glyph.layers:
+                    if master.id == layer.associatedMasterId:
+                        if layer.name not in layers_by_name:
+                            layers_by_name[layer.name] = []
+                        layers_by_name[layer.name].append(layer)
+
+            # Create layers that must have different names in UFO and
+            # store layerId and original layer name for foreground layers
+            # or foreground layerId for background layers.
+            for name, layers_with_name in layers_by_name.items():
+                layerIds = {l.layerId for l in layers_with_name}
+                if len(layerIds) == 1:
+                    continue
+                if BRACKET_LAYER_RE.match(name):
+                    continue
+
+                for layer in layers_with_name:
+                    if layer.layerId == master.id:
+                        continue
+
+                    layer_name = next(
+                        (
+                            l.name
+                            for l in ufo_font.layers
+                            if l.lib.get(LAYER_ID_KEY) == layer.layerId
+                        ),
+                        None,
+                    )
+
+                    if layer_name is not None:
+                        ufo_layer = ufo_font.layers[layer_name]
+                    else:
+                        n = 1
+                        layer_name = layer.name
+                        while layer_name in ufo_font.layers:
+                            layer_name = f"{layer.name} #{n!r}"
+                            n += 1
+                        ufo_layer = ufo_font.newLayer(layer_name)
+                        if layer.name != "Color 1":
+                            ufo_layer.lib[LAYER_ORIGINAL_NAME_KEY] = layer.name
+                            ufo_layer.lib[LAYER_ID_KEY] = layer.layerId
+
+                    if layer.hasBackground:
+                        layer_name = f"{layer_name}.background"
+                        if layer_name not in ufo_font.layers:
+                            background_layer = ufo_font.newLayer(layer_name)
+                            background_layer.lib[
+                                FOREGROUND_LAYER_ID_KEY
+                            ] = layer.layerId
+
     # Implementation is split into one file per feature
     from .anchors import to_ufo_propagate_font_anchors, to_ufo_glyph_anchors
     from .annotations import to_ufo_annotations

Lib/glyphsLib/builder/constants.py

@@ -25,6 +25,10 @@
 MASTER_CUSTOM_PARAM_PREFIX = GLYPHS_PREFIX + "customParameter.GSFontMaster."
 FONT_CUSTOM_PARAM_PREFIX = GLYPHS_PREFIX + "customParameter.GSFont."
 
+LAYER_ID_KEY = GLYPHS_PREFIX + "layerId"
+LAYER_ORIGINAL_NAME_KEY = GLYPHS_PREFIX + "layerOriginalName"
+FOREGROUND_LAYER_ID_KEY = GLYPHS_PREFIX + "foregroundLayerId"
+
 GLYPHS_COLORS = (
     "0.85,0.26,0.06,1",
     "0.99,0.62,0.11,1",

Lib/glyphsLib/builder/glyph.py

@@ -310,7 +310,7 @@ def to_ufo_glyph_background(self, glyph, layer):
         return
 
     background = layer.background
-    ufo_layer = self.to_ufo_background_layer(layer)
+    ufo_layer = self.to_ufo_background_layer(glyph, layer)
     new_glyph = ufo_layer.newGlyph(glyph.name)
 
     width = background.userData[BACKGROUND_WIDTH_KEY]

Lib/glyphsLib/builder/layers.py

@@ -12,53 +12,83 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from .constants import GLYPHS_PREFIX
+import re
+
+from .constants import (
+    FOREGROUND_LAYER_ID_KEY,
+    GLYPHS_PREFIX,
+    LAYER_ID_KEY,
+    LAYER_ORIGINAL_NAME_KEY,
+)
 
-LAYER_ID_KEY = GLYPHS_PREFIX + "layerId"
 LAYER_ORDER_PREFIX = GLYPHS_PREFIX + "layerOrderInGlyph."
 LAYER_ORDER_TEMP_USER_DATA_KEY = "__layerOrder"
 
 
 def to_ufo_layer(self, glyph, layer):
+    glyphs_layers = self.font.glyphs[glyph.name].layers
     ufo_font = self._sources[layer.associatedMasterId or layer.layerId].font
+
     if layer.associatedMasterId == layer.layerId:
         ufo_layer = ufo_font.layers.defaultLayer
-    elif layer.name not in ufo_font.layers:
-        ufo_layer = ufo_font.newLayer(layer.name)
-    elif layer.name in ufo_font.layers and glyph.name in ufo_font.layers[layer.name]:
-        self.logger.warning(
-            "%s %s: Glyph %s, layer %s: Duplicate glyph layer name",
-            ufo_font.info.familyName,
-            ufo_font.info.styleName,
-            glyph.name,
-            layer.name,
-        )
-        n = 1
-        new_layer_name = layer.name
-        while new_layer_name in ufo_font.layers:
-            new_layer_name = layer.name + " #" + repr(n)
-            n += 1
-        ufo_layer = ufo_font.newLayer(new_layer_name)
     else:
-        ufo_layer = ufo_font.layers[layer.name]
-    if self.minimize_glyphs_diffs:
+        # Find foreground layer and use the same name as a base for the
+        # background layer name.
+        layer_name = next(
+            (
+                l.name
+                for l in ufo_font.layers
+                if l.lib.get(LAYER_ID_KEY) == layer.layerId
+            ),
+            None,
+        )
+        if layer_name is None and layer.name in ufo_font.layers:
+            ufo_layer = ufo_font.layers[layer.name]
+        elif layer_name is None:
+            ufo_layer = ufo_font.newLayer(layer.name)
+        else:
+            ufo_layer = ufo_font.layers[layer_name]
+
+    if LAYER_ID_KEY not in ufo_layer.lib and self.minimize_glyphs_diffs:
         ufo_layer.lib[LAYER_ID_KEY] = layer.layerId
+
+    # Store the layer order when color layers are present or when minimizing
+    # Glyphs diffs.
+    if (
+        any([re.match(r"^Color \d+$", l.name) for l in glyphs_layers])
+        or self.minimize_glyphs_diffs
+    ):
         ufo_layer.lib[LAYER_ORDER_PREFIX + glyph.name] = _layer_order_in_glyph(
             self, layer
         )
     return ufo_layer
 
 
-def to_ufo_background_layer(self, layer):
+def to_ufo_background_layer(self, glyph, layer):
     ufo_font = self._sources[layer.associatedMasterId or layer.layerId].font
     if layer.associatedMasterId == layer.layerId:
         layer_name = "public.background"
     else:
-        layer_name = layer.name + ".background"
+        # Find foreground layer and use the same name as a base for the
+        # background layer name.
+        foreground_name = next(
+            (
+                l.name
+                for l in ufo_font.layers
+                if l.lib.get(LAYER_ID_KEY) == layer.layerId
+            ),
+            None,
+        )
+        if foreground_name is None:
+            layer_name = f"{layer.name}.background"
+        else:
+            layer_name = f"{foreground_name}.background"
+
     if layer_name not in ufo_font.layers:
         background_layer = ufo_font.newLayer(layer_name)
     else:
         background_layer = ufo_font.layers[layer_name]
+
     return background_layer
 
 
@@ -78,13 +108,32 @@ def to_glyphs_layer(self, ufo_layer, glyph, master):
         layer = master_layer.background
     elif ufo_layer.name.endswith(".background"):
         # Find or create the foreground layer
-        # TODO: (jany) add lib attribute to find foreground by layer id
-        foreground_name = ufo_layer.name[: -len(".background")]
+        if FOREGROUND_LAYER_ID_KEY in ufo_layer.lib:
+            foreground_layerId = ufo_layer.lib[FOREGROUND_LAYER_ID_KEY]
+            foreground_layer = next(
+                (
+                    l
+                    for l in self._sources[master.id].font.layers
+                    if (l.lib.get(LAYER_ID_KEY) == foreground_layerId)
+                ),
+                None,
+            )
+            assert foreground_layer is not None
+            foreground_name = foreground_layer.lib.get(LAYER_ORIGINAL_NAME_KEY)
+        else:
+            foreground_layerId = None
+            foreground_name = None
+
+        if foreground_name is None:
+            foreground_name = ufo_layer.name[: -len(".background")]
+
         foreground = next(
             (
                 l
                 for l in glyph.layers
-                if l.name == foreground_name and l.associatedMasterId == master.id
+                if (foreground_layerId is None or foreground_layerId == l.layerId)
+                and l.name == foreground_name
+                and l.associatedMasterId == master.id
             ),
             None,
         )
@@ -105,10 +154,27 @@ def to_glyphs_layer(self, ufo_layer, glyph, master):
         )
         if layer is None:
             layer = self.glyphs_module.GSLayer()
+
         layer.associatedMasterId = master.id
         if LAYER_ID_KEY in ufo_layer.lib:
             layer.layerId = ufo_layer.lib[LAYER_ID_KEY]
-        layer.name = ufo_layer.name
+        else:
+            # Try to find the layerId associated with the ufo_layer and use it
+            layerId = next(
+                (
+                    l.layerId
+                    for g in self.font.glyphs
+                    for l in g.layers
+                    if l.name == ufo_layer.name and l.associatedMasterId == master.id
+                ),
+                None,
+            )
+            if layerId:
+                layer.layerId = layerId
+        if LAYER_ORIGINAL_NAME_KEY in ufo_layer.lib:
+            layer.name = ufo_layer.lib[LAYER_ORIGINAL_NAME_KEY]
+        else:
+            layer.name = ufo_layer.name
         glyph.layers.append(layer)
     order_key = LAYER_ORDER_PREFIX + glyph.name
     if order_key in ufo_layer.lib: