manim
manim copied to clipboard
enable z_index in Circumscribe
Overview: What does this pull request change?
adds z_index parameter to Circumscribe constructor so it can be used for the z_index of the frame (SurroundingRectangle or Circle)
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
That SGTM, but I'd rather wait for @behackl approval.
Do you mind trying to add some test about that ? It's actually a corner case of manim interesting to test (the behaviour of surrounding shapes when z_index is special)
Sure - I'll take a look at the test suite at the weekend and see what I can do.
Keep in mind the effects for the opengl renderer! It doesn't support z_index
Keep in mind the effects for the opengl renderer! It doesn't support
z_index
Just to piggyback on what MrDiver said :sweat_smile: I don't really like the concept of z_index in general and IMO it is more intuitive to replace that with a change of the Z-coordinate in general. I think that would functionally be equally useful as an indicator for Cairo to decide which Mobjects to draw in front of the others in a 2D scenario (maybe I'm missing something for the 3D scenes in Cairo, tho), while at the same time making sense for OpenGL and just being more intuitive and consistent with our human 3D reality in general.
But apart from that, I think that everything that the three of us said is not really in the scope of this PR, and I also like the changes.
I would like to again spark some discussion about this :eyes:
I would be on team @behackl and think we should move it and maybe also have something like a Constructor parameter for a more general Circumscribe animation? But that's just an idea.
But mainly moving the parameters would be great!
Closing due to inactivity.