ManimCommunity / ManimCommunity/manim

Refactoring Scene.mobjects

Open
#358 6 comments 0 reactions 1 assignee View on GitHub

@leotrs is already working on this.

Since Aug 26, 2020.

refactor
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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.