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

call finish() when animations finishes #3951

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Laifsyn
Copy link

@Laifsyn Laifsyn commented Oct 9, 2024

Fix Animation's getting toggled off but not back on

Overview: What does this pull request change?

Animations sent through AnimationGroup and LaggedStart will have their updaters toggled off without toggling them back on after the GroupAnimation finishes.

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

Users playing the animations of their Mobjects in AnimationGroup will find themselves troubled due to updaters silently getting turned off but not back on after the AnimationGroup finishes

Links to added or changed documentation pages

N/A

Further Information and Comments

This could be a fix to the issue found in #3950

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

This could be a fix to the issue found in ManimCommunity#3950
@JasonGrace2282 JasonGrace2282 marked this pull request as ready for review October 9, 2024 21:33
@Laifsyn
Copy link
Author

Laifsyn commented Oct 10, 2024

If you want to skip the Wall Texts go to suggested solutions.

You can also check for the Issue at this next link: #3950 "Manim 0.18.1: Updaters don't work with objects which have been added using LaggedStart()"

So based on the method finish(self) from the class Animation which is located at animation.py file(I'll call it Animation::finish(self)@animation.py for now), if the animation was set to suspend the mobject's updaters, which happens when calling Animation::begin(self)@animation.py,the updaters won't be "unsuspended" until Animation::finish(self), which checks for the self.suspend_mobject_updating : bool attribute, is called.

However AnimationGroup(Animation)::finish(self) is overriden, and in its place we have the following snippet Code Block #1, which we can see that what's being resumed is actually AnimationGroup(Animation)::group : Group | VGroup | OpenGLGroup | OpenGLVGroup which at this moment of time, I could say it's a list of Animations whose Animation::introducer : bool property returns bool (Check Snippet #1 to see AnimationGroup(Animation)::__init__(self), and how animations are being added to AnimationGroup(Animation)::group).

If we see at the tested_code, and follow the definitions to Create(ShowPartial)::__init__(self)@creation.py we can see that the __init__(self) does set introducer to True as default (Also worth to mention I tried to run LaggedStart(....,introducer=True)). So the issue at hand might not necessarilly be as straightforward due to design decisions which can be traced to #2396 (worth to mention I didn't dive deep into the commit's details), so updating the default of Create() to introducer=False should undoubtedly be out of question.

Why false? Because at AnimatioGroup(Animation)::__init__(self) we have this snippet: [anim.mobject for anim in self.animations if not anim.is_introducer()] which negates the Animation::introducer property.

Solutions

So here's 3 possible solutions

Solution 1:

Update AnimationGroup(Animation)::__init__(), re-defining self.group definition

This approach might cause breaking changes because I (or we) don't know what's self.group is actually used for.

# Copied from `AnimationGroup(Animation)::__init__(self)`
arg_anim = flatten_iterable_parameters(animations)
self.animations = [prepare_animation(anim) for anim in arg_anim]
self.rate_func = rate_func
self.group = group
if self.group is None:
    mobjects = remove_list_redundancies(
        # [anim.mobject for anim in self.animations if not anim.is_introducer()], # <---- Old Line
        [anim.mobject for anim in self.animations if anim.is_introducer()], # (1) Invert the negation
        # (2) Or alternatively remove the `if` condition altogether
    )
    if config["renderer"] == RendererType.OPENGL:
        self.group = OpenGLGroup(*mobjects)
    else:
        self.group = Group(*mobjects)
super().__init__(
    self.group, rate_func=self.rate_func, lag_ratio=lag_ratio, **kwargs
)
self.run_time: float = self.init_run_time(run_time)

Solution 2:

Update the AnimationGroup(Animation)::finish(self) definition to individually call Animation::mobject.resume_updating()
on each item of AnimationGroup(Animation)::animations: list[Animation]. (or alternatively call the .finish() method on each animation (check Solution 3))

# Copied from `AnimationGroup(Animation)::finish(self)`
    def finish(self) -> None:
        self.interpolate(1)
        self.anims_begun[:] = True
        self.anims_finished[:] = True
        super().finish()
        if self.suspend_mobject_updating:
            for anim in self.animations:        # <----- Added Line
                anim.mobject.resume_updating()  # <----- Added Line
            self.group.resume_updating()

Solution 3:

call .finish() method on each individual AnimationGroup(Animation)::animations: list[Animation] (or check Solution 1 where I suggest updating the self.group attribute definition)

We would be re-adding code that was removed in PR: #3542 (Dating back to 2023 December)

# Copied from `AnimationGroup(Animation)::finish(self)`
    def finish(self) -> None:
        self.interpolate(1)
        self.anims_begun[:] = True
        self.anims_finished[:] = True
        super().finish()
        if self.suspend_mobject_updating:
            for anim in self.animations:    # <----- Added Line
                anim.finish()               # <----- Added Line
            self.group.resume_updating()


Code Block 1

Current finish(self) implementation

# Copied from AnimationGroup(Animation)::finish(self)
def finish(self) -> None:
    self.interpolate(1)
    self.anims_begun[:] = True
    self.anims_finished[:] = True
    if self.suspend_mobject_updating:
        self.group.resume_updating()

Code Block 2

# 1 (Let's call animation.py as the base file)
# `# 1` emulates the hand-written callstack, and its our base call

# 1
def finish(self) -> None: # Animation::finish(self) @ animation.py
    """Finish the animation.

    This method gets called when the animation is over.

    """
    self.interpolate(1)
    if self.suspend_mobject_updating and self.mobject is not None:
        self.mobject.resume_updating()

# 2
def interpolate(self, alpha: float) -> None: # Animation::interpolate(self) @ animation.py
    """Set the animation progress.

    This method gets called for every frame during an animation.

    Parameters
    ----------
    alpha
        The relative time to set the animation to, 0 meaning the start, 1 meaning
        the end.
    """
    self.interpolate_mobject(alpha)
# 2 (Foreign File)
def resume_updating(self, recursive: bool = True) -> Self: # Mobject::resume_updating(self) @ mobject.py
    self.updating_suspended = False
    if recursive:
        for submob in self.submobjects:
            submob.resume_updating(recursive)
    self.update(dt=0, recursive=recursive)
    return self

# 3 
def interpolate_mobject(self, alpha: float) -> None: # Animation::interpolate_mobject(self) @ animation.py
    """Interpolates the mobject of the :class:`Animation` based on alpha value.

    Parameters
    ----------
    alpha
        A float between 0 and 1 expressing the ratio to which the animation
        is completed. For example, alpha-values of 0, 0.5, and 1 correspond
        to the animation being completed 0%, 50%, and 100%, respectively.
    """
    families = list(self.get_all_families_zipped())
    for i, mobs in enumerate(families):
        sub_alpha = self.get_sub_alpha(alpha, i, len(families))
        self.interpolate_submobject(*mobs, sub_alpha)

# 4 (Does nothing (?))
def interpolate_submobject(
    self,
    submobject: Mobject,
    starting_submobject: Mobject,
    alpha: float,
) -> Animation: # Animation::interpolate_submobject(self) @ animation.py
    pass
# 4 
def get_sub_alpha(self, alpha: float, index: int, num_submobjects: int) -> float: # Animation::get_sub_alpha(self) @ animation.py
    """Get the animation progress of any submobjects subanimation.

    Parameters
    ----------
    alpha
        The overall animation progress
    index
        The index of the subanimation.
    num_submobjects
        The total count of subanimations.

    Returns
    -------
    float
        The progress of the subanimation.
    """
    lag_ratio = self.lag_ratio
    full_length = (num_submobjects - 1) * lag_ratio + 1
    value = alpha * full_length
    lower = index * lag_ratio
    if self.reverse_rate_function:
        # self.rate_func: Callable[[float], float]
        # default callable: rate_functions.smooth
        # Set at Animation::__init__(self) @ animation.py
        return self.rate_func(1 - (value - lower))
    else:
        return self.rate_func(value - lower)

Snippet 1

def __init__(
    self,
    *animations: Animation | Iterable[Animation] | types.GeneratorType[Animation],
    group: Group | VGroup | OpenGLGroup | OpenGLVGroup = None,
    run_time: float | None = None,
    rate_func: Callable[[float], float] = linear,
    lag_ratio: float = 0,
    **kwargs,
) -> None:
    arg_anim = flatten_iterable_parameters(animations)
    self.animations = [prepare_animation(anim) for anim in arg_anim]
    self.rate_func = rate_func
    self.group = group
    if self.group is None:
        mobjects = remove_list_redundancies(
            [anim.mobject for anim in self.animations if not anim.is_introducer()], # "Appending" to `self.group`
        )
        if config["renderer"] == RendererType.OPENGL:
            self.group = OpenGLGroup(*mobjects)
        else:
            self.group = Group(*mobjects)
    super().__init__(
        self.group, rate_func=self.rate_func, lag_ratio=lag_ratio, **kwargs
    )
    self.run_time: float = self.init_run_time(run_time)

Snippet 2

# Copied from Animation::__init__(self) @ animation.py
def __init__(
    self,
    mobject: Mobject | None,
    lag_ratio: float = DEFAULT_ANIMATION_LAG_RATIO,
    run_time: float = DEFAULT_ANIMATION_RUN_TIME,
    rate_func: Callable[[float], float] = smooth,
    reverse_rate_function: bool = False,
    name: str = None,
    remover: bool = False,  # remove a mobject from the screen?
    suspend_mobject_updating: bool = True,
    introducer: bool = False,
    *,
    _on_finish: Callable[[], None] = lambda _: None,
    **kwargs,
) -> None:
    self._typecheck_input(mobject)
    self.run_time: float = run_time
    self.rate_func: Callable[[float], float] = rate_func
    self.reverse_rate_function: bool = reverse_rate_function
    self.name: str | None = name
    self.remover: bool = remover
    self.introducer: bool = introducer
    self.suspend_mobject_updating: bool = suspend_mobject_updating
    self.lag_ratio: float = lag_ratio
    self._on_finish: Callable[[Scene], None] = _on_finish
    if config["renderer"] == RendererType.OPENGL:
        self.starting_mobject: OpenGLMobject = OpenGLMobject()
        self.mobject: OpenGLMobject = (
            mobject if mobject is not None else OpenGLMobject()
        )
    else:
        self.starting_mobject: Mobject = Mobject()
        self.mobject: Mobject = mobject if mobject is not None else Mobject()
    if kwargs:
        logger.debug("Animation received extra kwargs: %s", kwargs)

    if hasattr(self, "CONFIG"):
        logger.error(
            (
                "CONFIG has been removed from ManimCommunity.",
                "Please use keyword arguments instead.",
            ),
        )

Code Tested With

from manim import *

class laggingUpdater(Scene):
    def construct(self):
        vt = ValueTracker(0)
        dot1a = Dot().shift(3 * UP)

        def updater(mobj):
            mobj.set_x(vt.get_value())

        dot1a.add_updater(updater)

        self.play(
            LaggedStart(
                Create(dot1a),
            )
        )

        dot1a.resume_updating()

        self.wait()
        self.play(vt.animate.set_value(7), run_time=4)
        self.wait()

        self.play(vt.animate.set_value(0), run_time=2)
        self.wait()


laggingUpdater().render(preview=True)

Edit: removing newlines

Copy link
Contributor

@chopan050 chopan050 left a comment

Choose a reason for hiding this comment

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

Hello, and thanks for your PR!

I'm the author of the PR #3542. I accidentally introduced this bug after attempting to fix the following issue:

There was a bug where, if you used a "reverse" rate_func which intended to make the submobjects reach an alpha == 0 state, when AnimationGroup.finish() gets called, it would call all of its subanimations' finish() method which would make all the submobjects be reset to an alpha == 1 state instead. To fix this, an AnimationGroup.interpolate(1) was added to the AnimationGroup.finish() method.

I realized that it would be more difficult than that: in order to accomplish that, an option would be to add an initial_alpha: float = 0.0 parameter to Animation.begin() and a final_alpha: float = 1.0 parameter to Animation.finish(), among other things. In that way, I would allow an animation to finish with, for example, alpha=0.0, which is useful for reversed animations. That was something I discussed with Benjamin Hackl a long time ago, without a definitive solution, and it should be done in another PR.

The issue is: it seems I forgot to revert that change in AnimationGroup.finish(). So, thanks for making this PR to fix it!

I would say that the correct code for AnimationGroup.finish() should be the following:

    def finish(self) -> None:
        for anim in self.animations:
            anim.finish()
        self.anims_begun[:] = True
        self.anims_finished[:] = True
        if self.suspend_mobject_updating:
            self.group.resume_updating()

Notice that:

  • The self.interpolate(1) is gone, because the effect I intended to cause with it won't work for now.
  • The for anim in self.animations: anim.finish() is moved outside the if, because it should always be executed, like it was originally.

@JasonGrace2282
Copy link
Member

Perhaps it's also a good idea for us to add a test for this in order to prevent future regressions.

@chopan050
Copy link
Contributor

TL;DR: if you could please modify finish() like this:

    def finish(self) -> None:
        for anim in self.animations:
            anim.finish()
        self.anims_begun[:] = True
        self.anims_finished[:] = True
        if self.suspend_mobject_updating:
            self.group.resume_updating()

that would be great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

3 participants