From 7c67e4a63da1b586d7c63d79d2d1d02e17eb242b Mon Sep 17 00:00:00 2001 From: Sascha Brawer Date: Thu, 16 Feb 2017 15:06:02 +0100 Subject: [PATCH] [feaLib] Compile zero values to different formats based on context If a zero value appears in a SinglePos statement, feaLib continues to produce ValueRecords of format 0. But if a zero value appears in a PairPos statement inside a horizontal compilation context, feaLib now produces ValueRecords of format 4. Likewise, if a zero value appears in a PairPos statement inside a vertical compilation context, feaLib now produces ValueRecords of format 8. The OpenType Feature Syntax specification is completely silent about this, but the new behavior matches that of makeotf. https://github.com/fonttools/fonttools/issues/633 --- Lib/fontTools/feaLib/builder.py | 18 +++-- Tests/feaLib/builder_test.py | 1 + .../data/ZeroValue_PairPos_horizontal.fea | 9 +++ .../data/ZeroValue_PairPos_horizontal.ttx | 75 +++++++++++++++++++ .../data/ZeroValue_PairPos_vertical.fea | 9 +++ .../data/ZeroValue_PairPos_vertical.ttx | 75 +++++++++++++++++++ 6 files changed, 180 insertions(+), 7 deletions(-) create mode 100644 Tests/feaLib/data/ZeroValue_PairPos_horizontal.fea create mode 100644 Tests/feaLib/data/ZeroValue_PairPos_horizontal.ttx create mode 100644 Tests/feaLib/data/ZeroValue_PairPos_vertical.fea create mode 100644 Tests/feaLib/data/ZeroValue_PairPos_vertical.ttx diff --git a/Lib/fontTools/feaLib/builder.py b/Lib/fontTools/feaLib/builder.py index 7bed9a180..b29c41b5c 100644 --- a/Lib/fontTools/feaLib/builder.py +++ b/Lib/fontTools/feaLib/builder.py @@ -1012,7 +1012,7 @@ _VALUEREC_ATTRS = { } -def makeOpenTypeValueRecord(v): +def makeOpenTypeValueRecord(v, pairPosContext): """ast.ValueRecord --> (otBase.ValueRecord, int ValueFormat)""" if v is None: return None, 0 @@ -1022,7 +1022,8 @@ def makeOpenTypeValueRecord(v): val = getattr(v, astName, None) if val: vr[otName] = otl.buildDevice(dict(val)) if isDevice else val - + if pairPosContext and not vr: + vr = {"YAdvance": 0} if v.vertical else {"XAdvance": 0} valRec = otl.buildValue(vr) return valRec, valRec.getFormat() @@ -1420,8 +1421,8 @@ class PairPosBuilder(LookupBuilder): 'Already defined position for pair %s %s at %s:%d:%d' % (glyph1, glyph2, otherLoc[0], otherLoc[1], otherLoc[2]), location) - val1, _ = makeOpenTypeValueRecord(value1) - val2, _ = makeOpenTypeValueRecord(value2) + val1, _ = makeOpenTypeValueRecord(value1, pairPosContext=True) + val2, _ = makeOpenTypeValueRecord(value2, pairPosContext=True) self.glyphPairs[key] = (val1, val2) self.locations[key] = location @@ -1442,8 +1443,10 @@ class PairPosBuilder(LookupBuilder): if builder is not None: builder.addSubtableBreak() continue - val1, valFormat1 = makeOpenTypeValueRecord(value1) - val2, valFormat2 = makeOpenTypeValueRecord(value2) + val1, valFormat1 = makeOpenTypeValueRecord( + value1, pairPosContext=True) + val2, valFormat2 = makeOpenTypeValueRecord( + value2, pairPosContext=True) builder = builders.get((valFormat1, valFormat2)) if builder is None: builder = ClassPairPosSubtableBuilder( @@ -1466,7 +1469,8 @@ class SinglePosBuilder(LookupBuilder): self.mapping = {} # glyph -> otTables.ValueRecord def add_pos(self, location, glyph, valueRecord): - otValueRecord, _ = makeOpenTypeValueRecord(valueRecord) + otValueRecord, _ = makeOpenTypeValueRecord( + valueRecord, pairPosContext=False) curValue = self.mapping.get(glyph) if curValue is not None and curValue != otValueRecord: otherLoc = self.locations[glyph] diff --git a/Tests/feaLib/builder_test.py b/Tests/feaLib/builder_test.py index da743db1c..3f1b1d70e 100644 --- a/Tests/feaLib/builder_test.py +++ b/Tests/feaLib/builder_test.py @@ -66,6 +66,7 @@ class BuilderTest(unittest.TestCase): bug512 bug568 name size size2 multiple_feature_blocks omitted_GlyphClassDef ZeroValue_SinglePos + ZeroValue_PairPos_horizontal ZeroValue_PairPos_vertical """.split() def __init__(self, methodName): diff --git a/Tests/feaLib/data/ZeroValue_PairPos_horizontal.fea b/Tests/feaLib/data/ZeroValue_PairPos_horizontal.fea new file mode 100644 index 000000000..b5e48f4d9 --- /dev/null +++ b/Tests/feaLib/data/ZeroValue_PairPos_horizontal.fea @@ -0,0 +1,9 @@ +# For PairPos statements in horizontal compilation contexts, +# zero values should get compiled to ValueRecord format 4. +# https://github.com/fonttools/fonttools/issues/633 +feature kern { + pos A 0 A 0; + pos A 0 B <0 0 0 0>; + pos B <0 0 0 0> A 0; + pos B <0 0 0 0> B <0 0 0 0>; +} kern; diff --git a/Tests/feaLib/data/ZeroValue_PairPos_horizontal.ttx b/Tests/feaLib/data/ZeroValue_PairPos_horizontal.ttx new file mode 100644 index 000000000..6537852b5 --- /dev/null +++ b/Tests/feaLib/data/ZeroValue_PairPos_horizontal.ttx @@ -0,0 +1,75 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Tests/feaLib/data/ZeroValue_PairPos_vertical.fea b/Tests/feaLib/data/ZeroValue_PairPos_vertical.fea new file mode 100644 index 000000000..1ab33a903 --- /dev/null +++ b/Tests/feaLib/data/ZeroValue_PairPos_vertical.fea @@ -0,0 +1,9 @@ +# For PairPos statements in vertical compilation contexts, +# zero values should get compiled to ValueRecord format 8. +# https://github.com/fonttools/fonttools/issues/633 +feature vkrn { + pos A 0 A 0; + pos A 0 B <0 0 0 0>; + pos B <0 0 0 0> A 0; + pos B <0 0 0 0> B <0 0 0 0>; +} vkrn; diff --git a/Tests/feaLib/data/ZeroValue_PairPos_vertical.ttx b/Tests/feaLib/data/ZeroValue_PairPos_vertical.ttx new file mode 100644 index 000000000..774bae017 --- /dev/null +++ b/Tests/feaLib/data/ZeroValue_PairPos_vertical.ttx @@ -0,0 +1,75 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +