Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance for :meth:.Mobject.become #2508

Merged
merged 25 commits into from Jan 29, 2022

Conversation

Darylgolden
Copy link
Member

@Darylgolden Darylgolden commented Jan 29, 2022

Overview: What does this pull request change?

  • Added a boolean parameter, skip_point_alignment to the align_data method. When True, the expensive point alignment method is not called.
  • Added documentation for the align_data method.
  • Added three tests testing the become method.

Motivation and Explanation: Why and how do your changes improve the library?

While profiling the following scene:

class Test(Scene):
    def construct(self):
        nplane = NumberPlane()
        b = ValueTracker(1)
        a = always_redraw(
            lambda: nplane.plot_implicit_curve(
                lambda x, y: x ** 3 - y ** 2 - x - b.get_value()
            )
        )
        self.add(nplane, a)
        self.wait()
        self.play(b.animate.set_value(-1))
        self.wait()

I noticed that Manim was spending an inordinate amount of time on the become method of Mobject. In particular, when using the become method for VMobjects, a lot of time is wasted aligning the points of the VMobject to its target. This is despite the fact that the become method simply changes a VMobject's point data to match its target, with no need to interpolate where its initial points should lie (that will immediately get overwritten). Skipping point alignment caused the render time for the above scene on medium quality on my PC to drop from 80 seconds to 15 seconds with Cairo. For OpenGL, the time spent on the become method was already negligible, so I left it untouched.

Links to added or changed documentation pages

Further Information and Comments

Reviewer Checklist

  • The PR title is descriptive enough for the changelog, and the PR is labeled correctly
  • If applicable: newly added non-private functions and classes have a docstring including a short summary and a PARAMETERS section
  • If applicable: newly added functions and classes are tested

@Darylgolden
Copy link
Member Author

Docs build has dropped from ~980 seconds to ~930 seconds 💯

@Darylgolden Darylgolden marked this pull request as ready for review January 29, 2022 05:50
@Darylgolden Darylgolden marked this pull request as draft January 29, 2022 05:55
@Darylgolden Darylgolden marked this pull request as ready for review January 29, 2022 09:14
Copy link
Member

@behackl behackl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Left one suggestion for documentation, but other than that this looks good to me.

manim/mobject/mobject.py Show resolved Hide resolved
Co-authored-by: Benjamin Hackl <devel@benjamin-hackl.at>
manim/mobject/mobject.py Outdated Show resolved Hide resolved
@Darylgolden Darylgolden merged commit 8d94325 into ManimCommunity:main Jan 29, 2022
@behackl behackl changed the title Improve performance for :meth:~become Improve performance for :meth:.Mobject.become Feb 20, 2022
@behackl behackl added the enhancement Additions and improvements in general label Feb 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Additions and improvements in general performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants