Merge remote-tracking branch 'origin/main' into fix-propagate-cursive-anchors

# Conflicts:
#	tests/builder/transformations/propagate_anchors_test.py

Author: anthrotype

Lib/glyphsLib/builder/builders.py

@@ -190,6 +190,27 @@ def __init__(
         self.glyphdata = glyph_data
 
     def _is_vertical(self):
+        # Check for vhea custom parameters at font or master level.
+        # To match ufo2ft, require all three vhea params (ascender, descender, lineGap):
+        # https://github.com/googlefonts/ufo2ft/blob/28acf8870487/Lib/ufo2ft/outlineCompiler.py#L154-L163
+        # https://github.com/googlefonts/glyphsLib/issues/1132
+        # Each pair contains aliases for the same metric
+        vhea_metrics = (
+            {"vheaVertAscender", "vheaVertTypoAscender"},
+            {"vheaVertDescender", "vheaVertTypoDescender"},
+            {"vheaVertLineGap", "vheaVertTypoLineGap"},
+        )
+
+        def has_all_vhea_params(custom_params):
+            names = {p.name for p in custom_params}
+            return all(names & metric for metric in vhea_metrics)
+
+        if has_all_vhea_params(self.font.customParameters) or any(
+            has_all_vhea_params(master.customParameters) for master in self.font.masters
+        ):
+            return True
+
+        # Check for glyph-level vertical attributes
         master_ids = {m.id for m in self.font.masters}
         for glyph in self.font.glyphs:
             for layer in glyph.layers:

Lib/glyphsLib/builder/smart_components.py

@@ -129,8 +129,8 @@ def decompose_smart_components_in_layer(self, layer):
     return new_layer
 
 
-def instantiate_smart_component(self, layer, component, pen):
-    """Instantiate a smart component by interpolating and drawing to a pointPen."""
+def get_smart_component_variation_model(layer, component):
+    """Get the variation model for a smart component."""
     # Find the GSGlyph that is being used as a component by this GSComponent
     root = component.component
 
@@ -149,9 +149,7 @@ def instantiate_smart_component(self, layer, component, pen):
         )
 
     if len(masters) == 1:
-        # Treat this as a dumb component.
-        pen.addComponent(component.name, component.transform)
-        return
+        return None, None, None
 
     model = variation_model(root, masters, layer)
 
@@ -169,6 +167,23 @@ def instantiate_smart_component(self, layer, component, pen):
         name: normalizeValue(value, axes_tuples[name], extrapolate=True)
         for name, value in component.smartComponentValues.items()
     }
+
+    return model, normalized_location, masters
+
+
+def instantiate_smart_component(self, layer, component, pen):
+    """Instantiate a smart component by interpolating and drawing to a pointPen."""
+    # Find the GSGlyph that is being used as a component by this GSComponent
+    root = component.component
+
+    model, normalized_location, masters = get_smart_component_variation_model(
+        layer, component
+    )
+    if model is None:
+        # Treat this as a dumb component.
+        pen.addComponent(component.name, component.transform)
+        return
+
     # Decompose nested smart components before extracting coordinates
     decomposed_masters = [decompose_smart_components_in_layer(self, l) for l in masters]
     coordinates = [get_coordinates(l) for l in decomposed_masters]

Lib/glyphsLib/builder/transformations/propagate_anchors.py

@@ -99,13 +99,20 @@ def maybe_log_new_anchors(
         )
 
 
+def _is_master_layer(layer: GSLayer) -> bool:
+    # Treat smart component layers as master layers
+    return layer._is_master_layer or (
+        layer.parent.smartComponentAxes and layer.smartComponentPoleMapping
+    )
+
+
 def _interesting_layers(glyph):
     # only master layers are currently supported for anchor propagation:
     # https://github.com/googlefonts/glyphsLib/issues/1017
     return (
         l
         for l in glyph.layers
-        if l._is_master_layer or l._is_bracket_layer()
+        if _is_master_layer(l) or l._is_bracket_layer()
         # or l._is_brace_layer
         # etc.
     )
@@ -139,6 +146,42 @@ def _get_subCategory(
     )
 
 
+def _interpolate_smart_component_anchors(
+    layer: GSLayer,
+    component: GSComponent,
+    glyphs: dict[str, GSGlyph],
+    done_anchors: dict[str, dict[str, list[GSAnchor]]],
+    anchors: list[GSAnchor],
+) -> None:
+    from ..smart_components import get_smart_component_variation_model
+    from fontTools.ttLib.tables._g_l_y_f import GlyphCoordinates
+
+    model, location, masters = get_smart_component_variation_model(layer, component)
+    if model is not None:
+        coords = [
+            GlyphCoordinates(
+                [
+                    anchor.position
+                    for anchor in get_component_layer_anchors(
+                        component, master, glyphs, done_anchors
+                    )
+                ]
+            )
+            for master in masters
+        ]
+
+        try:
+            new_coords = model.interpolateFromMasters(location, coords)
+        except Exception as e:
+            raise ValueError(
+                "Could not interpolate smart component %s used in %s"
+                % (component.name, layer)
+            ) from e
+
+        for anchor, new_coord in zip(anchors, new_coords):
+            anchor.position = Point(new_coord[0], new_coord[1])
+
+
 def anchors_traversing_components(
     glyph: GSGlyph,
     layer: GSLayer,
@@ -187,6 +230,12 @@ def anchors_traversing_components(
             )
             continue
 
+        if component.smartComponentValues and component.component.smartComponentAxes:
+            # If this is a smart component, we need to interpolate the anchors
+            _interpolate_smart_component_anchors(
+                layer, component, glyphs, done_anchors, anchors
+            )
+
         # if this component has an explicitly set attachment anchor, use it
         if component_idx > 0 and component.anchor:
             maybe_rename_component_anchor(component.anchor, anchors)
@@ -401,7 +450,7 @@ def get_component_layer_anchors(
     # 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_master = _is_master_layer(layer)
     parent_is_bracket = layer._is_bracket_layer()
     parent_axis_rules = (
         [] if not parent_is_bracket else list(layer._bracket_axis_rules())

tests/builder/builder_test.py

@@ -2857,3 +2857,37 @@ def test_glyphs_to_ufo_with_partial_glyphOrder(self, ufo_module):
         ufo = self.from_glyphs(ufo_module)
         assert ["xxx1", "f", "xxx2", "c", "a"] == ufo.lib["public.glyphOrder"]
         assert GLYPHS_PREFIX + "glyphOrder" not in ufo.lib
+
+
+def test_vhea_custom_params_trigger_is_vertical(ufo_module):
+    """Test that vhea custom parameters (without per-glyph vertical metrics)
+    trigger is_vertical=True and populate glyph heights in UFO.
+
+    https://github.com/googlefonts/glyphsLib/issues/1132
+    """
+    font = generate_minimal_font()
+    font.masters[0].ascender = 800
+    font.masters[0].descender = -200
+
+    # Set vhea custom parameters on master (no per-glyph vertical metrics)
+    font.masters[0].customParameters["vheaVertAscender"] = 500
+    font.masters[0].customParameters["vheaVertDescender"] = -500
+    font.masters[0].customParameters["vheaVertLineGap"] = 0
+
+    # Add a glyph without explicit vertWidth/vertOrigin
+    add_glyph(font, "a")
+
+    builder = UFOBuilder(font, ufo_module=ufo_module)
+    assert builder.is_vertical is True
+
+    ufo = next(iter(builder.masters))
+
+    # Glyph should have height set to typoAscender - typoDescender
+    # (which defaults to master ascender - descender when not explicitly set)
+    expected_height = font.masters[0].ascender - font.masters[0].descender
+    assert ufo["a"].height == expected_height
+
+    # vhea UFO info should be populated
+    assert ufo.info.openTypeVheaVertTypoAscender == 500
+    assert ufo.info.openTypeVheaVertTypoDescender == -500
+    assert ufo.info.openTypeVheaVertTypoLineGap == 0

tests/builder/transformations/propagate_anchors_test.py

@@ -12,7 +12,14 @@
 
 from fontTools.misc.transform import Transform as Affine
 
-from glyphsLib.classes import GSAnchor, GSFont, GSGlyph, GSLayer, GSComponent
+from glyphsLib.classes import (
+    GSAnchor,
+    GSFont,
+    GSGlyph,
+    GSLayer,
+    GSComponent,
+    GSSmartComponentAxis,
+)
 from glyphsLib.glyphdata import get_glyph
 from glyphsLib.types import Point, Transform
 from glyphsLib.writer import dumps
@@ -50,7 +57,7 @@ def build(self) -> dict[str, GSGlyph]:
         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)
+        glyph = GlyphBuilder(name, self.font)
         build_fn(glyph)
         # 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
@@ -60,13 +67,14 @@ def add_glyph(self, name: str, build_fn: Callable[["GlyphBuilder"], None]) -> Se
 
 
 class GlyphBuilder:
-    def __init__(self, name: str):
+    def __init__(self, name: str, font: GSFont = None):
         info = get_glyph(name)
         self.glyph = glyph = GSGlyph()
         glyph.name = name
         glyph.unicode = info.unicode
         glyph.category = info.category
         glyph.subCategory = info.subCategory
+        glyph.parent = font
         self.num_masters = 0
         self.num_alternates = 0
         self.add_master_layer()
@@ -117,9 +125,30 @@ def set_subCategory(self, subCategory: str) -> Self:
         self.glyph.subCategory = subCategory
         return self
 
-    def add_component(self, name: str, pos: tuple[float, float]) -> Self:
+    def add_smartComponentAxis(
+        self, name: str, top_value: float, bottom_value: float
+    ) -> Self:
+        axis = GSSmartComponentAxis()
+        axis.name = name
+        axis.topValue = top_value
+        axis.bottomValue = bottom_value
+        self.glyph.smartComponentAxes.append(axis)
+        return self
+
+    def set_smartComponentPoleMapping(self, name: str, pole: int) -> Self:
+        self.current_layer.smartComponentPoleMapping[name] = pole
+        return self
+
+    def add_component(
+        self,
+        name: str,
+        pos: tuple[float, float],
+        smart_component_values: dict[str, float] = None,
+    ) -> Self:
         component = GSComponent(name, offset=pos)
         self.current_layer.components.append(component)
+        if smart_component_values:
+            component.smartComponentValues = smart_component_values
         return self
 
     def rotate_component(self, degrees: float) -> Self:
@@ -884,3 +913,69 @@ def test_cursive_anchors_ligature():
             ("exit.1", (100, 0)),
         ],
     )
+
+
+def test_smart_component_anchors():
+    glyphs = (
+        GlyphSetBuilder()
+        .add_glyph(
+            "base",
+            lambda glyph: (
+                glyph.add_smartComponentAxis("TEST", 100, -100)
+                .set_smartComponentPoleMapping("TEST", 1)
+                .add_anchor("top", (23, 103))
+                .add_anchor("bottom", (36, -51))
+                .add_backup_layer()
+                .set_smartComponentPoleMapping("TEST", 2)
+                .add_anchor("top", (33, 123))
+                .add_anchor("bottom", (36, -51))
+            ),
+        )
+        .add_glyph(
+            "smart1",
+            lambda glyph: (glyph.add_component("base", (0, 0), {"TEST": -100})),
+        )
+        .add_glyph(
+            "smart2",
+            lambda glyph: (glyph.add_component("base", (0, 0), {"TEST": 100})),
+        )
+        .add_glyph(
+            "smart3",
+            lambda glyph: (glyph.add_component("base", (0, 0), {"TEST": 0})),
+        )
+        .add_glyph(
+            "smart4",
+            lambda glyph: (glyph.add_component("base", (20, 10), {"TEST": 0})),
+        )
+        .build()
+    )
+    propagate_all_anchors_impl(glyphs)
+
+    assert_anchors(
+        glyphs["smart1"].layers[0].anchors,
+        [
+            ("top", (23, 103)),
+            ("bottom", (36, -51)),
+        ],
+    )
+    assert_anchors(
+        glyphs["smart2"].layers[0].anchors,
+        [
+            ("top", (33, 123)),
+            ("bottom", (36, -51)),
+        ],
+    )
+    assert_anchors(
+        glyphs["smart3"].layers[0].anchors,
+        [
+            ("top", (28, 113)),
+            ("bottom", (36, -51)),
+        ],
+    )
+    assert_anchors(
+        glyphs["smart4"].layers[0].anchors,
+        [
+            ("top", (48, 123)),
+            ("bottom", (56, -41)),
+        ],
+    )