[Engine] Add RoundSuperellipse to drawing OP #160883

dkwingsmt · 2024-12-26T22:21:45Z

This PR adds support for clipping round superellipse to the engine.

For what a rounded superellipse is, see this design doc. 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.

engine/src/flutter/display_list/dl_builder.cc

flar · 2025-02-05T19:27:16Z

engine/src/flutter/display_list/dl_builder.cc

+void DisplayListBuilder::DrawRoundSuperellipse(const DlRoundSuperellipse& rse,
+                                               const DlPaint& paint) {
+  SetAttributesFromPaint(paint, DisplayListOpFlags::kDrawRSuperellipseFlags);
+  drawRoundSuperellipse(rse);


Because the draw call might optimize to rect and circle, and because we may have synchronized a (style==Stroke) attribute, you can make sure that the right attributes are in the stream by using something like

saved_style = GetDrawStyle(); // Only needed while we temporarily ignore stroking for RoundSuperellipses // We change the attribute even though it is ignored so that optimizing to // drawRect and drawCircle don't stroke it (yet). setDrawStyle(kFill); drawRSE(...); setDrawStyle(saved_style);

Note that I used the lower case set() call because that does the testing for whether or not it needs to synch that attribute.

Since it is only 3 lines of code, this technique could be used inside drawRSE only when we optimize, I'm not sure the case happens often enough to optimize especially since we plan to implement stroking soon.

Another thought. If we have rse => roundRect conversion capability, then perhaps we can check for stroking and redirect to drawRoundRect in the short term?

Since I've added support for stroking RSEs, is this comment no longer relevant?

engine/src/flutter/display_list/dl_builder.cc

flar · 2025-02-05T19:34:41Z

engine/src/flutter/display_list/dl_op_flags.h

+  static constexpr DisplayListAttributeFlags kDrawRSuperellipseFlags{
+      kBasePaintFlags |  //
+      kIsFilledGeometry  //
+                         // TODO(dkwingsmt): Support strokes.


also kUsesMaskFilter and kUsesAntialias. Everything in kBaseStrokeFlags except for IsDrawn... (We do need mask filters on these for shadows, don't we? And AA is ignored by Impeller, but it matters for Skia so we need to make sure we list that flag so we synch that attribute.)

Since I've added support for stroking RSEs, I've changed the flags to be the same as RRects. Can you take a look if it's correct?

engine/src/flutter/display_list/utils/dl_matrix_clip_tracker.cc

engine/src/flutter/flow/layers/layer_state_stack.cc

flar

I left some comments on the DL side of things and skimmed over the Impeller side.

One thing that I think is required here are unit tests that cover these new calls, including:

adding clipRSE variants to the array of invokers in dl_test_snippets.cc - use the "Clip/DrawRRect" sections as inspiration.
adding clipRSE variants to the test parameters in dl_rendering_unittests.cc - look for the code that tests a couple of variants of clipRRect for inspiration. clipRRect is considered an attribute to test against so it will need code in the RenderWithClips method. drawRRect is considered a rendering call so it will need a new TEST(...) instance - again look for the TEST(..., DrawRoundRect) test for inspiration
a full suite of ClipRSELayer tests per any of the other foo_layer_unittests.cc in that same directory
adding test cases to layer_stack and/or matrixcliptracker unit test files
tests in impeller/entity/geometry and impeller/geometry to match other geometry classes

…#160883)

Manual roll requested by [email protected] flutter/flutter@043b719...1659206 2025-02-24 [email protected] Roll Skia from 3dfb3fee52e1 to 28017200173a (2 revisions) (flutter/flutter#163981) 2025-02-24 [email protected] Add integration test for Gradle-initiated android builds with flavors (flutter/flutter#163737) 2025-02-23 [email protected] Roll Skia from 72f949950adb to 3dfb3fee52e1 (1 revision) (flutter/flutter#163959) 2025-02-23 [email protected] Roll Skia from 4bee660601de to 72f949950adb (1 revision) (flutter/flutter#163948) 2025-02-22 [email protected] Roll Skia from cca9328df6ca to 4bee660601de (1 revision) (flutter/flutter#163927) 2025-02-22 [email protected] Roll Dart SDK from bad289580d9b to aea6fff33f06 (3 revisions) (flutter/flutter#163912) 2025-02-22 [email protected] Roll Skia from 1d884bab8593 to cca9328df6ca (12 revisions) (flutter/flutter#163910) 2025-02-22 [email protected] [fuchsia] include more tests in the fuchsia builders (flutter/flutter#163800) 2025-02-21 [email protected] [Engine] Add RoundSuperellipse to drawing OP (flutter/flutter#160883) 2025-02-21 [email protected] [remake] Restore old back handling for FlutterFragmentActivity (flutter/flutter#161545) 2025-02-21 [email protected] Revert "Marks Mac_benchmark basic_material_app_macos__compile to be flaky" (flutter/flutter#163878) 2025-02-21 [email protected] Revert "Marks Mac_benchmark flutter_view_macos__start_up to be flaky" (flutter/flutter#163880) 2025-02-21 [email protected] Roll Dart SDK from c5e582f15b6c to bad289580d9b (1 revision) (flutter/flutter#163885) 2025-02-21 [email protected] Revert "Marks Windows_mokey native_assets_android to be flaky" (flutter/flutter#163881) 2025-02-21 [email protected] Mark platform_views_hcpp_scroll_perf__timeline_summary out of bringup (flutter/flutter#163883) 2025-02-21 [email protected] Roll Skia from 6da10829d017 to 1d884bab8593 (42 revisions) (flutter/flutter#163789) 2025-02-21 [email protected] Enable `linux_android_emulator_tests` on presubmit. (flutter/flutter#163879) 2025-02-21 [email protected] Update gradle memory properties in example and test projects (flutter/flutter#163798) 2025-02-21 [email protected] Secure paste milestone 2 (flutter/flutter#159013) 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] 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

…#160883)

) Manual roll requested by [email protected] flutter/flutter@043b719...1659206 2025-02-24 [email protected] Roll Skia from 3dfb3fee52e1 to 28017200173a (2 revisions) (flutter/flutter#163981) 2025-02-24 [email protected] Add integration test for Gradle-initiated android builds with flavors (flutter/flutter#163737) 2025-02-23 [email protected] Roll Skia from 72f949950adb to 3dfb3fee52e1 (1 revision) (flutter/flutter#163959) 2025-02-23 [email protected] Roll Skia from 4bee660601de to 72f949950adb (1 revision) (flutter/flutter#163948) 2025-02-22 [email protected] Roll Skia from cca9328df6ca to 4bee660601de (1 revision) (flutter/flutter#163927) 2025-02-22 [email protected] Roll Dart SDK from bad289580d9b to aea6fff33f06 (3 revisions) (flutter/flutter#163912) 2025-02-22 [email protected] Roll Skia from 1d884bab8593 to cca9328df6ca (12 revisions) (flutter/flutter#163910) 2025-02-22 [email protected] [fuchsia] include more tests in the fuchsia builders (flutter/flutter#163800) 2025-02-21 [email protected] [Engine] Add RoundSuperellipse to drawing OP (flutter/flutter#160883) 2025-02-21 [email protected] [remake] Restore old back handling for FlutterFragmentActivity (flutter/flutter#161545) 2025-02-21 [email protected] Revert "Marks Mac_benchmark basic_material_app_macos__compile to be flaky" (flutter/flutter#163878) 2025-02-21 [email protected] Revert "Marks Mac_benchmark flutter_view_macos__start_up to be flaky" (flutter/flutter#163880) 2025-02-21 [email protected] Roll Dart SDK from c5e582f15b6c to bad289580d9b (1 revision) (flutter/flutter#163885) 2025-02-21 [email protected] Revert "Marks Windows_mokey native_assets_android to be flaky" (flutter/flutter#163881) 2025-02-21 [email protected] Mark platform_views_hcpp_scroll_perf__timeline_summary out of bringup (flutter/flutter#163883) 2025-02-21 [email protected] Roll Skia from 6da10829d017 to 1d884bab8593 (42 revisions) (flutter/flutter#163789) 2025-02-21 [email protected] Enable `linux_android_emulator_tests` on presubmit. (flutter/flutter#163879) 2025-02-21 [email protected] Update gradle memory properties in example and test projects (flutter/flutter#163798) 2025-02-21 [email protected] Secure paste milestone 2 (flutter/flutter#159013) 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] 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

dkwingsmt mentioned this pull request Dec 26, 2024

Add RoundSuperellipse to drawing OP flutter/engine#57205

Closed

8 tasks

github-actions bot added the engine flutter/engine related. See also e: labels. label Dec 26, 2024

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: cupertino flutter/packages/flutter/cupertino repository labels Jan 3, 2025

Rebase

2f8ab69

dkwingsmt force-pushed the rse-api branch from bae658d to 2f8ab69 Compare January 3, 2025 22:36

github-actions bot removed framework flutter/packages/flutter repository. See also f: labels. f: cupertino flutter/packages/flutter/cupertino repository labels Jan 3, 2025

dkwingsmt added 4 commits January 6, 2025 13:04

format

88d6e49

Merge remote-tracking branch 'origin/main' into rse-api

2b80446

Add a unit test and improve introduction to rses

4071bdf

Update TODO link

886f29e

dkwingsmt mentioned this pull request Jan 30, 2025

[Impeller] Make rounded superellipse a style of RoundRect #162349

Closed

9 tasks