From f0798ae6f46db3bd51e8ba4441f04c30aad4fe10 Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Thu, 21 Nov 2024 16:32:32 +0000 Subject: [PATCH 1/3] Refactor MarkFeatureWriter._makeContextualAttachments Make attachments for mark2base and mark2liga in the same pass Coerce numberless liga anchors to mark2base --- .../featureWriters/markFeatureWriter.py | 51 +++++++++++-------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/Lib/ufo2ft/featureWriters/markFeatureWriter.py b/Lib/ufo2ft/featureWriters/markFeatureWriter.py index a71a88b4..dc1b24be 100644 --- a/Lib/ufo2ft/featureWriters/markFeatureWriter.py +++ b/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,38 @@ 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: + + 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 +766,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 +1045,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() From d1f22f7de71b71ff35b7cbcc0f6e810cbe4ceb25 Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Mon, 25 Nov 2024 13:32:05 +0000 Subject: [PATCH 2/3] Document logic --- Lib/ufo2ft/featureWriters/markFeatureWriter.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/ufo2ft/featureWriters/markFeatureWriter.py b/Lib/ufo2ft/featureWriters/markFeatureWriter.py index dc1b24be..9542baba 100644 --- a/Lib/ufo2ft/featureWriters/markFeatureWriter.py +++ b/Lib/ufo2ft/featureWriters/markFeatureWriter.py @@ -746,6 +746,8 @@ def includedInClass(gdefClass: Optional[Set[str]], glyphName: str) -> bool: if not anchor.isContextual: continue + # 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 ): From 4432096310de775b1b9e293b76bf3fdf284de7a1 Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Mon, 25 Nov 2024 17:36:05 +0000 Subject: [PATCH 3/3] Test contextual ligature anchor with no number behaviour --- .../featureWriters/markFeatureWriter_test.py | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/tests/featureWriters/markFeatureWriter_test.py b/tests/featureWriters/markFeatureWriter_test.py index a504f60e..5edcaeff 100644 --- a/tests/featureWriters/markFeatureWriter_test.py +++ b/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 @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; + } mark2liga; + + lookup ContextualMark_0 { + pos base f_i + mark @MC_top; + } ContextualMark_0; + + lookup ContextualMark_1 { + pos ligature f_i + mark @MC_top + ligComponent + ; + } 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 + 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"})