From c025be65164622b6a5feb74b4faf620adbcc8422 Mon Sep 17 00:00:00 2001 From: Abulafia <44573666+abul4fia@users.noreply.github.com> Date: Mon, 8 Apr 2024 18:46:43 +0200 Subject: [PATCH] Fix bug in :class:`.VMobjectFromSVGPath` (#3677) * Fixes #3676 * Update manim/mobject/svg/svg_mobject.py Co-authored-by: adeshpande <110117391+JasonGrace2282@users.noreply.github.com> * Fixed problem and added test --------- Co-authored-by: adeshpande <110117391+JasonGrace2282@users.noreply.github.com> --- manim/mobject/svg/svg_mobject.py | 18 +++-- tests/module/mobject/svg/test_svg_mobject.py | 68 +++++++++++++++++++ .../img_svg_resources/A.svg | 13 ++++ 3 files changed, 89 insertions(+), 10 deletions(-) create mode 100644 tests/test_graphical_units/img_svg_resources/A.svg diff --git a/manim/mobject/svg/svg_mobject.py b/manim/mobject/svg/svg_mobject.py index 87cad3bda0..c029daa942 100644 --- a/manim/mobject/svg/svg_mobject.py +++ b/manim/mobject/svg/svg_mobject.py @@ -510,17 +510,15 @@ def handle_commands(self) -> None: all_points: list[np.ndarray] = [] last_move = None curve_start = None + last_true_move = None - # These lambdas behave the same as similar functions in - # vectorized_mobject, except they add to a list of points instead - # of updating this Mobject's numpy array of points. This way, - # we don't observe O(n^2) behavior for complex paths due to - # numpy's need to re-allocate memory on every append. - def move_pen(pt): - nonlocal last_move, curve_start + def move_pen(pt, *, true_move: bool = False): + nonlocal last_move, curve_start, last_true_move last_move = pt if curve_start is None: curve_start = last_move + if true_move: + last_true_move = last_move if self.n_points_per_curve == 4: @@ -568,7 +566,7 @@ def add_line(start, end): for segment in self.path_obj: segment_class = segment.__class__ if segment_class == se.Move: - move_pen(_convert_point_to_3d(*segment.end)) + move_pen(_convert_point_to_3d(*segment.end), true_move=True) elif segment_class == se.Line: add_line(last_move, _convert_point_to_3d(*segment.end)) elif segment_class == se.QuadraticBezier: @@ -588,8 +586,8 @@ def add_line(start, end): # If the SVG path naturally ends at the beginning of the curve, # we do *not* need to draw a closing line. To account for floating # point precision, we use a small value to compare the two points. - if abs(np.linalg.norm(last_move - curve_start)) > 0.0001: - add_line(last_move, curve_start) + if abs(np.linalg.norm(last_move - last_true_move)) > 0.0001: + add_line(last_move, last_true_move) curve_start = None else: raise AssertionError(f"Not implemented: {segment_class}") diff --git a/tests/module/mobject/svg/test_svg_mobject.py b/tests/module/mobject/svg/test_svg_mobject.py index eba36fb3ab..1d8f6190ef 100644 --- a/tests/module/mobject/svg/test_svg_mobject.py +++ b/tests/module/mobject/svg/test_svg_mobject.py @@ -134,3 +134,71 @@ def test_closed_path_does_not_have_extra_point(): ), decimal=5, ) + + +def test_close_command_closes_last_move_not_the_starting_one(): + # This A.svg is the output of a Text("A") in some systems + # It contains a path that moves from the outer boundary of the A + # to the boundary of the inner triangle, anc then closes the path + # which should close the inner triangle and not the outer boundary. + svg = SVGMobject( + get_svg_resource("A.svg"), + ) + assert len(svg.points) == 0, svg.points + assert len(svg.submobjects) == 1, svg.submobjects + capital_A = svg.submobjects[0] + + # The last point should not be the same as the first point + assert not all(capital_A.points[0] == capital_A.points[-1]) + np.testing.assert_almost_equal( + capital_A.points, + np.array( + [ + [-0.8380339075214888, -1.0, 1.2246467991473532e-16], + [-0.6132152047642527, -0.3333333333333336, 4.082155997157847e-17], + [-0.388396502007016, 0.3333333333333336, -4.082155997157847e-17], + [-0.16357779924977994, 1.0, -1.2246467991473532e-16], + [-0.16357779924977994, 1.0, -1.2246467991473532e-16], + [-0.05425733591657368, 1.0, -1.2246467991473532e-16], + [0.05506312741663405, 1.0, -1.2246467991473532e-16], + [0.16438359074984032, 1.0, -1.2246467991473532e-16], + [0.16438359074984032, 1.0, -1.2246467991473532e-16], + [0.3889336963403905, 0.3333333333333336, -4.082155997157847e-17], + [0.6134838019309422, -0.3333333333333336, 4.082155997157847e-17], + [0.8380339075214923, -1.0, 1.2246467991473532e-16], + [0.8380339075214923, -1.0, 1.2246467991473532e-16], + [0.744560897060354, -1.0, 1.2246467991473532e-16], + [0.6510878865992157, -1.0, 1.2246467991473532e-16], + [0.5576148761380774, -1.0, 1.2246467991473532e-16], + [0.5576148761380774, -1.0, 1.2246467991473532e-16], + [0.49717968849274957, -0.8138597980824822, 9.966907966764229e-17], + [0.4367445008474217, -0.6277195961649644, 7.687347942054928e-17], + [0.3763093132020939, -0.4415793942474466, 5.407787917345625e-17], + [0.3763093132020939, -0.4415793942474466, 5.407787917345625e-17], + [0.12167600863867864, -0.4415793942474466, 5.407787917345625e-17], + [-0.13295729592473662, -0.4415793942474466, 5.407787917345625e-17], + [-0.38759060048815186, -0.4415793942474466, 5.407787917345625e-17], + [-0.38759060048815186, -0.4415793942474466, 5.407787917345625e-17], + [-0.4480257881334797, -0.6277195961649644, 7.687347942054928e-17], + [-0.5084609757788076, -0.8138597980824822, 9.966907966764229e-17], + [-0.5688961634241354, -1.0, 1.2246467991473532e-16], + [-0.5688961634241354, -1.0, 1.2246467991473532e-16], + [-0.6586087447899202, -1.0, 1.2246467991473532e-16], + [-0.7483213261557048, -1.0, 1.2246467991473532e-16], + [-0.8380339075214888, -1.0, 1.2246467991473532e-16], + [0.3021757525699033, -0.21434317946653003, 2.6249468865275272e-17], + [0.1993017037512583, 0.09991949373745423, -1.2236608817799732e-17], + [0.09642765493261184, 0.4141821669414385, -5.072268650087473e-17], + [-0.006446393886033166, 0.7284448401454228, -8.920876418394973e-17], + [-0.006446393886033166, 0.7284448401454228, -8.920876418394973e-17], + [-0.10905185929034443, 0.4141821669414385, -5.072268650087473e-17], + [-0.2116573246946542, 0.09991949373745423, -1.2236608817799732e-17], + [-0.31426279009896546, -0.21434317946653003, 2.6249468865275272e-17], + [-0.31426279009896546, -0.21434317946653003, 2.6249468865275272e-17], + [-0.10878327587600921, -0.21434317946653003, 2.6249468865275272e-17], + [0.09669623834694704, -0.21434317946653003, 2.6249468865275272e-17], + [0.3021757525699033, -0.21434317946653003, 2.6249468865275272e-17], + ] + ), + decimal=5, + ) diff --git a/tests/test_graphical_units/img_svg_resources/A.svg b/tests/test_graphical_units/img_svg_resources/A.svg new file mode 100644 index 0000000000..b8a0bafab4 --- /dev/null +++ b/tests/test_graphical_units/img_svg_resources/A.svg @@ -0,0 +1,13 @@ + + + + + + + + + + + + +