ManimCommunity / manim

A community-maintained Python framework for creating mathematical animations.
https://www.manim.community
MIT License
21.02k stars 1.53k forks source link

Lagging animations with updaters and self.add #1648

Open PhilippImhof opened 3 years ago

PhilippImhof commented 3 years ago

Description of bug / unexpected behavior

Animations behave differently depending on the way the mobject was added to the scene. If I use self.play(Create(mobj)) there is no lag, if I use self.add(mobj) the animation lags in certain cases.

Expected behavior

IMHO there should be no difference between the two cases. Also, if using self.add is discouraged for objects that have updaters, we might consider printing a warning.

How to reproduce the issue

See the following code:

Code running with no lagging ```py from manim import * class Test(Scene): def construct(self): tracker = ValueTracker(-3) rect = Rectangle().set_x(tracker.get_value()) label = Text("x") rect.add_updater(lambda m: m.set_x(tracker.get_value())) label.add_updater(lambda m: m.move_to(rect.get_center())) self.add(rect,label) self.play(tracker.animate.set_value(4)) self.wait() self.play(FadeIn(rect)) self.play(tracker.animate.set_value(-4)) self.wait() ```
Lagging animation in the second part ```py from manim import * class Test(Scene): def construct(self): tracker = ValueTracker(-3) rect = Rectangle().set_x(tracker.get_value()) label = Text("x") rect.add_updater(lambda m: m.set_x(tracker.get_value())) label.add_updater(lambda m: m.move_to(rect.get_center())) self.add(rect,label) self.play(tracker.animate.set_value(4)) self.wait() #self.play(FadeIn(rect)) self.add(rect) self.play(tracker.animate.set_value(-4)) self.wait() ```

Additional media files

https://user-images.githubusercontent.com/52650214/120992430-e36a4300-c782-11eb-80e4-d228dae55858.mp4

jsonvillanueva commented 3 years ago

Something I noticed in printing the Scene's self.mobjects list is that the ordering is different between the two.

With self.play(FadeIn(rect)):

[Rectangle, Text('x'), ValueTracker, Mobject]
[06/07/21 13:10:32] INFO     Animation 2 : Partial movie file written in {'C:\\Users\\a\\Documents\\repos\\ManimCommunity\\media\\videos\\test\\480p15\\partial_movie_files\\Test\\607903841_1194556497_858162446.mp4'}                              scene_file_writer.py:401[Rectangle, Text('x'), ValueTracker, Mobject]

And with self.add(rect):

[Rectangle, Text('x'), ValueTracker, Mobject]
[Text('x'), ValueTracker, Mobject, Rectangle]
                    INFO     Animation 2 : Using cached data (hash : 607903841_4008248011_4109438031)                                                                                                                                                   cairo_renderer.py:100

Might be worth investigating that aspect further to ensure consistent ordering as animations essentially iterate through the mobject list in order to create the animations. Relevant code: https://github.com/ManimCommunity/manim/blob/34a330eb9fbecf13a7127a6d03ca3bb551d438d5/manim/scene/scene.py#L414 Essentially this probably has to be moved within def add()

jsonvillanueva commented 3 years ago

Meh, I had time here's a quick very likely incorrect fix (for the case of adding multiple mobjects), although I'm sure the issue stems here. This manages to preserve the mobject ordering and fix the lag issue for your test case:

in scene.add

        for mobject in mobjects:
            if mobject not in self.mobjects:
                if config.renderer == "opengl":
                    new_mobjects = []
                    new_meshes = []
                    for mobject_or_mesh in mobjects:
                        if isinstance(mobject_or_mesh, Mesh):
                            new_meshes.append(mobject_or_mesh)
                        else:
                            new_mobjects.append(mobject_or_mesh)
                    self.remove(*new_mobjects)
                    self.mobjects += new_mobjects
                    self.remove(*new_meshes)
                    self.meshes += new_meshes
                else:
                    mobjects = [*mobjects, *self.foreground_mobjects]
                    self.restructure_mobjects(to_remove=mobjects)
                    self.mobjects += mobjects
                    if self.moving_mobjects:
                        self.restructure_mobjects(
                            to_remove=mobjects, mobject_list_name="moving_mobjects"
                        )
                        self.moving_mobjects += mobjects
        return self

Also, the fact that we're only testing for existence here makes me think that self.mobjects should be a dictionary instead of a list. Thoughts? As of Python version 3.7, dictionaries are ordered.

k4pran commented 3 years ago

I was looking at this a bit and it does seem to be related to the order, you can reproduce it and see it lagging in both directions by adding label and rect in reverse order:

class Test(Scene):
    def construct(self):
        tracker = ValueTracker(-3)
        rect = Rectangle().set_x(tracker.get_value())
        label = Text("x")
        rect.add_updater(lambda m: m.set_x(tracker.get_value()))
        label.add_updater(lambda m: m.move_to(rect.get_center()))
        self.add(label, rect)
        self.play(tracker.animate.set_value(4))
        self.wait()
        self.play(tracker.animate.set_value(-4))
        self.wait()

The different is that in reverse order the label's move_to updater gets called before the rect's set_x so it always lags behind. I am not sure of the solution though, seems like you would need to track dependencies between updaters ?

Octonions commented 1 year ago

Really hoping to have this fixed anytime soon :/