From c5bbd0630fddf037152be8d696b3a0857569c922 Mon Sep 17 00:00:00 2001 From: Khaled Hosny Date: Wed, 4 Sep 2024 18:56:16 +0300 Subject: [PATCH 1/5] markFeatureWriter: Support contextual anchors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This merges glyphsLib’s ContextualMarkFeatureWriter subclass back into MarkFeatureWriter. This is a mechanical merge with only the bare minimum changes to get things working. Some bug fixes and refactoring will happen in next commits. --- .../featureWriters/markFeatureWriter.py | 128 ++++++++++++++++-- .../featureWriters/markFeatureWriter_test.py | 22 +-- 2 files changed, 134 insertions(+), 16 deletions(-) diff --git a/Lib/ufo2ft/featureWriters/markFeatureWriter.py b/Lib/ufo2ft/featureWriters/markFeatureWriter.py index 9a79bb44..84efaa70 100644 --- a/Lib/ufo2ft/featureWriters/markFeatureWriter.py +++ b/Lib/ufo2ft/featureWriters/markFeatureWriter.py @@ -3,7 +3,7 @@ from collections import OrderedDict, defaultdict from functools import partial -from ufo2ft.constants import INDIC_SCRIPTS, USE_SCRIPTS +from ufo2ft.constants import INDIC_SCRIPTS, OBJECT_LIBS_KEY, USE_SCRIPTS from ufo2ft.featureWriters import BaseFeatureWriter, ast from ufo2ft.util import ( classifyGlyphs, @@ -127,9 +127,15 @@ def parseAnchorName( three elements above. """ number = None + isContextual = False if ignoreRE is not None: anchorName = re.sub(ignoreRE, "", anchorName) + if anchorName[0] == "*": + isContextual = True + anchorName = anchorName[1:] + anchorName = re.sub(r"\..*", "", anchorName) + m = ligaNumRE.match(anchorName) if not m: key = anchorName @@ -156,13 +162,26 @@ def parseAnchorName( else: isMark = False - return isMark, key, number + isIgnorable = not key[0].isalpha() + + return isMark, key, number, isContextual, isIgnorable class NamedAnchor: """A position with a name, and an associated markClass.""" - __slots__ = ("name", "x", "y", "isMark", "key", "number", "markClass") + __slots__ = ( + "name", + "x", + "y", + "isMark", + "key", + "number", + "markClass", + "isContextual", + "isIgnorable", + "libData", + ) # subclasses can customize these to use different anchor naming schemes markPrefix = MARK_PREFIX @@ -170,11 +189,11 @@ class NamedAnchor: ligaSeparator = LIGA_SEPARATOR ligaNumRE = LIGA_NUM_RE - def __init__(self, name, x, y, markClass=None): + def __init__(self, name, x, y, markClass=None, libData=None): self.name = name self.x = x self.y = y - isMark, key, number = parseAnchorName( + isMark, key, number, isContextual, isIgnorable = parseAnchorName( name, markPrefix=self.markPrefix, ligaSeparator=self.ligaSeparator, @@ -190,6 +209,9 @@ def __init__(self, name, x, y, markClass=None): self.key = key self.number = number self.markClass = markClass + self.isContextual = isContextual + self.isIgnorable = isIgnorable + self.libData = libData @property def markAnchorName(self): @@ -357,7 +379,14 @@ def _getAnchorLists(self): "duplicate anchor '%s' in glyph '%s'", anchorName, glyphName ) x, y = self._getAnchor(glyphName, anchorName, anchor=anchor) - a = self.NamedAnchor(name=anchorName, x=x, y=y) + libData = None + if anchor.identifier: + libData = glyph.lib[OBJECT_LIBS_KEY].get(anchor.identifier) + a = self.NamedAnchor(name=anchorName, x=x, y=y, libData=libData) + if a.isContextual and not libData: + continue + if a.isIgnorable: + continue anchorDict[anchorName] = a if anchorDict: result[glyphName] = list(anchorDict.values()) @@ -854,6 +883,7 @@ def _makeAbvmOrBlwmFeature(self, tag, include): def _makeFeatures(self): ctx = self.context + # First do non-contextual lookups ctx.groupedMarkToBaseAttachments = self._groupAttachments( self._makeMarkToBaseAttachments() ) @@ -889,7 +919,88 @@ def isNotAbvm(glyphName): if feature is not None: features[tag] = feature - return features + # Now do the contextual ones + # 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 features, [] + + # Pull the lookups from the feature and replace them with lookup references, + # to ensure the order is correct + lookups = features["mark"].statements + features["mark"].statements = [ + ast.LookupReferenceStatement(lu) for lu in lookups + ] + dispatch_lookups = {} + # 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])) + ): + # Make the contextual lookup + lookupname = "ContextualMark_%i" % 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 = "" + after = after.strip() + if before not in dispatch_lookups: + dispatch_lookups[before] = ast.LookupBlock( + "ContextualMarkDispatch_%i" % len(dispatch_lookups.keys()) + ) + if before: + dispatch_lookups[before].statements.append( + ast.Comment(f"{before};") + ) + features["mark"].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) + + # 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() + + # Replace * with base glyph names + contextual = after.replace("*", f"[{bases}]") + + # 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 features, lookups def _getAbvmGlyphs(self): glyphSet = set(self.getOrderedGlyphSet().keys()) @@ -937,7 +1048,7 @@ def _write(self): newClassDefs = self._makeMarkClassDefinitions() self._setBaseAnchorMarkClasses() - features = self._makeFeatures() + features, lookups = self._makeFeatures() if not features: return False @@ -947,6 +1058,7 @@ def _write(self): feaFile=feaFile, markClassDefs=newClassDefs, features=[features[tag] for tag in sorted(features.keys())], + lookups=lookups, ) return True diff --git a/tests/featureWriters/markFeatureWriter_test.py b/tests/featureWriters/markFeatureWriter_test.py index 75d83078..afbe70af 100644 --- a/tests/featureWriters/markFeatureWriter_test.py +++ b/tests/featureWriters/markFeatureWriter_test.py @@ -34,17 +34,23 @@ def testufo(FontClass): @pytest.mark.parametrize( "input_expected", [ - ("top", (False, "top", None)), - ("top_", (False, "top_", None)), - ("top1", (False, "top1", None)), - ("_bottom", (True, "bottom", None)), - ("bottom_2", (False, "bottom", 2)), - ("top_right_1", (False, "top_right", 1)), + ("top", (False, "top", None, False, False)), + ("top_", (False, "top_", None, False, False)), + ("top1", (False, "top1", None, False, False)), + ("_bottom", (True, "bottom", None, False, False)), + ("bottom_2", (False, "bottom", 2, False, False)), + ("top_right_1", (False, "top_right", 1, False, False)), ], ) def test_parseAnchorName(input_expected): - anchorName, (isMark, key, number) = input_expected - assert parseAnchorName(anchorName) == (isMark, key, number) + anchorName, (isMark, key, number, isContextual, isIgnorable) = input_expected + assert parseAnchorName(anchorName) == ( + isMark, + key, + number, + isContextual, + isIgnorable, + ) def test_parseAnchorName_invalid(): From a6e57b33c60dd403434ca46129b12d0466ea4728 Mon Sep 17 00:00:00 2001 From: Khaled Hosny Date: Wed, 4 Sep 2024 19:01:20 +0300 Subject: [PATCH 2/5] markFeatureWriter: Fix test failures Check `key` is not empty before accessing first character. --- Lib/ufo2ft/featureWriters/markFeatureWriter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/ufo2ft/featureWriters/markFeatureWriter.py b/Lib/ufo2ft/featureWriters/markFeatureWriter.py index 84efaa70..b8c03853 100644 --- a/Lib/ufo2ft/featureWriters/markFeatureWriter.py +++ b/Lib/ufo2ft/featureWriters/markFeatureWriter.py @@ -162,7 +162,7 @@ def parseAnchorName( else: isMark = False - isIgnorable = not key[0].isalpha() + isIgnorable = key and not key[0].isalpha() return isMark, key, number, isContextual, isIgnorable From 38fb4354f86410f703989aa7a8558a7dbcc86696 Mon Sep 17 00:00:00 2001 From: Khaled Hosny Date: Wed, 4 Sep 2024 20:05:04 +0300 Subject: [PATCH 3/5] markFeatureWriter: Port contextual anchor tests from glyphsLib --- .../features.fea | 37 +++++ .../fontinfo.plist | 76 +++++++++ .../glyphs/beh-ar.glif | 9 ++ .../glyphs/behD_otless-ar.fina.glif | 22 +++ .../glyphs/behD_otless-ar.glif | 22 +++ .../glyphs/behD_otless-ar.init.alt.glif | 28 ++++ .../glyphs/behD_otless-ar.init.glif | 40 +++++ .../glyphs/behD_otless-ar.medi.glif | 17 ++ .../glyphs/behT_wodotsbelowD_otabove-ar.glif | 10 ++ .../glyphs/contents.plist | 30 ++++ .../glyphs/dotabove-ar.glif | 24 +++ .../glyphs/dotbelow-ar.glif | 26 +++ .../glyphs/layerinfo.plist | 35 ++++ .../glyphs/reh-ar.glif | 18 +++ .../glyphs/twodotshorizontalbelow-ar.glif | 40 +++++ .../glyphs/twodotsverticalbelow-ar.glif | 40 +++++ .../layercontents.plist | 10 ++ .../lib.plist | 151 ++++++++++++++++++ .../metainfo.plist | 10 ++ .../IgnoreAnchorsTest-Thin.ufo/features.fea | 4 + .../IgnoreAnchorsTest-Thin.ufo/fontinfo.plist | 52 ++++++ .../IgnoreAnchorsTest-Thin.ufo/glyphs/A_.glif | 13 ++ .../glyphs/A_dieresis.glif | 10 ++ .../IgnoreAnchorsTest-Thin.ufo/glyphs/a.glif | 17 ++ .../glyphs/adieresis.glif | 11 ++ .../glyphs/contents.plist | 16 ++ .../glyphs/dieresiscomb.glif | 27 ++++ .../glyphs/layerinfo.plist | 21 +++ .../IgnoreAnchorsTest-Thin.ufo/groups.plist | 22 +++ .../layercontents.plist | 10 ++ .../data/IgnoreAnchorsTest-Thin.ufo/lib.plist | 81 ++++++++++ .../IgnoreAnchorsTest-Thin.ufo/metainfo.plist | 10 ++ .../featureWriters/markFeatureWriter_test.py | 85 ++++++++++ 33 files changed, 1024 insertions(+) create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/features.fea create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/fontinfo.plist create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/beh-ar.glif create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.fina.glif create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.glif create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.init.alt.glif create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.init.glif create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.medi.glif create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behT_wodotsbelowD_otabove-ar.glif create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/contents.plist create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/dotabove-ar.glif create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/dotbelow-ar.glif create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/layerinfo.plist create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/reh-ar.glif create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/twodotshorizontalbelow-ar.glif create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/twodotsverticalbelow-ar.glif create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/layercontents.plist create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/lib.plist create mode 100644 tests/data/ContextualAnchorsTest-Regular.ufo/metainfo.plist create mode 100644 tests/data/IgnoreAnchorsTest-Thin.ufo/features.fea create mode 100644 tests/data/IgnoreAnchorsTest-Thin.ufo/fontinfo.plist create mode 100644 tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/A_.glif create mode 100644 tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/A_dieresis.glif create mode 100644 tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/a.glif create mode 100644 tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/adieresis.glif create mode 100644 tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/contents.plist create mode 100644 tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/dieresiscomb.glif create mode 100644 tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/layerinfo.plist create mode 100644 tests/data/IgnoreAnchorsTest-Thin.ufo/groups.plist create mode 100644 tests/data/IgnoreAnchorsTest-Thin.ufo/layercontents.plist create mode 100644 tests/data/IgnoreAnchorsTest-Thin.ufo/lib.plist create mode 100644 tests/data/IgnoreAnchorsTest-Thin.ufo/metainfo.plist diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/features.fea b/tests/data/ContextualAnchorsTest-Regular.ufo/features.fea new file mode 100644 index 00000000..4188cfc8 --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/features.fea @@ -0,0 +1,37 @@ +# Prefix: Languagesystems +# automatic +languagesystem DFLT dflt; + +languagesystem arab dflt; + + +feature aalt { +# automatic +feature init; +feature medi; +feature fina; + +} aalt; + +feature ccmp { +sub beh-ar by behDotless-ar dotbelow-ar; + +} ccmp; + +feature init { +# automatic +sub behDotless-ar by behDotless-ar.init; + +} init; + +feature medi { +# automatic +sub behDotless-ar by behDotless-ar.medi; + +} medi; + +feature fina { +# automatic +sub behDotless-ar by behDotless-ar.fina; + +} fina; diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/fontinfo.plist b/tests/data/ContextualAnchorsTest-Regular.ufo/fontinfo.plist new file mode 100644 index 00000000..63ee46ba --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/fontinfo.plist @@ -0,0 +1,76 @@ + + + + + ascender + 800 + capHeight + 700 + descender + -200 + familyName + Contextual Anchors Test + guidelines + + + angle + 0 + name + [locked] + x + -126 + y + 90 + + + italicAngle + 0 + openTypeHeadCreated + 2023/07/31 15:25:34 + openTypeOS2Type + + 3 + + postscriptBlueValues + + -12 + 0 + 480 + 492 + 700 + 712 + 800 + 812 + + postscriptOtherBlues + + -212 + -200 + + postscriptStemSnapH + + 80 + 88 + 91 + + postscriptStemSnapV + + 90 + 93 + + postscriptUnderlinePosition + -100 + postscriptUnderlineThickness + 50 + styleName + Regular + unitsPerEm + 1000 + versionMajor + 1 + versionMinor + 0 + xHeight + 480 + + diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/beh-ar.glif b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/beh-ar.glif new file mode 100644 index 00000000..9b02ed8e --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/beh-ar.glif @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.fina.glif b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.fina.glif new file mode 100644 index 00000000..1fdb5d7b --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.fina.glif @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.glif b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.glif new file mode 100644 index 00000000..78f7fb4d --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.glif @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.init.alt.glif b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.init.alt.glif new file mode 100644 index 00000000..90e89917 --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.init.alt.glif @@ -0,0 +1,28 @@ + + + + + + + + + + + + + + + + + + public.objectLibs + + 051ABFBB-3A07-48C5-A5A3-82D4291DEAD1 + + GPOS_Context + lookupflag UseMarrkFilteringSet [twodotsverticalbelow]; reh-ar * + + + + + diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.init.glif b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.init.glif new file mode 100644 index 00000000..abf21cf6 --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.init.glif @@ -0,0 +1,40 @@ + + + + + + + + + + + + + + + + + + + + public.objectLibs + + 50838DB6-85ED-49AE-8935-C6201FC3FCD8 + + GPOS_Context + lookupflag UseMarrkFilteringSet [twodotsverticalbelow]; reh-ar * + + 575ADDBE-67A1-4781-8244-18DEC9396567 + + GPOS_Context + reh-ar * + + 8AEC6EF0-8B9B-481A-AC0E-A95051FD2D12 + + GPOS_Context + lookupflag UseMarrkFilteringSet [twodotshorizontalbelow]; reh-ar * behDotess-ar.medi & + + + + + diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.medi.glif b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.medi.glif new file mode 100644 index 00000000..8d5220a2 --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behD_otless-ar.medi.glif @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + + diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behT_wodotsbelowD_otabove-ar.glif b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behT_wodotsbelowD_otabove-ar.glif new file mode 100644 index 00000000..236b9661 --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/behT_wodotsbelowD_otabove-ar.glif @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/contents.plist b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/contents.plist new file mode 100644 index 00000000..d40b99a4 --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/contents.plist @@ -0,0 +1,30 @@ + + + + + beh-ar + beh-ar.glif + behDotless-ar + behD_otless-ar.glif + behDotless-ar.fina + behD_otless-ar.fina.glif + behDotless-ar.init + behD_otless-ar.init.glif + behDotless-ar.init.alt + behD_otless-ar.init.alt.glif + behDotless-ar.medi + behD_otless-ar.medi.glif + behTwodotsbelowDotabove-ar + behT_wodotsbelowD_otabove-ar.glif + dotabove-ar + dotabove-ar.glif + dotbelow-ar + dotbelow-ar.glif + reh-ar + reh-ar.glif + twodotshorizontalbelow-ar + twodotshorizontalbelow-ar.glif + twodotsverticalbelow-ar + twodotsverticalbelow-ar.glif + + diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/dotabove-ar.glif b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/dotabove-ar.glif new file mode 100644 index 00000000..73d6c94d --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/dotabove-ar.glif @@ -0,0 +1,24 @@ + + + + + + + + + com.schriftgestaltung.Glyphs.ComponentInfo + + + alignment + 1 + index + 0 + name + dotbelow-ar + + + com.schriftgestaltung.Glyphs.originalWidth + 300 + + + diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/dotbelow-ar.glif b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/dotbelow-ar.glif new file mode 100644 index 00000000..2e36ec00 --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/dotbelow-ar.glif @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + + com.schriftgestaltung.Glyphs.originalWidth + 300 + + + diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/layerinfo.plist b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/layerinfo.plist new file mode 100644 index 00000000..6dad622b --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/layerinfo.plist @@ -0,0 +1,35 @@ + + + + + lib + + com.schriftgestaltung.layerId + 3E7589AA-8194-470F-8E2F-13C1C581BE24 + com.schriftgestaltung.layerOrderInGlyph.beh-ar + 1 + com.schriftgestaltung.layerOrderInGlyph.behDotless-ar + 1 + com.schriftgestaltung.layerOrderInGlyph.behDotless-ar.fina + 1 + com.schriftgestaltung.layerOrderInGlyph.behDotless-ar.init + 1 + com.schriftgestaltung.layerOrderInGlyph.behDotless-ar.init.alt + 1 + com.schriftgestaltung.layerOrderInGlyph.behDotless-ar.medi + 1 + com.schriftgestaltung.layerOrderInGlyph.behTwodotsbelowDotabove-ar + 1 + com.schriftgestaltung.layerOrderInGlyph.dotabove-ar + 1 + com.schriftgestaltung.layerOrderInGlyph.dotbelow-ar + 1 + com.schriftgestaltung.layerOrderInGlyph.reh-ar + 1 + com.schriftgestaltung.layerOrderInGlyph.twodotshorizontalbelow-ar + 1 + com.schriftgestaltung.layerOrderInGlyph.twodotsverticalbelow-ar + 1 + + + diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/reh-ar.glif b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/reh-ar.glif new file mode 100644 index 00000000..392b26a4 --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/reh-ar.glif @@ -0,0 +1,18 @@ + + + + + + + + + + + + + + + + + + diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/twodotshorizontalbelow-ar.glif b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/twodotshorizontalbelow-ar.glif new file mode 100644 index 00000000..3dd307f4 --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/twodotshorizontalbelow-ar.glif @@ -0,0 +1,40 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + com.schriftgestaltung.Glyphs.originalWidth + 440 + + + diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/twodotsverticalbelow-ar.glif b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/twodotsverticalbelow-ar.glif new file mode 100644 index 00000000..9b135521 --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/glyphs/twodotsverticalbelow-ar.glif @@ -0,0 +1,40 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + com.schriftgestaltung.Glyphs.originalWidth + 300 + + + diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/layercontents.plist b/tests/data/ContextualAnchorsTest-Regular.ufo/layercontents.plist new file mode 100644 index 00000000..b9c1a4f2 --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/layercontents.plist @@ -0,0 +1,10 @@ + + + + + + public.default + glyphs + + + diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/lib.plist b/tests/data/ContextualAnchorsTest-Regular.ufo/lib.plist new file mode 100644 index 00000000..1ddfd4de --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/lib.plist @@ -0,0 +1,151 @@ + + + + + GSOffsetHorizontal + 45 + GSOffsetMakeStroke + 1 + GSOffsetVertical + 44 + GSRoughenHorizontal + 15 + GSRoughenSegmentLength + 15 + GSRoughenVertical + 10 + com.github.googlei18n.ufo2ft.featureWriters + + + class + CursFeatureWriter + + + class + KernFeatureWriter + + + class + ContextualMarkFeatureWriter + module + glyphsLib.featureWriters.markFeatureWriter + + + class + GdefFeatureWriter + + + com.github.googlei18n.ufo2ft.filters + + + name + eraseOpenCorners + namespace + glyphsLib.filters + pre + + + + com.schriftgestaltung.customParameter.GSFont.Propagate Anchors + 0 + com.schriftgestaltung.customParameter.GSFont.Write DisplayStrings + 0 + com.schriftgestaltung.customParameter.GSFont.Write lastChange + 0 + com.schriftgestaltung.customParameter.GSFont.disablesAutomaticAlignment + + com.schriftgestaltung.customParameter.GSFont.useNiceNames + 1 + com.schriftgestaltung.customParameter.GSFontMaster.TTFStems + + + horizontal + 1 + name + Thin + width + 80 + + + horizontal + 1 + name + Lowercase + width + 88 + + + horizontal + 1 + name + Uppercase + width + 91 + + + com.schriftgestaltung.customParameter.GSFontMaster.customValue + 0 + com.schriftgestaltung.customParameter.GSFontMaster.customValue1 + 0 + com.schriftgestaltung.customParameter.GSFontMaster.customValue2 + 0 + com.schriftgestaltung.customParameter.GSFontMaster.customValue3 + 0 + com.schriftgestaltung.customParameter.GSFontMaster.iconName + + com.schriftgestaltung.customParameter.GSFontMaster.weightValue + 90 + com.schriftgestaltung.customParameter.GSFontMaster.widthValue + 100 + com.schriftgestaltung.fontMasterID + 3E7589AA-8194-470F-8E2F-13C1C581BE24 + com.schriftgestaltung.fontMasterOrder + 1 + com.schriftgestaltung.weightValue + 90 + com.schriftgestaltung.widthValue + 100 + public.glyphOrder + + behDotless-ar + behDotless-ar.fina + behDotless-ar.medi + behDotless-ar.init + behDotless-ar.init.alt + beh-ar + behTwodotsbelowDotabove-ar + reh-ar + dotabove-ar + dotbelow-ar + twodotsverticalbelow-ar + twodotshorizontalbelow-ar + + public.postscriptNames + + beh-ar + uni0628 + behDotless-ar + uni066E + behDotless-ar.fina + uni066E.fina + behDotless-ar.init + uni066E.init + behDotless-ar.init.alt + uni066E.init.alt + behDotless-ar.medi + uni066E.medi + behTwodotsbelowDotabove-ar + uni0754 + dotabove-ar + dotabovear + dotbelow-ar + dotbelowar + reh-ar + uni0631 + twodotshorizontalbelow-ar + twodotshorizontalbelowar + twodotsverticalbelow-ar + twodotsverticalbelowar + + + diff --git a/tests/data/ContextualAnchorsTest-Regular.ufo/metainfo.plist b/tests/data/ContextualAnchorsTest-Regular.ufo/metainfo.plist new file mode 100644 index 00000000..7b8b34ac --- /dev/null +++ b/tests/data/ContextualAnchorsTest-Regular.ufo/metainfo.plist @@ -0,0 +1,10 @@ + + + + + creator + com.github.fonttools.ufoLib + formatVersion + 3 + + diff --git a/tests/data/IgnoreAnchorsTest-Thin.ufo/features.fea b/tests/data/IgnoreAnchorsTest-Thin.ufo/features.fea new file mode 100644 index 00000000..24c3a6b6 --- /dev/null +++ b/tests/data/IgnoreAnchorsTest-Thin.ufo/features.fea @@ -0,0 +1,4 @@ +# Prefix: Languagesystems +languagesystem DFLT dflt; +languagesystem latn dflt; + diff --git a/tests/data/IgnoreAnchorsTest-Thin.ufo/fontinfo.plist b/tests/data/IgnoreAnchorsTest-Thin.ufo/fontinfo.plist new file mode 100644 index 00000000..32b741b5 --- /dev/null +++ b/tests/data/IgnoreAnchorsTest-Thin.ufo/fontinfo.plist @@ -0,0 +1,52 @@ + + + + + ascender + 800 + capHeight + 700 + descender + -200 + familyName + Ignore Anchors Test + italicAngle + 0 + openTypeHeadCreated + 2023/11/22 14:08:36 + openTypeOS2Type + + 3 + + postscriptBlueValues + + -10 + 0 + 470 + 480 + 700 + 710 + 800 + 810 + + postscriptOtherBlues + + -210 + -200 + + postscriptUnderlinePosition + -100 + postscriptUnderlineThickness + 50 + styleName + Thin + unitsPerEm + 1000 + versionMajor + 1 + versionMinor + 0 + xHeight + 470 + + diff --git a/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/A_.glif b/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/A_.glif new file mode 100644 index 00000000..941e324c --- /dev/null +++ b/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/A_.glif @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/A_dieresis.glif b/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/A_dieresis.glif new file mode 100644 index 00000000..495ce133 --- /dev/null +++ b/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/A_dieresis.glif @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/a.glif b/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/a.glif new file mode 100644 index 00000000..34f8ea80 --- /dev/null +++ b/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/a.glif @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + + diff --git a/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/adieresis.glif b/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/adieresis.glif new file mode 100644 index 00000000..0710ad06 --- /dev/null +++ b/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/adieresis.glif @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/contents.plist b/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/contents.plist new file mode 100644 index 00000000..ab734fd5 --- /dev/null +++ b/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/contents.plist @@ -0,0 +1,16 @@ + + + + + A + A_.glif + Adieresis + A_dieresis.glif + a + a.glif + adieresis + adieresis.glif + dieresiscomb + dieresiscomb.glif + + diff --git a/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/dieresiscomb.glif b/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/dieresiscomb.glif new file mode 100644 index 00000000..972f7af1 --- /dev/null +++ b/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/dieresiscomb.glif @@ -0,0 +1,27 @@ + + + + + + + + + + + + + + + + + + + + + + + com.schriftgestaltung.Glyphs.originalWidth + 600 + + + diff --git a/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/layerinfo.plist b/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/layerinfo.plist new file mode 100644 index 00000000..c0b74db5 --- /dev/null +++ b/tests/data/IgnoreAnchorsTest-Thin.ufo/glyphs/layerinfo.plist @@ -0,0 +1,21 @@ + + + + + lib + + com.schriftgestaltung.layerId + C4872ECA-A3A9-40AB-960A-1DB2202F16DE + com.schriftgestaltung.layerOrderInGlyph.A + 1 + com.schriftgestaltung.layerOrderInGlyph.Adieresis + 0 + com.schriftgestaltung.layerOrderInGlyph.a + 1 + com.schriftgestaltung.layerOrderInGlyph.adieresis + 0 + com.schriftgestaltung.layerOrderInGlyph.dieresiscomb + 0 + + + diff --git a/tests/data/IgnoreAnchorsTest-Thin.ufo/groups.plist b/tests/data/IgnoreAnchorsTest-Thin.ufo/groups.plist new file mode 100644 index 00000000..3a57a41f --- /dev/null +++ b/tests/data/IgnoreAnchorsTest-Thin.ufo/groups.plist @@ -0,0 +1,22 @@ + + + + + public.kern1.A + + A + + public.kern1.a + + a + + public.kern2.A + + A + + public.kern2.a + + a + + + diff --git a/tests/data/IgnoreAnchorsTest-Thin.ufo/layercontents.plist b/tests/data/IgnoreAnchorsTest-Thin.ufo/layercontents.plist new file mode 100644 index 00000000..b9c1a4f2 --- /dev/null +++ b/tests/data/IgnoreAnchorsTest-Thin.ufo/layercontents.plist @@ -0,0 +1,10 @@ + + + + + + public.default + glyphs + + + diff --git a/tests/data/IgnoreAnchorsTest-Thin.ufo/lib.plist b/tests/data/IgnoreAnchorsTest-Thin.ufo/lib.plist new file mode 100644 index 00000000..923b1909 --- /dev/null +++ b/tests/data/IgnoreAnchorsTest-Thin.ufo/lib.plist @@ -0,0 +1,81 @@ + + + + + com.github.googlei18n.ufo2ft.featureWriters + + + class + CursFeatureWriter + + + class + KernFeatureWriter + + + class + ContextualMarkFeatureWriter + module + glyphsLib.featureWriters.markFeatureWriter + + + class + GdefFeatureWriter + + + com.github.googlei18n.ufo2ft.filters + + + name + eraseOpenCorners + namespace + glyphsLib.filters + pre + + + + com.schriftgestaltung.customParameter.GSFont.Write DisplayStrings + 0 + com.schriftgestaltung.customParameter.GSFont.Write lastChange + 0 + com.schriftgestaltung.customParameter.GSFont.disablesAutomaticAlignment + + com.schriftgestaltung.customParameter.GSFont.useNiceNames + 1 + com.schriftgestaltung.customParameter.GSFontMaster.customValue + 0 + com.schriftgestaltung.customParameter.GSFontMaster.customValue1 + 0 + com.schriftgestaltung.customParameter.GSFontMaster.customValue2 + 0 + com.schriftgestaltung.customParameter.GSFontMaster.customValue3 + 0 + com.schriftgestaltung.customParameter.GSFontMaster.iconName + Light + com.schriftgestaltung.customParameter.GSFontMaster.weightValue + 100 + com.schriftgestaltung.customParameter.GSFontMaster.widthValue + 100 + com.schriftgestaltung.fontMasterID + C4872ECA-A3A9-40AB-960A-1DB2202F16DE + com.schriftgestaltung.fontMasterOrder + 0 + com.schriftgestaltung.weightValue + 100 + com.schriftgestaltung.widthValue + 100 + public.glyphOrder + + A + Adieresis + a + adieresis + dieresiscomb + + public.postscriptNames + + dieresiscomb + uni0308 + + + diff --git a/tests/data/IgnoreAnchorsTest-Thin.ufo/metainfo.plist b/tests/data/IgnoreAnchorsTest-Thin.ufo/metainfo.plist new file mode 100644 index 00000000..7b8b34ac --- /dev/null +++ b/tests/data/IgnoreAnchorsTest-Thin.ufo/metainfo.plist @@ -0,0 +1,10 @@ + + + + + creator + com.github.fonttools.ufoLib + formatVersion + 3 + + diff --git a/tests/featureWriters/markFeatureWriter_test.py b/tests/featureWriters/markFeatureWriter_test.py index afbe70af..efd0f15a 100644 --- a/tests/featureWriters/markFeatureWriter_test.py +++ b/tests/featureWriters/markFeatureWriter_test.py @@ -1712,6 +1712,91 @@ def test_extra_substitutions(self, FontClass): "uuMatra-oriya.BRACKET.varAlt01]" in str(generated) ) + def test_contextual_anchors(self, FontClass): + dirname = os.path.dirname(os.path.dirname(__file__)) + fontPath = os.path.join(dirname, "data", "ContextualAnchorsTest-Regular.ufo") + testufo = FontClass(fontPath) + + writer = MarkFeatureWriter() + feaFile = ast.FeatureFile() + assert str(feaFile) == "" + assert writer.write(testufo, feaFile) + + assert len(feaFile.markClasses) == 2 + assert "MC_bottom" in feaFile.markClasses + + feature = feaFile.statements[-1] + assert feature.name == "mark" + # note there are two mark2base lookups because ufo2ft v3 generates one lookup + # per mark class (previously 'top' and 'bottom' would go into one lookup) + assert str(feature) == dedent( + """\ + feature mark { + lookup mark2base; + lookup mark2base_1; + lookup ContextualMarkDispatch_0; + lookup ContextualMarkDispatch_1; + lookup ContextualMarkDispatch_2; + } mark; + """ + ) + + lookup = feature.statements[-3].lookup + assert str(lookup) == ( + "lookup ContextualMarkDispatch_0 {\n" + " 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" + "} ContextualMarkDispatch_0;\n" + ) + + lookup = feature.statements[-2].lookup + assert str(lookup) == ( + "lookup ContextualMarkDispatch_1 {\n" + " 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" + "} ContextualMarkDispatch_1;\n" + ) + + lookup = feature.statements[-1].lookup + assert str(lookup) == ( + "lookup ContextualMarkDispatch_2 {\n" + " # reh-ar *\n" + " pos reh-ar [behDotless-ar.init]" + " [dotbelow-ar twodotsverticalbelow-ar twodotshorizontalbelow-ar]'" + " lookup ContextualMark_2; # *bottom\n" + "} ContextualMarkDispatch_2;\n" + ) + + def test_ignorable_anchors(self, FontClass): + dirname = os.path.dirname(os.path.dirname(__file__)) + fontPath = os.path.join(dirname, "data", "IgnoreAnchorsTest-Thin.ufo") + testufo = FontClass(fontPath) + + writer = MarkFeatureWriter() + feaFile = ast.FeatureFile() + assert str(feaFile) == "" + assert writer.write(testufo, feaFile) + + assert len(feaFile.markClasses) == 1 + assert "MC_top" in feaFile.markClasses + + feature = feaFile.statements[-2] + assert feature.name == "mark" + assert len(feature.statements) == 1 + + lookup = feature.statements[0] + assert len(lookup.statements) == 4 + for statement in lookup.statements: + assert isinstance(statement, ast.MarkBasePosStatement) + assert len(statement.marks) == 1 + assert statement.marks[0][1].name == "MC_top" + if __name__ == "__main__": import sys From fd0b5e3175e927045497126b5488dfe71c7b23d0 Mon Sep 17 00:00:00 2001 From: Khaled Hosny Date: Wed, 4 Sep 2024 20:16:20 +0300 Subject: [PATCH 4/5] markFeatureWriter: Make contextual lookups only if mark feature is requested We were trying to access non-existent features["mark"] when mark feature was disabled. --- .../featureWriters/markFeatureWriter.py | 169 +++++++++--------- .../featureWriters/markFeatureWriter_test.py | 84 +++++++++ 2 files changed, 172 insertions(+), 81 deletions(-) diff --git a/Lib/ufo2ft/featureWriters/markFeatureWriter.py b/Lib/ufo2ft/featureWriters/markFeatureWriter.py index b8c03853..16df5b4d 100644 --- a/Lib/ufo2ft/featureWriters/markFeatureWriter.py +++ b/Lib/ufo2ft/featureWriters/markFeatureWriter.py @@ -795,6 +795,91 @@ def _makeMarkFeature(self, include): 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 = {} + # 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])) + ): + # Make the contextual lookup + lookupname = "ContextualMark_%i" % 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 = "" + after = after.strip() + if before not in dispatch_lookups: + dispatch_lookups[before] = ast.LookupBlock( + "ContextualMarkDispatch_%i" % len(dispatch_lookups.keys()) + ) + if before: + dispatch_lookups[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) + + # 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() + + # Replace * with base glyph names + contextual = after.replace("*", f"[{bases}]") + + # 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 + def _makeMkmkFeature(self, include): feature = ast.FeatureBlock("mkmk") @@ -901,11 +986,14 @@ def isNotAbvm(glyphName): return glyphName in notAbvmGlyphs features = {} + lookups = [] todo = ctx.todo if "mark" in todo: mark = self._makeMarkFeature(include=isNotAbvm) + mark, markLookups = self._makeContextualMarkFeature(mark) if mark is not None: features["mark"] = mark + lookups.extend(markLookups) if "mkmk" in todo: mkmk = self._makeMkmkFeature(include=isNotAbvm) if mkmk is not None: @@ -919,87 +1007,6 @@ def isNotAbvm(glyphName): if feature is not None: features[tag] = feature - # Now do the contextual ones - # 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 features, [] - - # Pull the lookups from the feature and replace them with lookup references, - # to ensure the order is correct - lookups = features["mark"].statements - features["mark"].statements = [ - ast.LookupReferenceStatement(lu) for lu in lookups - ] - dispatch_lookups = {} - # 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])) - ): - # Make the contextual lookup - lookupname = "ContextualMark_%i" % 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 = "" - after = after.strip() - if before not in dispatch_lookups: - dispatch_lookups[before] = ast.LookupBlock( - "ContextualMarkDispatch_%i" % len(dispatch_lookups.keys()) - ) - if before: - dispatch_lookups[before].statements.append( - ast.Comment(f"{before};") - ) - features["mark"].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) - - # 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() - - # Replace * with base glyph names - contextual = after.replace("*", f"[{bases}]") - - # 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 features, lookups def _getAbvmGlyphs(self): diff --git a/tests/featureWriters/markFeatureWriter_test.py b/tests/featureWriters/markFeatureWriter_test.py index efd0f15a..47c50920 100644 --- a/tests/featureWriters/markFeatureWriter_test.py +++ b/tests/featureWriters/markFeatureWriter_test.py @@ -5,6 +5,7 @@ import pytest +from ufo2ft.constants import OBJECT_LIBS_KEY from ufo2ft.errors import InvalidFeaturesData from ufo2ft.featureCompiler import FeatureCompiler, parseLayoutFeatures from ufo2ft.featureWriters import ast @@ -1773,6 +1774,89 @@ def test_contextual_anchors(self, FontClass): "} ContextualMarkDispatch_2;\n" ) + def test_contextual_anchors_no_mark_feature(self, testufo): + a = testufo["a"] + + a.appendAnchor({"name": "*top", "x": 200, "y": 200, "identifier": "*top"}) + a.lib[OBJECT_LIBS_KEY] = { + "*top": { + "GPOS_Context": "f *", + }, + } + + writer = MarkFeatureWriter(features=["mkmk"]) + feaFile = ast.FeatureFile() + assert str(feaFile) == "" + assert writer.write(testufo, feaFile) + + assert str(feaFile) == dedent( + """\ + markClass acutecomb @MC_top; + markClass tildecomb @MC_top; + + feature mkmk { + lookup mark2mark_top { + @MFS_mark2mark_top = [acutecomb tildecomb]; + lookupflag UseMarkFilteringSet @MFS_mark2mark_top; + pos mark tildecomb + mark @MC_top; + } mark2mark_top; + + } mkmk; + """ + ) + + 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 + mark @MC_top; + } mark2base; + + lookup mark2liga { + pos ligature f_i + mark @MC_top + ligComponent + mark @MC_top; + } mark2liga; + + lookup ContextualMark_0 { + pos base a + mark @MC_top; + } ContextualMark_0; + + lookup ContextualMarkDispatch_0 { + # f * + pos f [a] [acutecomb tildecomb]' lookup ContextualMark_0; # *top + } 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_ignorable_anchors(self, FontClass): dirname = os.path.dirname(os.path.dirname(__file__)) fontPath = os.path.join(dirname, "data", "IgnoreAnchorsTest-Thin.ufo") From e6ec270a31bd6e332b0839b78ec7a8c42c82cd78 Mon Sep 17 00:00:00 2001 From: Khaled Hosny Date: Wed, 4 Sep 2024 22:20:38 +0300 Subject: [PATCH 5/5] markFeatureWriter: Ignore contextual anchors in non-contextual lookups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise we end up with positioning statements that have duplicated mark classes like this: pos base a mark @MC_top mark @MC_top; (one is the regular anchor, and the other is the contextual one). Which makes no sense (feaLib shouldn’t probably allow the same mark class to be used multiple times in the same statement). --- Lib/ufo2ft/featureWriters/markFeatureWriter.py | 9 +++++++++ tests/featureWriters/markFeatureWriter_test.py | 1 - 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Lib/ufo2ft/featureWriters/markFeatureWriter.py b/Lib/ufo2ft/featureWriters/markFeatureWriter.py index 16df5b4d..1acf65d1 100644 --- a/Lib/ufo2ft/featureWriters/markFeatureWriter.py +++ b/Lib/ufo2ft/featureWriters/markFeatureWriter.py @@ -649,6 +649,9 @@ def _makeMarkToBaseAttachments(self): # skip '_1', '_2', etc. suffixed anchors for this lookup # type; these will be are added in the mark2liga lookup continue + if anchor.isContextual: + # skip contextual anchors. They are handled separately. + continue assert not anchor.isMark baseMarks.append(anchor) if not baseMarks: @@ -669,6 +672,9 @@ def _makeMarkToMarkAttachments(self): # skip anchors for which no mark class is defined if anchor.markClass is None or anchor.isMark: continue + if anchor.isContextual: + # skip contextual anchors. They are handled separately. + continue if anchor.number is not None: self.log.warning( "invalid ligature anchor '%s' in mark glyph '%s'; " "skipped", @@ -700,6 +706,9 @@ def _makeMarkToLigaAttachments(self): if number is None: # we handled these in the mark2base lookup continue + if anchor.isContextual: + # skip contextual anchors. They are handled separately. + continue # unnamed anchors with only a number suffix "_1", "_2", etc. # are understood as the ligature component having if not anchor.key: diff --git a/tests/featureWriters/markFeatureWriter_test.py b/tests/featureWriters/markFeatureWriter_test.py index 47c50920..a0566792 100644 --- a/tests/featureWriters/markFeatureWriter_test.py +++ b/tests/featureWriters/markFeatureWriter_test.py @@ -1818,7 +1818,6 @@ def test_contextual_anchors_no_mark_feature(self, testufo): lookup mark2base { pos base a - mark @MC_top mark @MC_top; } mark2base;