[root layer scrolls] failure to promote child of uncomposited scroller which overlaps promoted content outside it |
||||
Issue descriptionWith RLS (all platforms), this test fails with an image diff caused by incorrect stacking of the blue and green boxes. $ blink/tools/run_layout_tests.sh -t d --additional-driver-flag=--root-layer-scrolls compositing/overflow/nested-render-surfaces-with-intervening-clip.html
,
Oct 24 2017
Ok, I've found the reason for the difference but not the solution yet. The issue lies in the clipped/unclipped split of overlap map. Since the blue boxes are inside an uncomposited scroller, they always add their *clipped* bounding box. Without RLS, the frame isn't considered a composited scrolling ancestor layer so the position fixed element does too. But if we turn on RLS then suddenly the frame is a composited scrolling ancestor and so the position: fixed box adds its _unclipped_ bounding box. Since it's added first, the clipped bounding boxes following it won't check for overlap against the unclipped box and so they don't realize they need to composite for correct paint order. I'm not yet sure what the correct solution is - I'll continue studying the code to understand better (I don't completely understand why unclipped can be compared with either type of rect but not vice versa) but if any of you know off the top of your head you could help expedite the process. Perhaps an exception for the root layer is appropriate? IIRC from diving through PaintLayerClipper we don't clip the root layer at all...
,
Oct 25 2017
We do special-case the root layer for some position:fixed stuff, that might be the best thing here.
,
Oct 26 2017
This might be the same root cause as issue 776969 .
,
Nov 6 2017
Here is a reduction of the overlap map bug that does not depend on root layer scrolling or position: fixed. https://output.jsbin.com/putulim/quiet (On high DPI, pass --disable-prefer-compositing-to-lcd-text.) I think a good fix would address this case as well, which probably means not special-casing position: fixed.
,
Nov 7 2017
,
Nov 7 2017
Thanks Steve, you're right (as usual), my analysis in the review was wrong. So a stacking context inside a non-composited scroller A, nested in a composited scroller B, won't correctly overlap test a sibling that's in B. One of the assumptions we make in the overlap map is wrong but I'll need some time to study it more carefully and TPAC is quite the distraction. I'll take a deeper look by Thursday.
,
Nov 13 2017
Reducing scope of title. General (non-RLS) bug now tracked by issue 783532. Per offline discussion we should try to unblock RLS by preserving existing promotion behavior if possible - for example, by making CompositingRequirementsUpdater::RecursionData::has_composited_scrolling_ancestor_ ignore the root layer. But this alone may not be sufficient due to RLS changing the behavior of PaintLayerClipper.
,
Nov 17 2017
Adding notes from chat logs and such, for posterity. Last week bokan reported that ignoring root layer for the purpose of setting has_composited_scrolling_ancestor_ doesn't fix offscreen overlap scenarios. Our theory was that PaintLayerClipper applies viewport clip with RLS, so even if CompositingRequirementsUpdater matches pre-RLS behavior for picking between clipped vs. unclipped, it's getting different "clipped" rects from PaintLayerClipper. During weekly sync meeting chrishtr mentioned PaintLayerClipper has options to control this stuff, which I had forgotten about. Specifically the caller specifies a ClipRectsCacheSlot corresponding to a set of behaviors. I then found that overlap testing uses PaintLayer::AncestorDependentCompositingInputs::clipped_absolute_bounding_box which is based on kAbsoluteClipRects, which is not used by anything else, and suggested that we change kAbsoluteClipRects to ignore the viewport clip, similar to how kRootRelativeClipRectsIgnoringViewportClip ignores the viewport clip. That should let us use the pre-RLS rects for the overlap test.
,
Nov 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/51eeec6a3fcdb833e81fa8cc73165613a997009d commit 51eeec6a3fcdb833e81fa8cc73165613a997009d Author: David Bokan <bokan@chromium.org> Date: Mon Nov 20 16:09:55 2017 [root-layer-scrolling] Fix compositing overlap test When RLS is turned on, children of the root layer now have a composited scrolling ancestor. This causes them to use their unclipped bounding rects to test overlap. It turns out overlap testing inside composited scrollers is somewhat buggy (https://crbug.com/783532). In order to prevent regressing existing behavior, this CL makes an exception for children whose composited scrolling ancestor is the root layer. In that that case, we use the clipped rects. This is necessary so that overlap testing is performed against other clipped rects (see comments in OverlapMap::Add for how clipped vs unclipped comparisons work). However, since the root layer is now a scrolling layer, this would break overlap testing for boxes that are offscreen. This CL additionally makes overlap test-related clipping ignore the root layer so that the clipped_absolute_bounding_box for the root layer is actually unclipped, as pre-RLS. The newly added test covers this case. Bug: 783532 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_root_layer_scrolls;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I5e302f3f29775d829b94593e8053025c5da0f980 Reviewed-on: https://chromium-review.googlesource.com/742506 Reviewed-by: Steve Kobes <skobes@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#517835} [modify] https://crrev.com/51eeec6a3fcdb833e81fa8cc73165613a997009d/third_party/WebKit/LayoutTests/FlagExpectations/root-layer-scrolls [add] https://crrev.com/51eeec6a3fcdb833e81fa8cc73165613a997009d/third_party/WebKit/LayoutTests/compositing/overflow/surface-over-composited-offscreen-expected.html [add] https://crrev.com/51eeec6a3fcdb833e81fa8cc73165613a997009d/third_party/WebKit/LayoutTests/compositing/overflow/surface-over-composited-offscreen.html [modify] https://crrev.com/51eeec6a3fcdb833e81fa8cc73165613a997009d/third_party/WebKit/Source/core/paint/ClipRectsCache.h [modify] https://crrev.com/51eeec6a3fcdb833e81fa8cc73165613a997009d/third_party/WebKit/Source/core/paint/PaintLayer.cpp [modify] https://crrev.com/51eeec6a3fcdb833e81fa8cc73165613a997009d/third_party/WebKit/Source/core/paint/PaintLayerClipper.h [modify] https://crrev.com/51eeec6a3fcdb833e81fa8cc73165613a997009d/third_party/WebKit/Source/core/paint/compositing/CompositingInputsUpdater.cpp [modify] https://crrev.com/51eeec6a3fcdb833e81fa8cc73165613a997009d/third_party/WebKit/Source/core/paint/compositing/CompositingRequirementsUpdater.cpp
,
Nov 20 2017
|
||||
►
Sign in to add a comment |
||||
Comment 1 by bokan@chromium.org
, Oct 24 2017