[Impeller] Implement draw order optimization. #54067

bdero · 2024-07-24T03:22:04Z

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

bdero · 2024-07-24T03:33:17Z

impeller/entity/contents/color_source_contents.h

+    // Enable depth writing for all opaque entities in order to allow
+    // reordering. Opaque entities are coerced to source blending by
+    // `EntityPass::AddEntity`.
+    options.depth_write_enabled = options.blend_mode == BlendMode::kSource;


Here's where depth writing gets turned on for all opaque draws.

flutter-dashboard · 2024-07-24T19:59:54Z

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 #54067 at sha a6fdabe

bdero · 2024-07-25T03:28:53Z

I think the remaining goldens are stuck on the previous revision. Can't repro the Play/AiksTest.MaskBlurVariantTestSolidTranslucentExclusionBlend/Metal issue after the second depth writing fix. Did an empty push to see if I can get it to budge.

flutter-dashboard · 2024-07-25T04:35:14Z

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #54067 at sha b641611

jonahwilliams

Youve got some golden diffs that look unexpected. beyond that - if we forget to update the depth write somewhere is that going to lead to visual bugs? For example filters, runtime effect, et cetera?

jonahwilliams · 2024-07-25T04:55:15Z

impeller/entity/draw_order_resolver.h

+    /// are order independent, and so we draw them optimally render these
+    /// elements in reverse painter's order so that they cull one another


Suggested change

/// are order independent, and so we draw them optimally render these

/// elements in reverse painter's order so that they cull one another

/// are order independent, and so we draw them optimally in reverse painter's order so that they cull one another

sentence seems weird

jonahwilliams · 2024-07-25T04:55:41Z

impeller/entity/draw_order_resolver.h

+    /// The list of backdrop-dependent elements with respect to this draw
+    /// order layer. These elements are drawn after all of the independent
+    /// elements.
+    /// The elements of all child draw layers will be resolved to this list.


The elements of all child draw layers will be resolved to this list

What does this mean?

Meant to say child clips. Removed, it's redundant anyway.

flutter-dashboard · 2024-07-25T09:17:06Z

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #54067 at sha 2a3239a

…ilable

bdero · 2024-07-25T13:24:32Z

if we forget to update the depth write somewhere is that going to lead to visual bugs? For example filters, runtime effect, et cetera?

Yes. Color sources are taken care of and filters are considered translucent by default. The corner cases were with stuff like backdrop filters/restores, the advanced blend fallback path, filters that return an AnonymousContents that happens to draw other contents, etc.

jonahwilliams

RuntimeEffect?

jonahwilliams · 2024-07-25T17:28:27Z

impeller/entity/entity_pass.cc

+  ElementIterator element_iterator;
+
+  if (renderer.GetDeviceCapabilities().SupportsFramebufferFetch()) {
+    element_iterator =


I think this should be fairly straightforward to port to experimental canvas, but we'd need to be able to configure the dispatch behavior.

We'd need to dispatch (per save Layer) once backwards through the draws for backdrop independent elements, and once forward pass for backdrop dependent elements,

FYI @flar

The storage buffer is not bidirectionally traversable for a couple of reasons

it is packed rather than indexed with offsets between elements

if we created an index, much of the state data is recorded incrementally

this includes transform, clips, paint attributes

The entities are not incremental for any state, they hold all of their state in absolute terms so it is easy to go forwards/backwards or even to just visit a single entity in isolation.

There is a bit of redesign that would have to happen to make that work.

bidirection iteration isn't strictly necessary, another approach would be to only iterate forwards and record the internal offsets, then allow redispatching a subset of them.

I should have said "... bidirectional, or even random access dispatching ...". Note the second bullet point starts with "If we created an index" meaning that isn't enough to get us where we need to be for this technique...

We might not want to expose that as "here you go save some pointers and offsets", but since the recorded structure is immutable-ish I'd imagine there is a safe interface we could build for that?

We might be able to get away with a list of structures containing:

buffer offset of op

fully populated Paint representing the attribute state at that offset

impeller::Matrix representing the fully composed transform at that offset

(clip stuff??)

I'm not sure what clip stuff we'd need. Probably whatever the DrawOrder optimizer here is recording.

bdero · 2024-07-25T22:47:56Z

RuntimeEffect?

Yup this is covered. ColorSourceContents::DrawGeometry takes care of it.

jonahwilliams

LGTM

…r if no flushes have occurred yet

…ne#54067)

jonahwilliams · 2024-07-26T03:42:57Z

reason for revert: disappearing fabs and background color in framework golden tests https://flutter-gold.skia.org/search?issue=152354&crs=github&patchsets=2&corpus=flutter

This reverts commit 1df62d3.

) Reverts: #54067 Initiated by: jonahwilliams Reason for reverting: disappearing fabs and background color in framework golden tests https://flutter-gold.skia.org/search?issue=152354&crs=github&patchsets=2&corpus=flutter Original PR Author: bdero Reviewed By: {jonahwilliams} This change reverts the following previous change: 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)

…ne#54067)

…ions) (#152379) Manual roll requested by zra@google.com flutter/engine@8714b54...342a425 2024-07-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] Implement draw order optimization. (#54067)" (flutter/engine#54136) 2024-07-26 skia-flutter-autoroll@skia.org Roll Skia from 06b26a1d51d7 to cd892b576ade (1 revision) (flutter/engine#54134) 2024-07-26 chinmaygarde@google.com Directly use 4x4 matrices with surface textures instead of converting to and from the 3x3 variants. (flutter/engine#54126) 2024-07-26 skia-flutter-autoroll@skia.org Roll Skia from e9b8585af6b5 to 06b26a1d51d7 (1 revision) (flutter/engine#54132) 2024-07-26 bdero@google.com [Impeller] Implement draw order optimization. (flutter/engine#54067) 2024-07-26 skia-flutter-autoroll@skia.org Roll Fuchsia Test Scripts from clqtZA8cx4GEXwcOe... to dUCMHqU6ihfIFKAw8... (flutter/engine#54130) 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 jonahwilliams@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

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

Original PR: #54136 Revert PR: #54067 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.

…" (#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)

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

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

…ions) (flutter#152379) Manual roll requested by zra@google.com flutter/engine@8714b54...342a425 2024-07-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] Implement draw order optimization. (flutter#54067)" (flutter/engine#54136) 2024-07-26 skia-flutter-autoroll@skia.org Roll Skia from 06b26a1d51d7 to cd892b576ade (1 revision) (flutter/engine#54134) 2024-07-26 chinmaygarde@google.com Directly use 4x4 matrices with surface textures instead of converting to and from the 3x3 variants. (flutter/engine#54126) 2024-07-26 skia-flutter-autoroll@skia.org Roll Skia from e9b8585af6b5 to 06b26a1d51d7 (1 revision) (flutter/engine#54132) 2024-07-26 bdero@google.com [Impeller] Implement draw order optimization. (flutter/engine#54067) 2024-07-26 skia-flutter-autoroll@skia.org Roll Fuchsia Test Scripts from clqtZA8cx4GEXwcOe... to dUCMHqU6ihfIFKAw8... (flutter/engine#54130) 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 jonahwilliams@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

bdero added the e: impeller label Jul 24, 2024

bdero self-assigned this Jul 24, 2024

bdero commented Jul 24, 2024

View reviewed changes

bdero added 5 commits July 24, 2024 12:47

[Impeller] Implement draw order optimization.

6790f13

Use offsets instead of pointers

38e33e6

Refactor into separate TU and add tests.

19754c0

Fix use-after-move

2679d8f

Correct tests

0357f8b

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

Fix depth writing for TextureContents

d918b47

bdero force-pushed the bdero/draw-order-optimization branch from a6fdabe to d918b47 Compare July 24, 2024 20:50

bdero added 2 commits July 24, 2024 19:48

Disable depth writing for the MSAA backdrop case

ae467b2

blank

cffcd69

bdero requested review from chinmaygarde and jonahwilliams July 25, 2024 03:29

Correct doc strings

b641611

jonahwilliams reviewed Jul 25, 2024

View reviewed changes

bdero added 2 commits July 25, 2024 01:18

Fix comments

70d065e

Switch order for kSolid blur style in the 2-pass blur

2a3239a

Handle backdrop restores and disable when FramebufferFetch is not ava…

4fa2da3

…ilable

bdero requested a review from jonahwilliams July 25, 2024 13:26

bdero added 2 commits July 25, 2024 06:35

Fix crasher for backdrop restore

68656ff

Respect the clear color optimization in the fallback

d874c32

bdero force-pushed the bdero/draw-order-optimization branch from 41309b8 to d874c32 Compare July 25, 2024 13:56

jonahwilliams reviewed Jul 25, 2024

View reviewed changes

jonahwilliams approved these changes Jul 25, 2024

View reviewed changes

When resolving the final list, only apply the values to the root laye…

3e5d0ab

…r if no flushes have occurred yet

bdero merged commit 1df62d3 into flutter:main Jul 26, 2024
28 checks passed

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

Roll Flutter Engine from 8714b54a87c0 to 1df62d3326d1 (2 revisions) flutter/flutter#152354

Closed

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

1df62d332 [Impeller] Implement draw order optimization. (flutter/engi…

8a70c22

…ne#54067)

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

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

Revert "[Impeller] Implement draw order optimization. (#54067)"

db2c987

This reverts commit 1df62d3.

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

Reverts "[Impeller] Implement draw order optimization. (#54067)" #54136

Merged

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

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

Manual roll Flutter Engine from 8714b54a87c0 to 342a42547822 (6 revisions) flutter/flutter#152379

Merged

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

1df62d332 [Impeller] Implement draw order optimization. (flutter/engi…

f6fea90

…ne#54067)

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

Roll Flutter Engine from 8714b54a87c0 to 354abf2800a0 (13 revisions) flutter/flutter#152381

Closed

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

1df62d332 [Impeller] Implement draw order optimization. (flutter/engi…

b138a66

…ne#54067)

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

[Impeller] Reland: Implement draw order optimization.

0754511

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

bdero mentioned this pull request Jul 30, 2024

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

Merged

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

[Impeller] Reland: Implement draw order optimization.

9e28901

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

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

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

Merged

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

[Impeller] Reland: Implement draw order optimization.

a3d33b9

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

bdero mentioned this pull request Jul 31, 2024

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

Merged

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] Implement draw order optimization. #54067

[Impeller] Implement draw order optimization. #54067

bdero commented Jul 24, 2024 •

edited

Loading

bdero Jul 24, 2024 •

edited

Loading

flutter-dashboard bot commented Jul 24, 2024

bdero commented Jul 25, 2024 •

edited

Loading

flutter-dashboard bot commented Jul 25, 2024

jonahwilliams left a comment

jonahwilliams Jul 25, 2024

bdero Jul 25, 2024

jonahwilliams Jul 25, 2024

bdero Jul 25, 2024

flutter-dashboard bot commented Jul 25, 2024

bdero commented Jul 25, 2024

jonahwilliams left a comment

jonahwilliams Jul 25, 2024

flar Jul 25, 2024

jonahwilliams Jul 25, 2024

flar Jul 25, 2024

jonahwilliams Jul 25, 2024

flar Jul 25, 2024 •

edited

Loading

jonahwilliams Jul 25, 2024

bdero commented Jul 25, 2024

jonahwilliams left a comment

jonahwilliams commented Jul 26, 2024

		/// are order independent, and so we draw them optimally render these
		/// elements in reverse painter's order so that they cull one another

[Impeller] Implement draw order optimization. #54067

[Impeller] Implement draw order optimization. #54067

Conversation

bdero commented Jul 24, 2024 • edited Loading

Local new_gallery before/after (iPhone 12 mini):

bdero Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

flutter-dashboard bot commented Jul 24, 2024

bdero commented Jul 25, 2024 • edited Loading

flutter-dashboard bot commented Jul 25, 2024

jonahwilliams left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flutter-dashboard bot commented Jul 25, 2024

bdero commented Jul 25, 2024

jonahwilliams left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flar Jul 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdero commented Jul 25, 2024

jonahwilliams left a comment

Choose a reason for hiding this comment

jonahwilliams commented Jul 26, 2024

bdero commented Jul 24, 2024 •

edited

Loading

bdero Jul 24, 2024 •

edited

Loading

bdero commented Jul 25, 2024 •

edited

Loading

flar Jul 25, 2024 •

edited

Loading