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

gpui: Add PathBuilder based on lyon to build Path #22808

Merged
merged 13 commits into from
Jan 29, 2025

Conversation

huacnlee
Copy link
Contributor

@huacnlee huacnlee commented Jan 8, 2025

Release Notes:

  • N/A

Continue #20499

We to draw more complex Path. Before this change, we only have line_to, but it is not enough.

Add a new PathBuilder to use lyon to build more complex path.

And then with PR #22812 to enable anti-aliasing, all thing will be perfect.

Show case

cargo run -p gpui --example painting

Before:

image

After:

image

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Jan 8, 2025
@huacnlee huacnlee changed the title gpui: Add move_to method to Path. gpui: Add move_to method to Path Jan 8, 2025
@huacnlee

This comment was marked as outdated.

@huacnlee

This comment was marked as outdated.

@JakkuSakura
Copy link

Thanks for pointing out. The algorithm was written so for lacking such stroking algo and for performance consideration

@Angelk90
Copy link
Contributor

Angelk90 commented Jan 8, 2025

@huacnlee and @JakkuSakura : If we wanted to animate a graph showing the performance of a program, how much of an impact would it have on the RAM usage? Would the animation require a significant amount of resources, and could it impact the overall performance of the system?

Registrazione.schermo.2025-01-08.alle.09.33.56.mov

@huacnlee

This comment was marked as outdated.

@huacnlee huacnlee marked this pull request as draft January 15, 2025 06:24
@huacnlee huacnlee changed the title gpui: Add move_to method to Path gpui: Add PathBuilder based on lyon to build Path Jan 15, 2025
@huacnlee
Copy link
Contributor Author

Today I make a update to add PathBuilder

@huacnlee huacnlee changed the title gpui: Add PathBuilder based on lyon to build Path gpui: Add PathBuilder based on lyon to build Path Jan 15, 2025
@huacnlee huacnlee marked this pull request as ready for review January 15, 2025 08:20
@huacnlee
Copy link
Contributor Author

This is with MSAA enable by #22812

image image image

@huacnlee
Copy link
Contributor Author

huacnlee commented Jan 15, 2025

We have used this changes and with MSAA in our project. Now, our Chart render is perfect now.

And we also deletes many of algorithms that before we handle details before the line, and only requires simple line_to, move_to, etc. of PathBuilder to complete it.

Before

image image image

After

image
image
image

Ref JakkuSakura/plotters-gpui#5

@SomeoneToIgnore SomeoneToIgnore self-assigned this Jan 28, 2025
Copy link
Contributor

@SomeoneToIgnore SomeoneToIgnore left a comment

Choose a reason for hiding this comment

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

Same comment as in #22812 (review)

@SomeoneToIgnore
Copy link
Contributor

@huacnlee can we merge/rebase on fresh main please?

@huacnlee
Copy link
Contributor Author

@SomeoneToIgnore Done.

@SomeoneToIgnore
Copy link
Contributor

Thank you.

If you could also do the same with #22812 and #23058 it would be great — then I'll be merging this all after today's release (most probably on Thursday, so no rush).

@SomeoneToIgnore SomeoneToIgnore merged commit 31fa414 into zed-industries:main Jan 29, 2025
12 checks passed
SomeoneToIgnore pushed a commit that referenced this pull request Jan 29, 2025
Closes #20762

Release Notes:

- N/A

---

Enable MSAA for Anti-Aliasing to Path (`cx.paint_path`) for drawing a
better vector graphics.

```bash
cargo run -p gpui --example gradient --features macos-blade
cargo run -p gpui --example gradient

cargo run -p gpui --example painting --features macos-blade
cargo run -p gpui --example painting
```

**Before**

<img width="1089" alt="image"
src="https://github.com/user-attachments/assets/0ae7240f-4ba9-4ef5-896c-e436c1282770"
/>

**After**

<img width="944" alt="image"
src="https://github.com/user-attachments/assets/71a07ae8-be54-452c-aacc-b8cec1f810c0"
/>

## TODO

- [x] Support Metal and Blade.
- [x] Detect system support to set up sample count.
- [x] Fix extra lines between Path vertices wait #22808 to merge.

Ref kvark/blade#213

Ask @kvark to review.

I am not sure if there is anything I missed. I modified it according to
the
[particle](https://github.com/kvark/blade/tree/main/examples/particle)
example of Blade project. But the difference is that after the first
MSAA render, I did not do it a second time, I tested it and found it was
not necessary.
SomeoneToIgnore added a commit that referenced this pull request Feb 5, 2025
SomeoneToIgnore pushed a commit that referenced this pull request Feb 6, 2025
…lection top right corner radius issue (#24342)

Release Notes:

- N/A

----

To fix #24289 mention issue and revert PathBuilder and MSAA.

I'm sorry about of this, in #22808 I was forgotten this bit of detail.


![image](https://github.com/user-attachments/assets/112afda2-088c-41d0-83bd-808f6cd2f9d5)

So, add `move_to` here, we can fix the selection top right corner radius
issue.

## After change

<img width="1383" alt="image"
src="https://github.com/user-attachments/assets/28ea103c-d652-41d6-bbe0-7fd042d81e77"
/>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants