From eb5b8f15d44ff0be346f41e2c95f1e62fd95e3ea Mon Sep 17 00:00:00 2001 From: Khaled Hosny Date: Thu, 5 Sep 2024 20:35:09 +0300 Subject: [PATCH] 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. --- .../featureWriters/markFeatureWriter.py | 194 +++++++++++------- .../featureWriters/markFeatureWriter_test.py | 126 +++++++++++- 2 files changed, 238 insertions(+), 82 deletions(-) diff --git a/Lib/ufo2ft/featureWriters/markFeatureWriter.py b/Lib/ufo2ft/featureWriters/markFeatureWriter.py index 1acf65d1..e3a9f740 100644 --- a/Lib/ufo2ft/featureWriters/markFeatureWriter.py +++ b/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) diff --git a/tests/featureWriters/markFeatureWriter_test.py b/tests/featureWriters/markFeatureWriter_test.py index a0566792..0b21dda6 100644 --- a/tests/featureWriters/markFeatureWriter_test.py +++ b/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 + 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 @MC_top; + markClass tildecomb @MC_top; + + lookup mark2base { + pos base a + mark @MC_top; + } mark2base; + + lookup mark2liga { + pos ligature f_i + mark @MC_top + ligComponent + mark @MC_top; + pos ligature f_l + mark @MC_top + ligComponent + mark @MC_top; + } mark2liga; + + lookup ContextualMark_0 { + pos base a + mark @MC_top; + } ContextualMark_0; + + lookup ContextualMark_1 { + pos ligature f_i + mark @MC_top + ligComponent + ; + pos ligature f_l + + ligComponent + mark @MC_top; + } ContextualMark_1; + + lookup ContextualMark_2 { + pos ligature f_i + mark @MC_top + ligComponent + ; + } 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 {