[otlLib] Refactor chained contextual builders (#2007)

* Introduce a new subclass for chained contextual (sub and pos)

* Rename .substitutions to .rules in subst builders to allow for code reuse

* Make format of subtable break marker tuple common between sub/pos

Note that prior to this patch, add_subtable_break in a Subst builder adds:

(self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_)

while add_subtable_break in a Pos builder adds:

(self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_, [self.SUBTABLE_BREAK_])

This is messy. If we read the first element from the tuple instead of the last one to test if a rule is a subtable break, we can make the marker tuple the same.

* And now the subtable break code can be hoisted into superclass

* These helper methods will make the build routine common

* Hoist common build method to superclass

The diff doesn’t show it very clearly because it’s being too clever, but all I’ve done is moved one method. Everything works apart from the error message, which comes next.

* Fix the error message
This commit is contained in:
Simon Cozens 2020-07-02 18:40:20 +01:00 committed by GitHub
parent ebfa4ba1fe
commit 6364d36633
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 58 additions and 68 deletions

View File

@ -868,7 +868,7 @@ class Builder(object):
def add_chain_context_subst(self, location, def add_chain_context_subst(self, location,
prefix, glyphs, suffix, lookups): prefix, glyphs, suffix, lookups):
lookup = self.get_lookup_(location, ChainContextSubstBuilder) lookup = self.get_lookup_(location, ChainContextSubstBuilder)
lookup.substitutions.append((prefix, glyphs, suffix, lookup.rules.append((prefix, glyphs, suffix,
self.find_lookup_builders_(lookups))) self.find_lookup_builders_(lookups)))
def add_alternate_subst(self, location, def add_alternate_subst(self, location,
@ -880,7 +880,7 @@ class Builder(object):
if prefix or suffix: if prefix or suffix:
chain = self.get_lookup_(location, ChainContextSubstBuilder) chain = self.get_lookup_(location, ChainContextSubstBuilder)
lookup = self.get_chained_lookup_(location, AlternateSubstBuilder) lookup = self.get_chained_lookup_(location, AlternateSubstBuilder)
chain.substitutions.append((prefix, [{glyph}], suffix, [lookup])) chain.rules.append((prefix, [{glyph}], suffix, [lookup]))
else: else:
lookup = self.get_lookup_(location, AlternateSubstBuilder) lookup = self.get_lookup_(location, AlternateSubstBuilder)
if glyph in lookup.alternates: if glyph in lookup.alternates:
@ -935,7 +935,7 @@ class Builder(object):
if prefix or suffix or forceChain: if prefix or suffix or forceChain:
chain = self.get_lookup_(location, ChainContextSubstBuilder) chain = self.get_lookup_(location, ChainContextSubstBuilder)
lookup = self.get_chained_lookup_(location, LigatureSubstBuilder) lookup = self.get_chained_lookup_(location, LigatureSubstBuilder)
chain.substitutions.append((prefix, glyphs, suffix, [lookup])) chain.rules.append((prefix, glyphs, suffix, [lookup]))
else: else:
lookup = self.get_lookup_(location, LigatureSubstBuilder) lookup = self.get_lookup_(location, LigatureSubstBuilder)
@ -953,7 +953,7 @@ class Builder(object):
chain = self.get_lookup_(location, ChainContextSubstBuilder) chain = self.get_lookup_(location, ChainContextSubstBuilder)
sub = self.get_chained_lookup_(location, MultipleSubstBuilder) sub = self.get_chained_lookup_(location, MultipleSubstBuilder)
sub.mapping[glyph] = replacements sub.mapping[glyph] = replacements
chain.substitutions.append((prefix, [{glyph}], suffix, [sub])) chain.rules.append((prefix, [{glyph}], suffix, [sub]))
return return
lookup = self.get_lookup_(location, MultipleSubstBuilder) lookup = self.get_lookup_(location, MultipleSubstBuilder)
if glyph in lookup.mapping: if glyph in lookup.mapping:
@ -973,7 +973,7 @@ class Builder(object):
def add_reverse_chain_single_subst(self, location, old_prefix, def add_reverse_chain_single_subst(self, location, old_prefix,
old_suffix, mapping): old_suffix, mapping):
lookup = self.get_lookup_(location, ReverseChainSingleSubstBuilder) lookup = self.get_lookup_(location, ReverseChainSingleSubstBuilder)
lookup.substitutions.append((old_prefix, old_suffix, mapping)) lookup.rules.append((old_prefix, old_suffix, mapping))
def add_single_subst(self, location, prefix, suffix, mapping, forceChain): def add_single_subst(self, location, prefix, suffix, mapping, forceChain):
if self.cur_feature_name_ == "aalt": if self.cur_feature_name_ == "aalt":
@ -1007,7 +1007,7 @@ class Builder(object):
if sub is None: if sub is None:
sub = self.get_chained_lookup_(location, SingleSubstBuilder) sub = self.get_chained_lookup_(location, SingleSubstBuilder)
sub.mapping.update(mapping) sub.mapping.update(mapping)
chain.substitutions.append((prefix, [mapping.keys()], suffix, [sub])) chain.rules.append((prefix, [mapping.keys()], suffix, [sub]))
def add_cursive_pos(self, location, glyphclass, entryAnchor, exitAnchor): def add_cursive_pos(self, location, glyphclass, entryAnchor, exitAnchor):
lookup = self.get_lookup_(location, CursivePosBuilder) lookup = self.get_lookup_(location, CursivePosBuilder)

View File

@ -156,11 +156,7 @@ class AlternateSubstBuilder(LookupBuilder):
self.alternates[(self.SUBTABLE_BREAK_, location)] = self.SUBTABLE_BREAK_ self.alternates[(self.SUBTABLE_BREAK_, location)] = self.SUBTABLE_BREAK_
class ChainContextPosBuilder(LookupBuilder): class ChainContextualBuilder(LookupBuilder):
def __init__(self, font, location):
LookupBuilder.__init__(self, font, location, 'GPOS', 8)
self.rules = [] # (prefix, input, suffix, lookups)
def equals(self, other): def equals(self, other):
return (LookupBuilder.equals(self, other) and return (LookupBuilder.equals(self, other) and
self.rules == other.rules) self.rules == other.rules)
@ -170,32 +166,56 @@ class ChainContextPosBuilder(LookupBuilder):
for (prefix, glyphs, suffix, lookups) in self.rules: for (prefix, glyphs, suffix, lookups) in self.rules:
if prefix == self.SUBTABLE_BREAK_: if prefix == self.SUBTABLE_BREAK_:
continue continue
st = ot.ChainContextPos() st = self.newSubtable_()
subtables.append(st) subtables.append(st)
st.Format = 3 st.Format = 3
self.setBacktrackCoverage_(prefix, st) self.setBacktrackCoverage_(prefix, st)
self.setLookAheadCoverage_(suffix, st) self.setLookAheadCoverage_(suffix, st)
self.setInputCoverage_(glyphs, st) self.setInputCoverage_(glyphs, st)
st.PosCount = 0
st.PosLookupRecord = []
for sequenceIndex, lookupList in enumerate(lookups): for sequenceIndex, lookupList in enumerate(lookups):
if lookupList is not None: if lookupList is not None:
if not isinstance(lookupList, list): if not isinstance(lookupList, list):
# Can happen with synthesised lookups # Can happen with synthesised lookups
lookupList = [ lookupList ] lookupList = [ lookupList ]
for l in lookupList: for l in lookupList:
st.PosCount += 1
if l.lookup_index is None: if l.lookup_index is None:
if isinstance(self, ChainContextPosBuilder):
other = "substitution"
else:
other = "positioning"
raise OpenTypeLibError('Missing index of the specified ' raise OpenTypeLibError('Missing index of the specified '
'lookup, might be a substitution lookup', f'lookup, might be a {other} lookup',
self.location) self.location)
rec = ot.PosLookupRecord() rec = self.newLookupRecord_()
rec.SequenceIndex = sequenceIndex rec.SequenceIndex = sequenceIndex
rec.LookupListIndex = l.lookup_index rec.LookupListIndex = l.lookup_index
st.PosLookupRecord.append(rec) self.addLookupRecordToSubtable_(st, rec)
return self.buildLookup_(subtables) return self.buildLookup_(subtables)
def add_subtable_break(self, location):
self.rules.append((self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_,
self.SUBTABLE_BREAK_, [self.SUBTABLE_BREAK_]))
class ChainContextPosBuilder(ChainContextualBuilder):
def __init__(self, font, location):
LookupBuilder.__init__(self, font, location, 'GPOS', 8)
self.rules = [] # (prefix, input, suffix, lookups)
def newSubtable_(self):
st = ot.ChainContextPos()
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)
def find_chainable_single_pos(self, lookups, glyphs, value): def find_chainable_single_pos(self, lookups, glyphs, value):
"""Helper for add_single_pos_chained_()""" """Helper for add_single_pos_chained_()"""
res = None res = None
@ -207,55 +227,29 @@ class ChainContextPosBuilder(LookupBuilder):
res = lookup res = lookup
return res return res
def add_subtable_break(self, location):
self.rules.append((self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_,
self.SUBTABLE_BREAK_, [self.SUBTABLE_BREAK_]))
class ChainContextSubstBuilder(ChainContextualBuilder):
class ChainContextSubstBuilder(LookupBuilder):
def __init__(self, font, location): def __init__(self, font, location):
LookupBuilder.__init__(self, font, location, 'GSUB', 6) LookupBuilder.__init__(self, font, location, 'GSUB', 6)
self.substitutions = [] # (prefix, input, suffix, lookups) self.rules = [] # (prefix, input, suffix, lookups)
def equals(self, other): def newSubtable_(self):
return (LookupBuilder.equals(self, other) and
self.substitutions == other.substitutions)
def build(self):
subtables = []
for (prefix, input, suffix, lookups) in self.substitutions:
if prefix == self.SUBTABLE_BREAK_:
continue
st = ot.ChainContextSubst() st = ot.ChainContextSubst()
subtables.append(st)
st.Format = 3
self.setBacktrackCoverage_(prefix, st)
self.setLookAheadCoverage_(suffix, st)
self.setInputCoverage_(input, st)
st.SubstCount = 0 st.SubstCount = 0
st.SubstLookupRecord = [] st.SubstLookupRecord = []
for sequenceIndex, lookupList in enumerate(lookups): return st
if lookupList is not None:
if not isinstance(lookupList, list): def newLookupRecord_(self):
# Can happen with synthesised lookups return ot.SubstLookupRecord()
lookupList = [ lookupList ]
for l in lookupList: def addLookupRecordToSubtable_(self, st, rec):
st.SubstCount += 1 st.SubstCount += 1
if l.lookup_index is None:
raise OpenTypeLibError('Missing index of the specified '
'lookup, might be a positioning lookup',
self.location)
rec = ot.SubstLookupRecord()
rec.SequenceIndex = sequenceIndex
rec.LookupListIndex = l.lookup_index
st.SubstLookupRecord.append(rec) st.SubstLookupRecord.append(rec)
return self.buildLookup_(subtables)
def getAlternateGlyphs(self): def getAlternateGlyphs(self):
result = {} result = {}
for (_, _, _, lookuplist) in self.substitutions: for (prefix, _, _, lookuplist) in self.rules:
if lookuplist == self.SUBTABLE_BREAK_: if prefix == self.SUBTABLE_BREAK_:
continue continue
for lookups in lookuplist: for lookups in lookuplist:
if not isinstance(lookups, list): if not isinstance(lookups, list):
@ -270,19 +264,15 @@ class ChainContextSubstBuilder(LookupBuilder):
def find_chainable_single_subst(self, glyphs): def find_chainable_single_subst(self, glyphs):
"""Helper for add_single_subst_chained_()""" """Helper for add_single_subst_chained_()"""
res = None res = None
for _, _, _, substitutions in self.substitutions[::-1]: for prefix, _, _, rules in self.rules[::-1]:
if substitutions == self.SUBTABLE_BREAK_: if prefix == self.SUBTABLE_BREAK_:
return res return res
for sub in substitutions: for sub in rules:
if (isinstance(sub, SingleSubstBuilder) and if (isinstance(sub, SingleSubstBuilder) and
not any(g in glyphs for g in sub.mapping.keys())): not any(g in glyphs for g in sub.mapping.keys())):
res = sub res = sub
return res return res
def add_subtable_break(self, location):
self.substitutions.append((self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_,
self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_))
class LigatureSubstBuilder(LookupBuilder): class LigatureSubstBuilder(LookupBuilder):
def __init__(self, font, location): def __init__(self, font, location):
@ -435,15 +425,15 @@ class MarkMarkPosBuilder(LookupBuilder):
class ReverseChainSingleSubstBuilder(LookupBuilder): class ReverseChainSingleSubstBuilder(LookupBuilder):
def __init__(self, font, location): def __init__(self, font, location):
LookupBuilder.__init__(self, font, location, 'GSUB', 8) LookupBuilder.__init__(self, font, location, 'GSUB', 8)
self.substitutions = [] # (prefix, suffix, mapping) self.rules = [] # (prefix, suffix, mapping)
def equals(self, other): def equals(self, other):
return (LookupBuilder.equals(self, other) and return (LookupBuilder.equals(self, other) and
self.substitutions == other.substitutions) self.rules == other.rules)
def build(self): def build(self):
subtables = [] subtables = []
for prefix, suffix, mapping in self.substitutions: for prefix, suffix, mapping in self.rules:
st = ot.ReverseChainSingleSubst() st = ot.ReverseChainSingleSubst()
st.Format = 1 st.Format = 1
self.setBacktrackCoverage_(prefix, st) self.setBacktrackCoverage_(prefix, st)