Skip to content

Fix VGroup.scale(scale_stroke=True) to Respect Submobjects' Stroke Widths #4231

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

irvanalhaq9
Copy link
Contributor

Overview: What does this pull request change?

  1. Fix: Argument scale_stroke of scale method does not behaves as expected on a VGroup #4229
  2. Add related test

Further Information and Comments

Example:

class FixVGroupScaling(Scene):
    def construct(self):
        square = Square(side_length=2).set_stroke(color=RED_E, width=20)
        square.set_stroke(color=WHITE, width=60, background=True)
        self.add(square)
        vg = VGroup(square)
        
        # Scale 1.0 (scale_stroke=True): No changes expected
        self.play(vg.animate.scale(1, scale_stroke=True))

        # Scale 0.5 (scale_stroke=True): Size and stroke width halved
        self.play(vg.animate.scale(0.5, scale_stroke=True))
        
        # Scale 2.0 (scale_stroke=False): Size doubled, stroke width unchanged
        self.play(vg.animate.scale(2))
        self.wait()
FixVGroupScaling.mp4

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

@ccalauzenes
Copy link

Hello,

Thank you for the super quick fix. I have one question though: Isn't the fix super dependent on the implicit fact that get_family() returns self first? Because the set_stroke of self is called first, setting wrongly the value recursively to the submobjects, and then it's overloaded by calling again the set_stroke of each of the submobjects

manim/manim/mobject/mobject.py

Lines 2311 to 2339 in f304bd9

def get_family(self, recurse: bool = True) -> list[Self]:
"""Lists all mobjects in the hierarchy (family) of the given mobject,
including the mobject itself and all its submobjects recursively.
Parameters
----------
recurse
Just for consistency with get_family method in OpenGLMobject.
Returns
-------
list
A list of mobjects in the family of the given mobject.
Examples
--------
::
>>> from manim import Square, Rectangle, VGroup, Group, Mobject, VMobject
>>> s, r, m, v = Square(), Rectangle(), Mobject(), VMobject()
>>> vg = VGroup(s, r)
>>> gr = Group(vg, m, v)
>>> gr.get_family()
[Group, VGroup(Square, Rectangle), Square, Rectangle, Mobject, VMobject]
"""
sub_families = [x.get_family() for x in self.submobjects]
all_mobjects = [self] + list(it.chain(*sub_families))
return remove_list_redundancies(all_mobjects)

Maybe, we could do the same thing, but explicitly, by calling first set_stroke on self and then on each of the self.submobjects.

@irvanalhaq9
Copy link
Contributor Author

irvanalhaq9 commented Apr 30, 2025

Hi @ccalauzenes
Thanks for reviewing. You're right. The method .set_stroke() should only be applied to the mobject, not to its submobjects.
I have fixed it by adding family=False.
Test:

class FixVGroupScaling2(Scene):
    def construct(self):
        square = Square(side_length=2)
        square.set_stroke(color=RED_E, width=40)
        square.set_stroke(color=WHITE, width=120, background=True)
        rec = Rectangle(height=4,width=5)
        rec.set_stroke(color=RED, width=10)
        rec.set_stroke(color=YELLOW_A, width=20, background=True)
        
        vg = VGroup(square)
        rec.add(vg)
        self.add(rec)

        self.play(rec.animate.scale(1, scale_stroke=True))
        self.play(rec.animate.scale(0.5, scale_stroke=True))
        self.play(rec.animate.scale(2))
        self.wait()
FixVGroupScaling2.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

Argument scale_stroke of scale method does not behaves as expected on a VGroup
2 participants