Skip to content

fix initialization for :class:~.Paragraph #2014

Closed
UraniumCronorum wants to merge 5 commits intoManimCommunity:mainfrom
UraniumCronorum:patch-2
Closed

fix initialization for :class:~.Paragraph #2014
UraniumCronorum wants to merge 5 commits intoManimCommunity:mainfrom
UraniumCronorum:patch-2

Conversation

@UraniumCronorum
Copy link
Copy Markdown
Contributor

Overview: What does this pull request change?

The current implementation of Paragraph's initialization erroneously counts whitespace when initializing internal variables, leading to line division in self.lines and self.chars being off. This in turn causes unexpected behavior when attempting to set or change the alignment of those lines.

This PR corrects this behavior by basing calculation of line lengths on the length of the corresponding Text object instead of the length of the string.

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

class ParagraphExample(Scene):
    def construct(self):
        right = Paragraph('This is a paragraph with text aligned',
                          'to the', 'right', alignment='right')
        left = Paragraph('This is a paragraph with text aligned',
                          'to the', 'left', alignment='left').next_to(right,UP)
        center = Paragraph('Here is a paragraph with text',
                           'centered',
                           alignment='center').next_to(right,DOWN)

        self.add(right, left, center)

Before:
before
After:
after

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

]
)
char_index_counter += lines_str_list[line_no].__len__() + 1
char_index_counter += len(Text(lines_str_list[line_no]))
Copy link
Copy Markdown
Collaborator

@k4pran k4pran Sep 8, 2021

Choose a reason for hiding this comment

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

Seems this is only created to get the length of the Text right? Is there a way to avoid creating this? It seems to impact performance for the opengl renderer (that already has performance issues). Same with the other places where Text is created only to check the length

Copy link
Copy Markdown
Collaborator

@k4pran k4pran left a comment

Choose a reason for hiding this comment

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

Thank you for this fix. Tested locally and the fix works, I am concerned though with performance impacts of creating multiple Text objects to check the lengths

@k4pran k4pran added the pr:bugfix Bug fix for use in PRs solving a specific issue:bug label Sep 8, 2021
@MrDiver
Copy link
Copy Markdown
Member

MrDiver commented Jun 18, 2022

This is probably fixed by this PR too. Correct me if im wrong and reopen but i will close this for now!

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

Labels

pr:bugfix Bug fix for use in PRs solving a specific issue:bug

Projects

Status: ❌ Rejected

Development

Successfully merging this pull request may close these issues.

3 participants