New issue
Advanced search Search tips

Issue 782991 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 590334



Sign in to add a comment

scrollers promoted for indirect reasons don't get composited scrolling

Project Member Reported by skobes@chromium.org, Nov 9 2017

Issue description

In general, low-DPI transparent-background overflow scrollers are not composited, to preserve LCD text.

However, if the scroller has will-change: transform or similar, it gets a CompositedLayerMapping and creates composited scrolling layers (CLM::scroll_layer_, CLM::scrolling_contents_layer_).

If the scroller does not have will-change: transform, but gets layer-promoted for some other reason such as overlap or composited descendants, then it ends up in a strange hybrid state where it has a CompositedLayerMapping without the scrolling layers.

Not having scrolling layers forces main-thread scrolling when it could otherwise be done on the compositor thread.

Repro: https://output.jsbin.com/neqaxax/quiet
Click Jank, and try to scroll.  (Use --disable-prefer-compositing-to-lcd-text to repro on high DPI.)

There's no good reason not to create scrolling layers here - subpixel antialiasing is already broken due to the layer promotion.

Fixing this requires untangling some circular dependencies in CompositingRequirementsUpdater::UpdateRecursive, which updates PLSA::needs_composited_scrolling_ before computing subtree_compositing_reasons, and tests overlap differently depending on the result (via has_composited_scrolling_ancestor).

There's also an edge case to watch out for where negative z-index children require paint invalidations on scroll (crrev.com/402612).  We use PaintLayer::IsAllScrollingContentComposited to decide whether to issue this invalidation, but that bit gets updated after CompositingRequirementsUpdater is done doing its stuff.  It's also arguably too conservative - any non-stacking-context composited child kicks you off the fast path but if they don't have negative z-index they're probably fine.

(Also we should continue to not use composited scrolling in situations when PLSA::ComputeNeedsCompositedScrolling returns false despite layer_has_been_composited == true, such as kHasBorderRadius and kHasClipRelatedProperty.)

A quick indicator that you're in this situation is if the scrollbar layers have orange borders with --show-composited-layer-borders.  This says you have a CLM scrollbar layer that isn't backed by a WebScrollbarLayer.  A "good" scrollbar layer has a green border instead of an orange border.
 
Screenshot from 2017-11-08 16:56:20.png
15.4 KB View Download
Note this is essentially a generalization of http://crrev.com/573963006 to include indirect reasons as well as direct ones.

I discovered it during investigation of  issue 778469 , but I think we can do something special for iframes there.
Blocking: 590334
Labels: -Pri-3 Pri-2

Comment 4 by skobes@chromium.org, Nov 16 2017

An interesting variation on this bug appears in the second iframe in compositing/iframes/overlapped-iframe-iframe.html (with id="overlap"), on low DPI (or --disable-prefer-compositing-to-lcd-text).

The <iframe> element is composited by overlap testing, and has user-scrollable overflow, but it doesn't get composited scrolling, even with RLS after r515995, because its PaintLayerCompositor doesn't enter compositing mode (PLC::compositing_), so its LayoutView doesn't even get a CLM, even though the owning LayoutIFrame has one.
Re indirect compositing: yeah, that's what I expected. Is it causing a specific
problem? Happy to dig in and help.

Comment 6 by skobes@chromium.org, Nov 17 2017

This bug isn't blocking anything, it's just a potential perf win.  I'm sort of using it as a place to document findings from related bugs like  issue 778469  and  issue 784053 . :)
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 11 2018

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

commit 38825770276009fd291931b3a8027102dd80a558
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Mon Jun 11 04:33:52 2018

[PE] Rationalize compositing triggers.

* Compute all triggers except kComboSquashableReasons, kComboCompositedDescendants
and kComboAllCompositedScrollingDeterminedReasons in either style update or
CompositingInputsUpdater.
* Determine composited scrolling (but not the compositing trigger bit) in CompositingInputsUpdater.
* Cache these results on PaintLayer.

This achieves the following goals:
- Separates composited scrolling and awkward LCD text decisions from
the CompositingRequirementsUpdater step.
- Caches more triggers, leading to faster compositing updates in some cases
- Unblocks CL 1072155

Summary of implementation approach:

1. Dirty compositing inputs for all triggers computed there.
2. Compute, and cache on PaintLayer, kComboAllDirectNonStyleDeterminedReasons during
CompositingInputsUpdater.
3. Compute and cache a new DescendantHasDirectCompositingReason dirty bit on PaintLayer.
4. Update composited scrolling in PaintLayerScrollableArea during CompositingInputsUpdater.
This includes the root layer, which uses DescendantHasDirectCompositingReason as one of its
triggers.

Bug:814439,782991

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I21095762cbf806008395c2cf79f4ee038c39cb5f
Reviewed-on: https://chromium-review.googlesource.com/1087791
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565925}
[modify] https://crrev.com/38825770276009fd291931b3a8027102dd80a558/third_party/blink/renderer/core/animation/scroll_timeline.cc
[modify] https://crrev.com/38825770276009fd291931b3a8027102dd80a558/third_party/blink/renderer/core/html/html_frame_owner_element.cc
[modify] https://crrev.com/38825770276009fd291931b3a8027102dd80a558/third_party/blink/renderer/core/page/scrolling/root_scroller_controller.cc
[modify] https://crrev.com/38825770276009fd291931b3a8027102dd80a558/third_party/blink/renderer/core/paint/README.md
[modify] https://crrev.com/38825770276009fd291931b3a8027102dd80a558/third_party/blink/renderer/core/paint/compositing/compositing_inputs_updater.cc
[modify] https://crrev.com/38825770276009fd291931b3a8027102dd80a558/third_party/blink/renderer/core/paint/compositing/compositing_inputs_updater.h
[modify] https://crrev.com/38825770276009fd291931b3a8027102dd80a558/third_party/blink/renderer/core/paint/compositing/compositing_reason_finder.cc
[modify] https://crrev.com/38825770276009fd291931b3a8027102dd80a558/third_party/blink/renderer/core/paint/compositing/compositing_reason_finder.h
[modify] https://crrev.com/38825770276009fd291931b3a8027102dd80a558/third_party/blink/renderer/core/paint/compositing/compositing_reason_finder_test.cc
[modify] https://crrev.com/38825770276009fd291931b3a8027102dd80a558/third_party/blink/renderer/core/paint/compositing/compositing_requirements_updater.cc
[modify] https://crrev.com/38825770276009fd291931b3a8027102dd80a558/third_party/blink/renderer/core/paint/compositing/compositing_requirements_updater.h
[modify] https://crrev.com/38825770276009fd291931b3a8027102dd80a558/third_party/blink/renderer/core/paint/compositing/paint_layer_compositor.cc
[modify] https://crrev.com/38825770276009fd291931b3a8027102dd80a558/third_party/blink/renderer/core/paint/paint_layer.cc
[modify] https://crrev.com/38825770276009fd291931b3a8027102dd80a558/third_party/blink/renderer/core/paint/paint_layer.h
[modify] https://crrev.com/38825770276009fd291931b3a8027102dd80a558/third_party/blink/renderer/platform/graphics/compositing_reasons.h

Sign in to add a comment