-
Notifications
You must be signed in to change notification settings - Fork 6k
[Impeller] Reland: Use the scissor to limit all draws by clip coverage. #51731
Conversation
The new golden reproduces the issue that triggered the revert. The problem was that the MSAA backdrop restore was getting erroneously limited by the clip coverage scissor. The fix is to just draw the restore before replaying the clips. Before: Screen.Recording.2024-03-27.at.3.13.55.PM.movAfter: Screen.Recording.2024-03-27.at.3.14.29.PM.mov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The new golden is missing for some reason. I'm going to push a blank commit to see if it shows up... |
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. |
…145877) flutter/engine@c602abd...922c7b1 2024-03-28 [email protected] Roll Fuchsia Linux SDK from Lk8KBU-c97ROj-YHm... to uzI3wnbEGlZ_dtO0Z... (flutter/engine#51737) 2024-03-28 [email protected] Roll Skia from b6160ffc0b96 to 66241be7c81c (16 revisions) (flutter/engine#51733) 2024-03-28 [email protected] [Impeller] Reland: Use the scissor to limit all draws by clip coverage. (flutter/engine#51731) 2024-03-27 [email protected] [Impeller] adds a `plus` advanced blend for f16 pixel formats (flutter/engine#51589) 2024-03-27 [email protected] Platform channel for predictive back in route transitions on android (flutter/engine#49093) 2024-03-27 [email protected] [Impeller] render empty filled paths without crashing. (flutter/engine#51713) 2024-03-27 [email protected] [Impeller] use optimal depth attachment, remove useless barrier. (flutter/engine#51723) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from Lk8KBU-c97RO to uzI3wnbEGlZ_ 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 [email protected],[email protected],[email protected] 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
At least on the flutter gallery benchmark, we're back to the pre StC baseline: |
…isions) (#145877)" (#145901) Reverts: #145877 Initiated by: zanderso Reason for reverting: #145899 Original PR Author: engine-flutter-autoroll Reviewed By: {fluttergithubbot} This change reverts the following previous change: flutter/engine@c602abd...922c7b1 2024-03-28 [email protected] Roll Fuchsia Linux SDK from Lk8KBU-c97ROj-YHm... to uzI3wnbEGlZ_dtO0Z... (flutter/engine#51737) 2024-03-28 [email protected] Roll Skia from b6160ffc0b96 to 66241be7c81c (16 revisions) (flutter/engine#51733) 2024-03-28 [email protected] [Impeller] Reland: Use the scissor to limit all draws by clip coverage. (flutter/engine#51731) 2024-03-27 [email protected] [Impeller] adds a `plus` advanced blend for f16 pixel formats (flutter/engine#51589) 2024-03-27 [email protected] Platform channel for predictive back in route transitions on android (flutter/engine#49093) 2024-03-27 [email protected] [Impeller] render empty filled paths without crashing. (flutter/engine#51713) 2024-03-27 [email protected] [Impeller] use optimal depth attachment, remove useless barrier. (flutter/engine#51723) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from Lk8KBU-c97RO to uzI3wnbEGlZ_ 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 [email protected],[email protected],[email protected] 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
…sions) (#145903) Manual roll requested by [email protected] flutter/engine@c602abd...9df2d3a 2024-03-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] adds a `plus` advanced blend for f16 pixel formats (#51589)" (flutter/engine#51741) 2024-03-28 [email protected] [Impeller] correct multisample resolve/store configuration for Vulkan. (flutter/engine#51740) 2024-03-28 [email protected] Remove Impeller/OpenGLES from CI branch for Android e2e tests. (flutter/engine#51734) 2024-03-28 [email protected] Roll Fuchsia Linux SDK from Lk8KBU-c97ROj-YHm... to uzI3wnbEGlZ_dtO0Z... (flutter/engine#51737) 2024-03-28 [email protected] Roll Skia from b6160ffc0b96 to 66241be7c81c (16 revisions) (flutter/engine#51733) 2024-03-28 [email protected] [Impeller] Reland: Use the scissor to limit all draws by clip coverage. (flutter/engine#51731) 2024-03-27 [email protected] [Impeller] adds a `plus` advanced blend for f16 pixel formats (flutter/engine#51589) 2024-03-27 [email protected] Platform channel for predictive back in route transitions on android (flutter/engine#49093) 2024-03-27 [email protected] [Impeller] render empty filled paths without crashing. (flutter/engine#51713) 2024-03-27 [email protected] [Impeller] use optimal depth attachment, remove useless barrier. (flutter/engine#51723) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from Lk8KBU-c97RO to uzI3wnbEGlZ_ 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 [email protected],[email protected],[email protected] 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
Reland #51698.
This reverts commit 73c145c.
Attempts to improve flutter/flutter#145274.
Our new clipping technique paints walls on the depth buffer "in front" of the Entities that will be affected by said clips. So when an intersect clip is drawn (the common case), the clip will cover the whole framebuffer.
Depth is divvied up such that deeper clips get drawn behind shallower clips, and so many clips actually don't end up drawing depth across the whole framebuffer. However, if the app does a lot of transitioning from a huge clips to a small clips, a lot of unnecessary depth churn occurs (very common in Flutter -- this happens with both the app bar and floating action button in the counter template, for example).
Since progressively deeper layers in the clip coverage stack always subset their parent layers, we can reduce waste for small intersect clips by setting the scissor to the clip coverage rect instead of drawing the clip to the whole screen.
Note that this change does not help much with huge/fullscreen clips.
Also, we could potentially improve this further by computing much stricter bounds. Rather than just using clip coverage for the scissor, we could intersect it with the union of all draws affected by the clip at the cost of a bit more CPU churn per draw. I don't think that's enough juice for the squeeze though.
Before (
Play/AiksTest.CanRenderNestedClips/Metal
):Screen.Recording.2024-03-26.at.7.34.29.PM.mov
After (
Play/AiksTest.CanRenderNestedClips/Metal
):Screen.Recording.2024-03-26.at.7.38.33.PM.mov