DisplayListBuilder internal reorganization with better rendering op overlap detection #52646
Conversation
flar
force-pushed
the
dl-builder-reorg-and-peephole-overlap
branch
from
649557c to
1b80ea0
Compare
There was a problem hiding this comment.
Overall looks good to me, some notes:
-
While we don't support this yet, I bet we could handle opacity peephole even with overlapping draws if we correctly applied the overdraw prevention logic - though I think we'd have to draw in reverse order?. I'm not sure how this interacts with the stencil then cover changes though, and we don't have an API to request that yet. Something to chat with Brandon about when he is back.
-
An issue for another day, but for drawAtlas we might want to always treat as overlapping and/or honor the cull rect as the bounds if provided. In cases where users create 1000s of sprites I could imagine recomputing the bounds being expensive.
-
While the code looks good to me, I'm not sure which part of our test harness will actually execute this. I know aiks unittessts will skip it, but there is the dl playground - though that isn't hooked up to gold. I would recommend some additional offline verification if you haven't already done that.
LGTM
I'm not sure how that would work (what is overdraw prevention logic? reverse painting with a depth buffer?) since you'd have to compute the blends of the overlapping sections...? I'd love to see a great solution there.
The cull rect bounds are documented as a suggestion only for quick rejection, so ... hmmm... A developer could interpret them as something to be very conservative with and we'd end up doing extra work on partial repaints or the dispatch culling that happens a lot in Platform Views. Something to benchmark and maybe we could have a threshold or a flag indicating how conservative they were.
I had to deal with a lot of failures from the dl_unittests that test these conditions. Trust me that minor defects in the DL code can easily generate hundreds of failures from past experience. We don't have decent coverage of Impeller driven from DL, but that doesn't mean that DL is not well tested... |
|
Thinking about it more, the overdraw prevention would only work if all of the entities we would be apply opacity peephole to were originally fully opaque, otherwise the blending problem, like you mentioned. |
…ering op overlap detection (flutter/engine#52646)
…ering op overlap detection (flutter/engine#52646)
…147994) flutter/engine@f3ce5a5...9cb60de 2024-05-08 skia-flutter-autoroll@skia.org Roll Skia from c7794fee8063 to d8427844b843 (2 revisions) (flutter/engine#52671) 2024-05-08 skia-flutter-autoroll@skia.org Manual roll Dart SDK from a9cf6a411c71 to b7cad2edae4b (7 revisions) (flutter/engine#52669) 2024-05-08 jonahwilliams@google.com [Impeller] Update BlitPass::AddCopy to use destination_region instead of origin for buffer to texture copies. (flutter/engine#52555) 2024-05-08 flar@google.com DisplayListBuilder internal reorganization with better rendering op overlap detection (flutter/engine#52646) 2024-05-08 zanderso@users.noreply.github.com Roll reclient forward (flutter/engine#52632) 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
|
Some of the golden changes look like bugs... Reverting... |
This commit makes some long-needed internal improvements to the way that the DisplayListBuilder manages its per-save/saveLayer data. The information for layer bounds and matrix/clips is now maintained in the layer info structure itself rather than shared across a number of stack structures which required careful alignment of the 3 different stacks and made it more difficult to compare and update adjacent layers during save and restore operations.
The new code stores all information for a layer within a single structure and the save and restore operations can be more clear about which information they are getting or setting in the current and parent layers.
In addition, the layer bounds accumulations were updated to have a more flexible algorithm for detecting overlap of rendering operations for the opacity peephole optimization. Previously, more than one rendering op on a layer would prevent opacity peephole optimizations, but now the condition will be recognized until the first rendering op that overlaps the bounds of the previous rendering operations. This will help for some potentially common cases of 2 non-overlapping ops or even a list of rendering operations laid out in a row.