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

Issue 642370 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 616995



Sign in to add a comment

Scroll always happens on main thread on low dpi

Project Member Reported by ymalik@chromium.org, Aug 30 2016

Issue description

Version: 55.0.2844

What steps will reproduce the problem?
(1) Go to https://dl.dropboxusercontent.com/u/103707690/nested_scroll.html
(2) Scroll the main frame scroller
(3) Wait for some time and go to chrome://histograms and observe the "MainThreadWheelScrollReason".

 

Comment 1 by ymalik@chromium.org, Aug 30 2016

Cc: skobes@chromium.org
Cc: flackr@chromium.org vollick@chromium.org

Comment 3 by flackr@chromium.org, Aug 30 2016

Status: WontFix (was: Untriaged)
This is expected. We don't automatically promote to composited scrolling when doing so would prevent us from having subpixel anti-aliasing. The reason it would prevent that in this case is that we are drawing the scrolling contents onto a transparent layer so we don't know what color it will eventually be in front of (in the general case). Because of this we disable sub-pixel anti-aliasing when the scrolling contents layer is promoted which degrades the text quality.

Soon, however, if you apply a "background: white" style to the overflow element we will automatically promote while maintaining subpixel text anti-aliasing, see  issue 381840 .

Comment 4 by flackr@chromium.org, Aug 30 2016

I should mention, you can see the non-subpixel anti-aliasing today if you add "will-change: transform" to the scroller to force it to be promoted.
Yash, you said you saw this page scroll on the compositor in a previous version of Chrome, didn't you?

Comment 6 by ymalik@chromium.org, Aug 30 2016

Yes that's correct. It was scrolling on the compositor until last week. Is this a recent change?

Comment 7 by flackr@chromium.org, Aug 30 2016

That's strange, the test page doesn't scroll on the compositor for me on low-dpi on	53.0.2785.70 (Official Build) beta (64-bit). Have you tested on stable or beta?

Comment 8 by flackr@chromium.org, Aug 30 2016

Status: Available (was: WontFix)
My mistake, we're talking about the main scroller, reopening.

Comment 9 by skobes@chromium.org, Aug 30 2016

Owner: sahel@chromium.org
Status: Assigned (was: Available)
I think http://crrev.com/414515 broke this by causing LayerTreeHostImpl::ScrollBegin to return SCROLL_IGNORED.

@sahel, can you confirm?
Blocking: 616995
Verified that reverting http://crrev.com/414515 fixes this.


Comment 11 Deleted

Comment 12 by sahel@chromium.org, Aug 30 2016

The scroll animation logic has still dependency on clearing the currentlyscrollinglayer after each animation. Previously this was done by calling scrollEnd at the end of scroll animation functions.

In my patch I don't clear the currently scrolling layer in scrollEnd to be able to latch a fling to its related scroll.

For now, I explicitly cleared the currently scrolling layer after the scrollEnd. (similar to what I've done in http://crrev.com/414515 for scrollAnimated function.)

The patch is under review now.
Project Member

Comment 13 by bugdroid1@chromium.org, Sep 1 2016

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

commit ebcfc3309fba892b25d84f943d26bbdab6d100ee
Author: sahel <sahel@chromium.org>
Date: Thu Sep 01 00:13:37 2016

currentlyscrollinglayer cleared in scrollAnimatedBegin.

For now the latching is implemented only for mac, and the
scrollAnimation logic is dependent on clearing the scrolling layer after each animation.
The dependency will resolved in follow up patches that will address latching on other devices.

Similar changes is done for scrollAnimated function in:
https://codereview.chromium.org/2256733003

BUG= 642370 
TEST=LayerTreeHostImplTest.SecondScrollAnimatedBeginNotIgnored

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

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

[modify] https://crrev.com/ebcfc3309fba892b25d84f943d26bbdab6d100ee/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/ebcfc3309fba892b25d84f943d26bbdab6d100ee/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/ebcfc3309fba892b25d84f943d26bbdab6d100ee/ui/events/blink/input_handler_proxy.cc

Status: Fixed (was: Started)

Sign in to add a comment