-
Notifications
You must be signed in to change notification settings - Fork 2.3k
DRAFT: remove all OpenGL<x>
mobs and the ConvertToOpenGL
metaclass.
#2454
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
for more information, see https://pre-commit.ci
Closing in favor of discussing a new architecture first |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
maintenance
refactoring, typos, removing clutter/dead code, and other code quality improvements
opengl
Concerning the OpenGL renderer.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview: What does this pull request change?
The theme of this PR is removing all
OpengGL<x>
mobs and the consequences of their existence. It's still a work in progress, but I decided to share it as a proof of concept for how to approach addressing the problem.The commit history outlines the changes, but here are specific descriptions of what was done:
Summary of changes:
if config.renderer
/config["renderer"] == "opengl"
checks were removed from the library."opengl"
choices in these checks were kept.opengl_geometry.py
was removed.OpenGLMobject --> Mobject
mobject.py
was removed, andopengl_mobject.py
was renamed tomobject.py
.OpenGL
prefixes in the file were removed.OpenGLGroup
->Group
OpenGLVMobject --> VMobject
vectorized_mobject.py
was removed, andopengl_vectorized_mobject.py
was renamed tovectorized_mobject.py
.DashedVMobject
was ported over toOpenGLDashedVMobject
VDict
was ported over to the file.OpenGL
prefixes were removed.OpenGLSurface (and related 3d opengl mobs) -- > Surface
opengl_surface.py
was renamed tosurface.py
.opengl_three_dimensions.py
was removed, and its only mob ofOpenGLSurfaceMesh
was moved intosurface.py
and renamed toSurfaceMesh
OpenGL
prefixes removedOpenGLPMobject (and related PMobjects) --> PMobject
point_cloud_mobject.py
was removed andopengl_point_cloud_mobject.py
was renamed topoint_cloud_mobject.py
.OpenGL
prefixes were removed.ConvertToOpenGL
opengl_compatibilty.py
was removed, and all mentions to it too.all
metaclass = ConvertToOpenGL
in class definitions were removed.Axes
/ mobs ingeometry.py
andthree_dimensions.py
.removed
manim.opengl
import in self.interactive_embed() too.Outcome
with the following command:
The following scene successfully renders and produces an interactive window.
--renderer=opengl
is still required since the flags are not configured properly. Also,manim/__init__.py
has not been cleaned up appropiately, so it's currently missing:that are included in
manim/opengl/__init__.py
.There are also some redundant cairo files, such as
cairo_renderer.py
/camera.py
that should be removed.OpenGL
's default camera makesThreeDScene
mostly redundant, but I did not remove it since it still has some useful methods that should be ported beforehand.Motivation and Explanation: Why and how do your changes improve the library?
Instead of further trying to accommodate the clashing implementations in the library, I think the best solution is to simply remove the old
Mobject
etc.. mobs and have them be replaced by theirOpenGL
counterparts.With some notable exceptions such as #2155 #1240 and
ImageMobject
(in progress at #1837) , the OpenGL renderer generally has the same functionality as cairo. There are almost certainly some bugs / behaviour that we are unaware of, but IMO this PR is a step in the right direction towards solving those issues.Links to added or changed documentation pages
Docs won't render.
Further Information and Comments
Tests will fail.
Reviewer Checklist