Skip to content

Fixed inconsistent stroke width scaling for text in compound objects#4694

Open
vihdutta wants to merge 3 commits intoManimCommunity:mainfrom
vihdutta:consistent_mobject_scaling
Open

Fixed inconsistent stroke width scaling for text in compound objects#4694
vihdutta wants to merge 3 commits intoManimCommunity:mainfrom
vihdutta:consistent_mobject_scaling

Conversation

@vihdutta
Copy link
Copy Markdown

Overview: What does this pull request change?

The current code for scale() in vectorized_mobject.py takes the root mobject's stroke width and sets stroke with family=True as the default. This results in that width being pushed to all descendants including objects with a zero stroke width which should keep their width #4648 . I've modified the code to scale each object's own stroke width which resolves the issue.

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

Links to added or changed documentation pages

I didn't modify any documentation other than the docstring text for vectorized_mobject.py under VMobject's scale() function for the scale_stroke description.

Further Information and Comments

This is my first time contributing to open source! Let me know if there's other things I had forgotten to consider (comments, documentation wise, etc.)

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

Copy link
Copy Markdown
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 does look reasonable to me, thanks for catching and fixing this!

@vihdutta
Copy link
Copy Markdown
Author

Looks like everything's great! I'm new to open source so forgive the question, but how do I have this merged with the main branch now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants