From 091b05296d45984189514ba3246fd35edee5fcaf Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Sat, 19 Jan 2019 11:46:05 +0000 Subject: [PATCH 1/3] [feaLib] distinguish missing value and explicit '' for PairPos2 format A Fixes #1459 --- Lib/fontTools/feaLib/ast.py | 20 ++++++++++++++++++++ Lib/fontTools/feaLib/parser.py | 2 +- Tests/feaLib/parser_test.py | 23 ++++++++++++++++++++--- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/Lib/fontTools/feaLib/ast.py b/Lib/fontTools/feaLib/ast.py index 7f33ca41c..06a018ef7 100644 --- a/Lib/fontTools/feaLib/ast.py +++ b/Lib/fontTools/feaLib/ast.py @@ -1115,6 +1115,9 @@ class ValueRecord(Expression): hash(self.xAdvDevice) ^ hash(self.yAdvDevice)) def makeString(self, vertical=None): + if not self: + return "" + x, y = self.xPlacement, self.yPlacement xAdvance, yAdvance = self.xAdvance, self.yAdvance xPlaDevice, yPlaDevice = self.xPlaDevice, self.yPlaDevice @@ -1140,6 +1143,23 @@ class ValueRecord(Expression): deviceToString(xPlaDevice), deviceToString(yPlaDevice), deviceToString(xAdvDevice), deviceToString(yAdvDevice)) + def __bool__(self): + return any( + getattr(self, v) is not None + for v in [ + "xPlacement", + "yPlacement", + "xAdvance", + "yAdvance", + "xPlaDevice", + "yPlaDevice", + "xAdvDevice", + "yAdvDevice", + ] + ) + + __nonzero__ = __bool__ + class ValueRecordDefinition(Statement): def __init__(self, name, value, location=None): diff --git a/Lib/fontTools/feaLib/parser.py b/Lib/fontTools/feaLib/parser.py index 304269875..61d371111 100644 --- a/Lib/fontTools/feaLib/parser.py +++ b/Lib/fontTools/feaLib/parser.py @@ -1153,7 +1153,7 @@ class Parser(object): name = self.expect_name_() if name == "NULL": self.expect_symbol_(">") - return None + return self.ast.ValueRecord() vrd = self.valuerecords_.resolve(name) if vrd is None: raise FeatureLibError("Unknown valueRecordDef \"%s\"" % name, diff --git a/Tests/feaLib/parser_test.py b/Tests/feaLib/parser_test.py index f9b1063b9..900792196 100644 --- a/Tests/feaLib/parser_test.py +++ b/Tests/feaLib/parser_test.py @@ -800,7 +800,22 @@ class ParserTest(unittest.TestCase): self.assertEqual(pos.valuerecord2.makeString(vertical=False), "<1 2 3 4>") - def test_gpos_type_2_format_a_with_null(self): + def test_gpos_type_2_format_a_with_null_first(self): + doc = self.parse("feature kern {" + " pos [T V] [a b c] <1 2 3 4>;" + "} kern;") + pos = doc.statements[0].statements[0] + self.assertEqual(type(pos), ast.PairPosStatement) + self.assertFalse(pos.enumerated) + self.assertEqual(glyphstr([pos.glyphs1]), "[T V]") + self.assertFalse(pos.valuerecord1) + self.assertEqual(pos.valuerecord1.makeString(), "") + self.assertEqual(glyphstr([pos.glyphs2]), "[a b c]") + self.assertEqual(pos.valuerecord2.makeString(vertical=False), + "<1 2 3 4>") + self.assertEqual(pos.asFea(), "pos [T V] [a b c] <1 2 3 4>;") + + def test_gpos_type_2_format_a_with_null_second(self): doc = self.parse("feature kern {" " pos [T V] <1 2 3 4> [a b c] ;" "} kern;") @@ -811,7 +826,8 @@ class ParserTest(unittest.TestCase): self.assertEqual(pos.valuerecord1.makeString(vertical=False), "<1 2 3 4>") self.assertEqual(glyphstr([pos.glyphs2]), "[a b c]") - self.assertIsNone(pos.valuerecord2) + self.assertFalse(pos.valuerecord2) + self.assertEqual(pos.asFea(), "pos [T V] [a b c] <1 2 3 4>;") def test_gpos_type_2_format_b(self): doc = self.parse("feature kern {" @@ -1523,7 +1539,8 @@ class ParserTest(unittest.TestCase): def test_valuerecord_format_d(self): doc = self.parse("feature test {valueRecordDef foo;} test;") value = doc.statements[0].statements[0].value - self.assertIsNone(value) + self.assertFalse(value) + self.assertEqual(value.makeString(), "") def test_valuerecord_named(self): doc = self.parse("valueRecordDef <1 2 3 4> foo;" From 23f0b5f5b1b8aa72639dc42aaef11d626d555690 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Sat, 19 Jan 2019 12:11:14 +0000 Subject: [PATCH 2/3] [feaLib.builder] do not make ValueRecord --- Lib/fontTools/feaLib/builder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/fontTools/feaLib/builder.py b/Lib/fontTools/feaLib/builder.py index 957b01d64..e521a0edc 100644 --- a/Lib/fontTools/feaLib/builder.py +++ b/Lib/fontTools/feaLib/builder.py @@ -1114,7 +1114,7 @@ _VALUEREC_ATTRS = { def makeOpenTypeValueRecord(v, pairPosContext): """ast.ValueRecord --> (otBase.ValueRecord, int ValueFormat)""" - if v is None: + if not v: return None, 0 vr = {} From baf11f64f404c911e4acb3e40657744a4bce8c72 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Sat, 19 Jan 2019 12:17:38 +0000 Subject: [PATCH 3/3] feaLib/builder_test: add test for #1459 --- Tests/feaLib/builder_test.py | 2 +- Tests/feaLib/data/bug1459.fea | 7 +++++ Tests/feaLib/data/bug1459.ttx | 55 +++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 Tests/feaLib/data/bug1459.fea create mode 100644 Tests/feaLib/data/bug1459.ttx diff --git a/Tests/feaLib/builder_test.py b/Tests/feaLib/builder_test.py index 18ece3b1e..fc1141a88 100644 --- a/Tests/feaLib/builder_test.py +++ b/Tests/feaLib/builder_test.py @@ -65,7 +65,7 @@ class BuilderTest(unittest.TestCase): spec9a spec9b spec9c1 spec9c2 spec9c3 spec9d spec9e spec9f spec9g spec10 bug453 bug457 bug463 bug501 bug502 bug504 bug505 bug506 bug509 - bug512 bug514 bug568 bug633 bug1307 + bug512 bug514 bug568 bug633 bug1307 bug1459 name size size2 multiple_feature_blocks omitted_GlyphClassDef ZeroValue_SinglePos_horizontal ZeroValue_SinglePos_vertical ZeroValue_PairPos_horizontal ZeroValue_PairPos_vertical diff --git a/Tests/feaLib/data/bug1459.fea b/Tests/feaLib/data/bug1459.fea new file mode 100644 index 000000000..1ad688e8a --- /dev/null +++ b/Tests/feaLib/data/bug1459.fea @@ -0,0 +1,7 @@ +# A pair position lookup where only the second glyph has a non-empty valuerecord +# while the first glyph has a NULL valuerecord. The ValueFormat1 for the first +# glyph is expected to be 0. +# https://github.com/fonttools/fonttools/issues/1459 +feature kern { + pos A V <-180 0 -90 0>; +} kern; diff --git a/Tests/feaLib/data/bug1459.ttx b/Tests/feaLib/data/bug1459.ttx new file mode 100644 index 000000000..8a7c09627 --- /dev/null +++ b/Tests/feaLib/data/bug1459.ttx @@ -0,0 +1,55 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +