manim icon indicating copy to clipboard operation
manim copied to clipboard

Ported improved implementation of :class:`.SVGMobject` from 3b1b/manim

Open behackl opened this issue 2 years ago • 2 comments

Overview: What does this pull request change?

This PR is an attempt at porting the improved implementation of SVGMobject by @YishiMichael to the community edition.

  • svg_mobject.py is mostly a copy+paste, plus (more or less) minor changes to make it compatible with some of the changes we have applied to VMobject etc.
  • The tests are not passing (for now). One particular feature of the new implementation is that e.g. the stroke width of the original SVG is respected (and not just overwritten by 4), and so almost all of the SVG tests fail. Work in progress until all of the tests have been closely investigated and control data regenerated.
  • This uses svgelements to parse the SVG (which is better than doing it ourselves :-))

Breaking changes

This completely changes the implementation of :class:.SVGMobject to do the parsing via svgelements. As a consequence, some classes and methods related to the old implementation are no longer required, and have been removed:

  • SVGPathMobject has been removed. :class:.VMobjectFromSVGPath takes its place, but has a slightly different interface: given a string describing an SVG path element path_string, the old mobject was initialized as SVGPathMobject(path_string). For the new mobject, import svgelements as se and create VMobjectFromSVGPath(se.Path(path_string)).
  • TexSymbol (an alias of SVGPathMobject) has been removed. Use the workaround described above if required.
  • The manim.mobject.svg.svg_path module and its members have been removed.
  • The manim.mobject.svg.style_utils module and its members have been removed.

Open Tasks

  • [x] port basic version of upstream SVGMobject
  • [x] Check failing tests; regenerate control data of those only failing due to changed stroke width
  • [x] Adapt implementation of Brace, get rid of svg_path.py
  • [x] Write documentation
  • [x] and appropriate breaking change changelog entry

behackl avatar Jul 23 '22 17:07 behackl

Here is an overview over the updated control data (caused by either correcting the test or by the changed stroke width):

  • test_Arcs01: stroke width image

  • test_Arcs02: stroke width image

  • test_ContiguousUSMap: stroke width (changed in .svg) image

  • test_DesmosGraph1: stroke width image

  • test_HalfEllipse: not sure why scaling of control data looked so weird, new data is definitely correct. changed stroke to white to see the line segments on the side too image

  • test_Heart: stroke width image

  • test_Line: stroke width image

  • test_ManimLogo: stroke width image

  • test_MultipleTransform: stroke width image

  • test_Penrose: stroke width image

  • test_PixelizedText: stroke width image

  • test_Rhomboid: stroke width; fixed fill image

  • test_RotateTransform: control data completely broken, now fixed image

  • test_SkewXTransform: control data completely broken, now fixed image

  • test_SkewYTransform: control data completely broken, now fixed image

  • test_SmoothCurves: stroke width image

  • test_WeightSVG: stroke width image

behackl avatar Jul 24 '22 16:07 behackl

I consider this ready to be looked at now!

It would be great if someone working on Linux could try to run the tests locally as well (for whatever reason, the two tests in tests/test_graphical_units/test_text.py don't pass locally for me anymore, but they do pass in the pipeline). Maybe we've hit another road block when it comes to graphical unit tests involving text rendering; maybe it is necessary to allow (more?) pixel mismatches for these tests. (Locally the tests are failing with a 1.1% and a 2% mismatch though, which is rather substantial -- --show_diff looks like the stroke width is off again, which I don't have an explanation for.)

behackl avatar Jul 25 '22 02:07 behackl