From a421f9045a4d1e130daece08350ed7c1ed300ee7 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 23 May 2023 12:37:53 +0100 Subject: [PATCH 1/4] fontBuilder: add glyphDataFormat=0; error with accidentally cubic outlines Fixes #3113 --- Lib/fontTools/fontBuilder.py | 40 +++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/Lib/fontTools/fontBuilder.py b/Lib/fontTools/fontBuilder.py index ec1e26459..58ecce43a 100644 --- a/Lib/fontTools/fontBuilder.py +++ b/Lib/fontTools/fontBuilder.py @@ -131,6 +131,7 @@ fb.save("test.otf") from .ttLib import TTFont, newTable from .ttLib.tables._c_m_a_p import cmap_classes +from .ttLib.tables._g_l_y_f import flagCubic from .misc.timeTools import timestampNow import struct from collections import OrderedDict @@ -319,7 +320,7 @@ _OS2Defaults = dict( class FontBuilder(object): - def __init__(self, unitsPerEm=None, font=None, isTTF=True): + def __init__(self, unitsPerEm=None, font=None, isTTF=True, glyphDataFormat=0): """Initialize a FontBuilder instance. If the `font` argument is not given, a new `TTFont` will be @@ -327,15 +328,31 @@ class FontBuilder(object): the font will be a glyf-based TTF; if `isTTF` is False it will be a CFF-based OTF. + The `glyphDataFormat` argument corresponds to the `head` table field + that defines the format of the TrueType `glyf` table (default=0). + TrueType glyphs historically can only contain quadratic splines and static + components, but there's a proposal to add support for cubic Bezier curves as well + as variable composites/components at + https://github.com/harfbuzz/boring-expansion-spec/blob/main/glyf1.md + You can experiment with the new features by setting `glyphDataFormat` to 1. + A ValueError is raised if `glyphDataFormat` is left at 0 but glyphs are added + that contain cubic splines or varcomposites. This is to prevent accidentally + creating fonts that are incompatible with existing TrueType implementations. + If `font` is given, it must be a `TTFont` instance and `unitsPerEm` - must _not_ be given. The `isTTF` argument will be ignored. + must _not_ be given. The `isTTF` and `glyphDataFormat` arguments will be ignored. """ if font is None: self.font = TTFont(recalcTimestamp=False) self.isTTF = isTTF now = timestampNow() assert unitsPerEm is not None - self.setupHead(unitsPerEm=unitsPerEm, created=now, modified=now) + self.setupHead( + unitsPerEm=unitsPerEm, + create=now, + modified=now, + glyphDataFormat=glyphDataFormat, + ) self.setupMaxp() else: assert unitsPerEm is None @@ -639,8 +656,25 @@ class FontBuilder(object): If `calcGlyphBounds` is True, the bounds of all glyphs will be calculated. Only pass False if your glyph objects already have their bounding box values set. + + Raises ValueError if any of the glyphs contains cubic curves or is a variable + composite but the `head` table has `glyphDataFormat` set to 0 (instead of 1). """ assert self.isTTF + + glyphDataFormat = self.font["head"].glyphDataFormat + if glyphDataFormat == 0: + for name, g in glyphs.items(): + if g.isVarComposite(): + raise ValueError( + f"Glyph {name!r} is a variable composite, but glyphDataFormat=0" + ) + elif g.numberOfContours > 0 and any(f & flagCubic for f in g.flags): + raise ValueError( + f"Glyph {name!r} has cubic Bezier outlines, but glyphDataFormat=0; " + "either convert to quadratics with cu2qu or set glyphDataFormat=1." + ) + self.font["loca"] = newTable("loca") self.font["glyf"] = newTable("glyf") self.font["glyf"].glyphs = glyphs From 9bb3b652b0ab737f40ec1a124b6fd248ac6fbe9e Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 23 May 2023 12:55:59 +0100 Subject: [PATCH 2/4] fontBuilder_test: test error with cubics but glyf format 0 --- Tests/fontBuilder/fontBuilder_test.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Tests/fontBuilder/fontBuilder_test.py b/Tests/fontBuilder/fontBuilder_test.py index 169e90fbc..589bc676b 100644 --- a/Tests/fontBuilder/fontBuilder_test.py +++ b/Tests/fontBuilder/fontBuilder_test.py @@ -137,6 +137,25 @@ def test_build_ttf(tmpdir): _verifyOutput(outPath) +def test_build_cubic_ttf(tmp_path): + pen = TTGlyphPen(None) + pen.moveTo((100, 100)) + pen.curveTo((200, 200), (300, 300), (400, 400)) + pen.closePath() + glyph = pen.glyph() + glyphs = {"A": glyph} + + fb = FontBuilder(1000, isTTF=True, glyphDataFormat=0) + with pytest.raises( + ValueError, match="Glyph 'A' has cubic Bezier outlines, but glyphDataFormat=0" + ): + fb.setupGlyf(glyphs) + + fb = FontBuilder(1000, isTTF=True, glyphDataFormat=1) + fb.setupGlyf(glyphs) + assert "A" in fb.font["glyf"].glyphs + + def test_build_otf(tmpdir): outPath = os.path.join(str(tmpdir), "test.otf") From 469c9ad963294a3acc2f48f16171dfdb131d0af1 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 23 May 2023 13:16:02 +0100 Subject: [PATCH 3/4] add option to skip glyphDataFormat check for speed If one is certain that the glyph data has compatible format and prefers not to wait for each glyph flag to checked... --- Lib/fontTools/fontBuilder.py | 9 ++++----- Tests/fontBuilder/fontBuilder_test.py | 4 ++++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Lib/fontTools/fontBuilder.py b/Lib/fontTools/fontBuilder.py index 58ecce43a..8bbcc6ac5 100644 --- a/Lib/fontTools/fontBuilder.py +++ b/Lib/fontTools/fontBuilder.py @@ -648,7 +648,7 @@ class FontBuilder(object): for fontDict in topDict.FDArray: fontDict.Private.vstore = vstore - def setupGlyf(self, glyphs, calcGlyphBounds=True): + def setupGlyf(self, glyphs, calcGlyphBounds=True, checkFormat=True): """Create the `glyf` table from a dict, that maps glyph names to `fontTools.ttLib.tables._g_l_y_f.Glyph` objects, for example as made by `fontTools.pens.ttGlyphPen.TTGlyphPen`. @@ -657,13 +657,12 @@ class FontBuilder(object): calculated. Only pass False if your glyph objects already have their bounding box values set. - Raises ValueError if any of the glyphs contains cubic curves or is a variable - composite but the `head` table has `glyphDataFormat` set to 0 (instead of 1). + If `checkFormat` is True, raise ValueError if any of the glyphs contains + cubic curves or is a variable composite but head.glyphDataFormat=0. """ assert self.isTTF - glyphDataFormat = self.font["head"].glyphDataFormat - if glyphDataFormat == 0: + if checkFormat and self.font["head"].glyphDataFormat == 0: for name, g in glyphs.items(): if g.isVarComposite(): raise ValueError( diff --git a/Tests/fontBuilder/fontBuilder_test.py b/Tests/fontBuilder/fontBuilder_test.py index 589bc676b..6131a972c 100644 --- a/Tests/fontBuilder/fontBuilder_test.py +++ b/Tests/fontBuilder/fontBuilder_test.py @@ -145,12 +145,16 @@ def test_build_cubic_ttf(tmp_path): glyph = pen.glyph() glyphs = {"A": glyph} + # cubic outlines are not allowed in glyf table format 0 fb = FontBuilder(1000, isTTF=True, glyphDataFormat=0) with pytest.raises( ValueError, match="Glyph 'A' has cubic Bezier outlines, but glyphDataFormat=0" ): fb.setupGlyf(glyphs) + # can skip check if feeling adventurous + fb.setupGlyf(glyphs, checkFormat=False) + # cubics are (will be) allowed in glyf table format 1 fb = FontBuilder(1000, isTTF=True, glyphDataFormat=1) fb.setupGlyf(glyphs) assert "A" in fb.font["glyf"].glyphs From 57d5da2039f0b5624771759c0d149b9c44383a4d Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 23 May 2023 14:41:16 +0100 Subject: [PATCH 4/4] rename parameter validateGlyphFormat and reword docstring as per review --- Lib/fontTools/fontBuilder.py | 8 +++++--- Tests/fontBuilder/fontBuilder_test.py | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Lib/fontTools/fontBuilder.py b/Lib/fontTools/fontBuilder.py index 8bbcc6ac5..6a76740fd 100644 --- a/Lib/fontTools/fontBuilder.py +++ b/Lib/fontTools/fontBuilder.py @@ -648,7 +648,7 @@ class FontBuilder(object): for fontDict in topDict.FDArray: fontDict.Private.vstore = vstore - def setupGlyf(self, glyphs, calcGlyphBounds=True, checkFormat=True): + def setupGlyf(self, glyphs, calcGlyphBounds=True, validateGlyphFormat=True): """Create the `glyf` table from a dict, that maps glyph names to `fontTools.ttLib.tables._g_l_y_f.Glyph` objects, for example as made by `fontTools.pens.ttGlyphPen.TTGlyphPen`. @@ -657,12 +657,14 @@ class FontBuilder(object): calculated. Only pass False if your glyph objects already have their bounding box values set. - If `checkFormat` is True, raise ValueError if any of the glyphs contains + If `validateGlyphFormat` is True, raise ValueError if any of the glyphs contains cubic curves or is a variable composite but head.glyphDataFormat=0. + Set it to False to skip the check if you know in advance all the glyphs are + compatible with the specified glyphDataFormat. """ assert self.isTTF - if checkFormat and self.font["head"].glyphDataFormat == 0: + if validateGlyphFormat and self.font["head"].glyphDataFormat == 0: for name, g in glyphs.items(): if g.isVarComposite(): raise ValueError( diff --git a/Tests/fontBuilder/fontBuilder_test.py b/Tests/fontBuilder/fontBuilder_test.py index 6131a972c..bdb8a3c93 100644 --- a/Tests/fontBuilder/fontBuilder_test.py +++ b/Tests/fontBuilder/fontBuilder_test.py @@ -152,7 +152,7 @@ def test_build_cubic_ttf(tmp_path): ): fb.setupGlyf(glyphs) # can skip check if feeling adventurous - fb.setupGlyf(glyphs, checkFormat=False) + fb.setupGlyf(glyphs, validateGlyphFormat=False) # cubics are (will be) allowed in glyf table format 1 fb = FontBuilder(1000, isTTF=True, glyphDataFormat=1)