Skip to content

Conversation

@behackl
Copy link
Member

@behackl behackl commented Mar 5, 2022

Overview: What does this pull request change?

The recent PR adding the is_introducer mechanic for animations broke the way moving_mobjects and static_mobjects were determined. This PR fixes these issues.

Fixes #2585, #2598.

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

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

@behackl behackl added the pr:bugfix Bug fix for use in PRs solving a specific issue:bug label Mar 5, 2022
@behackl behackl marked this pull request as ready for review March 5, 2022 22:22
@behackl behackl added this to the v0.15.1 milestone Mar 5, 2022
@Darylgolden
Copy link
Member

Can you explain why the vector scene test data was changed?

@behackl
Copy link
Member Author

behackl commented Mar 6, 2022

Can you explain why the vector scene test data was changed?

Sure, apologies for the very minimalistic PR description. I've had troubles with getting the last frame of the vector scene test data right with my changes (the diff always diverged in frame 36/36, which I still don't really have a good explanation for). Looking at the implementation of this animation sequence that is run in the test shows that actually, after the last animation, some mobjects are removed from the scene after the last animation has played.

I've added an additional wait in the end to make sure that the "flickering" that the diff said was present is actually not there -- and it did indeed disappear from the control data as well after the wait was added.

It might be worth to look into the scenario of removing a mobject after the last animation some more, but for all practical purposes this should not matter.

@behackl behackl changed the title Fixed two issues with introducer animations Fixed render flow issues with introducer animations Mar 6, 2022
@behackl behackl linked an issue Mar 6, 2022 that may be closed by this pull request
@behackl
Copy link
Member Author

behackl commented Mar 8, 2022

If there are no general concerns, I'll merge this later today (in about 8h-ish or so from now) and proceed with preparing the bugfix release.

Copy link
Collaborator

@icedcoffeeee icedcoffeeee left a comment

Choose a reason for hiding this comment

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

I reviewed this earlier, but forgot to submit the review. LGTM thanks!

@behackl behackl enabled auto-merge (squash) March 8, 2022 20:24
@behackl behackl merged commit 294efcd into ManimCommunity:main Mar 8, 2022
@behackl behackl deleted the fix-introducers branch March 8, 2022 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:bugfix Bug fix for use in PRs solving a specific issue:bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v0.15.0 breaks LinearTransformationScene Mobject Ordering Regression in 0.15

3 participants