Clip layers reduce rrects and paths to simpler shapes when possible #164693
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
There will be 1 more step to this after #164258 lands to remove a similar optimization used there as a workaround for a clip fidelity issue in the new hcpp code. Some follow-on work. There is no kClipOval Mutator. Adding one would be hard because ClipOval vs ClipRect cannot be distinguished from each other by their object type, so auto-typing the std::variant in the Mutator class will not function without further work. Also, we might want to consider exposing a ClipOvalLayer to Flutter in the SceneBuilder to reduce the amount of reverse engineering in the system. |
flar
force-pushed
the
clip-layer-shape-reduction
branch
from
1606f2c to
ec5836b
Compare
Contributor
Author
|
Hmmm, or should the logic go in the SceneBuilder::pushClipLayer methods? |
jonahwilliams
approved these changes
Mar 7, 2025
Contributor
jonahwilliams
left a comment
jonahwilliams
left a comment
There was a problem hiding this comment.
I don't know that there is a perfect place for this.
A lot of what flow does is slightly redundant with structures like the LSS and mutator, but maybe that is OK?
LGTM
flutter-dashboard
bot
removed
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Mar 7, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Mar 7, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Mar 7, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Mar 7, 2025
auto-submit bot
pushed a commit
to flutter/packages
that referenced
this pull request
Mar 7, 2025
flutter/flutter@321fbc0...6b93cf9 2025-03-07 [email protected] Roll Skia from 32c1931117b8 to cbc7e99d6c2f (1 revision) (flutter/flutter#164788) 2025-03-07 [email protected] Roll Packages from fc9d5ca to 4c5a7ed (4 revisions) (flutter/flutter#164785) 2025-03-07 [email protected] Roll Skia from 79f8af105a61 to 32c1931117b8 (1 revision) (flutter/flutter#164782) 2025-03-07 [email protected] Roll Fuchsia Linux SDK from fhm5z889sA5T1AQao... to ixl5bKWCqsRiYGvps... (flutter/flutter#164780) 2025-03-07 [email protected] Roll Skia from 181d81920670 to 79f8af105a61 (1 revision) (flutter/flutter#164770) 2025-03-07 [email protected] Roll Skia from cc74d34e7e68 to 181d81920670 (1 revision) (flutter/flutter#164766) 2025-03-07 [email protected] Clip layers reduce rrects and paths to simpler shapes when possible (flutter/flutter#164693) 2025-03-07 [email protected] [Impeller] test empty snapshot and allocation failure. (flutter/flutter#164668) 2025-03-07 [email protected] Roll Skia from 263308ea4386 to cc74d34e7e68 (2 revisions) (flutter/flutter#164746) 2025-03-06 [email protected] Adds aria-controls support (flutter/flutter#163894) 2025-03-06 [email protected] Migrate Mutators to DisplayList/Impeller geometry (flutter/flutter#164258) 2025-03-06 [email protected] Add lldb init file (flutter/flutter#164344) 2025-03-06 [email protected] Use separate artifacts for arm64 and x64 versions of gen_snapshot on Apple platforms (flutter/flutter#164419) 2025-03-06 [email protected] Roll Skia from ccd8cc23aa94 to 263308ea4386 (1 revision) (flutter/flutter#164728) 2025-03-06 [email protected] [hcpp] Add tests for transform mutator (flutter/flutter#164664) 2025-03-06 [email protected] Roll pub packages (flutter/flutter#164721) 2025-03-06 [email protected] Roll Packages from abba683 to fc9d5ca (3 revisions) (flutter/flutter#164714) 2025-03-06 [email protected] Roll pub packages (flutter/flutter#164713) 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
Contributor
Author
I think we have a lot of these decisions in the code about where we do optimizations in multiple places. Much of it depends on where you consider the public vs internal APIs. The higher you put the optimization, the more it is amortized. If DlBuilder does an optimization it gets reused if we reuse the DL. If SceneBuilder does an optimization when building a layer tree then it gets reused if we have retained layers. But, if you also construct or use interfaces at a lower level (like using imp::Canvas directly for some internal work vs. using it indirectly via DLs passed down from Flutter), then either having it at the lower level or having it in 2 places might be desirable - the upper location so that it is amortized, and the lower location in case the upper code was bypassed. |
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 20, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 20, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 21, 2025
androidseb
pushed a commit
to androidseb/packages
that referenced
this pull request
Jun 8, 2025
flutter/flutter@321fbc0...6b93cf9 2025-03-07 [email protected] Roll Skia from 32c1931117b8 to cbc7e99d6c2f (1 revision) (flutter/flutter#164788) 2025-03-07 [email protected] Roll Packages from fc9d5ca to 4c5a7ed (4 revisions) (flutter/flutter#164785) 2025-03-07 [email protected] Roll Skia from 79f8af105a61 to 32c1931117b8 (1 revision) (flutter/flutter#164782) 2025-03-07 [email protected] Roll Fuchsia Linux SDK from fhm5z889sA5T1AQao... to ixl5bKWCqsRiYGvps... (flutter/flutter#164780) 2025-03-07 [email protected] Roll Skia from 181d81920670 to 79f8af105a61 (1 revision) (flutter/flutter#164770) 2025-03-07 [email protected] Roll Skia from cc74d34e7e68 to 181d81920670 (1 revision) (flutter/flutter#164766) 2025-03-07 [email protected] Clip layers reduce rrects and paths to simpler shapes when possible (flutter/flutter#164693) 2025-03-07 [email protected] [Impeller] test empty snapshot and allocation failure. (flutter/flutter#164668) 2025-03-07 [email protected] Roll Skia from 263308ea4386 to cc74d34e7e68 (2 revisions) (flutter/flutter#164746) 2025-03-06 [email protected] Adds aria-controls support (flutter/flutter#163894) 2025-03-06 [email protected] Migrate Mutators to DisplayList/Impeller geometry (flutter/flutter#164258) 2025-03-06 [email protected] Add lldb init file (flutter/flutter#164344) 2025-03-06 [email protected] Use separate artifacts for arm64 and x64 versions of gen_snapshot on Apple platforms (flutter/flutter#164419) 2025-03-06 [email protected] Roll Skia from ccd8cc23aa94 to 263308ea4386 (1 revision) (flutter/flutter#164728) 2025-03-06 [email protected] [hcpp] Add tests for transform mutator (flutter/flutter#164664) 2025-03-06 [email protected] Roll pub packages (flutter/flutter#164721) 2025-03-06 [email protected] Roll Packages from abba683 to fc9d5ca (3 revisions) (flutter/flutter#164714) 2025-03-06 [email protected] Roll pub packages (flutter/flutter#164713) 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
FMorschel
pushed a commit
to FMorschel/packages
that referenced
this pull request
Jun 9, 2025
flutter/flutter@321fbc0...6b93cf9 2025-03-07 [email protected] Roll Skia from 32c1931117b8 to cbc7e99d6c2f (1 revision) (flutter/flutter#164788) 2025-03-07 [email protected] Roll Packages from fc9d5ca to 4c5a7ed (4 revisions) (flutter/flutter#164785) 2025-03-07 [email protected] Roll Skia from 79f8af105a61 to 32c1931117b8 (1 revision) (flutter/flutter#164782) 2025-03-07 [email protected] Roll Fuchsia Linux SDK from fhm5z889sA5T1AQao... to ixl5bKWCqsRiYGvps... (flutter/flutter#164780) 2025-03-07 [email protected] Roll Skia from 181d81920670 to 79f8af105a61 (1 revision) (flutter/flutter#164770) 2025-03-07 [email protected] Roll Skia from cc74d34e7e68 to 181d81920670 (1 revision) (flutter/flutter#164766) 2025-03-07 [email protected] Clip layers reduce rrects and paths to simpler shapes when possible (flutter/flutter#164693) 2025-03-07 [email protected] [Impeller] test empty snapshot and allocation failure. (flutter/flutter#164668) 2025-03-07 [email protected] Roll Skia from 263308ea4386 to cc74d34e7e68 (2 revisions) (flutter/flutter#164746) 2025-03-06 [email protected] Adds aria-controls support (flutter/flutter#163894) 2025-03-06 [email protected] Migrate Mutators to DisplayList/Impeller geometry (flutter/flutter#164258) 2025-03-06 [email protected] Add lldb init file (flutter/flutter#164344) 2025-03-06 [email protected] Use separate artifacts for arm64 and x64 versions of gen_snapshot on Apple platforms (flutter/flutter#164419) 2025-03-06 [email protected] Roll Skia from ccd8cc23aa94 to 263308ea4386 (1 revision) (flutter/flutter#164728) 2025-03-06 [email protected] [hcpp] Add tests for transform mutator (flutter/flutter#164664) 2025-03-06 [email protected] Roll pub packages (flutter/flutter#164721) 2025-03-06 [email protected] Roll Packages from abba683 to fc9d5ca (3 revisions) (flutter/flutter#164714) 2025-03-06 [email protected] Roll pub packages (flutter/flutter#164713) 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Flutter code can pass clips in the widget tree down as Path objects even if they were originally simpler shapes. We now catch those simplifications in the clip_*_layer code and perform reduced operations in their place.