manim icon indicating copy to clipboard operation
manim copied to clipboard

Ellipse preserves width and height when animating (behaves like circle)

Open Splines opened this issue 3 years ago • 4 comments

Description of bug / unexpected behavior

class CircleStretch(Scene):
    def construct(self):
        ellipse = Ellipse(width=1, height=2)
        self.wait()
        self.play(ellipse.animate.set(width=6))
        self.wait()

This code results in the following animation where the Ellipse, inheriting from Circle, preserves the ratio of its half-axes. We get the same output when explicitly specifying the height:

self.play(ellipse.animate.set(width=6).set(height=2))

https://user-images.githubusercontent.com/37160523/152845603-4c7c8e04-dccf-4e5f-9aa6-805d42b1fe56.mp4

Expected behavior

What I expected is that only the width changes, while the height stays the same. This is why I'm using the Ellipse instead of the Circle in the first place. The expected output can be achieved using this code:

class CircleStretch(Scene):
    def construct(self):
        ellipse = Ellipse(width=1, height=2)
        self.add(ellipse)
        self.wait()
        self.play(ellipse.animate.stretch_to_fit_width(6))
        self.wait()

While this works fine, it's just a bit irritating that the other way doesn't work.

https://user-images.githubusercontent.com/37160523/152845838-d084d7b3-7e8a-419d-b171-57247f3d1ba3.mp4

System specifications

System Details
  • OS: Windows 10 Version 21H1
  • RAM: 16 GB
  • Python version: 3.9.7
  • Installed modules (provide output from pip list):
Package             Version
------------------- -----------
atomicwrites        1.4.0      
attrs               21.2.0     
autopep8            1.5.7      
backcall            0.2.0      
beautifulsoup4      4.10.0     
certifi             2021.5.30  
charset-normalizer  2.0.4      
click               8.0.3      
click-default-group 1.2.2
cloup               0.7.1
colorama            0.4.4
colour              0.1.5
commonmark          0.9.1
cycler              0.10.0
debugpy             1.4.1
decorator           5.1.1
dimod               0.10.7
dwave-networkx      0.8.10
dwave-preprocessing 0.3.1.post0
et-xmlfile          1.1.0
glcontext           2.3.4
idna                3.2
iniconfig           1.1.1
ipykernel           6.2.0
ipython             7.26.0
ipython-genutils    0.2.0
isosurfaces         0.1.0
jedi                0.18.0
jupyter-client      6.1.12
jupyter-core        4.7.1
kiwisolver          1.3.1
manim               0.14.0
ManimPango          0.4.0.post2
mapbox-earcut       0.12.11
matplotlib          3.4.3
matplotlib-inline   0.1.2
moderngl            5.6.4
moderngl-window     2.4.1
multipledispatch    0.6.0
networkx            2.6.3
numpy               1.19.3
openpyxl            3.0.8
packaging           21.0
pandas              1.3.2
parso               0.8.2
pickleshare         0.7.5
Pillow              8.0.1
pip                 22.0.3
pluggy              1.0.0
prompt-toolkit      3.0.19
py                  1.10.0
pycairo             1.20.1
pycodestyle         2.7.0
pydub               0.25.1
pyglet              1.5.21
Pygments            2.10.0
pyparsing           2.4.7
pyrr                0.10.3
pytest              6.2.5
python-dateutil     2.8.1
pytz                2021.1
pywin32             301
pyzmq               22.2.1
requests            2.26.0
rich                11.1.0
scipy               1.8.0
screeninfo          0.6.7
setuptools          57.4.0
six                 1.15.0
skia-pathops        0.7.2
soupsieve           2.2.1
srt                 3.5.1
toml                0.10.2
tornado             6.1
tqdm                4.62.3
traitlets           5.0.5
urllib3             1.26.6
watchdog            2.1.6
wcwidth             0.2.5
XlsxWriter          3.0.1
LaTeX details
  • LaTeX distribution: using MiKTeX Console 4.6
  • Installed LaTeX packages: probably not needed for this issue (packages list is huge for MikTeX, one single screenshot won't suffice)
FFMPEG

Output of ffmpeg -version:

4.2.3

Additional comments

Splines avatar Feb 07 '22 18:02 Splines

@Darylgolden Can you confirm this is a bug? I was looking through the animate/mobject methods and saw that @width.setter and @height.setter use scale_to_fit_<height/width> even with objects that have >= 2 specific (and settable) dimensions/axes.

@Splines I cannot reproduce the issue with your code block: self.play(ellipse.animate.set(width=6).set(height=2)) This line does not animate the ellipse at all. Your first code block, however, works and matches the attached video of the ellipse scaling proportionally.

I'd love to be assigned this bug if you think this is something needing to be changed as I already have a potential solution.

WillSoltas avatar Mar 22 '22 22:03 WillSoltas

@WillSoltas You are right, the line should have been: self.play(ellipse.animate.set(height=2).set(width=6)), where we set the height first and then the width. This produces the same (undesirable) result as seen in the first video circle-stretch-1.mp4

The described behavior occurs due to the property setters in mobject.py, here for the width:

@width.setter
def width(self, value):
    self.scale_to_fit_width(value)

whereas we'd like to call stretch_to_fit_width instead as is done in the constructor of the Ellipse in the file geometry.py:

def __init__(self, width=2, height=1, **kwargs):
    super().__init__(**kwargs)
    self.stretch_to_fit_width(width)
    self.stretch_to_fit_height(height)

(just saw this is exactly what you wrote 😅😇)

Splines avatar Mar 23 '22 00:03 Splines

I am not so sure whether there really is a right or wrong here. Generally speaking, given the implementation of the height and width setters for Mobject, the behavior of setting the width or height is consistent for all mobjects throughout Manim -- which is a good thing!

For ellipses and rectangles there is the problem that the keyword arguments are also called height and width, which then interferes with the expected behavior for the global height and width setters.

I dislike the idea of making the height and width setters behave differently for certain mobjects, so either

  • it should be changed globally,
  • or we rename the keyword arguments initializing ellipses and rectangles and make sure that the new keyword arguments are properly set-able.

So far for my 2c, what do you guys think?

behackl avatar Mar 23 '22 07:03 behackl

I like your two possible solutions. My only concern with them, and specifically with renaming the keyword arguments, is backwards compatibility. This is my first time working on Manim, so I'm not sure how big of a concern this should be to begin with. I am also looking into adding an additional attribute like "stretch" to a Mobject animation.

Something like the block below, which would enable users to explicitly select how they want their shape to behave.

self.play(ellipse.animate.stretch.set(width=6)),

or make a new setter for rectangle, ellipse, and any other Mobject that have multiple independent axes.

self.play(ellipse.animate.stretch(width=6)),

Any thoughts on these above?

WillSoltas avatar Mar 29 '22 01:03 WillSoltas