Merge pull request #671 from googlefonts/no-reload-font

avoid reloading font unconditionally in postProcessor

Lib/ufo2ft/outlineCompiler.py

@@ -21,6 +21,7 @@
 from fontTools.pens.t2CharStringPen import T2CharStringPen
 from fontTools.pens.ttGlyphPen import TTGlyphPointPen
 from fontTools.ttLib import TTFont, newTable
+from fontTools.ttLib.standardGlyphOrder import standardGlyphOrder
 from fontTools.ttLib.tables._g_l_y_f import USE_MY_METRICS, Glyph
 from fontTools.ttLib.tables._h_e_a_d import mac_epoch_diff
 from fontTools.ttLib.tables.O_S_2f_2 import Panose
@@ -787,7 +788,7 @@ def _setupTable_hhea_or_vhea(self, tag):
         secondSideBearings = []  # right in hhea, bottom in vhea
         extents = []
         if mtxTable is not None:
-            for glyphName in self.allGlyphs:
+            for glyphName in self.glyphOrder:
                 advance, firstSideBearing = mtxTable[glyphName]
                 advances.append(advance)
                 bounds = self.glyphBoundingBoxes[glyphName]
@@ -836,10 +837,18 @@ def _setupTable_hhea_or_vhea(self, tag):
         for i in reserved:
             setattr(table, "reserved%i" % i, 0)
         table.metricDataFormat = 0
-        # glyph count
-        setattr(
-            table, "numberOf%sMetrics" % ("H" if isHhea else "V"), len(self.allGlyphs)
-        )
+        # precompute the number of long{Hor,Ver}Metric records in 'hmtx' table
+        # so we don't need to compile the latter to get this updated
+        numLongMetrics = len(advances)
+        if numLongMetrics > 1:
+            lastAdvance = advances[-1]
+            while advances[numLongMetrics - 2] == lastAdvance:
+                numLongMetrics -= 1
+                if numLongMetrics <= 1:
+                    # all advances are equal
+                    numLongMetrics = 1
+                    break
+        setattr(table, "numberOf%sMetrics" % ("H" if isHhea else "V"), numLongMetrics)
 
     def setupTable_hhea(self):
         """
@@ -1456,7 +1465,10 @@ def setupTable_post(self):
 
         post = self.otf["post"]
         post.formatType = 2.0
-        post.extraNames = []
+        # if we set extraNames = [], it will be automatically computed upon compile as
+        # we do below; if we do it upfront we can skip reloading in postProcessor.
+        post.extraNames = [g for g in self.glyphOrder if g not in standardGlyphOrder]
+
         post.mapping = {}
         post.glyphOrder = self.glyphOrder
 
@@ -1483,6 +1495,10 @@ def setupTable_glyf(self):
                 self.autoUseMyMetrics(ttGlyph, name, hmtx)
             glyf[name] = ttGlyph
 
+        # update various maxp fields based on glyf without needing to compile the font
+        if "maxp" in self.otf:
+            self.otf["maxp"].recalc(self.otf)
+
     @staticmethod
     def autoUseMyMetrics(ttGlyph, glyphName, hmtx):
         """Set the "USE_MY_METRICS" flag on the first component having the

Lib/ufo2ft/postProcessor.py

@@ -4,6 +4,7 @@
 from io import BytesIO
 
 from fontTools.ttLib import TTFont
+from fontTools.ttLib.standardGlyphOrder import standardGlyphOrder
 
 from ufo2ft.constants import (
     GLYPHS_DONT_USE_PRODUCTION_NAMES,
@@ -41,13 +42,7 @@ def __init__(self, otf, ufo, glyphSet=None):
         self.ufo = ufo
         self.glyphSet = glyphSet if glyphSet is not None else ufo
 
-        # FIXME: Stop reloading all incoming fonts here. It ensures that 1) we
-        # get the final binary layout, which canonicalizes data for us and 2)
-        # can easily rename glyphs later. The former point should be fixed, as
-        # reloading is expensive and it is within reason for the compiler to
-        # spit out something that can be used without reloading.
-        # https://github.com/googlefonts/ufo2ft/issues/485
-        self.otf = _reloadFont(otf)
+        self.otf = otf
 
         self._postscriptNames = ufo.lib.get("public.postscriptNames")
 
@@ -155,6 +150,12 @@ def process_glyph_names(self, useProductionNames=None):
 
             if useProductionNames:
                 logger.info("Renaming glyphs to final production names")
+                # We need to reload the font *before* renaming glyphs, since various
+                # tables may have been build/loaded using the original glyph names.
+                # After reloading, we can immediately set a new glyph order and update
+                # the tables (post or CFF) that stores the new postcript names; any
+                # other tables that get loaded subsequently will use the new glyph names.
+                self.otf = _reloadFont(self.otf)
                 self._rename_glyphs_from_ufo()
 
         else:
@@ -163,7 +164,11 @@ def process_glyph_names(self, useProductionNames=None):
                     "Dropping glyph names from CFF 1.0 is currently unsupported"
                 )
             else:
+                # To drop glyph names from TTF or CFF2, we must reload the font *after*
+                # setting the post format to 3.0, since other tables may still use
+                # the old glyph names.
                 self.set_post_table_format(self.otf, 3.0)
+                self.otf = _reloadFont(self.otf)
 
     def _rename_glyphs_from_ufo(self):
         """Rename glyphs using ufo.lib.public.postscriptNames in UFO."""
@@ -172,16 +177,17 @@ def _rename_glyphs_from_ufo(self):
 
     @staticmethod
     def rename_glyphs(otf, rename_map):
-        otf.setGlyphOrder([rename_map.get(n, n) for n in otf.getGlyphOrder()])
+        newGlyphOrder = [rename_map.get(n, n) for n in otf.getGlyphOrder()]
+        otf.setGlyphOrder(newGlyphOrder)
 
-        # we need to compile format 2 'post' table so that the 'extraNames'
-        # attribute is updated with the list of the names outside the
-        # standard Macintosh glyph order; otherwise, if one dumps the font
-        # to TTX directly before compiling first, the post table will not
-        # contain the extraNames.
         if "post" in otf and otf["post"].formatType == 2.0:
-            otf["post"].extraNames = []
-            otf["post"].compile(otf)
+            # annoyingly we need to update extraNames to match the new glyph order,
+            # otherwise, if dumping the font to TTX directly before compiling first,
+            # the post table will not contain the extraNames...
+            otf["post"].extraNames = [
+                g for g in newGlyphOrder if g not in standardGlyphOrder
+            ]
+            otf["post"].mapping = {}
 
         cff_tag = "CFF " if "CFF " in otf else "CFF2" if "CFF2" in otf else None
         if cff_tag == "CFF " or (cff_tag == "CFF2" and otf.isLoaded(cff_tag)):
@@ -277,11 +283,16 @@ def set_post_table_format(otf, formatType):
             raise NotImplementedError(formatType)
 
         post = otf.get("post")
-        if post and post.formatType != formatType:
-            logger.info("Setting post.formatType = %s", formatType)
-            post.formatType = formatType
+        if post:
+            if post.formatType != formatType:
+                logger.info("Setting post.formatType = %s", formatType)
+                post.formatType = formatType
+            # we want to update extraNames list even if formatType is the same
+            # so we don't have to reload the font
             if formatType == 2.0:
-                post.extraNames = []
+                post.extraNames = [
+                    g for g in otf.getGlyphOrder() if g not in standardGlyphOrder
+                ]
                 post.mapping = {}
             else:
                 for attr in ("extraNames", "mapping"):

requirements.txt

@@ -1,4 +1,4 @@
-fonttools[lxml,ufo]==4.34.0
+fonttools[lxml,ufo]==4.38.0
 defcon==0.10.0
 cu2qu==1.6.7.post1
 compreffor==0.5.1.post1

setup.py

@@ -29,7 +29,7 @@
     setup_requires=pytest_runner + wheel + ["setuptools_scm"],
     tests_require=["pytest>=2.8"],
     install_requires=[
-        "fonttools[ufo]>=4.34.0",
+        "fonttools[ufo]>=4.38.0",
         "cu2qu>=1.6.7",
         "cffsubr>=0.2.8",
         "booleanOperations>=0.9.0",

tests/data/DSv5/MutatorSansVariable_Weight_Width-CFF2.ttx

@@ -384,22 +384,22 @@
           900 300 -900 -300 vlineto
         </CharString>
         <CharString name="I">
-          0 40 10 20 -10 0 14 1 blend
+          0 40 10 20 -10 0 14.30922 1 blend
           hmoveto
           10 10 -10 hlineto
         </CharString>
         <CharString name="I.narrow">
-          0 40 10 20 -10 0 14 1 blend
+          0 40 10 20 -10 0 14.30922 1 blend
           hmoveto
           10 10 -10 hlineto
         </CharString>
         <CharString name="S">
-          0 40 10 20 -10 0 14 1 blend
+          0 40 10 20 -10 0 14.30922 1 blend
           hmoveto
           10 10 -10 hlineto
         </CharString>
         <CharString name="S.closed">
-          0 40 10 20 -10 0 14 1 blend
+          0 40 10 20 -10 0 14.30922 1 blend
           hmoveto
           10 10 -10 hlineto
         </CharString>