-
Notifications
You must be signed in to change notification settings - Fork 6k
[Impeller] Add rounded superellipse #56726
Conversation
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.
Really excited to see this work coming along!
|
I changed canvas.drawRect to use this geometry and then ran an app that draws a few dozen rectangles to get this profile. It looks like the vast majority of overhead is from temporary allocation. That should disappear if you use the preallocated arenas. Also kAngleStep may be too small for small rectangles. I'm getting ~182 points for a 20x20 squircle. You may want to eyeball some numbers for this based off of the rectangle size. |
|
I suggest we simplify the name to just |
|
@mit-mit That's the definition I adopted as well. However, my shape is a superellipse of this definition combined with 4 circles at the corner. |
|
Sorry, I'm confused, don't all superellipses have rounded corners? And isn't a squircle just a superellipse with n=4? https://news.ycombinator.com/item?id=37871982 |
Well, to my understanding, squircles are a general term that describe any shape intermediate between a rectangle and an oval. There can be infinitely many kinds of squircles. Well I'm probably wrong... but anyway, I shouldn't call my shape squircle. (Jonah's previous PR said that superellipse with n=4 is called rectellipse. This is the only place I've heard this term though. :) )
You're right, but we're looking for a very specific family of shapes, the shapes used by Apple in its designs (which I call the Apple squircles). I've found (based on prior arts) that the Apple squircles look very much like superellipses, but with the rounded corners more dull. By replacing the tip of the rounded corners with appropriate circular arcs, the resulting shape happens to be almost, if not exactly, identical to the Apple squircles. I have a technical report in store, where I described my research process and the math behind. My apology that I've only shared the tech report with a few folks, mostly because the PR is still experimental. I plan to publish the tech report as soon as possible. But still, feel free to ask any questions! Edit: For a preview, here are two sets of comparisons, both are best approximations to a SwiftUI rounded rectangle with |
|
Thanks for all the details, glad to see so much care being put into this! |
|
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. |
|
@jonahwilliams I've addressed all your comments. Notably:
I'm not sure how to profile the app, so let me know if there are still performance bottlenecks. Thank you! |
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!
…159865) flutter/engine@3a8204a...bbe33f2 2024-12-05 [email protected] [DisplayList] Disable group opacity when a RuntimeEffect is in use (flutter/engine#56936) 2024-12-05 [email protected] Removes ReactorGLES::Ref (flutter/engine#56981) 2024-12-05 [email protected] [Impeller] create a 300 es variant of all GLES shaders to support UBO binding. (flutter/engine#56960) 2024-12-05 [email protected] [Impeller] Add rounded superellipse (flutter/engine#56726) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: 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
|
This is awesome, can't wait for it to become available as shape in Flutter. When it does I will update this and it to the comparison study https://github.com/rydmike/squircle_study |
Support rounded superellipse. Part of flutter#139321 and flutter#13914, also related to flutter#91523. ### Open questions * Alternative names: * Round**ed**Superellipse * Squircle * ContinuousBorderRectangle (or something like this...) * I chose rounded superellipse because this name, albeit its length, precisely describe this shape. "Squircle" is not strictly defined but generally refers any shape intermediate between a rectangle and a circle. * Alternative definition for `corner_radius`: * Currently the `corner_radius` corresponds to SwiftUI parameters. To make the shape definition more generalized, we can instead define the `corner_radius` to be the radius of the corner circles, and make the framework do a look up table. * The down side is, not only the work to re-calculate the table, but also that it doesn't completely eliminates the relationship with SwiftUI, since currently the degree of the superellipse (`n`) is also mapped from the SwiftUI `cornerRadius`, which is not necessary for the shape per se. * To some extent it boils down to the question of whether we'd like this shape to support anything beyond SwiftUI. ### Demo https://github.com/user-attachments/assets/806ac0e9-d62f-4b04-ab6a-83436a11f6f3 Low ratio: (900, 900, 445) <img width="520" alt="image" src="https://github.com/user-attachments/assets/54087467-85cd-4021-91cc-a948866ab5a8"> Mid ratio: (900, 650, 180) <img width="508" alt="image" src="https://github.com/user-attachments/assets/460a4927-0396-462b-948d-0846a781c92c"> High ratio: (900, 650, 17) <img width="490" alt="image" src="https://github.com/user-attachments/assets/8d7f625d-8a3b-4aba-b3f9-f292b874b606"> ## 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] and the [C++, Objective-C, Java style guides]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I signed the [CLA]. - [ ] 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/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
This PR adds support for clipping round superellipse to the engine. For what a rounded superellipse is, see [this design doc](https://docs.google.com/document/d/1CJXULKJGQt22FOFsrlm2TKVjKBtif1yU4U50cMfL6Kc/edit?tab=t.0). Video demos can be found at flutter/engine#56726 and #161409. Only impeller can actually render it. On Skia and Web, this shape falls back to `RRect`. Part of #139321 and #13914, also related to #91523. ## 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



Support rounded superellipse.
Part of flutter/flutter#139321 and flutter/flutter#13914, also related to flutter/flutter#91523.
Open questions
corner_radius:corner_radiuscorresponds to SwiftUI parameters. To make the shape definition more generalized, we can instead define thecorner_radiusto be the radius of the corner circles, and make the framework do a look up table.n) is also mapped from the SwiftUIcornerRadius, which is not necessary for the shape per se.Demo
Screen.Recording.2024-11-21.at.5.26.56.PM.mp4
Low ratio: (900, 900, 445)
Mid ratio: (900, 650, 180)
High ratio: (900, 650, 17)
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.