Merge pull request #1088 from googlefonts/propagate-bracket-anchors

[propagate_anchors] support propagating anchors from components for bracket/alternate layers

Author: anthrotype

Lib/glyphsLib/builder/transformations/propagate_anchors.py

@@ -105,9 +105,8 @@ def _interesting_layers(glyph):
     return (
         l
         for l in glyph.layers
-        if l._is_master_layer
+        if l._is_master_layer or l._is_bracket_layer()
         # or l._is_brace_layer
-        # or l._is_bracket_layer
         # etc.
     )
 
@@ -385,17 +384,48 @@ def get_component_layer_anchors(
     glyphs: dict[str, GSGlyph],
     anchors: dict[str, dict[str, list[GSAnchor]]],
 ) -> list[GSAnchor] | None:
+    if component.name not in anchors:
+        return None  # nothing to propagate
+
     glyph = glyphs.get(component.name)
     if glyph is None:
-        return None
+        return None  # invalid component reference, skip
+
     # in Glyphs.app, the `componentLayer` property would synthesize a layer
     # if it is missing. glyphsLib does not have that yet, so for now we
-    # only support the corresponding 'master' layer of a component's base glyph.
+    # only support the corresponding 'master' or alternate ('bracket') layers
+    # of a component's base glyph.
+
     layer_anchors = None
+
+    # whether the parent layer where the component is defined is a 'master' layer
+    # and/or a 'bracket' or alternate layer (masters can have bracket layers too but
+    # glyphsLib doesn't support that yet).
+    parent_is_master = layer._is_master_layer
+    parent_is_bracket = layer._is_bracket_layer()
+    parent_axis_rules = (
+        [] if not parent_is_bracket else list(layer._bracket_axis_rules())
+    )
+
+    # we support propagating anchors from the component base glyph's 'master' layer
+    # (with same layerId), or from a 'bracket' alternate layer with matching axis
+    # rules and the same associated master; we try the latter first
     for comp_layer in _interesting_layers(glyph):
-        if comp_layer.layerId == layer.layerId and component.name in anchors:
+        if (
+            parent_is_bracket
+            and comp_layer._is_bracket_layer()
+            and comp_layer.associatedMasterId == layer.associatedMasterId
+            and (list(comp_layer._bracket_axis_rules()) == parent_axis_rules)
+        ) or (parent_is_master and comp_layer.layerId == layer.layerId):
             layer_anchors = anchors[component.name][comp_layer.layerId]
             break
+
+    # else we fall back to the associated master layer; this is guaranteed to exist
+    # since all glyphs must at least define one layer per master; if this raised
+    # KeyError, the font is broken
+    if layer_anchors is None:
+        layer_anchors = anchors[component.name][layer.associatedMasterId]
+
     if layer_anchors is not None:
         # return a copy as they may be modified in place
         layer_anchors = [

Lib/glyphsLib/classes.py

@@ -3947,40 +3947,42 @@ def _is_bracket_layer(self):
             return "axisRules" in self.attributes  # Glyphs 3
         return re.match(self.BRACKET_LAYER_RE, self.name)  # Glyphs 2
 
-    def _bracket_info(self, axes):
-        # Returns a region expressed as a {axis_tag: (min, max)} box
-        # (dictionary), once the axes have been computed
+    def _bracket_axis_rules(self):
         if not self._is_bracket_layer():
-            return {}
-
+            return
         if self.parent.parent.format_version > 2:
             # Glyphs 3
-            info = {}
-            for axis, rule in zip(axes, self.attributes["axisRules"]):
-                if "min" not in rule and "max" not in rule:
-                    continue
-                # Rules are expressed in designspace coordinates,
-                # so map appropriately.
-                designspace_min, designspace_max = designspace_min_max(axis)
-                axis_min = rule.get("min", designspace_min)
-                axis_max = rule.get("max", designspace_max)
+            for rule in self.attributes["axisRules"]:
+                axis_min = rule.get("min")
+                axis_max = rule.get("max")
                 if isinstance(axis_min, str):
                     axis_min = float(axis_min)
                 if isinstance(axis_max, str):
                     axis_max = float(axis_max)
-                info[axis.tag] = (axis_min, axis_max)
-            return info
+                yield (axis_min, axis_max)
+            return
 
         # Glyphs 2
         m = re.match(self.BRACKET_LAYER_RE, self.name)
-        axis = axes[0]  # For glyphs 2
-        designspace_min, designspace_max = designspace_min_max(axis)
         reverse = m.group("first_bracket") == "]"
         bracket_crossover = int(m.group("value"))
         if reverse:
-            return {axis.tag: (designspace_min, bracket_crossover)}
+            yield (None, bracket_crossover)
         else:
-            return {axis.tag: (bracket_crossover, designspace_max)}
+            yield (bracket_crossover, None)
+
+    def _bracket_info(self, axes):
+        # Returns a region expressed as a {axis_tag: (min, max)} box
+        # (dictionary), once the axes have been computed
+        info = {}
+        for axis, (axis_min, axis_max) in zip(axes, self._bracket_axis_rules()):
+            # Rules are expressed in designspace coordinates,
+            # so map appropriately.
+            designspace_min, designspace_max = designspace_min_max(axis)
+            axis_min = axis_min if axis_min is not None else designspace_min
+            axis_max = axis_max if axis_max is not None else designspace_max
+            info[axis.tag] = (axis_min, axis_max)
+        return info
 
     def _is_brace_layer(self):
         if self.parent.parent.format_version > 2:

tests/builder/transformations/propagate_anchors_test.py

@@ -39,15 +39,19 @@ class GlyphSetBuilder:
     glyphs: dict[str, GSGlyph]
 
     def __init__(self):
-        self.glyphs = {}
+        self.font = GSFont()
+        self.font.format_version = 3
 
     def build(self) -> dict[str, GSGlyph]:
-        return self.glyphs
+        return {g.name: g for g in self.font.glyphs}
 
     def add_glyph(self, name: str, build_fn: Callable[["GlyphBuilder"], None]) -> Self:
         glyph = GlyphBuilder(name)
         build_fn(glyph)
-        self.glyphs[name] = glyph.build()
+        # this inserts the glyph in the font and sets the glyph.parent attribute to it;
+        # GSLayer._is_bracket_layer()/_bracket_axis_rules() require this to check the
+        # glyph.parent.format_version to determine where to look for the axis rules
+        self.font.glyphs.append(glyph.build())
         return self
 
 
@@ -60,6 +64,7 @@ def __init__(self, name: str):
         glyph.category = info.category
         glyph.subCategory = info.subCategory
         self.num_masters = 0
+        self.num_alternates = 0
         self.add_master_layer()
 
     def build(self) -> GSGlyph:
@@ -71,6 +76,7 @@ def add_master_layer(self) -> Self:
             f"master-{self.num_masters}"
         )
         self.num_masters += 1
+        self.num_alternates = 0
         self.glyph.layers.append(layer)
         self.current_layer = layer
         return self
@@ -85,6 +91,20 @@ def add_backup_layer(self, associated_master_idx=0):
         self.current_layer = layer
         return self
 
+    def add_bracket_layer(self, axis_rules):
+        assert self.current_layer._is_master_layer
+        associated_master_id = self.current_layer.layerId
+        master_layer = self.glyph.layers[associated_master_id]
+        layer = GSLayer()
+        layer.name = f"{master_layer.name} bracket-{self.num_alternates}"
+        layer.layerId = str(uuid.uuid4()).upper()
+        layer.associatedMasterId = associated_master_id
+        layer.attributes["axisRules"] = axis_rules
+        self.num_alternates += 1
+        self.glyph.layers.append(layer)
+        self.current_layer = layer
+        return self
+
     def set_category(self, category: str) -> Self:
         self.glyph.category = category
         return self
@@ -362,31 +382,37 @@ def test_digraphs_arent_ligatures():
 
 
 def test_propagate_across_layers(caplog):
-    # derived from the observed behaviour of glyphs 3.2.2 (3259)
+    # derived from the observed behaviour of glyphs 3.4 (3414)
     glyphs = (
         GlyphSetBuilder()
         .add_glyph(
             "A",
             lambda glyph: (
-                glyph.add_anchor("bottom", (290, 10))
-                .add_anchor("ogonek", (490, 3))
-                .add_anchor("top", (290, 690))
+                glyph.add_anchor("bottom", (206, 16))
+                .add_anchor("ogonek", (360, 13))
+                .add_anchor("top", (212, 724))
+                .add_bracket_layer([{"min": 500}])
+                .add_anchor("bottom", (206, 16))
+                .add_anchor("top", (212, 724))
                 .add_master_layer()
-                .add_anchor("bottom", (300, 0))
-                .add_anchor("ogonek", (540, 10))
-                .add_anchor("top", (300, 700))
+                .add_anchor("bottom", (278, 12))
+                .add_anchor("ogonek", (464, 13))
+                .add_anchor("top", (281, 758))
+                .add_bracket_layer([{"min": 500}])
+                .add_anchor("bottom", (278, 12))
+                .add_anchor("top", (281, 758))
                 .add_backup_layer()
                 .add_anchor("top", (290, 690))
             ),
         )
         .add_glyph(
             "acutecomb",
             lambda glyph: (
-                glyph.add_anchor("_top", (335, 502))
-                .add_anchor("top", (353, 721))
+                glyph.add_anchor("_top", (150, 580))
+                .add_anchor("top", (170, 792))
                 .add_master_layer()
-                .add_anchor("_top", (366, 500))
-                .add_anchor("top", (366, 765))
+                .add_anchor("_top", (167, 580))
+                .add_anchor("top", (170, 792))
                 .add_backup_layer()
                 .add_anchor("_top", (335, 502))
             ),
@@ -395,10 +421,16 @@ def test_propagate_across_layers(caplog):
             "Aacute",
             lambda glyph: (
                 glyph.add_component("A", (0, 0))
-                .add_component("acutecomb", (-45, 188))
+                .add_component("acutecomb", (62, 144))
+                .add_bracket_layer([{"min": 500}])
+                .add_component("A", (20, 0))
+                .add_component("acutecomb", (82, 144))
                 .add_master_layer()
                 .add_component("A", (0, 0))
-                .add_component("acutecomb", (-66, 200))
+                .add_component("acutecomb", (114, 178))
+                .add_bracket_layer([{"min": 500}])
+                .add_component("A", (30, 0))
+                .add_component("acutecomb", (144, 178))
                 .add_backup_layer()
                 .add_component("A", (0, 0))
                 .add_component("acutecomb", (-45, 188))
@@ -433,26 +465,45 @@ def test_propagate_across_layers(caplog):
 
     new_glyph = glyphs["Aacute"]
     assert_anchors(
-        new_glyph.layers[0].anchors,
+        new_glyph.layers["master-0"].anchors,
         [
-            ("bottom", (290, 10)),
-            ("ogonek", (490, 3)),
-            ("top", (308, 909)),
+            ("bottom", (206, 16)),
+            ("ogonek", (360, 13)),
+            ("top", (232, 936)),
         ],
     )
 
     assert_anchors(
-        new_glyph.layers[1].anchors,
+        new_glyph.layers["master-1"].anchors,
+        [
+            ("bottom", (278, 12)),
+            ("ogonek", (464, 13)),
+            ("top", (284, 970)),
+        ],
+    )
+
+    # alternate 'bracket' layers should work as well
+    alternate_layers = [l for l in new_glyph.layers if l._is_bracket_layer()]
+    assert len(alternate_layers) == 2
+    assert_anchors(
+        alternate_layers[0].anchors,
+        [
+            ("bottom", (226, 16)),
+            ("top", (252, 936)),
+        ],
+    )
+    assert_anchors(
+        alternate_layers[1].anchors,
         [
-            ("bottom", (300, 0)),
-            ("ogonek", (540, 10)),
-            ("top", (300, 965)),
+            ("bottom", (308, 12)),
+            ("top", (314, 970)),
         ],
     )
 
     # non-master (e.g. backup) layers are silently skipped
-    assert not new_glyph.layers[2]._is_master_layer
-    assert_anchors(new_glyph.layers[2].anchors, [])
+    assert not new_glyph.layers[-1]._is_master_layer
+    assert not new_glyph.layers[-1]._is_bracket_layer()
+    assert_anchors(new_glyph.layers[-1].anchors, [])
 
     assert len(caplog.records) == 0
 
@@ -730,6 +781,9 @@ def test_real_files():
     for g1, g2 in zip(font.glyphs, expected.glyphs):
         assert len(g1.layers) == len(g2.layers)
         for l1, l2 in zip(g1.layers, g2.layers):
-            assert [(a.name, tuple(a.position)) for a in l1.anchors] == [
+            # seems like the latest Glyphs.app insists on sorting anchors alphabetically
+            # whereas we keep the original order; but what matters is their name and
+            # position
+            assert sorted((a.name, tuple(a.position)) for a in l1.anchors) == sorted(
                 (a.name, tuple(a.position)) for a in l2.anchors
-            ]
+            )