From 4f1102ac6ef1033a5bc8d1c080f89bbf79d472db Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 15 Jan 2021 16:59:17 +0000 Subject: [PATCH] add a Circle class, handle concentrical case, explain why 2 iterations are enough --- Lib/fontTools/colorLib/builder.py | 5 +- Lib/fontTools/colorLib/geometry.py | 101 ++++++++++++++++++++--------- Tests/colorLib/builder_test.py | 43 ++++++------ 3 files changed, 94 insertions(+), 55 deletions(-) diff --git a/Lib/fontTools/colorLib/builder.py b/Lib/fontTools/colorLib/builder.py index 5cbe8d53d..c0329abcc 100644 --- a/Lib/fontTools/colorLib/builder.py +++ b/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 diff --git a/Lib/fontTools/colorLib/geometry.py b/Lib/fontTools/colorLib/geometry.py index cf83b67d0..c23a84126 100644 --- a/Lib/fontTools/colorLib/geometry.py +++ b/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) - rc0, rr0 = _round_circle(c0, r0) - rc1, rr1 = _round_circle(c1, r1) - inside_after_round = _is_circle_inside_circle(rc0, rr0, rc1, rr1) + start_circle, end_circle = Circle(c0, r0), Circle(c1, r1) - 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 + inside_before_round = start_circle.inside(end_circle) + + round_start = start_circle.round() + round_end = end_circle.round() + + # 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) - 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!" - ) + round_start.radius -= 1.0 + else: + if inside_after_round: + direction = _vector_between(round_end.centre, round_start.centre) + else: + direction = _vector_between(round_start.centre, round_end.centre) + round_start.nudge_towards(direction) + else: # likely a bug + raise AssertionError( + 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 diff --git a/Tests/colorLib/builder_test.py b/Tests/colorLib/builder_test.py index f8eaea6e5..d4376f455 100644 --- a/Tests/colorLib/builder_test.py +++ b/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 ( - nudge_start_circle_almost_inside, - _is_circle_inside_circle, - _round_circle, -) +from fontTools.colorLib.geometry import nudge_start_circle_almost_inside, 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( - *_round_circle(c0, r0), *_round_circle(c1, r1) - ) + return Circle(c0, r0).round().inside(Circle(c1, r1).round()) 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) - assert self.circle_inside_circle(c0, r0, c1, r1, rounded=True) is inside - return c0 + r = nudge_start_circle_almost_inside(c0, r0, c1, r1) + assert ( + 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) - assert rc0 == (386, 71) + result = self.nudge_start_circle_position(c0, r0, c1, r1, inside=True) + 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, 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)), + ((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)), + ((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.5), 0, (0, -2.5), 1.5, False, (0, 1)), + ((0, 0), 0, (2, 0), 1.5, False, ((-1, 0), 0)), + ((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)), - ((1.5, 1.5), 0, (0.49, 0.49), 1.49, True, (0, 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), 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):