Implement CAP compositing reasons for root scroller |
||||
Issue descriptionWe need to implement the compositing reasons for scroll offset transforms in SPV2, in PaintPropertyTreeBuilder. This will largely mirror the SPV1 PaintLayerScrollableArea::computeNeedsCompositedScrolling code.
,
Mar 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fb079780798a45f84b3fb9c0f1ba02e8fb800f3d commit fb079780798a45f84b3fb9c0f1ba02e8fb800f3d Author: Philip Rogers <pdr@chromium.org> Date: Wed Mar 21 02:41:51 2018 [SPV2] Set compositing reasons for the root scroller This patch extracts out the existing root scroll compositing logic into CompositingReasonFinder::RequiresCompositingForRootScroller and uses it for SPV2. This fixes a bug where the document was not scrollable with the mousewheel when --enable-slimming-paint-v2 was used. This layout test now passes: background-paint-into-scrolling-contents-layer-with-root-layer-scrolls-offset.html Although there are spv2 differences, these look to be newly passing: paint/invalidation/scroll/outline-change-scrollable.html paint/invalidation/outline/outline-change-vertical-rl.html This layout test now shows a failure: paint/invalidation/reflection/scroll-absolute-layer-with-reflection.html These tests had minor pixel differences: fast/css/sticky/sticky-both-sides-top-left-constrained.html fast/dom/scroll-reveal-top-overflow.html fast/css/sticky/sticky-overflowing.html fast/overflow/overflow_hidden.html fast/css/sticky/sticky-vertically-overconstrained.html fast/dom/scroll-reveal-left-overflow.html fast/writing-mode/border-radius-clipping-vertical-lr.html Bug: 693741 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I467008ac569ce1dd998caba7689c50b91d3687e4 Reviewed-on: https://chromium-review.googlesource.com/968983 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#544624} [modify] https://crrev.com/fb079780798a45f84b3fb9c0f1ba02e8fb800f3d/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2 [add] https://crrev.com/fb079780798a45f84b3fb9c0f1ba02e8fb800f3d/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/fast/dom/scroll-reveal-left-overflow-expected.png [add] https://crrev.com/fb079780798a45f84b3fb9c0f1ba02e8fb800f3d/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/fast/dom/scroll-reveal-top-overflow-expected.png [add] https://crrev.com/fb079780798a45f84b3fb9c0f1ba02e8fb800f3d/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/fast/overflow/overflow_hidden-expected.png [add] https://crrev.com/fb079780798a45f84b3fb9c0f1ba02e8fb800f3d/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/fast/overflow/overflow_hidden-expected.txt [add] https://crrev.com/fb079780798a45f84b3fb9c0f1ba02e8fb800f3d/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/fast/writing-mode/border-radius-clipping-vertical-lr-expected.png [modify] https://crrev.com/fb079780798a45f84b3fb9c0f1ba02e8fb800f3d/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/paint/invalidation/outline/outline-change-vertical-rl-expected.txt [modify] https://crrev.com/fb079780798a45f84b3fb9c0f1ba02e8fb800f3d/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/paint/invalidation/scroll/outline-change-scrollable-expected.txt [modify] https://crrev.com/fb079780798a45f84b3fb9c0f1ba02e8fb800f3d/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp [modify] https://crrev.com/fb079780798a45f84b3fb9c0f1ba02e8fb800f3d/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp [modify] https://crrev.com/fb079780798a45f84b3fb9c0f1ba02e8fb800f3d/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp [modify] https://crrev.com/fb079780798a45f84b3fb9c0f1ba02e8fb800f3d/third_party/WebKit/Source/core/paint/compositing/CompositingReasonFinder.cpp [modify] https://crrev.com/fb079780798a45f84b3fb9c0f1ba02e8fb800f3d/third_party/WebKit/Source/core/paint/compositing/CompositingReasonFinder.h
,
Aug 15
In https://chromium-review.googlesource.com/c/chromium/src/+/1156002 we discovered two SPV2 scroll-related deficiencies: 1) The scroll hit test layer will prevent the scrolling contents from squashing into the background. 2) There is no logic for drawing solid-color backgrounds into the scrolling contents (aka with the scrolling contents properties). Because this does not exist, the background and foreground are in different layers which results in subpixel differences. These need to be addressed before we can add compositing reasons for the LayoutView.
,
Nov 30
I believe 2) in #c3 has been fixed. What's the plan for scroll hit test layer?
,
Nov 30
Since that comment was written, we have launched PaintTouchActionRects which paints hit test rects with the correct property tree states. I recommend we move the implementation of BlockPainter::PaintScrollHitTestDisplayItem into BoxPainter::RecordHitTestData, and make CompositeAfterPaint depend on PaintTouchActionRects.
,
Jan 16
,
Jan 16
Can LayoutNG launch before this bug is fixed? We have a failing webkit_unit_test: All/FrameThrottlingTest.ThrottleSubtreeAtomically/2 According to @pdr, root cause of failure is this bug. We'd like to disable the test for NG if it is not a launch blocker.
,
Jan 16
@atotic, I wonder if this was actually fixed. I just landed a fix to the same crash you were hitting. Can you see if the crash still occurs?
Originally I thought the NG failure was due to this TODO:
static bool NeedsScrollNode(const LayoutObject& object) {
...
// TODO(pdr): CAP has invalidation issues ( crbug.com/732611 ) as well as
// subpixel issues (crbug.com/693741) which prevent us from compositing the
// root scroller.
if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
return HasScrollsOverflow(box);
return HasScrollsOverflow(box) || IsRootScroller(box);
}
But then I found a fundamental issue with the scroll tree which has been fixed as of https://crrev.com/622886.
,
Jan 16
All/FrameThrottlingTest.ThrottleSubtreeAtomically/2 is still failing.
,
Jan 16
Ah ok :/ To answer your question: LayoutNG can launch without fixing this bug. CompositeAfterPaint is quite a ways out.
,
Yesterday
(29 hours ago)
,
Yesterday
(29 hours ago)
|
||||
►
Sign in to add a comment |
||||
Comment 1 by pdr@chromium.org
, Mar 16 2018