Merge pull request #486 from googlefonts/avoid-font-reloading

VF compilation: post-process fonts once

Lib/ufo2ft/__init__.py

@@ -1,5 +1,6 @@
 import logging
 from enum import IntEnum
+from typing import Any, Optional
 
 from fontTools import varLib
 
@@ -53,6 +54,7 @@ def compileOTF(
     cffVersion=1,
     subroutinizer=None,
     notdefGlyph=None,
+    postProcessorClass: Optional[Any] = PostProcessor,
     _tables=None,
 ):
     """Create FontTools CFF font from a UFO.
@@ -153,13 +155,14 @@ def compileOTF(
             debugFeatureFile=debugFeatureFile,
         )
 
-    postProcessor = PostProcessor(otf, ufo, glyphSet=glyphSet)
-    otf = postProcessor.process(
-        useProductionNames,
-        optimizeCFF=optimizeCFF >= CFFOptimization.SUBROUTINIZE,
-        subroutinizer=subroutinizer,
-        cffVersion=cffVersion,
-    )
+    if postProcessorClass is not None:
+        postProcessor = postProcessorClass(otf, ufo, glyphSet=glyphSet)
+        otf = postProcessor.process(
+            useProductionNames,
+            optimizeCFF=optimizeCFF >= CFFOptimization.SUBROUTINIZE,
+            subroutinizer=subroutinizer,
+            cffVersion=cffVersion,
+        )
 
     return otf
 
@@ -185,6 +188,7 @@ def compileTTF(
     skipExportGlyphs=None,
     debugFeatureFile=None,
     notdefGlyph=None,
+    postProcessorClass: Optional[Any] = PostProcessor,
 ):
     """Create FontTools TrueType font from a UFO.
 
@@ -244,8 +248,9 @@ def compileTTF(
             debugFeatureFile=debugFeatureFile,
         )
 
-    postProcessor = PostProcessor(otf, ufo, glyphSet=glyphSet)
-    otf = postProcessor.process(useProductionNames)
+    if postProcessorClass is not None:
+        postProcessor = postProcessorClass(otf, ufo, glyphSet=glyphSet)
+        otf = postProcessor.process(useProductionNames)
 
     return otf
 
@@ -267,6 +272,7 @@ def compileInterpolatableTTFs(
     skipExportGlyphs=None,
     debugFeatureFile=None,
     notdefGlyph=None,
+    postProcessorClass: Optional[Any] = PostProcessor,
 ):
     """Create FontTools TrueType fonts from a list of UFOs with interpolatable
     outlines. Cubic curves are converted compatibly to quadratic curves using
@@ -343,8 +349,9 @@ def compileInterpolatableTTFs(
                 debugFeatureFile=debugFeatureFile,
             )
 
-        postProcessor = PostProcessor(ttf, ufo, glyphSet=glyphSet)
-        ttf = postProcessor.process(useProductionNames)
+        if postProcessorClass is not None:
+            postProcessor = postProcessorClass(ttf, ufo, glyphSet=glyphSet)
+            ttf = postProcessor.process(useProductionNames)
 
         if layerName is not None:
             # for sparse masters (i.e. containing only a subset of the glyphs), we
@@ -375,6 +382,7 @@ def compileInterpolatableTTFsFromDS(
     inplace=False,
     debugFeatureFile=None,
     notdefGlyph=None,
+    postProcessorClass: Optional[Any] = PostProcessor,
 ):
     """Create FontTools TrueType fonts from the DesignSpaceDocument UFO sources
     with interpolatable outlines. Cubic curves are converted compatibly to
@@ -433,6 +441,7 @@ def compileInterpolatableTTFsFromDS(
         skipExportGlyphs=skipExportGlyphs,
         debugFeatureFile=debugFeatureFile,
         notdefGlyph=notdefGlyph,
+        postProcessorClass=postProcessorClass,
     )
 
     if inplace:
@@ -458,6 +467,7 @@ def compileInterpolatableOTFsFromDS(
     inplace=False,
     debugFeatureFile=None,
     notdefGlyph=None,
+    postProcessorClass: Optional[Any] = PostProcessor,
 ):
     """Create FontTools CFF fonts from the DesignSpaceDocument UFO sources
     with interpolatable outlines.
@@ -519,6 +529,7 @@ def compileInterpolatableOTFsFromDS(
                 debugFeatureFile=debugFeatureFile,
                 notdefGlyph=notdefGlyph,
                 _tables=SPARSE_OTF_MASTER_TABLES if source.layerName else None,
+                postProcessorClass=postProcessorClass,
             )
         )
 
@@ -596,6 +607,7 @@ def compileVariableTTF(
     inplace=False,
     debugFeatureFile=None,
     notdefGlyph=None,
+    postProcessorClass: Optional[Any] = PostProcessor,
 ):
     """Create FontTools TrueType variable font from the DesignSpaceDocument UFO sources
     with interpolatable outlines, using fontTools.varLib.build.
@@ -627,6 +639,8 @@ def compileVariableTTF(
         inplace=inplace,
         debugFeatureFile=debugFeatureFile,
         notdefGlyph=notdefGlyph,
+        # No need to post-process intermediate fonts.
+        postProcessorClass=None,
     )
 
     logger.info("Building variable TTF font")
@@ -635,8 +649,9 @@ def compileVariableTTF(
         ttfDesignSpace, exclude=excludeVariationTables, optimize=optimizeGvar
     )[0]
 
-    postProcessor = PostProcessor(varfont, baseUfo)
-    varfont = postProcessor.process(useProductionNames)
+    if postProcessorClass is not None:
+        postProcessor = postProcessorClass(varfont, baseUfo)
+        varfont = postProcessor.process(useProductionNames)
 
     return varfont
 
@@ -656,6 +671,7 @@ def compileVariableCFF2(
     debugFeatureFile=None,
     optimizeCFF=CFFOptimization.SPECIALIZE,
     notdefGlyph=None,
+    postProcessorClass: Optional[Any] = PostProcessor,
 ):
     """Create FontTools CFF2 variable font from the DesignSpaceDocument UFO sources
     with interpolatable outlines, using fontTools.varLib.build.
@@ -690,6 +706,8 @@ def compileVariableCFF2(
         inplace=inplace,
         debugFeatureFile=debugFeatureFile,
         notdefGlyph=notdefGlyph,
+        # No need to post-process intermediate fonts.
+        postProcessorClass=None,
     )
 
     logger.info("Building variable CFF2 font")
@@ -704,10 +722,11 @@ def compileVariableCFF2(
         optimize=optimizeCFF >= CFFOptimization.SPECIALIZE,
     )[0]
 
-    postProcessor = PostProcessor(varfont, baseUfo)
-    varfont = postProcessor.process(
-        useProductionNames,
-        optimizeCFF=optimizeCFF >= CFFOptimization.SUBROUTINIZE,
-    )
+    if postProcessorClass is not None:
+        postProcessor = postProcessorClass(varfont, baseUfo)
+        varfont = postProcessor.process(
+            useProductionNames,
+            optimizeCFF=optimizeCFF >= CFFOptimization.SUBROUTINIZE,
+        )
 
     return varfont

Lib/ufo2ft/outlineCompiler.py

@@ -537,7 +537,7 @@ def setupTable_OS2(self):
         # subscript, superscript, strikeout values, taken from AFDKO:
         # FDK/Tools/Programs/makeotf/makeotf_lib/source/hotconv/hot.c
         unitsPerEm = getAttrWithFallback(font.info, "unitsPerEm")
-        italicAngle = getAttrWithFallback(font.info, "italicAngle")
+        italicAngle = float(getAttrWithFallback(font.info, "italicAngle"))
         xHeight = getAttrWithFallback(font.info, "xHeight")
 
         def adjustOffset(offset, angle):
@@ -889,7 +889,7 @@ def setupTable_post(self):
         font = self.ufo
         post.formatType = 3.0
         # italic angle
-        italicAngle = getAttrWithFallback(font.info, "italicAngle")
+        italicAngle = float(getAttrWithFallback(font.info, "italicAngle"))
         post.italicAngle = italicAngle
         # underline
         underlinePosition = getAttrWithFallback(
@@ -900,7 +900,9 @@ def setupTable_post(self):
             font.info, "postscriptUnderlineThickness"
         )
         post.underlineThickness = otRound(underlineThickness)
-        post.isFixedPitch = getAttrWithFallback(font.info, "postscriptIsFixedPitch")
+        post.isFixedPitch = int(
+            getAttrWithFallback(font.info, "postscriptIsFixedPitch")
+        )
         # misc
         post.minMemType42 = 0
         post.maxMemType42 = 0
@@ -1140,6 +1142,9 @@ def setupTable_CFF(self):
 
         self.otf["CFF "] = cff = newTable("CFF ")
         cff = cff.cff
+        # NOTE: Set up a back-reference to be used by some CFFFontSet methods
+        # down the line (as of fontTools 4.21.1).
+        cff.otFont = self.otf
         # set up the basics
         cff.major = 1
         cff.minor = 0
@@ -1215,8 +1220,8 @@ def setupTable_CFF(self):
         )
         topDict.Weight = getAttrWithFallback(info, "postscriptWeightName")
         # populate various numbers
-        topDict.isFixedPitch = getAttrWithFallback(info, "postscriptIsFixedPitch")
-        topDict.ItalicAngle = getAttrWithFallback(info, "italicAngle")
+        topDict.isFixedPitch = int(getAttrWithFallback(info, "postscriptIsFixedPitch"))
+        topDict.ItalicAngle = float(getAttrWithFallback(info, "italicAngle"))
         underlinePosition = getAttrWithFallback(info, "postscriptUnderlinePosition")
         topDict.UnderlinePosition = otRound(underlinePosition)
         underlineThickness = getAttrWithFallback(info, "postscriptUnderlineThickness")

Lib/ufo2ft/postProcessor.py

@@ -40,10 +40,15 @@ class SubroutinizerBackend(enum.Enum):
     def __init__(self, otf, ufo, glyphSet=None):
         self.ufo = ufo
         self.glyphSet = glyphSet if glyphSet is not None else ufo
-        stream = BytesIO()
-        otf.save(stream)
-        stream.seek(0)
-        self.otf = TTFont(stream)
+
+        # 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._postscriptNames = ufo.lib.get("public.postscriptNames")
 
     def process(
@@ -379,3 +384,11 @@ def _stripCharStringWidth(program):
     if stack:
         result.extend(stack)
     return result
+
+
+def _reloadFont(font: TTFont) -> TTFont:
+    """Recompile a font to arrive at the final internal layout."""
+    stream = BytesIO()
+    font.save(stream)
+    stream.seek(0)
+    return TTFont(stream)

requirements.txt

@@ -1,4 +1,4 @@
-fonttools[lxml,ufo]==4.21.1
+fonttools[lxml,ufo]==4.22.0
 defcon==0.8.1
 cu2qu==1.6.7
 compreffor==0.5.1

setup.py

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

tests/data/Bug108.ttx

@@ -34,7 +34,7 @@
         <LookupFlag value="0"/>
         <!-- SubTableCount=1 -->
         <PairPos index="0" Format="1">
-          <Coverage Format="1">
+          <Coverage>
             <Glyph value="uni0061"/>
           </Coverage>
           <ValueFormat1 value="4"/>

tests/data/MTIFeatures.ttx

@@ -33,7 +33,7 @@
         <LookupType value="4"/>
         <LookupFlag value="0"/>
         <!-- SubTableCount=1 -->
-        <LigatureSubst index="0" Format="1">
+        <LigatureSubst index="0">
           <LigatureSet glyph="uni0062">
             <Ligature components="uni0063" glyph="uni0061"/>
           </LigatureSet>

tests/data/TestFont-CFF-compreffor.ttx

@@ -420,7 +420,7 @@
         <LookupFlag value="8"/><!-- ignoreMarks -->
         <!-- SubTableCount=1 -->
         <PairPos index="0" Format="1">
-          <Coverage Format="1">
+          <Coverage>
             <Glyph value="uni0061"/>
             <Glyph value="uni0062"/>
           </Coverage>

tests/data/TestFont-CFF.ttx

@@ -436,7 +436,7 @@
         <LookupFlag value="8"/><!-- ignoreMarks -->
         <!-- SubTableCount=1 -->
         <PairPos index="0" Format="1">
-          <Coverage Format="1">
+          <Coverage>
             <Glyph value="uni0061"/>
             <Glyph value="uni0062"/>
           </Coverage>

tests/data/TestFont-CFF2-cffsubr.ttx

@@ -291,6 +291,8 @@
             <StdVW value="80"/>
             <StemSnapH value="100 120"/>
             <StemSnapV value="80 90"/>
+            <LanguageGroup value="0"/>
+            <ExpansionFactor value="0.06"/>
             <Subrs>
               <!-- The 'index' attribute is only for humans; it is ignored when parsed. -->
               <CharString index="0">
@@ -440,7 +442,7 @@
         <LookupFlag value="8"/><!-- ignoreMarks -->
         <!-- SubTableCount=1 -->
         <PairPos index="0" Format="1">
-          <Coverage Format="1">
+          <Coverage>
             <Glyph value="uni0061"/>
             <Glyph value="uni0062"/>
           </Coverage>

tests/data/TestFont-CFF2-compreffor.ttx

@@ -294,6 +294,8 @@
             <StdVW value="80"/>
             <StemSnapH value="100 120"/>
             <StemSnapV value="80 90"/>
+            <LanguageGroup value="0"/>
+            <ExpansionFactor value="0.06"/>
             <Subrs>
               <!-- The 'index' attribute is only for humans; it is ignored when parsed. -->
               <CharString index="0">
@@ -433,7 +435,7 @@
         <LookupFlag value="8"/><!-- ignoreMarks -->
         <!-- SubTableCount=1 -->
         <PairPos index="0" Format="1">
-          <Coverage Format="1">
+          <Coverage>
             <Glyph value="uni0061"/>
             <Glyph value="uni0062"/>
           </Coverage>

tests/data/TestFont-CFF2-post3.ttx

@@ -266,6 +266,8 @@
             <StdVW value="80"/>
             <StemSnapH value="100 120"/>
             <StemSnapV value="80 90"/>
+            <LanguageGroup value="0"/>
+            <ExpansionFactor value="0.06"/>
             <Subrs>
               <!-- The 'index' attribute is only for humans; it is ignored when parsed. -->
               <CharString index="0">
@@ -415,7 +417,7 @@
         <LookupFlag value="8"/><!-- ignoreMarks -->
         <!-- SubTableCount=1 -->
         <PairPos index="0" Format="1">
-          <Coverage Format="1">
+          <Coverage>
             <Glyph value="a"/>
             <Glyph value="b"/>
           </Coverage>

tests/data/TestFont-NoOptimize-CFF.ttx

@@ -439,7 +439,7 @@
         <LookupFlag value="8"/><!-- ignoreMarks -->
         <!-- SubTableCount=1 -->
         <PairPos index="0" Format="1">
-          <Coverage Format="1">
+          <Coverage>
             <Glyph value="uni0061"/>
             <Glyph value="uni0062"/>
           </Coverage>

tests/data/TestFont-NoOptimize-CFF2.ttx

@@ -294,6 +294,8 @@
             <StdVW value="80"/>
             <StemSnapH value="100 120"/>
             <StemSnapV value="80 90"/>
+            <LanguageGroup value="0"/>
+            <ExpansionFactor value="0.06"/>
           </Private>
         </FontDict>
       </FDArray>
@@ -449,7 +451,7 @@
         <LookupFlag value="8"/><!-- ignoreMarks -->
         <!-- SubTableCount=1 -->
         <PairPos index="0" Format="1">
-          <Coverage Format="1">
+          <Coverage>
             <Glyph value="uni0061"/>
             <Glyph value="uni0062"/>
           </Coverage>

tests/data/TestFont-NoOverlaps-CFF-pathops.ttx

@@ -431,7 +431,7 @@
         <LookupFlag value="8"/><!-- ignoreMarks -->
         <!-- SubTableCount=1 -->
         <PairPos index="0" Format="1">
-          <Coverage Format="1">
+          <Coverage>
             <Glyph value="uni0061"/>
             <Glyph value="uni0062"/>
           </Coverage>