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

Fixed Some bugs of code_mobject.py and Text_mobject.py #1071

Closed
wants to merge 42 commits into from
Closed

Fixed Some bugs of code_mobject.py and Text_mobject.py #1071

wants to merge 42 commits into from

Conversation

NavpreetDevpuri
Copy link
Contributor

@NavpreetDevpuri NavpreetDevpuri commented May 14, 2020

Fixed #1067
NOTE : SurroundingRectangle() only contains visible text.

class te(Scene):
    def construct(self):
        text1 = Text(" ab ", font="Consolas", size=2)
        rect1 = SurroundingRectangle(text1)
        text1.chars[1].set_color(GREEN)
        self.add(text1,rect1)

Output
te

class bug1(Scene):
    def construct(self):
        text1 = Text("  ab\ncd", font="Consolas", size=2)
        text2 = Text("ab\ngh", font="Consolas", size=2)
        rect1 = SurroundingRectangle(text1)
        rect2 = SurroundingRectangle(text2)
        self.play(Transform(remove_invisible_chars(text1), remove_invisible_chars(text2)), Transform(rect1, rect2))
        self.wait()

Output
bug1

Added new parameters background_stroke_width and background_stroke_color

class bug2andbug3(Scene):
    def construct(self):
        helloworldc = Code(
            "helloworldc.c",
            tab_width=4,
            background_stroke_width=1,
            background_stroke_color=WHITE,
            insert_line_no=True,
            style=Code.styles_list[4],
            background="window",
            language="cpp",
        )
        helloworldcpp = Code(
            "helloworldcpp.cpp",
            tab_width=4,
            background_stroke_width=1,
            background_stroke_color=WHITE,
            insert_line_no=True,
            style=Code.styles_list[15],
            background="window",
            language="cpp",
        )
        helloworldc.move_to(np.array([-3.6, 0, 0]))
        helloworldcpp.move_to(np.array([3.1, 0, 0]))
        self.add(helloworldc,helloworldcpp)

Output
bug2andbug3

Updated Paragraph() and added new methods set_line_to_initial_position() and set_all_lines_to_initial_positions()

class para(Scene):
    def construct(self):
        t = Paragraph('this is a awesome', 'paragraph', 'With \nNewlines', '\tWith Tabs', '  With Spaces',
                      'With Alignments',
                      'center', "left", "right")
        t.set_line_alignment("center", 7)
        t.set_line_alignment("right", 9)
        t.shift(2 * RIGHT)
        rect = SurroundingRectangle(t)
        self.add(t, rect)
        self.wait()
        self.play(t.set_all_lines_alignments, "right")
        self.play(t.set_line_to_initial_position, 4)
        self.play(t.set_all_lines_to_initial_positions)
        self.play(t.set_line_alignment, "center", 7)
        self.play(t.set_line_alignment, "right", 9)
        t.chars[0][5:7].set_color(GREEN)
        t[1][0:4].set_color(YELLOW)
        self.wait()

Output
ezgif-3-3dc23c79c5fa

@TonyCrane
Copy link
Collaborator

TonyCrane commented May 15, 2020

Good job!
But can you fix the second bug mentioned in #1067, by making Code() distinguish between space and tabs for identation.

And I think it's better to put front space characters in the first display character's position, which makes them in the first line. This can make Transform more natural.

@NavpreetDevpuri
Copy link
Contributor Author

NavpreetDevpuri commented May 15, 2020

But can you fix the second bug mentioned in #1067, by making Code() distinguish between space and tabs for identation.
And I think it's better to put front space characters in the first display character's position, which makes them in the first line. This can make Transform more natural.

i don't get you.

i just make cairo to handle spaces or tabs (replaced by tab_width number of spaces before cairo)
and put empty Dot() as spaces at the center of previous character in text so characters indexes works correctly and transform() works correctly because Dot() are placed under some other character.

@TonyCrane
Copy link
Collaborator

Sorry for your failure to understand me.
The indexes work well. But there is still a little problem with Transform()
now the structure of Text() is like this:
image
all of the front spaces(Dot()s) are in the place of the last character(e), this cause the last character Transform to the first character(not in the same line) when TransformText.
So I think it will be better if you can put the front spaces(Dot()s which are not displayed) in the place of the first character('a' which in the same line as front spaces), just like:
image
I try it manually:
before:

text = Text("  ab\ncd\nef", font="Consolas", size=2)
text2 = Text("ab\n  cd\nef", font="Consolas", size=2)
self.add(text)
self.wait()
self.play(Transform(text, text2))
self.wait()

Test105
(See that? 'ab' is growing from 'f' when transforming)
after:

text = Text("  ab\ncd\nef", font="Consolas", size=2)
text2 = Text("ab\n  cd\nef", font="Consolas", size=2)

text[:2].move_to(text[2])

self.add(text)
self.wait()
self.play(Transform(text, text2))
self.wait()

Test105

@NavpreetDevpuri
Copy link
Contributor Author

NavpreetDevpuri commented May 15, 2020

Now i get it
actually i did that i copied line and forgot to remove -1 or may be i undo for something or mistakenly did it more then one.

    def apply_space_chars(self):
        for char_index in range(self.text.__len__()):
            if self.text[char_index] == " " or self.text[char_index] == "\t" or self.text[char_index] == "\n":
                space = Dot(redius=0, fill_opacity=0, stroke_opacity=0)
                if char_index == 0:
                    #Here is the mistake i forgot to remove -1 
                    space.move_to(self.submobjects[char_index - 1].get_center())
                else:
                    space.move_to(self.submobjects[char_index - 1].get_center())
                self.submobjects.insert(char_index, space)

Comment on lines 95 to 96
if char_index == 0:
space.move_to(self.submobjects[char_index - 1].get_center())
Copy link
Collaborator

Choose a reason for hiding this comment

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

focus on here.
You put all non-display characters in the position of the previous character.
But if char_index==0, the previous character is the last display character in the entire Text.

It's better to put them in the position of the first display character instead of the last one.

@NavpreetDevpuri
Copy link
Contributor Author

Yes corrected !

@TonyCrane
Copy link
Collaborator

Good Job!
What about Bug 2. You can check the input code file first for identation, and replace tab_width spaces with a 'tab' before all.

@NavpreetDevpuri
Copy link
Contributor Author

if i remove this lines

        SVGMobject.__init__(self, file_name, **config)
        #self.text = text
        #self.apply_space_chars()

It shows more smooth Transformation
ezgif-6-1faf6ae91b48

But this will result into failure of indexes i.e spaces are not counted

@NavpreetDevpuri
Copy link
Contributor Author

NavpreetDevpuri commented May 15, 2020

its better if we just accept that space characters are not counted and make Text() that way.
what do you think?

@TonyCrane
Copy link
Collaborator

Yes, Tranform's problems are all above the spaces.
If you can only receive spaces, but do not allow spaces to participate in Transform, it is better.
However, it is still required to calculate spaces on the index, which is a feature of Text.
For example: text=Text("ab cd")
if we don't display spaces(don't let spaces participate in Transform), but still let text[4]='c', This is the best. But it seems difficult to achieve.

@NavpreetDevpuri
Copy link
Contributor Author

well i have an idea but for now i don't know to achieve that
the idea is to introduce a None submobject that don't participate in any of submobject's things like transformation.

@NavpreetDevpuri
Copy link
Contributor Author

NavpreetDevpuri commented May 15, 2020

can you explain me
let text=Text("ab cd")
now my question is why text is acting as an array like text[4]='c' and why text is equivalate to text.submobject

@TonyCrane
Copy link
Collaborator

oops, I typed two spaces between b and c, but maybe GitHub shows only one space.

Text is a subclass of SVGMobject, so the submobjects of Text is each path(character) in the svg which cairo made.
I said text[4]='c' is just for convenience. My meaning is that text[4] points to the object with the character 'c' in text.

@TonyCrane
Copy link
Collaborator

TonyCrane commented May 15, 2020

@NavpreetDevpuri I solved Bug2.

def replace_identation(self):
    with open(self.file_path, 'r') as fpr:
        content = fpr.read()
    content = re.sub(' ' * self.tab_width, '\t', content)
    with open(self.file_path, 'w') as fpw:
        fpw.write(content)

add this method to Code(), and use it after ensure_valid_fild()

self.ensure_valid_file()
self.replace_identation()
self.style = self.style.lower()

You can commit it to your pr. and it will be perfect.

@NavpreetDevpuri
Copy link
Contributor Author

NavpreetDevpuri commented May 15, 2020

@NavpreetDevpuri I solved Bug2.
content = re.sub(' ' * self.tab_width, '\t', content)

this will replace all spaces from file for example printf(" ab") will convert to printf("\tab"
you may confused about tab_width and indentation_char
tab_width defines number of spaces in indentation
there is another parameter indentation_char which defines how many spaces is equal to one indentation for example some may have only two spaces defines one indentation as for your code its four spaces equal to one indentation

we have to use indentation_char because we don't know how many spaces user defines equal to indentation

@NavpreetDevpuri
Copy link
Contributor Author

m working on it

class te1(Scene):
    def construct(self):
        text = Text("  ab\ncd\nef", font="Consolas", size=2)
        text2 = Text("ab\n  cd\nef", font="Consolas", size=2)
        self.add(text)
        text.chars[2].set_color(YELLOW)
        self.wait()
        text.chars[0:4].set_color(RED)
        text2.chars[0:2].set_color(GREEN)
        text.set_color_by_t2c(t2c={"cd": GREEN})
        self.wait()
        # Wrong Transform(text.chars[0:4], text2.chars[0:2])
        self.play(Transform(text.get_sub_text(0, 4), text2.get_sub_text(0, 2)))
        self.wait()

ezgif-6-7961e5648f9b

Transformations are smoother and we can access chars by text.chars[] and get sub text by text.get_sub_text()

now m working on Paragraph() and Code()

@TonyCrane
Copy link
Collaborator

we have to use indentation_char because we don't know how many spaces user defines equal to indentation

Oh, Sorry. I didn't notice the keyword indentation_char before. This is not a bug, so sorry.

@NavpreetDevpuri
Copy link
Contributor Author

No problem

@TonyCrane
Copy link
Collaborator

m working on it

Transformations are smoother and we can access chars by text.chars[] and get sub text by text.get_sub_text()

now m working on Paragraph() and Code()

But don't you think this will make Transform more complicated.
I think it is good enough now.

@NavpreetDevpuri
Copy link
Contributor Author

NavpreetDevpuri commented May 15, 2020

But don't you think this will make Transform more complicated.

yes, but then we just have choice that

  • use simple text[] to access characters without counting spaces(invisible chars)
  • use text.chars[] and text.get_sub_text() counting spaces.

@NavpreetDevpuri
Copy link
Contributor Author

# WRONG
        #self.play(Transform(text.chars[0:4], text2.chars[0:2]))
        self.play(Transform(remove_spaces_from_chars(text.chars[0:4]), remove_spaces_from_chars(text2.chars[0:2])))

Now look better remove_spaces_from_chars() for removing spaces from text.chars[]

@TonyCrane
Copy link
Collaborator

I still think it is too complicated.

@NavpreetDevpuri NavpreetDevpuri marked this pull request as ready for review May 26, 2020 16:40
NavpreetDevpuri and others added 15 commits June 21, 2020 07:52
Co-authored-by: Pg Biel <9021226+PgBiel@users.noreply.github.com>
Co-authored-by: Pg Biel <9021226+PgBiel@users.noreply.github.com>
Co-authored-by: Pg Biel <9021226+PgBiel@users.noreply.github.com>
Co-authored-by: Pg Biel <9021226+PgBiel@users.noreply.github.com>
Co-authored-by: Pg Biel <9021226+PgBiel@users.noreply.github.com>
Co-authored-by: Pg Biel <9021226+PgBiel@users.noreply.github.com>
Co-authored-by: Pg Biel <9021226+PgBiel@users.noreply.github.com>
Co-authored-by: Pg Biel <9021226+PgBiel@users.noreply.github.com>
Co-authored-by: Pg Biel <9021226+PgBiel@users.noreply.github.com>
Co-authored-by: Pg Biel <9021226+PgBiel@users.noreply.github.com>
@NavpreetDevpuri
Copy link
Contributor Author

reopened at ManimCommunity/manim#198

eulertour pushed a commit to eulertour/manim-3b1b that referenced this pull request Mar 31, 2021
* Enabled coverage.py based code coverage

* integrate codecov into ci

* move to pytest-cov and include .coveragerc into pyproject.toml

* update poetry

* Also include coverage in CI logs

* updated poetry
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.

Some bugs of code_mobject.py
2 participants