From db14e6375ebeef170d757e201f0f25aa2d9d2db3 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 14 Jan 2021 17:13:22 +0000 Subject: [PATCH] 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 | 26 +++++-- Lib/fontTools/colorLib/geometry.py | 110 +++++++++++++++++++++++++++++ Tests/colorLib/builder_test.py | 50 +++++++++++++ 3 files changed, 180 insertions(+), 6 deletions(-) create mode 100644 Lib/fontTools/colorLib/geometry.py diff --git a/Lib/fontTools/colorLib/builder.py b/Lib/fontTools/colorLib/builder.py index 724136ab1..5cbe8d53d 100644 --- a/Lib/fontTools/colorLib/builder.py +++ b/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, - maxValue: _Number, - cls: Type[VariableValue], + cls: Type[VariableValue] = VariableFloat, + minValue: Optional[_Number] = None, + 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)) diff --git a/Lib/fontTools/colorLib/geometry.py b/Lib/fontTools/colorLib/geometry.py new file mode 100644 index 000000000..2fbd68882 --- /dev/null +++ b/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 " + f"{'inside' if inside_before_round else 'outside'} " + f" failed after two attempts!" + ) + + return rc0 diff --git a/Tests/colorLib/builder_test.py b/Tests/colorLib/builder_test.py index d1e94df93..f8eaea6e5 100644 --- a/Tests/colorLib/builder_test.py +++ b/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