From 74fae244573934588480fe70db26d2c56684cfb5 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 27 Nov 2019 18:44:09 +0000 Subject: [PATCH 1/6] outlineCompiler: compute bounding boxes on compiled glyphs Fixes https://github.com/googlefonts/fontmake/issues/593 The input UFO glyphs may contain float coordinates. Instead of computing the bounding boxes from the UFO glyphs, calculcate them based on the compiled TTGlyph or CFF CharString objects which have already been rounded off to integer. This way, the hmtx.lsb will correctly match the glyph xMin, even in presence of an input UFO with float coordinates. --- Lib/ufo2ft/outlineCompiler.py | 214 +++++++++++++++++++++------------- 1 file changed, 135 insertions(+), 79 deletions(-) diff --git a/Lib/ufo2ft/outlineCompiler.py b/Lib/ufo2ft/outlineCompiler.py index 955969ca6..eccc68062 100644 --- a/Lib/ufo2ft/outlineCompiler.py +++ b/Lib/ufo2ft/outlineCompiler.py @@ -5,6 +5,7 @@ import logging import math from collections import Counter, namedtuple +from types import SimpleNamespace from fontTools.ttLib import TTFont, newTable from fontTools.cffLib import ( @@ -43,6 +44,7 @@ BoundingBox = namedtuple("BoundingBox", ["xMin", "yMin", "xMax", "yMax"]) +EMPTY_BOUNDING_BOX = BoundingBox(0, 0, 0, 0) def _isNonBMP(s): @@ -81,13 +83,14 @@ def __init__(self, font, glyphSet=None, glyphOrder=None, tables=None): if glyphOrder is None: glyphOrder = font.glyphOrder self.glyphOrder = self.makeOfficialGlyphOrder(glyphOrder) - # make reusable glyphs/font bounding boxes - self.glyphBoundingBoxes = self.makeGlyphsBoundingBoxes() - self.fontBoundingBox = self.makeFontBoundingBox() # make a reusable character mapping self.unicodeToGlyphNameMapping = self.makeUnicodeToGlyphNameMapping() if tables is not None: self.tables = tables + # cached values defined later on + self._glyphBoundingBoxes = None + self._fontBoundingBox = None + self._compiledGlyphs = None def compile(self): """ @@ -126,35 +129,38 @@ def compile(self): return self.otf + def compileGlyphs(self): + """Compile glyphs and return dict keyed by glyph name. + + **This should not be called externally.** + Subclasses must override this method to handle compilation of glyphs. + """ + raise NotImplementedError + + def getCompiledGlyphs(self): + if self._compiledGlyphs is None: + self._compiledGlyphs = self.compileGlyphs() + return self._compiledGlyphs + def makeGlyphsBoundingBoxes(self): """ Make bounding boxes for all the glyphs, and return a dictionary of BoundingBox(xMin, xMax, yMin, yMax) namedtuples keyed by glyph names. The bounding box of empty glyphs (without contours or components) is set to None. + The bbox values are integers. - Float values are rounded to integers using fontTools otRound(). - - **This should not be called externally.** Subclasses - may override this method to handle the bounds creation - in a different way if desired. + **This should not be called externally.** + Subclasses must override this method to handle the bounds creation for + their specific glyph type. """ + raise NotImplementedError - def getControlPointBounds(glyph): - pen.init() - glyph.draw(pen) - return pen.bounds - - glyphBoxes = {} - pen = ControlBoundsPen(self.allGlyphs) - for glyphName, glyph in self.allGlyphs.items(): - bounds = None - if glyph or glyph.components: - bounds = getControlPointBounds(glyph) - if bounds: - bounds = BoundingBox(*(otRound(v) for v in bounds)) - glyphBoxes[glyphName] = bounds - return glyphBoxes + @property + def glyphBoundingBoxes(self): + if self._glyphBoundingBoxes is None: + self._glyphBoundingBoxes = self.makeGlyphsBoundingBoxes() + return self._glyphBoundingBoxes def makeFontBoundingBox(self): """ @@ -164,8 +170,6 @@ def makeFontBoundingBox(self): may override this method to handle the bounds creation in a different way if desired. """ - if not hasattr(self, "glyphBoundingBoxes"): - self.glyphBoundingBoxes = self.makeGlyphsBoundingBoxes() fontBox = None for glyphName, glyphBox in self.glyphBoundingBoxes.items(): if glyphBox is None: @@ -175,9 +179,15 @@ def makeFontBoundingBox(self): else: fontBox = unionRect(fontBox, glyphBox) if fontBox is None: # unlikely - fontBox = BoundingBox(0, 0, 0, 0) + fontBox = EMPTY_BOUNDING_BOX return fontBox + @property + def fontBoundingBox(self): + if self._fontBoundingBox is None: + self._fontBoundingBox = self.makeFontBoundingBox() + return self._fontBoundingBox + def makeUnicodeToGlyphNameMapping(self): """ Make a ``unicode : glyph name`` mapping for the font. @@ -927,6 +937,47 @@ def __init__( font, glyphSet=glyphSet, glyphOrder=glyphOrder, tables=tables ) self.optimizeCFF = optimizeCFF + self._defaultAndNominalWidths = None + + def getDefaultAndNominalWidths(self): + """Return (defaultWidthX, nominalWidthX). + + If fontinfo.plist doesn't define these explicitly, compute optimal values + from the glyphs' advance widths. + """ + if self._defaultAndNominalWidths is None: + info = self.ufo.info + # populate the width values + if not any( + hasattr(info, attr) and getattr(info, attr) is not None + for attr in ("postscriptDefaultWidthX", "postscriptNominalWidthX") + ): + # no custom values set in fontinfo.plist; compute optimal ones + from fontTools.cffLib.width import optimizeWidths + + widths = [otRound(glyph.width) for glyph in self.allGlyphs.values()] + defaultWidthX, nominalWidthX = optimizeWidths(widths) + else: + defaultWidthX = otRound(info.postscriptDefaultWidthX) + nominalWidthX = otRound(info.postscriptNominalWidthX) + self._defaultAndNominalWidths = (defaultWidthX, nominalWidthX) + return self._defaultAndNominalWidths + + def compileGlyphs(self): + """Compile and return the CFF T2CharStrings for this font.""" + defaultWidth, nominalWidth = self.getDefaultAndNominalWidths() + # The real PrivateDict will be created later on in setupTable_CFF. + # For convenience here we use a namespace object to pass the default/nominal + # widths that we need to draw the charstrings when computing their bounds. + private = SimpleNamespace( + defaultWidthX=defaultWidth, nominalWidthX=nominalWidth + ) + compiledGlyphs = {} + for glyphName in self.glyphOrder: + glyph = self.allGlyphs[glyphName] + cs = self.getCharStringForGlyph(glyph, private) + compiledGlyphs[glyphName] = cs + return compiledGlyphs def makeGlyphsBoundingBoxes(self): """ @@ -941,11 +992,6 @@ def makeGlyphsBoundingBoxes(self): values. """ - def getControlPointBounds(glyph): - pen.init() - glyph.draw(pen) - return pen.bounds - def toInt(value, else_callback): rounded = otRound(value) if tolerance >= 0.5 or abs(rounded - value) <= tolerance: @@ -955,22 +1001,22 @@ def toInt(value, else_callback): tolerance = self.roundTolerance glyphBoxes = {} - pen = ControlBoundsPen(self.allGlyphs) - for glyphName, glyph in self.allGlyphs.items(): - bounds = None - if glyph or glyph.components: - bounds = getControlPointBounds(glyph) - if bounds: - rounded = [] - for value in bounds[:2]: - rounded.append(toInt(value, math.floor)) - for value in bounds[2:]: - rounded.append(toInt(value, math.ceil)) - bounds = BoundingBox(*rounded) - glyphBoxes[glyphName] = bounds + charStrings = self.getCompiledGlyphs() + for name, cs in charStrings.items(): + bounds = cs.calcBounds(charStrings) + if bounds is not None: + rounded = [] + for value in bounds[:2]: + rounded.append(toInt(value, math.floor)) + for value in bounds[2:]: + rounded.append(toInt(value, math.ceil)) + bounds = BoundingBox(*rounded) + if bounds == EMPTY_BOUNDING_BOX: + bounds = None + glyphBoxes[name] = bounds return glyphBoxes - def getCharStringForGlyph(self, glyph, private, globalSubrs): + def getCharStringForGlyph(self, glyph, private, globalSubrs=None): """ Get a Type2CharString for the *glyph* @@ -1100,26 +1146,7 @@ def setupTable_CFF(self): unitsPerEm = otRound(getAttrWithFallback(info, "unitsPerEm")) topDict.FontMatrix = [1.0 / unitsPerEm, 0, 0, 1.0 / unitsPerEm, 0, 0] # populate the width values - if ( - not any( - hasattr(info, attr) and getattr(info, attr) is not None - for attr in ("postscriptDefaultWidthX", "postscriptNominalWidthX") - ) - and "hmtx" in self.otf - ): - # no custom values set in fontinfo.plist; compute optimal ones - from fontTools.cffLib.width import optimizeWidths - - hmtx = self.otf["hmtx"] - widths = [m[0] for m in hmtx.metrics.values()] - defaultWidthX, nominalWidthX = optimizeWidths(widths) - else: - defaultWidthX = otRound( - getAttrWithFallback(info, "postscriptDefaultWidthX") - ) - nominalWidthX = otRound( - getAttrWithFallback(info, "postscriptNominalWidthX") - ) + defaultWidthX, nominalWidthX = self.getDefaultAndNominalWidths() if defaultWidthX: private.rawDict["defaultWidthX"] = defaultWidthX if nominalWidthX: @@ -1168,9 +1195,11 @@ def setupTable_CFF(self): private.rawDict["StemSnapV"] = stemSnapV private.rawDict["StdVW"] = stemSnapV[0] # populate glyphs + cffGlyphs = self.getCompiledGlyphs() for glyphName in self.glyphOrder: - glyph = self.allGlyphs[glyphName] - charString = self.getCharStringForGlyph(glyph, private, globalSubrs) + charString = cffGlyphs[glyphName] + charString.private = private + charString.globalSubrs = globalSubrs # add to the font if glyphName in charStrings: # XXX a glyph already has this name. should we choke? @@ -1191,6 +1220,41 @@ class OutlineTTFCompiler(BaseOutlineCompiler): sfntVersion = "\000\001\000\000" tables = BaseOutlineCompiler.tables | {"loca", "gasp", "glyf"} + def compileGlyphs(self): + """Compile and return the TrueType glyphs for this font.""" + allGlyphs = self.allGlyphs + ttGlyphs = {} + for name in self.glyphOrder: + glyph = allGlyphs[name] + pen = TTGlyphPen(allGlyphs) + try: + glyph.draw(pen) + except NotImplementedError: + logger.error("%r has invalid curve format; skipped", name) + ttGlyph = Glyph() + else: + ttGlyph = pen.glyph() + ttGlyphs[name] = ttGlyph + return ttGlyphs + + def makeGlyphsBoundingBoxes(self): + """Make bounding boxes for all the glyphs. + + Return a dictionary of BoundingBox(xMin, xMax, yMin, yMax) namedtuples + keyed by glyph names. + The bounding box of empty glyphs (without contours or components) is + set to None. + """ + glyphBoxes = {} + ttGlyphs = self.getCompiledGlyphs() + for glyphName, glyph in ttGlyphs.items(): + glyph.recalcBounds(ttGlyphs) + bounds = BoundingBox(glyph.xMin, glyph.yMin, glyph.xMax, glyph.yMax) + if bounds == EMPTY_BOUNDING_BOX: + bounds = None + glyphBoxes[glyphName] = bounds + return glyphBoxes + def setupTable_maxp(self): """Make the maxp table.""" if "maxp" not in self.tables: @@ -1238,19 +1302,11 @@ def setupTable_glyf(self): glyf.glyphOrder = self.glyphOrder hmtx = self.otf.get("hmtx") - allGlyphs = self.allGlyphs + ttGlyphs = self.getCompiledGlyphs() for name in self.glyphOrder: - glyph = allGlyphs[name] - pen = TTGlyphPen(allGlyphs) - try: - glyph.draw(pen) - except NotImplementedError: - logger.error("%r has invalid curve format; skipped", name) - ttGlyph = Glyph() - else: - ttGlyph = pen.glyph() - if ttGlyph.isComposite() and hmtx is not None and self.autoUseMyMetrics: - self.autoUseMyMetrics(ttGlyph, name, hmtx) + ttGlyph = ttGlyphs[name] + if ttGlyph.isComposite() and hmtx is not None and self.autoUseMyMetrics: + self.autoUseMyMetrics(ttGlyph, name, hmtx) glyf[name] = ttGlyph @staticmethod From 19d49d34b7e19d7a5e8dca3099d4a01ccd1e4df4 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 27 Nov 2019 18:45:24 +0000 Subject: [PATCH 2/6] outlineCompiler_test: makeGlyphsBoundingBoxes test expects qudratic outlines --- tests/outlineCompiler_test.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/outlineCompiler_test.py b/tests/outlineCompiler_test.py index 1a86cb618..cc2e2d143 100644 --- a/tests/outlineCompiler_test.py +++ b/tests/outlineCompiler_test.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- from __future__ import print_function, absolute_import, division, unicode_literals +from cu2qu.ufo import font_to_quadratic from fontTools.ttLib import TTFont from fontTools.misc.py23 import basestring, unichr, byteord from fontTools import designspaceLib @@ -36,6 +37,13 @@ def testufo(FontClass): return font +@pytest.fixture +def quadufo(FontClass): + font = FontClass(getpath("TestFont.ufo")) + font_to_quadratic(font) + return font + + @pytest.fixture def use_my_metrics_ufo(FontClass): return FontClass(getpath("UseMyMetrics.ufo")) @@ -81,9 +89,8 @@ def test_compile_empty_gasp(self, testufo): compiler.compile() assert "gasp" not in compiler.otf - def test_makeGlyphsBoundingBoxes(self, testufo): - # the call to 'makeGlyphsBoundingBoxes' happen in the __init__ method - compiler = OutlineTTFCompiler(testufo) + def test_makeGlyphsBoundingBoxes(self, quadufo): + compiler = OutlineTTFCompiler(quadufo) assert compiler.glyphBoundingBoxes[".notdef"] == (50, 0, 450, 750) # no outline data assert compiler.glyphBoundingBoxes["space"] is None @@ -414,7 +421,6 @@ def test_setupTable_CFF_no_optimize(self, testufo): ) def test_makeGlyphsBoundingBoxes(self, testufo): - # the call to 'makeGlyphsBoundingBoxes' happen in the __init__ method compiler = OutlineOTFCompiler(testufo) # with default roundTolerance, all coordinates and hence the bounding # box values are rounded with otRound() From bb5755ba732f491ad74cb7a11b4d46df564596ea Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 28 Nov 2019 17:43:14 +0000 Subject: [PATCH 3/6] setup.py: require Python >= 3.6 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 8f539946d..0b6952771 100644 --- a/setup.py +++ b/setup.py @@ -34,9 +34,9 @@ "cu2qu>=1.6.5", "compreffor>=0.4.6", "booleanOperations>=0.8.2", - "enum34>=1.1.6 ; python_version < '3.4'", ], extras_require={"pathops": ["skia-pathops>=0.2.0"]}, + python_requires=">=3.6", classifiers=[ "Development Status :: 4 - Beta", "Environment :: Console", From 8132fc9aae613795f84154f486aee2364c074cb1 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 28 Nov 2019 17:43:34 +0000 Subject: [PATCH 4/6] bump minimum requirements --- requirements.txt | 10 +++++----- setup.py | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/requirements.txt b/requirements.txt index cfd81ee26..df02a62c3 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,8 +1,8 @@ -fonttools[lxml,ufo]==3.44.0 +fonttools[lxml,ufo]==4.2.0 defcon==0.6.0 -cu2qu==1.6.5 +cu2qu==1.6.6 compreffor==0.4.6.post1 -booleanOperations==0.8.2 +booleanOperations==0.9.0 -# alternative UFO implementation (still experimental) -ufoLib2==0.3.2.post2; python_version >= '3.6' +# alternative UFO implementation +ufoLib2==0.5.1 diff --git a/setup.py b/setup.py index 0b6952771..922354751 100644 --- a/setup.py +++ b/setup.py @@ -30,10 +30,10 @@ setup_requires=pytest_runner + wheel + ["setuptools_scm"], tests_require=["pytest>=2.8"], install_requires=[ - "fonttools[ufo]>=3.43.0", - "cu2qu>=1.6.5", + "fonttools[ufo]>=4.2.0", + "cu2qu>=1.6.6", "compreffor>=0.4.6", - "booleanOperations>=0.8.2", + "booleanOperations>=0.9.0", ], extras_require={"pathops": ["skia-pathops>=0.2.0"]}, python_requires=">=3.6", From 1eddbc5d52e015b4728b06ce27b7bbce3a97a675 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 28 Nov 2019 17:47:07 +0000 Subject: [PATCH 5/6] [ci] don't run test on py27 any more; run tests on 3.7 --- .travis.yml | 8 ++++---- appveyor.yml | 24 ++++++++++++------------ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/.travis.yml b/.travis.yml index ca2f8051f..6f84f66fd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,3 @@ -sudo: false language: python env: global: @@ -7,10 +6,11 @@ env: matrix: include: - - python: 2.7 - env: TOXENV=py27-cov - python: 3.6 env: TOXENV=py36-cov + - python: 3.7 + dist: xenial + env: TOXENV=py37-cov branches: only: @@ -24,7 +24,7 @@ script: tox after_success: - tox -e codecov - | - if [ -n "$TRAVIS_TAG" ] && [ "$TRAVIS_REPO_SLUG" == "googlefonts/ufo2ft" ] && [ "$TRAVIS_PYTHON_VERSION" == "3.6" ]; then + if [ -n "$TRAVIS_TAG" ] && [ "$TRAVIS_REPO_SLUG" == "googlefonts/ufo2ft" ] && [ "$TRAVIS_PYTHON_VERSION" == "3.7" ]; then pip install --upgrade twine pip setuptools wheel python setup.py sdist pip wheel --no-deps --wheel-dir dist . diff --git a/appveyor.yml b/appveyor.yml index ca292ec43..ab917decf 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,26 +1,26 @@ environment: matrix: - - JOB: "2.7 32-bit" - PYTHON_HOME: "C:\\Python27" - TOXENV: "py27-cov" - TOXPYTHON: "C:\\Python27\\python.exe" - - JOB: "3.6 32-bit" PYTHON_HOME: "C:\\Python36" TOXENV: "py36-cov" TOXPYTHON: "C:\\Python36\\python.exe" - - JOB: "2.7 64-bit" - PYTHON_HOME: "C:\\Python27-x64" - TOXENV: "py27-cov" - TOXPYTHON: "C:\\Python27-x64\\python.exe" - - JOB: "3.6 64-bit" - PYTHON_HOME: "C:\\Python35-x64" + PYTHON_HOME: "C:\\Python36-x64" TOXENV: "py36-cov" TOXPYTHON: "C:\\Python36-x64\\python.exe" -# Do not build feature branches with open Pull Requests after the initial + - JOB: "3.7 32-bit" + PYTHON_HOME: "C:\\Python37" + TOXENV: "py37-cov" + TOXPYTHON: "C:\\Python37\\python.exe" + + - JOB: "3.7 64-bit" + PYTHON_HOME: "C:\\Python37-x64" + TOXENV: "py37-cov" + TOXPYTHON: "C:\\Python37-x64\\python.exe" + +# Do not build feature branches with open Pull Requests after the initial # opening of a PR. skip_branch_with_pr: true From b5a9974cbbffb5d4c53cce0216848a04803c06a8 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 3 Dec 2019 12:13:31 +0000 Subject: [PATCH 6/6] outlineCompiler: use getattr(info, attr, None) as suggested by madig Co-Authored-By: Nikolaus Waxweiler --- Lib/ufo2ft/outlineCompiler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/ufo2ft/outlineCompiler.py b/Lib/ufo2ft/outlineCompiler.py index eccc68062..055819da4 100644 --- a/Lib/ufo2ft/outlineCompiler.py +++ b/Lib/ufo2ft/outlineCompiler.py @@ -949,7 +949,7 @@ def getDefaultAndNominalWidths(self): info = self.ufo.info # populate the width values if not any( - hasattr(info, attr) and getattr(info, attr) is not None + getattr(info, attr, None) is not None for attr in ("postscriptDefaultWidthX", "postscriptNominalWidthX") ): # no custom values set in fontinfo.plist; compute optimal ones