From f45fab8c3a4c7b7a110c6b22b02d46794f290e6c Mon Sep 17 00:00:00 2001 From: Sascha Brawer Date: Fri, 4 Dec 2015 11:04:37 +0100 Subject: [PATCH] [feaLib] Sort GSUB glyph coverage tables by glyph ID Before this change, feaLib would sort coverage tables by glyph name, which is against the OpenType specification. The current unittests happen to use only glyphs where the ordering is identical whether sorting by name or by ID; but I am about to add unittests (for GPOS) where the ordering is different. The ordering cannot be enforced by otTables because otTables does not have access to the font's glyph order; therefore, the sorting needs to happen inside feaLib. --- Lib/fontTools/feaLib/builder.py | 47 +++++++++++++------------- Lib/fontTools/feaLib/builder_test.py | 50 +++++++++++++++++++--------- 2 files changed, 58 insertions(+), 39 deletions(-) diff --git a/Lib/fontTools/feaLib/builder.py b/Lib/fontTools/feaLib/builder.py index 73e33560d..862892924 100644 --- a/Lib/fontTools/feaLib/builder.py +++ b/Lib/fontTools/feaLib/builder.py @@ -44,7 +44,8 @@ class Builder(object): raise FeatureLibError( "Within a named lookup block, all rules must be of " "the same lookup type and flag", location) - self.cur_lookup_ = builder_class(location, self.lookup_flag_) + self.cur_lookup_ = builder_class( + self.font, location, self.lookup_flag_) self.lookups_.append(self.cur_lookup_) if self.cur_lookup_name_: # We are starting a lookup rule inside a named lookup block. @@ -305,7 +306,8 @@ class Builder(object): class LookupBuilder(object): - def __init__(self, location, table, lookup_type, lookup_flag): + def __init__(self, font, location, table, lookup_type, lookup_flag): + self.font = font self.location = location self.table, self.lookup_type = table, lookup_type self.lookup_flag = lookup_flag @@ -317,37 +319,34 @@ class LookupBuilder(object): self.table == other.table and self.lookup_flag == other.lookup_flag) - @staticmethod - def setBacktrackCoverage_(prefix, subtable): + def setBacktrackCoverage_(self, prefix, subtable): subtable.BacktrackGlyphCount = len(prefix) subtable.BacktrackCoverage = [] for p in reversed(prefix): coverage = otTables.BacktrackCoverage() - coverage.glyphs = sorted(list(p)) + coverage.glyphs = sorted(list(p), key=self.font.getGlyphID) subtable.BacktrackCoverage.append(coverage) - @staticmethod - def setLookAheadCoverage_(suffix, subtable): + def setLookAheadCoverage_(self, suffix, subtable): subtable.LookAheadGlyphCount = len(suffix) subtable.LookAheadCoverage = [] for s in suffix: coverage = otTables.LookAheadCoverage() - coverage.glyphs = sorted(list(s)) + coverage.glyphs = sorted(list(s), key=self.font.getGlyphID) subtable.LookAheadCoverage.append(coverage) - @staticmethod - def setInputCoverage_(glyphs, subtable): + def setInputCoverage_(self, glyphs, subtable): subtable.InputGlyphCount = len(glyphs) subtable.InputCoverage = [] for g in glyphs: coverage = otTables.InputCoverage() - coverage.glyphs = sorted(list(g)) + coverage.glyphs = sorted(list(g), key=self.font.getGlyphID) subtable.InputCoverage.append(coverage) class AlternateSubstBuilder(LookupBuilder): - def __init__(self, location, lookup_flag): - LookupBuilder.__init__(self, location, 'GSUB', 3, lookup_flag) + def __init__(self, font, location, lookup_flag): + LookupBuilder.__init__(self, font, location, 'GSUB', 3, lookup_flag) self.alternates = {} def equals(self, other): @@ -368,8 +367,8 @@ class AlternateSubstBuilder(LookupBuilder): class ChainContextSubstBuilder(LookupBuilder): - def __init__(self, location, lookup_flag): - LookupBuilder.__init__(self, location, 'GSUB', 6, lookup_flag) + def __init__(self, font, location, lookup_flag): + LookupBuilder.__init__(self, font, location, 'GSUB', 6, lookup_flag) self.substitutions = [] # (prefix, input, suffix, lookups) def equals(self, other): @@ -403,8 +402,8 @@ class ChainContextSubstBuilder(LookupBuilder): class LigatureSubstBuilder(LookupBuilder): - def __init__(self, location, lookup_flag): - LookupBuilder.__init__(self, location, 'GSUB', 4, lookup_flag) + def __init__(self, font, location, lookup_flag): + LookupBuilder.__init__(self, font, location, 'GSUB', 4, lookup_flag) self.ligatures = {} # {('f','f','i'): 'f_f_i'} def equals(self, other): @@ -444,8 +443,8 @@ class LigatureSubstBuilder(LookupBuilder): class MultipleSubstBuilder(LookupBuilder): - def __init__(self, location, lookup_flag): - LookupBuilder.__init__(self, location, 'GSUB', 2, lookup_flag) + def __init__(self, font, location, lookup_flag): + LookupBuilder.__init__(self, font, location, 'GSUB', 2, lookup_flag) self.mapping = {} def equals(self, other): @@ -465,8 +464,8 @@ class MultipleSubstBuilder(LookupBuilder): class ReverseChainSingleSubstBuilder(LookupBuilder): - def __init__(self, location, lookup_flag): - LookupBuilder.__init__(self, location, 'GSUB', 8, lookup_flag) + def __init__(self, font, location, lookup_flag): + LookupBuilder.__init__(self, font, location, 'GSUB', 8, lookup_flag) self.substitutions = [] # (prefix, suffix, mapping) def equals(self, other): @@ -482,7 +481,7 @@ class ReverseChainSingleSubstBuilder(LookupBuilder): lookup.SubTable.append(st) self.setBacktrackCoverage_(prefix, st) self.setLookAheadCoverage_(suffix, st) - coverage = sorted(list(mapping.keys())) + coverage = sorted(mapping.keys(), key=self.font.getGlyphID) st.Coverage = otTables.Coverage() st.Coverage.glyphs = coverage st.GlyphCount = len(coverage) @@ -495,8 +494,8 @@ class ReverseChainSingleSubstBuilder(LookupBuilder): class SingleSubstBuilder(LookupBuilder): - def __init__(self, location, lookup_flag): - LookupBuilder.__init__(self, location, 'GSUB', 1, lookup_flag) + def __init__(self, font, location, lookup_flag): + LookupBuilder.__init__(self, font, location, 'GSUB', 1, lookup_flag) self.mapping = {} def equals(self, other): diff --git a/Lib/fontTools/feaLib/builder_test.py b/Lib/fontTools/feaLib/builder_test.py index adf85e309..61e16bcc7 100644 --- a/Lib/fontTools/feaLib/builder_test.py +++ b/Lib/fontTools/feaLib/builder_test.py @@ -13,6 +13,26 @@ import tempfile import unittest +def makeTTFont(): + glyphs = ( + ".notdef space slash fraction " + "zero one two three four five six seven eight nine " + "zero.oldstyle one.oldstyle two.oldstyle three.oldstyle " + "four.oldstyle five.oldstyle six.oldstyle seven.oldstyle " + "eight.oldstyle nine.oldstyle onehalf " + "A B C D E F G H I J K L M N O P Q R S T U V W X Y Z " + "a b c d e f g h i j k l m n o p q r s t u v w x y z " + "A.sc B.sc C.sc D.sc E.sc F.sc G.sc H.sc I.sc J.sc K.sc L.sc M.sc " + "N.sc O.sc P.sc Q.sc R.sc S.sc T.sc U.sc V.sc W.sc X.sc Y.sc Z.sc " + "A.alt1 A.alt2 A.alt3 B.alt1 B.alt2 B.alt3 C.alt1 C.alt2 C.alt3 " + "d.alt n.end s.end " + "c_h c_k c_s c_t f_f f_f_i f_i f_l o_f_f_i " + ).split() + font = TTFont() + font.setGlyphOrder(glyphs) + return font + + class BuilderTest(unittest.TestCase): def __init__(self, methodName): unittest.TestCase.__init__(self, methodName) @@ -68,12 +88,12 @@ class BuilderTest(unittest.TestCase): path = self.temp_path(suffix=".fea") with codecs.open(path, "wb", "utf-8") as outfile: outfile.write(featureFile) - font = TTFont() + font = makeTTFont() addOpenTypeFeatures(path, font) return font def test_alternateSubst(self): - font = TTFont() + font = makeTTFont() addOpenTypeFeatures(self.getpath("GSUB_3.fea"), font) self.expect_ttx(font, self.getpath("GSUB_3.ttx")) @@ -89,7 +109,7 @@ class BuilderTest(unittest.TestCase): "} test;") def test_alternateSubst(self): - font = TTFont() + font = makeTTFont() addOpenTypeFeatures(self.getpath("GSUB_2.fea"), font) self.expect_ttx(font, self.getpath("GSUB_2.ttx")) @@ -105,7 +125,7 @@ class BuilderTest(unittest.TestCase): "} test;") def test_reverseChainingSingleSubst(self): - font = TTFont() + font = makeTTFont() addOpenTypeFeatures(self.getpath("GSUB_8.fea"), font) self.expect_ttx(font, self.getpath("GSUB_8.ttx")) @@ -121,36 +141,36 @@ class BuilderTest(unittest.TestCase): def test_spec4h1(self): # OpenType Feature File specification, section 4.h, example 1. - font = TTFont() + font = makeTTFont() addOpenTypeFeatures(self.getpath("spec4h1.fea"), font) self.expect_ttx(font, self.getpath("spec4h1.ttx")) def test_spec5d1(self): # OpenType Feature File specification, section 5.d, example 1. - font = TTFont() + font = makeTTFont() addOpenTypeFeatures(self.getpath("spec5d1.fea"), font) self.expect_ttx(font, self.getpath("spec5d1.ttx")) def test_spec5d2(self): # OpenType Feature File specification, section 5.d, example 2. - font = TTFont() + font = makeTTFont() addOpenTypeFeatures(self.getpath("spec5d2.fea"), font) self.expect_ttx(font, self.getpath("spec5d2.ttx")) def test_spec5fi1(self): # OpenType Feature File specification, section 5.f.i, example 1. - font = TTFont() + font = makeTTFont() addOpenTypeFeatures(self.getpath("spec5fi1.fea"), font) self.expect_ttx(font, self.getpath("spec5fi1.ttx")) def test_spec5h1(self): # OpenType Feature File specification, section 5.h, example 1. - font = TTFont() + font = makeTTFont() addOpenTypeFeatures(self.getpath("spec5h1.fea"), font) self.expect_ttx(font, self.getpath("spec5h1.ttx")) def test_languagesystem(self): - builder = Builder(None, TTFont()) + builder = Builder(None, makeTTFont()) builder.add_language_system(None, 'latn', 'FRA') builder.add_language_system(None, 'cyrl', 'RUS') builder.start_feature(location=None, name='test') @@ -164,7 +184,7 @@ class BuilderTest(unittest.TestCase): self.build, "languagesystem cyrl RUS; languagesystem cyrl RUS;") def test_languagesystem_none_specified(self): - builder = Builder(None, TTFont()) + builder = Builder(None, makeTTFont()) builder.start_feature(location=None, name='test') self.assertEqual(builder.language_systems, {('DFLT', 'dflt')}) @@ -176,7 +196,7 @@ class BuilderTest(unittest.TestCase): self.build, "languagesystem latn TRK; languagesystem DFLT dflt;") def test_script(self): - builder = Builder(None, TTFont()) + builder = Builder(None, makeTTFont()) builder.start_feature(location=None, name='test') builder.set_script(location=None, script='cyrl') self.assertEqual(builder.language_systems, @@ -202,7 +222,7 @@ class BuilderTest(unittest.TestCase): self.build, "feature size { script latn; } size;") def test_language(self): - builder = Builder(None, TTFont()) + builder = Builder(None, makeTTFont()) builder.add_language_system(None, 'latn', 'FRA ') builder.start_feature(location=None, name='test') builder.set_script(location=None, script='cyrl') @@ -234,7 +254,7 @@ class BuilderTest(unittest.TestCase): self.build, "feature size { language FRA; } size;") def test_language_required(self): - font = TTFont() + font = makeTTFont() addOpenTypeFeatures(self.getpath("language_required.fea"), font) self.expect_ttx(font, self.getpath("language_required.ttx")) @@ -256,7 +276,7 @@ class BuilderTest(unittest.TestCase): "} test;") def test_lookup(self): - font = TTFont() + font = makeTTFont() addOpenTypeFeatures(self.getpath("lookup.fea"), font) self.expect_ttx(font, self.getpath("lookup.ttx"))