add a Circle class, handle concentrical case, explain why 2 iterations are enough

Lib/fontTools/colorLib/builder.py

@@ -534,10 +534,11 @@ class LayerV1ListBuilder:
         r1 = _to_variable_value(r1)
 
         # avoid abrupt change after rounding when c0 is near c1's perimeter
-        c0x, c0y = nudge_start_circle_almost_inside(
+        c = nudge_start_circle_almost_inside(
             (x0.value, y0.value), r0.value, (x1.value, y1.value), r1.value
         )
-        x0, y0 = x0._replace(value=c0x), y0._replace(value=c0y)
+        x0, y0 = x0._replace(value=c.centre[0]), y0._replace(value=c.centre[1])
+        r0 = r0._replace(value=c.radius)
 
         for i, (x, y, r) in enumerate(((x0, y0, r0), (x1, y1, r1))):
             # rounding happens here as floats are converted to integers

Lib/fontTools/colorLib/geometry.py

@@ -12,10 +12,6 @@ def _round_point(pt):
     return (otRound(pt[0]), otRound(pt[1]))
 
 
-def _round_circle(centre, radius):
-    return _round_point(centre), otRound(radius)
-
-
 def _unit_vector(vec):
     length = hypot(*vec)
     if length == 0:
@@ -28,11 +24,6 @@ def _unit_vector(vec):
 _NEARLY_ZERO = 1 / (1 << 12)  # 0.000244140625
 
 
-def _is_circle_inside_circle(inner_centre, inner_radius, outer_centre, outer_radius):
-    dist = inner_radius + hypot(*_vector_between(inner_centre, outer_centre))
-    return abs(outer_radius - dist) <= _NEARLY_ZERO or outer_radius > dist
-
-
 # The unit vector's X and Y components are respectively
 #   U = (cos(α), sin(α))
 # where α is the angle between the unit vector and the positive x axis.
@@ -62,8 +53,33 @@ def _nudge_point(pt, direction):
     return tuple(result)
 
 
+class Circle:
+    def __init__(self, centre, radius):
+        self.centre = centre
+        self.radius = radius
+
+    def __repr__(self):
+        return f"Circle(centre={self.centre}, radius={self.radius})"
+
+    def round(self):
+        return Circle(_round_point(self.centre), otRound(self.radius))
+
+    def inside(self, outer_circle):
+        dist = self.radius + hypot(*_vector_between(self.centre, outer_circle.centre))
+        return (
+            abs(outer_circle.radius - dist) <= _NEARLY_ZERO
+            or outer_circle.radius > dist
+        )
+
+    def concentric(self, other):
+        return self.centre == other.centre
+
+    def nudge_towards(self, direction):
+        self.centre = _nudge_point(self.centre, direction)
+
+
 def nudge_start_circle_almost_inside(c0, r0, c1, r1):
-    """ Nudge c0 so it continues to be inside/outside c1 after rounding.
+    """Nudge c0 so it continues to be inside/outside c1 after rounding.
 
     The rounding of circle coordinates to integers may cause an abrupt change
     if the start circle c0 is so close to the end circle c1's perimiter that
@@ -74,29 +90,50 @@ def nudge_start_circle_almost_inside(c0, r0, c1, r1):
     https://github.com/googlefonts/colr-gradients-spec/issues/204
     https://github.com/googlefonts/picosvg/issues/158
     """
-    inside_before_round = _is_circle_inside_circle(c0, r0, c1, r1)
+    start_circle, end_circle = Circle(c0, r0), Circle(c1, r1)
-    rc0, rr0 = _round_circle(c0, r0)
-    rc1, rr1 = _round_circle(c1, r1)
-    inside_after_round = _is_circle_inside_circle(rc0, rr0, rc1, rr1)
 
-    if inside_before_round != inside_after_round:
+    inside_before_round = start_circle.inside(end_circle)
-        # at most 2 iterations ought to be enough to converge
+
-        for _ in range(2):
+    round_start = start_circle.round()
-            if rc0 == rc1:  # nowhere to nudge along a zero vector, bail out
+    round_end = end_circle.round()
-                break
+
+    # At most 3 iterations ought to be enough to converge. In the first, we
+    # check if the start circle keeps containment after normal rounding; then
+    # we continue adjusting by -/+ 1.0 until containment is restored.
+    # Normal rounding can at most move each coordinates -/+0.5; in the worst case
+    # both the start and end circle's centres and radii will be rounded in opposite
+    # directions, e.g. when they move along a 45 degree diagonal:
+    #   c0 = (1.5, 1.5) ===> (2.0, 2.0)
+    #   r0 = 0.5 ===> 1.0
+    #   c1 = (0.499, 0.499) ===> (0.0, 0.0)
+    #   r1 = 2.499 ===> 2.0
+    # In this example, the relative distance between the circles, calculated
+    # as r1 - (r0 + distance(c0, c1)) is initially 0.57437 (c0 is inside c1), and
+    # -1.82842 after rounding (c0 is now outside c1). Nudging c0 by -1.0 on both
+    # x and y axes moves it towards c1 by hypot(-1.0, -1.0) = 1.41421. Two of these
+    # moves cover twice that distance, which is enough to restore containment.
+    max_attempts = 3
+    for _ in range(max_attempts):
+        inside_after_round = round_start.inside(round_end)
+        if inside_before_round == inside_after_round:
+            break
+        if round_start.concentric(round_end):
+            # can't move c0 towards c1 (they are the same), so we change the radius
             if inside_after_round:
-                direction = _vector_between(rc1, rc0)
+                round_start.radius += 1.0
             else:
-                direction = _vector_between(rc0, rc1)
+                round_start.radius -= 1.0
-            rc0 = _nudge_point(rc0, direction)
+        else:
-            inside_after_round = _is_circle_inside_circle(rc0, rr0, rc1, rr1)
+            if inside_after_round:
-            if inside_before_round == inside_after_round:
+                direction = _vector_between(round_end.centre, round_start.centre)
-                break
+            else:
-        else:  # ... or it's a bug
+                direction = _vector_between(round_start.centre, round_end.centre)
-            raise AssertionError(
+            round_start.nudge_towards(direction)
-                f"Nudging circle <c0={c0}, r0={r0}> "
+    else:  # likely a bug
-                f"{'inside' if inside_before_round else 'outside'} "
+        raise AssertionError(
-                f"<c1={c1}, r1={r1}> failed after two attempts!"
+            f"Rounding circle {start_circle} "
-            )
+            f"{'inside' if inside_before_round else 'outside'} "
+            f"{end_circle} failed after {max_attempts} attempts!"
+        )
 
-    return rc0
+    return round_start

Tests/colorLib/builder_test.py

@@ -1,11 +1,7 @@
 from fontTools.ttLib import newTable
 from fontTools.ttLib.tables import otTables as ot
 from fontTools.colorLib import builder
-from fontTools.colorLib.geometry import (
+from fontTools.colorLib.geometry import nudge_start_circle_almost_inside, Circle
-    nudge_start_circle_almost_inside,
-    _is_circle_inside_circle,
-    _round_circle,
-)
 from fontTools.colorLib.builder import LayerV1ListBuilder
 from fontTools.colorLib.errors import ColorLibError
 import pytest
@@ -1066,18 +1062,19 @@ class TrickyRadialGradientTest:
     @staticmethod
     def circle_inside_circle(c0, r0, c1, r1, rounded=False):
         if rounded:
-            return _is_circle_inside_circle(
+            return Circle(c0, r0).round().inside(Circle(c1, r1).round())
-                *_round_circle(c0, r0), *_round_circle(c1, r1)
-            )
         else:
-            return _is_circle_inside_circle(c0, r0, c1, r1)
+            return Circle(c0, r0).inside(Circle(c1, r1))
 
     def nudge_start_circle_position(self, c0, r0, c1, r1, inside=True):
         assert self.circle_inside_circle(c0, r0, c1, r1) is inside
         assert self.circle_inside_circle(c0, r0, c1, r1, rounded=True) is not inside
-        c0 = nudge_start_circle_almost_inside(c0, r0, c1, r1)
+        r = nudge_start_circle_almost_inside(c0, r0, c1, r1)
-        assert self.circle_inside_circle(c0, r0, c1, r1, rounded=True) is inside
+        assert (
-        return c0
+            self.circle_inside_circle(r.centre, r.radius, c1, r1, rounded=True)
+            is inside
+        )
+        return r.centre, r.radius
 
     def test_noto_emoji_mosquito_u1f99f(self):
         # https://github.com/googlefonts/picosvg/issues/158
@@ -1085,22 +1082,26 @@ class TrickyRadialGradientTest:
         r0 = 0
         c1 = (642.99108, 104.70327999999995)
         r1 = 260.0072
-        rc0 = self.nudge_start_circle_position(c0, r0, c1, r1, inside=True)
+        result = self.nudge_start_circle_position(c0, r0, c1, r1, inside=True)
-        assert rc0 == (386, 71)
+        assert result == ((386, 71), 0)
 
     @pytest.mark.parametrize(
         "c0, r0, c1, r1, inside, expected",
         [
             # inside before round, outside after round
-            ((1.4, 0), 0, (2.6, 0), 1.3, True, (2, 0)),
+            ((1.4, 0), 0, (2.6, 0), 1.3, True, ((2, 0), 0)),
-            ((1, 0), 0.6, (2.8, 0), 2.45, True, (2, 0)),
+            ((1, 0), 0.6, (2.8, 0), 2.45, True, ((2, 0), 1)),
-            ((6.49, 6.49), 0, (0.49, 0.49), 8.49, True, (5, 5)),
+            ((6.49, 6.49), 0, (0.49, 0.49), 8.49, True, ((5, 5), 0)),
             # outside before round, inside after round
-            ((0, 0), 0, (2, 0), 1.5, False, (-1, 0)),
+            ((0, 0), 0, (2, 0), 1.5, False, ((-1, 0), 0)),
-            ((0, -0.5), 0, (0, -2.5), 1.5, False, (0, 1)),
+            ((0, -0.5), 0, (0, -2.5), 1.5, False, ((0, 1), 0)),
             # the following ones require two nudges to round correctly
-            ((0.5, 0), 0, (9.4, 0), 8.8, False, (-1, 0)),
+            ((0.5, 0), 0, (9.4, 0), 8.8, False, ((-1, 0), 0)),
-            ((1.5, 1.5), 0, (0.49, 0.49), 1.49, True, (0, 0)),
+            ((1.5, 1.5), 0, (0.49, 0.49), 1.49, True, ((0, 0), 0)),
+            # limit case when circle almost exactly overlap
+            ((0.5000001, 0), 0.5000001, (0.499999, 0), 0.4999999, True, ((0, 0), 0)),
+            # concentrical circles, r0 > r1
+            ((0, 0), 1.49, (0, 0), 1, False, ((0, 0), 2)),
         ],
     )
     def test_nudge_start_circle_position(self, c0, r0, c1, r1, inside, expected):