Reland "DisplayListBuilder internal reorganization with better rendering op overlap detection" (52646) #53002
Conversation
…ring op overlap detection" (flutter#52725) This reverts commit f312281.
|
This PR is now ready for review. The first commit is the fix from before, the second commit is the fixes, and the rest of the commits are formatting and new unit tests to stop regressions. |
| @@ -545,7 +545,7 @@ class DisplayListBuilder final : public virtual DlCanvas, | |||
| is_save_layer(true), | |||
| rtree_rects_start_index(rtree_rect_index), | |||
| global_state(parent_info->global_state), | |||
| layer_state(parent_info->global_state.local_cull_rect()), | |||
| layer_state(kMaxCullRect), | |||
There was a problem hiding this comment.
So the bug was that we were sizing the cull rects of child layers too tightly, whereas they'd need to expand for operations like destructive blends?
There was a problem hiding this comment.
The primary bug was that when a matrix transforming ImageFilter is involved, I have to adjust all of the values. I changed the way I adjust bounds for the filter to be more accurate in a couple of places and changed how I determine the "clip" to add to the layer in the AccumulateUnbounded method, but this was another aspect - I was using the local cull rect of the parent's global state - which is in the right coordinate transform space - but it isn't adjusted by any actions of the layer's filter. I could have done an adjustment here, but there was another problem happening which I discovered during the fixing that caused some unit tests to fail which is...
The primary reason that I'm accumulating layer bounds in the first place is so that I can compare the true unclipped bounds of the content to the bounds they gave us and see if the bounds they gave us clipped some of the content. If I pre-seed the layer_state with a clip, then I don't really know for certain and some of the "layer bounds clipped content" tests started failing. I'm not sure why they passed originally, but it might be because enough of the rest of the problems existed that this didn't really come into play.
In either case, the "G3 bug" part of this line changing was that the bounds used to seed the layer state were not adjusted by the ImageFilter on the layer. The other problem was independent and another reason to make this one change.
There was a problem hiding this comment.
In other words, the difference in "coordinate system" taken in a very loose definition is both because the local and global states have different matrices - and partially because the ImageFilter itself can grow/shrink/move the content anywhere in the parent's plane so all bounds adjustments between layer and parent need to take the ImageFilter into account in addition to their different transform matrices.
…ter rendering op overlap detection" (52646) (flutter/engine#53002)
…149068) flutter/engine@f693fdd...e020713 2024-05-24 skia-flutter-autoroll@skia.org Roll Dart SDK from 4ef8ed68a8d4 to 641d61332238 (1 revision) (flutter/engine#53029) 2024-05-24 skia-flutter-autoroll@skia.org Roll Skia from 10459d97152a to 0b7d656b9c03 (3 revisions) (flutter/engine#53028) 2024-05-24 flar@google.com Reland "DisplayListBuilder internal reorganization with better rendering op overlap detection" (52646) (flutter/engine#53002) 2024-05-24 skia-flutter-autoroll@skia.org Roll Skia from 137a4ea4e033 to 10459d97152a (3 revisions) (flutter/engine#53027) 2024-05-24 skia-flutter-autoroll@skia.org Roll Skia from d252bca326a6 to 137a4ea4e033 (1 revision) (flutter/engine#53026) 2024-05-24 skia-flutter-autoroll@skia.org Roll Skia from 97783ac3000d to d252bca326a6 (2 revisions) (flutter/engine#53025) 2024-05-24 chinmaygarde@google.com Rename Skia specific TUs. (flutter/engine#52855) 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
…lutter#149068) flutter/engine@f693fdd...e020713 2024-05-24 skia-flutter-autoroll@skia.org Roll Dart SDK from 4ef8ed68a8d4 to 641d61332238 (1 revision) (flutter/engine#53029) 2024-05-24 skia-flutter-autoroll@skia.org Roll Skia from 10459d97152a to 0b7d656b9c03 (3 revisions) (flutter/engine#53028) 2024-05-24 flar@google.com Reland "DisplayListBuilder internal reorganization with better rendering op overlap detection" (52646) (flutter/engine#53002) 2024-05-24 skia-flutter-autoroll@skia.org Roll Skia from 137a4ea4e033 to 10459d97152a (3 revisions) (flutter/engine#53027) 2024-05-24 skia-flutter-autoroll@skia.org Roll Skia from d252bca326a6 to 137a4ea4e033 (1 revision) (flutter/engine#53026) 2024-05-24 skia-flutter-autoroll@skia.org Roll Skia from 97783ac3000d to d252bca326a6 (2 revisions) (flutter/engine#53025) 2024-05-24 chinmaygarde@google.com Rename Skia specific TUs. (flutter/engine#52855) 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
Reverts #52725
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.