Merge pull request #3752 from fonttools/nested-components-bounds

[recalcBounds] fix rounding of bbox for nested transformed components

Lib/fontTools/ttLib/tables/_g_l_y_f.py

@@ -1206,9 +1206,7 @@ class Glyph(object):
         Return True if bounds were calculated, False otherwise.
         """
         for compo in self.components:
-            if hasattr(compo, "firstPt") or hasattr(compo, "transform"):
-                return False
-            if not float(compo.x).is_integer() or not float(compo.y).is_integer():
+            if not compo._hasOnlyIntegerTranslate():
                 return False
 
         # All components are untransformed and have an integer x/y translate
@@ -1241,7 +1239,7 @@ class Glyph(object):
         else:
             return self.numberOfContours == -1
 
-    def getCoordinates(self, glyfTable, round=noRound):
+    def getCoordinates(self, glyfTable, *, round=noRound):
         """Return the coordinates, end points and flags
 
         This method returns three values: A :py:class:`GlyphCoordinates` object,
@@ -1276,7 +1274,18 @@ class Glyph(object):
                         % compo.glyphName
                     )
                 coordinates = GlyphCoordinates(coordinates)
-                coordinates.toInt(round=round)
+                # if asked to round e.g. while computing bboxes, it's important we
+                # do it immediately before a component transform is applied to a
+                # simple glyph's coordinates in case these might still contain floats;
+                # however, if the referenced component glyph is another composite, we
+                # must not round here but only at the end, after all the nested
+                # transforms have been applied, or else rounding errors will compound.
+                if (
+                    round is not noRound
+                    and g.numberOfContours > 0
+                    and not compo._hasOnlyIntegerTranslate()
+                ):
+                    coordinates.toInt(round=round)
                 if hasattr(compo, "firstPt"):
                     # component uses two reference points: we apply the transform _before_
                     # computing the offset between the points
@@ -1933,6 +1942,18 @@ class GlyphComponent(object):
         result = self.__eq__(other)
         return result if result is NotImplemented else not result
 
+    def _hasOnlyIntegerTranslate(self):
+        """Return True if it's a 'simple' component.
+
+        That is, it has no anchor points and no transform other than integer translate.
+        """
+        return (
+            not hasattr(self, "firstPt")
+            and not hasattr(self, "transform")
+            and float(self.x).is_integer()
+            and float(self.y).is_integer()
+        )
+
 
 class GlyphCoordinates(object):
     """A list of glyph coordinates.

Tests/pens/ttGlyphPen_test.py

@@ -614,6 +614,52 @@ class TTGlyphPointPenTest(TTGlyphPenTestBase):
         # these are the expected bounds as computed from the rounded coordinates
         self.assertGlyphBoundsEqual(compositeGlyph, (74, 680, 329, 834))
 
+    def test_composite_bounds_with_nested_components(self):
+        # The following test case is taken from Joan.glyphs at
+        # https://github.com/PaoloBiagini/Joan
+        # There's a composite glyph 'bullet.sc' which contains a transformed
+        # component 'bullet', which in turn is a composite glyph referencing
+        # another transformed components ('period'). The two transformations
+        # combine to produce the final transformed coordinates, but the necessary
+        # rounding should only happen at the end, and not at each intermediate
+        # level of the nested component tree.
+        glyphSet = {}
+        pen = TTGlyphPointPen(glyphSet)
+
+        pen.beginPath()
+        pen.addPoint((141.0, -8.0), "qcurve")
+        pen.addPoint((168.0, -8.0), None)
+        pen.addPoint((205.0, 30.5), None)
+        pen.addPoint((205.0, 56.0), "qcurve")
+        pen.addPoint((205.0, 82.25), None)
+        pen.addPoint((168.0, 118.0), None)
+        pen.addPoint((141.0, 118.0), "qcurve")
+        pen.addPoint((114.0, 118.0), None)
+        pen.addPoint((78.0, 79.5), None)
+        pen.addPoint((78.0, 54.0), "qcurve")
+        pen.addPoint((78.0, 27.75), None)
+        pen.addPoint((114.0, -8.0), None)
+        pen.endPath()
+        glyphSet["period"] = pen.glyph()
+
+        pen.addComponent("period", (1.6, 0, 0, 1.6, -41, 140))
+        glyphSet["bullet"] = pen.glyph()
+
+        pen.addComponent("bullet", (0.9, 0, 0, 0.9, 9, 49))
+        compositeGlyph = glyphSet["bullet.sc"] = pen.glyph()
+
+        # this is the xMin of 'bullet.sc' glyph if coordinates were left unrounded
+        coords, _, _ = compositeGlyph.getCoordinates(glyphSet)
+        assert pytest.approx(min(x for x, y in coords)) == 84.42033
+
+        compositeGlyph.recalcBounds(glyphSet)
+
+        # if rounding happened at intermediate stages (i.e. after extracting transformed
+        # coordinates of 'period' component from 'bullet', before transforming 'bullet'
+        # component in 'bullet.sc'), the rounding errors would compound leading to xMin
+        # incorrectly be equal to 85. Whereas 84.42033 should round down to 84.
+        self.assertGlyphBoundsEqual(compositeGlyph, (84, 163, 267, 345))
+
     def test_open_path_starting_with_move(self):
         # when a contour starts with a 'move' point, it signifies the beginnig
         # of an open contour.