New issue
Advanced search Search tips

Issue 667946 link

Starred by 3 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug


Sign in to add a comment

Scrolling is too broken with --enable-slimming-paint-v2

Project Member Reported by pdr@chromium.org, Nov 22 2016

Issue description

Broken scrolling is one reason perf test comparisons between spv2 and spv1 are not correct. It's also not really possible to use the browser with spv2.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 29 2016

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

commit 9a8e6df44aadd67e582db1200cd25ccd7ecf8d43
Author: pdr <pdr@chromium.org>
Date: Tue Nov 29 00:13:03 2016

Ensure paint properties are rebuilt on window resizes

This patch ensures the FrameView paint properties are rebuilt when the window
resizes. This fixes a crash if you resized wikipedia.

BUG=667946
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[modify] https://crrev.com/9a8e6df44aadd67e582db1200cd25ccd7ecf8d43/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/9a8e6df44aadd67e582db1200cd25ccd7ecf8d43/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 23 2017

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

commit de4408730000072669b7f4d27d0e5e8066988724
Author: pdr <pdr@chromium.org>
Date: Mon Jan 23 17:37:55 2017

Use blink's scroll offset to update cc's transform scroll offset for SPV2

Blink and cc use slightly different transform nodes for scroll offset.
Blink creates a transform node just for scroll offset whereas cc has
a special field (scroll_offset) embedded in the transform node. To
account for this, PropertyTreeManager::updateScrollOffset adjusted the
cc transform node.

updateScrollOffset had a bug where the compositor node's scroll offset would
be set from the compositor node's transform, then the transform would
be zero'd. This was broken when updateScrollOffset was called a second
time because the newly-zero'd transform would be used to overwrite the
scroll offset.

Fixing this bug exposed a second bug where setting both the transform
node's scroll_offset and calling scrollTree().SetScrollOffset(...) would
lead to missed compositor-side scroll offset updates (this is tested in
fast/scrolling/fixed-position.html). The SetScrollOffset call has been
temporarily replaced by a TODO for a followup patch.

With this patch google.com is usable with SPV2 \o/

BUG=667946

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[modify] https://crrev.com/de4408730000072669b7f4d27d0e5e8066988724/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
[modify] https://crrev.com/de4408730000072669b7f4d27d0e5e8066988724/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositorTest.cpp
[modify] https://crrev.com/de4408730000072669b7f4d27d0e5e8066988724/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp
[modify] https://crrev.com/de4408730000072669b7f4d27d0e5e8066988724/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.h

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 8 2017

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

commit 27a8cfde871ad4786943a345569f36c9a5ba6e29
Author: pdr <pdr@chromium.org>
Date: Wed Feb 08 20:12:21 2017

Remove GraphicsLayer::didScroll and directly call ScrollableArea::didScroll

This patch removes GraphicsLayer::didScroll, replacing this cc->blink
scroll callback with a direct call to Scrollablearea::didScroll. The
old function on GraphicsLayer was only used to lookup the scroll
offset but we can pass that along in the didScroll callback.

This patch will be useful for SPV2 which does not use GraphicsLayer and
can benefit from avoiding it, but this patch is intended to be useful
for SPV1. This code has been split off of a larger patch for composited
scrolling: https://codereview.chromium.org/2676923002.

BUG=667946
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/cc/blink/web_layer_impl.cc
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/cc/blink/web_layer_impl.h
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/cc/layers/layer.cc
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/cc/layers/layer.h
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/cc/trees/layer_tree_host_unittest_scroll.cc
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/third_party/WebKit/Source/platform/graphics/GraphicsLayer.h
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/third_party/WebKit/Source/platform/graphics/GraphicsLayerTest.cpp
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/third_party/WebKit/Source/platform/scroll/ScrollableArea.cpp
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/third_party/WebKit/Source/platform/scroll/ScrollableArea.h
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/third_party/WebKit/Source/platform/scroll/ScrollableAreaTest.cpp
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/third_party/WebKit/Source/platform/scroll/ScrollbarTestSuite.h
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/third_party/WebKit/public/platform/WebLayer.h
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/third_party/WebKit/public/platform/WebLayerScrollClient.h
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/ui/compositor/layer.cc
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/ui/compositor/layer.h
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/ui/views/controls/scroll_view.cc
[modify] https://crrev.com/27a8cfde871ad4786943a345569f36c9a5ba6e29/ui/views/controls/scroll_view.h

Comment 4 by pdr@chromium.org, Feb 17 2017

Blockedon: 693740

Comment 5 by pdr@chromium.org, Feb 17 2017

Blockedon: 693741
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 18 2017

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

commit 5d0f1c5d4ef47d3438c5b51db5da4aa0b1441a5b
Author: pdr <pdr@chromium.org>
Date: Sat Feb 18 18:35:36 2017

Set layer scroll data from PaintArtifactCompositor

This is a followup to [1] and begins setting scroll data on cc::Layer from
Blink's PaintArtifactCompositor. Because there's a 1:1 mapping between
scroll property nodes and ScrollableAreas, a WebLayerScrollClient field
has been added to the scroll property node. This field is then used in
the paint artifact compositor to set cc::Layer's did scroll callback.

With this patch, simple pages can be compositor wheel-scrolled on Windows.

[1] https://codereview.chromium.org/2680953002

BUG=667946
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[modify] https://crrev.com/5d0f1c5d4ef47d3438c5b51db5da4aa0b1441a5b/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
[modify] https://crrev.com/5d0f1c5d4ef47d3438c5b51db5da4aa0b1441a5b/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
[modify] https://crrev.com/5d0f1c5d4ef47d3438c5b51db5da4aa0b1441a5b/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositorTest.cpp
[modify] https://crrev.com/5d0f1c5d4ef47d3438c5b51db5da4aa0b1441a5b/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp
[modify] https://crrev.com/5d0f1c5d4ef47d3438c5b51db5da4aa0b1441a5b/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.h
[modify] https://crrev.com/5d0f1c5d4ef47d3438c5b51db5da4aa0b1441a5b/third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.cpp
[modify] https://crrev.com/5d0f1c5d4ef47d3438c5b51db5da4aa0b1441a5b/third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.h
[modify] https://crrev.com/5d0f1c5d4ef47d3438c5b51db5da4aa0b1441a5b/third_party/WebKit/Source/platform/graphics/paint/TransformPaintPropertyNode.h

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 21 2017

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

commit ff33efd971d548425b385adc947f8618788eb666
Author: pdr <pdr@chromium.org>
Date: Tue Feb 21 18:13:21 2017

Temporarily suppress element id collision DCHECK for SPV2

This patch temporarily suppresses an id collision DCHECK with SPV2 which
is failing because multiple layers have the same element id. This occurs for
simple cases [1] because many unnecessary layers are created with the
same property tree state. We explored fixing this with [2] but decided the
long-term fix described in  crbug.com/693693  would be better.

This suppression should be removed once  crbug.com/693693  is fixed.

[1] Enable SPV2 with --enable-slimming-paint-v2, then open:
<!doctype html>
abc<div style="width: 10px; height: 5000px; transform: translate3d(0,0,0);">def</div>

[2] https://codereview.chromium.org/2698673007

BUG=667946,  693693 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[modify] https://crrev.com/ff33efd971d548425b385adc947f8618788eb666/cc/trees/layer_tree_impl.cc

Comment 8 by pdr@chromium.org, May 5 2017

Blockedon: 718971

Comment 9 by pdr@chromium.org, Jul 13 2017

Blocking: 471333

Comment 10 by pdr@chromium.org, Aug 7 2017

Blockedon: 738613

Comment 11 by pdr@chromium.org, Aug 24 2017

Blockedon: 758421
Project Member

Comment 12 by bugdroid1@chromium.org, Sep 2 2017

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

commit b9571c5bd17129c4bbd21a6e6ebae1be6cba2c3d
Author: Philip Rogers <pdr@chromium.org>
Date: Sat Sep 02 00:05:06 2017

[SPV2] Fix frame view composited scrolling

This patch fixes a bug introduced by [1] for composited scrolling
the frame view without root layer scrolling. For this to work, the
frame view's scroll element id (set in PropertyTreeBuilder) must
match what is checked in LocalFrameView::ScrollableAreaWithElementId.
A test has been added to ensure this does not break again.

[1] https://chromium.googlesource.com/chromium/src/+/e7b75b84514adffc3f0a494e82c7de22708b0ec6

Bug: 667946
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I12fb9caa8e14d7ddd11c5875a748f9bcb81dbeb7
Reviewed-on: https://chromium-review.googlesource.com/644907
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499351}
[modify] https://crrev.com/b9571c5bd17129c4bbd21a6e6ebae1be6cba2c3d/third_party/WebKit/Source/core/exported/WebFrameTest.cpp
[modify] https://crrev.com/b9571c5bd17129c4bbd21a6e6ebae1be6cba2c3d/third_party/WebKit/Source/core/frame/LocalFrameView.cpp
[modify] https://crrev.com/b9571c5bd17129c4bbd21a6e6ebae1be6cba2c3d/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
[modify] https://crrev.com/b9571c5bd17129c4bbd21a6e6ebae1be6cba2c3d/third_party/WebKit/Source/core/paint/compositing/CompositedLayerMapping.cpp
[modify] https://crrev.com/b9571c5bd17129c4bbd21a6e6ebae1be6cba2c3d/third_party/WebKit/Source/core/paint/compositing/PaintLayerCompositor.cpp
[modify] https://crrev.com/b9571c5bd17129c4bbd21a6e6ebae1be6cba2c3d/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositorTest.cpp

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 29 2017

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

commit e5063b6417bfed27a2472c2a4189d77b79df36f0
Author: pdr@chromium.org <pdr@chromium.org>
Date: Fri Sep 29 01:03:54 2017

Do not fully invalidate paint for composited scrolls with SPV2

This patch removes the forced paint invalidation when scroll offset
changes with slimming paint v2 (SPV2).

This required adding four new invalidations for scroll offset changes:
1) background attachment local because we do not have compositor support.
2) A TODO has been added for background attachment fixed.
3) Overflow:hidden scroll offset changes. A scroll offset node can be
gained or lost when scroll offset crosses zero due to an IsZero check
in PropertyTreeBuilder. This IsZero check is important for reducing the
number of transform nodes in the common case that overflow:hidden has
no scroll offset.
4) The paint layer interest rect may change. This is tested by
PaintControllerPaintTestForSlimmingPaintV2.BlockScrollingNonLayeredContents
but was masked by over-invalidation in InvalidatePaintOfScrollControlIfNeeded.

Bug: 667946
Change-Id: Ic591684923ea5352706a91e65160310b81ba220a
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/644556
Commit-Queue: Philip Rogers <pdr@chromium.org>
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505237}
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/paint/invalidation/overflow-hidden-in-overflow-hidden-scrolled-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/paint/invalidation/scroll-in-clipped-layer-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/paint/invalidation/scroll-in-transformed-layer-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/paint/invalidation/scroll-with-transformed-parent-layer-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/invalidate-caret-in-composited-scrolling-container-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/invalidate-caret-in-non-composited-scrolling-container-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/selection/selection-in-composited-scrolling-container-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/selection/selection-in-non-composited-scrolling-container-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/platform/mac-mac10.9/paint/invalidation/invalidate-caret-in-composited-scrolling-container-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/platform/mac-mac10.9/paint/invalidation/invalidate-caret-in-non-composited-scrolling-container-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/platform/mac-mac10.9/paint/invalidation/selection/selection-in-composited-scrolling-container-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/platform/mac-mac10.9/paint/invalidation/selection/selection-in-non-composited-scrolling-container-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/invalidate-caret-in-composited-scrolling-container-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/invalidate-caret-in-non-composited-scrolling-container-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/selection/selection-in-composited-scrolling-container-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/selection/selection-in-non-composited-scrolling-container-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/invalidate-caret-in-composited-scrolling-container-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/invalidate-caret-in-non-composited-scrolling-container-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/selection/selection-in-composited-scrolling-container-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/selection/selection-in-non-composited-scrolling-container-expected.txt
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/Source/core/paint/PaintInvalidationCapableScrollableArea.cpp
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.h
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/Source/core/paint/PaintLayerScrollableAreaTest.cpp
[modify] https://crrev.com/e5063b6417bfed27a2472c2a4189d77b79df36f0/third_party/WebKit/Source/core/paint/PaintLayerTest.cpp

Comment 14 by pdr@chromium.org, Yesterday (34 hours ago)

Blockedon: 909749

Comment 15 by pdr@chromium.org, Yesterday (34 hours ago)

Blockedon: -909749

Comment 16 by pdr@chromium.org, Yesterday (34 hours ago)

Blockedon: 664249

Sign in to add a comment