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 triangulation #1586

Merged
merged 1 commit into from
Jul 28, 2021
Merged

Fix triangulation #1586

merged 1 commit into from
Jul 28, 2021

Conversation

Wallbreaker5th
Copy link
Contributor

Motivation

From #1331 we can know that the triangulation is sometimes incorrect. #1552 offers a temporary hack for showing text correctly, but it still can't deal with all situations. This pull request aims to fix the bug completely.

Proposed changes

I rewrote the function earclip_triangulation. The function earcut can deal with one polygon with empty holes (without other rings in the hole), so I just deal with the shapes in a hole separately.

Also, I removed the temporary hack in text_mobject.py.

It runs almost as fast as before (before #1552).

Test

Code:

from manimlib import *
import time

class FontTest(Scene):
    def construct(self):
        start=time.time()

        chinese_text="疆域 画面 藤蔓 寒颤 回复 困难 \n蛐蛐 量取 魓 圖 轟 畾 體 團 垇 儽"
        text1 = Text(chinese_text, font="SimSun").scale(1.3).shift(UP*3)
        text2 = Text(chinese_text, font="SimHei").scale(1.3).next_to(text1,DOWN)
        text3 = Text(chinese_text, font="YouYuan").scale(1.3).next_to(text2,DOWN)
        emoji = Text("😀🛒🤯👾🙈🤞🦔🦚🐞💐🚧🧮🏁\n\n🍇🈚🥦🍔🍟🍱🎂🏺🏥🎡🥅🧬📱").scale(1.3).next_to(text3,DOWN)
        self.add(text1, text2, text3, emoji)

        end=time.time()
        print(end-start)
        # text=Text("回", font="SimHei").scale(0.1)
        # self.add(text)

Result:
FontTest

@TonyCrane TonyCrane requested a review from 3b1b July 28, 2021 11:18
@YishiMichael
Copy link
Contributor

This is a nice fix, and I'm glad to see no Tex or Text has broken so far after applying this PR.
Unfortunately, I'm afraid there're still some situations where the bug remains. A bug appeared as I added a thin Annulus into the Scene.
Code:

class AnnulusScene(Scene):
    def construct(self):
        a = Annulus(
            inner_radius=2,
            outer_radius=2.1,
            color=BLUE_C,
            fill_opacity=0.6,
            stroke_width=0
        )
        #a.insert_n_curves(len(a.get_points()))
        self.add(a)
        self.wait()

The result is showed below:
broken_annulus

By the way, if the PR isn't applied, the same code outputs:
broken_annulus_before

On the other hand, if the line a.insert_n_curves(len(a.get_points())) is added, no matter whether the PR is applied or not, the code can still work properly:
complete_annulus

I've also done another test: if outer_radius is set to 2.5, the code outputs correctly even without the hack line. So I guess some problems still exists in triangulation. When the control points of the inner circle happen to be outside the outer circle, the algorithm fails.

Footnote: This is my first time to comment on a pull request, so please excuse my linguistic mistakes.

@3b1b
Copy link
Owner

3b1b commented Jul 28, 2021

Thanks for making this change. Even if there are some edge cases, like a thin annulus, I'll go ahead and merge this since it's an improvement to the current way. One small note would be to try to ensure that PRs adhere to PEP formatting conventions.

@3b1b 3b1b merged commit 7da6179 into 3b1b:master Jul 28, 2021
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.

None yet

3 participants