From 9f4cc2f1cb8cdc2d86c102c13a2e9f95f1a44580 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Tue, 7 Jul 2020 12:38:15 +0100 Subject: [PATCH 01/14] Introduce the concept of a ClassContextualRuleSet Currently unused but will be super helpful for optimizing the formats of contextual lookups. * Splits the rules of a class contextual lookup based on explicit subtable breaks * Returns various properties on the ruleset to help determine appropriate layout format. * (More properties, such as "touched glyphs", planned - will be added when needed.) --- Lib/fontTools/otlLib/builder.py | 59 +++++++++++++++++++++++++++++++++ Tests/otlLib/builder_test.py | 57 +++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+) diff --git a/Lib/fontTools/otlLib/builder.py b/Lib/fontTools/otlLib/builder.py index 5731f51c4..968a327a6 100644 --- a/Lib/fontTools/otlLib/builder.py +++ b/Lib/fontTools/otlLib/builder.py @@ -244,11 +244,70 @@ class AlternateSubstBuilder(LookupBuilder): self.alternates[(self.SUBTABLE_BREAK_, location)] = self.SUBTABLE_BREAK_ +class ChainContextualRuleset(): + def __init__(self): + self.rules = [] + + def addRule(self, prefix, glyphs, suffix, lookups): + self.rules.append((prefix, glyphs, suffix, lookups)) + + @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: return True + return False + + @property + 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): + if any([len(x) > 1 for x in coverage]): + return True + return False + + def format1Classdefs(self): + PREFIX, GLYPHS, SUFFIX = 0,1,2 + classdefbuilders = [] + for ix in [PREFIX, GLYPHS, SUFFIX]: + context = [] + for r in self.rules: + context.append(r[ix]) + classes = self._classBuilderForContext(context) + if not classes: + return None + classdefbuilders.append(classes) + return classdefbuilders + + def _classBuilderForContext(self, context): + classdefbuilder = ClassDefBuilder(useClass0=False) + for position in context: + for glyphset in position: + if not classdefbuilder.canAdd(glyphset): + return None + classdefbuilder.add(glyphset) + return classdefbuilder + class ChainContextualBuilder(LookupBuilder): def equals(self, other): return (LookupBuilder.equals(self, other) and self.rules == other.rules) + def rulesets(self): + # 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_: + ruleset.append(ChainContextualRuleset() ) + continue + ruleset[-1].addRule(prefix, glyphs, suffix, lookups) + # Squish any empty subtables + return [x for x in ruleset if len(x.rules) > 0] + def build(self): """Build the lookup. diff --git a/Tests/otlLib/builder_test.py b/Tests/otlLib/builder_test.py index 667e0826f..27acba70d 100644 --- a/Tests/otlLib/builder_test.py +++ b/Tests/otlLib/builder_test.py @@ -1392,6 +1392,63 @@ def test_stat_infinities(): assert struct.pack(">l", posInf) == b"\x7f\xff\xff\xff" +class ChainContextualRulesetTest(object): + def test_makeRulesets(self): + font = ttLib.TTFont() + font.setGlyphOrder(["a","b","c","d","A","B","C","D","E"]) + sb = builder.ChainContextSubstBuilder(font, None) + prefix = [ ["a"], ["b"] ] + input_ = [ ["c"] ] + suffix = [ ] + lookups = [ None ] + sb.rules.append((prefix, input_, suffix, lookups)) + + prefix = [ ["a"], ["d"] ] + sb.rules.append((prefix, input_, suffix, lookups)) + + sb.add_subtable_break(None) + + # Second subtable has some glyph classes + prefix = [ ["A"] ] + input_ = [ ["E"] ] + sb.rules.append((prefix, input_, suffix, lookups)) + input_ = [ ["C","D"] ] + sb.rules.append((prefix, input_, suffix, lookups)) + prefix = [ ["A", "B"] ] + input_ = [ ["E"] ] + sb.rules.append((prefix, input_, suffix, lookups)) + + sb.add_subtable_break(None) + + # Third subtable has no pre/post context + prefix = [] + suffix = [] + sb.rules.append((prefix, input_, suffix, lookups)) + input_ = [ ["C","D"] ] + sb.rules.append((prefix, input_, suffix, lookups)) + + rulesets = sb.rulesets() + assert len(rulesets) == 3 + assert rulesets[0].hasPrefixOrSuffix + assert not rulesets[0].hasAnyGlyphClasses + cd = rulesets[0].format1Classdefs() + assert set(cd[0].classes()[1:]) == set([("d",),("b",),("a",)]) + assert set(cd[1].classes()[1:]) == set([("c",)]) + assert set(cd[2].classes()[1:]) == set() + + assert rulesets[1].hasPrefixOrSuffix + assert rulesets[1].hasAnyGlyphClasses + assert not rulesets[1].format1Classdefs() + + assert not rulesets[2].hasPrefixOrSuffix + assert rulesets[2].hasAnyGlyphClasses + assert rulesets[2].format1Classdefs() + cd = rulesets[2].format1Classdefs() + assert set(cd[0].classes()[1:]) == set() + assert set(cd[1].classes()[1:]) == set([("C","D"), ("E",)]) + assert set(cd[2].classes()[1:]) == set() + + if __name__ == "__main__": import sys From 06c270582ab93e781cb515042c44852d64c40b1a Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Tue, 7 Jul 2020 12:48:53 +0100 Subject: [PATCH 02/14] Begin to use the new rulesets. Be explicit that we are building Format 3 lookups right now. --- Lib/fontTools/otlLib/builder.py | 59 +++++++++++++++++---------------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/Lib/fontTools/otlLib/builder.py b/Lib/fontTools/otlLib/builder.py index 968a327a6..d7a2f166a 100644 --- a/Lib/fontTools/otlLib/builder.py +++ b/Lib/fontTools/otlLib/builder.py @@ -316,36 +316,39 @@ class ChainContextualBuilder(LookupBuilder): contextual positioning lookup. """ subtables = [] - for (prefix, glyphs, suffix, lookups) in self.rules: - if prefix == self.SUBTABLE_BREAK_: - continue - st = self.newSubtable_() - subtables.append(st) - st.Format = 3 - self.setBacktrackCoverage_(prefix, st) - self.setLookAheadCoverage_(suffix, st) - self.setInputCoverage_(glyphs, st) - - for sequenceIndex, lookupList in enumerate(lookups): - if lookupList is not None: - if not isinstance(lookupList, list): - # Can happen with synthesised lookups - lookupList = [ lookupList ] - for l in lookupList: - if l.lookup_index is None: - if isinstance(self, ChainContextPosBuilder): - other = "substitution" - else: - other = "positioning" - raise OpenTypeLibError('Missing index of the specified ' - f'lookup, might be a {other} lookup', - self.location) - rec = self.newLookupRecord_() - rec.SequenceIndex = sequenceIndex - rec.LookupListIndex = l.lookup_index - self.addLookupRecordToSubtable_(st, rec) + for ruleset in self.rulesets(): + for rule in ruleset.rules: + subtables.append(self.buildFormat3Subtable(rule)) return self.buildLookup_(subtables) + def buildFormat3Subtable(self, rule): + (prefix, glyphs, suffix, lookups) = rule + st = self.newSubtable_() + st.Format = 3 + self.setBacktrackCoverage_(prefix, st) + self.setLookAheadCoverage_(suffix, st) + self.setInputCoverage_(glyphs, st) + + for sequenceIndex, lookupList in enumerate(lookups): + if lookupList is not None: + if not isinstance(lookupList, list): + # Can happen with synthesised lookups + lookupList = [ lookupList ] + for l in lookupList: + if l.lookup_index is None: + if isinstance(self, ChainContextPosBuilder): + other = "substitution" + else: + other = "positioning" + raise OpenTypeLibError('Missing index of the specified ' + f'lookup, might be a {other} lookup', + self.location) + rec = self.newLookupRecord_() + rec.SequenceIndex = sequenceIndex + rec.LookupListIndex = l.lookup_index + self.addLookupRecordToSubtable_(st, rec) + return st + def add_subtable_break(self, location): self.rules.append((self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_, [self.SUBTABLE_BREAK_])) From df3e08bb224d1bea31e084a97b3d0048b1f674bf Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Tue, 7 Jul 2020 13:15:27 +0100 Subject: [PATCH 03/14] Add a function to set coverage for GSUB5/GPOS7 lookups --- Lib/fontTools/otlLib/builder.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Lib/fontTools/otlLib/builder.py b/Lib/fontTools/otlLib/builder.py index d7a2f166a..50cd6475c 100644 --- a/Lib/fontTools/otlLib/builder.py +++ b/Lib/fontTools/otlLib/builder.py @@ -172,6 +172,13 @@ class LookupBuilder(object): coverage = buildCoverage(g, self.glyphMap) subtable.InputCoverage.append(coverage) + def setCoverage_(self, glyphs, subtable): + subtable.GlyphCount = len(glyphs) + subtable.Coverage = [] + for g in glyphs: + coverage = buildCoverage(g, self.glyphMap) + subtable.Coverage.append(coverage) + def build_subst_subtables(self, mapping, klass): substitutions = [{}] for key in mapping: From f04ffe41311e5af64215aa111e7309ef536799b3 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Tue, 7 Jul 2020 13:19:36 +0100 Subject: [PATCH 04/14] Make non-chaining contextuals where we can. (Fixes #1856) --- Lib/fontTools/otlLib/builder.py | 36 +++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/Lib/fontTools/otlLib/builder.py b/Lib/fontTools/otlLib/builder.py index 50cd6475c..770372c3f 100644 --- a/Lib/fontTools/otlLib/builder.py +++ b/Lib/fontTools/otlLib/builder.py @@ -323,18 +323,26 @@ class ChainContextualBuilder(LookupBuilder): contextual positioning lookup. """ subtables = [] - for ruleset in self.rulesets(): + chaining = False + rulesets = self.rulesets() + chaining = any([ruleset.hasPrefixOrSuffix for ruleset in rulesets]) + for ruleset in rulesets: for rule in ruleset.rules: - subtables.append(self.buildFormat3Subtable(rule)) + subtables.append(self.buildFormat3Subtable(rule, chaining)) + # If we are not chaining, lookup type will be automatically fixed by + # buildLookup_ return self.buildLookup_(subtables) - def buildFormat3Subtable(self, rule): + def buildFormat3Subtable(self, rule, chaining=True): (prefix, glyphs, suffix, lookups) = rule - st = self.newSubtable_() + st = self.newSubtable_(chaining=chaining) st.Format = 3 - self.setBacktrackCoverage_(prefix, st) - self.setLookAheadCoverage_(suffix, st) - self.setInputCoverage_(glyphs, st) + if chaining: + self.setBacktrackCoverage_(prefix, st) + self.setLookAheadCoverage_(suffix, st) + self.setInputCoverage_(glyphs, st) + else: + self.setCoverage_(glyphs, st) for sequenceIndex, lookupList in enumerate(lookups): if lookupList is not None: @@ -391,8 +399,11 @@ class ChainContextPosBuilder(ChainContextualBuilder): LookupBuilder.__init__(self, font, location, 'GPOS', 8) self.rules = [] # (prefix, input, suffix, lookups) - def newSubtable_(self): - st = ot.ChainContextPos() + def newSubtable_(self, chaining=True): + if chaining: + st = ot.ChainContextPos() + else: + st = ot.ContextPos() st.PosCount = 0 st.PosLookupRecord = [] return st @@ -446,8 +457,11 @@ class ChainContextSubstBuilder(ChainContextualBuilder): LookupBuilder.__init__(self, font, location, 'GSUB', 6) self.rules = [] # (prefix, input, suffix, lookups) - def newSubtable_(self): - st = ot.ChainContextSubst() + def newSubtable_(self, chaining=True): + if chaining: + st = ot.ChainContextSubst() + else: + st = ot.ContextSubst() st.SubstCount = 0 st.SubstLookupRecord = [] return st From 2cf9f681a27f18e1b5f5929ab051be178cce7eac Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Tue, 7 Jul 2020 13:19:50 +0100 Subject: [PATCH 05/14] Fix test expectation. Test is now badly named. --- Tests/feaLib/data/ChainSubstSubtable.ttx | 58 ++++++++++-------------- 1 file changed, 25 insertions(+), 33 deletions(-) diff --git a/Tests/feaLib/data/ChainSubstSubtable.ttx b/Tests/feaLib/data/ChainSubstSubtable.ttx index f7a09c7f7..8718f46f8 100644 --- a/Tests/feaLib/data/ChainSubstSubtable.ttx +++ b/Tests/feaLib/data/ChainSubstSubtable.ttx @@ -30,61 +30,53 @@ - + - - - - - - - + + + + + - - - - - - - - + + + + + + - - - - - - - - + + + + + + - - - - - - - - + + + + + + - + From e8798059f206ffba67765f7079e5ad468e3bd9bc Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Tue, 7 Jul 2020 13:20:57 +0100 Subject: [PATCH 06/14] Rename test --- Tests/feaLib/builder_test.py | 2 +- Tests/feaLib/data/{ChainSubstSubtable.fea => SubstSubtable.fea} | 0 Tests/feaLib/data/{ChainSubstSubtable.ttx => SubstSubtable.ttx} | 0 3 files changed, 1 insertion(+), 1 deletion(-) rename Tests/feaLib/data/{ChainSubstSubtable.fea => SubstSubtable.fea} (100%) rename Tests/feaLib/data/{ChainSubstSubtable.ttx => SubstSubtable.ttx} (100%) diff --git a/Tests/feaLib/builder_test.py b/Tests/feaLib/builder_test.py index 512f60821..89d0b71e3 100644 --- a/Tests/feaLib/builder_test.py +++ b/Tests/feaLib/builder_test.py @@ -69,7 +69,7 @@ class BuilderTest(unittest.TestCase): ZeroValue_SinglePos_horizontal ZeroValue_SinglePos_vertical ZeroValue_PairPos_horizontal ZeroValue_PairPos_vertical ZeroValue_ChainSinglePos_horizontal ZeroValue_ChainSinglePos_vertical - PairPosSubtable ChainSubstSubtable ChainPosSubtable LigatureSubtable + PairPosSubtable SubstSubtable ChainPosSubtable LigatureSubtable AlternateSubtable MultipleSubstSubtable SingleSubstSubtable aalt_chain_contextual_subst AlternateChained MultipleLookupsPerGlyph MultipleLookupsPerGlyph2 diff --git a/Tests/feaLib/data/ChainSubstSubtable.fea b/Tests/feaLib/data/SubstSubtable.fea similarity index 100% rename from Tests/feaLib/data/ChainSubstSubtable.fea rename to Tests/feaLib/data/SubstSubtable.fea diff --git a/Tests/feaLib/data/ChainSubstSubtable.ttx b/Tests/feaLib/data/SubstSubtable.ttx similarity index 100% rename from Tests/feaLib/data/ChainSubstSubtable.ttx rename to Tests/feaLib/data/SubstSubtable.ttx From 7db13c9872884772312727e3478fb36ed9883004 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Tue, 7 Jul 2020 13:24:41 +0100 Subject: [PATCH 07/14] Fixup test expectations --- Tests/feaLib/data/bug506.ttx | 22 +++++------ Tests/feaLib/data/bug509.ttx | 30 +++++++-------- Tests/feaLib/data/bug512.ttx | 58 +++++++++++++---------------- Tests/feaLib/data/spec6h_ii.ttx | 24 ++++++------ Tests/feaLib/data/spec6h_iii_3d.ttx | 22 +++++------ 5 files changed, 69 insertions(+), 87 deletions(-) diff --git a/Tests/feaLib/data/bug506.ttx b/Tests/feaLib/data/bug506.ttx index 9aff91358..4daba45b2 100644 --- a/Tests/feaLib/data/bug506.ttx +++ b/Tests/feaLib/data/bug506.ttx @@ -30,25 +30,23 @@ - + - - - - - - - - - - + + + + + + + + - + diff --git a/Tests/feaLib/data/bug509.ttx b/Tests/feaLib/data/bug509.ttx index e5a36afd3..52fba2013 100644 --- a/Tests/feaLib/data/bug509.ttx +++ b/Tests/feaLib/data/bug509.ttx @@ -30,33 +30,29 @@ - + - - - - - - - + + - - - - - + + + + + + + + - - - + - + diff --git a/Tests/feaLib/data/bug512.ttx b/Tests/feaLib/data/bug512.ttx index 1dfe63fed..50b76b465 100644 --- a/Tests/feaLib/data/bug512.ttx +++ b/Tests/feaLib/data/bug512.ttx @@ -30,61 +30,53 @@ - + - - - - - - - + + + + + - - - - - - - - + + + + + + - - - - - + + + + + - - - + - - - - - + + + + + - - - + - + diff --git a/Tests/feaLib/data/spec6h_ii.ttx b/Tests/feaLib/data/spec6h_ii.ttx index e8ec85f28..2f0efc6f2 100644 --- a/Tests/feaLib/data/spec6h_ii.ttx +++ b/Tests/feaLib/data/spec6h_ii.ttx @@ -112,25 +112,23 @@ - + - - - - + + + + - - + + - - + + - - - + @@ -139,7 +137,7 @@ - + diff --git a/Tests/feaLib/data/spec6h_iii_3d.ttx b/Tests/feaLib/data/spec6h_iii_3d.ttx index a608f0e62..2335dd0b4 100644 --- a/Tests/feaLib/data/spec6h_iii_3d.ttx +++ b/Tests/feaLib/data/spec6h_iii_3d.ttx @@ -30,25 +30,23 @@ - + - - - - - - - - - - + + + + + + + + - + From bc0f69884e8da0a187fe6aac49a7c5984f2b487c Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Tue, 7 Jul 2020 13:28:44 +0100 Subject: [PATCH 08/14] Thinko --- Lib/fontTools/otlLib/builder.py | 2 +- Tests/otlLib/builder_test.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Lib/fontTools/otlLib/builder.py b/Lib/fontTools/otlLib/builder.py index 770372c3f..e84daec13 100644 --- a/Lib/fontTools/otlLib/builder.py +++ b/Lib/fontTools/otlLib/builder.py @@ -276,7 +276,7 @@ class ChainContextualRuleset(): return True return False - def format1Classdefs(self): + def format2Classdefs(self): PREFIX, GLYPHS, SUFFIX = 0,1,2 classdefbuilders = [] for ix in [PREFIX, GLYPHS, SUFFIX]: diff --git a/Tests/otlLib/builder_test.py b/Tests/otlLib/builder_test.py index 27acba70d..5b98a853a 100644 --- a/Tests/otlLib/builder_test.py +++ b/Tests/otlLib/builder_test.py @@ -1431,19 +1431,19 @@ class ChainContextualRulesetTest(object): assert len(rulesets) == 3 assert rulesets[0].hasPrefixOrSuffix assert not rulesets[0].hasAnyGlyphClasses - cd = rulesets[0].format1Classdefs() + cd = rulesets[0].format2Classdefs() assert set(cd[0].classes()[1:]) == set([("d",),("b",),("a",)]) assert set(cd[1].classes()[1:]) == set([("c",)]) assert set(cd[2].classes()[1:]) == set() assert rulesets[1].hasPrefixOrSuffix assert rulesets[1].hasAnyGlyphClasses - assert not rulesets[1].format1Classdefs() + assert not rulesets[1].format2Classdefs() assert not rulesets[2].hasPrefixOrSuffix assert rulesets[2].hasAnyGlyphClasses - assert rulesets[2].format1Classdefs() - cd = rulesets[2].format1Classdefs() + assert rulesets[2].format2Classdefs() + cd = rulesets[2].format2Classdefs() assert set(cd[0].classes()[1:]) == set() assert set(cd[1].classes()[1:]) == set([("C","D"), ("E",)]) assert set(cd[2].classes()[1:]) == set() From 4adea942cc73b6afe58e00278da6cb3795935970 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Tue, 7 Jul 2020 13:33:32 +0100 Subject: [PATCH 09/14] Fix varlib test expectations - now badly named. --- .../InterpolateLayoutGPOS_8_diff.ttx | 28 +++++++++---------- .../InterpolateLayoutGPOS_8_same.ttx | 28 +++++++++---------- 2 files changed, 26 insertions(+), 30 deletions(-) diff --git a/Tests/varLib/data/test_results/InterpolateLayoutGPOS_8_diff.ttx b/Tests/varLib/data/test_results/InterpolateLayoutGPOS_8_diff.ttx index 12f426981..8112d0f67 100644 --- a/Tests/varLib/data/test_results/InterpolateLayoutGPOS_8_diff.ttx +++ b/Tests/varLib/data/test_results/InterpolateLayoutGPOS_8_diff.ttx @@ -83,23 +83,21 @@ - + - - - - - - - - - - - - - + + + + + + + + + + + @@ -108,7 +106,7 @@ - + diff --git a/Tests/varLib/data/test_results/InterpolateLayoutGPOS_8_same.ttx b/Tests/varLib/data/test_results/InterpolateLayoutGPOS_8_same.ttx index b7e86ba2d..f56b0503c 100644 --- a/Tests/varLib/data/test_results/InterpolateLayoutGPOS_8_same.ttx +++ b/Tests/varLib/data/test_results/InterpolateLayoutGPOS_8_same.ttx @@ -83,23 +83,21 @@ - + - - - - - - - - - - - - - + + + + + + + + + + + @@ -108,7 +106,7 @@ - + From 6d4c5fe31c9199e6d3e46cd0808e7640d1610e75 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Tue, 7 Jul 2020 13:35:33 +0100 Subject: [PATCH 10/14] Rename GPOS8->GPOS7 --- ...S_8_diff.ttx => InterpolateLayoutGPOS_7_diff.ttx} | 0 ...S_8_same.ttx => InterpolateLayoutGPOS_7_same.ttx} | 0 Tests/varLib/interpolate_layout_test.py | 12 ++++++------ 3 files changed, 6 insertions(+), 6 deletions(-) rename Tests/varLib/data/test_results/{InterpolateLayoutGPOS_8_diff.ttx => InterpolateLayoutGPOS_7_diff.ttx} (100%) rename Tests/varLib/data/test_results/{InterpolateLayoutGPOS_8_same.ttx => InterpolateLayoutGPOS_7_same.ttx} (100%) diff --git a/Tests/varLib/data/test_results/InterpolateLayoutGPOS_8_diff.ttx b/Tests/varLib/data/test_results/InterpolateLayoutGPOS_7_diff.ttx similarity index 100% rename from Tests/varLib/data/test_results/InterpolateLayoutGPOS_8_diff.ttx rename to Tests/varLib/data/test_results/InterpolateLayoutGPOS_7_diff.ttx diff --git a/Tests/varLib/data/test_results/InterpolateLayoutGPOS_8_same.ttx b/Tests/varLib/data/test_results/InterpolateLayoutGPOS_7_same.ttx similarity index 100% rename from Tests/varLib/data/test_results/InterpolateLayoutGPOS_8_same.ttx rename to Tests/varLib/data/test_results/InterpolateLayoutGPOS_7_same.ttx diff --git a/Tests/varLib/interpolate_layout_test.py b/Tests/varLib/interpolate_layout_test.py index 572dd3ec6..8fdf60f52 100644 --- a/Tests/varLib/interpolate_layout_test.py +++ b/Tests/varLib/interpolate_layout_test.py @@ -748,8 +748,8 @@ class InterpolateLayoutTest(unittest.TestCase): self.check_ttx_dump(instfont, expected_ttx_path, tables, suffix) - def test_varlib_interpolate_layout_GPOS_only_LookupType_8_same_val_ttf(self): - """Only GPOS; LookupType 8; same values in all masters. + def test_varlib_interpolate_layout_GPOS_only_LookupType_7_same_val_ttf(self): + """Only GPOS; LookupType 7; same values in all masters. """ suffix = '.ttf' ds_path = self.get_test_input('InterpolateLayout.designspace') @@ -781,13 +781,13 @@ class InterpolateLayoutTest(unittest.TestCase): instfont = interpolate_layout(ds_path, {'weight': 500}, finder) tables = ['GPOS'] - expected_ttx_path = self.get_test_output('InterpolateLayoutGPOS_8_same.ttx') + expected_ttx_path = self.get_test_output('InterpolateLayoutGPOS_7_same.ttx') self.expect_ttx(instfont, expected_ttx_path, tables) self.check_ttx_dump(instfont, expected_ttx_path, tables, suffix) - def test_varlib_interpolate_layout_GPOS_only_LookupType_8_diff_val_ttf(self): - """Only GPOS; LookupType 8; different values in each master. + def test_varlib_interpolate_layout_GPOS_only_LookupType_7_diff_val_ttf(self): + """Only GPOS; LookupType 7; different values in each master. """ suffix = '.ttf' ds_path = self.get_test_input('InterpolateLayout.designspace') @@ -833,7 +833,7 @@ class InterpolateLayoutTest(unittest.TestCase): instfont = interpolate_layout(ds_path, {'weight': 500}, finder) tables = ['GPOS'] - expected_ttx_path = self.get_test_output('InterpolateLayoutGPOS_8_diff.ttx') + expected_ttx_path = self.get_test_output('InterpolateLayoutGPOS_7_diff.ttx') self.expect_ttx(instfont, expected_ttx_path, tables) self.check_ttx_dump(instfont, expected_ttx_path, tables, suffix) From 8c3dfc4e33d97b83e55a6e88cf89d4f5f4927c30 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Tue, 7 Jul 2020 17:12:09 +0100 Subject: [PATCH 11/14] Refactor subtable/lookup record building code It's a bit gross, but I'm blaming OpenType for that. I'm doing this now because otherwise it would be even more repetitive once we start adding format 1 Rule and Ruleset subtables. --- Lib/fontTools/otlLib/builder.py | 78 +++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 34 deletions(-) diff --git a/Lib/fontTools/otlLib/builder.py b/Lib/fontTools/otlLib/builder.py index e84daec13..97d983013 100644 --- a/Lib/fontTools/otlLib/builder.py +++ b/Lib/fontTools/otlLib/builder.py @@ -358,16 +358,56 @@ class ChainContextualBuilder(LookupBuilder): raise OpenTypeLibError('Missing index of the specified ' f'lookup, might be a {other} lookup', self.location) - rec = self.newLookupRecord_() + rec = self.newLookupRecord_(st) rec.SequenceIndex = sequenceIndex rec.LookupListIndex = l.lookup_index - self.addLookupRecordToSubtable_(st, rec) return st def add_subtable_break(self, location): self.rules.append((self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_, [self.SUBTABLE_BREAK_])) + def newSubtable_(self, chaining=True): + subtablename = f"Context{self.subtable_type}" + if chaining: + subtablename = "Chain"+subtablename + st = getattr(ot,subtablename)() # ot.ChainContextPos()/ot.ChainSubst()/etc. + setattr(st, f"{self.subtable_type}Count", 0) + setattr(st, f"{self.subtable_type}LookupRecord", []) + return st + + def attachSubtableWithCount_(self, st, + subtable_name, count_name, + existing=None, + index=None, chaining=False): + if chaining: + subtable_name = "Chain"+subtable_name + count_name = "Chain"+count_name + + if not hasattr(st, count_name): + setattr(st, count_name, 0) + setattr(st, subtable_name, []) + + if existing: + new_subtable = existing + else: + # Create a new, empty subtable from otTables + new_subtable = getattr(ot, subtable_name)() + + setattr(st, count_name, getattr(st, count_name) + 1) + + if index: + getattr(st, subtable_name).insert(index, new_subtable) + else: + getattr(st, subtable_name).append(new_subtable) + + return new_subtable + + def newLookupRecord_(self, st): + return self.attachSubtableWithCount_(st, + f"{self.subtable_type}LookupRecord", + f"{self.subtable_type}Count", + chaining=False) # Oddly, it isn't ChainSubstLookupRecord class ChainContextPosBuilder(ChainContextualBuilder): """Builds a Chained Contextual Positioning (GPOS8) lookup. @@ -398,22 +438,7 @@ class ChainContextPosBuilder(ChainContextualBuilder): def __init__(self, font, location): LookupBuilder.__init__(self, font, location, 'GPOS', 8) self.rules = [] # (prefix, input, suffix, lookups) - - def newSubtable_(self, chaining=True): - if chaining: - st = ot.ChainContextPos() - else: - st = ot.ContextPos() - st.PosCount = 0 - st.PosLookupRecord = [] - return st - - def newLookupRecord_(self): - return ot.PosLookupRecord() - - def addLookupRecordToSubtable_(self, st, rec): - st.PosCount += 1 - st.PosLookupRecord.append(rec) + self.subtable_type = "Pos" def find_chainable_single_pos(self, lookups, glyphs, value): """Helper for add_single_pos_chained_()""" @@ -456,22 +481,7 @@ class ChainContextSubstBuilder(ChainContextualBuilder): def __init__(self, font, location): LookupBuilder.__init__(self, font, location, 'GSUB', 6) self.rules = [] # (prefix, input, suffix, lookups) - - def newSubtable_(self, chaining=True): - if chaining: - st = ot.ChainContextSubst() - else: - st = ot.ContextSubst() - st.SubstCount = 0 - st.SubstLookupRecord = [] - return st - - def newLookupRecord_(self): - return ot.SubstLookupRecord() - - def addLookupRecordToSubtable_(self, st, rec): - st.SubstCount += 1 - st.SubstLookupRecord.append(rec) + self.subtable_type = "Subst" def getAlternateGlyphs(self): result = {} From b6f7b2dc4b1b80691ae7db2d2850e71d4c9cfc4b Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Sat, 11 Jul 2020 19:52:05 +0100 Subject: [PATCH 12/14] Stylistic fixes from feedback --- Lib/fontTools/otlLib/builder.py | 19 ++++++++++--------- Tests/otlLib/builder_test.py | 28 +++++++++++----------------- 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/Lib/fontTools/otlLib/builder.py b/Lib/fontTools/otlLib/builder.py index 97d983013..5b142d8fd 100644 --- a/Lib/fontTools/otlLib/builder.py +++ b/Lib/fontTools/otlLib/builder.py @@ -263,7 +263,8 @@ class ChainContextualRuleset(): # 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: return True + if len(prefix) > 0 or len(suffix) > 0: + return True return False @property @@ -272,13 +273,13 @@ class ChainContextualRuleset(): # we can express this subtable as a Format 1. for (prefix, glyphs, suffix, lookups) in self.rules: for coverage in (prefix, glyphs, suffix): - if any([len(x) > 1 for x in coverage]): + if any(len(x) > 1 for x in coverage): return True return False - def format2Classdefs(self): + def format2ClassDefs(self): PREFIX, GLYPHS, SUFFIX = 0,1,2 - classdefbuilders = [] + classDefBuilders = [] for ix in [PREFIX, GLYPHS, SUFFIX]: context = [] for r in self.rules: @@ -286,8 +287,8 @@ class ChainContextualRuleset(): classes = self._classBuilderForContext(context) if not classes: return None - classdefbuilders.append(classes) - return classdefbuilders + classDefBuilders.append(classes) + return classDefBuilders def _classBuilderForContext(self, context): classdefbuilder = ClassDefBuilder(useClass0=False) @@ -306,10 +307,10 @@ class ChainContextualBuilder(LookupBuilder): def rulesets(self): # Return a list of ChainContextRuleset objects, taking explicit # subtable breaks into account - ruleset = [ ChainContextualRuleset() ] + ruleset = [ChainContextualRuleset()] for (prefix, glyphs, suffix, lookups) in self.rules: if prefix == self.SUBTABLE_BREAK_: - ruleset.append(ChainContextualRuleset() ) + ruleset.append(ChainContextualRuleset()) continue ruleset[-1].addRule(prefix, glyphs, suffix, lookups) # Squish any empty subtables @@ -325,7 +326,7 @@ class ChainContextualBuilder(LookupBuilder): subtables = [] chaining = False rulesets = self.rulesets() - chaining = any([ruleset.hasPrefixOrSuffix for ruleset in rulesets]) + chaining = any(ruleset.hasPrefixOrSuffix for ruleset in rulesets) for ruleset in rulesets: for rule in ruleset.rules: subtables.append(self.buildFormat3Subtable(rule, chaining)) diff --git a/Tests/otlLib/builder_test.py b/Tests/otlLib/builder_test.py index 5b98a853a..54de6fccb 100644 --- a/Tests/otlLib/builder_test.py +++ b/Tests/otlLib/builder_test.py @@ -1397,53 +1397,47 @@ class ChainContextualRulesetTest(object): font = ttLib.TTFont() font.setGlyphOrder(["a","b","c","d","A","B","C","D","E"]) sb = builder.ChainContextSubstBuilder(font, None) - prefix = [ ["a"], ["b"] ] - input_ = [ ["c"] ] - suffix = [ ] - lookups = [ None ] + prefix, input_, suffix, lookups = [["a"], ["b"]], [["c"]], [], [None] sb.rules.append((prefix, input_, suffix, lookups)) - prefix = [ ["a"], ["d"] ] + prefix, input_, suffix, lookups = [["a"], ["d"]], [["c"]], [], [None] sb.rules.append((prefix, input_, suffix, lookups)) sb.add_subtable_break(None) # Second subtable has some glyph classes - prefix = [ ["A"] ] - input_ = [ ["E"] ] + prefix, input_, suffix, lookups = [["A"]], [["E"]], [], [None] sb.rules.append((prefix, input_, suffix, lookups)) - input_ = [ ["C","D"] ] + prefix, input_, suffix, lookups = [["A"]], [["C","D"]], [], [None] sb.rules.append((prefix, input_, suffix, lookups)) - prefix = [ ["A", "B"] ] - input_ = [ ["E"] ] + prefix, input_, suffix, lookups = [["A", "B"]], [["E"]], [], [None] sb.rules.append((prefix, input_, suffix, lookups)) sb.add_subtable_break(None) # Third subtable has no pre/post context - prefix = [] - suffix = [] + prefix, input_, suffix, lookups = [], [["E"]], [], [None] sb.rules.append((prefix, input_, suffix, lookups)) - input_ = [ ["C","D"] ] + prefix, input_, suffix, lookups = [], [["C","D"]], [], [None] sb.rules.append((prefix, input_, suffix, lookups)) rulesets = sb.rulesets() assert len(rulesets) == 3 assert rulesets[0].hasPrefixOrSuffix assert not rulesets[0].hasAnyGlyphClasses - cd = rulesets[0].format2Classdefs() + cd = rulesets[0].format2ClassDefs() assert set(cd[0].classes()[1:]) == set([("d",),("b",),("a",)]) assert set(cd[1].classes()[1:]) == set([("c",)]) assert set(cd[2].classes()[1:]) == set() assert rulesets[1].hasPrefixOrSuffix assert rulesets[1].hasAnyGlyphClasses - assert not rulesets[1].format2Classdefs() + assert not rulesets[1].format2ClassDefs() assert not rulesets[2].hasPrefixOrSuffix assert rulesets[2].hasAnyGlyphClasses - assert rulesets[2].format2Classdefs() - cd = rulesets[2].format2Classdefs() + assert rulesets[2].format2ClassDefs() + cd = rulesets[2].format2ClassDefs() assert set(cd[0].classes()[1:]) == set() assert set(cd[1].classes()[1:]) == set([("C","D"), ("E",)]) assert set(cd[2].classes()[1:]) == set() From 0b397458907d7cd36303d7eb1e470d181e41a766 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Sat, 11 Jul 2020 19:56:29 +0100 Subject: [PATCH 13/14] Restore test for chaining version --- Tests/feaLib/builder_test.py | 8 +- Tests/feaLib/data/ChainSubstSubtable.fea | 9 ++ Tests/feaLib/data/ChainSubstSubtable.ttx | 136 +++++++++++++++++++++++ 3 files changed, 149 insertions(+), 4 deletions(-) create mode 100644 Tests/feaLib/data/ChainSubstSubtable.fea create mode 100644 Tests/feaLib/data/ChainSubstSubtable.ttx diff --git a/Tests/feaLib/builder_test.py b/Tests/feaLib/builder_test.py index 89d0b71e3..5a8c562d6 100644 --- a/Tests/feaLib/builder_test.py +++ b/Tests/feaLib/builder_test.py @@ -69,10 +69,10 @@ class BuilderTest(unittest.TestCase): ZeroValue_SinglePos_horizontal ZeroValue_SinglePos_vertical ZeroValue_PairPos_horizontal ZeroValue_PairPos_vertical ZeroValue_ChainSinglePos_horizontal ZeroValue_ChainSinglePos_vertical - PairPosSubtable SubstSubtable ChainPosSubtable LigatureSubtable - AlternateSubtable MultipleSubstSubtable SingleSubstSubtable - aalt_chain_contextual_subst AlternateChained MultipleLookupsPerGlyph - MultipleLookupsPerGlyph2 + PairPosSubtable ChainSubstSubtable SubstSubtable ChainPosSubtable + LigatureSubtable AlternateSubtable MultipleSubstSubtable + SingleSubstSubtable aalt_chain_contextual_subst AlternateChained + MultipleLookupsPerGlyph MultipleLookupsPerGlyph2 """.split() def __init__(self, methodName): diff --git a/Tests/feaLib/data/ChainSubstSubtable.fea b/Tests/feaLib/data/ChainSubstSubtable.fea new file mode 100644 index 000000000..ec248805c --- /dev/null +++ b/Tests/feaLib/data/ChainSubstSubtable.fea @@ -0,0 +1,9 @@ +feature test { + sub A G' by G.swash; + subtable; + sub A H' by H.swash; + subtable; + sub A G' by g; + subtable; + sub A H' by H.swash; +} test; diff --git a/Tests/feaLib/data/ChainSubstSubtable.ttx b/Tests/feaLib/data/ChainSubstSubtable.ttx new file mode 100644 index 000000000..75348dc20 --- /dev/null +++ b/Tests/feaLib/data/ChainSubstSubtable.ttx @@ -0,0 +1,136 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From c1a885ad554667de88e07016c8db64acd2288bfd Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Sat, 11 Jul 2020 19:58:06 +0100 Subject: [PATCH 14/14] Missed stylistic fix --- Lib/fontTools/otlLib/builder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/fontTools/otlLib/builder.py b/Lib/fontTools/otlLib/builder.py index 5b142d8fd..9a143eac6 100644 --- a/Lib/fontTools/otlLib/builder.py +++ b/Lib/fontTools/otlLib/builder.py @@ -349,7 +349,7 @@ class ChainContextualBuilder(LookupBuilder): if lookupList is not None: if not isinstance(lookupList, list): # Can happen with synthesised lookups - lookupList = [ lookupList ] + lookupList = [lookupList] for l in lookupList: if l.lookup_index is None: if isinstance(self, ChainContextPosBuilder):