New base class for Mobject and OpenGLMobject
Overview: What does this pull request change?
This is a first step towards cleaning up the current code duplication mess between Mobject and OpenGLMobject. The aim is to factor out all Mobject methods that can be implemented more or less painlessly in a renderer-agnostic way in a joint base class of Mobject and OpenGLMobject.
As of typing this, this PR currently does that for bounding_box related methods, some tests still fail locally. Work in progress.
Further Information and Comments
This is only in case someone is curious about what I'm doing, it's not ready to be reviewed. Will probably also not be merged into main branch, but rather our refactor development branch, once it exists.
Wouldn't it introduce less redundant code to simply do away with Mobject and have it be superseded by OpenGLMobject? Adding a MobjectBase doesn't seem like a long term solution 🤔.
Wouldn't it introduce less redundant code to simply do away with
Mobjectand have it be superseded byOpenGLMobject? Adding aMobjectBasedoesn't seem like a long term solution 🤔.
Ultimately, this is what will happen -- but in a very controlled way. The implementations in MobjectBase should be as close to the ones in OpenGLMobject as possible, and ideally there would be one mobject base class in the end instead of two.
I don't trust OpenGL rendering enough to just rip Cairo out, but that's just my 2 cents.
Current state of this: last time I pushed this there was a serious performance issue, this should now be resolved (or at the very least be substantially less serious).
The two biggest changes to the Mobject base class (so far) are:
- it now uses the same (actually an improved version)
bounding_boximplementation as OpenGLMobject (i.e., the bounding box is computed upon request, cached, and invalidated once the mobject or its children changes -- and recomputed only upon request again; this is done with thebounding_boxproperty and related methods). Critical points (aka bounding box points for OpenGL mobjects) are computed using the cached bounding box. - mobjects now also keep track of which other mobjects they are contained in (via the
mobject.parentsattribute).
There are still many further methods that can be factored out. In the end, both mobject.py and opengl_mobject.py should only have very few methods (maybe around 5-10?) left that require large-scale changes to unify. I'd like to do these in a separate PR, just to keep things clean. Tests don't all pass yet (there are 3 failing SVGMobject tests) due to (as far as I can tell) a change in how they are positioned. Might be a bug with the bounding_box implementation; might also be because the current positioning is actually buggy. Not sure yet, am investigating.
I'm thinking we should delay merging this until we have a lot more unit tests. What do you think?
I'm thinking we should delay merging this until we have a lot more unit tests. What do you think?
It is not ready for being merged anyways. My plan would have been to add tests for all methods of the new base class before merging this.