Merge pull request #895 from googlefonts/contextual-mkmk

[markFeatureWriter] Support contextual mark2mark anchors

Lib/ufo2ft/featureWriters/markFeatureWriter.py

@@ -727,28 +727,46 @@ def _makeMarkToLigaAttachments(self):
         return result
 
     def _makeContextualAttachments(
-        self, baseClass: Optional[Set[str]], ligatureClass: Optional[Set[str]]
+        self,
+        baseClass: Optional[Set[str]],
+        ligatureClass: Optional[Set[str]],
+        markClass: 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
 
+        markGlyphNames = self.context.markGlyphNames
+
         baseResult = defaultdict(list)
         ligatureResult = defaultdict(list)
+        markResult = 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
 
+                # Mark glyphs go to mkmk lookups
+                if glyphName in markGlyphNames:
+                    # skip anchors for which no mark class is defined
+                    if anchor.markClass is None or anchor.isMark:
+                        continue
+                    if anchor.number is not None:
+                        self.log.warning(
+                            "invalid contextual ligature anchor '%s' in mark glyph '%s'; "
+                            "skipped",
+                            anchor.name,
+                            glyphName,
+                        )
+                        continue
+                    dest = markResult
                 # 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(
+                elif anchor.number is not None and includedOrNoClass(
                     ligatureClass, glyphName
                 ):
                     dest = ligatureResult
@@ -769,7 +787,7 @@ def includedInClass(gdefClass: Optional[Set[str]], glyphName: str) -> bool:
                     )
                     continue
                 dest[anchor_context].append((glyphName, anchor))
-        return baseResult, ligatureResult
+        return baseResult, ligatureResult, markResult
 
     @staticmethod
     def _iterAttachments(attachments, include=None, marksFilter=None):
@@ -910,6 +928,7 @@ def _makeContextualMarkLookup(
         fullcontext,
         refLkps,
         ctxLkps,
+        prefix="ContextualMark",
     ):
         for anchorKey, statements in attachments.items():
             # First make the contextual lookup
@@ -919,9 +938,7 @@ def _makeContextualMarkLookup(
                 before, after = "", fullcontext
             after = after.strip()
             if before not in ctxLkps:
-                ctxLkps[before] = ast.LookupBlock(
-                    f"ContextualMarkDispatch_{len(ctxLkps)}"
-                )
+                ctxLkps[before] = ast.LookupBlock(f"{prefix}Dispatch_{len(ctxLkps)}")
                 if before:
                     # I know it's not really a comment but this is the easiest way
                     # to get the lookup flag in there without reparsing it.
@@ -940,7 +957,7 @@ def _makeContextualMarkLookup(
             contextual = after.replace("*", f"[{baseGlyphNames}]")
 
             # Replace & with mark glyph names
-            refLkpName = f"ContextualMark_{len(refLkps)}"
+            refLkpName = f"{prefix}_{len(refLkps)}"
             contextual = contextual.replace("&", f"{marks}' lookup {refLkpName}")
             ctxLkp.statements.append(ast.Comment(f"pos {contextual};"))
 
@@ -950,16 +967,51 @@ def _makeContextualMarkLookup(
             refLkps.append(refLkp)
 
     def _makeMkmkFeature(self, include):
-        feature = ast.FeatureBlock("mkmk")
-
+        # First make the non-contextual lookups
+        markLkps = []
         for anchorName, attachments in sorted(
             self.context.markToMarkAttachments.items()
         ):
             lkp = self._makeMarkToMarkLookup(anchorName, attachments, include)
             if lkp is not None:
-                feature.statements.append(lkp)
+                markLkps.append(lkp)
 
-        return feature if feature.statements else None
+        # Then make the contextual ones
+        refLkps = []
+        ctxLkps = {}
+        # 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 context, glyph_anchor_pair in sorted(
+            self.context.contextualMarkToMarkAnchors.items(), key=lambda x: -len(x[0])
+        ):
+            # Group by anchor
+            attachments = defaultdict(list)
+            for glyphName, anchor in glyph_anchor_pair:
+                attachments[anchor.key].append(MarkToMarkPos(glyphName, [anchor]))
+            self._makeContextualMarkLookup(
+                attachments,
+                context,
+                refLkps,
+                ctxLkps,
+                prefix="ContextualMarkToMark",
+            )
+
+        ctxLkps = list(ctxLkps.values())
+        if not markLkps and not ctxLkps:
+            return None, []
+
+        feature = ast.FeatureBlock("mkmk")
+        if ctxLkps:
+            lookups = markLkps + refLkps + ctxLkps
+            for lookup in markLkps + ctxLkps:
+                feature.statements.append(ast.LookupReferenceStatement(lookup))
+        else:
+            lookups = []
+            for lookup in markLkps:
+                feature.statements.append(lookup)
+
+        return feature, lookups
 
     def _isAboveMark(self, anchor):
         if anchor.name in self.abvmAnchorNames:
@@ -1048,9 +1100,12 @@ def _makeFeatures(self):
 
         baseClass = self.context.gdefClasses.base
         ligatureClass = self.context.gdefClasses.ligature
-        ctx.contextualMarkToBaseAnchors, ctx.contextualMarkToLigaAnchors = (
-            self._makeContextualAttachments(baseClass, ligatureClass)
-        )
+        markClass = self.context.gdefClasses.mark
+        (
+            ctx.contextualMarkToBaseAnchors,
+            ctx.contextualMarkToLigaAnchors,
+            ctx.contextualMarkToMarkAnchors,
+        ) = self._makeContextualAttachments(baseClass, ligatureClass, markClass)
 
         abvmGlyphs, notAbvmGlyphs = self._getAbvmGlyphs()
 
@@ -1069,9 +1124,10 @@ def isNotAbvm(glyphName):
                 features["mark"] = mark
                 lookups.extend(markLookups)
         if "mkmk" in todo:
-            mkmk = self._makeMkmkFeature(include=isNotAbvm)
+            mkmk, mkmkLookups = self._makeMkmkFeature(include=isNotAbvm)
             if mkmk is not None:
                 features["mkmk"] = mkmk
+                lookups.extend(mkmkLookups)
         if "abvm" in todo or "blwm" in todo:
             if abvmGlyphs:
                 for tag in ("abvm", "blwm"):

tests/featureWriters/markFeatureWriter_test.py

@@ -2086,6 +2086,67 @@ def test_contextual_liga_anchor_no_number(self, testufo):
             """
         )
 
+    def test_contextual_mkmk_anchors(self, testufo):
+        tildecomb = testufo["tildecomb"]
+
+        tildecomb.appendAnchor(
+            {"name": "*top", "x": 120, "y": 400, "identifier": "*top"}
+        )
+        tildecomb.lib[OBJECT_LIBS_KEY] = {
+            "*top": {
+                "GPOS_Context": "f *",
+            },
+        }
+
+        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 mark2mark_top {
+                @MFS_mark2mark_top = [acutecomb tildecomb];
+                lookupflag UseMarkFilteringSet @MFS_mark2mark_top;
+                pos mark tildecomb
+                    <anchor 100 300> mark @MC_top;
+            } mark2mark_top;
+
+            lookup ContextualMarkToMark_0 {
+                pos mark tildecomb
+                    <anchor 120 400> mark @MC_top;
+            } ContextualMarkToMark_0;
+
+            lookup ContextualMarkToMarkDispatch_0 {
+                # f *
+                pos f [tildecomb] @MC_top' lookup ContextualMarkToMark_0;
+            } ContextualMarkToMarkDispatch_0;
+
+            feature mark {
+                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;
+
+            } mark;
+
+            feature mkmk {
+                lookup mark2mark_top;
+                lookup ContextualMarkToMarkDispatch_0;
+            } mkmk;
+            """
+        )
+
     def test_contextual_anchor_no_context(self, testufo, caplog):
         a = testufo["a"]
         a.appendAnchor({"name": "*top", "x": 200, "y": 200, "identifier": "*top"})