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

Issue 744665 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Compat



Sign in to add a comment

Seemingly unrelated scrolling div is re-painted and composited

Reported by dennis.f...@gmail.com, Jul 17 2017

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36

Example URL:
https://stackoverflow.com/questions/45137147/why-is-the-seemingly-unrelated-div-re-painted-in-chrome

Steps to reproduce the problem:
1. Inspect the code in the following jsfiddle:
https://jsfiddle.net/q5owta80/

2. Start scrolling the left div
3. Notice that the right div is being re-painted on each scroll, although it is a different layer.

What is the expected behavior?
The scrolling container in the unrelated div should not be repainted.

What went wrong?
The div in another layer gets re-painted and composited. In the large document with many such scrolling containers this results in UpdateLayersTree + Composition to be the bottleneck of scrolling anywhere on the page.

Does it occur on multiple sites: N/A

Is it a problem with a plugin? No 

Did this work before? N/A 

Does this work in other browsers? N/A

Chrome version: 59.0.3071.115  Channel: stable
OS Version: OS X 10.10.5
Flash Version:
 
Cc: brajkumar@chromium.org
Components: Blink
Labels: Needs-Feedback
Unable to reproduce this issue on Mac OS 10.12.5 using chrome latest stable #59.0.3071.115 by following steps mentioned in the original comment. Observed no re-painting in the right div while scrolling the left div.

dennis.frostlander@ Are you able to reproduce this issue consistently including incognito mode? Please provide screen-shot or screen-cast of actual behavior for better understanding. 
Components: -Blink Blink>Scroll
@brajkumar Yes, consistently on a Mac. In incognito window on the reported version, in the latest Canary version - everywhere is the same behavior.
Please see the attached gif with the recording of the screen.
The .html file is also attached.
chrome-scrolling.gif
3.1 MB View Download
test.html
999 bytes View Download
Project Member

Comment 4 by sheriffbot@chromium.org, Jul 19 2017

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding requester "brajkumar@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: pbomm...@chromium.org
Labels: TE-NeedsTriageFromMTV
Passing this issue to MTV team as we don't have 10.10.5 Mac OS with us.

Thanks!!
Same behaviour on MacOS Sierra 10.12.6, Chrome 59.0.3071.115 (Official Build) (64-bit)

Comment 7 by bokan@chromium.org, Jul 27 2017

Cc: bokan@chromium.org
Labels: Needs-Feedback
Quick question: is this on a retina (high density) screen or a regular monitor? Could you try this out in Canary or Dev channel and see if it's still reproducing?

Thanks.
Yes, this is retina screens. MacBooks Pro, around 4-5 years old. Still reproduces on Chrome Canary Version 62.0.3169.0 (Official Build) canary (64-bit) on MacOS 10.10.5 and 10.12.6.

Thanks!
Project Member

Comment 9 by sheriffbot@chromium.org, Jul 28 2017

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding requester "bokan@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
By the way, the composition reason for the layers is listed as "Composition due to association with an element with a overflow-scrolling: touch".

Could you please let me know what it means? I tried setting all types of overflow-scrolling and -webkit-overflow-scrolling on all elements, but this has no effect.

Comment 11 by bokan@chromium.org, Jul 31 2017

Cc: flackr@chromium.org
I think the composition reason label here is outdated and misleading. +flackr@, I think we should change it. Looking at the code[1], we set this reason any time a scroller needs compositing. When on a high-dpi monitor, we generally composite scrollers since it means we generally don't have to repaint or go to the main thread at all when scrolling it. On low DPI monitors we usually avoid this as it causes a scroller to lose LCT-text antialiasing which isn't needed on high DPI. 

The question remains why we're repainting the unrelated, composited scroller. I'll take a closer look when I get back to my high-dpi MBP tonight.

[1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp?type=cs&sq=package:chromium&l=152
Could you try this out on the latest stable channel? M60 shipped recently and I wasn't able to reproduce on a MBP w/retina, running Chrome 60.0.3112.78 or later. 
Hmm, I still have a repro in 60.0.3112.78 (Official Build) (64-bit).
So just to double check. A scrolling div creates two layers:
1. The upper layer with a reason "due to association with an element with a overflow-scrolling: touch"
2. The lower layer with a reason layerForScrollingContents.

In my gif, the first layer gets repainted. This is not what you see and is not correct, right?
Ah, sorry, I missed your gif abouve. I was using DevTools' paint flashing feature and it wasn't flashing. I'll have to take another look from a Mac.
Cc: pdr@chromium.org
Components: -Blink>Scroll Blink>Paint>Invalidation
Status: Untriaged (was: Unconfirmed)
I tried this with latest canary, I don't see any "paint flashing" when I use "devtools> rendering > paint flashing" beside the scrollbar. But I do see the paint events recorded in performance profile and they also show up in trace under LocalFrameView::paintTree as well. (Attached trace)

The two paint clips corresponds to two scrolling viewports (0,0)x[100, 50] and (300,0)x[100, 50]. I am not sure why we are repainting these and why they are not showing as re-paint flash :(.
A paint expert should look at these.


CPU Duration: 0.016 ms
data	
{frame: "0x2159b7d21df8",
 clip: [300,
        0,
        400,
        0,
        400,
        50,
        300,
        50],
 nodeId: 4,
 layerId: 20}


CPU Duration: 0.009 ms, data	
{frame: "0x2159b7d21df8",
 clip: [0,
        0,
        100,
        0,
        100,
        50,
        0,
        50],
 nodeId: 5,
 layerId: 21}



trace_paint-of-unrelated-layer.json.gz
1.8 MB Download
Labels: -TE-NeedsTriageFromMTV PaintTeamTriaged-20170822 BugSource-User
Owner: wangxianzhu@chromium.org
Status: Assigned (was: Untriaged)
Assigned to get an opinion on whether this is actually painting work or just work done to discover that we do not need to paint.
Labels: -Pri-2 Pri-3
We don't need to worry about these "paints" because they actually do almost nothing. 

However, we should avoid them at all. They show up because a small problem in the following (equivalent) code in GraphicsLayer::PaintWithoutCommit():
  if (NeedsRepaint() || GetPaintController().CacheIsEmpty())
    Paint();

When the above code is executed for the scrolling container layers of the scrollers, NeedsRepaint() is false, but GetPaintController().CacheIsEmpty() is true because the previous paint painted nothing. Then we call Paint() which will still paint nothing.

Will make the condition more accurate to avoid unnecessary (though trivial) Paint calls.
Project Member

Comment 18 by bugdroid1@chromium.org, Oct 14 2017

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

commit d6c6e08aad8b026aa0c4b5cf4219e52e3bf1d2ab
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Sat Oct 14 01:01:47 2017

PaintController: Distinguish "all invalid" and "empty but valid"

Previously GraphicsLayer checked PaintController::CacheIsEmpty() (and
other conditions) to determine whether to repaint. However, sometimes
the cache is empty but is valid when the previous paint was empty, but
we painted unnecessarily.

Distinguish "empty but valid" from "all invalid".

Bug:  744665 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I5080b83d4643b66061b6acfd8f805748c795620c
Reviewed-on: https://chromium-review.googlesource.com/716928
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508896}
[modify] https://crrev.com/d6c6e08aad8b026aa0c4b5cf4219e52e3bf1d2ab/third_party/WebKit/Source/core/page/PageOverlay.cpp
[modify] https://crrev.com/d6c6e08aad8b026aa0c4b5cf4219e52e3bf1d2ab/third_party/WebKit/Source/core/paint/PaintControllerPaintTest.h
[modify] https://crrev.com/d6c6e08aad8b026aa0c4b5cf4219e52e3bf1d2ab/third_party/WebKit/Source/core/paint/PaintInvalidationCapableScrollableArea.cpp
[modify] https://crrev.com/d6c6e08aad8b026aa0c4b5cf4219e52e3bf1d2ab/third_party/WebKit/Source/core/paint/PaintLayerPainterTest.cpp
[modify] https://crrev.com/d6c6e08aad8b026aa0c4b5cf4219e52e3bf1d2ab/third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp
[modify] https://crrev.com/d6c6e08aad8b026aa0c4b5cf4219e52e3bf1d2ab/third_party/WebKit/Source/platform/graphics/GraphicsLayer.h
[modify] https://crrev.com/d6c6e08aad8b026aa0c4b5cf4219e52e3bf1d2ab/third_party/WebKit/Source/platform/graphics/GraphicsLayerTest.cpp
[modify] https://crrev.com/d6c6e08aad8b026aa0c4b5cf4219e52e3bf1d2ab/third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
[modify] https://crrev.com/d6c6e08aad8b026aa0c4b5cf4219e52e3bf1d2ab/third_party/WebKit/Source/platform/graphics/paint/PaintController.h
[modify] https://crrev.com/d6c6e08aad8b026aa0c4b5cf4219e52e3bf1d2ab/third_party/WebKit/Source/platform/graphics/paint/PaintControllerTest.cpp
[modify] https://crrev.com/d6c6e08aad8b026aa0c4b5cf4219e52e3bf1d2ab/third_party/WebKit/Source/platform/testing/FakeGraphicsLayerClient.h

Status: Fixed (was: Assigned)

Sign in to add a comment