New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 660156 link

Starred by 1 user
Status: Fixed
Owner:
wangxianzhu@chromium.org
Closed: Nov 2016
Cc:
█ trchen@chromium.org
szager@chromium.org
skobes@chromium.org
chrishtr@chromium.org
vmp...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Support incremental invalidation for boxes with local attached background when layout overflow changes

Project Member Reported by wangxianzhu@chromium.org, Oct 27 2016 
We need to invalidate a box local attached background when its layout overflow changes because the background is painted behind the whole scrolling contents. For now this paint invalidation is a full paint invalidation of the box.

With rootLayerScrolling enabled, LayoutView will become such a box. Appending contents at the end of the document will cause layout overflow change and full paint invalidation. Without rootLayerScrolling, paint invalidation works because LayoutView's paint invalidation rect is the layout overflow rect, so layout overflow change will cause paint invalidation rect change and we'll do an incremental invalidation of the changed part.

We may need to support incremental invalidation for the case because appending contents at the end of a document is a common case.

Comment 1 by wangxianzhu@chromium.org, Oct 28 2016

Cc: skobes@chromium.org szager@chromium.org
CC szager@ and skobes@: you can just rebaseline layout tests that fail with RLS because of this bug. They will look like:

         {
           "object": "LayoutView #document",
+          "rect": [0, 0, 1500, 585],
+          "reason": "layout overflow box change"
-          "rect": [1250, 0, 250, 585],
-          "reason": "incremental"
         },
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 18 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e

commit a2907993ffbc7f8e21b2dce12c2ff5d541afca7e
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Fri Nov 18 04:16:06 2016

Paint invalidation of local attachment backgrounds

Previously if a local attachment background was painted on the scrolling
contents layer, we called invalidatePaintUsingContainer() with the visual
rect of the object, and in setBackingNeedsPaintInvalidationInRect()
we invalidated the rect on the main layer, and invalidated the whole
scrolling contents layer.

This CL specially handles background invalidation on scrolling contents
layer with LayoutObject::backgroundChangedSinceLastPaintInvalidation()
and a new paint invalidation reason PaintInvalidationBackgroundOnScrollingContentsLayer.
Also supports incremental invalidation of backgrounds on scrolling
contents layer caused by layout overflow change, if possible.

We need this change because now more backgrounds are/will be treated as local
attachment [equivalent] and previous handling of paint invalidation of
them caused too many unnecessary full invalidation on both the container
layer and the scrolling contents layer.

We (will) have more background treated as local attachment because of
the following reasons:
- With rootLayerScrolling, most LayoutViews (except those with fixed background)
  are treated as normal scrolling object with local attachment background.
  Always full invalidation of either container or contents layer is not
  acceptable at least on Android on which hiding/showing the top control
  (which causes resize of the LayoutView) is a key scenario of performance
  and power consumption;
- With composited scrolling with opaque background regardless of LCD text,
  we treat normal solid color backgrounds as local attachment backgrounds.
  Full paint invalidation of scrolling contents layer of them is a big
  regression.

BUG= 660156 , 568847 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2498823002
Cr-Commit-Position: refs/heads/master@{#433093}

[modify] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/LayoutTests/paint/invalidation/composited-overflow-with-local-background-expected.txt
[add] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/LayoutTests/paint/invalidation/compositing/background-attachment-local-composited-expected.html
[add] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/LayoutTests/paint/invalidation/compositing/background-attachment-local-composited-expected.txt
[add] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/LayoutTests/paint/invalidation/compositing/background-attachment-local-composited.html
[add] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/LayoutTests/paint/invalidation/compositing/background-attachment-local-equivalent-expected.html
[add] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/LayoutTests/paint/invalidation/compositing/background-attachment-local-equivalent-expected.txt
[add] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/LayoutTests/paint/invalidation/compositing/background-attachment-local-equivalent.html
[add] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/LayoutTests/paint/invalidation/compositing/should-not-repaint-scrolling-contents-outline-change-expected.html
[add] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/LayoutTests/paint/invalidation/compositing/should-not-repaint-scrolling-contents-outline-change-expected.txt
[add] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/LayoutTests/paint/invalidation/compositing/should-not-repaint-scrolling-contents-outline-change.html
[modify] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/LayoutTests/paint/invalidation/overflow-scroll-local-background-text-color-change-expected.txt
[modify] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/LayoutTests/paint/invalidation/repaint-composited-child-in-scrolled-container.html
[modify] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/overflow-scroll-local-background-text-color-change-expected.txt
[modify] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
[modify] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/Source/core/paint/BoxPaintInvalidator.cpp
[modify] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/Source/core/paint/BoxPaintInvalidator.h
[modify] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/Source/core/paint/BoxPaintInvalidatorTest.cpp
[modify] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
[modify] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.h
[modify] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/Source/platform/graphics/PaintInvalidationReason.cpp
[modify] https://crrev.com/a2907993ffbc7f8e21b2dce12c2ff5d541afca7e/third_party/WebKit/Source/platform/graphics/PaintInvalidationReason.h

Comment 3 by wangxianzhu@chromium.org, Nov 18 2016

Status: Fixed (was: Assigned)
For RLS mode we still force full paint invalidation (on the container layer) for LayoutViews. See https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/FrameView.cpp?q=FrameView::viewportSizeChange&sq=package:chromium&l=1522. This will be tracked by the TODO and  bug 568847 .

Note that the TODO is quite important for Android top control showing/hiding performance.
►

Sign in to add a comment