ManimCommunity / ManimCommunity/manim
Refactoring Scene.mobjects
@leotrs is already working on this.
Since Aug 26, 2020.
- Dominant language
- Python
- Stars
- 40.9k
- Forks
- 3.1k
- Avg merge
- 3d 12h
- Merged PRs (30d)
- 25
Description
Currently, `Scene.mobjects` is implemented as a list, and the `Scene` class does a lot of bookkeeping in order to keep it neatly organized. This is necessary because `mobjects` must be kept in the order that mobjects are going to be rendered on screen.
Or at least that _was_ the case, until z-index was implemented #117. It is not without bugs (#327) but I think that keeping the z-order of each mobject is far better than trying to keep the list organized.
The truth is that `Scene.mobjects` should _never_ have been a list. There main reason is that you cannot just choose to `append` something to it. You have to use `restructure_mobjects` every time you touch it. This is done so that mobjects are kept in the right order, but also to avoid duplication, deal with `VGroup`s, etc. So, `Scene.mobjects` is implemented as a list but it's never used as one. Sounds familiar? This means that `Scene.mobjects` should be its own class that handles all of these operations. If `Scene.mobjects` were a different class, then any dev working on `Scene`-derived classes will never have to think about whether to use `append`, `add`, or when to call `restructure_mobjects`.
In my mind I can think of a few things to do here:
1. Extract all of the `Scene.mobjects` logic and define a new class `OrderedMobjectList` or something along those lines.
2. Inside the new class, the `mobjects` collection need not be a list. I think it should be a priority queue instead, where the priority values are the z-order of each mobject.
3. (Maybe) I'm not sure if `mobjects` needs to ever contain a `Group` or `VGroup`. I cannot find a reason why `self.add(some_group)` could't just add each element in the group to the scene, instead of adding the group itself to the scene. Does a `Scene` really need to keep track of which objects are grouped together? If the only reason to do this is to keep track of rendering order, we already have z-index for that!
Please share your thoughts.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.