markFeatureWriter: Support contextual ligature anchors

The current code was assuming all contextual anchors are mark-to-base
and was producing wrong output for ligature anchors. With this change,
contextual ligature anchors are properly supported.

Lib/ufo2ft/featureWriters/markFeatureWriter.py

@@ -725,6 +725,26 @@ def _makeMarkToLigaAttachments(self):
             result.append(MarkToLigaPos(glyphName, ligatureMarks))
         return result
 
+    def _makeContextualAttachments(self, glyphClass, ligature=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:
+                continue
+            for anchor in anchors:
+                if not anchor.isContextual:
+                    continue
+                if ligature and anchor.number is None:
+                    continue
+                if not ligature and anchor.number is not None:
+                    continue
+                anchor_context = anchor.libData["GPOS_Context"].strip()
+                result[anchor_context].append((glyphName, anchor))
+        return result
+
     @staticmethod
     def _iterAttachments(attachments, include=None, marksFilter=None):
         for pos in attachments:
@@ -794,100 +814,116 @@ def _makeMarkFeature(self, include):
             )
             if lookup:
                 ligaLkps.append(lookup)
-        if not baseLkps and not ligaLkps:
-            return
 
-        feature = ast.FeatureBlock("mark")
-        for baseLkp in baseLkps:
-            feature.statements.append(baseLkp)
-        for ligaLkp in ligaLkps:
-            feature.statements.append(ligaLkp)
-        return feature
-
-    def _makeContextualMarkFeature(self, feature):
-        ctx = self.context
-
-        # Arrange by context
-        by_context = defaultdict(list)
-        markGlyphNames = ctx.markGlyphNames
-
-        for glyphName, anchors in sorted(ctx.anchorLists.items()):
-            if glyphName in markGlyphNames:
-                continue
-            for anchor in anchors:
-                if not anchor.isContextual:
-                    continue
-                anchor_context = anchor.libData["GPOS_Context"].strip()
-                by_context[anchor_context].append((glyphName, anchor))
-        if not by_context:
-            return feature, []
-
-        if feature is None:
-            feature = ast.FeatureBlock("mark")
-
-        # Pull the lookups from the feature and replace them with lookup references,
-        # to ensure the order is correct
-        lookups = feature.statements
-        feature.statements = [ast.LookupReferenceStatement(lu) for lu in lookups]
-        dispatch_lookups = {}
+        contextualLookups = {}
+        referencedLookups = []
         # We sort the full context by longest first. This isn't perfect
         # but it gives us the best chance that more specific contexts
         # (typically longer) will take precedence over more general ones.
-        for ix, (fullcontext, glyph_anchor_pair) in enumerate(
-            sorted(by_context.items(), key=lambda x: -len(x[0]))
+        for context, glyph_anchor_pair in sorted(
+            self.context.contextualMarkToBaseAnchors.items(), key=lambda x: -len(x[0])
+        ):
+            # Group base glyphs by anchor
+            glyphNames = defaultdict(list)
+            for glyphName, anchor in glyph_anchor_pair:
+                glyphNames[anchor.key].append(MarkToBasePos(glyphName, [anchor]))
+            self._makeContextualMarkLookup(
+                glyphNames,
+                context,
+                referencedLookups,
+                contextualLookups,
+            )
+
+        for context, glyph_anchor_pair in sorted(
+            self.context.contextualMarkToLigaAnchors.items(), key=lambda x: -len(x[0])
         ):
+            # Group base glyphs by anchor
+            glyphNames = defaultdict(list)
+            for glyphName, anchor in glyph_anchor_pair:
+                marks = [[]] * max(
+                    a.number
+                    for a in self.context.anchorLists[glyphName]
+                    if a.key and a.number is not None
+                )
+                marks[anchor.number - 1] = [anchor]
+                glyphNames[anchor.key].append(MarkToLigaPos(glyphName, marks))
+            self._makeContextualMarkLookup(
+                glyphNames,
+                context,
+                referencedLookups,
+                contextualLookups,
+            )
+
+        contextualLookups = list(contextualLookups.values())
+        if not baseLkps and not ligaLkps and not contextualLookups:
+            return None, []
+
+        feature = ast.FeatureBlock("mark")
+        if contextualLookups:
+            # when we have contextual lookups, we need to make sure that the
+            # contextual and non-contextual lookups are in the right order
+            # and we can’t use nested lookups inside the feature block for
+            # the referenced lookups, so we put all lookups outside the feature
+            # and use lookup references instead.
+            # We should probably always do this, as nested lookups are full of
+            # gotchas, but this will require updating many test expectations.
+            lookups = baseLkps + ligaLkps + referencedLookups + contextualLookups
+            for lookup in baseLkps + ligaLkps + contextualLookups:
+                feature.statements.append(ast.LookupReferenceStatement(lookup))
+        else:
+            lookups = []
+            for lookup in baseLkps + ligaLkps:
+                feature.statements.append(lookup)
+        return feature, lookups
+
+    def _makeContextualMarkLookup(
+        self,
+        glyphNames,
+        fullcontext,
+        referencedLookups,
+        contextualLookups,
+    ):
+        for anchorKey, statements in glyphNames.items():
             # Make the contextual lookup
-            lookupname = "ContextualMark_%i" % ix
+            ix = len(referencedLookups)
+            lookupName = f"ContextualMark_{ix}"
             if ";" in fullcontext:
                 before, after = fullcontext.split(";")
-                # I know it's not really a comment but this is the easiest way
-                # to get the lookup flag in there without reparsing it.
             else:
-                after = fullcontext
-                before = ""
+                before, after = "", fullcontext
             after = after.strip()
-            if before not in dispatch_lookups:
-                dispatch_lookups[before] = ast.LookupBlock(
-                    "ContextualMarkDispatch_%i" % len(dispatch_lookups.keys())
+            if before not in contextualLookups:
+                ix = len(contextualLookups)
+                contextualLookups[before] = ast.LookupBlock(
+                    f"ContextualMarkDispatch_{ix}"
                 )
                 if before:
-                    dispatch_lookups[before].statements.append(
+                    # I know it's not really a comment but this is the easiest way
+                    # to get the lookup flag in there without reparsing it.
+                    contextualLookups[before].statements.append(
                         ast.Comment(f"{before};")
                     )
-                feature.statements.append(
-                    ast.LookupReferenceStatement(dispatch_lookups[before])
-                )
-            lkp = dispatch_lookups[before]
-            lkp.statements.append(ast.Comment(f"# {after}"))
-            lookup = ast.LookupBlock(lookupname)
-            for glyph, anchor in glyph_anchor_pair:
-                lookup.statements.append(MarkToBasePos(glyph, [anchor]).asAST())
-            lookups.append(lookup)
+
+            lookup = ast.LookupBlock(lookupName)
+            lookup.statements = [s.asAST() for s in statements]
+            referencedLookups.append(lookup)
+
+            lookup = contextualLookups[before]
+            lookup.statements.append(ast.Comment(f"# {after}"))
 
             # Insert mark glyph names after base glyph names if not specified otherwise.
             if "&" not in after:
                 after = after.replace("*", "* &")
 
-            # Group base glyphs by anchor
-            glyphs = {}
-            for glyph, anchor in glyph_anchor_pair:
-                glyphs.setdefault(anchor.key, [anchor, []])[1].append(glyph)
-
-            for anchor, bases in glyphs.values():
-                bases = " ".join(bases)
-                marks = ast.GlyphClass(
-                    self.context.markClasses[anchor.key].glyphs.keys()
-                ).asFea()
+            baseGlyphNames = " ".join([s.name for s in statements])
+            marks = ast.MarkClassName(self.context.markClasses[anchorKey]).asFea()
 
-                # Replace * with base glyph names
-                contextual = after.replace("*", f"[{bases}]")
+            # Replace * with base glyph names
+            contextual = after.replace("*", f"[{baseGlyphNames}]")
 
-                # Replace & with mark glyph names
-                contextual = contextual.replace("&", f"{marks}' lookup {lookupname}")
-                lkp.statements.append(ast.Comment(f"pos {contextual}; # {anchor.name}"))
-
-        lookups.extend(dispatch_lookups.values())
-        return feature, lookups
+            # Replace & with mark glyph names
+            contextual = contextual.replace("&", f"{marks}' lookup {lookupName}")
+            lookup.statements.append(ast.Comment(f"pos {contextual};"))
 
     def _makeMkmkFeature(self, include):
         feature = ast.FeatureBlock("mkmk")
@@ -986,6 +1022,15 @@ 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,
+        )
+
         abvmGlyphs, notAbvmGlyphs = self._getAbvmGlyphs()
 
         def isAbvm(glyphName):
@@ -998,8 +1043,7 @@ def isNotAbvm(glyphName):
         lookups = []
         todo = ctx.todo
         if "mark" in todo:
-            mark = self._makeMarkFeature(include=isNotAbvm)
-            mark, markLookups = self._makeContextualMarkFeature(mark)
+            mark, markLookups = self._makeMarkFeature(include=isNotAbvm)
             if mark is not None:
                 features["mark"] = mark
                 lookups.extend(markLookups)

tests/featureWriters/markFeatureWriter_test.py

@@ -1748,8 +1748,8 @@ def test_contextual_anchors(self, FontClass):
             "    lookupflag UseMarrkFilteringSet [twodotshorizontalbelow];\n"
             "    # reh-ar * behDotess-ar.medi &\n"
             "    pos reh-ar [behDotless-ar.init] behDotess-ar.medi"
-            " [dotbelow-ar twodotsverticalbelow-ar twodotshorizontalbelow-ar]'"
-            " lookup ContextualMark_0; # *bottom.twodots\n"
+            " @MC_bottom'"
+            " lookup ContextualMark_0;\n"
             "} ContextualMarkDispatch_0;\n"
         )
 
@@ -1759,8 +1759,8 @@ def test_contextual_anchors(self, FontClass):
             "    lookupflag UseMarrkFilteringSet [twodotsverticalbelow];\n"
             "    # reh-ar *\n"
             "    pos reh-ar [behDotless-ar.init behDotless-ar.init.alt]"
-            " [dotbelow-ar twodotsverticalbelow-ar twodotshorizontalbelow-ar]'"
-            " lookup ContextualMark_1; # *bottom.vtwodots\n"
+            " @MC_bottom'"
+            " lookup ContextualMark_1;\n"
             "} ContextualMarkDispatch_1;\n"
         )
 
@@ -1769,8 +1769,8 @@ def test_contextual_anchors(self, FontClass):
             "lookup ContextualMarkDispatch_2 {\n"
             "    # reh-ar *\n"
             "    pos reh-ar [behDotless-ar.init]"
-            " [dotbelow-ar twodotsverticalbelow-ar twodotshorizontalbelow-ar]'"
-            " lookup ContextualMark_2; # *bottom\n"
+            " @MC_bottom'"
+            " lookup ContextualMark_2;\n"
             "} ContextualMarkDispatch_2;\n"
         )
 
@@ -1835,7 +1835,119 @@ def test_contextual_anchors_no_mark_feature(self, testufo):
 
             lookup ContextualMarkDispatch_0 {
                 # f *
-                pos f [a] [acutecomb tildecomb]' lookup ContextualMark_0; # *top
+                pos f [a] @MC_top' lookup ContextualMark_0;
+            } 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_liga_anchors(self, testufo):
+        a = testufo["a"]
+
+        a.appendAnchor({"name": "*top", "x": 200, "y": 200, "identifier": "*top"})
+        a.lib[OBJECT_LIBS_KEY] = {
+            "*top": {
+                "GPOS_Context": "f *",
+            },
+        }
+
+        fi = testufo["f_i"]
+        fi.appendAnchor(
+            {"name": "*top_1.tilde", "x": 300, "y": 500, "identifier": "*top_1.tilde"}
+        )
+        fi.appendAnchor(
+            {"name": "*top_1.acute", "x": 200, "y": 300, "identifier": "*top_1.acute"}
+        )
+        fi.lib[OBJECT_LIBS_KEY] = {
+            "*top_1.tilde": {
+                "GPOS_Context": "* tildecomb",
+            },
+            "*top_1.acute": {
+                "GPOS_Context": "* acutecomb",
+            },
+        }
+
+        fl = testufo.newGlyph("f_l")
+        fl.appendAnchor({"name": "top_1", "x": 200, "y": 400})
+        fl.appendAnchor({"name": "top_2", "x": 500, "y": 400})
+        fl.appendAnchor({"name": "*top_2", "x": 100, "y": 400, "identifier": "*top_2"})
+        fl.lib[OBJECT_LIBS_KEY] = {
+            "*top_2": {
+                "GPOS_Context": "* tildecomb",
+            },
+        }
+
+        writer = MarkFeatureWriter()
+        feaFile = ast.FeatureFile()
+        assert str(feaFile) == ""
+        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;
+                pos ligature f_l
+                        <anchor 200 400> mark @MC_top
+                    ligComponent
+                        <anchor 500 400> mark @MC_top;
+            } mark2liga;
+
+            lookup ContextualMark_0 {
+                pos base a
+                    <anchor 200 200> mark @MC_top;
+            } ContextualMark_0;
+
+            lookup ContextualMark_1 {
+                pos ligature f_i
+                        <anchor 300 500> mark @MC_top
+                    ligComponent
+                        <anchor NULL>;
+                pos ligature f_l
+                        <anchor NULL>
+                    ligComponent
+                        <anchor 100 400> mark @MC_top;
+            } ContextualMark_1;
+
+            lookup ContextualMark_2 {
+                pos ligature f_i
+                        <anchor 200 300> mark @MC_top
+                    ligComponent
+                        <anchor NULL>;
+            } ContextualMark_2;
+
+            lookup ContextualMarkDispatch_0 {
+                # f *
+                pos f [a] @MC_top' lookup ContextualMark_0;
+                # * tildecomb
+                pos [f_i f_l] @MC_top' lookup ContextualMark_1 tildecomb;
+                # * acutecomb
+                pos [f_i] @MC_top' lookup ContextualMark_2 acutecomb;
             } ContextualMarkDispatch_0;
 
             feature mark {