Port Graph to OpenGL #1202

eulertour · 2021-03-31T07:53:05Z

Changelog / Overview

Port Graph to OpenGL

Depends on #1200

Motivation

Allows using Graph in OpenGL

Explanation for Changes

Ports LabeledDot and Graph to OpenGL

Testing Status

None, though I'd be willing to add some if I can find how to create OpenGL test data.

Further Comments

Checklist

I have read the Contributing Guidelines
I have written a descriptive PR title (see top of PR template for examples)
I have written a changelog entry for the PR or deem it unnecessary
My new functions/classes either have a docstring or are private
My new functions/classes have tests added and (optional) examples in the docs
My new documentation builds, looks correctly formatted, and adds no additional build warnings

Reviewer Checklist

The PR title is descriptive enough
The PR is labeled correctly
The changelog entry is completed if necessary
Newly added functions/classes either have a docstring or are private
Newly added functions/classes have tests added and (optional) examples in the docs
Newly added documentation builds, looks correctly formatted, and adds no additional build warnings

github-actions · 2021-04-12T00:08:56Z

🎉 Great news! Looks like all the dependencies have been resolved:

~~Add text and SVG mobjects to OpenGL #1200~~

💡 To add or remove a dependency please update this issue/PR description.

Brought to you by Dependent Issues (:robot: ). Happy coding!

jsonvillanueva

So far this looks good. config.renderer == "opengl" is more recent/not deprecated, so just minor issues. I'm still looking through the three largest files (opengl_graph.py, opengl_tex_mobject.py, and opengl_text_mobject.py). More on those files later, but I have to run for now.

jsonvillanueva · 2021-04-14T19:36:51Z

manim/animation/transform.py

        # Note, this potentially changes the structure
        # of both mobject and target_mobject
-        self.mobject.align_data(self.target_copy)
+        if config["use_opengl_renderer"]:


Suggested change

if config["use_opengl_renderer"]:

if config.renderer == "opengl":

jsonvillanueva · 2021-04-14T19:37:22Z

manim/mobject/mobject.py


    def __repr__(self):
-        return str(self.name)
+        if config["use_opengl_renderer"]:


Suggested change

if config["use_opengl_renderer"]:

if config.renderer == "opengl":

jsonvillanueva · 2021-04-14T19:39:24Z

manim/utils/space_ops.py

    This angle will always be btw 0 and pi
    """
-    return np.arccos(fdiv(np.dot(v1, v2), get_norm(v1) * get_norm(v2)))
+    if config["use_opengl_renderer"]:


Suggested change

if config["use_opengl_renderer"]:

if config.renderer == "opengl":

jsonvillanueva · 2021-04-14T19:40:28Z

manim/mobject/opengl_geometry.py



+class OpenGLLabeledDot(OpenGLDot):
+    """A :class:`Dot` containing a label in its center.


Suggested change

"""A :class:`Dot` containing a label in its center.

"""A :class:`~.OpenGLDot` containing a label in its center.

jsonvillanueva · 2021-04-14T19:41:26Z

manim/mobject/svg/svg_path.py

        for command, coord_string in pairs:
            self.handle_command(command, coord_string, prev_command)
            prev_command = command
+        if config["use_opengl_renderer"]:


Suggested change

if config["use_opengl_renderer"]:

if config.renderer == "opengl":

jsonvillanueva · 2021-04-14T19:42:23Z

manim/mobject/svg/svg_path.py

            return

        elif command == "S":  # Smooth cubic
+            if config["use_opengl_renderer"]:


Suggested change

if config["use_opengl_renderer"]:

if config.renderer == "opengl":

jsonvillanueva · 2021-04-14T19:42:38Z

manim/mobject/svg/svg_path.py


        elif command == "Z":  # closepath
-            self.add_line_to(self.current_path_start)
+            if config["use_opengl_renderer"]:


Suggested change

if config["use_opengl_renderer"]:

if config.renderer == "opengl":

jsonvillanueva · 2021-04-14T19:43:53Z

manim/mobject/svg/svg_mobject.py

-                height=self.attribute_to_float(rect_element.getAttribute("height")),
-                **parsed_style,
-            )
+            if config["use_opengl_renderer"]:


Suggested change

if config["use_opengl_renderer"]:

if config.renderer == "opengl":

jsonvillanueva · 2021-04-14T19:44:12Z

manim/mobject/svg/svg_mobject.py

+            self.handle_transforms(element, VGroup(*result))
        if len(result) > 1 and not self.unpack_groups:
-            result = [VGroup(*result)]
+            if config["use_opengl_renderer"]:


Suggested change

if config["use_opengl_renderer"]:

if config.renderer == "opengl":

jsonvillanueva · 2021-04-14T19:44:21Z

manim/mobject/svg/svg_mobject.py


        result = [m for m in result if m is not None]
-        self.handle_transforms(element, VGroup(*result))
+        if config["use_opengl_renderer"]:


Suggested change

if config["use_opengl_renderer"]:

if config.renderer == "opengl":

behackl · 2021-04-14T20:07:37Z

The actual diff for this PR should be much smaller (and as soon as the merge conflict is resolved it should show in the GH interface), given that the #1200 has already been merged. I've diffed graph.py and opengl_graph.py, the changes there pretty much look like all of this could be covered by the metaclass approach discussed in #1272 without any further adjustments.

Given that I don't have a lot of time in the coming days, I fear that #1272 won't be finalized/merged too soon -- so in principle I'd be fine with merging this PR and adding opengl_graph.py to the list of files that will be removed again after #1272 allows avoiding code duplication.

jsonvillanueva · 2021-04-14T21:01:44Z

The actual diff for this PR should be much smaller (and as soon as the merge conflict is resolved it should show in the GH interface), given that the #1200 has already been merged.

@eulertour Since that's the case, I'll wait until the merge-conflicts are handled before finishing my review. Just request a review or ping me on Discord and I'll get to it ASAP.

behackl · 2021-05-14T19:52:18Z

I'd prefer to make Graph available via #1512 and avoid duplication of the graph module.

behackl · 2021-05-22T08:50:01Z

Closing this, as #1512 has been merged.

eulertour added 9 commits March 27, 2021 01:14

Tex working with artifacts

ab8b92d

Add OpenGLTex

752dce3

Add OpenGLText

e3153f7

Add OpenGLText and related classes

0c7e98a

Add OpenGLText and related classes

c61bf02

Merge remote-tracking branch 'origin/master' into opengl-tex

1f010ac

Undo changes to example code

bf3dba2

Checks

2a861d0

Add OpenGLGraph, OpenGLLabelledDot

7393b36

github-actions bot added the pr:dependent This PR or issue requires that another PR is merged first label Mar 31, 2021

github-actions bot removed the pr:dependent This PR or issue requires that another PR is merged first label Apr 12, 2021

jsonvillanueva requested changes Apr 14, 2021

View reviewed changes

jsonvillanueva added the new feature Enhancement specifically adding a new feature (feature request should be used for issues instead) label Apr 15, 2021

behackl closed this May 22, 2021

behackl mentioned this pull request Jun 30, 2021

Let OpenGLMobject.copy return a deep copy by default #1733

Merged

12 tasks

	if config["use_opengl_renderer"]:
	if config.renderer == "opengl":



		class OpenGLLabeledDot(OpenGLDot):
		"""A :class:`Dot` containing a label in its center.

	"""A :class:`Dot` containing a label in its center.
	"""A :class:`~.OpenGLDot` containing a label in its center.

Port Graph to OpenGL #1202

Port Graph to OpenGL #1202

Uh oh!

Conversation

eulertour commented Mar 31, 2021

Changelog / Overview

Motivation

Explanation for Changes

Testing Status

Further Comments

Checklist

Reviewer Checklist

Uh oh!

github-actions bot commented Apr 12, 2021

Uh oh!

jsonvillanueva left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

behackl commented Apr 14, 2021

Uh oh!

jsonvillanueva commented Apr 14, 2021

Uh oh!

behackl commented May 14, 2021

Uh oh!

behackl commented May 22, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants