From b9f2482c42383b176dd61dc03bb8839131db5620 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Wed, 15 Jul 2020 17:06:12 +0100 Subject: [PATCH 1/3] Add a ChainContextualRule class --- Lib/fontTools/otlLib/builder.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Lib/fontTools/otlLib/builder.py b/Lib/fontTools/otlLib/builder.py index 5f0b3f781..75cad019c 100644 --- a/Lib/fontTools/otlLib/builder.py +++ b/Lib/fontTools/otlLib/builder.py @@ -257,6 +257,14 @@ class AlternateSubstBuilder(LookupBuilder): self.alternates[(self.SUBTABLE_BREAK_, location)] = self.SUBTABLE_BREAK_ +class ChainContextualRule( + namedtuple("ChainContextualRule", ["prefix", "glyphs", "suffix", "lookups"]) +): + @property + def is_subtable_break(self): + return self.prefix == LookupBuilder.SUBTABLE_BREAK_ + + class ChainContextualRuleset: def __init__(self): self.rules = [] From 85edf0e380345e3f419a129278a36339596e79ec Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Wed, 15 Jul 2020 17:07:19 +0100 Subject: [PATCH 2/3] Use the ChainContextualRule class instead of bare tuples --- Lib/fontTools/feaLib/builder.py | 23 ++++++++++++----- Lib/fontTools/otlLib/builder.py | 45 ++++++++++++++++----------------- Tests/otlLib/builder_test.py | 14 +++++----- 3 files changed, 45 insertions(+), 37 deletions(-) diff --git a/Lib/fontTools/feaLib/builder.py b/Lib/fontTools/feaLib/builder.py index 70ef6851b..00c6d85bd 100644 --- a/Lib/fontTools/feaLib/builder.py +++ b/Lib/fontTools/feaLib/builder.py @@ -23,6 +23,7 @@ from fontTools.otlLib.builder import ( ClassPairPosSubtableBuilder, PairPosBuilder, SinglePosBuilder, + ChainContextualRule, ) from fontTools.otlLib.error import OpenTypeLibError from collections import defaultdict @@ -948,13 +949,17 @@ class Builder(object): def add_chain_context_pos(self, location, prefix, glyphs, suffix, lookups): lookup = self.get_lookup_(location, ChainContextPosBuilder) lookup.rules.append( - (prefix, glyphs, suffix, self.find_lookup_builders_(lookups)) + ChainContextualRule( + prefix, glyphs, suffix, self.find_lookup_builders_(lookups) + ) ) def add_chain_context_subst(self, location, prefix, glyphs, suffix, lookups): lookup = self.get_lookup_(location, ChainContextSubstBuilder) lookup.rules.append( - (prefix, glyphs, suffix, self.find_lookup_builders_(lookups)) + ChainContextualRule( + prefix, glyphs, suffix, self.find_lookup_builders_(lookups) + ) ) def add_alternate_subst(self, location, prefix, glyph, suffix, replacement): @@ -965,7 +970,7 @@ class Builder(object): if prefix or suffix: chain = self.get_lookup_(location, ChainContextSubstBuilder) lookup = self.get_chained_lookup_(location, AlternateSubstBuilder) - chain.rules.append((prefix, [{glyph}], suffix, [lookup])) + chain.rules.append(ChainContextualRule(prefix, [{glyph}], suffix, [lookup])) else: lookup = self.get_lookup_(location, AlternateSubstBuilder) if glyph in lookup.alternates: @@ -1024,7 +1029,7 @@ class Builder(object): if prefix or suffix or forceChain: chain = self.get_lookup_(location, ChainContextSubstBuilder) lookup = self.get_chained_lookup_(location, LigatureSubstBuilder) - chain.rules.append((prefix, glyphs, suffix, [lookup])) + chain.rules.append(ChainContextualRule(prefix, glyphs, suffix, [lookup])) else: lookup = self.get_lookup_(location, LigatureSubstBuilder) @@ -1043,7 +1048,7 @@ class Builder(object): chain = self.get_lookup_(location, ChainContextSubstBuilder) sub = self.get_chained_lookup_(location, MultipleSubstBuilder) sub.mapping[glyph] = replacements - chain.rules.append((prefix, [{glyph}], suffix, [sub])) + chain.rules.append(ChainContextualRule(prefix, [{glyph}], suffix, [sub])) return lookup = self.get_lookup_(location, MultipleSubstBuilder) if glyph in lookup.mapping: @@ -1100,7 +1105,9 @@ class Builder(object): if sub is None: sub = self.get_chained_lookup_(location, SingleSubstBuilder) sub.mapping.update(mapping) - chain.rules.append((prefix, [mapping.keys()], suffix, [sub])) + chain.rules.append( + ChainContextualRule(prefix, [list(mapping.keys())], suffix, [sub]) + ) def add_cursive_pos(self, location, glyphclass, entryAnchor, exitAnchor): lookup = self.get_lookup_(location, CursivePosBuilder) @@ -1206,7 +1213,9 @@ class Builder(object): sub.add_pos(location, glyph, otValue) subs.append(sub) assert len(pos) == len(subs), (pos, subs) - chain.rules.append((prefix, [g for g, v in pos], suffix, subs)) + chain.rules.append( + ChainContextualRule(prefix, [g for g, v in pos], suffix, subs) + ) def setGlyphClass_(self, location, glyph, glyphClass): oldClass, oldLocation = self.glyphClassDefs_.get(glyph, (None, None)) diff --git a/Lib/fontTools/otlLib/builder.py b/Lib/fontTools/otlLib/builder.py index 75cad019c..a9d13ec6b 100644 --- a/Lib/fontTools/otlLib/builder.py +++ b/Lib/fontTools/otlLib/builder.py @@ -269,15 +269,15 @@ class ChainContextualRuleset: def __init__(self): self.rules = [] - def addRule(self, prefix, glyphs, suffix, lookups): - self.rules.append((prefix, glyphs, suffix, lookups)) + def addRule(self, rule): + self.rules.append(rule) @property def hasPrefixOrSuffix(self): # Do we have any prefixes/suffixes? If this is False for all # rulesets, we can express the whole lookup as GPOS5/GSUB7. - for (prefix, glyphs, suffix, lookups) in self.rules: - if len(prefix) > 0 or len(suffix) > 0: + for rule in self.rules: + if len(rule.prefix) > 0 or len(rule.suffix) > 0: return True return False @@ -285,8 +285,8 @@ class ChainContextualRuleset: def hasAnyGlyphClasses(self): # Do we use glyph classes anywhere in the rules? If this is False # we can express this subtable as a Format 1. - for (prefix, glyphs, suffix, lookups) in self.rules: - for coverage in (prefix, glyphs, suffix): + for rule in self.rules: + for coverage in (rule.prefix, rule.glyphs, rule.suffix): if any(len(x) > 1 for x in coverage): return True return False @@ -322,11 +322,11 @@ class ChainContextualBuilder(LookupBuilder): # Return a list of ChainContextRuleset objects, taking explicit # subtable breaks into account ruleset = [ChainContextualRuleset()] - for (prefix, glyphs, suffix, lookups) in self.rules: - if prefix == self.SUBTABLE_BREAK_: + for rule in self.rules: + if rule.is_subtable_break: ruleset.append(ChainContextualRuleset()) continue - ruleset[-1].addRule(prefix, glyphs, suffix, lookups) + ruleset[-1].addRule(rule) # Squish any empty subtables return [x for x in ruleset if len(x.rules) > 0] @@ -349,17 +349,16 @@ class ChainContextualBuilder(LookupBuilder): return self.buildLookup_(subtables) def buildFormat3Subtable(self, rule, chaining=True): - (prefix, glyphs, suffix, lookups) = rule st = self.newSubtable_(chaining=chaining) st.Format = 3 if chaining: - self.setBacktrackCoverage_(prefix, st) - self.setLookAheadCoverage_(suffix, st) - self.setInputCoverage_(glyphs, st) + self.setBacktrackCoverage_(rule.prefix, st) + self.setLookAheadCoverage_(rule.suffix, st) + self.setInputCoverage_(rule.glyphs, st) else: - self.setCoverage_(glyphs, st) + self.setCoverage_(rule.glyphs, st) - for sequenceIndex, lookupList in enumerate(lookups): + for sequenceIndex, lookupList in enumerate(rule.lookups): if lookupList is not None: if not isinstance(lookupList, list): # Can happen with synthesised lookups @@ -382,7 +381,7 @@ class ChainContextualBuilder(LookupBuilder): def add_subtable_break(self, location): self.rules.append( - ( + ChainContextualRule( self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_, @@ -463,7 +462,7 @@ class ChainContextPosBuilder(ChainContextualBuilder): def __init__(self, font, location): LookupBuilder.__init__(self, font, location, "GPOS", 8) - self.rules = [] # (prefix, input, suffix, lookups) + self.rules = [] self.subtable_type = "Pos" def find_chainable_single_pos(self, lookups, glyphs, value): @@ -513,10 +512,10 @@ class ChainContextSubstBuilder(ChainContextualBuilder): def getAlternateGlyphs(self): result = {} - for (prefix, _, _, lookuplist) in self.rules: - if prefix == self.SUBTABLE_BREAK_: + for rule in self.rules: + if rule.is_subtable_break: continue - for lookups in lookuplist: + for lookups in rule.lookups: if not isinstance(lookups, list): lookups = [lookups] for lookup in lookups: @@ -529,10 +528,10 @@ class ChainContextSubstBuilder(ChainContextualBuilder): def find_chainable_single_subst(self, glyphs): """Helper for add_single_subst_chained_()""" res = None - for prefix, _, _, rules in self.rules[::-1]: - if prefix == self.SUBTABLE_BREAK_: + for rule in self.rules[::-1]: + if rule.is_subtable_break: return res - for sub in rules: + for sub in rule.lookups: if isinstance(sub, SingleSubstBuilder) and not any( g in glyphs for g in sub.mapping.keys() ): diff --git a/Tests/otlLib/builder_test.py b/Tests/otlLib/builder_test.py index 54de6fccb..bdfc64509 100644 --- a/Tests/otlLib/builder_test.py +++ b/Tests/otlLib/builder_test.py @@ -1398,28 +1398,28 @@ class ChainContextualRulesetTest(object): font.setGlyphOrder(["a","b","c","d","A","B","C","D","E"]) sb = builder.ChainContextSubstBuilder(font, None) prefix, input_, suffix, lookups = [["a"], ["b"]], [["c"]], [], [None] - sb.rules.append((prefix, input_, suffix, lookups)) + sb.rules.append(builder.ChainContextualRule(prefix, input_, suffix, lookups)) prefix, input_, suffix, lookups = [["a"], ["d"]], [["c"]], [], [None] - sb.rules.append((prefix, input_, suffix, lookups)) + sb.rules.append(builder.ChainContextualRule(prefix, input_, suffix, lookups)) sb.add_subtable_break(None) # Second subtable has some glyph classes prefix, input_, suffix, lookups = [["A"]], [["E"]], [], [None] - sb.rules.append((prefix, input_, suffix, lookups)) + sb.rules.append(builder.ChainContextualRule(prefix, input_, suffix, lookups)) prefix, input_, suffix, lookups = [["A"]], [["C","D"]], [], [None] - sb.rules.append((prefix, input_, suffix, lookups)) + sb.rules.append(builder.ChainContextualRule(prefix, input_, suffix, lookups)) prefix, input_, suffix, lookups = [["A", "B"]], [["E"]], [], [None] - sb.rules.append((prefix, input_, suffix, lookups)) + sb.rules.append(builder.ChainContextualRule(prefix, input_, suffix, lookups)) sb.add_subtable_break(None) # Third subtable has no pre/post context prefix, input_, suffix, lookups = [], [["E"]], [], [None] - sb.rules.append((prefix, input_, suffix, lookups)) + sb.rules.append(builder.ChainContextualRule(prefix, input_, suffix, lookups)) prefix, input_, suffix, lookups = [], [["C","D"]], [], [None] - sb.rules.append((prefix, input_, suffix, lookups)) + sb.rules.append(builder.ChainContextualRule(prefix, input_, suffix, lookups)) rulesets = sb.rulesets() assert len(rulesets) == 3 From e3e12fe60d0f0ce07b9b77187972018ef9036605 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Tue, 21 Jul 2020 17:22:47 +0100 Subject: [PATCH 3/3] Fix mock builder test --- Tests/otlLib/mock_builder_test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tests/otlLib/mock_builder_test.py b/Tests/otlLib/mock_builder_test.py index 2b697ac72..b3fecd835 100644 --- a/Tests/otlLib/mock_builder_test.py +++ b/Tests/otlLib/mock_builder_test.py @@ -13,6 +13,7 @@ from fontTools.otlLib.builder import ( ClassPairPosSubtableBuilder, PairPosBuilder, SinglePosBuilder, + ChainContextualRule ) from fontTools.otlLib.error import OpenTypeLibError from fontTools.ttLib import TTFont @@ -79,7 +80,7 @@ def test_chain_pos_references_GSUB_lookup(ttfont): location = MockBuilderLocation((0, "alpha")) builder = ChainContextPosBuilder(ttfont, location) builder2 = SingleSubstBuilder(ttfont, location) - builder.rules.append(([], [], [], [[builder2]])) + builder.rules.append(ChainContextualRule([], [], [], [[builder2]])) with pytest.raises(OpenTypeLibError, match="0:alpha: Missing index of the specified lookup, might be a substitution lookup"): builder.build()