From ef2742593c117073525403994f3c41821986860e Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Tue, 4 Feb 2020 11:08:45 +0000 Subject: [PATCH 01/44] Compare realpath against realpath On Windows, tests may be run in the user's temp directory, so the previous code may compare C:\Users\nikolaus.waxweiler\ against C:\Users\NIKOLA~1.WAX\. --- Tests/feaLib/lexer_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/feaLib/lexer_test.py b/Tests/feaLib/lexer_test.py index f8f8e75c9..fa3f0ea57 100644 --- a/Tests/feaLib/lexer_test.py +++ b/Tests/feaLib/lexer_test.py @@ -223,7 +223,7 @@ class IncludingLexerTest(unittest.TestCase): # an in-memory stream, so it will use the current working # directory to resolve relative include statements lexer = IncludingLexer(UnicodeIO("include(included.fea);")) - files = set(loc[0] for _, _, loc in lexer) + files = set(os.path.realpath(loc[0]) for _, _, loc in lexer) expected = os.path.realpath(included.name) self.assertIn(expected, files) finally: From eb77a3be220953738c8b4d1033002af226ebffab Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Tue, 11 Feb 2020 15:45:49 +0000 Subject: [PATCH 02/44] [varLib] Fill in the forward-mapped location --- Lib/fontTools/varLib/__init__.py | 3 ++- .../data/VarLibLocationTest.designspace | 26 +++++++++++++++++++ Tests/varLib/varLib_test.py | 9 ++++++- 3 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 Tests/varLib/data/VarLibLocationTest.designspace diff --git a/Lib/fontTools/varLib/__init__.py b/Lib/fontTools/varLib/__init__.py index 818c484c4..9bae80d12 100644 --- a/Lib/fontTools/varLib/__init__.py +++ b/Lib/fontTools/varLib/__init__.py @@ -743,7 +743,8 @@ def load_designspace(designspace): assert axis_name in axes, "Location axis '%s' unknown for '%s'." % (axis_name, obj_name) for axis_name,axis in axes.items(): if axis_name not in loc: - loc[axis_name] = axis.default + # NOTE: `axis.default` is always user-space, but `obj.location` always design-space. + loc[axis_name] = axis.map_forward(axis.default) else: v = axis.map_backward(loc[axis_name]) assert axis.minimum <= v <= axis.maximum, "Location for axis '%s' (mapped to %s) out of range for '%s' [%s..%s]" % (axis_name, v, obj_name, axis.minimum, axis.maximum) diff --git a/Tests/varLib/data/VarLibLocationTest.designspace b/Tests/varLib/data/VarLibLocationTest.designspace new file mode 100644 index 000000000..b73e1b50d --- /dev/null +++ b/Tests/varLib/data/VarLibLocationTest.designspace @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Tests/varLib/varLib_test.py b/Tests/varLib/varLib_test.py index 02b893ad7..b1faa50b0 100644 --- a/Tests/varLib/varLib_test.py +++ b/Tests/varLib/varLib_test.py @@ -1,6 +1,6 @@ from fontTools.misc.py23 import * from fontTools.ttLib import TTFont, newTable -from fontTools.varLib import build +from fontTools.varLib import build, load_designspace from fontTools.varLib.mutator import instantiateVariableFont from fontTools.varLib import main as varLib_main, load_masters from fontTools.varLib import set_default_weight_width_slant @@ -728,6 +728,13 @@ class BuildTest(unittest.TestCase): ("B", "D"): 40, } + def test_designspace_fill_in_location(self): + ds_path = self.get_test_input("VarLibLocationTest.designspace") + ds = DesignSpaceDocument.fromfile(ds_path) + ds_loaded = load_designspace(ds) + + assert ds_loaded.instances[0].location == {"weight": 0, "width": 50} + def test_load_masters_layerName_without_required_font(): ds = DesignSpaceDocument() From 16bff17483c1b41628f9b5b944a6325e60757f1c Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 12 Feb 2020 12:53:09 +0000 Subject: [PATCH 03/44] head: when checking for extra padding at end of table, must compare bytes, not str --- Lib/fontTools/ttLib/tables/_h_e_a_d.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/fontTools/ttLib/tables/_h_e_a_d.py b/Lib/fontTools/ttLib/tables/_h_e_a_d.py index cef0bf180..154669aee 100644 --- a/Lib/fontTools/ttLib/tables/_h_e_a_d.py +++ b/Lib/fontTools/ttLib/tables/_h_e_a_d.py @@ -41,7 +41,7 @@ class table__h_e_a_d(DefaultTable.DefaultTable): if rest: # this is quite illegal, but there seem to be fonts out there that do this log.warning("extra bytes at the end of 'head' table") - assert rest == "\0\0" + assert rest == b"\0\0" # For timestamp fields, ignore the top four bytes. Some fonts have # bogus values there. Since till 2038 those bytes only can be zero, From 578aa16a365d9cc459eeb9cd1230444bcdc08c61 Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Wed, 12 Feb 2020 12:55:07 +0000 Subject: [PATCH 04/44] Add NEWS entry --- NEWS.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 4b65220aa..b04acbd98 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -1,3 +1,6 @@ +- [varLib] When filling in the default axis value for a missing location of a source or + instance, correctly map the value forward. + 4.3.0 (released 2020-02-03) --------------------------- From 9c7ceadc0ee05276bd897937a22f1caec57e7e75 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 12 Feb 2020 14:06:31 +0000 Subject: [PATCH 05/44] [glyf] compile empty table as 1 null byte to make OTS and Windows happy Fixes #899 See https://github.com/khaledhosny/ots/issues/52#issuecomment-289369267 --- Lib/fontTools/ttLib/tables/_g_l_y_f.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Lib/fontTools/ttLib/tables/_g_l_y_f.py b/Lib/fontTools/ttLib/tables/_g_l_y_f.py index cc22ad0a8..8cbaa4c3e 100644 --- a/Lib/fontTools/ttLib/tables/_g_l_y_f.py +++ b/Lib/fontTools/ttLib/tables/_g_l_y_f.py @@ -122,6 +122,12 @@ class table__g_l_y_f(DefaultTable.DefaultTable): ttFont['loca'].set(locations) if 'maxp' in ttFont: ttFont['maxp'].numGlyphs = len(self.glyphs) + if not data: + # As a special case when all glyph in the font are empty, add a zero byte + # to the table, so that OTS doesn’t reject it, and to make the table work + # on Windows as well. + # See https://github.com/khaledhosny/ots/issues/52 + data = b"\0" return data def toXML(self, writer, ttFont, splitGlyphs=False): From 92e770ea16aff9ca8b95a17299984a3a5bb01fc6 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 12 Feb 2020 14:19:35 +0000 Subject: [PATCH 06/44] _g_l_y_f_test: add tests for compiling/decompiling empty glyf table --- Tests/ttLib/tables/_g_l_y_f_test.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/Tests/ttLib/tables/_g_l_y_f_test.py b/Tests/ttLib/tables/_g_l_y_f_test.py index 9e2b3555e..f0a42eee7 100644 --- a/Tests/ttLib/tables/_g_l_y_f_test.py +++ b/Tests/ttLib/tables/_g_l_y_f_test.py @@ -6,6 +6,7 @@ from fontTools.pens.recordingPen import RecordingPen, RecordingPointPen from fontTools.pens.pointPen import PointToSegmentPen from fontTools.ttLib import TTFont, newTable, TTLibError from fontTools.ttLib.tables._g_l_y_f import ( + Glyph, GlyphCoordinates, GlyphComponent, ARGS_ARE_XY_VALUES, @@ -190,7 +191,7 @@ def strip_ttLibVersion(string): return re.sub(' ttLibVersion=".*"', '', string) -class glyfTableTest(unittest.TestCase): +class GlyfTableTest(unittest.TestCase): def __init__(self, methodName): unittest.TestCase.__init__(self, methodName) @@ -338,6 +339,28 @@ class glyfTableTest(unittest.TestCase): glyfTable["glyph00003"].drawPoints(PointToSegmentPen(pen2), glyfTable) self.assertEqual(pen1.value, pen2.value) + def test_compile_empty_table(self): + font = TTFont(sfntVersion="\x00\x01\x00\x00") + font.importXML(GLYF_TTX) + glyfTable = font['glyf'] + # set all glyphs to zero contours + glyfTable.glyphs = {glyphName: Glyph() for glyphName in font.getGlyphOrder()} + glyfData = glyfTable.compile(font) + self.assertEqual(glyfData, b"\x00") + self.assertEqual(list(font["loca"]), [0] * (font["maxp"].numGlyphs+1)) + + def test_decompile_empty_table(self): + font = TTFont() + glyphNames = [".notdef", "space"] + font.setGlyphOrder(glyphNames) + font["loca"] = newTable("loca") + font["loca"].locations = [0] * (len(glyphNames) + 1) + font["glyf"] = newTable("glyf") + font["glyf"].decompile(b"\x00", font) + self.assertEqual(len(font["glyf"]), 2) + self.assertEqual(font["glyf"][".notdef"].numberOfContours, 0) + self.assertEqual(font["glyf"]["space"].numberOfContours, 0) + class GlyphComponentTest: From cf0567d8a588797e3df267f53f807749c986f503 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 12 Feb 2020 14:20:36 +0000 Subject: [PATCH 07/44] subset_test: test subsetting font to empty glyf table --- Tests/subset/subset_test.py | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/Tests/subset/subset_test.py b/Tests/subset/subset_test.py index 2ffc55501..14ca3137b 100644 --- a/Tests/subset/subset_test.py +++ b/Tests/subset/subset_test.py @@ -12,6 +12,8 @@ import shutil import sys import tempfile import unittest +import pathlib +import pytest class SubsetTest(unittest.TestCase): @@ -835,5 +837,40 @@ def test_subset_single_pos_format(): ] +@pytest.fixture +def ttf_path(tmp_path): + # $(dirname $0)/../ttLib/data + ttLib_data = pathlib.Path(__file__).parent.parent / "ttLib" / "data" + font = TTFont() + font.importXML(ttLib_data / "TestTTF-Regular.ttx") + font_path = tmp_path / "TestTTF-Regular.ttf" + font.save(font_path) + return font_path + + +def test_subset_empty_glyf(tmp_path, ttf_path): + subset_path = tmp_path / (ttf_path.name + ".subset") + # only keep empty .notdef and space glyph, resulting in an empty glyf table + subset.main( + [ + str(ttf_path), + "--no-notdef-outline", + "--glyph-names", + f"--output-file={subset_path}", + "--glyphs=.notdef space", + ] + ) + subset_font = TTFont(subset_path) + + assert subset_font.getGlyphOrder() == [".notdef", "space"] + assert subset_font.reader['glyf'] == b"\x00" + + glyf = subset_font["glyf"] + assert all(glyf[g].numberOfContours == 0 for g in subset_font.getGlyphOrder()) + + loca = subset_font["loca"] + assert all(loc == 0 for loc in loca) + + if __name__ == "__main__": sys.exit(unittest.main()) From b6467b7e85adfa24ee42b0dd9dea869bb765cb4a Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 13 Feb 2020 13:49:01 +0000 Subject: [PATCH 08/44] ttGlyphPen: quantize component.transform to F2Dot14 to fix issue with bbox Fixes https://github.com/googlefonts/fontmake/issues/558 When drawing a composite glyph with a scaled component using the TTGlyphPen, the bounding box coordinates may change depending on whether one computes them *before* compiling or *after* decompiling. Before compiling, component.transform holds double precision floats, but after compiling and decompiling, these are necessarily clamped to F2Dot14 fixed precision. The TTGlyphPen needs to quantize transform floats to F2Dot14 so that the values don't change after compilation. Without this change, it may happen that round-tripping fonts through ttx (which by default recalcBBoxes) will produce different bounding boxes for composite glyphs that have scaled or transformed components. --- Lib/fontTools/pens/ttGlyphPen.py | 8 ++++++-- Tests/pens/ttGlyphPen_test.py | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/Lib/fontTools/pens/ttGlyphPen.py b/Lib/fontTools/pens/ttGlyphPen.py index f7b1483b2..0b64cb380 100644 --- a/Lib/fontTools/pens/ttGlyphPen.py +++ b/Lib/fontTools/pens/ttGlyphPen.py @@ -1,6 +1,6 @@ from fontTools.misc.py23 import * from array import array -from fontTools.misc.fixedTools import MAX_F2DOT14, otRound +from fontTools.misc.fixedTools import MAX_F2DOT14, otRound, floatToFixedToFloat from fontTools.pens.basePen import LoggingPen from fontTools.pens.transformPen import TransformPen from fontTools.ttLib.tables import ttProgram @@ -119,7 +119,11 @@ class TTGlyphPen(LoggingPen): component = GlyphComponent() component.glyphName = glyphName component.x, component.y = (otRound(v) for v in transformation[4:]) - transformation = transformation[:4] + # quantize floats to F2Dot14 so we get same values as when decompiled + # from a binary glyf table + transformation = tuple( + floatToFixedToFloat(v, 14) for v in transformation[:4] + ) if transformation != (1, 0, 0, 1): if (self.handleOverflowingTransforms and any(MAX_F2DOT14 < s <= 2 for s in transformation)): diff --git a/Tests/pens/ttGlyphPen_test.py b/Tests/pens/ttGlyphPen_test.py index 961738d47..f6ad84859 100644 --- a/Tests/pens/ttGlyphPen_test.py +++ b/Tests/pens/ttGlyphPen_test.py @@ -264,6 +264,33 @@ class TTGlyphPenTest(TestCase): compositeGlyph.recalcBounds(glyphSet) self.assertGlyphBoundsEqual(compositeGlyph, (-86, 0, 282, 1)) + def test_scaled_component_bounds(self): + glyphSet = {} + + pen = TTGlyphPen(glyphSet) + pen.moveTo((-231, 939)) + pen.lineTo((-55, 939)) + pen.lineTo((-55, 745)) + pen.lineTo((-231, 745)) + pen.closePath() + glyphSet["gravecomb"] = gravecomb = pen.glyph() + + pen = TTGlyphPen(glyphSet) + pen.moveTo((-278, 939)) + pen.lineTo((8, 939)) + pen.lineTo((8, 745)) + pen.lineTo((-278, 745)) + pen.closePath() + glyphSet["circumflexcomb"] = circumflexcomb = pen.glyph() + + pen = TTGlyphPen(glyphSet) + pen.addComponent("circumflexcomb", (1, 0, 0, 1, 0, 0)) + pen.addComponent("gravecomb", (0.9, 0, 0, 0.9, 198, 180)) + glyphSet["uni0302_uni0300"] = uni0302_uni0300 = pen.glyph() + + uni0302_uni0300.recalcBounds(glyphSet) + self.assertGlyphBoundsEqual(uni0302_uni0300, (-278, 745, 148, 1025)) + class _TestGlyph(object): def __init__(self, glyph): From 5cda8381f93c7b0962560a578bfcea3c4fa47d32 Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Thu, 13 Feb 2020 14:47:29 +0000 Subject: [PATCH 09/44] [feaLib] Check that glyph names referenced in the feature file are part of the glyph set (#1828) This checks that glyph names that appear in a feature file are actually in the glyph set provided in glyphNames. If the set is empty, no check is done. This preempts a KeyError later during saving of a TTFont object and makes this case much more easily catchable. Closes #1723. --- Lib/fontTools/feaLib/parser.py | 35 +++++++++++++++++++++++++++++++++- NEWS.rst | 2 ++ Tests/feaLib/builder_test.py | 2 +- Tests/feaLib/parser_test.py | 14 ++++++++++++++ 4 files changed, 51 insertions(+), 2 deletions(-) diff --git a/Lib/fontTools/feaLib/parser.py b/Lib/fontTools/feaLib/parser.py index ca036f281..bda34e261 100644 --- a/Lib/fontTools/feaLib/parser.py +++ b/Lib/fontTools/feaLib/parser.py @@ -19,6 +19,14 @@ class Parser(object): def __init__(self, featurefile, glyphNames=(), followIncludes=True, **kwargs): + """Initializes a Parser object. + + Note: the `glyphNames` iterable serves a double role to help distinguish + glyph names from ranges in the presence of hyphens and to ensure that glyph + names referenced in a feature file are actually part of a font's glyph set. + If the iterable is left empty, no glyph name in glyph set checking takes + place. + """ if "glyphMap" in kwargs: from fontTools.misc.loggingTools import deprecateArgument deprecateArgument("glyphMap", "use 'glyphNames' (iterable) instead") @@ -268,6 +276,7 @@ class Parser(object): if (accept_glyphname and self.next_token_type_ in (Lexer.NAME, Lexer.CID)): glyph = self.expect_glyph_() + self.check_glyph_name_in_glyph_set(glyph) return self.ast.GlyphName(glyph, location=self.cur_token_location_) if self.next_token_type_ is Lexer.GLYPHCLASS: self.advance_lexer_() @@ -292,6 +301,7 @@ class Parser(object): location = self.cur_token_location_ if '-' in glyph and glyph not in self.glyphNames_: start, limit = self.split_glyph_range_(glyph, location) + self.check_glyph_name_in_glyph_set(start, limit) glyphs.add_range( start, limit, self.make_glyph_range_(location, start, limit)) @@ -299,10 +309,12 @@ class Parser(object): start = glyph self.expect_symbol_("-") limit = self.expect_glyph_() + self.check_glyph_name_in_glyph_set(start, limit) glyphs.add_range( start, limit, self.make_glyph_range_(location, start, limit)) else: + self.check_glyph_name_in_glyph_set(glyph) glyphs.append(glyph) elif self.next_token_type_ is Lexer.CID: glyph = self.expect_glyph_() @@ -311,11 +323,17 @@ class Parser(object): range_start = self.cur_token_ self.expect_symbol_("-") range_end = self.expect_cid_() + self.check_glyph_name_in_glyph_set( + f"cid{range_start:05d}", + f"cid{range_end:05d}", + ) glyphs.add_cid_range(range_start, range_end, self.make_cid_range_(range_location, range_start, range_end)) else: - glyphs.append("cid%05d" % self.cur_token_) + glyph_name = f"cid{self.cur_token_:05d}" + self.check_glyph_name_in_glyph_set(glyph_name) + glyphs.append(glyph_name) elif self.next_token_type_ is Lexer.GLYPHCLASS: self.advance_lexer_() gc = self.glyphclasses_.resolve(self.cur_token_) @@ -1509,6 +1527,21 @@ class Parser(object): raise FeatureLibError("Expected a glyph name or CID", self.cur_token_location_) + def check_glyph_name_in_glyph_set(self, *names): + """Raises if glyph name (just `start`) or glyph names of a + range (`start` and `end`) are not in the glyph set. + + If no glyph set is present, does nothing. + """ + if self.glyphNames_: + missing = [name for name in names if name not in self.glyphNames_] + if missing: + raise FeatureLibError( + "The following glyph names are referenced but are missing from the " + f"glyph set: {', '.join(missing)}", + self.cur_token_location_ + ) + def expect_markClass_reference_(self): name = self.expect_class_name_() mc = self.glyphclasses_.resolve(name) diff --git a/NEWS.rst b/NEWS.rst index b04acbd98..166b2b36e 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -1,3 +1,5 @@ +- [feaLib] Parsing feature code now ensures that referenced glyph names are part of + the known glyph set, unless a glyph set was not provided. - [varLib] When filling in the default axis value for a missing location of a source or instance, correctly map the value forward. diff --git a/Tests/feaLib/builder_test.py b/Tests/feaLib/builder_test.py index b36a6d5d5..04002106a 100644 --- a/Tests/feaLib/builder_test.py +++ b/Tests/feaLib/builder_test.py @@ -41,7 +41,7 @@ def makeTTFont(): a_n_d T_h T_h.swash germandbls ydieresis yacute breve grave acute dieresis macron circumflex cedilla umlaut ogonek caron damma hamza sukun kasratan lam_meem_jeem noon.final noon.initial - by feature lookup sub table + by feature lookup sub table uni0327 uni0328 e.fina """.split() font = TTFont() font.setGlyphOrder(glyphs) diff --git a/Tests/feaLib/parser_test.py b/Tests/feaLib/parser_test.py index e11a68789..cb4e689b7 100644 --- a/Tests/feaLib/parser_test.py +++ b/Tests/feaLib/parser_test.py @@ -39,6 +39,14 @@ GLYPHNAMES = (""" 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.swash b.swash x.swash y.swash z.swash foobar foo.09 foo.1234 foo.9876 + one two five six acute grave dieresis umlaut cedilla ogonek macron + a_f_f_i o_f_f_i f_i f_f_i one.fitted one.oldstyle a.1 a.2 a.3 c_t + PRE SUF FIX BACK TRACK LOOK AHEAD ampersand ampersand.1 ampersand.2 + cid00001 cid00002 cid00003 cid00004 cid00005 cid00006 cid00007 + cid12345 cid78987 cid00999 cid01000 cid01001 cid00998 cid00995 + cid00111 cid00222 + comma endash emdash figuredash damma hamza + c_d d.alt n.end s.end f_f """).split() + ["foo.%d" % i for i in range(1, 200)] @@ -260,6 +268,12 @@ class ParserTest(unittest.TestCase): FeatureLibError, "Font revision numbers must be positive", self.parse, "table head {FontRevision -17.2;} head;") + def test_strict_glyph_name_check(self): + self.parse("@bad = [a b ccc];", glyphNames=("a", "b", "ccc")) + + with self.assertRaisesRegex(FeatureLibError, "missing from the glyph set: ccc"): + self.parse("@bad = [a b ccc];", glyphNames=("a", "b")) + def test_glyphclass(self): [gc] = self.parse("@dash = [endash emdash figuredash];").statements self.assertEqual(gc.name, "dash") From 55bfa4e076d1dbc81e29a68a112260dc9d121e2e Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Tue, 11 Feb 2020 13:41:51 +0000 Subject: [PATCH 10/44] Introduce errors submodule --- Lib/fontTools/varLib/errors.py | 39 ++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 Lib/fontTools/varLib/errors.py diff --git a/Lib/fontTools/varLib/errors.py b/Lib/fontTools/varLib/errors.py new file mode 100644 index 000000000..b73f1886a --- /dev/null +++ b/Lib/fontTools/varLib/errors.py @@ -0,0 +1,39 @@ +class VarLibError(Exception): + """Base exception for the varLib module.""" + + +class VarLibValidationError(VarLibError): + """Raised when input data is invalid from varLib's point of view.""" + + +class VarLibMergeError(VarLibError): + """Raised when input data cannot be merged into a variable font.""" + + +class VarLibCFFDictMergeError(VarLibMergeError): + """Raised when a CFF PrivateDict cannot be merged.""" + + def __init__(self, key, value, values): + error_msg = ( + f"For the Private Dict key '{key}', the default font value list:" + f"\n\t{value}\nhad a different number of values than a region font:" + ) + for region_value in values: + error_msg += f"\n\t{region_value}" + self.args = (error_msg,) + + +class VarLibCFFPointTypeMergeError(VarLibMergeError): + """Raised when a CFF glyph cannot be merged.""" + + def __init__(self, point_type, pt_index, m_index, default_type, glyph_name): + error_msg = ( + f"Glyph '{glyph_name}': '{point_type}' at point index {pt_index} in " + f"master index {m_index} differs from the default font point type " + f"'{default_type}'" + ) + self.args = (error_msg,) + + +class VariationModelError(VarLibError): + """Raised when a variation model is faulty.""" From 8f7a796bd3206c0a9d8a17482f450ab5936c1d77 Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Tue, 11 Feb 2020 13:51:07 +0000 Subject: [PATCH 11/44] init: Convert existing raised exceptions to new appropriate ones --- Lib/fontTools/varLib/__init__.py | 16 +++++++--------- Tests/varLib/varLib_test.py | 3 ++- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/Lib/fontTools/varLib/__init__.py b/Lib/fontTools/varLib/__init__.py index 9bae80d12..d965e8884 100644 --- a/Lib/fontTools/varLib/__init__.py +++ b/Lib/fontTools/varLib/__init__.py @@ -39,13 +39,11 @@ import os.path import logging from copy import deepcopy from pprint import pformat +from .errors import VarLibError, VarLibValidationError log = logging.getLogger("fontTools.varLib") -class VarLibError(Exception): - pass - # # Creation routines # @@ -703,7 +701,7 @@ def load_designspace(designspace): masters = ds.sources if not masters: - raise VarLibError("no sources found in .designspace") + raise VarLibValidationError("Designspace must have at least one source.") instances = ds.instances standard_axis_map = OrderedDict([ @@ -927,7 +925,7 @@ def _open_font(path, master_finder=lambda s: s): elif tp in ("TTF", "OTF", "WOFF", "WOFF2"): font = TTFont(master_path) else: - raise VarLibError("Invalid master path: %r" % master_path) + raise VarLibValidationError("Invalid master path: %r" % master_path) return font @@ -947,10 +945,10 @@ def load_masters(designspace, master_finder=lambda s: s): # If a SourceDescriptor has a layer name, demand that the compiled TTFont # be supplied by the caller. This spares us from modifying MasterFinder. if master.layerName and master.font is None: - raise AttributeError( - "Designspace source '%s' specified a layer name but lacks the " - "required TTFont object in the 'font' attribute." - % (master.name or "") + raise VarLibValidationError( + f"Designspace source '{master.name or ''}' specified a " + "layer name but lacks the required TTFont object in the 'font' " + "attribute." ) return designspace.loadSourceFonts(_open_font, master_finder=master_finder) diff --git a/Tests/varLib/varLib_test.py b/Tests/varLib/varLib_test.py index b1faa50b0..a663eaeb4 100644 --- a/Tests/varLib/varLib_test.py +++ b/Tests/varLib/varLib_test.py @@ -1,6 +1,7 @@ from fontTools.misc.py23 import * from fontTools.ttLib import TTFont, newTable from fontTools.varLib import build, load_designspace +from fontTools.varLib.errors import VarLibValidationError from fontTools.varLib.mutator import instantiateVariableFont from fontTools.varLib import main as varLib_main, load_masters from fontTools.varLib import set_default_weight_width_slant @@ -744,7 +745,7 @@ def test_load_masters_layerName_without_required_font(): ds.addSource(s) with pytest.raises( - AttributeError, + VarLibValidationError, match="specified a layer name but lacks the required TTFont object", ): load_masters(ds) From c5c30588b5e67818391b5594099dabde01f2228a Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Tue, 11 Feb 2020 13:51:52 +0000 Subject: [PATCH 12/44] init: Convert input checking asserts into proper exceptions Also fix the avar output mapping check to allow values greater than OR EQUAL to the preceeding values. --- Lib/fontTools/varLib/__init__.py | 88 +++++++++++++++++++++++--------- 1 file changed, 65 insertions(+), 23 deletions(-) diff --git a/Lib/fontTools/varLib/__init__.py b/Lib/fontTools/varLib/__init__.py index d965e8884..c5402035b 100644 --- a/Lib/fontTools/varLib/__init__.py +++ b/Lib/fontTools/varLib/__init__.py @@ -79,7 +79,12 @@ def _add_fvar(font, axes, instances): coordinates = instance.location if "en" not in instance.localisedStyleName: - assert instance.styleName + if not instance.styleName: + raise VarLibValidationError( + f"Instance at location '{coordinates}' must have a default English " + "style name ('stylename' attribute on the instance element or a " + "stylename element with an 'xml:lang=\"en\"' attribute)." + ) localisedStyleName = dict(instance.localisedStyleName) localisedStyleName["en"] = tounicode(instance.styleName) else: @@ -135,20 +140,32 @@ def _add_avar(font, axes): # Current avar requirements. We don't have to enforce # these on the designer and can deduce some ourselves, # but for now just enforce them. - assert axis.minimum == min(keys) - assert axis.maximum == max(keys) - assert axis.default in keys - # No duplicates - assert len(set(keys)) == len(keys), ( - f"{axis.tag} axis: All axis mapping input='...' " - "values must be unique, but we found duplicates." - ) - assert len(set(vals)) == len(vals), ( - f"{axis.tag} axis: All axis mapping output='...' " - "values must be unique, but we found duplicates." - ) + if axis.minimum != min(keys): + raise VarLibValidationError( + f"Axis '{axis.name}': there must be a mapping for the axis minimum " + f"value {axis.minimum} and it must be the lowest input mapping value." + ) + if axis.maximum != max(keys): + raise VarLibValidationError( + f"Axis '{axis.name}': there must be a mapping for the axis maximum " + f"value {axis.maximum} and it must be the highest input mapping value." + ) + if axis.default not in keys: + raise VarLibValidationError( + f"Axis '{axis.name}': there must be a mapping for the axis default " + f"value {axis.default}." + ) + # No duplicate input values (output values can be >= their preceeding value). + if len(set(keys)) != len(keys): + raise VarLibValidationError( + f"Axis '{axis.name}': All axis mapping input='...' values must be " + "unique, but we found duplicates." + ) # Ascending values - assert sorted(vals) == vals + if sorted(vals) != vals: + raise VarLibValidationError( + f"Axis '{axis.name}': mapping output values must be in ascending order." + ) keys_triple = (axis.minimum, axis.default, axis.maximum) vals_triple = tuple(axis.map_forward(v) for v in keys_triple) @@ -212,8 +229,8 @@ def _add_stat(font, axes): def _add_gvar(font, masterModel, master_ttfs, tolerance=0.5, optimize=True): - - assert tolerance >= 0 + if tolerance < 0: + raise ValueError("`tolerance` must be a positive number.") log.info("Generating gvar") assert "gvar" not in font @@ -704,6 +721,7 @@ def load_designspace(designspace): raise VarLibValidationError("Designspace must have at least one source.") instances = ds.instances + # TODO: Use fontTools.designspaceLib.tagForAxisName instead. standard_axis_map = OrderedDict([ ('weight', ('wght', {'en': u'Weight'})), ('width', ('wdth', {'en': u'Width'})), @@ -713,11 +731,15 @@ def load_designspace(designspace): ]) # Setup axes + if not ds.axes: + raise VarLibValidationError(f"Designspace must have at least one axis.") + axes = OrderedDict() - for axis in ds.axes: + for axis_index, axis in enumerate(ds.axes): axis_name = axis.name if not axis_name: - assert axis.tag is not None + if not axis.tag: + raise VarLibValidationError(f"Axis at index {axis_index} needs a tag.") axis_name = axis.name = axis.tag if axis_name in standard_axis_map: @@ -726,7 +748,8 @@ def load_designspace(designspace): if not axis.labelNames: axis.labelNames.update(standard_axis_map[axis_name][1]) else: - assert axis.tag is not None + if not axis.tag: + raise VarLibValidationError(f"Axis at index {axis_index} needs a tag.") if not axis.labelNames: axis.labelNames["en"] = tounicode(axis_name) @@ -737,15 +760,28 @@ def load_designspace(designspace): for obj in masters+instances: obj_name = obj.name or obj.styleName or '' loc = obj.location + if loc is None: + raise VarLibValidationError( + f"Source or instance '{obj_name}' has no location." + ) for axis_name in loc.keys(): - assert axis_name in axes, "Location axis '%s' unknown for '%s'." % (axis_name, obj_name) + if axis_name not in axes: + raise VarLibValidationError( + f"Location axis '{axis_name}' unknown for '{obj_name}'." + ) for axis_name,axis in axes.items(): if axis_name not in loc: # NOTE: `axis.default` is always user-space, but `obj.location` always design-space. loc[axis_name] = axis.map_forward(axis.default) else: v = axis.map_backward(loc[axis_name]) - assert axis.minimum <= v <= axis.maximum, "Location for axis '%s' (mapped to %s) out of range for '%s' [%s..%s]" % (axis_name, v, obj_name, axis.minimum, axis.maximum) + if not (axis.minimum <= v <= axis.maximum): + raise VarLibValidationError( + f"Source or instance '{obj_name}' has out-of-range location " + f"for axis '{axis_name}': is mapped to {v} but must be in " + f"mapped range [{axis.minimum}..{axis.maximum}] (NOTE: all " + "values are in user-space)." + ) # Normalize master locations @@ -766,9 +802,15 @@ def load_designspace(designspace): base_idx = None for i,m in enumerate(normalized_master_locs): if all(v == 0 for v in m.values()): - assert base_idx is None + if base_idx is not None: + raise VarLibValidationError( + "More than one base master found in Designspace." + ) base_idx = i - assert base_idx is not None, "Base master not found; no master at default location?" + if base_idx is None: + raise VarLibValidationError( + "Base master not found; no master at default location?" + ) log.info("Index of base master: %s", base_idx) return _DesignSpaceData( From 5a53d1d4ad63b94c05b51c3ddf4465b459d3e1a7 Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Tue, 11 Feb 2020 13:53:25 +0000 Subject: [PATCH 13/44] models: Use new exceptions where input is checked --- Lib/fontTools/varLib/models.py | 13 ++++++++++--- Tests/varLib/models_test.py | 4 ++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/Lib/fontTools/varLib/models.py b/Lib/fontTools/varLib/models.py index 2ffe33e2a..d6837ee62 100644 --- a/Lib/fontTools/varLib/models.py +++ b/Lib/fontTools/varLib/models.py @@ -5,6 +5,8 @@ __all__ = ['nonNone', 'allNone', 'allEqual', 'allEqualTo', 'subList', 'supportScalar', 'VariationModel'] +from .errors import VariationModelError + def nonNone(lst): return [l for l in lst if l is not None] @@ -43,7 +45,11 @@ def normalizeValue(v, triple): 0.5 """ lower, default, upper = triple - assert lower <= default <= upper, "invalid axis values: %3.3f, %3.3f %3.3f"%(lower, default, upper) + if not (lower <= default <= upper): + raise ValueError( + f"Invalid axis values, must be minimum, default, maximum: " + f"{lower:3.3f}, {default:3.3f}, {upper:3.3f}" + ) v = max(min(v, upper), lower) if v == default: v = 0. @@ -192,7 +198,7 @@ class VariationModel(object): def __init__(self, locations, axisOrder=None): if len(set(tuple(sorted(l.items())) for l in locations)) != len(locations): - raise ValueError("locations must be unique") + raise VariationModelError("Locations must be unique.") self.origLocations = locations self.axisOrder = axisOrder if axisOrder is not None else [] @@ -220,7 +226,8 @@ class VariationModel(object): @staticmethod def getMasterLocationsSortKeyFunc(locations, axisOrder=[]): - assert {} in locations, "Base master not found." + if {} not in locations: + raise VariationModelError("Base master not found.") axisPoints = {} for loc in locations: if len(loc) != 1: diff --git a/Tests/varLib/models_test.py b/Tests/varLib/models_test.py index 56f7104a2..1027f29f6 100644 --- a/Tests/varLib/models_test.py +++ b/Tests/varLib/models_test.py @@ -1,6 +1,6 @@ from fontTools.misc.py23 import * from fontTools.varLib.models import ( - normalizeLocation, supportScalar, VariationModel) + normalizeLocation, supportScalar, VariationModel, VariationModelError) import pytest @@ -145,7 +145,7 @@ class VariationModelTest(object): assert model.deltaWeights == deltaWeights def test_init_duplicate_locations(self): - with pytest.raises(ValueError, match="locations must be unique"): + with pytest.raises(VariationModelError, match="Locations must be unique."): VariationModel( [ {"foo": 0.0, "bar": 0.0}, From 5b5c964b0fca56c3ea0e81f5f4e7c99747918841 Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Tue, 11 Feb 2020 13:53:56 +0000 Subject: [PATCH 14/44] cff: Use new exceptions Aliases for the old errors will stay because the AFDKO and maybe others use them. --- Lib/fontTools/varLib/cff.py | 45 +++++++++++-------------------------- 1 file changed, 13 insertions(+), 32 deletions(-) diff --git a/Lib/fontTools/varLib/cff.py b/Lib/fontTools/varLib/cff.py index 7895d274f..000e1b34a 100644 --- a/Lib/fontTools/varLib/cff.py +++ b/Lib/fontTools/varLib/cff.py @@ -1,5 +1,4 @@ from collections import namedtuple -import os from fontTools.cffLib import ( maxStackLimit, TopDictIndex, @@ -21,6 +20,13 @@ from fontTools.varLib.models import allEqual from fontTools.misc.psCharStrings import T2CharString, T2OutlineExtractor from fontTools.pens.t2CharStringPen import T2CharStringPen, t2c_round +from .errors import VarLibCFFDictMergeError, VarLibCFFPointTypeMergeError, VarLibMergeError + + +# Backwards compatibility +MergeDictError = VarLibCFFDictMergeError +MergeTypeError = VarLibCFFPointTypeMergeError + def addCFFVarStore(varFont, varModel, varDataList, masterSupports): fvarTable = varFont['fvar'] @@ -126,16 +132,6 @@ def convertCFFtoCFF2(varFont): del varFont['CFF '] -class MergeDictError(TypeError): - def __init__(self, key, value, values): - error_msg = ["For the Private Dict key '{}', ".format(key), - "the default font value list:", - "\t{}".format(value), - "had a different number of values than a region font:"] - error_msg += ["\t{}".format(region_value) for region_value in values] - error_msg = os.linesep.join(error_msg) - - def conv_to_int(num): if isinstance(num, float) and num.is_integer(): return int(num) @@ -219,7 +215,7 @@ def merge_PrivateDicts(top_dicts, vsindex_dict, var_model, fd_map): try: values = zip(*values) except IndexError: - raise MergeDictError(key, value, values) + raise VarLibCFFDictMergeError(key, value, values) """ Row 0 contains the first value from each master. Convert each row from absolute values to relative @@ -426,21 +422,6 @@ def merge_charstrings(glyphOrder, num_masters, top_dicts, masterModel): return cvData -class MergeTypeError(TypeError): - def __init__(self, point_type, pt_index, m_index, default_type, glyphName): - self.error_msg = [ - "In glyph '{gname}' " - "'{point_type}' at point index {pt_index} in master " - "index {m_index} differs from the default font point " - "type '{default_type}'" - "".format( - gname=glyphName, - point_type=point_type, pt_index=pt_index, - m_index=m_index, default_type=default_type) - ][0] - super(MergeTypeError, self).__init__(self.error_msg) - - def makeRoundNumberFunc(tolerance): if tolerance < 0: raise ValueError("Rounding tolerance must be positive") @@ -547,7 +528,7 @@ class CFF2CharStringMergePen(T2CharStringPen): else: cmd = self._commands[self.pt_index] if cmd[0] != point_type: - raise MergeTypeError( + raise VarLibCFFPointTypeMergeError( point_type, self.pt_index, len(cmd[1]), cmd[0], self.glyphName) @@ -560,7 +541,7 @@ class CFF2CharStringMergePen(T2CharStringPen): else: cmd = self._commands[self.pt_index] if cmd[0] != hint_type: - raise MergeTypeError(hint_type, self.pt_index, len(cmd[1]), + raise VarLibCFFPointTypeMergeError(hint_type, self.pt_index, len(cmd[1]), cmd[0], self.glyphName) cmd[1].append(args) self.pt_index += 1 @@ -576,7 +557,7 @@ class CFF2CharStringMergePen(T2CharStringPen): else: cmd = self._commands[self.pt_index] if cmd[0] != hint_type: - raise MergeTypeError(hint_type, self.pt_index, len(cmd[1]), + raise VarLibCFFPointTypeMergeError(hint_type, self.pt_index, len(cmd[1]), cmd[0], self.glyphName) self.pt_index += 1 cmd = self._commands[self.pt_index] @@ -646,8 +627,8 @@ class CFF2CharStringMergePen(T2CharStringPen): # second has only args. if lastOp in ['hintmask', 'cntrmask']: coord = list(cmd[1]) - assert allEqual(coord), ( - "hintmask values cannot differ between source fonts.") + if not allEqual(coord): + raise VarLibMergeError("Hintmask values cannot differ between source fonts.") cmd[1] = [coord[0][0]] else: coords = cmd[1] From af0567f84751bcb92e530470d6d1bb4727bb9deb Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Tue, 11 Feb 2020 13:54:08 +0000 Subject: [PATCH 15/44] featureVars: Use new exceptions --- Lib/fontTools/varLib/featureVars.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Lib/fontTools/varLib/featureVars.py b/Lib/fontTools/varLib/featureVars.py index 287a885d7..dab9a0b1c 100644 --- a/Lib/fontTools/varLib/featureVars.py +++ b/Lib/fontTools/varLib/featureVars.py @@ -10,6 +10,8 @@ from fontTools.ttLib.tables import otTables as ot from fontTools.otlLib.builder import buildLookup, buildSingleSubstSubtable from collections import OrderedDict +from .errors import VarLibValidationError + def addFeatureVariations(font, conditionalSubstitutions, featureTag='rvrn'): """Add conditional substitutions to a Variable Font. @@ -312,7 +314,10 @@ def addFeatureVariationsRaw(font, conditionalSubstitutions, featureTag='rvrn'): for conditionSet, substitutions in conditionalSubstitutions: conditionTable = [] for axisTag, (minValue, maxValue) in sorted(conditionSet.items()): - assert minValue < maxValue + if minValue > maxValue: + raise VarLibValidationError( + "A condition set has a minimum value above the maximum value." + ) ct = buildConditionTable(axisIndices[axisTag], minValue, maxValue) conditionTable.append(ct) From 4320392eb5c94fcd2d90fba1b1853c037dbde203 Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Tue, 11 Feb 2020 14:24:50 +0000 Subject: [PATCH 16/44] merger: Convert input checking asserts into proper exceptions --- Lib/fontTools/varLib/merger.py | 85 +++++++++++++++++++++++++--------- 1 file changed, 63 insertions(+), 22 deletions(-) diff --git a/Lib/fontTools/varLib/merger.py b/Lib/fontTools/varLib/merger.py index f298b64d8..3f04364fe 100644 --- a/Lib/fontTools/varLib/merger.py +++ b/Lib/fontTools/varLib/merger.py @@ -14,6 +14,8 @@ from fontTools.varLib.varStore import VarStoreInstancer from functools import reduce from fontTools.otlLib.builder import buildSinglePos +from .errors import VarLibMergeError + class Merger(object): @@ -66,8 +68,8 @@ class Merger(object): if hasattr(item, "ensureDecompiled"): item.ensureDecompiled() keys = sorted(vars(out).keys()) - assert all(keys == sorted(vars(v).keys()) for v in lst), \ - (keys, [sorted(vars(v).keys()) for v in lst]) + if not all(keys == sorted(vars(v).keys()) for v in lst): + raise VarLibMergeError((keys, [sorted(vars(v).keys()) for v in lst])) mergers = self.mergersFor(out) defaultMerger = mergers.get('*', self.__class__.mergeThings) try: @@ -82,7 +84,8 @@ class Merger(object): raise def mergeLists(self, out, lst): - assert allEqualTo(out, lst, len), (len(out), [len(v) for v in lst]) + if not allEqualTo(out, lst, len): + raise VarLibMergeError((len(out), [len(v) for v in lst])) for i,(value,values) in enumerate(zip(out, zip(*lst))): try: self.mergeThings(value, values) @@ -92,7 +95,8 @@ class Merger(object): def mergeThings(self, out, lst): try: - assert allEqualTo(out, lst, type), (out, lst) + if not allEqualTo(out, lst, type): + raise VarLibMergeError((out, lst)) mergerFunc = self.mergersFor(out).get(None, None) if mergerFunc is not None: mergerFunc(self, out, lst) @@ -101,7 +105,8 @@ class Merger(object): elif isinstance(out, list): self.mergeLists(out, lst) else: - assert allEqualTo(out, lst), (out, lst) + if not allEqualTo(out, lst): + raise VarLibMergeError((out, lst)) except Exception as e: e.args = e.args + (type(out).__name__,) raise @@ -122,7 +127,8 @@ class AligningMerger(Merger): @AligningMerger.merger(ot.GDEF, "GlyphClassDef") def merge(merger, self, lst): if self is None: - assert allNone(lst), (lst) + if not allNone(lst): + raise VarLibMergeError(lst) return lst = [l.classDefs for l in lst] @@ -134,7 +140,8 @@ def merge(merger, self, lst): allKeys.update(*[l.keys() for l in lst]) for k in allKeys: allValues = nonNone(l.get(k) for l in lst) - assert allEqual(allValues), allValues + if not allEqual(allValues): + raise VarLibMergeError(allValues) if not allValues: self[k] = None else: @@ -170,7 +177,8 @@ def _merge_GlyphOrders(font, lst, values_lst=None, default=None): sortKey = font.getReverseGlyphMap().__getitem__ order = sorted(combined, key=sortKey) # Make sure all input glyphsets were in proper order - assert all(sorted(vs, key=sortKey) == vs for vs in lst), "glyph orders are not consistent across masters" + if not all(sorted(vs, key=sortKey) == vs for vs in lst): + raise VarLibMergeError("Glyph order inconsistent across masters.") del combined paddedValues = None @@ -197,7 +205,10 @@ def _Lookup_SinglePos_get_effective_value(subtables, glyph): elif self.Format == 2: return self.Value[self.Coverage.glyphs.index(glyph)] else: - assert 0 + raise VarLibMergeError( + "Cannot retrieve effective value for SinglePos lookup, unsupported " + f"format {self.Format}." + ) return None def _Lookup_PairPos_get_effective_value_pair(subtables, firstGlyph, secondGlyph): @@ -219,13 +230,17 @@ def _Lookup_PairPos_get_effective_value_pair(subtables, firstGlyph, secondGlyph) klass2 = self.ClassDef2.classDefs.get(secondGlyph, 0) return self.Class1Record[klass1].Class2Record[klass2] else: - assert 0 + raise VarLibMergeError( + "Cannot retrieve effective value pair for PairPos lookup, unsupported " + f"format {self.Format}." + ) return None @AligningMerger.merger(ot.SinglePos) def merge(merger, self, lst): self.ValueFormat = valueFormat = reduce(int.__or__, [l.ValueFormat for l in lst], 0) - assert len(lst) == 1 or (valueFormat & ~0xF == 0), valueFormat + if not (len(lst) == 1 or (valueFormat & ~0xF == 0)): + raise VarLibMergeError(f"SinglePos format {valueFormat} is unsupported.") # If all have same coverage table and all are format 1, coverageGlyphs = self.Coverage.glyphs @@ -511,7 +526,9 @@ def merge(merger, self, lst): elif self.Format == 2: _PairPosFormat2_merge(self, lst, merger) else: - assert False + raise VarLibMergeError( + f"Cannot merge PairPos lookup, unsupported format {self.Format}." + ) del merger.valueFormat1, merger.valueFormat2 @@ -576,7 +593,8 @@ def _MarkBasePosFormat1_merge(self, lst, merger, Mark='Mark', Base='Base'): # failures in that case will probably signify mistakes in the # input masters. - assert allEqual(allClasses), allClasses + if not allEqual(allClasses): + raise VarLibMergeError(allClasses) if not allClasses: rec = None else: @@ -625,19 +643,31 @@ def _MarkBasePosFormat1_merge(self, lst, merger, Mark='Mark', Base='Base'): @AligningMerger.merger(ot.MarkBasePos) def merge(merger, self, lst): - assert allEqualTo(self.Format, (l.Format for l in lst)) + if not allEqualTo(self.Format, (l.Format for l in lst)): + raise VarLibMergeError( + f"MarkBasePos formats inconsistent across masters, " + f"expected {self.Format} but got {[l.Format for l in lst]}." + ) if self.Format == 1: _MarkBasePosFormat1_merge(self, lst, merger) else: - assert False + raise VarLibMergeError( + f"Cannot merge MarkBasePos lookup, unsupported format {self.Format}." + ) @AligningMerger.merger(ot.MarkMarkPos) def merge(merger, self, lst): - assert allEqualTo(self.Format, (l.Format for l in lst)) + if not allEqualTo(self.Format, (l.Format for l in lst)): + raise VarLibMergeError( + f"MarkMarkPos formats inconsistent across masters, " + f"expected {self.Format} but got {[l.Format for l in lst]}." + ) if self.Format == 1: _MarkBasePosFormat1_merge(self, lst, merger, 'Mark1', 'Mark2') else: - assert False + raise VarLibMergeError( + f"Cannot merge MarkMarkPos lookup, unsupported format {self.Format}." + ) def _PairSet_flatten(lst, font): @@ -766,8 +796,16 @@ def merge(merger, self, lst): if not sts: continue if sts[0].__class__.__name__.startswith('Extension'): - assert allEqual([st.__class__ for st in sts]) - assert allEqual([st.ExtensionLookupType for st in sts]) + if not allEqual([st.__class__ for st in sts]): + raise VarLibMergeError( + "Use of extensions inconsistent between masters: " + f"{[st.__class__.__name__ for st in sts]}." + ) + if not allEqual([st.ExtensionLookupType for st in sts]): + raise VarLibMergeError( + "Extension lookup type differs between masters: " + f"{[st.ExtensionLookupType for st in sts]}." + ) l.LookupType = sts[0].ExtensionLookupType new_sts = [st.ExtSubTable for st in sts] del sts[:] @@ -995,7 +1033,8 @@ class VariationMerger(AligningMerger): masterModel = None if None in lst: if allNone(lst): - assert out is None, (out, lst) + if out is not None: + raise VarLibMergeError((out, lst)) return masterModel = self.model model, lst = masterModel.getSubModel(lst) @@ -1015,7 +1054,8 @@ def buildVarDevTable(store_builder, master_values): @VariationMerger.merger(ot.CaretValue) def merge(merger, self, lst): - assert self.Format == 1 + if self.Format != 1: + raise VarLibMergeError(f"CaretValue format {self.Format} unsupported.") self.Coordinate, DeviceTable = buildVarDevTable(merger.store_builder, [a.Coordinate for a in lst]) if DeviceTable: self.Format = 3 @@ -1023,7 +1063,8 @@ def merge(merger, self, lst): @VariationMerger.merger(ot.Anchor) def merge(merger, self, lst): - assert self.Format == 1 + if self.Format != 1: + raise VarLibMergeError(f"Anchor format {self.Format} unsupported.") self.XCoordinate, XDeviceTable = buildVarDevTable(merger.store_builder, [a.XCoordinate for a in lst]) self.YCoordinate, YDeviceTable = buildVarDevTable(merger.store_builder, [a.YCoordinate for a in lst]) if XDeviceTable or YDeviceTable: From 3c12f51c2341fab2cc20179b47de6a4a1f560522 Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Thu, 13 Feb 2020 15:00:14 +0000 Subject: [PATCH 17/44] Update NEWS.rst --- NEWS.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 166b2b36e..98f16600a 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -2,6 +2,10 @@ the known glyph set, unless a glyph set was not provided. - [varLib] When filling in the default axis value for a missing location of a source or instance, correctly map the value forward. +- [varLib] The avar table can now contain mapping output values that are greater than + OR EQUAL to the preceeding value, as the avar specification allows this. +- [varLib] The errors of the module are now ordered hierarchically below VarLibError. + See #1821. 4.3.0 (released 2020-02-03) --------------------------- From 7c023c42af52e4628d0d82b5cfe77e3ab8e644cc Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 13 Feb 2020 18:07:57 +0000 Subject: [PATCH 18/44] [glyf] if comp uses anchors compute firstPt-secondPt offset after applying transform Fixes https://github.com/fonttools/fonttools/issues/1556 When a component uses firstPt/secondPt reference anchor points instead of XY offsets, and the component also has a transform, fonttools is incorrectly computing its bounding box. This is because we are computing the translation offset between firstPt and secondPt before applying the 2x2 scale/rotation/shear transform. By the time we do the translation, the offset is now incorrect. We need to compute the translation offset after we have applied the 2x2 transform. --- Lib/fontTools/ttLib/tables/_g_l_y_f.py | 42 ++++++++++++++------------ 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/Lib/fontTools/ttLib/tables/_g_l_y_f.py b/Lib/fontTools/ttLib/tables/_g_l_y_f.py index 8cbaa4c3e..a6cd1fc46 100644 --- a/Lib/fontTools/ttLib/tables/_g_l_y_f.py +++ b/Lib/fontTools/ttLib/tables/_g_l_y_f.py @@ -1012,33 +1012,37 @@ class Glyph(object): coordinates, endPts, flags = g.getCoordinates(glyfTable) except RecursionError: raise ttLib.TTLibError("glyph '%s' contains a recursive component reference" % compo.glyphName) + coordinates = GlyphCoordinates(coordinates) if hasattr(compo, "firstPt"): - # move according to two reference points + # component uses two reference points: we apply the transform _before_ + # computing the offset between the points + if hasattr(compo, "transform"): + coordinates.transform(compo.transform) x1,y1 = allCoords[compo.firstPt] x2,y2 = coordinates[compo.secondPt] move = x1-x2, y1-y2 - else: - move = compo.x, compo.y - - coordinates = GlyphCoordinates(coordinates) - if not hasattr(compo, "transform"): coordinates.translate(move) else: - apple_way = compo.flags & SCALED_COMPONENT_OFFSET - ms_way = compo.flags & UNSCALED_COMPONENT_OFFSET - assert not (apple_way and ms_way) - if not (apple_way or ms_way): - scale_component_offset = SCALE_COMPONENT_OFFSET_DEFAULT # see top of this file - else: - scale_component_offset = apple_way - if scale_component_offset: - # the Apple way: first move, then scale (ie. scale the component offset) + # component uses XY offsets + move = compo.x, compo.y + if not hasattr(compo, "transform"): coordinates.translate(move) - coordinates.transform(compo.transform) else: - # the MS way: first scale, then move - coordinates.transform(compo.transform) - coordinates.translate(move) + apple_way = compo.flags & SCALED_COMPONENT_OFFSET + ms_way = compo.flags & UNSCALED_COMPONENT_OFFSET + assert not (apple_way and ms_way) + if not (apple_way or ms_way): + scale_component_offset = SCALE_COMPONENT_OFFSET_DEFAULT # see top of this file + else: + scale_component_offset = apple_way + if scale_component_offset: + # the Apple way: first move, then scale (ie. scale the component offset) + coordinates.translate(move) + coordinates.transform(compo.transform) + else: + # the MS way: first scale, then move + coordinates.transform(compo.transform) + coordinates.translate(move) offset = len(allCoords) allEndPts.extend(e + offset for e in endPts) allCoords.extend(coordinates) From 183792c747d795c1ba1809c6efae627f80baa9f3 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 13 Feb 2020 18:08:38 +0000 Subject: [PATCH 19/44] _g_l_y_f_test: add tests for Glyph.getCoordinates and GlyphComponent.to/fromXML --- Tests/ttLib/tables/_g_l_y_f_test.py | 133 ++++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) diff --git a/Tests/ttLib/tables/_g_l_y_f_test.py b/Tests/ttLib/tables/_g_l_y_f_test.py index f0a42eee7..cec15cce6 100644 --- a/Tests/ttLib/tables/_g_l_y_f_test.py +++ b/Tests/ttLib/tables/_g_l_y_f_test.py @@ -10,6 +10,8 @@ from fontTools.ttLib.tables._g_l_y_f import ( GlyphCoordinates, GlyphComponent, ARGS_ARE_XY_VALUES, + SCALED_COMPONENT_OFFSET, + UNSCALED_COMPONENT_OFFSET, WE_HAVE_A_SCALE, WE_HAVE_A_TWO_BY_TWO, WE_HAVE_AN_X_AND_Y_SCALE, @@ -362,6 +364,114 @@ class GlyfTableTest(unittest.TestCase): self.assertEqual(font["glyf"]["space"].numberOfContours, 0) +class GlyphTest: + + def test_getCoordinates(self): + glyphSet = {} + pen = TTGlyphPen(glyphSet) + pen.moveTo((0, 0)) + pen.lineTo((100, 0)) + pen.lineTo((100, 100)) + pen.lineTo((0, 100)) + pen.closePath() + # simple contour glyph + glyphSet["a"] = a = pen.glyph() + + assert a.getCoordinates(glyphSet) == ( + GlyphCoordinates([(0, 0), (100, 0), (100, 100), (0, 100)]), + [3], + array.array("B", [1, 1, 1, 1]), + ) + + # composite glyph with only XY offset + pen = TTGlyphPen(glyphSet) + pen.addComponent("a", (1, 0, 0, 1, 10, 20)) + glyphSet["b"] = b = pen.glyph() + + assert b.getCoordinates(glyphSet) == ( + GlyphCoordinates([(10, 20), (110, 20), (110, 120), (10, 120)]), + [3], + array.array("B", [1, 1, 1, 1]), + ) + + # composite glyph with a scale (and referencing another composite glyph) + pen = TTGlyphPen(glyphSet) + pen.addComponent("b", (0.5, 0, 0, 0.5, 0, 0)) + glyphSet["c"] = c = pen.glyph() + + assert c.getCoordinates(glyphSet) == ( + GlyphCoordinates([(5, 10), (55, 10), (55, 60), (5, 60)]), + [3], + array.array("B", [1, 1, 1, 1]), + ) + + # composite glyph with unscaled offset (MS-style) + pen = TTGlyphPen(glyphSet) + pen.addComponent("a", (0.5, 0, 0, 0.5, 10, 20)) + glyphSet["d"] = d = pen.glyph() + d.components[0].flags |= UNSCALED_COMPONENT_OFFSET + + assert d.getCoordinates(glyphSet) == ( + GlyphCoordinates([(10, 20), (60, 20), (60, 70), (10, 70)]), + [3], + array.array("B", [1, 1, 1, 1]), + ) + + # composite glyph with a scaled offset (Apple-style) + pen = TTGlyphPen(glyphSet) + pen.addComponent("a", (0.5, 0, 0, 0.5, 10, 20)) + glyphSet["e"] = e = pen.glyph() + e.components[0].flags |= SCALED_COMPONENT_OFFSET + + assert e.getCoordinates(glyphSet) == ( + GlyphCoordinates([(5, 10), (55, 10), (55, 60), (5, 60)]), + [3], + array.array("B", [1, 1, 1, 1]), + ) + + # composite glyph where the 2nd and 3rd components use anchor points + pen = TTGlyphPen(glyphSet) + pen.addComponent("a", (1, 0, 0, 1, 0, 0)) + glyphSet["f"] = f = pen.glyph() + + comp1 = GlyphComponent() + comp1.glyphName = "a" + # aling the new component's pt 0 to pt 2 of contour points added so far + comp1.firstPt = 2 + comp1.secondPt = 0 + comp1.flags = 0 + f.components.append(comp1) + + comp2 = GlyphComponent() + comp2.glyphName = "a" + # aling the new component's pt 0 to pt 6 of contour points added so far + comp2.firstPt = 6 + comp2.secondPt = 0 + comp2.transform = [[0.707107, 0.707107], [-0.707107, 0.707107]] # rotate 45 deg + comp2.flags = WE_HAVE_A_TWO_BY_TWO + f.components.append(comp2) + + coords, end_pts, flags = f.getCoordinates(glyphSet) + assert end_pts == [3, 7, 11] + assert flags == array.array("B", [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]) + assert list(sum(coords, ())) == pytest.approx( + [ + 0, 0, + 100, 0, + 100, 100, + 0, 100, + 100, 100, + 200, 100, + 200, 200, + 100, 200, + 200, 200, + 270.7107, 270.7107, + 200.0, 341.4214, + 129.2893, 270.7107, + ] + ) + + class GlyphComponentTest: def test_toXML_no_transform(self): @@ -479,6 +589,29 @@ class GlyphComponentTest: ): assert value == pytest.approx(expected) + def test_toXML_reference_points(self): + comp = GlyphComponent() + comp.glyphName = "a" + comp.flags = 0 + comp.firstPt = 1 + comp.secondPt = 2 + + assert getXML(comp.toXML) == [ + '' + ] + + def test_fromXML_reference_points(self): + comp = GlyphComponent() + for name, attrs, content in parseXML( + [''] + ): + comp.fromXML(name, attrs, content, ttFont=None) + + assert comp.glyphName == "a" + assert comp.flags == 0 + assert (comp.firstPt, comp.secondPt) == (1, 2) + assert not hasattr(comp, "transform") + if __name__ == "__main__": import sys From 8423e01a182f20a5dc18d1295f0d22b98269f334 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Sat, 15 Feb 2020 13:25:45 +0000 Subject: [PATCH 20/44] ufoLib: don't crash if UFO2 has openTypeHheaCaretOffset=0.0 validators are picky so give them integers when they want integers --- Lib/fontTools/ufoLib/__init__.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/Lib/fontTools/ufoLib/__init__.py b/Lib/fontTools/ufoLib/__init__.py index fb09bf6e5..4b864378a 100755 --- a/Lib/fontTools/ufoLib/__init__.py +++ b/Lib/fontTools/ufoLib/__init__.py @@ -2146,18 +2146,14 @@ def convertFontInfoValueForAttributeFromVersion2ToVersion3(attr, value): """ if attr in _ufo2To3FloatToInt: try: - v = int(round(value)) + value = round(value) except (ValueError, TypeError): raise UFOLibError("Could not convert value for %s." % attr) - if v != value: - value = v if attr in _ufo2To3NonNegativeInt: try: - v = int(abs(value)) + value = int(abs(value)) except (ValueError, TypeError): raise UFOLibError("Could not convert value for %s." % attr) - if v != value: - value = v elif attr in _ufo2To3NonNegativeIntOrFloat: try: v = float(abs(value)) From 191a036f2676d756b3c0835a513e41597bf7ad63 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 10 Feb 2020 17:31:14 +0000 Subject: [PATCH 21/44] colorLib: add buildCOLR and buildCPAL --- Lib/fontTools/colorLib/__init__.py | 0 Lib/fontTools/colorLib/builder.py | 54 ++++++++++++++++++++++++++++++ Lib/fontTools/colorLib/errors.py | 3 ++ 3 files changed, 57 insertions(+) create mode 100644 Lib/fontTools/colorLib/__init__.py create mode 100644 Lib/fontTools/colorLib/builder.py create mode 100644 Lib/fontTools/colorLib/errors.py diff --git a/Lib/fontTools/colorLib/__init__.py b/Lib/fontTools/colorLib/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/Lib/fontTools/colorLib/builder.py b/Lib/fontTools/colorLib/builder.py new file mode 100644 index 000000000..7141120e7 --- /dev/null +++ b/Lib/fontTools/colorLib/builder.py @@ -0,0 +1,54 @@ +from fontTools.ttLib import newTable +from .errors import ColorLibError + + +def buildCOLR(colorLayers): + """Build COLR table from color layers mapping. + + Args: + colorLayers: Dict[str, List[Tuple[str, int]]]: map of base glyph names (str) + to lists of layer glyph names (str) and palette indices (int) tuples. + + Return: + A new COLRv0 table. + """ + from fontTools.ttLib.tables.C_O_L_R_ import LayerRecord + + colorLayerLists = {} + for baseGlyphName, layers in colorLayers.items(): + colorLayerLists[baseGlyphName] = [ + LayerRecord(layerGlyphName, colorID) for layerGlyphName, colorID in layers + ] + + colr = newTable("COLR") + colr.version = 0 + colr.ColorLayers = colorLayerLists + return colr + + +def buildCPAL(palettes): + """Build CPAL table from list of color palettes. + + Args: + palettes: List[List[Tuple[float, float, float, float]]]: list of lists + colors encoded as tuples of (R, G, B, A) floats. + + Return: + A new CPALv0 table. + """ + from fontTools.ttLib.tables.C_P_A_L_ import Color + + if len({len(p) for p in palettes}) != 1: + raise ColorLibError("color palettes have different lengths") + cpal = newTable("CPAL") + # TODO(anthotype): Support version 1 with palette types, labels and entry labels. + cpal.version = 0 + cpal.numPaletteEntries = len(palettes[0]) + cpal.palettes = [ + [ + Color(*(round(v * 255) for v in (blue, green, red, alpha))) + for red, green, blue, alpha in palette + ] + for palette in palettes + ] + return cpal diff --git a/Lib/fontTools/colorLib/errors.py b/Lib/fontTools/colorLib/errors.py new file mode 100644 index 000000000..a0bdda174 --- /dev/null +++ b/Lib/fontTools/colorLib/errors.py @@ -0,0 +1,3 @@ + +class ColorLibError(Exception): + pass From f8b9887f856f62e325ba31c54ea55ac5f55a43f7 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 10 Feb 2020 17:35:05 +0000 Subject: [PATCH 22/44] fontBuilder: add setupCOLR and setupCPAL methods --- Lib/fontTools/fontBuilder.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Lib/fontTools/fontBuilder.py b/Lib/fontTools/fontBuilder.py index 6e72dd520..f46db6f87 100644 --- a/Lib/fontTools/fontBuilder.py +++ b/Lib/fontTools/fontBuilder.py @@ -768,6 +768,24 @@ class FontBuilder(object): self.font, conditionalSubstitutions, featureTag=featureTag ) + def setupCOLR(self, colorLayers): + """Build new COLR table using color layers dictionary. + + Cf. `fontTools.colorLib.builder.buildCOLR`. + """ + from fontTools.colorLib.builder import buildCOLR + + self.font["COLR"] = buildCOLR(colorLayers) + + def setupCPAL(self, palettes): + """Build new CPAL table using list of palettes. + + Cf. `fontTools.colorLib.builder.buildCPAL`. + """ + from fontTools.colorLib.builder import buildCPAL + + self.font["CPAL"] = buildCPAL(palettes) + def buildCmapSubTable(cmapping, format, platformID, platEncID): subTable = cmap_classes[format](format) From acfae6721b868de490180aff7a7cdd8c74a6ca9c Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 17 Feb 2020 11:58:29 +0000 Subject: [PATCH 23/44] add tests for buildCORL and buildCPAL --- Tests/colorLib/__init__.py | 0 Tests/colorLib/builder_test.py | 56 ++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 Tests/colorLib/__init__.py create mode 100644 Tests/colorLib/builder_test.py diff --git a/Tests/colorLib/__init__.py b/Tests/colorLib/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/Tests/colorLib/builder_test.py b/Tests/colorLib/builder_test.py new file mode 100644 index 000000000..bd3898a8b --- /dev/null +++ b/Tests/colorLib/builder_test.py @@ -0,0 +1,56 @@ +from fontTools.colorLib import builder +from fontTools.colorLib.errors import ColorLibError +import pytest + + +def test_buildCOLR_v0(): + color_layer_lists = { + "a": [("a.color0", 0), ("a.color1", 1)], + "b": [("b.color1", 1), ("b.color0", 0)], + } + + colr = builder.buildCOLR(color_layer_lists) + + assert colr.tableTag == "COLR" + assert colr.version == 0 + assert colr.ColorLayers["a"][0].name == "a.color0" + assert colr.ColorLayers["a"][0].colorID == 0 + assert colr.ColorLayers["a"][1].name == "a.color1" + assert colr.ColorLayers["a"][1].colorID == 1 + assert colr.ColorLayers["b"][0].name == "b.color1" + assert colr.ColorLayers["b"][0].colorID == 1 + assert colr.ColorLayers["b"][1].name == "b.color0" + assert colr.ColorLayers["b"][1].colorID == 0 + + +def test_buildCPAL_v0(): + palettes = [ + [(0.68, 0.20, 0.32, 1.0), (0.45, 0.68, 0.21, 1.0)], + [(0.68, 0.20, 0.32, 0.6), (0.45, 0.68, 0.21, 0.6)], + [(0.68, 0.20, 0.32, 0.3), (0.45, 0.68, 0.21, 0.3)], + ] + + cpal = builder.buildCPAL(palettes) + + assert cpal.tableTag == "CPAL" + assert cpal.version == 0 + assert cpal.numPaletteEntries == 2 + + assert len(cpal.palettes) == 3 + assert [tuple(c) for c in cpal.palettes[0]] == [ + (82, 51, 173, 255), + (54, 173, 115, 255), + ] + assert [tuple(c) for c in cpal.palettes[1]] == [ + (82, 51, 173, 153), + (54, 173, 115, 153), + ] + assert [tuple(c) for c in cpal.palettes[2]] == [ + (82, 51, 173, 76), + (54, 173, 115, 76), + ] + + +def test_buildCPAL_palettes_different_lengths(): + with pytest.raises(ColorLibError, match="have different lengths"): + builder.buildCPAL([[(1, 1, 1, 1)], [(0, 0, 0, 1), (0.5, 0.5, 0.5, 1)]]) From 7a2f68a317781e8d1ed7873c301042c2432cd92b Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 17 Feb 2020 12:11:32 +0000 Subject: [PATCH 24/44] colorLib: add type annotations --- Lib/fontTools/colorLib/builder.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/Lib/fontTools/colorLib/builder.py b/Lib/fontTools/colorLib/builder.py index 7141120e7..59a650aed 100644 --- a/Lib/fontTools/colorLib/builder.py +++ b/Lib/fontTools/colorLib/builder.py @@ -1,46 +1,45 @@ -from fontTools.ttLib import newTable +from typing import Dict, List, Tuple +from fontTools.ttLib.tables.C_O_L_R_ import LayerRecord, table_C_O_L_R_ +from fontTools.ttLib.tables.C_P_A_L_ import Color, table_C_P_A_L_ from .errors import ColorLibError -def buildCOLR(colorLayers): +def buildCOLR(colorLayers: Dict[str, List[Tuple[str, int]]]) -> table_C_O_L_R_: """Build COLR table from color layers mapping. Args: - colorLayers: Dict[str, List[Tuple[str, int]]]: map of base glyph names (str) - to lists of layer glyph names (str) and palette indices (int) tuples. + colorLayers: : map of base glyph names to lists of (layer glyph names, + palette indices) tuples. Return: A new COLRv0 table. """ - from fontTools.ttLib.tables.C_O_L_R_ import LayerRecord - colorLayerLists = {} for baseGlyphName, layers in colorLayers.items(): colorLayerLists[baseGlyphName] = [ LayerRecord(layerGlyphName, colorID) for layerGlyphName, colorID in layers ] - colr = newTable("COLR") + colr = table_C_O_L_R_() colr.version = 0 colr.ColorLayers = colorLayerLists return colr -def buildCPAL(palettes): +def buildCPAL( + palettes: List[List[Tuple[float, float, float, float]]] +) -> table_C_P_A_L_: """Build CPAL table from list of color palettes. Args: - palettes: List[List[Tuple[float, float, float, float]]]: list of lists - colors encoded as tuples of (R, G, B, A) floats. + palettes: : list of lists of colors encoded as tuples of (R, G, B, A) floats. Return: A new CPALv0 table. """ - from fontTools.ttLib.tables.C_P_A_L_ import Color - if len({len(p) for p in palettes}) != 1: raise ColorLibError("color palettes have different lengths") - cpal = newTable("CPAL") + cpal = table_C_P_A_L_() # TODO(anthotype): Support version 1 with palette types, labels and entry labels. cpal.version = 0 cpal.numPaletteEntries = len(palettes[0]) From f60bcc2c5a7f404fa5592afd672d89c577509c1f Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 17 Feb 2020 17:02:58 +0000 Subject: [PATCH 25/44] [CPAL] the absence of a color palette label nameID is 0xFFFF, not 0 --- Lib/fontTools/ttLib/tables/C_P_A_L_.py | 44 ++++++++++++++------------ Tests/ttLib/tables/C_P_A_L_test.py | 17 +++------- 2 files changed, 29 insertions(+), 32 deletions(-) diff --git a/Lib/fontTools/ttLib/tables/C_P_A_L_.py b/Lib/fontTools/ttLib/tables/C_P_A_L_.py index f0415600c..a7b4ad2bb 100644 --- a/Lib/fontTools/ttLib/tables/C_P_A_L_.py +++ b/Lib/fontTools/ttLib/tables/C_P_A_L_.py @@ -13,6 +13,9 @@ import sys class table_C_P_A_L_(DefaultTable.DefaultTable): + NO_NAME_ID = 0xFFFF + DEFAULT_PALETTE_TYPE = 0 + def __init__(self, tag=None): DefaultTable.DefaultTable.__init__(self, tag) self.palettes = [] @@ -45,24 +48,25 @@ class table_C_P_A_L_(DefaultTable.DefaultTable): offsetToPaletteEntryLabelArray) = ( struct.unpack(">LLL", data[pos:pos+12])) self.paletteTypes = self._decompileUInt32Array( - data, offsetToPaletteTypeArray, numPalettes) + data, offsetToPaletteTypeArray, numPalettes, + default=self.DEFAULT_PALETTE_TYPE) self.paletteLabels = self._decompileUInt16Array( - data, offsetToPaletteLabelArray, numPalettes) + data, offsetToPaletteLabelArray, numPalettes, default=self.NO_NAME_ID) self.paletteEntryLabels = self._decompileUInt16Array( data, offsetToPaletteEntryLabelArray, - self.numPaletteEntries) + self.numPaletteEntries, default=self.NO_NAME_ID) - def _decompileUInt16Array(self, data, offset, numElements): + def _decompileUInt16Array(self, data, offset, numElements, default=0): if offset == 0: - return [0] * numElements + return [default] * numElements result = array.array("H", data[offset : offset + 2 * numElements]) if sys.byteorder != "big": result.byteswap() assert len(result) == numElements, result return result.tolist() - def _decompileUInt32Array(self, data, offset, numElements): + def _decompileUInt32Array(self, data, offset, numElements, default=0): if offset == 0: - return [0] * numElements + return [default] * numElements result = array.array("I", data[offset : offset + 4 * numElements]) if sys.byteorder != "big": result.byteswap() assert len(result) == numElements, result @@ -136,7 +140,7 @@ class table_C_P_A_L_(DefaultTable.DefaultTable): return result def _compilePaletteLabels(self): - if self.version == 0 or not any(self.paletteLabels): + if self.version == 0 or all(l == self.NO_NAME_ID for l in self.paletteLabels): return b'' assert len(self.paletteLabels) == len(self.palettes) result = bytesjoin([struct.pack(">H", label) @@ -145,7 +149,7 @@ class table_C_P_A_L_(DefaultTable.DefaultTable): return result def _compilePaletteEntryLabels(self): - if self.version == 0 or not any(self.paletteEntryLabels): + if self.version == 0 or all(l == self.NO_NAME_ID for l in self.paletteEntryLabels): return b'' assert len(self.paletteEntryLabels) == self.numPaletteEntries result = bytesjoin([struct.pack(">H", label) @@ -165,15 +169,15 @@ class table_C_P_A_L_(DefaultTable.DefaultTable): writer.newline() for index, palette in enumerate(self.palettes): attrs = {"index": index} - paletteType = paletteTypes.get(index) - paletteLabel = paletteLabels.get(index) - if self.version > 0 and paletteLabel is not None: + paletteType = paletteTypes.get(index, self.DEFAULT_PALETTE_TYPE) + paletteLabel = paletteLabels.get(index, self.NO_NAME_ID) + if self.version > 0 and paletteLabel != self.NO_NAME_ID: attrs["label"] = paletteLabel - if self.version > 0 and paletteType is not None: + if self.version > 0 and paletteType != self.DEFAULT_PALETTE_TYPE: attrs["type"] = paletteType writer.begintag("palette", **attrs) writer.newline() - if (self.version > 0 and paletteLabel and + if (self.version > 0 and paletteLabel != self.NO_NAME_ID and ttFont and "name" in ttFont): name = ttFont["name"].getDebugName(paletteLabel) if name is not None: @@ -184,11 +188,11 @@ class table_C_P_A_L_(DefaultTable.DefaultTable): color.toXML(writer, ttFont, cindex) writer.endtag("palette") writer.newline() - if self.version > 0 and any(self.paletteEntryLabels): + if self.version > 0 and not all(l == self.NO_NAME_ID for l in self.paletteEntryLabels): writer.begintag("paletteEntryLabels") writer.newline() for index, label in enumerate(self.paletteEntryLabels): - if label: + if label != self.NO_NAME_ID: writer.simpletag("label", index=index, value=label) if (self.version > 0 and label and ttFont and "name" in ttFont): name = ttFont["name"].getDebugName(label) @@ -200,8 +204,8 @@ class table_C_P_A_L_(DefaultTable.DefaultTable): def fromXML(self, name, attrs, content, ttFont): if name == "palette": - self.paletteLabels.append(int(attrs.get("label", "0"))) - self.paletteTypes.append(int(attrs.get("type", "0"))) + self.paletteLabels.append(int(attrs.get("label", self.NO_NAME_ID))) + self.paletteTypes.append(int(attrs.get("type", self.DEFAULT_PALETTE_TYPE))) palette = [] for element in content: if isinstance(element, basestring): @@ -221,13 +225,13 @@ class table_C_P_A_L_(DefaultTable.DefaultTable): nameID = safeEval(elementAttr["value"]) colorLabels[labelIndex] = nameID self.paletteEntryLabels = [ - colorLabels.get(i, 0) + colorLabels.get(i, self.NO_NAME_ID) for i in range(self.numPaletteEntries)] elif "value" in attrs: value = safeEval(attrs["value"]) setattr(self, name, value) if name == "numPaletteEntries": - self.paletteEntryLabels = [0] * self.numPaletteEntries + self.paletteEntryLabels = [self.NO_NAME_ID] * self.numPaletteEntries class Color(namedtuple("Color", "blue green red alpha")): diff --git a/Tests/ttLib/tables/C_P_A_L_test.py b/Tests/ttLib/tables/C_P_A_L_test.py index 68009874c..b018a5247 100644 --- a/Tests/ttLib/tables/C_P_A_L_test.py +++ b/Tests/ttLib/tables/C_P_A_L_test.py @@ -66,9 +66,6 @@ class CPALTest(unittest.TestCase): self.assertEqual(cpal.numPaletteEntries, 2) self.assertEqual(repr(cpal.palettes), '[[#000000FF, #66CCFFFF], [#000000FF, #800000FF]]') - self.assertEqual(cpal.paletteLabels, [0, 0]) - self.assertEqual(cpal.paletteTypes, [0, 0]) - self.assertEqual(cpal.paletteEntryLabels, [0, 0]) def test_decompile_v0_sharingColors(self): cpal = newTable('CPAL') @@ -80,9 +77,6 @@ class CPALTest(unittest.TestCase): '[#223344FF, #99887711, #55555555]', '[#223344FF, #99887711, #FFFFFFFF]', '[#223344FF, #99887711, #55555555]']) - self.assertEqual(cpal.paletteLabels, [0, 0, 0, 0]) - self.assertEqual(cpal.paletteTypes, [0, 0, 0, 0]) - self.assertEqual(cpal.paletteEntryLabels, [0, 0, 0]) def test_decompile_v1_noLabelsNoTypes(self): cpal = newTable('CPAL') @@ -92,9 +86,10 @@ class CPALTest(unittest.TestCase): self.assertEqual([repr(p) for p in cpal.palettes], [ '[#CAFECAFE, #22110033, #66554477]', # RGBA '[#59413127, #42424242, #13330037]']) - self.assertEqual(cpal.paletteLabels, [0, 0]) + self.assertEqual(cpal.paletteLabels, [cpal.NO_NAME_ID] * len(cpal.palettes)) self.assertEqual(cpal.paletteTypes, [0, 0]) - self.assertEqual(cpal.paletteEntryLabels, [0, 0, 0]) + self.assertEqual(cpal.paletteEntryLabels, + [cpal.NO_NAME_ID] * cpal.numPaletteEntries) def test_decompile_v1(self): cpal = newTable('CPAL') @@ -194,9 +189,6 @@ class CPALTest(unittest.TestCase): self.assertEqual(cpal.version, 0) self.assertEqual(cpal.numPaletteEntries, 2) self.assertEqual(repr(cpal.palettes), '[[#12345678, #FEDCBA98]]') - self.assertEqual(cpal.paletteLabels, [0]) - self.assertEqual(cpal.paletteTypes, [0]) - self.assertEqual(cpal.paletteEntryLabels, [0, 0]) def test_fromXML_v1(self): cpal = newTable('CPAL') @@ -218,7 +210,8 @@ class CPALTest(unittest.TestCase): '[[#12345678, #FEDCBA98, #CAFECAFE]]') self.assertEqual(cpal.paletteLabels, [259]) self.assertEqual(cpal.paletteTypes, [2]) - self.assertEqual(cpal.paletteEntryLabels, [0, 262, 0]) + self.assertEqual(cpal.paletteEntryLabels, + [cpal.NO_NAME_ID, 262, cpal.NO_NAME_ID]) if __name__ == "__main__": From bb46604ec24ea98c158940c2e513a945b0458d98 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 17 Feb 2020 18:25:45 +0000 Subject: [PATCH 26/44] colorLib: allow to build CPAL version=1 --- Lib/fontTools/colorLib/builder.py | 120 ++++++++++++++++++++++++++---- 1 file changed, 107 insertions(+), 13 deletions(-) diff --git a/Lib/fontTools/colorLib/builder.py b/Lib/fontTools/colorLib/builder.py index 59a650aed..486909e9e 100644 --- a/Lib/fontTools/colorLib/builder.py +++ b/Lib/fontTools/colorLib/builder.py @@ -1,6 +1,8 @@ -from typing import Dict, List, Tuple +import enum +from typing import Dict, Iterable, List, Optional, Tuple, Union from fontTools.ttLib.tables.C_O_L_R_ import LayerRecord, table_C_O_L_R_ from fontTools.ttLib.tables.C_P_A_L_ import Color, table_C_P_A_L_ +from fontTools.ttLib.tables._n_a_m_e import table__n_a_m_e from .errors import ColorLibError @@ -26,28 +28,120 @@ def buildCOLR(colorLayers: Dict[str, List[Tuple[str, int]]]) -> table_C_O_L_R_: return colr +class ColorPaletteType(enum.IntFlag): + USABLE_WITH_LIGHT_BACKGROUND = 0x0001 + USABLE_WITH_DARK_BACKGROUND = 0x0002 + + @classmethod + def _missing_(cls, value): + # enforce reserved bits + if isinstance(value, int) and (value < 0 or value & 0xFFFC != 0): + raise ValueError(f"{value} is not a valid {cls.__name__}") + return super()._missing_(value) + + +# None, 'abc' or {'en': 'abc', 'de': 'xyz'} +_OptionalLocalizedString = Union[None, str, Dict[str, str]] + + +def buildPaletteLabels( + labels: List[_OptionalLocalizedString], nameTable: table__n_a_m_e +) -> List[Optional[int]]: + return [ + nameTable.addMultilingualName(l, mac=False) + if isinstance(l, dict) + else table_C_P_A_L_.NO_NAME_ID + if l is None + else nameTable.addMultilingualName({"en": l}, mac=False) + for l in labels + ] + + def buildCPAL( - palettes: List[List[Tuple[float, float, float, float]]] + palettes: List[List[Tuple[float, float, float, float]]], + paletteTypes: Optional[List[ColorPaletteType]] = None, + paletteLabels: Optional[List[_OptionalLocalizedString]] = None, + paletteEntryLabels: Optional[List[_OptionalLocalizedString]] = None, + nameTable: Optional[table__n_a_m_e] = None, ) -> table_C_P_A_L_: """Build CPAL table from list of color palettes. Args: - palettes: : list of lists of colors encoded as tuples of (R, G, B, A) floats. + palettes: list of lists of colors encoded as tuples of (R, G, B, A) floats + in the range [0..1]. + paletteTypes: optional list of ColorPaletteType, one for each palette. + paletteLabels: optional list of palette labels. Each lable can be either: + None (no label), a string (for for default English labels), or a + localized string (as a dict keyed with BCP47 language codes). + paletteEntryLabels: optional list of palette entry labels, one for each + palette entry (see paletteLabels). + nameTable: optional name table where to store palette and palette entry + labels. Required if either paletteLabels or paletteEntryLabels is set. Return: - A new CPALv0 table. + A new CPAL v0 or v1 table, if custom palette types or labels are specified. """ if len({len(p) for p in palettes}) != 1: raise ColorLibError("color palettes have different lengths") + + if (paletteLabels or paletteEntryLabels) and not nameTable: + raise TypeError( + "nameTable is required if palette or palette entries have labels" + ) + cpal = table_C_P_A_L_() - # TODO(anthotype): Support version 1 with palette types, labels and entry labels. - cpal.version = 0 cpal.numPaletteEntries = len(palettes[0]) - cpal.palettes = [ - [ - Color(*(round(v * 255) for v in (blue, green, red, alpha))) - for red, green, blue, alpha in palette - ] - for palette in palettes - ] + + cpal.palettes = [] + for i, palette in enumerate(palettes): + colors = [] + for j, color in enumerate(palette): + if not isinstance(color, tuple) or len(color) != 4: + raise ColorLibError( + f"In palette[{i}][{j}]: expected (R, G, B, A) tuple, got {color!r}" + ) + if any(v > 1 or v < 0 for v in color): + raise ColorLibError( + f"palette[{i}][{j}] has invalid out-of-range [0..1] color: {color!r}" + ) + # input colors are RGBA, CPAL encodes them as BGRA + red, green, blue, alpha = color + colors.append(Color(*(round(v * 255) for v in (blue, green, red, alpha)))) + cpal.palettes.append(colors) + + if any(v is not None for v in (paletteTypes, paletteLabels, paletteEntryLabels)): + cpal.version = 1 + + if paletteTypes is not None: + if len(paletteTypes) != len(palettes): + raise ColorLibError( + f"Expected {len(palettes)} paletteTypes, got {len(paletteTypes)}" + ) + cpal.paletteTypes = [ColorPaletteType(t).value for t in paletteTypes] + else: + cpal.paletteTypes = [table_C_P_A_L_.DEFAULT_PALETTE_TYPE] * len(palettes) + + if paletteLabels is not None: + if len(paletteLabels) != len(palettes): + raise ColorLibError( + f"Expected {len(palettes)} paletteLabels, got {len(paletteLabels)}" + ) + cpal.paletteLabels = buildPaletteLabels(paletteLabels, nameTable) + else: + cpal.paletteLabels = [table_C_P_A_L_.NO_NAME_ID] * len(palettes) + + if paletteEntryLabels is not None: + if len(paletteEntryLabels) != cpal.numPaletteEntries: + raise ColorLibError( + f"Expected {cpal.numPaletteEntries} paletteEntryLabels, " + f"got {len(paletteEntryLabels)}" + ) + cpal.paletteEntryLabels = buildPaletteLabels(paletteEntryLabels, nameTable) + else: + cpal.paletteEntryLabels = [ + table_C_P_A_L_.NO_NAME_ID + ] * cpal.numPaletteEntries + else: + cpal.version = 0 + return cpal From a0a4901a5ef22d25aab5d81a464fc696f6e9ee8b Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 17 Feb 2020 18:28:24 +0000 Subject: [PATCH 27/44] colorLib_test: add tests for buildCPAL v1 --- Tests/colorLib/builder_test.py | 131 +++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) diff --git a/Tests/colorLib/builder_test.py b/Tests/colorLib/builder_test.py index bd3898a8b..c51718790 100644 --- a/Tests/colorLib/builder_test.py +++ b/Tests/colorLib/builder_test.py @@ -1,3 +1,4 @@ +from fontTools.ttLib import newTable from fontTools.colorLib import builder from fontTools.colorLib.errors import ColorLibError import pytest @@ -54,3 +55,133 @@ def test_buildCPAL_v0(): def test_buildCPAL_palettes_different_lengths(): with pytest.raises(ColorLibError, match="have different lengths"): builder.buildCPAL([[(1, 1, 1, 1)], [(0, 0, 0, 1), (0.5, 0.5, 0.5, 1)]]) + + +def test_buildPaletteLabels(): + name_table = newTable("name") + name_table.names = [] + + name_ids = builder.buildPaletteLabels( + [None, "hi", {"en": "hello", "de": "hallo"}], name_table + ) + + assert name_ids == [0xFFFF, 256, 257] + + assert len(name_table.names) == 3 + assert str(name_table.names[0]) == "hi" + assert name_table.names[0].nameID == 256 + + assert str(name_table.names[1]) == "hallo" + assert name_table.names[1].nameID == 257 + + assert str(name_table.names[2]) == "hello" + assert name_table.names[2].nameID == 257 + + +def test_build_CPAL_v1_types_no_labels(): + palettes = [ + [(0.1, 0.2, 0.3, 1.0), (0.4, 0.5, 0.6, 1.0)], + [(0.1, 0.2, 0.3, 0.6), (0.4, 0.5, 0.6, 0.6)], + [(0.1, 0.2, 0.3, 0.3), (0.4, 0.5, 0.6, 0.3)], + ] + paletteTypes = [ + builder.ColorPaletteType.USABLE_WITH_LIGHT_BACKGROUND, + builder.ColorPaletteType.USABLE_WITH_DARK_BACKGROUND, + builder.ColorPaletteType.USABLE_WITH_LIGHT_BACKGROUND + | builder.ColorPaletteType.USABLE_WITH_DARK_BACKGROUND, + ] + + cpal = builder.buildCPAL(palettes, paletteTypes=paletteTypes) + + assert cpal.tableTag == "CPAL" + assert cpal.version == 1 + assert cpal.numPaletteEntries == 2 + assert len(cpal.palettes) == 3 + + assert cpal.paletteTypes == paletteTypes + assert cpal.paletteLabels == [cpal.NO_NAME_ID] * len(palettes) + assert cpal.paletteEntryLabels == [cpal.NO_NAME_ID] * cpal.numPaletteEntries + + +def test_build_CPAL_v1_labels(): + palettes = [ + [(0.1, 0.2, 0.3, 1.0), (0.4, 0.5, 0.6, 1.0)], + [(0.1, 0.2, 0.3, 0.6), (0.4, 0.5, 0.6, 0.6)], + [(0.1, 0.2, 0.3, 0.3), (0.4, 0.5, 0.6, 0.3)], + ] + paletteLabels = ["First", {"en": "Second", "it": "Seconda"}, None] + paletteEntryLabels = ["Foo", "Bar"] + + with pytest.raises(TypeError, match="nameTable is required"): + builder.buildCPAL(palettes, paletteLabels=paletteLabels) + with pytest.raises(TypeError, match="nameTable is required"): + builder.buildCPAL(palettes, paletteEntryLabels=paletteEntryLabels) + + name_table = newTable("name") + name_table.names = [] + + cpal = builder.buildCPAL( + palettes, + paletteLabels=paletteLabels, + paletteEntryLabels=paletteEntryLabels, + nameTable=name_table, + ) + + assert cpal.tableTag == "CPAL" + assert cpal.version == 1 + assert cpal.numPaletteEntries == 2 + assert len(cpal.palettes) == 3 + + assert cpal.paletteTypes == [cpal.DEFAULT_PALETTE_TYPE] * len(palettes) + assert cpal.paletteLabels == [256, 257, cpal.NO_NAME_ID] + assert cpal.paletteEntryLabels == [258, 259] + + assert name_table.getDebugName(256) == "First" + assert name_table.getDebugName(257) == "Second" + assert name_table.getDebugName(258) == "Foo" + assert name_table.getDebugName(259) == "Bar" + + +def test_invalid_ColorPaletteType(): + with pytest.raises(ValueError, match="not a valid ColorPaletteType"): + builder.ColorPaletteType(-1) + with pytest.raises(ValueError, match="not a valid ColorPaletteType"): + builder.ColorPaletteType(4) + with pytest.raises(ValueError, match="not a valid ColorPaletteType"): + builder.ColorPaletteType("abc") + + +def test_buildCPAL_v1_invalid_args_length(): + with pytest.raises(ColorLibError, match="Expected 2 paletteTypes, got 1"): + builder.buildCPAL([[(0, 0, 0, 0)], [(1, 1, 1, 1)]], paletteTypes=[1]) + + with pytest.raises(ColorLibError, match="Expected 2 paletteLabels, got 1"): + builder.buildCPAL( + [[(0, 0, 0, 0)], [(1, 1, 1, 1)]], + paletteLabels=["foo"], + nameTable=newTable("name"), + ) + + with pytest.raises(ColorLibError, match="Expected 1 paletteEntryLabels, got 0"): + cpal = builder.buildCPAL( + [[(0, 0, 0, 0)], [(1, 1, 1, 1)]], + paletteEntryLabels=[], + nameTable=newTable("name"), + ) + + +def test_buildCPAL_invalid_color(): + with pytest.raises( + ColorLibError, + match=r"In palette\[0\]\[1\]: expected \(R, G, B, A\) tuple, got \(1, 1, 1\)", + ): + builder.buildCPAL([[(1, 1, 1, 1), (1, 1, 1)]]) + + with pytest.raises( + ColorLibError, + match=( + r"palette\[1\]\[0\] has invalid out-of-range " + r"\[0..1\] color: \(1, 1, -1, 2\)" + ), + ): + builder.buildCPAL([[(0, 0, 0, 0)], [(1, 1, -1, 2)]]) From d9250ddcf59f223f9f9fb239f938cc16be5ffdfd Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 17 Feb 2020 18:35:15 +0000 Subject: [PATCH 28/44] fontBuilder: allow to build v1 from setupCPAL method --- Lib/fontTools/fontBuilder.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/Lib/fontTools/fontBuilder.py b/Lib/fontTools/fontBuilder.py index f46db6f87..a08442abf 100644 --- a/Lib/fontTools/fontBuilder.py +++ b/Lib/fontTools/fontBuilder.py @@ -777,14 +777,29 @@ class FontBuilder(object): self.font["COLR"] = buildCOLR(colorLayers) - def setupCPAL(self, palettes): + def setupCPAL( + self, + palettes, + paletteTypes=None, + paletteLabels=None, + paletteEntryLabels=None, + ): """Build new CPAL table using list of palettes. + Optionally build CPAL v1 table using paletteTypes, paletteLabels and + paletteEntryLabels. + Cf. `fontTools.colorLib.builder.buildCPAL`. """ from fontTools.colorLib.builder import buildCPAL - self.font["CPAL"] = buildCPAL(palettes) + self.font["CPAL"] = buildCPAL( + palettes, + paletteTypes=paletteTypes, + paletteLabels=paletteLabels, + paletteEntryLabels=paletteEntryLabels, + nameTable=self.font.get("name") + ) def buildCmapSubTable(cmapping, format, platformID, platEncID): From 76d125b3e276730e9d8068ce77bd1a432eace603 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 18 Feb 2020 15:24:14 +0000 Subject: [PATCH 29/44] Update changelog [skip ci] --- NEWS.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 98f16600a..fed0b537b 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -1,3 +1,13 @@ +- [colorLib] Added ``fontTools.colorLib.builder`` module, initially with ``buildCOLR`` + and ``buildCPAL`` public functions. More color font formats will follow (#1827). +- [fontBuilder] Added ``setupCOLR`` and ``setupCPAL`` methods (#1826). +- [ttGlyphPen] Quantize ``GlyphComponent.transform`` floats to ``F2Dot14`` to fix + round-trip issue when computing bounding boxes of transformed components (#1830). +- [glyf] If a component uses reference points (``firstPt`` and ``secondPt``) for + alignment (instead of X and Y offsets), compute the effective translation offset + *after* having applied any transform (#1831). +- [glyf] When all glyphs have zero contours, compile ``glyf`` table data as a single + null byte in order to pass validation by OTS and Windows (#1829). - [feaLib] Parsing feature code now ensures that referenced glyph names are part of the known glyph set, unless a glyph set was not provided. - [varLib] When filling in the default axis value for a missing location of a source or From 996f2ac1a500cebff82d7fc8a656f62cba6cc940 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 18 Feb 2020 15:25:11 +0000 Subject: [PATCH 30/44] Release 4.4.0 --- Lib/fontTools/__init__.py | 2 +- NEWS.rst | 3 +++ setup.cfg | 2 +- setup.py | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Lib/fontTools/__init__.py b/Lib/fontTools/__init__.py index 29f0b545f..8b444c69f 100644 --- a/Lib/fontTools/__init__.py +++ b/Lib/fontTools/__init__.py @@ -4,6 +4,6 @@ from fontTools.misc.loggingTools import configLogger log = logging.getLogger(__name__) -version = __version__ = "4.3.1.dev0" +version = __version__ = "4.4.0" __all__ = ["version", "log", "configLogger"] diff --git a/NEWS.rst b/NEWS.rst index fed0b537b..6cca0800e 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -1,3 +1,6 @@ +4.4.0 (released 2020-02-18) +--------------------------- + - [colorLib] Added ``fontTools.colorLib.builder`` module, initially with ``buildCOLR`` and ``buildCPAL`` public functions. More color font formats will follow (#1827). - [fontBuilder] Added ``setupCOLR`` and ``setupCPAL`` methods (#1826). diff --git a/setup.cfg b/setup.cfg index a33dfd0dd..6e71bc02d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 4.3.1.dev0 +current_version = 4.4.0 commit = True tag = False tag_name = {new_version} diff --git a/setup.py b/setup.py index 8c7adfba1..2163b93f3 100755 --- a/setup.py +++ b/setup.py @@ -345,7 +345,7 @@ def find_data_files(manpath="share/man"): setup( name="fonttools", - version="4.3.1.dev0", + version="4.4.0", description="Tools to manipulate font files", author="Just van Rossum", author_email="just@letterror.com", From ee213a28f7b42f1379ed9781da6b20926f9a6076 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 18 Feb 2020 15:25:12 +0000 Subject: [PATCH 31/44] =?UTF-8?q?Bump=20version:=204.4.0=20=E2=86=92=204.4?= =?UTF-8?q?.1.dev0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Lib/fontTools/__init__.py | 2 +- setup.cfg | 2 +- setup.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/fontTools/__init__.py b/Lib/fontTools/__init__.py index 8b444c69f..089ac7b58 100644 --- a/Lib/fontTools/__init__.py +++ b/Lib/fontTools/__init__.py @@ -4,6 +4,6 @@ from fontTools.misc.loggingTools import configLogger log = logging.getLogger(__name__) -version = __version__ = "4.4.0" +version = __version__ = "4.4.1.dev0" __all__ = ["version", "log", "configLogger"] diff --git a/setup.cfg b/setup.cfg index 6e71bc02d..63f7a21bd 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 4.4.0 +current_version = 4.4.1.dev0 commit = True tag = False tag_name = {new_version} diff --git a/setup.py b/setup.py index 2163b93f3..6f5f558aa 100755 --- a/setup.py +++ b/setup.py @@ -345,7 +345,7 @@ def find_data_files(manpath="share/man"): setup( name="fonttools", - version="4.4.0", + version="4.4.1.dev0", description="Tools to manipulate font files", author="Just van Rossum", author_email="just@letterror.com", From 323d0c85d183f410c06f9d096b0d79ee6ae95dc8 Mon Sep 17 00:00:00 2001 From: Khaled Hosny Date: Sat, 22 Feb 2020 13:51:45 +0200 Subject: [PATCH 32/44] fontBuilder: Allow varLib to use fresh CFF table Make sure the CFF table generated by fontBuilder can be used by varLib without having to compile and decompile the table first. This was breaking in converting the CFF table to CFF2 due to some unset attributes. --- Lib/fontTools/fontBuilder.py | 2 ++ Tests/fontBuilder/fontBuilder_test.py | 13 +++++++++++++ 2 files changed, 15 insertions(+) diff --git a/Lib/fontTools/fontBuilder.py b/Lib/fontTools/fontBuilder.py index a08442abf..d4b940512 100644 --- a/Lib/fontTools/fontBuilder.py +++ b/Lib/fontTools/fontBuilder.py @@ -506,6 +506,7 @@ class FontBuilder(object): fontSet = CFFFontSet() fontSet.major = 1 fontSet.minor = 0 + fontSet.otFont = self.font fontSet.fontNames = [psName] fontSet.topDictIndex = TopDictIndex() @@ -520,6 +521,7 @@ class FontBuilder(object): topDict = TopDict() topDict.charset = self.font.getGlyphOrder() topDict.Private = private + topDict.GlobalSubrs = fontSet.GlobalSubrs for key, value in fontInfo.items(): setattr(topDict, key, value) if "FontMatrix" not in fontInfo: diff --git a/Tests/fontBuilder/fontBuilder_test.py b/Tests/fontBuilder/fontBuilder_test.py index 8c5127626..bc7837c30 100644 --- a/Tests/fontBuilder/fontBuilder_test.py +++ b/Tests/fontBuilder/fontBuilder_test.py @@ -255,6 +255,19 @@ def test_build_cff2(tmpdir): _verifyOutput(outPath) +def test_build_cff_to_cff2(tmpdir): + fb, _, _ = _setupFontBuilder(False, 1000) + + pen = T2CharStringPen(600, None) + drawTestGlyph(pen) + charString = pen.getCharString() + charStrings = {".notdef": charString, "A": charString, "a": charString, ".null": charString} + fb.setupCFF("TestFont", {}, charStrings, {}) + + from fontTools.varLib.cff import convertCFFtoCFF2 + convertCFFtoCFF2(fb.font) + + def test_setupNameTable_no_mac(): fb, _, nameStrings = _setupFontBuilder(True) fb.setupNameTable(nameStrings, mac=False) From 1a6cb48ea095241650821ddeabd7af6ae5a1327d Mon Sep 17 00:00:00 2001 From: Christof Kaufmann Date: Tue, 25 Feb 2020 11:59:31 +0100 Subject: [PATCH 33/44] Use non-localized date parsing to fix #1838 --- Lib/fontTools/misc/timeTools.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Lib/fontTools/misc/timeTools.py b/Lib/fontTools/misc/timeTools.py index e9c6f197c..13613827b 100644 --- a/Lib/fontTools/misc/timeTools.py +++ b/Lib/fontTools/misc/timeTools.py @@ -4,6 +4,7 @@ from fontTools.misc.py23 import * import os import time +from datetime import datetime, timezone import calendar @@ -44,7 +45,12 @@ def timestampToString(value): return asctime(time.gmtime(max(0, value + epoch_diff))) def timestampFromString(value): - return calendar.timegm(time.strptime(value)) - epoch_diff + wkday, mnth = value[:7].split() + t = datetime.strptime(value[7:], ' %d %H:%M:%S %Y') + t = t.replace(month=MONTHNAMES.index(mnth), tzinfo=timezone.utc) + wkday_idx = DAYNAMES.index(wkday) + assert t.weekday() == wkday_idx, '"' + value + '" has inconsistent weekday' + return int(t.timestamp()) - epoch_diff def timestampNow(): # https://reproducible-builds.org/specs/source-date-epoch/ From edbfe95f45974c206fb247a178eaf883e3b22e57 Mon Sep 17 00:00:00 2001 From: Christof Kaufmann Date: Tue, 25 Feb 2020 20:36:02 +0100 Subject: [PATCH 34/44] Add test for non-localized date parsing --- .travis.yml | 5 +++++ Tests/misc/timeTools_test.py | 17 ++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 20d45368b..cf0f424fa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -52,6 +52,11 @@ cache: - directories: - $HOME/.pyenv_cache +addons: + apt: + packages: + - language-pack-de + before_install: - source ./.travis/before_install.sh diff --git a/Tests/misc/timeTools_test.py b/Tests/misc/timeTools_test.py index e3ad3f6d2..601f357b8 100644 --- a/Tests/misc/timeTools_test.py +++ b/Tests/misc/timeTools_test.py @@ -1,7 +1,8 @@ from fontTools.misc.py23 import * -from fontTools.misc.timeTools import asctime, timestampNow, epoch_diff +from fontTools.misc.timeTools import asctime, timestampNow, timestampToString, timestampFromString, epoch_diff import os import time +import locale import pytest @@ -21,3 +22,17 @@ def test_source_date_epoch(): del os.environ["SOURCE_DATE_EPOCH"] assert timestampNow() + epoch_diff != 150687315 + + +# test for issue #1838 +def test_date_parsing_with_locale(): + l = locale.getlocale(locale.LC_TIME) + try: + locale.setlocale(locale.LC_TIME, 'de_DE.utf8') + except locale.Error: + pytest.skip("Locale de_DE not available") + + try: + assert timestampFromString(timestampToString(timestampNow())) + finally: + locale.setlocale(locale.LC_TIME, l) From be147e965d2036d42404549f4f962bd7dfd3aa01 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 26 Feb 2020 18:14:31 +0000 Subject: [PATCH 35/44] woff2: don't attempt to normalize glyf/loca if missing (e.g. CBDT/CBLC fonts like NotoColorEmoji.ttf) --- Lib/fontTools/ttLib/woff2.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Lib/fontTools/ttLib/woff2.py b/Lib/fontTools/ttLib/woff2.py index 0448546fd..849bf0ff7 100644 --- a/Lib/fontTools/ttLib/woff2.py +++ b/Lib/fontTools/ttLib/woff2.py @@ -226,7 +226,11 @@ class WOFF2Writer(SFNTWriter): # See: # https://github.com/khaledhosny/ots/issues/60 # https://github.com/google/woff2/issues/15 - if isTrueType and "glyf" in self.flavorData.transformedTables: + if ( + isTrueType + and "glyf" in self.flavorData.transformedTables + and "glyf" in self.tables + ): self._normaliseGlyfAndLoca(padding=4) self._setHeadTransformFlag() From 0d53cfbb99aac5e8638fe85baf0e13d237cc04fe Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 26 Feb 2020 18:24:53 +0000 Subject: [PATCH 36/44] woff2_test: add test with CBDT/CBLC font without glyf/loca tables --- Tests/ttLib/woff2_test.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Tests/ttLib/woff2_test.py b/Tests/ttLib/woff2_test.py index 10439bc5a..2651e8085 100644 --- a/Tests/ttLib/woff2_test.py +++ b/Tests/ttLib/woff2_test.py @@ -1199,6 +1199,24 @@ class WOFF2RoundtripTest(object): assert tmp.getvalue() == tmp2.getvalue() assert ttFont2.reader.flavorData.transformedTables == {"hmtx"} + def test_roundtrip_no_glyf_and_loca_tables(self): + ttx = os.path.join( + os.path.dirname(current_dir), "subset", "data", "google_color.ttx" + ) + ttFont = ttLib.TTFont() + ttFont.importXML(ttx) + + assert "glyf" not in ttFont + assert "loca" not in ttFont + + ttFont.flavor = "woff2" + tmp = BytesIO() + ttFont.save(tmp) + + tmp2, ttFont2 = self.roundtrip(tmp) + assert tmp.getvalue() == tmp2.getvalue() + assert ttFont.flavor == "woff2" + class MainTest(object): From f22efa511192e81e87d816f074f5ed034cba900d Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 26 Feb 2020 18:31:55 +0000 Subject: [PATCH 37/44] Update changelog [skip ci] --- NEWS.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 6cca0800e..3cd844771 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -1,3 +1,11 @@ +- [woff2] Skip normalizing ``glyf`` and ``loca`` tables if these are missing from + a font (e.g. in NotoColorEmoji using ``CBDT/CBLC`` tables). +- [timeTools] Use non-localized date parsing in ``timestampFromString``, to fix + error when non-English ``LC_TIME`` locale is set (#1838, #1839). +- [fontBuilder] Make sure the CFF table generated by fontBuilder can be used by varLib + without having to compile and decompile the table first. This was breaking in + converting the CFF table to CFF2 due to some unset attributes (#1836). + 4.4.0 (released 2020-02-18) --------------------------- From 569fa2dbc4141126b7ee1a465dbad93aec1a0868 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 26 Feb 2020 18:32:00 +0000 Subject: [PATCH 38/44] Release 4.4.1 --- Lib/fontTools/__init__.py | 2 +- NEWS.rst | 3 +++ setup.cfg | 2 +- setup.py | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Lib/fontTools/__init__.py b/Lib/fontTools/__init__.py index 089ac7b58..8636e7866 100644 --- a/Lib/fontTools/__init__.py +++ b/Lib/fontTools/__init__.py @@ -4,6 +4,6 @@ from fontTools.misc.loggingTools import configLogger log = logging.getLogger(__name__) -version = __version__ = "4.4.1.dev0" +version = __version__ = "4.4.1" __all__ = ["version", "log", "configLogger"] diff --git a/NEWS.rst b/NEWS.rst index 3cd844771..7fc3e1d4b 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -1,3 +1,6 @@ +4.4.1 (released 2020-02-26) +--------------------------- + - [woff2] Skip normalizing ``glyf`` and ``loca`` tables if these are missing from a font (e.g. in NotoColorEmoji using ``CBDT/CBLC`` tables). - [timeTools] Use non-localized date parsing in ``timestampFromString``, to fix diff --git a/setup.cfg b/setup.cfg index 63f7a21bd..4ae5cab7a 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 4.4.1.dev0 +current_version = 4.4.1 commit = True tag = False tag_name = {new_version} diff --git a/setup.py b/setup.py index 6f5f558aa..469d79805 100755 --- a/setup.py +++ b/setup.py @@ -345,7 +345,7 @@ def find_data_files(manpath="share/man"): setup( name="fonttools", - version="4.4.1.dev0", + version="4.4.1", description="Tools to manipulate font files", author="Just van Rossum", author_email="just@letterror.com", From 19ba958325c359d4756cf25ab08a6b292b936ff6 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 26 Feb 2020 18:32:01 +0000 Subject: [PATCH 39/44] =?UTF-8?q?Bump=20version:=204.4.1=20=E2=86=92=204.4?= =?UTF-8?q?.2.dev0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Lib/fontTools/__init__.py | 2 +- setup.cfg | 2 +- setup.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/fontTools/__init__.py b/Lib/fontTools/__init__.py index 8636e7866..8015a1030 100644 --- a/Lib/fontTools/__init__.py +++ b/Lib/fontTools/__init__.py @@ -4,6 +4,6 @@ from fontTools.misc.loggingTools import configLogger log = logging.getLogger(__name__) -version = __version__ = "4.4.1" +version = __version__ = "4.4.2.dev0" __all__ = ["version", "log", "configLogger"] diff --git a/setup.cfg b/setup.cfg index 4ae5cab7a..606faf03b 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 4.4.1 +current_version = 4.4.2.dev0 commit = True tag = False tag_name = {new_version} diff --git a/setup.py b/setup.py index 469d79805..604f6d7bc 100755 --- a/setup.py +++ b/setup.py @@ -345,7 +345,7 @@ def find_data_files(manpath="share/man"): setup( name="fonttools", - version="4.4.1", + version="4.4.2.dev0", description="Tools to manipulate font files", author="Just van Rossum", author_email="just@letterror.com", From 328d4cdb5313a13a951253664f834522c3f9cc44 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 26 Feb 2020 22:39:40 +0000 Subject: [PATCH 40/44] Update and rename .travis.yml to Update .travis.yml --- .travis.yml => Update .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename .travis.yml => Update .travis.yml (99%) diff --git a/.travis.yml b/Update .travis.yml similarity index 99% rename from .travis.yml rename to Update .travis.yml index cf0f424fa..f35cd351d 100644 --- a/.travis.yml +++ b/Update .travis.yml @@ -1,5 +1,5 @@ language: python -python: 3.5 +python: 3.6 env: global: From 181dc1dbfc58bae7550af4113e038c5e9e844b2d Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 26 Feb 2020 22:41:42 +0000 Subject: [PATCH 41/44] Revert "Update and rename .travis.yml to Update .travis.yml" This reverts commit 328d4cdb5313a13a951253664f834522c3f9cc44. Sorry, I was trying to commit a change from Github mobile UI on my phone and messed up. --- Update .travis.yml => .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename Update .travis.yml => .travis.yml (99%) diff --git a/Update .travis.yml b/.travis.yml similarity index 99% rename from Update .travis.yml rename to .travis.yml index f35cd351d..cf0f424fa 100644 --- a/Update .travis.yml +++ b/.travis.yml @@ -1,5 +1,5 @@ language: python -python: 3.6 +python: 3.5 env: global: From 31f7ce108e1b992eff98fe7de3f9c8dfde0106db Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 26 Feb 2020 22:43:21 +0000 Subject: [PATCH 42/44] .travis.yml: use python3.6 as default python --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index cf0f424fa..f35cd351d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,5 @@ language: python -python: 3.5 +python: 3.6 env: global: From 5025035f9e22030b12da37f293ace3c47c3fbaf7 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 26 Feb 2020 22:53:13 +0000 Subject: [PATCH 43/44] .travis.yml: try to remove 'exclude: python3.6' --- .travis.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index f35cd351d..5f98e3b7f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,9 +16,6 @@ branches: matrix: fast_finish: true - exclude: - # Exclude the default Python 3.6 build - - python: 3.6 include: - python: 3.6 env: From 36c64087a8757f12c7b5233a29c9df23bcc02138 Mon Sep 17 00:00:00 2001 From: Nikolaus Waxweiler Date: Thu, 27 Feb 2020 18:13:45 +0000 Subject: [PATCH 44/44] Add a proper message to IncludedFeaNotFound Also, minor f-string refactor. --- Lib/fontTools/feaLib/error.py | 12 ++++++++++-- Tests/feaLib/lexer_test.py | 4 +++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Lib/fontTools/feaLib/error.py b/Lib/fontTools/feaLib/error.py index 82a7dec5a..66c6c6eae 100644 --- a/Lib/fontTools/feaLib/error.py +++ b/Lib/fontTools/feaLib/error.py @@ -9,10 +9,18 @@ class FeatureLibError(Exception): message = Exception.__str__(self) if self.location: path, line, column = self.location - return "%s:%d:%d: %s" % (path, line, column, message) + return f"{path}:{line}:{column}: {message}" else: return message class IncludedFeaNotFound(FeatureLibError): - pass + def __str__(self): + assert self.location is not None + + message = ( + "The following feature file should be included but cannot be found: " + f"{Exception.__str__(self)}" + ) + path, line, column = self.location + return f"{path}:{line}:{column}: {message}" diff --git a/Tests/feaLib/lexer_test.py b/Tests/feaLib/lexer_test.py index fa3f0ea57..3837801f5 100644 --- a/Tests/feaLib/lexer_test.py +++ b/Tests/feaLib/lexer_test.py @@ -178,7 +178,9 @@ class IncludingLexerTest(unittest.TestCase): def test_include_missing_file(self): lexer = IncludingLexer(self.getpath("include/includemissingfile.fea")) self.assertRaisesRegex(IncludedFeaNotFound, - "includemissingfile.fea:1:8: missingfile.fea", + "includemissingfile.fea:1:8: The following feature file " + "should be included but cannot be found: " + "missingfile.fea", lambda: list(lexer)) def test_featurefilepath_None(self):