Merge pull request #1092 from googlefonts/align-alternates-before-anchor-prop

[bracket_layers] add align_alternate_layers preflight transformation

Author: anthrotype

Lib/glyphsLib/builder/bracket_layers.py

@@ -1,7 +1,6 @@
 from collections import defaultdict
 from functools import partial
 from typing import Any
-import copy
 
 from fontTools import designspaceLib
 from fontTools.varLib import FEAVAR_FEATURETAG_LIB_KEY
@@ -23,8 +22,6 @@ def to_designspace_bracket_layers(self):
             "Cannot apply bracket layers unless at least one axis is defined."
         )
 
-    find_component_use(self)
-
     # At this stage we will naively emit a designspace rule for every layer.
     bracket_layer_map = defaultdict(partial(defaultdict, list))
     rules = []
@@ -170,122 +167,3 @@ def _expand_kerning_to_brackets(
         elif second_match:
             bracket_kerning[(first, ufo_glyph_name)] = value
     ufo_font.kerning.update(bracket_kerning)
-
-
-def find_component_use(self):
-    """If a glyph uses a component which has alternate layers, that
-    glyph also must have the same alternate layers or else it will not
-    correctly swap. We copy the layer locations from the component into
-    the glyph which uses it."""
-    # First let's put all the layers in a sensible order so we can
-    # query them efficiently
-    master_layers = defaultdict(dict)
-    alternate_layers = defaultdict(lambda: defaultdict(list))
-    master_ids = set(master.id for master in self.font.masters)
-
-    for glyph in self.font.glyphs:
-        for layer in glyph.layers:
-            if layer.layerId in master_ids:
-                master_layers[layer.layerId][glyph.name] = layer
-            elif layer.associatedMasterId in master_ids:
-                alternate_layers[layer.associatedMasterId][glyph.name].append(layer)
-
-    # Now let's find those which have a problem: they use components,
-    # the components have some alternate layers, but the layer doesn't
-    # have the same.
-    # Because of the possibility of deeply nested components, we need
-    # to keep doing this, bubbling up fixes until there's nothing left
-    # to do.
-    while True:
-        problematic_glyphs = defaultdict(set)
-        for master, layers in master_layers.items():
-            for glyph_name, layer in layers.items():
-                my_bracket_layers = [
-                    layer._bracket_info(self._designspace.axes)
-                    for layer in alternate_layers[master][glyph_name]
-                ]
-                for comp in layer.components:
-                    # Check our alternate layer set-up agrees with theirs
-                    components_bracket_layers = [
-                        layer._bracket_info(self._designspace.axes)
-                        for layer in alternate_layers[master][comp.name]
-                    ]
-                    if my_bracket_layers != components_bracket_layers:
-                        # Find what we need to add, and make them hashable
-                        they_have = set(
-                            tuple(x.items())
-                            for x in components_bracket_layers
-                            if x.items()
-                        )
-                        i_have = set(
-                            tuple(x.items()) for x in my_bracket_layers if x.items()
-                        )
-                        needed = they_have - i_have
-                        if needed:
-                            problematic_glyphs[(glyph_name, master)] |= needed
-
-        if not problematic_glyphs:
-            break
-
-        # And now, fix the problem.
-        for (glyph_name, master), needed_brackets in problematic_glyphs.items():
-            my_bracket_layers = [
-                layer._bracket_info(self._designspace.axes)
-                for layer in alternate_layers[master][glyph_name]
-            ]
-            if my_bracket_layers:
-                # We have some bracket layers, but they're not the ones we
-                # expect. Do the wrong thing, because doing the right thing
-                # requires major investment.
-                master_name = self.font.masters[master].name
-                self.logger.warning(
-                    f"Glyph {glyph_name} in master {master_name} has different "
-                    "alternate layers to components that it uses. We don't "
-                    "currently support this case, so some alternate layers will "
-                    "not be applied. Consider fixing the source instead."
-                )
-            # Just copy the master layer for each thing we need.
-            for box in needed_brackets:
-                new_layer = synthesize_bracket_layer(
-                    master_layers[master][glyph_name], dict(box), self._designspace.axes
-                )
-                self.font.glyphs[glyph_name].layers.append(new_layer)
-                self.bracket_layers.append(new_layer)
-                alternate_layers[master][glyph_name].append(new_layer)
-
-    # any components have now just been appended to the end; let's sort bracket
-    # layers by glyph order to maintain a consistent, understandable order.
-    glyphOrder = {g.name: i for (i, g) in enumerate(self.font.glyphs)}
-    self.bracket_layers.sort(key=lambda layer: glyphOrder.get(layer.parent.name))
-
-
-def synthesize_bracket_layer(old_layer, box, axes):
-    new_layer = copy.copy(old_layer)  # We don't need a deep copy of everything
-    new_layer.layerId = ""
-    new_layer.associatedMasterId = old_layer.layerId
-
-    if new_layer.parent.parent.format_version == 2:
-        axis, (bottom, top) = next(iter(box.items()))
-        designspace_min, designspace_max = util.designspace_min_max(axes[0])
-        if designspace_min == bottom:
-            new_layer.name = old_layer.name + f" ]{top}]"
-        else:
-            new_layer.name = old_layer.name + f"[{bottom}]"
-    else:
-        new_layer.attributes = dict(
-            new_layer.attributes
-        )  # But we do need our own version of this
-        new_layer.attributes["axisRules"] = []
-        for axis in axes:
-            if axis.tag in box:
-                new_layer.attributes["axisRules"].append(
-                    {
-                        "min": box[axis.tag][0],
-                        "max": box[axis.tag][1],
-                    }
-                )
-            else:
-                new_layer.attributes["axisRules"].append({})
-
-    assert new_layer._bracket_info(axes) == box
-    return new_layer

Lib/glyphsLib/builder/transformations/__init__.py

@@ -1,9 +1,11 @@
 from types import MappingProxyType
 from typing import NamedTuple
 
+from .align_alternate_layers import align_alternate_layers
 from .propagate_anchors import propagate_all_anchors
 
 TRANSFORMATIONS = [
+    align_alternate_layers,
     propagate_all_anchors,
 ]
 

Lib/glyphsLib/builder/transformations/align_alternate_layers.py

@@ -0,0 +1,109 @@
+import copy
+import logging
+from collections import defaultdict
+
+
+logger = logging.getLogger(__name__)
+
+
+def align_alternate_layers(font, glyph_data=None):
+    """Ensure composites have the same alternate layers as their components.
+
+    If a glyph uses a component which has alternate (aka 'bracket') layers, that
+    glyph also must have the same alternate layers or else it will not correctly swap
+    when these are converted GSUB FeatureVariations.
+    We copy the layer locations from the component into the glyph which uses it.
+    """
+    # First let's put all the layers in a sensible order so we can
+    # query them efficiently
+    master_layers = defaultdict(dict)
+    alternate_layers = defaultdict(lambda: defaultdict(list))
+    master_ids = set(master.id for master in font.masters)
+
+    for glyph in font.glyphs:
+        if not glyph.export:
+            continue
+        for layer in glyph.layers:
+            if layer.layerId in master_ids:
+                master_layers[layer.layerId][glyph.name] = layer
+            elif layer.associatedMasterId in master_ids and layer._is_bracket_layer():
+                alternate_layers[layer.associatedMasterId][glyph.name].append(layer)
+
+    # Now let's find those which have a problem: they use components,
+    # the components have some alternate layers, but the layer doesn't
+    # have the same.
+    # Because of the possibility of deeply nested components, we need
+    # to keep doing this, bubbling up fixes until there's nothing left
+    # to do.
+    while True:
+        problematic_glyphs = defaultdict(set)
+        for master, layers in master_layers.items():
+            for glyph_name, layer in layers.items():
+                my_bracket_layers = {
+                    tuple(layer._bracket_axis_rules())
+                    for layer in alternate_layers[master][glyph_name]
+                }
+                for comp in layer.components:
+                    # Check our alternate layer set-up agrees with theirs
+                    components_bracket_layers = {
+                        tuple(layer._bracket_axis_rules())
+                        for layer in alternate_layers[master][comp.name]
+                    }
+                    if my_bracket_layers != components_bracket_layers:
+                        # Find what we need to add, and make them hashable
+                        needed = components_bracket_layers - my_bracket_layers
+                        if needed:
+                            problematic_glyphs[(glyph_name, master)] |= needed
+
+        if not problematic_glyphs:
+            break
+
+        # And now, fix the problem.
+        for (glyph_name, master), needed_brackets in problematic_glyphs.items():
+            my_bracket_layers = [
+                tuple(layer._bracket_axis_rules())
+                for layer in alternate_layers[master][glyph_name]
+            ]
+            if my_bracket_layers:
+                # We have some bracket layers, but they're not the ones we
+                # expect. Do the wrong thing, because doing the right thing
+                # requires major investment.
+                master_name = font.masters[master].name
+                logger.warning(
+                    f"Glyph {glyph_name} in master {master_name} has different "
+                    "alternate layers to components that it uses. We don't "
+                    "currently support this case, so some alternate layers will "
+                    "not be applied. Consider fixing the source instead."
+                )
+            # Just copy the master layer for each thing we need.
+            for axis_rules in needed_brackets:
+                new_layer = synthesize_bracket_layer(
+                    master_layers[master][glyph_name], axis_rules
+                )
+                font.glyphs[glyph_name].layers.append(new_layer)
+                alternate_layers[master][glyph_name].append(new_layer)
+
+
+def synthesize_bracket_layer(old_layer, axis_rules):
+    new_layer = copy.copy(old_layer)  # We don't need a deep copy of everything
+    new_layer.layerId = ""
+    new_layer.associatedMasterId = old_layer.layerId
+
+    if new_layer.parent.parent.format_version == 2:
+        bottom, top = next(iter(axis_rules))
+        if bottom is None:
+            new_layer.name = old_layer.name + f" ]{top}]"
+        elif top is None:
+            new_layer.name = old_layer.name + f" [{bottom}]"
+        else:
+            raise AssertionError(f"either {bottom} or {top} must be None")
+    else:
+        new_layer.attributes = dict(
+            new_layer.attributes
+        )  # But we do need our own version of this
+        new_layer.attributes["axisRules"] = [
+            {"min": bottom, "max": top} for (bottom, top) in axis_rules
+        ]
+
+    assert tuple(new_layer._bracket_axis_rules()) == axis_rules
+    return new_layer

tests/builder/transformations/align_alternate_layers_test.py

@@ -0,0 +1,48 @@
+from __future__ import annotations
+
+from pathlib import Path
+
+import pytest
+
+from glyphsLib import GSFont
+from glyphsLib.builder.transformations.align_alternate_layers import (
+    align_alternate_layers,
+)
+
+
+DATA = Path(__file__).parent.parent.parent / "data"
+
+
+@pytest.mark.parametrize(
+    "test_path",
+    [
+        DATA / "AlignAlternateLayers-g2.glyphs",
+        DATA / "AlignAlternateLayers-g3.glyphs",
+    ],
+)
+def test_align_alternate_layers(test_path):
+    font = GSFont(test_path)
+    # 'Cacute' has 3 master layers and no alternate layers
+    Cacute = font.glyphs["Cacute"]
+    assert len(Cacute.layers) == 3
+    assert all(l._is_master_layer for l in Cacute.layers)
+    assert not any(l._is_bracket_layer() for l in Cacute.layers)
+
+    # it uses a component 'C' which in turn contains 3 additional alternate layers,
+    # plus 'acutecomb.case' which has none
+    C = font.glyphs["C"]
+    assert len([l for l in font.glyphs["C"].layers if l._is_bracket_layer()]) == 3
+    assert not any(l._is_bracket_layer() for l in font.glyphs["acutecomb.case"].layers)
+
+    align_alternate_layers(font)
+
+    # we expect 'Cacute' to now have 3 new alternate layers which have the same
+    # axis coordinates as the ones from 'C'
+    assert len(Cacute.layers) == 6
+    assert len([l for l in Cacute.layers if l._is_master_layer]) == 3
+    assert len([l for l in Cacute.layers if l._is_bracket_layer()]) == 3
+    assert {
+        tuple(l._bracket_axis_rules()) for l in C.layers if l._is_bracket_layer()
+    } == {
+        tuple(l._bracket_axis_rules()) for l in Cacute.layers if l._is_bracket_layer()
+    }

tests/builder/transformations/propagate_anchors_test.py

@@ -5,6 +5,7 @@
 import os.path
 import uuid
 
+import pytest
 from copy import deepcopy
 from datetime import datetime
 from typing import TYPE_CHECKING
@@ -16,6 +17,9 @@
 from glyphsLib.types import Point, Transform
 from glyphsLib.writer import dumps
 
+from glyphsLib.builder.transformations.align_alternate_layers import (
+    align_alternate_layers,
+)
 from glyphsLib.builder.transformations.propagate_anchors import (
     compute_max_component_depths,
     get_xy_rotation,
@@ -787,3 +791,42 @@ def test_real_files():
             assert sorted((a.name, tuple(a.position)) for a in l1.anchors) == sorted(
                 (a.name, tuple(a.position)) for a in l2.anchors
             )
+
+
+@pytest.mark.parametrize(
+    "test_file",
+    [
+        "AlignAlternateLayers-g2.glyphs",
+        "AlignAlternateLayers-g3.glyphs",
+    ],
+)
+def test_propagate_anchors_after_aligining_alternates(test_file):
+    # https://github.com/googlefonts/glyphsLib/issues/1090
+    font = GSFont(os.path.join(DATA, test_file))
+
+    Cacute = font.glyphs["Cacute"]
+    assert not any(l._is_bracket_layer() for l in Cacute.layers)
+    for layer in Cacute.layers:
+        assert len(layer.anchors) == 0
+
+    align_alternate_layers(font)
+    propagate_all_anchors(font)
+
+    # all layers have now 2 anchors, including the new alternates
+    Cacute = font.glyphs["Cacute"]
+    alternate_layers = [l for l in Cacute.layers if l._is_bracket_layer()]
+    assert len(alternate_layers) == 3
+    for layer in Cacute.layers:
+        assert len(layer.anchors) == 2
+        assert [a.name for a in layer.anchors] == ["bottom", "top"]
+
+    # the 'bottom' anchor in one of the alternate layers has different
+    # position than the corresponding anchor in the associated master layer
+    # (i.e. it was propagated and not simply duplicated)
+    alt_layer = alternate_layers[0]
+    alt_bottom = alt_layer.anchors[0]
+    master_layer = Cacute.layers[alt_layer.associatedMasterId]
+    master_bottom = master_layer.anchors[0]
+    assert alt_bottom.name == master_bottom.name == "bottom"
+    assert tuple(alt_bottom.position) == (329, 0)
+    assert tuple(master_bottom.position) == (301, 0)