Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Aug 1, 2024

Conversation

bdero
Copy link
Member

@bdero bdero commented Jul 31, 2024

Attempt 1: #54136
Revert 1: #54067
Attempt 2: #54215
Revert 2: #54261

This time for sure!

Includes fix for the golden failures seen here: https://flutter-gold.skia.org/search?issue=152633&crs=github&patchsets=2&corpus=flutter

The break in the clear color counter was in the wrong spot, so the counter wasn't breaking when coming across subpass elements (which are never considered to be clear-colors right now). This caused the counter to continue counting entities after subpasses sometimes, which can cause the count to end up too high and skip over non-clear color elements.

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

@bdero bdero self-assigned this Jul 31, 2024
@bdero bdero force-pushed the bdero/reland2-draw-order-optimization branch from 8bd048a to 357e292 Compare August 1, 2024 01:21
Copy link
Member

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bdero bdero merged commit 4dc94d6 into flutter:main Aug 1, 2024
28 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 1, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 1, 2024
…152663)

flutter/engine@f546fef...4dc94d6

2024-08-01 bdero@google.com [Impeller] Reland 2: Implement draw order optimization. (flutter/engine#54268)

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
@bdero
Copy link
Member Author

bdero commented Aug 2, 2024

Reason for revert: A downstream app reproduced and issue where certain draws are getting drawn with an incorrect scissor and getting clipped out.

(@jason-simmons tracked the issue to this commit)

@bdero bdero added the revert Label used to revert changes in a closed and merged pull request. label Aug 2, 2024
Copy link
Contributor

auto-submit bot commented Aug 2, 2024

Time to revert pull request flutter/engine/54268 has elapsed.
You need to open the revert manually and process as a regular pull request.

@auto-submit auto-submit bot removed the revert Label used to revert changes in a closed and merged pull request. label Aug 2, 2024
bdero added a commit to bdero/flutter-engine that referenced this pull request Aug 2, 2024
auto-submit bot pushed a commit that referenced this pull request Aug 3, 2024
…) (#54325)

Manual revert of #54268, since the time expired for automatic reverting.

@jason-simmons found this patch to be causing a regression in a downstream app. Some draws are getting clipped away, and disabling the scissor fixes it. So there is likely still a case not being handled correctly with the clip replay or subpass draw deferral behavior.
TytaniumDev pushed a commit to TytaniumDev/flutter that referenced this pull request Aug 7, 2024
…lutter#152663)

flutter/engine@f546fef...4dc94d6

2024-08-01 bdero@google.com [Impeller] Reland 2: Implement draw order optimization. (flutter/engine#54268)

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
Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024
…lutter#152663)

flutter/engine@f546fef...4dc94d6

2024-08-01 bdero@google.com [Impeller] Reland 2: Implement draw order optimization. (flutter/engine#54268)

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants