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

Fix 3D overlap when animating by checking Mobject family members recursively instead of self.mobjects #2254

Merged
merged 3 commits into from
Dec 7, 2024

Conversation

MathItYT
Copy link
Contributor

@MathItYT MathItYT commented Dec 2, 2024

Motivation

As I mentioned in my previous pull request, I had problems when rendering VMobjects that overlap in 3D. The solution for that was making Scene aware of Mobject.z_index when grouping. Now, I had problems with animations, because Scene.begin_animations adds the mobject if it's not present in the scene, but it doesn't look recursively, so it could be present, but the scene adds it again, and that can be a bad behavior when submobjects had different z_index.

Proposed changes

  • manimlib.scene.scene
  • manimlib.animation.animation

Example scene

from manimlib import *

class ExampleScene(InteractiveScene):
    def construct(self) -> None:
        vg = VGroup(Square(), Circle().set_fill(opacity=0.5))
        vg[1].shift(0.5 * UP).set_z_index(1)
        self.add(*vg)
        self.play(GrowFromCenter(vg))

@MathItYT MathItYT changed the title Fix grouping by adding Animation.setup_scene method to customize the way of adding (or not) mobjects Fix 3D overlap when animating by adding Animation.setup_scene method to customize the way of adding (or not) mobjects Dec 3, 2024
@3b1b
Copy link
Owner

3b1b commented Dec 7, 2024

In this case, it’s better to just have begin_animation check whether the animation’s mobject is in all the family data, not just the top level mobjects. The added runtime is negligible, and it'd be fixing an oversight that may produce issues elsewhere anyway.

I'll add a suggestion to this effect, which would basically boil down to replacing “self.mobjects” with “self.get_mobject_family_members()” in the check done inside begin_animations.

In general, it’s best not to have code handling scene structuring live outside of the Scene class itself.

@MathItYT MathItYT changed the title Fix 3D overlap when animating by adding Animation.setup_scene method to customize the way of adding (or not) mobjects Fix 3D overlap when animating by checking Mobject family members recursively instead of self.mobjects Dec 7, 2024
@MathItYT
Copy link
Contributor Author

MathItYT commented Dec 7, 2024

@3b1b Ready

@3b1b
Copy link
Owner

3b1b commented Dec 7, 2024

Ah, you beat me to it. If this still fixes the issue you were facing, I'll go ahead and merge it.

Another option, if there is worry about too many calls to self.get_mobject_family_members() being innefficient, would be to write something like mobject_family = set(self.get_mobject_family_members()) at the top of the method, then check inclusion in that, and add the animation.mobject.get_family() to that set as you go.

@MathItYT
Copy link
Contributor Author

MathItYT commented Dec 7, 2024

Wdym?

@3b1b
Copy link
Owner

3b1b commented Dec 7, 2024

Eh, don't worry about it, I'll merge this.

@3b1b 3b1b merged commit df1e067 into 3b1b:master Dec 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants