Merge pull request #890 from daltonmaag/fix-dropped-contextual-ligatu…

…re-mark-attachment

Fix ligatures with unnumbered contextual anchors having their mark attachment dropped

Lib/ufo2ft/featureWriters/markFeatureWriter.py

@@ -2,6 +2,7 @@
 import re
 from collections import OrderedDict, defaultdict
 from functools import partial
+from typing import Dict, Optional, Set, Tuple
 
 from ufo2ft.constants import INDIC_SCRIPTS, OBJECT_LIBS_KEY, USE_SCRIPTS
 from ufo2ft.featureWriters import BaseFeatureWriter, ast
@@ -725,25 +726,40 @@ def _makeMarkToLigaAttachments(self):
             result.append(MarkToLigaPos(glyphName, ligatureMarks))
         return result
 
-    def _makeContextualAttachments(self, glyphClass, liga=False):
-        ctx = self.context
-        result = defaultdict(list)
-        markGlyphNames = ctx.markGlyphNames
-        for glyphName, anchors in sorted(ctx.anchorLists.items()):
-            if glyphName in markGlyphNames:
-                continue
-            if glyphClass and glyphName not in glyphClass:
+    def _makeContextualAttachments(
+        self, baseClass: Optional[Set[str]], ligatureClass: Optional[Set[str]]
+    ) -> Tuple[Dict[str, Tuple[str, NamedAnchor]], Dict[str, Tuple[str, NamedAnchor]]]:
+        def includedOrNoClass(gdefClass: Optional[Set[str]], glyphName: str) -> bool:
+            return glyphName in gdefClass if gdefClass is not None else True
+
+        def includedInClass(gdefClass: Optional[Set[str]], glyphName: str) -> bool:
+            return glyphName in gdefClass if gdefClass is not None else False
+
+        baseResult = defaultdict(list)
+        ligatureResult = defaultdict(list)
+
+        for glyphName, anchors in sorted(self.context.anchorLists.items()):
+            if glyphName in self.context.markGlyphNames:
                 continue
             for anchor in anchors:
                 # Skip non-contextual anchors
                 if not anchor.isContextual:
                     continue
-                # If we are building the mark2liga lookup, skip anchors without a number
-                if liga and anchor.number is None:
-                    continue
-                # If we are building the mark2base lookup, skip anchors with a number
-                if not liga and anchor.number is not None:
+
+                # See "after" truth table for what this logic hopes to achieve:
+                # https://github.com/googlefonts/ufo2ft/pull/890#issuecomment-2498032081
+                if anchor.number is not None and includedOrNoClass(
+                    ligatureClass, glyphName
+                ):
+                    dest = ligatureResult
+                elif anchor.number is None and (
+                    includedOrNoClass(baseClass, glyphName)
+                    or includedInClass(ligatureClass, glyphName)
+                ):
+                    dest = baseResult
+                else:
                     continue
+
                 anchor_context = anchor.libData.get("GPOS_Context", "").strip()
                 if not anchor_context:
                     self.log.warning(
@@ -752,8 +768,8 @@ def _makeContextualAttachments(self, glyphClass, liga=False):
                         glyphName,
                     )
                     continue
-                result[anchor_context].append((glyphName, anchor))
-        return result
+                dest[anchor_context].append((glyphName, anchor))
+        return baseResult, ligatureResult
 
     @staticmethod
     def _iterAttachments(attachments, include=None, marksFilter=None):
@@ -1031,12 +1047,9 @@ def _makeFeatures(self):
         ctx.markToMarkAttachments = self._makeMarkToMarkAttachments()
 
         baseClass = self.context.gdefClasses.base
-        ctx.contextualMarkToBaseAnchors = self._makeContextualAttachments(baseClass)
-
         ligatureClass = self.context.gdefClasses.ligature
-        ctx.contextualMarkToLigaAnchors = self._makeContextualAttachments(
-            ligatureClass,
-            True,
+        ctx.contextualMarkToBaseAnchors, ctx.contextualMarkToLigaAnchors = (
+            self._makeContextualAttachments(baseClass, ligatureClass)
         )
 
         abvmGlyphs, notAbvmGlyphs = self._getAbvmGlyphs()

tests/featureWriters/markFeatureWriter_test.py

@@ -2011,6 +2011,81 @@ def test_contextual_liga_anchors(self, testufo):
             """
         )
 
+    def test_contextual_liga_anchor_no_number(self, testufo):
+        fi = testufo["f_i"]
+        fi.appendAnchor(
+            {"name": "*top.tilde", "x": 300, "y": 500, "identifier": "*top.tilde"}
+        )
+        fi.appendAnchor(
+            {"name": "*top_1.acute", "x": 200, "y": 300, "identifier": "*top_1.acute"}
+        )
+        fi.lib[OBJECT_LIBS_KEY] = {
+            "*top.tilde": {
+                "GPOS_Context": "* tildecomb",
+            },
+            "*top_1.acute": {
+                "GPOS_Context": "* acutecomb",
+            },
+        }
+
+        writer = MarkFeatureWriter()
+        feaFile = ast.FeatureFile()
+        assert writer.write(testufo, feaFile)
+
+        assert str(feaFile) == dedent(
+            """\
+            markClass acutecomb <anchor 100 200> @MC_top;
+            markClass tildecomb <anchor 100 200> @MC_top;
+
+            lookup mark2base {
+                pos base a
+                    <anchor 100 200> mark @MC_top;
+            } mark2base;
+
+            lookup mark2liga {
+                pos ligature f_i
+                        <anchor 100 500> mark @MC_top
+                    ligComponent
+                        <anchor 600 500> mark @MC_top;
+            } mark2liga;
+
+            lookup ContextualMark_0 {
+                pos base f_i
+                    <anchor 300 500> mark @MC_top;
+            } ContextualMark_0;
+
+            lookup ContextualMark_1 {
+                pos ligature f_i
+                        <anchor 200 300> mark @MC_top
+                    ligComponent
+                        <anchor NULL>;
+            } ContextualMark_1;
+
+            lookup ContextualMarkDispatch_0 {
+                # * tildecomb
+                pos [f_i] @MC_top' lookup ContextualMark_0 tildecomb;
+                # * acutecomb
+                pos [f_i] @MC_top' lookup ContextualMark_1 acutecomb;
+            } ContextualMarkDispatch_0;
+
+            feature mark {
+                lookup mark2base;
+                lookup mark2liga;
+                lookup ContextualMarkDispatch_0;
+            } mark;
+
+            feature mkmk {
+                lookup mark2mark_top {
+                    @MFS_mark2mark_top = [acutecomb tildecomb];
+                    lookupflag UseMarkFilteringSet @MFS_mark2mark_top;
+                    pos mark tildecomb
+                        <anchor 100 300> mark @MC_top;
+                } mark2mark_top;
+
+            } mkmk;
+            """
+        )
+
     def test_contextual_anchor_no_context(self, testufo, caplog):
         a = testufo["a"]
         a.appendAnchor({"name": "*top", "x": 200, "y": 200, "identifier": "*top"})