Conversation
3 tasks
3 tasks
3 tasks
HamdiBarkous
added a commit
to HamdiBarkous/manim
that referenced
this pull request
Jul 7, 2026
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.
behackl
added a commit
that referenced
this pull request
Aug 9, 2026
* Optimize set_cairo_context_path: vectorize subpath splitting and use 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> * Eliminate redundant numpy copies in camera reset and frame retrieval - 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> * Add Lissajous Table benchmark for measuring render performance 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> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix mypy union-attr error in Camera.reset 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> * Fix single-curve VMobject skipped in set_cairo_context_path 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> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Accept list-typed VMobject.points in set_cairo_context_path 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> * Fix redundant copies in set_pixel_array instead of bypassing it 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 #4695. * Move subpath split-index calculation into VMobject 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 #3759. Addresses review feedback on #4695. * Fix VMobjectDemo docs example to keep points as an ndarray 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 #4695. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Satisfy ruff lint and format Add strict=True to zip() in the new subpath test (B905) and collapse two signatures/calls that fit on one line per ruff format. * Add type hints to Lissajous benchmark Type-annotate all functions/methods in bench_lissajous.py per review, and replace the attribute-tagged plain VMobject paths with a dedicated LissajousCurve(VMobject) class so the dot/row_circle/column_circle attributes are properly typed. Also note upstream author's permission in the module docstring. Addresses review feedback on #4695. * Match scalar non-finite handling in subpath split indices get_subpath_split_indices_from_points used the raw tolerance formula, which diverges from np.isclose/np.allclose on NaN/inf coordinates. This could yield non-pixel-identical output vs main when ThreeDCamera projection produces inf/NaN (a point whose rotated z equals the focal distance). Transcribe the scalar comparisons exactly: the asymmetric consider_points_equals_2d form for n_dims=2, and np.isclose (allclose semantics) for n_dims=3. Finite results are unchanged. Thanks @pjfo for the analysis on #4695. * Ensure consistent point comparison semantics * Allow unsafe casting in set_pixel_array's in-place copy np.copyto defaults to same_kind casting, which rejects the int64 array np.asarray produces for a plain Python list being written into the camera's uint8 buffer. The slice assignment this replaced casted unsafely, so restore that behaviour explicitly. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Benjamin Hackl <devel@benjamin-hackl.at>
Member
Author
|
Superseded by #4695. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview: What does this pull request change?
Related PR: #3292
Taking inspiration from ManimGL, I added new methods
VMobject.get_subpath_split_indices()andVMobject.get_subpath_split_indices_from_points(), which instead of explicitly obtaining a Python list of subpaths, it obtains an(n_subpaths, 2)ndarray of indices indicating where every subpath starts and ends.These methods also accept parameters
n_dims(which allows us to choose between 2D or 3D point comparison) andstrip_null_end_curves(useful in a future PR forVMobject.align_points(), where the null end curves at every subpath must be removed as a fix to certain bug).Then I rewrote
VMobject.get_subpaths_from_points()andVMobject.gen_subpaths_from_points_2d()to use this new methodVMobject.get_subpath_split_indices_from_points().TODO: vectorize
VMobject.consider_points_equals()in a subsequent PR for extra optimization inVMobject.get_subpath_split_indices_from_points().Motivation and Explanation: Why and how do your changes improve the library?
Methods like
VMobject.change_anchor_mode()andVMobject.align_points()could benefit from calculating split indices, since they allow for things such as calculating the lengths of the subpaths in a vectorized fashion, which then allows for creating a single NumPy empty array with the right length from the beginning, rather than appending into a NumPy array.Links to added or changed documentation pages
Further Information and Comments
Reviewer Checklist