From 97929b32369514642280be56c75d4e3c563efe09 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 3 Jun 2024 16:31:57 +0100 Subject: [PATCH] [feaLib] try reuse existing inline chained multiple subst lookups when possible We already do this for inline single substitutions in chained contextual lookups, this PR extends this for multiple substitutions as well. Fixes https://github.com/fonttools/fonttools/issues/3551 --- Lib/fontTools/feaLib/builder.py | 20 ++- Lib/fontTools/otlLib/builder.py | 10 +- Tests/feaLib/builder_test.py | 2 + .../contextual_inline_multi_sub_format_2.fea | 17 +++ .../contextual_inline_multi_sub_format_2.ttx | 135 ++++++++++++++++++ 5 files changed, 176 insertions(+), 8 deletions(-) create mode 100644 Tests/feaLib/data/contextual_inline_multi_sub_format_2.fea create mode 100644 Tests/feaLib/data/contextual_inline_multi_sub_format_2.ttx diff --git a/Lib/fontTools/feaLib/builder.py b/Lib/fontTools/feaLib/builder.py index a91381ddc..bda855e1e 100644 --- a/Lib/fontTools/feaLib/builder.py +++ b/Lib/fontTools/feaLib/builder.py @@ -1286,10 +1286,7 @@ class Builder(object): self, location, prefix, glyph, suffix, replacements, forceChain=False ): if prefix or suffix or forceChain: - chain = self.get_lookup_(location, ChainContextSubstBuilder) - sub = self.get_chained_lookup_(location, MultipleSubstBuilder) - sub.mapping[glyph] = replacements - chain.rules.append(ChainContextualRule(prefix, [{glyph}], suffix, [sub])) + self.add_multi_subst_chained_(location, prefix, glyph, suffix, replacements) return lookup = self.get_lookup_(location, MultipleSubstBuilder) if glyph in lookup.mapping: @@ -1369,7 +1366,7 @@ class Builder(object): # https://github.com/fonttools/fonttools/issues/512 # https://github.com/fonttools/fonttools/issues/2150 chain = self.get_lookup_(location, ChainContextSubstBuilder) - sub = chain.find_chainable_single_subst(mapping) + sub = chain.find_chainable_subst(mapping, SingleSubstBuilder) if sub is None: sub = self.get_chained_lookup_(location, SingleSubstBuilder) sub.mapping.update(mapping) @@ -1377,6 +1374,19 @@ class Builder(object): ChainContextualRule(prefix, [list(mapping.keys())], suffix, [sub]) ) + def add_multi_subst_chained_(self, location, prefix, glyph, suffix, replacements): + if not all(prefix) or not all(suffix): + raise FeatureLibError( + "Empty glyph class in contextual substitution", location + ) + # https://github.com/fonttools/fonttools/issues/3551 + chain = self.get_lookup_(location, ChainContextSubstBuilder) + sub = chain.find_chainable_subst({glyph: replacements}, MultipleSubstBuilder) + if sub is None: + sub = self.get_chained_lookup_(location, MultipleSubstBuilder) + sub.mapping[glyph] = replacements + chain.rules.append(ChainContextualRule(prefix, [{glyph}], suffix, [sub])) + # GSUB 8 def add_reverse_chain_single_subst(self, location, old_prefix, old_suffix, mapping): if not mapping: diff --git a/Lib/fontTools/otlLib/builder.py b/Lib/fontTools/otlLib/builder.py index 70fd87ab5..8fc685683 100644 --- a/Lib/fontTools/otlLib/builder.py +++ b/Lib/fontTools/otlLib/builder.py @@ -544,6 +544,10 @@ class ChainContextualBuilder(LookupBuilder): f"{classRuleAttr}Count", getattr(setForThisRule, f"{classRuleAttr}Count") + 1, ) + for i, classSet in enumerate(classSets): + if not getattr(classSet, classRuleAttr): + # class sets can be null so replace nop sets with None + classSets[i] = None setattr(st, self.ruleSetAttr_(format=2, chaining=chaining), classSets) setattr( st, self.ruleSetAttr_(format=2, chaining=chaining) + "Count", len(classSets) @@ -781,14 +785,14 @@ class ChainContextSubstBuilder(ChainContextualBuilder): ) return result - def find_chainable_single_subst(self, mapping): - """Helper for add_single_subst_chained_()""" + def find_chainable_subst(self, mapping, builder_class): + """Helper for add_{single,multi}_subst_chained_()""" res = None for rule in self.rules[::-1]: if rule.is_subtable_break: return res for sub in rule.lookups: - if isinstance(sub, SingleSubstBuilder) and not any( + if isinstance(sub, builder_class) and not any( g in mapping and mapping[g] != sub.mapping[g] for g in sub.mapping ): res = sub diff --git a/Tests/feaLib/builder_test.py b/Tests/feaLib/builder_test.py index 4cb9b4a1d..e2f6a1d4d 100644 --- a/Tests/feaLib/builder_test.py +++ b/Tests/feaLib/builder_test.py @@ -48,6 +48,7 @@ def makeTTFont(): grave acute dieresis macron circumflex cedilla umlaut ogonek caron damma hamza sukun kasratan lam_meem_jeem noon.final noon.initial by feature lookup sub table uni0327 uni0328 e.fina + idotbelow idotless iogonek acutecomb brevecomb ogonekcomb dotbelowcomb """.split() glyphs.extend("cid{:05d}".format(cid) for cid in range(800, 1001 + 1)) font = TTFont() @@ -82,6 +83,7 @@ class BuilderTest(unittest.TestCase): GSUB_5_formats delete_glyph STAT_test STAT_test_elidedFallbackNameID variable_scalar_valuerecord variable_scalar_anchor variable_conditionset variable_mark_anchor duplicate_lookup_reference + contextual_inline_multi_sub_format_2 """.split() VARFONT_AXES = [ diff --git a/Tests/feaLib/data/contextual_inline_multi_sub_format_2.fea b/Tests/feaLib/data/contextual_inline_multi_sub_format_2.fea new file mode 100644 index 000000000..9a9bfe20d --- /dev/null +++ b/Tests/feaLib/data/contextual_inline_multi_sub_format_2.fea @@ -0,0 +1,17 @@ +# reduced from the ccmp feature in Oswald +feature ccmp { + lookup ccmp_Other_1 { + @CombiningTopAccents = [acutecomb brevecomb]; + @CombiningNonTopAccents = [dotbelowcomb ogonekcomb]; + lookupflag UseMarkFilteringSet @CombiningTopAccents; + # we should only generate two lookups; one contextual and one multiple sub, + # containing 'sub idotbelow by idotless dotbelowcomb' and + # 'sub iogonek by idotless ogonekcomb' + sub idotbelow' @CombiningTopAccents by idotless dotbelowcomb; + sub iogonek' @CombiningTopAccents by idotless ogonekcomb; + sub idotbelow' @CombiningNonTopAccents @CombiningTopAccents by idotless dotbelowcomb; + sub iogonek' @CombiningNonTopAccents @CombiningTopAccents by idotless ogonekcomb; + } ccmp_Other_1; +} ccmp; + + diff --git a/Tests/feaLib/data/contextual_inline_multi_sub_format_2.ttx b/Tests/feaLib/data/contextual_inline_multi_sub_format_2.ttx new file mode 100644 index 000000000..d09431fa4 --- /dev/null +++ b/Tests/feaLib/data/contextual_inline_multi_sub_format_2.ttx @@ -0,0 +1,135 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +