No width linking of brace layers (#985)

* Add test

* No width linking of brace layers

it was getting it form the master they are attached too and that is supposed to have a different width

* Fix lint

---------

Co-authored-by: Jany Belluz <jany.belluz@daltonmaag.com>

Lib/glyphsLib/builder/glyph.py

@@ -250,22 +250,20 @@ def effective_width(layer, glyph):
     # The width may be taken from another master via the customParameters
     # 'Link Metrics With Master' or 'Link Metrics With First Master'.
     font = glyph.parent
-    master = font.masters[layer.associatedMasterId or layer.layerId]
-    metrics_source = master.metricsSource
-    if metrics_source is None:
-        width = layer.width
-    else:
-        metric_layer = font.glyphs[glyph.name].layers[metrics_source.id]
-        if metric_layer:
-            width = metric_layer.width
-            if layer.width != width:
-                logger.debug(
-                    f"{layer.parent.name}: Applying width from master "
-                    f"'{metrics_source.id}': {layer.width} -> {width}"
-                )
-        else:
-            width = None
-    return width
+    master = font.masters[layer.layerId]
+    if master:
+        metrics_source = master.metricsSource
+        if metrics_source:
+            metric_layer = font.glyphs[glyph.name].layers[metrics_source.id]
+            if metric_layer:
+                width = metric_layer.width
+                if layer.width != width:
+                    logger.debug(
+                        f"{layer.parent.name}: Applying width from master "
+                        f"'{metrics_source.id}': {layer.width} -> {width}"
+                    )
+                return width
+    return layer.width
 
 
 def to_ufo_glyph_color(self, ufo_glyph, layer, glyph, do_color_layers=True):

tests/data/IntermediateLayerWidth.glyphs

@@ -0,0 +1,241 @@
+{
+.appVersion = "3259";
+.formatVersion = 3;
+DisplayStrings = (
+a
+);
+axes = (
+{
+name = Weight;
+tag = wght;
+},
+{
+name = ROUND;
+tag = ROND;
+}
+);
+date = "2024-04-22 16:44:09 +0000";
+familyName = "New Font";
+fontMaster = (
+{
+axesValues = (
+400,
+0
+);
+id = "m-400-0";
+metricValues = (
+{
+over = 16;
+pos = 800;
+},
+{
+over = -16;
+},
+{
+over = -16;
+pos = -200;
+},
+{
+over = 16;
+pos = 700;
+},
+{
+over = 16;
+pos = 500;
+},
+{
+}
+);
+name = Regular;
+},
+{
+axesValues = (
+700,
+0
+);
+iconName = SemiBold;
+id = "m-700-0";
+metricValues = (
+{
+over = 16;
+pos = 800;
+},
+{
+over = -16;
+},
+{
+over = -16;
+pos = -200;
+},
+{
+over = 16;
+pos = 700;
+},
+{
+over = 16;
+pos = 500;
+},
+{
+}
+);
+name = Bold;
+},
+{
+axesValues = (
+400,
+1
+);
+customParameters = (
+{
+name = "Link Metrics With Master";
+value = "m-400-0";
+}
+);
+id = "m-400-1";
+metricValues = (
+{
+over = 16;
+pos = 800;
+},
+{
+over = -16;
+},
+{
+over = -16;
+pos = -200;
+},
+{
+over = 16;
+pos = 700;
+},
+{
+over = 16;
+pos = 500;
+},
+{
+}
+);
+name = "Regular ROUND";
+},
+{
+axesValues = (
+700,
+1
+);
+customParameters = (
+{
+name = "Link Metrics With Master";
+value = "m-700-0";
+}
+);
+iconName = SemiBold;
+id = "m-700-1";
+metricValues = (
+{
+over = 16;
+pos = 800;
+},
+{
+over = -16;
+},
+{
+over = -16;
+pos = -200;
+},
+{
+over = 16;
+pos = 700;
+},
+{
+over = 16;
+pos = 500;
+},
+{
+}
+);
+name = "Bold ROUND";
+}
+);
+glyphs = (
+{
+glyphname = a;
+lastChange = "2024-04-23 07:38:25 +0000";
+layers = (
+{
+layerId = "m-400-1";
+width = 100;
+},
+{
+layerId = "m-400-0";
+width = 100;
+},
+{
+layerId = "m-700-1";
+width = 300;
+},
+{
+associatedMasterId = "m-400-0";
+attr = {
+coordinates = (
+500,
+0
+);
+};
+layerId = "m-500-0";
+name = "Intermediate 500 0";
+width = 200;
+},
+{
+layerId = "m-700-0";
+width = 300;
+},
+{
+associatedMasterId = "m-400-1";
+attr = {
+coordinates = (
+500,
+1
+);
+};
+layerId = "m-500-1";
+name = "Intermediate 500 1";
+width = 200;
+}
+);
+unicode = 97;
+},
+{
+glyphname = space;
+layers = (
+{
+layerId = "m-400-1";
+width = 200;
+}
+);
+unicode = 32;
+}
+);
+metrics = (
+{
+type = ascender;
+},
+{
+type = baseline;
+},
+{
+type = descender;
+},
+{
+type = "cap height";
+},
+{
+type = "x-height";
+},
+{
+type = "italic angle";
+}
+);
+unitsPerEm = 1000;
+versionMajor = 1;
+versionMinor = 0;
+}

tests/special_layer_width_test.py

@@ -16,7 +16,7 @@
 
 import os
 
-from glyphsLib import load_to_ufos
+from glyphsLib import load_to_ufos, GSFont, to_designspace
 
 
 def glyphs_file_path():
@@ -42,3 +42,46 @@ def test_substitution_layer_width():
     assert masters[1]["B"].width == 600
     assert masters[0]["B.BRACKET.varAlt01"].width == 510
     assert masters[1]["B.BRACKET.varAlt01"].width == 610
+
+
+def test_intermediate_layer_width_with_metrics_source_on_parent():
+    """This checks that "intermediate layers", a.k.a. "brace layers", do not
+    incorrectly inherit an irrelevant width from their parent layer.
+
+    Scenario in the test file:
+
+    - Glyph /a
+        - Regular (400, 0): advance width = 100
+            - Intermediate layer {500, 0}: advance width = 200
+        - Bold (700, 0): advance width = 300
+        - Regular ROUND (400, 1); advance width = 100
+            * This master layer has a "metricsSource" pointing to Regular (400, 0)
+              to ensure the widths are consistent.
+            - Intermediate layer {500, 1}: advance width = 200
+                * This intermediate layer is under consideration for the test.
+        - Bold ROUND (700, 1): advance width = 300
+
+    Previously, the advance width of the intermediate layer `{500, 1}` would be
+    forcibly taken from the `metricsSource` of the master layer `Regular ROUND
+    (400, 1)` to which the intermediate layer `{500, 1}` was attached, and so it
+    would get 100 instead of 200.
+
+    With this patch, the advance width of the layer `{500, 1}` will not be
+    changed, because the the layer `{500, 1}` does not define itself a
+    `metricsSource`.
+
+    See https://github.com/googlefonts/glyphsLib/pull/985
+    """
+    test_path = os.path.join(
+        os.path.dirname(__file__), "data", "IntermediateLayerWidth.glyphs"
+    )
+    font = GSFont(test_path)
+    doc = to_designspace(font)
+    for intermediate_round in doc.sources:
+        if intermediate_round.getFullDesignLocation(doc) == {"Weight": 500, "ROUND": 1}:
+            break
+    else:
+        raise AssertionError("Can't find intermediate layer in the desigspace")
+    assert (
+        intermediate_round.font.layers[intermediate_round.layerName]["a"].width == 200
+    )