[Impeller] stencil buffer record/replay instead of MSAA storage. #47397

jonahwilliams · 2023-10-27T20:49:19Z

When restoring from a backdrop filter, replay the clip affecting cmds into a new stencil buffer instead of storing 4x MSAA stencil buffer.

Fixes flutter/flutter#137561
Fixes flutter/flutter#137448
Fixes flutter/flutter#137302

Helps flutter/flutter#137108

…ave_stencil

jonahwilliams · 2023-10-31T20:55:15Z

I think (hope) we have golden test coverage for this, right? 😄

jonahwilliams · 2023-10-31T21:05:53Z

Currently investigating performance on iOS using the backdrop filter perf (Partial repaint disabled). With this change:

  "average_gpu_frame_time": 247.9492189,
  "90th_percentile_gpu_frame_time": 249.023438,
  "99th_percentile_gpu_frame_time": 249.023438,
  "worst_gpu_frame_time": 249.023438,
  ...
   "average_frame_rasterizer_time_millis": 225.71757142857146,
  "stddev_frame_rasterizer_time_millis": 0.623428571428572,
  "90th_percentile_frame_rasterizer_time_millis": 226.62,
  "99th_percentile_frame_rasterizer_time_millis": 227.661,
  "worst_frame_rasterizer_time_millis": 227.661,

Without it

  "average_gpu_frame_time": 307.61718740000003,
"90th_percentile_gpu_frame_time": 310.058594,
"99th_percentile_gpu_frame_time": 310.058594,
"worst_gpu_frame_time": 310.058594,

...

  "average_frame_rasterizer_time_millis": 279.90400000000005,
"stddev_frame_rasterizer_time_millis": 1.0763076923076982,
"90th_percentile_frame_rasterizer_time_millis": 281.933,
"99th_percentile_frame_rasterizer_time_millis": 282.053,
"worst_frame_rasterizer_time_millis": 282.053,

So measurably better!

bdero

Just minor comments.

I really hope this doesn't generally regress Metal in the common case. I'd much rather have this recorder/replay mechanism than the total_pass_reads_ mess.

bdero · 2023-10-31T21:46:12Z

impeller/entity/entity_pass.h

+///
+///        These clips are replayed when restoring the backdrop so that the
+///        stencil buffer is left in an identical state.
+class EntityPassClipReplay {


EntityPassClipRecorder?

That is much better

bdero · 2023-10-31T21:50:11Z

impeller/entity/entity_pass.cc

-            .clear_color = clear_color},   // color_attachment_config
-        GetDefaultStencilConfig(readable)  // stencil_attachment_config
+            .clear_color = clear_color},  // color_attachment_config
+        kDefaultStencilConfig             // stencil_attachment_config


Should be able to remove the readable param from CreateRenderTarget now

bdero · 2023-10-31T21:51:42Z

impeller/entity/inline_pass_context.h

@@ -40,7 +40,7 @@ class InlinePassContext {
  std::shared_ptr<CommandBuffer> command_buffer_;
  std::shared_ptr<RenderPass> pass_;
  uint32_t pass_count_ = 0;
-  uint32_t total_pass_reads_ = 0;


Good riddance...

jonahwilliams · 2023-11-01T00:28:29Z

Alright lets see this number go down:

https://flutter-flutter-perf.skia.org/e/?queries=device_type%3DPixel_7_Pro%26sub_result%3Daverage_frame_rasterizer_time_millis%26sub_result%3Daverage_gpu_frame_time%26test%3Danimated_advanced_blend_perf__timeline_summary%26test%3Danimated_advanced_blend_perf_opengles__timeline_summary&selected=commit%3D37731%26name%3D%252Carch%253Dintel%252Cbranch%253Dmaster%252Cconfig%253Ddefault%252Cdevice_type%253DPixel_7_Pro%252Cdevice_version%253Dnone%252Chost_type%253Dlinux%252Csub_result%253Daverage_frame_rasterizer_time_millis%252Ctest%253Danimated_advanced_blend_perf_opengles__timeline_summary%252C

…rage. (flutter/engine#47397)

…137656) flutter/engine@db06c2e...a0ac6b4 2023-11-01 bdero@google.com [Impeller] Include cstdint everywhere that uint32_t is used. (flutter/engine#47533) 2023-11-01 bdero@google.com [Impeller] Fix nullopt access and simplify coverage computation in GetSubpassCoverage. (flutter/engine#47347) 2023-11-01 bdero@google.com [Impeller] OpenGLES: Ensure frag/vert textures are bound with unique texture units. (flutter/engine#47218) 2023-11-01 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from LCfhx_lTRJI51G0zc... to _TyF0etsONe5aqCbM... (flutter/engine#47532) 2023-11-01 jonahwilliams@google.com [Impeller] stencil buffer record/replay instead of MSAA storage. (flutter/engine#47397) 2023-11-01 chris@bracken.jp [macOS] Delete FlutterCompositor tests (flutter/engine#47527) 2023-10-31 bdero@google.com [Impeller] Place Rect statics under the Rect template. (flutter/engine#47529) 2023-10-31 skia-flutter-autoroll@skia.org Roll Skia from aaa225e0cc6d to 34ef20100acc (1 revision) (flutter/engine#47530) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from LCfhx_lTRJI5 to _TyF0etsONe5 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 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

…ge. (flutter#47397)" This reverts commit 04329c5.

…storage. (#51416) Reverts #47397 to fix flutter/flutter#144211 in 3.19. This reverts commit 04329c5.

jonahwilliams added 4 commits October 26, 2023 11:16

[Impeller] remove giant closure.

a74b15b

conditionally save stencil

fa8ebf8

Merge branch 'main' of github.com:flutter/engine into conditionally_s…

210cb6e

…ave_stencil

testing

46b416f

github-actions bot added the e: impeller label Oct 27, 2023

jonahwilliams mentioned this pull request Oct 27, 2023

[Impeller] Clip record/replay for backdrop filter flutter/flutter#137448

Closed

jonahwilliams added 5 commits October 31, 2023 10:24

Merge branch 'main' of github.com:flutter/engine into conditionally_s…

9649438

…ave_stencil

++

c8f6ca3

++

5bed1c2

++

96589db

++

0ad0fbc

jonahwilliams changed the title ~~Conditionally save stencil~~ [Impeller] stencil buffer record/replay instead of MSAA storage. Oct 31, 2023

jonahwilliams marked this pull request as ready for review October 31, 2023 20:52

jonahwilliams requested a review from bdero October 31, 2023 20:54

bdero approved these changes Oct 31, 2023

View reviewed changes

bdero review.

7ef468b

jonahwilliams mentioned this pull request Nov 1, 2023

[Impeller] add support for specialization constants. #47432

Merged

jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 1, 2023

auto-submit bot merged commit 04329c5 into flutter:main Nov 1, 2023
25 checks passed

jonahwilliams deleted the conditionally_save_stencil branch November 1, 2023 00:28

engine-flutter-autoroll mentioned this pull request Nov 1, 2023

Roll Flutter Engine from db06c2e10459 to 04329c5d00d7 (4 revisions) flutter/flutter#137653

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 1, 2023

04329c5d0 [Impeller] stencil buffer record/replay instead of MSAA sto…

502c303

…rage. (flutter/engine#47397)

engine-flutter-autoroll mentioned this pull request Nov 1, 2023

Roll Flutter Engine from db06c2e10459 to a0ac6b432cdf (8 revisions) flutter/flutter#137656

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 1, 2023

04329c5d0 [Impeller] stencil buffer record/replay instead of MSAA sto…

052d9b6

…rage. (flutter/engine#47397)

matanlurey mentioned this pull request Nov 14, 2023

Allow (on demand/toggle) running post-submits as pre-submits flutter/flutter#138430

Closed

1 task

mikeesouth mentioned this pull request Feb 25, 2024

Issue with Flutter 3.19 iOS impeller optimization for BackdropFilter rive-app/rive-flutter#360

Closed

jonahwilliams mentioned this pull request Mar 5, 2024

[Impeller] Issue with Flutter 3.19 optimization for BackdropFilter with animation flutter/flutter#144211

Closed

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

Revert "[Impeller] stencil buffer record/replay instead of MSAA stora…

599a2be

…ge. (flutter#47397)" This reverts commit 04329c5.

bdero mentioned this pull request Mar 14, 2024

[CP] Revert "[Impeller] stencil buffer record/replay instead of MSAA storage. #51416

Merged

auto-submit bot pushed a commit that referenced this pull request Mar 19, 2024

[CP] Revert "[Impeller] stencil buffer record/replay instead of MSAA …

ba0444a

…storage. (#51416) Reverts #47397 to fix flutter/flutter#144211 in 3.19. This reverts commit 04329c5.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Impeller] stencil buffer record/replay instead of MSAA storage. #47397

[Impeller] stencil buffer record/replay instead of MSAA storage. #47397

jonahwilliams commented Oct 27, 2023 •

edited

Loading

jonahwilliams commented Oct 31, 2023

jonahwilliams commented Oct 31, 2023

bdero left a comment •

edited

Loading

bdero Oct 31, 2023

jonahwilliams Oct 31, 2023

bdero Oct 31, 2023

jonahwilliams Oct 31, 2023

bdero Oct 31, 2023

jonahwilliams commented Nov 1, 2023

[Impeller] stencil buffer record/replay instead of MSAA storage. #47397

[Impeller] stencil buffer record/replay instead of MSAA storage. #47397

Conversation

jonahwilliams commented Oct 27, 2023 • edited Loading

jonahwilliams commented Oct 31, 2023

jonahwilliams commented Oct 31, 2023

bdero left a comment • edited Loading

Choose a reason for hiding this comment

bdero Oct 31, 2023

Choose a reason for hiding this comment

jonahwilliams Oct 31, 2023

Choose a reason for hiding this comment

bdero Oct 31, 2023

Choose a reason for hiding this comment

jonahwilliams Oct 31, 2023

Choose a reason for hiding this comment

bdero Oct 31, 2023

Choose a reason for hiding this comment

jonahwilliams commented Nov 1, 2023

jonahwilliams commented Oct 27, 2023 •

edited

Loading

bdero left a comment •

edited

Loading