[Engine] Support asymmetrical rounded superellipses #161409

dkwingsmt · 2025-01-10T02:36:22Z

This PR allows rounded superellipses to have asymmetrical and uneven radii, effectively supporting BorderRadius instead of mere double as corner radius. Fixes #161207.

Screen.Recording.2025-01-10.at.5.16.37.PM.mp4

These features exist in SwiftUI: The RoundedRectangle class provides an initializer with a cornerSize parameter, allowing different radii for the horizontal and vertical directions, while the UnevenRoundedRectangle class allows each corner to have a unique radius. However, SwiftUI does not allow the corners to be asymmetrical and uneven at the same time, which is supported by this PR.

Additionally, this change allows rounded superellipses to use the same API as rounded rectangles. This allows RSEs to be added as a style of RRect (just like in SwiftUI), which will use much fewer changes than adding it as a new shape (>1500 LOC according to my prototype).

This PR also improves performance by removing an intermediate cache for flipping. Now the point list is only copied once for the triangle strip rearrangement.

RoundingRadii is moved to a separate file, and the code to scale it based on bounds now its method.

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.

jonahwilliams · 2025-01-13T20:27:51Z

engine/src/flutter/impeller/entity/geometry/round_superellipse_geometry.cc

 namespace {
+
+// An interface for classes that arranges a point list that forms a convex
+// contour into a triangle strip.


nit: I think we already have a class named ConvexTessellator. Though this is defined in a .cc it would probably be better to rename to somethng like SuperEllipseTessellator or something like that.

Renamed to ConvexRearranger

jonahwilliams · 2025-01-13T20:29:17Z

engine/src/flutter/impeller/entity/geometry/round_superellipse_geometry.cc

+
+  virtual Point GetPoint(size_t i) const = 0;
+
+  void RearrangeIntoTriangleStrip(Point* output) {


I think we might have discussed this on the last PR, but why are you re-arranging instead of using the index buffer to encode the triange strip?

I don't recall we discussing it in the last PR, but I can answer that:

For the case where all corners are the same and one quadrant is mirrored to the other 3, the points need to be transformed during the last step anyway, so they are rearranged without additional cost - basically we're just using a specific order to transform them.

For the case of uneven corners, I think you raised a good point. Since there isn't a last-step transformation any more, the rearrangement is no longer free. However saving this rearrangement means that all points are not copied at all and must be directly generated onto the final buffer, meaning the size of the final buffer must be known beforehand. Maybe we can do a preroll to calculate the number of all points... but in general the code would be much more complicated. I can try it if you think it's worth it.

no that makes sense to me, thanks!

jonahwilliams

LGTM

auto-submit · 2025-01-14T01:09:16Z

auto label is removed for flutter/flutter/161409, due to - The status or check suite Windows windows_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

auto-submit · 2025-01-14T02:24:00Z

auto label is removed for flutter/flutter/161409, due to - The status or check suite Mac dart_plugin_registry_test has failed. Please fix the issues identified (or deflake) before re-applying this label.

auto-submit · 2025-01-14T02:50:25Z

auto label is removed for flutter/flutter/161409, due to - The status or check suite Linux linux_fuchsia_tests has failed. Please fix the issues identified (or deflake) before re-applying this label.

auto-submit · 2025-01-14T20:03:22Z

auto label is removed for flutter/flutter/161409, due to - The status or check suite Linux linux_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

This PR allows rounded superellipses to have asymmetrical and uneven radii, effectively supporting `BorderRadius` instead of mere `double` as corner radius. Fixes flutter#161207. https://github.com/user-attachments/assets/6293028c-d14b-4ffb-b93e-d0602f5ca0ee These features exist in SwiftUI: The `RoundedRectangle` class provides an initializer with a `cornerSize` parameter, allowing different radii for the horizontal and vertical directions, while the `UnevenRoundedRectangle` class allows each corner to have a unique radius. However, SwiftUI does not allow the corners to be asymmetrical _and_ uneven at the same time, which is supported by this PR. Additionally, this change allows rounded superellipses to use the same API as rounded rectangles. This allows RSEs to be added as a style of `RRect` (just like in SwiftUI), which will use much fewer changes than adding it as a new shape (>1500 LOC according to my prototype). This PR also improves performance by removing an intermediate cache for flipping. Now the point list is only copied once for the triangle strip rearrangement. `RoundingRadii` is moved to a separate file, and the code to scale it based on bounds now its method. ## 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].  [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

…flutter#161409)

flutter/flutter@40c2b86...5517cc9 2025-01-15 37028599+EArminjon@users.noreply.github.com feat: Change default value of keyboardDismissBehavior (flutter/flutter#158580) 2025-01-15 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#161680) 2025-01-15 jacob.simionato@gmail.com Revert "Autocomplete Options Width" (flutter/flutter#161666) 2025-01-15 stuartmorgan@google.com Update two_dimensional_scrollables triage routing (flutter/flutter#161667) 2025-01-15 flar@google.com [DisplayList] Migrate from SkRSXform to Impeller RSTransform (flutter/flutter#161652) 2025-01-15 engine-flutter-autoroll@skia.org Roll Packages from d1fd623 to f73cb00 (2 revisions) (flutter/flutter#161672) 2025-01-15 bruno.leroux@gmail.com Fix DropdownMenu isCollapsed decoration does not Reduce height (flutter/flutter#161427) 2025-01-15 jason-simmons@users.noreply.github.com Manual roll of Skia to e7b8d078851f (flutter/flutter#161609) 2025-01-15 tessertaha@gmail.com Fix `TabBar` glitchy elastic `Tab` animation (flutter/flutter#161514) 2025-01-15 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#161643) 2025-01-15 matanlurey@users.noreply.github.com Exclude the top-level `engine` directory from `generate_gradle_lockfiles`. (flutter/flutter#161635) 2025-01-15 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#161632) 2025-01-15 matanlurey@users.noreply.github.com Refactor `android_engine_test`, make it easier to debug/deflake locally. (flutter/flutter#161534) 2025-01-15 jonahwilliams@google.com [Impeller] null check device buffer in image encoding. (flutter/flutter#161194) 2025-01-15 nabilamevia2003@gmail.com Feature/twitter keyboard (flutter/flutter#161025) 2025-01-15 jessiewong401@gmail.com Fixed XiaoMi statusBar Bug (flutter/flutter#161271) 2025-01-15 goderbauer@google.com Clean up engine's analysis_options.yaml (flutter/flutter#161554) 2025-01-14 34871572+gmackall@users.noreply.github.com Remove `gradle_deprecated_settings` test app, and remove reference from lockfile exclusion yaml (flutter/flutter#161622) 2025-01-14 devoncarew@google.com [deps] remove no-longer-used repo deps (flutter/flutter#161605) 2025-01-14 flar@google.com [DisplayList] remove obsolete use of Skia goemetry objects in DL utils (flutter/flutter#161553) 2025-01-14 dkwingsmt@users.noreply.github.com [Engine] Support asymmetrical rounded superellipses (flutter/flutter#161409) 2025-01-14 737941+loic-sharma@users.noreply.github.com [SwiftPM] Make 'flutter build ios-framework' generate an empty Package.swift (flutter/flutter#161464) 2025-01-14 1961493+harryterkelsen@users.noreply.github.com [canvaskit] Fix GIF decode failure (flutter/flutter#161536) 2025-01-14 jonahwilliams@google.com [Impeller] fixes for AHB swapchains. (flutter/flutter#161562) 2025-01-14 goderbauer@google.com Last Engine<>Framework lint sync (flutter/flutter#161560) 2025-01-14 goderbauer@google.com Check that localization files of stocks app are up-to-date (flutter/flutter#161608) 2025-01-14 43054281+camsim99@users.noreply.github.com [Android] Actually remove dev dependencies from release builds (flutter/flutter#161343) 2025-01-14 bkonyi@google.com Update package revisions to latest (flutter/flutter#161525) 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 stuartmorgan@google.com 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#161409)

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].  [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

…flutter#161409)

flutter/flutter@40c2b86...5517cc9 2025-01-15 37028599+EArminjon@users.noreply.github.com feat: Change default value of keyboardDismissBehavior (flutter/flutter#158580) 2025-01-15 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#161680) 2025-01-15 jacob.simionato@gmail.com Revert "Autocomplete Options Width" (flutter/flutter#161666) 2025-01-15 stuartmorgan@google.com Update two_dimensional_scrollables triage routing (flutter/flutter#161667) 2025-01-15 flar@google.com [DisplayList] Migrate from SkRSXform to Impeller RSTransform (flutter/flutter#161652) 2025-01-15 engine-flutter-autoroll@skia.org Roll Packages from d1fd623 to f73cb00 (2 revisions) (flutter/flutter#161672) 2025-01-15 bruno.leroux@gmail.com Fix DropdownMenu isCollapsed decoration does not Reduce height (flutter/flutter#161427) 2025-01-15 jason-simmons@users.noreply.github.com Manual roll of Skia to e7b8d078851f (flutter/flutter#161609) 2025-01-15 tessertaha@gmail.com Fix `TabBar` glitchy elastic `Tab` animation (flutter/flutter#161514) 2025-01-15 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#161643) 2025-01-15 matanlurey@users.noreply.github.com Exclude the top-level `engine` directory from `generate_gradle_lockfiles`. (flutter/flutter#161635) 2025-01-15 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#161632) 2025-01-15 matanlurey@users.noreply.github.com Refactor `android_engine_test`, make it easier to debug/deflake locally. (flutter/flutter#161534) 2025-01-15 jonahwilliams@google.com [Impeller] null check device buffer in image encoding. (flutter/flutter#161194) 2025-01-15 nabilamevia2003@gmail.com Feature/twitter keyboard (flutter/flutter#161025) 2025-01-15 jessiewong401@gmail.com Fixed XiaoMi statusBar Bug (flutter/flutter#161271) 2025-01-15 goderbauer@google.com Clean up engine's analysis_options.yaml (flutter/flutter#161554) 2025-01-14 34871572+gmackall@users.noreply.github.com Remove `gradle_deprecated_settings` test app, and remove reference from lockfile exclusion yaml (flutter/flutter#161622) 2025-01-14 devoncarew@google.com [deps] remove no-longer-used repo deps (flutter/flutter#161605) 2025-01-14 flar@google.com [DisplayList] remove obsolete use of Skia goemetry objects in DL utils (flutter/flutter#161553) 2025-01-14 dkwingsmt@users.noreply.github.com [Engine] Support asymmetrical rounded superellipses (flutter/flutter#161409) 2025-01-14 737941+loic-sharma@users.noreply.github.com [SwiftPM] Make 'flutter build ios-framework' generate an empty Package.swift (flutter/flutter#161464) 2025-01-14 1961493+harryterkelsen@users.noreply.github.com [canvaskit] Fix GIF decode failure (flutter/flutter#161536) 2025-01-14 jonahwilliams@google.com [Impeller] fixes for AHB swapchains. (flutter/flutter#161562) 2025-01-14 goderbauer@google.com Last Engine<>Framework lint sync (flutter/flutter#161560) 2025-01-14 goderbauer@google.com Check that localization files of stocks app are up-to-date (flutter/flutter#161608) 2025-01-14 43054281+camsim99@users.noreply.github.com [Android] Actually remove dev dependencies from release builds (flutter/flutter#161343) 2025-01-14 bkonyi@google.com Update package revisions to latest (flutter/flutter#161525) 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 stuartmorgan@google.com 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

github-actions bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Jan 10, 2025

dkwingsmt marked this pull request as ready for review January 11, 2025 01:45

dkwingsmt requested review from flar and jonahwilliams January 11, 2025 01:45

jonahwilliams reviewed Jan 13, 2025

View reviewed changes

jonahwilliams approved these changes Jan 13, 2025

View reviewed changes