From 1b97f1fd582cd3944d55d3be127e8390f055bca9 Mon Sep 17 00:00:00 2001
From: Cosimo Lupo <clupo@google.com>
Date: Tue, 23 Jan 2024 16:40:16 +0000
Subject: [PATCH] do not propagate anchors to composite mark glyphs with
 anchors

This is meant to match Glyphs.app's logic, see https://github.com/googlefonts/ufo2ft/issues/802

(A similar fix should be added to glyphsLib's anchor_propagation.py until the latter is superseded by the ufo2ft filter)
---
 Lib/ufo2ft/filters/propagateAnchors.py | 23 ++++++++++++-------
 tests/filters/propagateAnchors_test.py | 31 +++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/Lib/ufo2ft/filters/propagateAnchors.py b/Lib/ufo2ft/filters/propagateAnchors.py
index a346ed1f2..a4147102f 100644
--- a/Lib/ufo2ft/filters/propagateAnchors.py
+++ b/Lib/ufo2ft/filters/propagateAnchors.py
@@ -18,6 +18,7 @@
 from fontTools.misc.transform import Transform
 
 from ufo2ft.filters import BaseFilter
+from ufo2ft.util import OpenTypeCategories
 
 logger = logging.getLogger(__name__)
 
@@ -26,14 +27,14 @@ class PropagateAnchorsFilter(BaseFilter):
     def set_context(self, font, glyphSet):
         ctx = super().set_context(font, glyphSet)
         ctx.processed = set()
+        ctx.categories = OpenTypeCategories.load(font)
         return ctx
 
     def __call__(self, font, glyphSet=None):
-        if super().__call__(font, glyphSet):
-            modified = self.context.modified
-            if modified:
-                logger.info("Glyphs with propagated anchors: %i" % len(modified))
-            return modified
+        modified = super().__call__(font, glyphSet)
+        if modified:
+            logger.info("Glyphs with propagated anchors: %i" % len(modified))
+        return modified
 
     def filter(self, glyph):
         if not glyph.components:
@@ -44,11 +45,12 @@ def filter(self, glyph):
             glyph,
             self.context.processed,
             self.context.modified,
+            self.context.categories,
         )
         return len(glyph.anchors) > before
 
 
-def _propagate_glyph_anchors(glyphSet, composite, processed, modified):
+def _propagate_glyph_anchors(glyphSet, composite, processed, modified, categories):
     """
     Propagate anchors from base glyphs to a given composite
     glyph, and to all composite glyphs used in between.
@@ -58,7 +60,12 @@ def _propagate_glyph_anchors(glyphSet, composite, processed, modified):
         return
     processed.add(composite.name)
 
-    if not composite.components:
+    if not composite.components or (
+        # "If it is a 'mark' and there are anchors, it will not look into components"
+        # Georg said: https://github.com/googlefonts/ufo2ft/issues/802#issuecomment-1904109457
+        composite.name in categories.mark
+        and composite.anchors
+    ):
         return
 
     base_components = []
@@ -74,7 +81,7 @@ def _propagate_glyph_anchors(glyphSet, composite, processed, modified):
                 "in glyph {}".format(component.baseGlyph, composite.name)
             )
         else:
-            _propagate_glyph_anchors(glyphSet, glyph, processed, modified)
+            _propagate_glyph_anchors(glyphSet, glyph, processed, modified, categories)
             if any(a.name.startswith("_") for a in glyph.anchors):
                 mark_components.append(component)
             else:
diff --git a/tests/filters/propagateAnchors_test.py b/tests/filters/propagateAnchors_test.py
index 695f3ba08..6b28682a3 100644
--- a/tests/filters/propagateAnchors_test.py
+++ b/tests/filters/propagateAnchors_test.py
@@ -106,6 +106,25 @@
                         ("addComponent", ("macroncomb", (1, 0, 0, 1, 175, 0))),
                     ],
                 },
+                {
+                    "name": "r",
+                    "width": 350,
+                    "outline": [
+                        ("moveTo", ((0, 0),)),
+                        ("lineTo", ((0, 300),)),
+                        ("lineTo", ((175, 300),)),
+                        ("closePath", ()),
+                    ],
+                    "anchors": [(175, 300, "top"), (175, 0, "bottom")],
+                },
+                {
+                    "name": "rcombbelow",
+                    "width": 0,
+                    "outline": [
+                        ("addComponent", ("r", (0.5, 0, 0, 0.5, -100, -100))),
+                    ],
+                    "anchors": [(0, 0, "_bottom")],
+                },
             ]
         }
     ]
@@ -120,6 +139,12 @@ def font(request, FontClass):
             getattr(pen, operator)(*operands)
         for x, y, name in param.get("anchors", []):
             glyph.appendAnchor(dict(x=x, y=y, name=name))
+    # classify as 'mark' all glyphs with zero width and 'comb' in their name
+    font.lib["public.openTypeCategories"] = {
+        g["name"]: "mark"
+        for g in request.param["glyphs"]
+        if g.get("width", 0) == 0 and "comb" in g["name"]
+    }
     return font
 
 
@@ -149,6 +174,10 @@ def font(request, FontClass):
         ],
         {"a_a"},
     ),
+    # the composite glyph is a mark with anchors, hence propagation is not performed,
+    # i.e. 'top' and 'bottom' are *not* copied to 'rcombbelow':
+    # https://github.com/googlefonts/ufo2ft/issues/802
+    "rcombbelow": ([("_bottom", 0, 0)], set()),
 }
 
 
@@ -173,7 +202,7 @@ def test_include_one_glyph_at_a_time(self, font, name):
     def test_whole_font(self, font):
         philter = PropagateAnchorsFilter()
         modified = philter(font)
-        assert modified == set(EXPECTED)
+        assert modified == {k for k in EXPECTED if k in EXPECTED[k][1]}
         for name, (expected_anchors, _) in EXPECTED.items():
             assert [(a.name, a.x, a.y) for a in font[name].anchors] == expected_anchors