COLRv1: avoid abrupt change after rounding c0 when too near c1's perimeter

Fixes https://github.com/googlefonts/picosvg/issues/158

Also see https://github.com/googlefonts/colr-gradients-spec/issues/204

Lib/fontTools/colorLib/builder.py

@@ -34,6 +34,7 @@ from fontTools.ttLib.tables.otTables import (
     VariableInt,
 )
 from .errors import ColorLibError
+from .geometry import nudge_start_circle_almost_inside
 
 
 # TODO move type aliases to colorLib.types?
@@ -328,9 +329,9 @@ def _split_color_glyphs_by_version(
 
 def _to_variable_value(
     value: _ScalarInput,
-    minValue: _Number,
+    cls: Type[VariableValue] = VariableFloat,
-    maxValue: _Number,
+    minValue: Optional[_Number] = None,
-    cls: Type[VariableValue],
+    maxValue: Optional[_Number] = None,
 ) -> VariableValue:
     if not isinstance(value, cls):
         try:
@@ -339,9 +340,9 @@ def _to_variable_value(
             value = cls(value)
         else:
             value = cls._make(it)
-    if value.value < minValue:
+    if minValue is not None and value.value < minValue:
         raise OverflowError(f"{cls.__name__}: {value.value} < {minValue}")
-    if value.value > maxValue:
+    if maxValue is not None and value.value > maxValue:
         raise OverflowError(f"{cls.__name__}: {value.value} < {maxValue}")
     return value
 
@@ -526,7 +527,20 @@ class LayerV1ListBuilder:
         ot_paint.Format = int(ot.Paint.Format.PaintRadialGradient)
         ot_paint.ColorLine = _to_color_line(colorLine)
 
-        for i, (x, y), r in [(0, c0, r0), (1, c1, r1)]:
+        # normalize input types (which may or may not specify a varIdx)
+        x0, y0 = _to_variable_value(c0[0]), _to_variable_value(c0[1])
+        r0 = _to_variable_value(r0)
+        x1, y1 = _to_variable_value(c1[0]), _to_variable_value(c1[1])
+        r1 = _to_variable_value(r1)
+
+        # avoid abrupt change after rounding when c0 is near c1's perimeter
+        c0x, c0y = 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)
+
+        for i, (x, y, r) in enumerate(((x0, y0, r0), (x1, y1, r1))):
+            # rounding happens here as floats are converted to integers
             setattr(ot_paint, f"x{i}", _to_variable_int16(x))
             setattr(ot_paint, f"y{i}", _to_variable_int16(y))
             setattr(ot_paint, f"r{i}", _to_variable_uint16(r))

Lib/fontTools/colorLib/geometry.py

@@ -0,0 +1,110 @@
+"""Helpers for manipulating 2D points and vectors in COLR table."""
+
+from math import copysign, cos, sqrt, pi
+from fontTools.misc.fixedTools import otRound
+
+
+def _vector_between_points(a, b):
+    return (b[0] - a[0], b[1] - a[1])
+
+
+def _vector_length(vec):
+    return sqrt(vec[0] * vec[0] + vec[1] * vec[1])
+
+
+def _distance_between_points(a, b):
+    return _vector_length(_vector_between_points(a, b))
+
+
+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 = _vector_length(vec)
+    if length == 0:
+        return None
+    return (vec[0] / length, vec[1] / length)
+
+
+# This is the same tolerance used by Skia's SkTwoPointConicalGradient.cpp to detect
+# when a radial gradient's focal point lies on the end circle.
+_NEARLY_ZERO = 1 / (1 << 12)  # 0.000244140625
+
+
+def _is_circle_inside_circle(c0, r0, c1, r1):
+    dist = r0 + _distance_between_points(c0, c1)
+    return abs(r1 - dist) <= _NEARLY_ZERO or r1 > 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.
+_UNIT_VECTOR_THRESHOLD = cos(3 / 8 * pi)  # == sin(1/8 * pi) == 0.38268343236508984
+
+
+def _nudge_point(pt, direction):
+    # Nudge point coordinates -/+ 1.0 approximately based on the direction vector.
+    # We divide the unit circle in 8 equal slices oriented towards the cardinal
+    # (N, E, S, W) and intermediate (NE, SE, SW, NW) directions. To each slice we
+    # map one of the possible cases: -1, 0, +1 for either X and Y coordinate.
+    # E.g. Return (x + 1.0, y - 1.0) if unit vector is oriented towards SE, or
+    # (x - 1.0, y) if it's pointing West, etc.
+    uv = _unit_vector(direction)
+    if not uv:
+        return pt
+
+    result = []
+    for coord, uv_component in zip(pt, uv):
+        if -_UNIT_VECTOR_THRESHOLD <= uv_component < _UNIT_VECTOR_THRESHOLD:
+            # unit vector component near 0: direction almost orthogonal to the
+            # direction of the current axis, thus keep coordinate unchanged
+            result.append(coord)
+        else:
+            # nudge coord by +/- 1.0 in direction of unit vector
+            result.append(coord + copysign(1.0, uv_component))
+    return tuple(result)
+
+
+def nudge_start_circle_almost_inside(c0, r0, c1, r1):
+    """ 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
+    it ends up falling outside (or inside) as a result of the rounding.
+    To keep the gradient unchanged, we nudge it in the right direction.
+
+    See:
+    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)
+    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:
+        # at most 2 iterations ought to be enough to converge
+        for _ in range(2):
+            if rc0 == rc1:  # nowhere to nudge along a zero vector, bail out
+                break
+            if inside_after_round:
+                direction = _vector_between_points(rc1, rc0)
+            else:
+                direction = _vector_between_points(rc0, rc1)
+            rc0 = _nudge_point(rc0, direction)
+            inside_after_round = _is_circle_inside_circle(rc0, rr0, rc1, rr1)
+            if inside_before_round == inside_after_round:
+                break
+        else:  # ... or it's a bug
+            raise AssertionError(
+                f"Nudging circle <c0={c0}, r0={r0}> "
+                f"{'inside' if inside_before_round else 'outside'} "
+                f"<c1={c1}, r1={r1}> failed after two attempts!"
+            )
+
+    return rc0

Tests/colorLib/builder_test.py

@@ -1,6 +1,11 @@
 from fontTools.ttLib import newTable
 from fontTools.ttLib.tables import otTables as ot
 from fontTools.colorLib import builder
+from fontTools.colorLib.geometry import (
+    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
@@ -1055,3 +1060,48 @@ class BuildCOLRTest(object):
         assert hasattr(colr, "table")
         assert isinstance(colr.table, ot.COLR)
         assert colr.table.VarStore is None
+
+
+class TrickyRadialGradientTest:
+    @staticmethod
+    def circle_inside_circle(c0, r0, c1, r1, rounded=False):
+        if rounded:
+            return _is_circle_inside_circle(
+                *_round_circle(c0, r0), *_round_circle(c1, r1)
+            )
+        else:
+            return _is_circle_inside_circle(c0, r0, 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)
+        assert self.circle_inside_circle(c0, r0, c1, r1, rounded=True) is inside
+        return c0
+
+    def test_noto_emoji_mosquito_u1f99f(self):
+        # https://github.com/googlefonts/picosvg/issues/158
+        c0 = (385.23508, 70.56727999999998)
+        r0 = 0
+        c1 = (642.99108, 104.70327999999995)
+        r1 = 260.0072
+        rc0 = self.nudge_start_circle_position(c0, r0, c1, r1, inside=True)
+        assert rc0 == (386, 71)
+
+    @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, 0), 0.6, (2.8, 0), 2.45, True, (2, 0)),
+            ((6.49, 6.49), 0, (0.49, 0.49), 8.49, True, (5, 5)),
+            # outside before round, inside after round
+            ((0, 0), 0, (2, 0), 1.5, False, (-1, 0)),
+            ((0, -0.5), 0, (0, -2.5), 1.5, False, (0, 1)),
+            # the following ones require two nudges to round correctly
+            ((0.5, 0), 0, (9.4, 0), 8.8, False, (-1, 0)),
+            ((1.5, 1.5), 0, (0.49, 0.49), 1.49, True, (0, 0)),
+        ],
+    )
+    def test_nudge_start_circle_position(self, c0, r0, c1, r1, inside, expected):
+        assert self.nudge_start_circle_position(c0, r0, c1, r1, inside) == expected