From 40474f1aab189494f30b669794c0802c1f75eca2 Mon Sep 17 00:00:00 2001 From: Sascha Brawer Date: Thu, 16 Feb 2017 13:29:04 +0100 Subject: [PATCH] Distinguish value records format A from format B with zero values Fixes https://github.com/fonttools/fonttools/issues/848. --- Lib/fontTools/feaLib/ast.py | 9 +++-- Lib/fontTools/feaLib/parser.py | 6 ++- Tests/feaLib/data/GPOS_1.fea | 1 - Tests/feaLib/data/spec6h_iii_1.fea | 1 - Tests/feaLib/parser_test.py | 61 ++++++++++++++++++++++++++---- 5 files changed, 62 insertions(+), 16 deletions(-) diff --git a/Lib/fontTools/feaLib/ast.py b/Lib/fontTools/feaLib/ast.py index a0fc09bc3..b6751ed0e 100644 --- a/Lib/fontTools/feaLib/ast.py +++ b/Lib/fontTools/feaLib/ast.py @@ -973,7 +973,8 @@ class SubtableStatement(Statement): class ValueRecord(Expression): - def __init__(self, location, vertical, xPlacement, yPlacement, xAdvance, yAdvance, + def __init__(self, location, vertical, + xPlacement, yPlacement, xAdvance, yAdvance, xPlaDevice, yPlaDevice, xAdvDevice, yAdvDevice): Expression.__init__(self, location) self.xPlacement, self.yPlacement = (xPlacement, yPlacement) @@ -1008,10 +1009,10 @@ class ValueRecord(Expression): vertical = self.vertical # Try format A, if possible. - if x == 0 and y == 0: - if xAdvance == 0 and vertical: + if x is None and y is None: + if xAdvance is None and vertical: return str(yAdvance) - elif yAdvance == 0 and not vertical: + elif yAdvance is None and not vertical: return str(xAdvance) # Try format B, if possible. diff --git a/Lib/fontTools/feaLib/parser.py b/Lib/fontTools/feaLib/parser.py index 8ea1a5ad2..988e5fe7a 100644 --- a/Lib/fontTools/feaLib/parser.py +++ b/Lib/fontTools/feaLib/parser.py @@ -1024,10 +1024,12 @@ class Parser(object): if self.next_token_type_ is Lexer.NUMBER: number, location = self.expect_number_(), self.cur_token_location_ if vertical: - val = self.ast.ValueRecord(location, vertical, 0, 0, 0, number, + val = self.ast.ValueRecord(location, vertical, + None, None, None, number, None, None, None, None) else: - val = self.ast.ValueRecord(location, vertical, 0, 0, number, 0, + val = self.ast.ValueRecord(location, vertical, + None, None, number, None, None, None, None, None) return val self.expect_symbol_("<") diff --git a/Tests/feaLib/data/GPOS_1.fea b/Tests/feaLib/data/GPOS_1.fea index 9b70bd90e..a9bccf2a4 100644 --- a/Tests/feaLib/data/GPOS_1.fea +++ b/Tests/feaLib/data/GPOS_1.fea @@ -26,7 +26,6 @@ feature kern { # single adjustment positionings, provided the re-definition is using # the same value. We replicate this behavior. pos four 400; -#test-fea2fea: pos four 400; pos four <0 0 400 0>; pos nine -100; } kern; diff --git a/Tests/feaLib/data/spec6h_iii_1.fea b/Tests/feaLib/data/spec6h_iii_1.fea index 13c13375b..276fa2f69 100644 --- a/Tests/feaLib/data/spec6h_iii_1.fea +++ b/Tests/feaLib/data/spec6h_iii_1.fea @@ -5,6 +5,5 @@ languagesystem DFLT dflt; feature test { -#test-fea2fea: pos [quoteleft quotedblleft] [Y T]' 20 [quoteright quotedblright]; pos [quoteleft quotedblleft] [Y T]' <0 0 20 0> [quoteright quotedblright]; } test; diff --git a/Tests/feaLib/parser_test.py b/Tests/feaLib/parser_test.py index c43c7ac73..4257fac53 100644 --- a/Tests/feaLib/parser_test.py +++ b/Tests/feaLib/parser_test.py @@ -529,7 +529,7 @@ class ParserTest(unittest.TestCase): [look] = liga.statements [foo] = look.statements self.assertEqual(foo.value.xAdvance, 123) - self.assertEqual(foo.value.yAdvance, 0) + self.assertIsNone(foo.value.yAdvance) def test_lookup_block_with_vertical_valueRecordDef(self): doc = self.parse("feature vkrn {" @@ -540,7 +540,7 @@ class ParserTest(unittest.TestCase): [vkrn] = doc.statements [look] = vkrn.statements [foo] = look.statements - self.assertEqual(foo.value.xAdvance, 0) + self.assertIsNone(foo.value.xAdvance) self.assertEqual(foo.value.yAdvance, 123) def test_lookup_reference(self): @@ -1248,26 +1248,54 @@ class ParserTest(unittest.TestCase): def test_valuerecord_format_a_horizontal(self): doc = self.parse("feature liga {valueRecordDef 123 foo;} liga;") value = doc.statements[0].statements[0].value - self.assertEqual(value.xPlacement, 0) - self.assertEqual(value.yPlacement, 0) + self.assertIsNone(value.xPlacement) + self.assertIsNone(value.yPlacement) self.assertEqual(value.xAdvance, 123) - self.assertEqual(value.yAdvance, 0) + self.assertIsNone(value.yAdvance) self.assertIsNone(value.xPlaDevice) self.assertIsNone(value.yPlaDevice) self.assertIsNone(value.xAdvDevice) self.assertIsNone(value.yAdvDevice) + self.assertEqual(value.makeString(vertical=False), "123") def test_valuerecord_format_a_vertical(self): doc = self.parse("feature vkrn {valueRecordDef 123 foo;} vkrn;") value = doc.statements[0].statements[0].value - self.assertEqual(value.xPlacement, 0) - self.assertEqual(value.yPlacement, 0) - self.assertEqual(value.xAdvance, 0) + self.assertIsNone(value.xPlacement) + self.assertIsNone(value.yPlacement) + self.assertIsNone(value.xAdvance) self.assertEqual(value.yAdvance, 123) self.assertIsNone(value.xPlaDevice) self.assertIsNone(value.yPlaDevice) self.assertIsNone(value.xAdvDevice) self.assertIsNone(value.yAdvDevice) + self.assertEqual(value.makeString(vertical=True), "123") + + def test_valuerecord_format_a_zero_horizontal(self): + doc = self.parse("feature liga {valueRecordDef 0 foo;} liga;") + value = doc.statements[0].statements[0].value + self.assertIsNone(value.xPlacement) + self.assertIsNone(value.yPlacement) + self.assertEqual(value.xAdvance, 0) + self.assertIsNone(value.yAdvance) + self.assertIsNone(value.xPlaDevice) + self.assertIsNone(value.yPlaDevice) + self.assertIsNone(value.xAdvDevice) + self.assertIsNone(value.yAdvDevice) + self.assertEqual(value.makeString(vertical=False), "0") + + def test_valuerecord_format_a_zero_vertical(self): + doc = self.parse("feature vkrn {valueRecordDef 0 foo;} vkrn;") + value = doc.statements[0].statements[0].value + self.assertIsNone(value.xPlacement) + self.assertIsNone(value.yPlacement) + self.assertIsNone(value.xAdvance) + self.assertEqual(value.yAdvance, 0) + self.assertIsNone(value.xPlaDevice) + self.assertIsNone(value.yPlaDevice) + self.assertIsNone(value.xAdvDevice) + self.assertIsNone(value.yAdvDevice) + self.assertEqual(value.makeString(vertical=True), "0") def test_valuerecord_format_a_vertical_contexts_(self): for tag in "vkrn vpal vhal valt".split(): @@ -1289,6 +1317,20 @@ class ParserTest(unittest.TestCase): self.assertIsNone(value.yPlaDevice) self.assertIsNone(value.xAdvDevice) self.assertIsNone(value.yAdvDevice) + self.assertEqual(value.makeString(vertical=False), "<1 2 3 4>") + + def test_valuerecord_format_b_zero(self): + doc = self.parse("feature liga {valueRecordDef <0 0 0 0> foo;} liga;") + value = doc.statements[0].statements[0].value + self.assertEqual(value.xPlacement, 0) + self.assertEqual(value.yPlacement, 0) + self.assertEqual(value.xAdvance, 0) + self.assertEqual(value.yAdvance, 0) + self.assertIsNone(value.xPlaDevice) + self.assertIsNone(value.yPlaDevice) + self.assertIsNone(value.xAdvDevice) + self.assertIsNone(value.yAdvDevice) + self.assertEqual(value.makeString(vertical=False), "<0 0 0 0>") def test_valuerecord_format_c(self): doc = self.parse( @@ -1310,6 +1352,9 @@ class ParserTest(unittest.TestCase): self.assertEqual(value.yPlaDevice, ((11, 111), (12, 112))) self.assertIsNone(value.xAdvDevice) self.assertEqual(value.yAdvDevice, ((33, -113), (44, -114), (55, 115))) + self.assertEqual(value.makeString(vertical=False), + "<1 2 3 4 " + " >") def test_valuerecord_format_d(self): doc = self.parse("feature test {valueRecordDef foo;} test;")