Fixing the behavior of `.become` to not modify target mobject via side effects fix color linking
Fixes #3505
So originally .become transforms the target mobject and moves all the values in to the transformed mobject. Which makes them linked in many ways because they share the same references to the data.
This resulted in some weird behavior when changing one mobject causes another to change without any direct causation or intuition for that.
This further resulted in a test of ours being reliant on this behavior because it was not visible what the side-effect happened to change.
Originally the test named test_become resulted in this
Code
def test_become(scene):
s = Rectangle(width=2, height=1, color=RED).shift(UP)
d1, d2, d3 = (Dot() for _ in range(3))
s1 = s.copy().become(d1, match_width=True).set_opacity(0.25).set_color(BLUE)
s2 = (
s.copy()
.become(d2, match_height=True, match_center=True)
.set_opacity(0.25)
.set_color(GREEN)
)
s3 = s.copy().become(d3, stretch=True).set_opacity(0.25).set_color(YELLOW)
scene.add(s, d1, d2, d3, s1, s2, s3)
Which seems correct on the first look, but if we take a closer look at what is actually displayed in that scene, we realize that d1,d2,d3 are also displayed, which should be dots.
Which then results in this behavior when we actually try to unlink the data by copying all colors from the mobjects instead of linking them
For example, by adding an if statement in interpolate_color inside VMobject
Now the question becomes why does it look so weird all of a sudden when we just copied the colors. Right so the problem here lies that now that the colors are unlinked the Dots retain their original color which is WHITE by default which makes it so bright all of a sudden.
But why where they modified in the first place. If we extend the .become method to copy the target mobject instead of transforming it we result in this scene.
Which suddenly makes a lot more sense, because we have 3 Dots overlayed onto each other in the middle with the color white. And then actual mobjects that should be displayed also have the correct color and became a Dot, but instead of transforming their target, they only transformed themselves.
Which then results in a lot more intuitive behavior and not a c-style return parameter behavior.
It will then also simplify the test to the following
Code
def test_become(scene):
s = Rectangle(width=2, height=1, color=RED).shift(UP)
d = Dot()
s1 = s.copy().become(d, match_width=True).set_opacity(0.25).set_color(BLUE)
s2 = (
s.copy()
.become(d, match_height=True, match_center=True)
.set_opacity(0.25)
.set_color(GREEN)
)
s3 = s.copy().become(d, stretch=True).set_opacity(0.25).set_color(YELLOW)
scene.add(s, d, s1, s2, s3)
Because why did we need 3 Dots in the first place if we are just calling .become on another Mobject.
This will also enable a few more intuitive animations like the following
Code
class Test(Scene):
def construct(self):
target_square = Square()
dot = Dot().shift(RIGHT*2+UP*2)
triangle = Triangle().shift(RIGHT*2)
letter = Tex("A").shift(RIGHT*2+DOWN*2)
self.add(target_square, dot, triangle, letter)
self.wait()
self.play(target_square.animate.become(dot, stretch=True, match_center=True))
self.wait()
self.play(target_square.animate.become(triangle, stretch=True, match_center=True))
self.wait()
self.play(target_square.animate.become(letter, stretch=True, match_center=True))
self.wait()
https://github.com/ManimCommunity/manim/assets/17803932/ef4eb59d-84fa-41a5-92c2-b9640c27d79a
Instead of the following happening in the current version.
https://github.com/ManimCommunity/manim/assets/17803932/f684ba6f-614f-4d0b-8cae-9aeacb483451
So I would propose the following changes with this PR
- A behavior change of
.becomewhich was unintuitive also with the test case - A copying of colors in the
interpolate_colorfunction to avoid linking colors with animations - A change and review of the test that is currently used for checking
.becomeand also additional testing on that part!
Should we add the code as a doctest for become somewhere ?
I've extended the docstring with some tests and examples.
There is a slight change in behavior with become now using a copy of the target instead of modifying the target itself. I'm fine with this, we might want to ask users in our Discord for their opinion too?
Otherwise this is good to go from my side.