Skip to content

Commit

Permalink
markFeatureWriter: Make contextual lookups only if mark feature is re…
Browse files Browse the repository at this point in the history 
…quested

We were trying to access non-existent features["mark"] when mark
feature was disabled.
  • Loading branch information
@khaledhosny
khaledhosny committed Sep 4, 2024
1 parent 58f28cf commit 472eb74
Show file tree
Hide file tree
Showing 2 changed files with 173 additions and 81 deletions.
169 changes: 88 additions & 81 deletions Lib/ufo2ft/featureWriters/markFeatureWriter.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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:
Expand All @@ -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):
Expand Down
85 changes: 85 additions & 0 deletions tests/featureWriters/markFeatureWriter_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import re
from textwrap import dedent

from ufo2ft.constants import OBJECT_LIBS_KEY

import pytest

from ufo2ft.errors import InvalidFeaturesData
Expand Down Expand Up @@ -1773,6 +1775,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": 100, "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 <anchor 100 200> @MC_top;
markClass tildecomb <anchor 100 200> @MC_top;
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;
"""
)

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
<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;
lookup ContextualMark_0 {
pos base a
<anchor 100 200> 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
<anchor 100 300> 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")
Expand Down

0 comments on commit 472eb74

Please sign in to comment.