Skip to content
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

Fix: OpenGL BackgroundRectangle infinite loop #3732

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

camburd2
Copy link

Overview: What does this pull request change?

Fixes #3634 by modifying adding a line and changing a line in opengl_vectorized_mobject.py. After reproducing the issue with the given test case, I found that there was an infinite loop involving calls to get_color() in opengl_vectorized_mobject.py for the background object.

Motivation and Explanation: Why and how do your changes improve the library?

I compared opengl_vectorized_mobject.py (OVM) to vectorized_mobject.py (VM) used by cairo. I noticed the if condition was the only difference. The condition in VM did not check stroke but instead checked fill opacities.

I essentially made the OVM condition equivalent to the VM condition - using OVM's has_fill(). This change, along with adding a missing attribute (mobject_style["fill_opacity"]), fixed the bug. For reference:

in vectorized_mobject.py:

    def get_color(self) -> ManimColor:
        if np.all(self.get_fill_opacities() == 0):
            return self.get_stroke_color()
        return self.get_fill_color()

current opengl_vectorized_mobject.py:

    def get_color(self):
        if self.has_stroke():
            return self.get_stroke_color()
        return self.get_fill_color()
    def has_fill(self):
        fill_opacities = self.get_fill_opacities()
        return fill_opacities is not None and any(fill_opacities)

proposed opengl_vectorized_mobject.py:

    def get_color(self):
        if not self.has_fill():
            return self.get_stroke_color()
        return self.get_fill_color()

Links to added or changed documentation pages

Further Information and Comments

Reviewer Checklist

  • The PR title is descriptive enough for the changelog, and the PR is labeled correctly
  • If applicable: newly added non-private functions and classes have a docstring including a short summary and a PARAMETERS section
  • If applicable: newly added functions and classes are tested

@MrDiver MrDiver added the opengl Concerning the OpenGL renderer. label Apr 28, 2024
@JasonGrace2282 JasonGrace2282 added the pr:bugfix Bug fix for use in PRs solving a specific issue:bug label Apr 29, 2024
Copy link
Member

@behackl behackl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine to me, thanks! It would probably be nice to include a non-graphical test in tests/opengl/test_opengl_vectorized_mobject.py -- would you be willing to look into this, @camburd2?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
opengl Concerning the OpenGL renderer. pr:bugfix Bug fix for use in PRs solving a specific issue:bug
Projects
Status: 👍 To be merged
Development

Successfully merging this pull request may close these issues.

OpenGL: BackgroundRectangle.color causes infinite loop
4 participants