Merge pull request #929 from googlefonts/fix-brace-layer-names-2

ensure DS source layer is named correctly for brace glyphs

Lib/glyphsLib/builder/layers.py

@@ -36,15 +36,8 @@ def to_ufo_layer(self, glyph, layer):
     # Give color layers better names
     if layer._is_color_palette_layer():
         layer_name = f"color.{layer._color_palette_index()}"
-    elif "coordinates" in layer.attributes:
-        # For Glyphs 3's intermediate (formerly 'brace') layers we must generate the
-        # name from the attributes (as it's shown in Glyphs.app UI) and discard
-        # the layer's actual 'name' as found in the source file, which is usually just
-        # the unique date-time when a layer was first created.
-        # Using the generated name ensures that all the intermediate glyph instances
-        # at a given location end up in the same UFO source layer, see:
-        # https://github.com/googlefonts/glyphsLib/issues/851
-        layer_name = f"{{{', '.join(str(v) for v in layer.attributes['coordinates'])}}}"
+    elif layer._is_brace_layer():
+        layer_name = layer._brace_layer_name()
 
     if layer.associatedMasterId == layer.layerId:
         ufo_layer = ufo_font.layers.defaultLayer

Lib/glyphsLib/builder/sources.py

@@ -156,7 +156,7 @@ def _to_designspace_source_layer(self):
     for glyph in self.font.glyphs:
         for layer in glyph.layers:
             if layer._is_brace_layer():
-                key = (layer.name, tuple(layer._brace_coordinates()))
+                key = (layer._brace_layer_name(), tuple(layer._brace_coordinates()))
                 layer_to_master_ids[key].add(layer.associatedMasterId)
                 layer_to_glyph_names[key].append(glyph.name)
 

Lib/glyphsLib/classes.py

@@ -3993,6 +3993,21 @@ def _brace_coordinates(self):
         coordinates = name[name.index("{") + 1 : name.index("}")]
         return [float(c) for c in coordinates.split(",")]
 
+    def _brace_layer_name(self):
+        # For Glyphs 3's intermediate (formerly 'brace') layers we must generate the
+        # name from the attributes (as it's shown in Glyphs.app UI) and discard
+        # the layer's actual 'name' as found in the source file, which is usually just
+        # the unique date-time when a layer was first created.
+        # Using the generated name ensures that all the intermediate glyph instances
+        # at a given location end up in the same UFO source layer, see:
+        # https://github.com/googlefonts/glyphsLib/issues/851
+        # TODO: Figure out a better API for layer.name vs layer.nameUI() mess...
+        if "coordinates" in self.attributes:
+            # Glyphs 3
+            return f"{{{', '.join(str(v) for v in self.attributes['coordinates'])}}}"
+        # Glyphs 2
+        return self.name
+
     COLOR_PALETTE_LAYER_RE = re.compile(r"^Color (?P<index>\*|\d+)$")
 
     def _is_color_palette_layer(self):

tests/builder/designspace_gen_test.py

@@ -169,8 +169,8 @@ def test_designspace_generation_brace_layers(datadir, filename, ufo_module):
         ("Weight", 100, 100, 700, [(100, 100.0), (700, 1000.0)]),
     ]
 
-    for (fname, layerName, name), (exp_fname, exp_layerName, exp_name) in zip(
-        [(s.filename, s.layerName, s.name) for s in designspace.sources],
+    for (fname, layerName, name, ufo), (exp_fname, exp_layerName, exp_name) in zip(
+        [(s.filename, s.layerName, s.name, s.font) for s in designspace.sources],
         [
             ("NewFont-Light.ufo", None, "New Font Light"),
             ("NewFont-Light.ufo", "{75}", "New Font Light {75}"),
@@ -189,6 +189,7 @@ def test_designspace_generation_brace_layers(datadir, filename, ufo_module):
         # https://github.com/googlefonts/glyphsLib/issues/851
         if exp_layerName is not None:
             assert fnmatch.fnmatch(layerName, exp_layerName)
+            assert layerName in ufo.layers
         else:
             assert layerName is None
         assert fnmatch.fnmatch(name, exp_name)