Speed up Cairo renderer by 2.2x on animation-heavy scenes#4695
Speed up Cairo renderer by 2.2x on animation-heavy scenes#4695HamdiBarkous wants to merge 14 commits into
Conversation
…flat array indexing Replace Python generators and tuple unpacking with numpy-based subpath splitting and direct flat-array indexing for bezier point lookups. Same Cairo calls, same output, ~2-7x faster path building. - Replace gen_subpaths_from_points_2d generator with vectorized numpy boundary detection using np.arange + boolean masking - Replace gen_cubic_bezier_tuples_from_points generator with direct integer-range iteration over pre-flattened xy array - Eliminate per-curve numpy slice creation (*p[:2] splat) - Cache method references (ctx.curve_to → local) to avoid attribute lookup per call Benchmarks (1920x1080 @ 60fps): - set_path: 2-7x faster across scene types - Overall: up to 1.5x faster on shape/text-heavy scenes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- camera.reset(): Replace set_pixel_array() → convert_pixel_array() → np.array() (copy) → slice assignment (second copy) with a single np.copyto() call. Removes one full-frame copy per frame. - set_frame_to_background(): Same optimization for static frame restore. - renderer.get_frame(): Replace np.array() with .copy() — avoids dtype inference overhead on an already-typed array. Benchmarks (1920x1080 @ 60fps): - camera_reset: 3-10x faster (e.g. 390ms → 120ms on AnimatedTransforms) - Overall: ~2x faster across scene types when combined with set_path opt Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds benchmarks/bench_lissajous.py — a heavy real-world animation workload with grid-of-circles updaters tracing Lissajous curves. Stresses the per-frame render path far more than static gallery scenes, making rendering optimizations visible end-to-end. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
for more information, see https://pre-commit.ci
self.background is typed as PixelArray | None (from __init__ param) but is guaranteed non-None after init_background() runs during construction. Add an assert to satisfy mypy. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
This looks quite promising -- but the claim of pixel-identical ouput appears to be wrong, given our pipelines. We can accept some variation in the tests, but we would need to understand (and verify individually) why the output has changed and that it still is practically correct. |
When a VMobject had exactly nppcc points (one cubic curve, e.g. Line), np.arange(nppcc, n_pts, nppcc) returned an empty array and the function exited before drawing. The original code handled this via split_indices of [0, n_pts], yielding one subpath of all 4 points. Handle the empty-boundary case explicitly as a single subpath. Verified pixel-identical vs main across 12 scenes (Line, Dot, Square, Circle, Arrow, Text, MathTex, Polyline, DashedLine, OpenPath, mixed, animated). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
for more information, see https://pre-commit.ci
|
@behackl Thanks for catching this. Fixed in the latest commit. Verified pixel-identical against main across 12 scenes (Line, Dot, Square, Circle, Arrow, Text, MathTex, Polyline, DashedLine, open polyline, mixed, animated). Can you please confirm? |
|
Looking much better, indeed -- I'll make sure to review this in more detail as soon as I can; very impressive! Our CI says that the documentation build and specifically the rendering of the Could you take a look at this? |
|
The docbuild issue happens because of this piece of code in the my_vmobject = VMobject(color=GREEN)
my_vmobject.points = [
np.array([-2, -1, 0]), # start of first curve
np.array([-3, 1, 0]),
np.array([0, 3, 0]),
np.array([1, 3, 0]), # end of first curve
np.array([1, 3, 0]), # start of second curve
np.array([0, 1, 0]),
np.array([4, 3, 0]),
np.array([4, -2, 0]), # end of second curve
]When doing this, You can fix this by changing the list above to a 2D NumPy array, but IMO a cleaner solution is to use the existing my_vmobject = VMobject(color=GREEN).set_points(
[
[-2, -1, 0], # start of first curve
[-3, 1, 0],
[0, 3, 0],
[1, 3, 0], # end of first curve
[1, 3, 0], # start of second curve
[0, 1, 0],
[4, 3, 0],
[4, -2, 0], # end of second curve
]
) |
The vectorized path builder uses numpy fancy indexing (e.g. points[boundary_indices - 1, :2]), which fails when vmobject.points is a plain Python list. The documented VMobjectDemo example sets points this way, which broke the docs build. np.asarray the points array once on entry; it's a no-op when the input is already an ndarray. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The docs' VMobjectDemo sets vmobject.points to a Python list, which broke my numpy fancy indexing. Fixed by normalizing to ndarray on entry (no-op when it already is one, which is the default). |
|
@behackl Any updates on this PR? |
chopan050
left a comment
chopan050
left a comment
There was a problem hiding this comment.
Thanks for the optimization! I left two requests, if you could please take a look at them:
| ): | ||
| np.copyto(self.pixel_array, self.background) | ||
| else: | ||
| self.pixel_array = self.background.copy() |
There was a problem hiding this comment.
Camera.reset()/set_frame_to_background(): replace
set_pixel_array() -> convert_pixel_array() -> np.array() (copy) -> slice assignment (second copy)with a singlenp.copyto().
Skipping unnecessary copies is great. However, rather than circumventing set_pixel_array() to skip these copies, it would be best to fix the issues in set_pixel_array() to make it faster.
The main issue is that it always calls Camera.convert_pixel_array(), which only does something meaningful when convert_from_floats == True, but always makes a copy of the passed array. Instead, you can call that method conditionally, only when convert_from_floats == True, rather than always. Moreover, if you rewrite the other two calls to Camera.convert_pixel_array() so that it's only called when you do actually want to convert from floats, then you can drop the convert_from_floats parameter from the Camera.convert_pixel_array() method itself. You can rewrite the method like this:
def convert_pixel_array(self, pixel_array: PixelArray | list | tuple) -> PixelArray:
"""docstring..."""
return np.apply_along_axis(
lambda f: (f * self.rgb_max_val).astype(self.pixel_array_dtype),
2,
retval,
)then rewrite Camera.set_pixel_array() as follows:
def set_pixel_array(
self, pixel_array: PixelArray | list | tuple, convert_from_floats: bool = False
) -> None:
"""Sets the pixel array of the camera to the passed pixel array.
Parameters
----------
pixel_array
The pixel array to convert and then set as the camera's pixel array.
convert_from_floats
Whether or not to convert float values to proper RGB values, by default False
"""
converted_array: PixelArray = (
self.convert_pixel_array(pixel_array)
if convert_from_floats
else np.asarray(pixel_array)
)
if (
hasattr(self, "pixel_array")
and self.pixel_array.shape == converted_array.shape
):
np.copyto(self.pixel_array, converted_array) # Set in place
else:
self.pixel_array = converted_array.copy()and then leave Camera.reset() and Camera.set_frame_to_background() as they were originally.
| if hasattr(self, "pixel_array") and self.pixel_array.shape == background.shape: | ||
| np.copyto(self.pixel_array, background) | ||
| else: | ||
| self.pixel_array = background.copy() |
| # Find subpath split points using vectorized comparison. | ||
| # A split occurs where consecutive anchors (at nppcc boundaries) | ||
| # are NOT close — i.e., there's a gap between subpaths. |
There was a problem hiding this comment.
Calculating subpath split indices is great and is what ManimGL does, actually.
Because VMobject has methods for getting subpaths from its points, could you please move this logic of calculating subpath split indices inside VMobject methods? I did something similar in an older and unmerged PR, #3759, which you can take a look at.
| # vmobject.points may be a Python list (see VMobjectDemo in the docs); | ||
| # the vectorized path-building below needs an ndarray. | ||
| points = np.asarray(points) |
There was a problem hiding this comment.
vmobject.points should actually never be a list. It should always be a NumPy array. The VMobjectDemo example in the docs set the points as a list, which is wrong. Rather, that example should be fixed so that the points are a NumPy array. One way to ensure that is with the fix I proposed in a comment I made earlier.
Rework set_pixel_array/convert_pixel_array so the per-frame reset path does a single in-place copy, then route Camera.reset() and set_frame_to_background() back through set_pixel_array() rather than inlining np.copyto. convert_pixel_array() no longer takes convert_from_floats and only performs the float->RGB conversion; callers convert only when needed. Addresses review feedback on ManimCommunity#4695.
Add VMobject.get_subpath_split_indices_from_points(), a vectorized computation of the point-index ranges delimiting each subpath, and have Camera.set_cairo_context_path() call it instead of computing the split indices inline. Keeps the vectorized comparison (the exact vectorized form of consider_points_equals_2d), so rendered output is unchanged. Mirrors the API of the unmerged ManimCommunity#3759. Addresses review feedback on ManimCommunity#4695.
The deep-dive VMobjectDemo example assigned a Python list to VMobject.points, violating the invariant that points is always a NumPy array (and breaking the docs build with the vectorized path builder). Use VMobject.set_points() instead, then drop the defensive np.asarray() workaround in set_cairo_context_path since points is now always an ndarray. Addresses review feedback on ManimCommunity#4695.
for more information, see https://pre-commit.ci
Add strict=True to zip() in the new subpath test (B905) and collapse two signatures/calls that fit on one line per ruff format.
chopan050
left a comment
chopan050
left a comment
There was a problem hiding this comment.
I just took a look at Abhijith Muthyala's repository where you mentioned that you took the files for the Lissajous curves benchmark from.
The README.md says at the end:
LICENSE
This repository is mainly intended to update and keep track of my personal projects made with Manim. It is made public in the hope that someone might find it useful while learning Manim. All the code is only meant as a reference and not for reuse without permission.
Do you have explicit permission from Abhitijh to include the contents of his projects almost unmodified? If not, I'm afraid that this file cannot be included in the PR...
3 participants
Summary
Two small, behavior-preserving optimizations to the Cairo renderer hot
path, plus a real-world benchmark to measure them.
Optimizations
1. Vectorized path building in
set_cairo_context_pathReplace Python generators (
gen_subpaths_from_points_2d,gen_cubic_bezier_tuples_from_points) and per-curve tupleunpacking with numpy-based subpath splitting and direct flat-array
indexing. Same Cairo API calls, same output, just less Python
overhead per bezier segment.
2. Eliminate redundant numpy copies
Camera.reset()/set_frame_to_background(): replaceset_pixel_array() -> convert_pixel_array() -> np.array() (copy) -> slice assignment (second copy)with a singlenp.copyto().CairoRenderer.get_frame(): replacenp.array()with.copy()to skip dtype inference on an already-typed array.
Both changes are purely Python-side and leave rendering output
pixel-identical.
Benchmark
Adds
benchmarks/bench_lissajous.py, a heavy real-world workload(adapted from Abhijith Muthyala's Lissajous project) with a grid
of circles + updaters tracing Lissajous curves. Stresses the per-frame
render path far more than static gallery scenes.
Results
Measured with
bench_lissajous.pyon the same machine, 1920x1080 @ 60fps:A ~20-minute wall-clock reduction on a single scene render.
Test plan
python benchmarks/bench_lissajous.pyruns to completion