-
Notifications
You must be signed in to change notification settings - Fork 29.7k
RoundSuperellipse algorithm v3: Ultrawideband heuristic formula #164755
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
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
| typedef std::function< | ||
| void(const Point&, const Point&, const Point&, const Point&)> | ||
| CubicAdder; | ||
| typedef std::function<void(const Point&)> PointAdder; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| typedef std::function<void(const Point&)> PointAdder; | |
| using PointAdder = std::function<void(const Point&)>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I probably should have caught the previous one, but you can use using instead of typedef
jonahwilliams
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Noob question: How do we implement this superelljpse shape? Is it just part and parcel when we use |
Thanks for asking. Currently it is available as ClipRSuperellipse (on main channel only for now, ofc). See #161111 for an example. I'm working on providing it as a path and a border decoration shape. |
WOW thanks for the info. If its that easy, I plan to use this extensivly in my app over the next few days. Maybe I can ditch figma squircle |
|
Thanks @dkwingsmt for this awesome work. Looking forward to seeing this as a paint and For the new RoundSuperEllipse
None of the current solutions can handle drawing a correct continuous Stadium (pill/capsule) shape. The dedicated shape Also some other note worthy issues on other Flutter shapes mentioned in the study:
|
|
@rydmike Never knew about this study you have created. This is the document I never knew I needed but now have been studying voraciously. thanks again for putting this together! |
|
IMO In theory IMO it could/should be an attribute in RoundedRectangleBorder, just like in Figma you can go from 0% to 100% and iOS is like 60%. I think Just like CircleBorder with eccentricity = 1 => OvalBorder, IMO RoundedRectangleBorder with cornerSmoothing = 60% => SuperEllipseBorder (or CupertinoBorder). |
|
If @dkwingsmt agrees on my idea/proposal, and it ends up being similar to Figma (corner smoothing in rounded rectangle border), I can implement it with lerp and everything super quickly. The only thing I need is the superellipse path it is going to be drawn. |
I would agree, although we don't have to. We don't need the name anyway since there will be a
This I can't. My algorithm is completely based on iOS's |
Yeah, but from my understanding it exists to make the iOS border, so with super ellipse it wouldn't have a reason to exist.
Ok, I didn't know that. |
I completely agree with you (AFAIK). But there can always be apps that still use this API, and removing it will disturb them, and a library should not pay this cost without good benefits. And just because it no longer serves the original purpose we designed them for is not a sufficient benefit. |
flutter/flutter@6b93cf9...b16430b 2025-03-10 [email protected] [macOS] Enable Impeller by default on macOS. (flutter/flutter#164572) 2025-03-10 [email protected] Roll Packages from 4c5a7ed to 464cea5 (5 revisions) (flutter/flutter#164904) 2025-03-10 [email protected] Roll Skia from f17d37ee0ac6 to 4ac86f17f2d4 (1 revision) (flutter/flutter#164893) 2025-03-10 [email protected] Roll Skia from 0f53870c7449 to f17d37ee0ac6 (1 revision) (flutter/flutter#164887) 2025-03-09 [email protected] Roll Fuchsia Linux SDK from 6tAcm4hdtXPE55GJP... to U-zlyIZrZRbr9I6gv... (flutter/flutter#164868) 2025-03-09 [email protected] Roll Skia from 345dc2d05dcd to 0f53870c7449 (1 revision) (flutter/flutter#164865) 2025-03-08 [email protected] Roll Skia from 916caa2f0102 to 345dc2d05dcd (1 revision) (flutter/flutter#164843) 2025-03-08 [email protected] Roll Fuchsia Linux SDK from ixl5bKWCqsRiYGvps... to 6tAcm4hdtXPE55GJP... (flutter/flutter#164838) 2025-03-08 [email protected] Roll Skia from b29851b2ada6 to 916caa2f0102 (1 revision) (flutter/flutter#164835) 2025-03-08 [email protected] [Impeller] add capability check for extended range formats. (flutter/flutter#164817) 2025-03-08 [email protected] Added calendar delegate to support custom calendar systems (flutter/flutter#161874) 2025-03-08 [email protected] RoundSuperellipse algorithm v3: Ultrawideband heuristic formula (flutter/flutter#164755) 2025-03-08 [email protected] Merge CHANGELOG for 3.29.1 stable release (flutter/flutter#164743) 2025-03-08 [email protected] Add and link to `Infra-Triage.md`. (flutter/flutter#164673) 2025-03-07 [email protected] Roll Skia from cbc7e99d6c2f to b29851b2ada6 (10 revisions) (flutter/flutter#164812) 2025-03-07 [email protected] [Impeller] dont redundantly set stencil reference on vulkan backend. (flutter/flutter#164763) 2025-03-07 [email protected] content-aware-hash experiment update (flutter/flutter#164803) 2025-03-07 [email protected] [Widget Inspector] Handle null exceptions calling `renderObject` (flutter/flutter#163642) 2025-03-07 [email protected] Use Python 3.12 to run the yapf formatter if no lower version is available (flutter/flutter#164807) 2025-03-07 [email protected] Roll gn to 7a8aa3a08a13521336853a28c46537ec04338a2d (flutter/flutter#164806) 2025-03-07 [email protected] [Impeller] Store the TextureGLES cached framebuffer object as a reactor handle (flutter/flutter#164761) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
flutter/flutter@6b93cf9...b16430b 2025-03-10 [email protected] [macOS] Enable Impeller by default on macOS. (flutter/flutter#164572) 2025-03-10 [email protected] Roll Packages from 4c5a7ed to 464cea5 (5 revisions) (flutter/flutter#164904) 2025-03-10 [email protected] Roll Skia from f17d37ee0ac6 to 4ac86f17f2d4 (1 revision) (flutter/flutter#164893) 2025-03-10 [email protected] Roll Skia from 0f53870c7449 to f17d37ee0ac6 (1 revision) (flutter/flutter#164887) 2025-03-09 [email protected] Roll Fuchsia Linux SDK from 6tAcm4hdtXPE55GJP... to U-zlyIZrZRbr9I6gv... (flutter/flutter#164868) 2025-03-09 [email protected] Roll Skia from 345dc2d05dcd to 0f53870c7449 (1 revision) (flutter/flutter#164865) 2025-03-08 [email protected] Roll Skia from 916caa2f0102 to 345dc2d05dcd (1 revision) (flutter/flutter#164843) 2025-03-08 [email protected] Roll Fuchsia Linux SDK from ixl5bKWCqsRiYGvps... to 6tAcm4hdtXPE55GJP... (flutter/flutter#164838) 2025-03-08 [email protected] Roll Skia from b29851b2ada6 to 916caa2f0102 (1 revision) (flutter/flutter#164835) 2025-03-08 [email protected] [Impeller] add capability check for extended range formats. (flutter/flutter#164817) 2025-03-08 [email protected] Added calendar delegate to support custom calendar systems (flutter/flutter#161874) 2025-03-08 [email protected] RoundSuperellipse algorithm v3: Ultrawideband heuristic formula (flutter/flutter#164755) 2025-03-08 [email protected] Merge CHANGELOG for 3.29.1 stable release (flutter/flutter#164743) 2025-03-08 [email protected] Add and link to `Infra-Triage.md`. (flutter/flutter#164673) 2025-03-07 [email protected] Roll Skia from cbc7e99d6c2f to b29851b2ada6 (10 revisions) (flutter/flutter#164812) 2025-03-07 [email protected] [Impeller] dont redundantly set stencil reference on vulkan backend. (flutter/flutter#164763) 2025-03-07 [email protected] content-aware-hash experiment update (flutter/flutter#164803) 2025-03-07 [email protected] [Widget Inspector] Handle null exceptions calling `renderObject` (flutter/flutter#163642) 2025-03-07 [email protected] Use Python 3.12 to run the yapf formatter if no lower version is available (flutter/flutter#164807) 2025-03-07 [email protected] Roll gn to 7a8aa3a08a13521336853a28c46537ec04338a2d (flutter/flutter#164806) 2025-03-07 [email protected] [Impeller] Store the TextureGLES cached framebuffer object as a reactor handle (flutter/flutter#164761) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
flutter/flutter@6b93cf9...b16430b 2025-03-10 [email protected] [macOS] Enable Impeller by default on macOS. (flutter/flutter#164572) 2025-03-10 [email protected] Roll Packages from 4c5a7ed to 464cea5 (5 revisions) (flutter/flutter#164904) 2025-03-10 [email protected] Roll Skia from f17d37ee0ac6 to 4ac86f17f2d4 (1 revision) (flutter/flutter#164893) 2025-03-10 [email protected] Roll Skia from 0f53870c7449 to f17d37ee0ac6 (1 revision) (flutter/flutter#164887) 2025-03-09 [email protected] Roll Fuchsia Linux SDK from 6tAcm4hdtXPE55GJP... to U-zlyIZrZRbr9I6gv... (flutter/flutter#164868) 2025-03-09 [email protected] Roll Skia from 345dc2d05dcd to 0f53870c7449 (1 revision) (flutter/flutter#164865) 2025-03-08 [email protected] Roll Skia from 916caa2f0102 to 345dc2d05dcd (1 revision) (flutter/flutter#164843) 2025-03-08 [email protected] Roll Fuchsia Linux SDK from ixl5bKWCqsRiYGvps... to 6tAcm4hdtXPE55GJP... (flutter/flutter#164838) 2025-03-08 [email protected] Roll Skia from b29851b2ada6 to 916caa2f0102 (1 revision) (flutter/flutter#164835) 2025-03-08 [email protected] [Impeller] add capability check for extended range formats. (flutter/flutter#164817) 2025-03-08 [email protected] Added calendar delegate to support custom calendar systems (flutter/flutter#161874) 2025-03-08 [email protected] RoundSuperellipse algorithm v3: Ultrawideband heuristic formula (flutter/flutter#164755) 2025-03-08 [email protected] Merge CHANGELOG for 3.29.1 stable release (flutter/flutter#164743) 2025-03-08 [email protected] Add and link to `Infra-Triage.md`. (flutter/flutter#164673) 2025-03-07 [email protected] Roll Skia from cbc7e99d6c2f to b29851b2ada6 (10 revisions) (flutter/flutter#164812) 2025-03-07 [email protected] [Impeller] dont redundantly set stencil reference on vulkan backend. (flutter/flutter#164763) 2025-03-07 [email protected] content-aware-hash experiment update (flutter/flutter#164803) 2025-03-07 [email protected] [Widget Inspector] Handle null exceptions calling `renderObject` (flutter/flutter#163642) 2025-03-07 [email protected] Use Python 3.12 to run the yapf formatter if no lower version is available (flutter/flutter#164807) 2025-03-07 [email protected] Roll gn to 7a8aa3a08a13521336853a28c46537ec04338a2d (flutter/flutter#164806) 2025-03-07 [email protected] [Impeller] Store the TextureGLES cached framebuffer object as a reactor handle (flutter/flutter#164761) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…ter#164755) This PR revises the algorithm for RoundSuperellipses, replacing the current "max ratio" approximation with an algorithm that works for ratios from 2.0 to infinity. The previous "max ratio" approximation, which replaces the middle of edges with straight lines when the ratio is above 2.3, turns out to produce results too close to classic RRects. After reexamining the shapes and more calculation, I discovered that the max-ratio approximation is flawed. Even squircles with with really high ratios (~100) have a significant part of the edges that must not be approximated by straight lines. The new version is much closer to native. ### Comparison Native: (Notice the long wedgy gap at the end of curves) <img src="https://github.com/user-attachments/assets/61b60191-7d45-4c49-9e09-b0422243cd8c" width="400"/> Before PR: (Notice the short wedgy gap at the end of curves) <img src="https://github.com/user-attachments/assets/15ea374b-4b16-4187-aaa4-94f432fbb61e" width="400"/> After PR: <img src="https://github.com/user-attachments/assets/973ef4d1-7c26-44a9-b45e-10d109d5618b" width="400"/> Another example (after PR). Even though the rectangular RSE has ratios of around 4, there are still curvature near the middle section of edges, which can be identified with the help of antialias pixels. <img width="838" alt="image" src="https://github.com/user-attachments/assets/5078d098-c582-48a8-81e5-615909def675" /> ### Details I found that `n` has really good linearity towards larger ratios. <img width="844" alt="image" src="https://github.com/user-attachments/assets/73e99e45-a0f0-450b-8e2b-f6fd97082958" /> I also found a good candidate for the precomputed unknown (called `k_xJ`), which has a smooth curve at the beginning and almost straight line towards larger `n`, removing the need to cap the scope of application of the formula. <img width="1203" alt="image" src="https://github.com/user-attachments/assets/67664898-2dbd-4f00-a9ba-d76030cf3742" /> The algorithm for paths are also updated in a similar way and approximated the Bezier factors with heuristic formulae for bigger `n`s. I've also verified that the path deviates from the geometry by no more than 0.01% over the range of n [15, 100] Theoretically removing "stretch" should simplify the algorithms. Unfortunately I had to spend more lines to process cases of zero radii, which were conveniently handled by stretches. ## Pre-launch Checklist - [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [ ] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [ ] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
This PR revises the algorithm for RoundSuperellipses, replacing the current "max ratio" approximation with an algorithm that works for ratios from 2.0 to infinity.
The previous "max ratio" approximation, which replaces the middle of edges with straight lines when the ratio is above 2.3, turns out to produce results too close to classic RRects. After reexamining the shapes and more calculation, I discovered that the max-ratio approximation is flawed. Even squircles with with really high ratios (~100) have a significant part of the edges that must not be approximated by straight lines.
The new version is much closer to native.
Comparison
Native: (Notice the long wedgy gap at the end of curves)

Before PR: (Notice the short wedgy gap at the end of curves)

After PR:

Another example (after PR). Even though the rectangular RSE has ratios of around 4, there are still curvature near the middle section of edges, which can be identified with the help of antialias pixels.

Details
I found that
nhas really good linearity towards larger ratios.I also found a good candidate for the precomputed unknown (called
k_xJ), which has a smooth curve at the beginning and almost straight line towards largern, removing the need to cap the scope of application of the formula.The algorithm for paths are also updated in a similar way and approximated the Bezier factors with heuristic formulae for bigger
ns. I've also verified that the path deviates from the geometry by no more than 0.01% over the range of n [15, 100]Theoretically removing "stretch" should simplify the algorithms. Unfortunately I had to spend more lines to process cases of zero radii, which were conveniently handled by stretches.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.