Refactor ttGlyphSet.py

- only differentiate between glyf and CFF/CFF2 implementations, not var vs non-var
- use collections.abc.Mapping to get a more consistent dict-like object with less code
- prefer HVAR metrics over gvar metrics
- move some responsibilities from the _TTGlyphSet objects to the _TTGlyph objects
- adjust some tests to the changes

Lib/fontTools/ttLib/ttFont.py

@@ -4,12 +4,7 @@ from fontTools.misc.configTools import AbstractConfig
 from fontTools.misc.textTools import Tag, byteord, tostr
 from fontTools.misc.loggingTools import deprecateArgument
 from fontTools.ttLib import TTLibError
-from fontTools.ttLib.ttGlyphSet import (
+from fontTools.ttLib.ttGlyphSet import _TTGlyph, _TTGlyphSetCFF, _TTGlyphSetGlyf
-    _TTGlyphSet, _TTGlyph,
-    _TTGlyphCFF, _TTGlyphGlyf,
-    _TTVarGlyphSet,
-    _TTVarGlyphCFF, _TTVarGlyphGlyf,
-)
 from fontTools.ttLib.sfnt import SFNTReader, SFNTWriter
 from io import BytesIO, StringIO
 import os
@@ -700,28 +695,18 @@ class TTFont(object):
 		as in the normalized (-1..+1) space, otherwise it is in the font's defined
 		axes space.
 		"""
-		glyphs = None
+		glyphSetClass = None
-		if (preferCFF and any(tb in self for tb in ["CFF ", "CFF2"]) or
+		haveCFF = "CFF " in self or "CFF2" in self
-		   ("glyf" not in self and any(tb in self for tb in ["CFF ", "CFF2"]))):
+		if haveCFF and (preferCFF or "glyf" not in self):
-			table_tag = "CFF2" if "CFF2" in self else "CFF "
+			glyphSetClass = _TTGlyphSetCFF
-			glyphs = list(self[table_tag].cff.values())[0].CharStrings
-			if location and 'fvar' in self:
-				glyphs = _TTVarGlyphSet(self, glyphs, _TTVarGlyphCFF,
-										location, normalized)
-			else:
-				glyphs = _TTGlyphSet(self, glyphs, _TTGlyphCFF)
 
-		if glyphs is None and "glyf" in self:
+		if glyphSetClass is None and "glyf" in self:
-			if location and 'gvar' in self:
+			glyphSetClass = _TTGlyphSetGlyf
-				glyphs = _TTVarGlyphSet(self, self["glyf"], _TTVarGlyphGlyf,
-										location, normalized)
-			else:
-				glyphs = _TTGlyphSet(self, self["glyf"], _TTGlyphGlyf)
 
-		if glyphs is None:
+		if glyphSetClass is None:
 			raise TTLibError("Font contains no outlines")
 
-		return glyphs
+		return glyphSetClass(self, location, normalized)
 
 	def getBestCmap(self, cmapPreferences=((3, 10), (0, 6), (0, 4), (3, 1), (0, 3), (0, 2), (0, 1), (0, 0))):
 		"""Returns the 'best' Unicode cmap dictionary available in the font

Lib/fontTools/ttLib/ttGlyphSet.py

@@ -1,158 +1,220 @@
 """GlyphSets returned by a TTFont."""
 
-from fontTools.misc.fixedTools import otRound
+from collections.abc import Mapping
 from copy import copy
+from fontTools.misc.fixedTools import otRound
+from fontTools.misc.loggingTools import deprecateFunction
 
 
-class _TTGlyphSet(object):
+class _TTGlyphSet(Mapping):
 
     """Generic dict-like GlyphSet class that pulls metrics from hmtx and
     glyph shape from TrueType or CFF.
     """
 
-    def __init__(self, ttFont, glyphs, glyphType):
+    def __init__(self, font, location, normalized, glyphsMapping):
-        """Construct a new glyphset.
+        self.font = font
+        self.location = _normalizeLocation(font, location, normalized)
+        self.glyphsMapping = glyphsMapping
+        self.hMetrics = font["hmtx"].metrics
+        self.vMetrics = getattr(font.get("vmtx"), "metrics", None)
+        if self.location:
+            from fontTools.varLib.varStore import VarStoreInstancer
 
-        Args:
+            self.hvarTable = getattr(font.get("HVAR"), "table", None)
-                        font (TTFont): The font object (used to get metrics).
+            self.hvarInstancer = (
-                        glyphs (dict): A dictionary mapping glyph names to ``_TTGlyph`` objects.
+                VarStoreInstancer(
-                        glyphType (class): Either ``_TTGlyphCFF`` or ``_TTGlyphGlyf``.
+                    self.hvarTable.VarStore, font["fvar"].axes, self.location
-        """
+                )
-        self._glyphs = glyphs
+                if self.hvarTable is not None
-        self._hmtx = ttFont["hmtx"]
+                else None
-        self._vmtx = ttFont["vmtx"] if "vmtx" in ttFont else None
+            )
-        self._glyphType = glyphType
+            # TODO VVAR, VORG
 
-    def keys(self):
+    def __contains__(self, glyphName):
-        return list(self._glyphs.keys())
+        return glyphName in self.glyphsMapping
 
-    def has_key(self, glyphName):
+    def __iter__(self):
-        return glyphName in self._glyphs
+        return iter(self.glyphsMapping.keys())
-
-    __contains__ = has_key
-
-    def __getitem__(self, glyphName):
-        horizontalMetrics = self._hmtx[glyphName]
-        verticalMetrics = self._vmtx[glyphName] if self._vmtx else None
-        return self._glyphType(
-            self, self._glyphs[glyphName], horizontalMetrics, verticalMetrics
-        )
 
     def __len__(self):
-        return len(self._glyphs)
+        return len(self.glyphsMapping)
 
-    def get(self, glyphName, default=None):
+    @deprecateFunction(
-        try:
+        "use 'glyphName in glyphSet' instead", category=DeprecationWarning
-            return self[glyphName]
+    )
-        except KeyError:
+    def has_key(self, glyphName):
-            return default
+        return glyphName in self.glyphsMapping
 
 
-class _TTGlyph(object):
+class _TTGlyphSetGlyf(_TTGlyphSet):
+    def __init__(self, font, location, normalized):
+        self.glyfTable = font["glyf"]
+        super().__init__(font, location, normalized, self.glyfTable)
+        if self.location:
+            self.gvarTable = font.get("gvar")
 
-    """Wrapper for a TrueType glyph that supports the Pen protocol, meaning
+    def __getitem__(self, glyphName):
-    that it has .draw() and .drawPoints() methods that take a pen object as
+        return _TTGlyphGlyf(self, glyphName)
-    their only argument. Additionally there are 'width' and 'lsb' attributes,
+
-    read from the 'hmtx' table.
+
+class _TTGlyphSetCFF(_TTGlyphSet):
+    def __init__(self, font, location, normalized):
+        tableTag = "CFF2" if "CFF2" in font else "CFF "
+        self.charStrings = list(font[tableTag].cff.values())[0].CharStrings
+        super().__init__(font, location, normalized, self.charStrings)
+        self.blender = None
+        if self.location:
+            from fontTools.varLib.varStore import VarStoreInstancer
+
+            varStore = getattr(self.charStrings, "varStore", None)
+            if varStore is not None:
+                instancer = VarStoreInstancer(
+                    varStore.otVarStore, font["fvar"].axes, self.location
+                )
+                self.blender = instancer.interpolateFromDeltas
+
+    def __getitem__(self, glyphName):
+        return _TTGlyphCFF(self, glyphName)
+
+
+class _TTGlyph:
+
+    """Glyph object that supports the Pen protocol, meaning that it has
+    .draw() and .drawPoints() methods that take a pen object as their only
+    argument. Additionally there are 'width' and 'lsb' attributes, read from
+    the 'hmtx' table.
 
     If the font contains a 'vmtx' table, there will also be 'height' and 'tsb'
     attributes.
     """
 
-    def __init__(self, glyphset, glyph, horizontalMetrics=None, verticalMetrics=None):
+    def __init__(self, glyphSet, glyphName):
-        """Construct a new _TTGlyph.
+        self.glyphSet = glyphSet
-
+        self.name = glyphName
-        Args:
+        self.width, self.lsb = glyphSet.hMetrics[glyphName]
-                        glyphset (_TTGlyphSet): A glyphset object used to resolve components.
+        if glyphSet.vMetrics is not None:
-                        glyph (ttLib.tables._g_l_y_f.Glyph): The glyph object.
+            self.height, self.tsb = glyphSet.vMetrics[glyphName]
-                        horizontalMetrics (int, int): The glyph's width and left sidebearing.
-        """
-        self._glyphset = glyphset
-        self._glyph = glyph
-        if horizontalMetrics:
-            self.width, self.lsb = horizontalMetrics
-        else:
-            self.width, self.lsb = None, None
-        if verticalMetrics:
-            self.height, self.tsb = verticalMetrics
         else:
             self.height, self.tsb = None, None
+        if glyphSet.location and glyphSet.hvarTable is not None:
+            varidx = (
+                glyphSet.font.getGlyphID(glyphName)
+                if glyphSet.hvarTable.AdvWidthMap is None
+                else glyphSet.hvarTable.AdvWidthMap.mapping[glyphName]
+            )
+            self.width += glyphSet.hvarInstancer[varidx]
+        # TODO: VVAR/VORG
 
+
+class _TTGlyphGlyf(_TTGlyph):
     def draw(self, pen):
         """Draw the glyph onto ``pen``. See fontTools.pens.basePen for details
         how that works.
         """
-        self._glyph.draw(pen)
+        glyph, offset = self._getGlyphAndOffset()
+        glyph.draw(pen, offset)
 
     def drawPoints(self, pen):
+        """Draw the glyph onto ``pen``. See fontTools.pens.pointPen for details
+        how that works.
+        """
+        glyph, offset = self._getGlyphAndOffset()
+        glyph.drawPoints(pen, offset)
+
+    def _getGlyphAndOffset(self):
+        if self.glyphSet.location and self.glyphSet.gvarTable is not None:
+            glyph = self._getGlyphInstance()
+        else:
+            glyph = self.glyphSet.glyfTable[self.name]
+
+        offset = self.lsb - glyph.xMin if hasattr(glyph, "xMin") else 0
+        return glyph, offset
+
+    def _getGlyphInstance(self):
+        from fontTools.varLib.iup import iup_delta
+        from fontTools.ttLib.tables._g_l_y_f import GlyphCoordinates
+        from fontTools.varLib.models import supportScalar
+
+        glyphSet = self.glyphSet
+        glyfTable = glyphSet.glyfTable
+        variations = glyphSet.gvarTable.variations[self.name]
+        hMetrics = glyphSet.hMetrics
+        vMetrics = glyphSet.vMetrics
+        coordinates, _ = glyfTable._getCoordinatesAndControls(
+            self.name, hMetrics, vMetrics
+        )
+        origCoords, endPts = None, None
+        for var in variations:
+            scalar = supportScalar(glyphSet.location, var.axes)
+            if not scalar:
+                continue
+            delta = var.coordinates
+            if None in delta:
+                if origCoords is None:
+                    origCoords, control = glyfTable._getCoordinatesAndControls(
+                        self.name, hMetrics, vMetrics
+                    )
+                    endPts = (
+                        control[1] if control[0] >= 1 else list(range(len(control[1])))
+                    )
+                delta = iup_delta(delta, origCoords, endPts)
+            coordinates += GlyphCoordinates(delta) * scalar
+
+        glyph = copy(glyfTable[self.name])  # Shallow copy
+        width, lsb, height, tsb = _setCoordinates(glyph, coordinates, glyfTable)
+        if glyphSet.hvarTable is None:
+            # no HVAR: let's set metrics from the phantom points
+            self.width = width
+            self.lsb = lsb
+            self.height = height
+            self.tsb = tsb
+        return glyph
+
+
+class _TTGlyphCFF(_TTGlyph):
+    def draw(self, pen):
+        """Draw the glyph onto ``pen``. See fontTools.pens.basePen for details
+        how that works.
+        """
+        self.glyphSet.charStrings[self.name].draw(pen, self.glyphSet.blender)
+
+    def drawPoints(self, pen):
+        """Draw the glyph onto ``pen``. See fontTools.pens.pointPen for details
+        how that works.
+        """
         from fontTools.pens.pointPen import SegmentToPointPen
 
         self.draw(SegmentToPointPen(pen))
 
 
-class _TTGlyphCFF(_TTGlyph):
+def _normalizeLocation(font, location, normalized):
-    pass
+    if location is None or "fvar" not in font:
+        return None
+    if not normalized:
+        from fontTools.varLib.models import normalizeLocation, piecewiseLinearMap
 
-
+        axes = {
-class _TTGlyphGlyf(_TTGlyph):
+            a.axisTag: (a.minValue, a.defaultValue, a.maxValue)
-    def draw(self, pen):
+            for a in font["fvar"].axes
-        """Draw the glyph onto Pen. See fontTools.pens.basePen for details
+        }
-        how that works.
+        location = normalizeLocation(location, axes)
-        """
+        if "avar" in font:
-        glyfTable = self._glyphset._glyphs
+            avar = font["avar"]
-        glyph = self._glyph
+            avarSegments = avar.segments
-        offset = self.lsb - glyph.xMin if hasattr(glyph, "xMin") else 0
+            mappedLocation = {}
-        glyph.draw(pen, glyfTable, offset)
+            for axisTag, value in location.items():
-
+                avarMapping = avarSegments.get(axisTag, None)
-    def drawPoints(self, pen):
+                if avarMapping is not None:
-        """Draw the glyph onto PointPen. See fontTools.pens.pointPen
+                    value = piecewiseLinearMap(value, avarMapping)
-        for details how that works.
+                mappedLocation[axisTag] = value
-        """
+            location = mappedLocation
-        glyfTable = self._glyphset._glyphs
+    return location
-        glyph = self._glyph
-        offset = self.lsb - glyph.xMin if hasattr(glyph, "xMin") else 0
-        glyph.drawPoints(pen, glyfTable, offset)
-
-
-class _TTVarGlyphSet(_TTGlyphSet):
-    def __init__(self, font, glyphs, glyphType, location, normalized):
-        self._ttFont = font
-        self._glyphs = glyphs
-        self._glyphType = glyphType
-
-        if not normalized:
-            from fontTools.varLib.models import normalizeLocation, piecewiseLinearMap
-
-            axes = {
-                a.axisTag: (a.minValue, a.defaultValue, a.maxValue)
-                for a in font["fvar"].axes
-            }
-            location = normalizeLocation(location, axes)
-            if "avar" in font:
-                avar = font["avar"]
-                avarSegments = avar.segments
-                new_location = {}
-                for axis_tag, value in location.items():
-                    avarMapping = avarSegments.get(axis_tag, None)
-                    if avarMapping is not None:
-                        value = piecewiseLinearMap(value, avarMapping)
-                    new_location[axis_tag] = value
-                location = new_location
-                del new_location
-
-        self.location = location
-
-    def __getitem__(self, glyphName):
-        if glyphName not in self._glyphs:
-            raise KeyError(glyphName)
-        return self._glyphType(self, glyphName, self.location)
 
 
 def _setCoordinates(glyph, coord, glyfTable):
     # Handle phantom points for (left, right, top, bottom) positions.
     assert len(coord) >= 4
-    if not hasattr(glyph, "xMin"):
-        glyph.recalcBounds(glyfTable)
     leftSideX = coord[-4][0]
     rightSideX = coord[-3][0]
     topSideY = coord[-2][1]
@@ -163,7 +225,7 @@ def _setCoordinates(glyph, coord, glyfTable):
 
     if glyph.isComposite():
         assert len(coord) == len(glyph.components)
-        glyph.components = [copy(comp) for comp in glyph.components]
+        glyph.components = [copy(comp) for comp in glyph.components]  # Shallow copy
         for p, comp in zip(coord, glyph.components):
             if hasattr(comp, "x"):
                 comp.x, comp.y = p
@@ -185,94 +247,3 @@ def _setCoordinates(glyph, coord, glyfTable):
         verticalAdvanceWidth,
         topSideBearing,
     )
-
-
-class _TTVarGlyph(_TTGlyph):
-    def __init__(self, glyphSet, glyphName, location):
-
-        super().__init__(glyphSet._glyphs, glyphSet._glyphs[glyphName])
-
-        self._glyphSet = glyphSet
-        self._ttFont = glyphSet._ttFont
-        self._glyphs = glyphSet._glyphs
-        self._glyphName = glyphName
-        self._location = location
-
-
-class _TTVarGlyphCFF(_TTVarGlyph):
-    def draw(self, pen):
-        varStore = self._glyphs.varStore
-        if varStore is None:
-            blender = None
-        else:
-            from fontTools.varLib.varStore import VarStoreInstancer
-
-            vsInstancer = getattr(self._glyphSet, "vsInstancer", None)
-            if vsInstancer is None:
-                self._glyphSet.vsInstancer = vsInstancer = VarStoreInstancer(
-                    varStore.otVarStore, self._ttFont["fvar"].axes, self._location
-                )
-            blender = vsInstancer.interpolateFromDeltas
-        self._glyph.draw(pen, blender)
-
-        self.width = self._ttFont["hmtx"][self._glyphName][0]
-        if "HVAR" in self._ttFont:
-            hvar = self._ttFont["HVAR"].table
-            varidx = self._ttFont.getGlyphID(self._glyphName)
-            if hvar.AdvWidthMap is not None:
-                varidx = hvar.AdvWidthMap.mapping[self._glyphName]
-            vsInstancer = VarStoreInstancer(
-                hvar.VarStore, self._ttFont["fvar"].axes, self._location
-            )
-            delta = vsInstancer[varidx]
-            self.width += delta
-
-
-class _TTVarGlyphGlyf(_TTVarGlyph):
-    def draw(self, pen):
-        self._drawWithPen(pen, isPointPen=False)
-
-    def drawPoints(self, pen):
-        self._drawWithPen(pen, isPointPen=True)
-
-    def _drawWithPen(self, pen, isPointPen):
-        from fontTools.varLib.iup import iup_delta
-        from fontTools.ttLib.tables._g_l_y_f import GlyphCoordinates
-        from fontTools.varLib.models import supportScalar
-
-        glyf = self._ttFont["glyf"]
-        hMetrics = self._ttFont["hmtx"].metrics
-        vMetrics = getattr(self._ttFont.get("vmtx"), "metrics", None)
-
-        variations = self._ttFont["gvar"].variations[self._glyphName]
-        coordinates, _ = glyf._getCoordinatesAndControls(
-            self._glyphName, hMetrics, vMetrics
-        )
-        origCoords, endPts = None, None
-        for var in variations:
-            scalar = supportScalar(self._location, var.axes)
-            if not scalar:
-                continue
-            delta = var.coordinates
-            if None in delta:
-                if origCoords is None:
-                    origCoords, control = glyf._getCoordinatesAndControls(
-                        self._glyphName, hMetrics, vMetrics
-                    )
-                    endPts = (
-                        control[1] if control[0] >= 1 else list(range(len(control[1])))
-                    )
-                delta = iup_delta(delta, origCoords, endPts)
-            coordinates += GlyphCoordinates(delta) * scalar
-
-        glyph = copy(glyf[self._glyphName])  # Shallow copy
-        width, lsb, height, tsb = _setCoordinates(glyph, coordinates, glyf)
-        self.width = width
-        self.lsb = lsb
-        self.height = height
-        self.tsb = tsb
-        offset = lsb - glyph.xMin if hasattr(glyph, "xMin") else 0
-        if isPointPen:
-            glyph.drawPoints(pen, glyf, offset)
-        else:
-            glyph.draw(pen, glyf, offset)

Tests/pens/ttGlyphPen_test.py

@@ -26,7 +26,7 @@ class TTGlyphPenTestBase:
         for name in font.getGlyphOrder():
             oldGlyph = glyphSet[name]
             getattr(oldGlyph, self.drawMethod)(pen)
-            oldGlyph = oldGlyph._glyph
+            oldGlyph = glyfTable[name]
             newGlyph = pen.glyph()
 
             if hasattr(oldGlyph, "program"):

Tests/ttLib/ttGlyphSet_test.py

@@ -119,8 +119,6 @@ class TTGlyphSetTest(object):
         glyphset = font.getGlyphSet(location=location)
 
         assert isinstance(glyphset, ttGlyphSet._TTGlyphSet)
-        if location:
-            assert isinstance(glyphset, ttGlyphSet._TTVarGlyphSet)
 
         assert list(glyphset.keys()) == [".notdef", "I"]
 
@@ -136,12 +134,7 @@ class TTGlyphSetTest(object):
 
         assert isinstance(glyph, ttGlyphSet._TTGlyph)
         is_glyf = fontfile.endswith(".ttf")
-        if location:
+        glyphType = ttGlyphSet._TTGlyphGlyf if is_glyf else ttGlyphSet._TTGlyphCFF
-            glyphType = (
-                ttGlyphSet._TTVarGlyphGlyf if is_glyf else ttGlyphSet._TTVarGlyphCFF
-            )
-        else:
-            glyphType = ttGlyphSet._TTGlyphGlyf if is_glyf else ttGlyphSet._TTGlyphCFF
         assert isinstance(glyph, glyphType)
 
         glyph.draw(pen)