From 6ff7d00e063ca2e36bd734bd2a4972c483f8e1e4 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 25 Jan 2024 13:25:00 +0000 Subject: [PATCH] [feaLib] fix ordering of alternates in aalt lookups Fixes https://github.com/fonttools/fonttools/issues/2937 --- Lib/fontTools/feaLib/builder.py | 30 ++++++++++++++++-------------- Lib/fontTools/otlLib/builder.py | 7 +++++-- Tests/feaLib/data/spec8a.ttx | 6 +++--- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/Lib/fontTools/feaLib/builder.py b/Lib/fontTools/feaLib/builder.py index 02151620b..7921a3f17 100644 --- a/Lib/fontTools/feaLib/builder.py +++ b/Lib/fontTools/feaLib/builder.py @@ -285,7 +285,11 @@ class Builder(object): def build_feature_aalt_(self): if not self.aalt_features_ and not self.aalt_alternates_: return - alternates = {g: set(a) for g, a in self.aalt_alternates_.items()} + # > alternate glyphs will be sorted in the order that the source features + # > are named in the aalt definition, not the order of the feature definitions + # > in the file. Alternates defined explicitly ... will precede all others. + # https://github.com/fonttools/fonttools/issues/836 + alternates = {g: list(a) for g, a in self.aalt_alternates_.items()} for location, name in self.aalt_features_ + [(None, "aalt")]: feature = [ (script, lang, feature, lookups) @@ -302,17 +306,14 @@ class Builder(object): lookuplist = [lookuplist] for lookup in lookuplist: for glyph, alts in lookup.getAlternateGlyphs().items(): - alternates.setdefault(glyph, set()).update(alts) + alts_for_glyph = alternates.setdefault(glyph, []) + alts_for_glyph.extend( + g for g in alts if g not in alts_for_glyph + ) single = { - glyph: list(repl)[0] for glyph, repl in alternates.items() if len(repl) == 1 - } - # TODO: Figure out the glyph alternate ordering used by makeotf. - # https://github.com/fonttools/fonttools/issues/836 - multi = { - glyph: sorted(repl, key=self.font.getGlyphID) - for glyph, repl in alternates.items() - if len(repl) > 1 + glyph: repl[0] for glyph, repl in alternates.items() if len(repl) == 1 } + multi = {glyph: repl for glyph, repl in alternates.items() if len(repl) > 1} if not single and not multi: return self.features_ = { @@ -1249,8 +1250,9 @@ class Builder(object): def add_single_subst(self, location, prefix, suffix, mapping, forceChain): if self.cur_feature_name_ == "aalt": for from_glyph, to_glyph in mapping.items(): - alts = self.aalt_alternates_.setdefault(from_glyph, set()) - alts.add(to_glyph) + alts = self.aalt_alternates_.setdefault(from_glyph, []) + if to_glyph not in alts: + alts.append(to_glyph) return if prefix or suffix or forceChain: self.add_single_subst_chained_(location, prefix, suffix, mapping) @@ -1303,8 +1305,8 @@ class Builder(object): # GSUB 3 def add_alternate_subst(self, location, prefix, glyph, suffix, replacement): if self.cur_feature_name_ == "aalt": - alts = self.aalt_alternates_.setdefault(glyph, set()) - alts.update(replacement) + alts = self.aalt_alternates_.setdefault(glyph, []) + alts.extend(g for g in replacement if g not in alts) return if prefix or suffix: chain = self.get_lookup_(location, ChainContextSubstBuilder) diff --git a/Lib/fontTools/otlLib/builder.py b/Lib/fontTools/otlLib/builder.py index 20cf9adb3..8e5039847 100644 --- a/Lib/fontTools/otlLib/builder.py +++ b/Lib/fontTools/otlLib/builder.py @@ -774,7 +774,10 @@ class ChainContextSubstBuilder(ChainContextualBuilder): if lookup is not None: alts = lookup.getAlternateGlyphs() for glyph, replacements in alts.items(): - result.setdefault(glyph, set()).update(replacements) + alts_for_glyph = result.setdefault(glyph, []) + alts_for_glyph.extend( + g for g in replacements if g not in alts_for_glyph + ) return result def find_chainable_single_subst(self, mapping): @@ -1238,7 +1241,7 @@ class SingleSubstBuilder(LookupBuilder): return self.buildLookup_(subtables) def getAlternateGlyphs(self): - return {glyph: set([repl]) for glyph, repl in self.mapping.items()} + return {glyph: [repl] for glyph, repl in self.mapping.items()} def add_subtable_break(self, location): self.mapping[(self.SUBTABLE_BREAK_, location)] = self.SUBTABLE_BREAK_ diff --git a/Tests/feaLib/data/spec8a.ttx b/Tests/feaLib/data/spec8a.ttx index 787ecfa13..9c8c758ef 100644 --- a/Tests/feaLib/data/spec8a.ttx +++ b/Tests/feaLib/data/spec8a.ttx @@ -99,18 +99,18 @@ - + - + - +