[Impeller] Reland: Implement draw order optimization. #54215

bdero · 2024-07-30T12:11:23Z

Original PR: #54136
Revert PR: #54067

Includes fixes for issue seen in these goldens.

The problem was that the scissor was ending up wrong after clip replay for opaque draws that are supposed to occur outside the parent clip scope(s).

To fix it, I made clip replay draws as well as the subpass texture draw apply lazily.
For the clip replay, there's no need to apply a given clip until we come across an entity that has a depth less than or equal to the clip.
And for the subpass texture (which is often translucent), we can defer drawing it until we come across another translucent draw. Deferring the subpass texture is important because if we draw it immediately, then all of the replay clips need to be drawn immediately too.

Description

For each clip scope, draw opaque items in reverse order and translucent/backdrop-independent items in their original order afterwards. Clips are treated as translucent by the parent scope.

Respects clips, subpass collapse, and the clear color optimization.

Local new_gallery before/after (iPhone 12 mini):

cd ~/projects/flutter/flutter/dev/integration_tests/new_gallery
flutter drive --profile --local-engine-src-path ~/projects/flutter/engine/src --local-engine=ios_profile --local-engine-host=host_profile_arm64 --trace-startup -t test_driver/transitions_perf.dart -d 00008101-000A59A93C10001E

Original PR: flutter#54136 Revert PR: flutter#54067

This reverts commit 6919636.

flutter-dashboard · 2024-07-30T14:24:17Z

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.

Changes reported for pull request #54215 at sha f6bcf1d

jonahwilliams · 2024-07-30T15:15:58Z

impeller/entity/entity.cc

@@ -43,6 +43,8 @@ Entity::Entity(Entity&&) = default;

 Entity::Entity(const Entity&) = default;

+Entity& Entity::operator=(Entity&&) = default;


What are we using this for?

also what is this 🙃 ?

This is the move assignment operator. I just needed it for the deferred_entity = std::move(result.entity) assignment.
(In general, we should always add the move assignment when adding the move constructor to prevent it from getting implicitly deleted.)

jonahwilliams

LGTM

interesting fix! still interested in how we can port this over to experimental canvas. We may need to give up on not ever storing entities, but as long as all of the entity data is references into the display list we can still save on allocations.

…p is drawn.

…ter/engine#54215)

bdero · 2024-07-31T19:54:55Z

Reason for revert: Causing golden diffs in framework roll https://flutter-gold.skia.org/search?issue=152633&crs=github&patchsets=2&corpus=flutter

This reverts commit 11bce42.

…" (#54261) Reverts: #54215 Initiated by: bdero Reason for reverting: Causing golden diffs in framework roll https://flutter-gold.skia.org/search?issue=152633&crs=github&patchsets=2&corpus=flutter Original PR Author: bdero Reviewed By: {jonahwilliams} This change reverts the following previous change: Original PR: #54136 Revert PR: #54067 Includes fixes for issue seen in [these goldens](https://flutter-gold.skia.org/search?issue=152354&crs=github&patchsets=2&corpus=flutter). The problem was that the scissor was ending up wrong after clip replay for opaque draws that are supposed to occur outside the parent clip scope(s). To fix it, I made clip replay draws as well as the subpass texture draw apply lazily. For the clip replay, there's no need to apply a given clip until we come across an entity that has a depth less than or equal to the clip. And for the subpass texture (which is often translucent), we can defer drawing it until we come across another translucent draw. Deferring the subpass texture is important because if we draw it immediately, then all of the replay clips need to be drawn immediately too. ## Description For each clip scope, draw opaque items in reverse order and translucent/backdrop-independent items in their original order afterwards. Clips are treated as translucent by the parent scope. Respects clips, subpass collapse, and the clear color optimization. ### Local new_gallery before/after (iPhone 12 mini): ``` cd ~/projects/flutter/flutter/dev/integration_tests/new_gallery flutter drive --profile --local-engine-src-path ~/projects/flutter/engine/src --local-engine=ios_profile --local-engine-host=host_profile_arm64 --trace-startup -t test_driver/transitions_perf.dart -d 00008101-000A59A93C10001E ``` ![image](https://github.com/user-attachments/assets/7372c128-ca71-44a6-8e6c-b0043025f751)

…ter/engine#54215)

…sions) (#152648) Manual roll requested by jacksongardner@google.com flutter/engine@1633272...32f7888 2024-07-31 skia-flutter-autoroll@skia.org Roll Skia from 431b57332241 to b5ad5bf3696d (3 revisions) (flutter/engine#54259) 2024-07-31 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from cYjTzxe0MskG7PtkB... to uF76DfQgigt4utdBv... (flutter/engine#54258) 2024-07-31 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] Reland: Implement draw order optimization. (#54215)" (flutter/engine#54261) 2024-07-31 skia-flutter-autoroll@skia.org Roll Skia from cd58e87c06f9 to 431b57332241 (2 revisions) (flutter/engine#54257) 2024-07-31 chris@bracken.jp [iOS][macOS] Eliminate use of bitcode_strip (flutter/engine#54254) 2024-07-31 skia-flutter-autoroll@skia.org Roll Skia from e9e423457655 to cd58e87c06f9 (5 revisions) (flutter/engine#54253) 2024-07-31 matanlurey@users.noreply.github.com Cleanup the shell test, removing unused code (flutter/engine#54238) 2024-07-31 yjbanov@google.com [web] rename dialog to route to match the framework (flutter/engine#54228) 2024-07-31 skia-flutter-autoroll@skia.org Roll Dart SDK from edace067d950 to 5df6a6e0c037 (1 revision) (flutter/engine#54252) 2024-07-31 bdero@google.com [Impeller] Reland: Implement draw order optimization. (flutter/engine#54215) 2024-07-31 matanlurey@users.noreply.github.com Migrate the remaining real pub packages to pub workspaces. (flutter/engine#54232) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from cYjTzxe0MskG to uF76DfQgigt4 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 jacksongardner@google.com,rmistry@google.com,zra@google.com 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 time for sure! For each clip scope, draw opaque items in reverse order and translucent/backdrop-independent items in their original order afterwards. Clips are treated as translucent by the parent scope. Respects clips, subpass collapse, and the clear color optimization. Attempt 1: #54136 Revert 1: #54067 Attempt 2: #54215 Revert 2: #54261

…sions) (flutter#152648) Manual roll requested by jacksongardner@google.com flutter/engine@1633272...32f7888 2024-07-31 skia-flutter-autoroll@skia.org Roll Skia from 431b57332241 to b5ad5bf3696d (3 revisions) (flutter/engine#54259) 2024-07-31 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from cYjTzxe0MskG7PtkB... to uF76DfQgigt4utdBv... (flutter/engine#54258) 2024-07-31 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] Reland: Implement draw order optimization. (flutter#54215)" (flutter/engine#54261) 2024-07-31 skia-flutter-autoroll@skia.org Roll Skia from cd58e87c06f9 to 431b57332241 (2 revisions) (flutter/engine#54257) 2024-07-31 chris@bracken.jp [iOS][macOS] Eliminate use of bitcode_strip (flutter/engine#54254) 2024-07-31 skia-flutter-autoroll@skia.org Roll Skia from e9e423457655 to cd58e87c06f9 (5 revisions) (flutter/engine#54253) 2024-07-31 matanlurey@users.noreply.github.com Cleanup the shell test, removing unused code (flutter/engine#54238) 2024-07-31 yjbanov@google.com [web] rename dialog to route to match the framework (flutter/engine#54228) 2024-07-31 skia-flutter-autoroll@skia.org Roll Dart SDK from edace067d950 to 5df6a6e0c037 (1 revision) (flutter/engine#54252) 2024-07-31 bdero@google.com [Impeller] Reland: Implement draw order optimization. (flutter/engine#54215) 2024-07-31 matanlurey@users.noreply.github.com Migrate the remaining real pub packages to pub workspaces. (flutter/engine#54232) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from cYjTzxe0MskG to uF76DfQgigt4 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 jacksongardner@google.com,rmistry@google.com,zra@google.com 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

github-actions bot added the e: impeller label Jul 30, 2024

bdero added 6 commits July 30, 2024 06:36

[Impeller] Reland: Implement draw order optimization.

9e28901

Original PR: flutter#54136 Revert PR: flutter#54067

Force a draw order flush after clip restores are appended.

b6d0506

Lazily replay clips as they're needed for entities about to be rendered.

1607c9a

Revert "Force a draw order flush after clip restores are appended."

4899711

This reverts commit 6919636.

Defer subpass texture draw until subsequent opaque draws are finished.

943a965

Reset next replay index when switching subpasses

f6bcf1d

bdero force-pushed the bdero/reland-draw-order-optimization branch from 5828898 to f6bcf1d Compare July 30, 2024 13:36

flutter-dashboard bot added the will affect goldens label Jul 30, 2024

jonahwilliams reviewed Jul 30, 2024

View reviewed changes

jonahwilliams approved these changes Jul 30, 2024

View reviewed changes

Activate clip restore on just_created instead of when an MSAA backdro…

f268c6d

…p is drawn.

bdero merged commit 11bce42 into flutter:main Jul 31, 2024
28 checks passed

engine-flutter-autoroll mentioned this pull request Jul 31, 2024

Roll Flutter Engine from 16332725788c to 11bce425f515 (2 revisions) flutter/flutter#152633

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 31, 2024

11bce425f [Impeller] Reland: Implement draw order optimization. (flut…

5c9c777

…ter/engine#54215)

bdero added a commit that referenced this pull request Jul 31, 2024

Revert "[Impeller] Reland: Implement draw order optimization. (#54215)"

1e032ab

This reverts commit 11bce42.

bdero added the revert Label used to revert changes in a closed and merged pull request. label Jul 31, 2024

auto-submit bot pushed a commit that referenced this pull request Jul 31, 2024

Revert "[Impeller] Reland: Implement draw order optimization. (#54215)"

778d6cd

This reverts commit 11bce42.

auto-submit bot mentioned this pull request Jul 31, 2024

Reverts "[Impeller] Reland: Implement draw order optimization. (#54215)" #54261

Merged

auto-submit bot removed the revert Label used to revert changes in a closed and merged pull request. label Jul 31, 2024

engine-flutter-autoroll mentioned this pull request Jul 31, 2024

Roll Flutter Engine from 16332725788c to 32f788823f43 (11 revisions) flutter/flutter#152641

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 31, 2024

11bce425f [Impeller] Reland: Implement draw order optimization. (flut…

1f7efa3

…ter/engine#54215)

engine-flutter-autoroll mentioned this pull request Jul 31, 2024

Roll Flutter Engine from 16332725788c to 4caaab9f2502 (12 revisions) flutter/flutter#152644

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 31, 2024

11bce425f [Impeller] Reland: Implement draw order optimization. (flut…

41571f8

…ter/engine#54215)

engine-flutter-autoroll mentioned this pull request Jul 31, 2024

Manual roll Flutter Engine from 16332725788c to 32f788823f43 (11 revisions) flutter/flutter#152648

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 31, 2024

11bce425f [Impeller] Reland: Implement draw order optimization. (flut…

4e53696

…ter/engine#54215)

bdero mentioned this pull request Jul 31, 2024

[Impeller] Reland 2: Implement draw order optimization. #54268

Merged

engine-flutter-autoroll mentioned this pull request Jul 31, 2024

Roll Flutter Engine from 16332725788c to ed95b491f260 (14 revisions) flutter/flutter#152649

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 31, 2024

11bce425f [Impeller] Reland: Implement draw order optimization. (flut…

4540179

…ter/engine#54215)

bdero mentioned this pull request Aug 21, 2024

[Impeller] Reland 3: Implement draw order optimization. #54673

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Impeller] Reland: Implement draw order optimization. #54215

[Impeller] Reland: Implement draw order optimization. #54215

bdero commented Jul 30, 2024

flutter-dashboard bot commented Jul 30, 2024

jonahwilliams Jul 30, 2024

jonahwilliams Jul 30, 2024

bdero Jul 30, 2024

jonahwilliams left a comment

bdero commented Jul 31, 2024

		@@ -43,6 +43,8 @@ Entity::Entity(Entity&&) = default;

		Entity::Entity(const Entity&) = default;

		Entity& Entity::operator=(Entity&&) = default;

[Impeller] Reland: Implement draw order optimization. #54215

[Impeller] Reland: Implement draw order optimization. #54215

Conversation

bdero commented Jul 30, 2024

Description

Local new_gallery before/after (iPhone 12 mini):

flutter-dashboard bot commented Jul 30, 2024

jonahwilliams Jul 30, 2024

Choose a reason for hiding this comment

jonahwilliams Jul 30, 2024

Choose a reason for hiding this comment

bdero Jul 30, 2024

Choose a reason for hiding this comment

jonahwilliams left a comment

Choose a reason for hiding this comment

bdero commented Jul 31, 2024