New issue
Advanced search Search tips

Issue 738613 link

Starred by 1 user
Status: Fixed
Owner:
pdr@chromium.org
Closed: Mar 2018
Cc:
enne@chromium.org
vmp...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature

Blocked on:
issue 758360
Blocking:
issue 667946



Sign in to add a comment

[SPV2] Create scrollable base layers for compositor hit testing

Project Member Reported by pdr@chromium.org, Jun 30 2017 
Hit testing in the compositor requires that there is a cc::Layer marked as being scrollable and sized to the scroll node's bounds. This is needed to ensure scroll events are always captured by a scrollable area and to ensure the hit test is reliable.

For example, layers A and B have been squashed into AB and are on top of scrollable layer C:
+--------------+-+
|              |^|
| +---+- - - + | |
| |   |      | | |
| | A |   X    | |
| |   |      | | |
| +---+        | |
| |      +---+ | |
|        |   | | |
| |      | B | | |
|        |   | | |
| +- - - +---+ | |
|              | |
|       C      |v|
+--------------+-+

If AB and C have different scroll nodes, the compositor does not have enough information to handle the hit test and goes to the main thread.

Comment 1 by chrishtr@chromium.org, Jun 30 2017 

If all of the reasons for compositing a scroller are computed in the PrePaintTreeWalk, then I think
a version of your original idea from yesterday could work:

Create one dummy layer that does not draw content, which represents each composited
scrolling element. Insert it into the layer list just before any layer that actually draws content and contains
content under that scroller is created.

Comment 2 by schenney@chromium.org, Jul 7 2017

Labels: PaintTeamTriaged-20170630 BugSource-Team

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

Blocking: 667946
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 8 2017 

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

commit 4e8e4b1a646e08641d14d19ec797809ec269746f
Author: Philip Rogers <pdr@chromium.org>
Date: Tue Aug 08 04:09:19 2017

Add paint offset and element id to ScrollPaintPropertyNode

This patch adds paint offset and a compositor element id to
ScrollPaintPropertyNode. Paint offset will be used in a future
patch for offsetting the scroll hit test layer associated with
the scroll node. The compositor element id for scrolling is now
stored explicitly on the scroll node which removes an ambiguity
when using the transform paint property element id.

This patch is part of a series of patches to add scrollable hit
test layers in SPV2. A proof-of-concept patch is at:
https://chromium-review.googlesource.com/c/554060.

Bug:  738613 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I913bb237361963312aa42d07584051fd5889233d
Reviewed-on: https://chromium-review.googlesource.com/602927
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492527}
[modify] https://crrev.com/4e8e4b1a646e08641d14d19ec797809ec269746f/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
[modify] https://crrev.com/4e8e4b1a646e08641d14d19ec797809ec269746f/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp
[modify] https://crrev.com/4e8e4b1a646e08641d14d19ec797809ec269746f/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositorTest.cpp
[modify] https://crrev.com/4e8e4b1a646e08641d14d19ec797809ec269746f/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp
[modify] https://crrev.com/4e8e4b1a646e08641d14d19ec797809ec269746f/third_party/WebKit/Source/platform/graphics/paint/PropertyTreeState.cpp
[modify] https://crrev.com/4e8e4b1a646e08641d14d19ec797809ec269746f/third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.cpp
[modify] https://crrev.com/4e8e4b1a646e08641d14d19ec797809ec269746f/third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.h
[modify] https://crrev.com/4e8e4b1a646e08641d14d19ec797809ec269746f/third_party/WebKit/Source/platform/graphics/paint/TransformPaintPropertyNode.h
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 8 2017 

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

commit cf5191259342680a2e578b08147522ba73e2cb78
Author: Philip Rogers <pdr@chromium.org>
Date: Tue Aug 08 08:13:49 2017

Separate scroll offset translation from scroll nodes

This patch separates scroll offset translation from scroll nodes
by moving ScrollPaintPropertyNode to be owned by ObjectPaintProperties
(and LocalFrameView) instead of the scroll offset translation
TransformPaintPropertyNode. This largely reverts [1]. With this
change, overflow: hidden has a scroll offset translation without
having a scroll node that could be composited.

There are 3 main changes in this patch:
1) Scroll nodes are no longer owned by the transform that handles
scroll offset translation.
2) Scroll nodes are no longer created for LayoutObjects that have
overflow hidden but do not actually scroll (see:
PaintPropertyTreeBuilder::UpdateScrollAndScrollTranslation).
3) Scroll nodes are no longer created for FrameViews that have
overflow hidden but do not actually scroll (see:
PaintPropertyTreeBuilder::UpdateProperties(LocalFrameView&, ...)

This patch is part of a series of patches to add scrollable hit
test layers in SPV2. A proof-of-concept patch is at:
https://chromium-review.googlesource.com/c/554060.

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

Bug:  738613 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I69cea34b513fce392adeebfd038cadc917eedcc8
Reviewed-on: https://chromium-review.googlesource.com/602081
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492570}
[modify] https://crrev.com/cf5191259342680a2e578b08147522ba73e2cb78/third_party/WebKit/Source/core/frame/LocalFrameView.h
[modify] https://crrev.com/cf5191259342680a2e578b08147522ba73e2cb78/third_party/WebKit/Source/core/paint/FindPropertiesNeedingUpdate.h
[modify] https://crrev.com/cf5191259342680a2e578b08147522ba73e2cb78/third_party/WebKit/Source/core/paint/ObjectPaintProperties.h
[modify] https://crrev.com/cf5191259342680a2e578b08147522ba73e2cb78/third_party/WebKit/Source/core/paint/PaintControllerPaintTest.cpp
[modify] https://crrev.com/cf5191259342680a2e578b08147522ba73e2cb78/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
[modify] https://crrev.com/cf5191259342680a2e578b08147522ba73e2cb78/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
[modify] https://crrev.com/cf5191259342680a2e578b08147522ba73e2cb78/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp
[modify] https://crrev.com/cf5191259342680a2e578b08147522ba73e2cb78/third_party/WebKit/Source/core/paint/PaintPropertyTreePrinter.cpp
[modify] https://crrev.com/cf5191259342680a2e578b08147522ba73e2cb78/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositorTest.cpp
[modify] https://crrev.com/cf5191259342680a2e578b08147522ba73e2cb78/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp
[modify] https://crrev.com/cf5191259342680a2e578b08147522ba73e2cb78/third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.h
[modify] https://crrev.com/cf5191259342680a2e578b08147522ba73e2cb78/third_party/WebKit/Source/platform/graphics/paint/TransformPaintPropertyNode.cpp
[modify] https://crrev.com/cf5191259342680a2e578b08147522ba73e2cb78/third_party/WebKit/Source/platform/graphics/paint/TransformPaintPropertyNode.h
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 8 2017 

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

commit a46f9f546de805609489acad32eb0b8c7337095c
Author: pdr <pdr@chromium.org>
Date: Tue Aug 08 17:26:01 2017

[SPV2] Switch to kScroll namespace from kScrollTranslation

After [1], scrolling compositor element ids are set on scroll
nodes and not scroll translation nodes. Though a scroll
translation node will always be required in these cases, there
is no reason to disambiguate kScroll from kScrollTranslation.

This patch also adds a DCHECK that ScrollPaintPropertyNodes only
have kScroll-namespaced compositor element ids set.

[1] https://chromium-review.googlesource.com/c/602927

Bug:  738613 ,  753100 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I4c28250c0b1446f75e0740b1ac93f3e63af63fa7
Reviewed-on: https://chromium-review.googlesource.com/604472
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Walter Korman <wkorman@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492682}
[modify] https://crrev.com/a46f9f546de805609489acad32eb0b8c7337095c/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
[modify] https://crrev.com/a46f9f546de805609489acad32eb0b8c7337095c/third_party/WebKit/Source/platform/graphics/CompositorElementId.cpp
[modify] https://crrev.com/a46f9f546de805609489acad32eb0b8c7337095c/third_party/WebKit/Source/platform/graphics/CompositorElementId.h
[modify] https://crrev.com/a46f9f546de805609489acad32eb0b8c7337095c/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositorTest.cpp
[modify] https://crrev.com/a46f9f546de805609489acad32eb0b8c7337095c/third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.h
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 9 2017 

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

commit 56ebfeb6267fcc0276ca5a103bb21e2f92ad4267
Author: Philip Rogers <pdr@chromium.org>
Date: Wed Aug 09 17:52:27 2017

Add a placeholder display item for scroll hit testing

This patch adds a new display item used for creating scrollable hit test
layers in the correct paint order. ScrollHitTestDisplayItems are created
during the background paint phase to ensure hit test order is correct.

This approach results in extra invalidations and a followup patch will
move ScrollHitTest display items to special scrollable cc::Layers which
should resolve these.

This patch is part of a series of patches to add scrollable hit test layers
in SPV2. A proof-of-concept patch is at:
https://chromium-review.googlesource.com/c/554060.

Bug:  738613 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I19482ac9da104e7a0a52873af48e8b0a069d5912
Reviewed-on: https://chromium-review.googlesource.com/592650
Commit-Queue: Philip Rogers <pdr@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493046}
[modify] https://crrev.com/56ebfeb6267fcc0276ca5a103bb21e2f92ad4267/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[modify] https://crrev.com/56ebfeb6267fcc0276ca5a103bb21e2f92ad4267/third_party/WebKit/Source/core/paint/BlockPainter.cpp
[modify] https://crrev.com/56ebfeb6267fcc0276ca5a103bb21e2f92ad4267/third_party/WebKit/Source/core/paint/BlockPainter.h
[modify] https://crrev.com/56ebfeb6267fcc0276ca5a103bb21e2f92ad4267/third_party/WebKit/Source/core/paint/PaintControllerPaintTest.cpp
[modify] https://crrev.com/56ebfeb6267fcc0276ca5a103bb21e2f92ad4267/third_party/WebKit/Source/core/paint/PaintControllerPaintTest.h
[modify] https://crrev.com/56ebfeb6267fcc0276ca5a103bb21e2f92ad4267/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/56ebfeb6267fcc0276ca5a103bb21e2f92ad4267/third_party/WebKit/Source/platform/graphics/compositing/PaintChunksToCcLayer.cpp
[modify] https://crrev.com/56ebfeb6267fcc0276ca5a103bb21e2f92ad4267/third_party/WebKit/Source/platform/graphics/paint/DisplayItem.cpp
[modify] https://crrev.com/56ebfeb6267fcc0276ca5a103bb21e2f92ad4267/third_party/WebKit/Source/platform/graphics/paint/DisplayItem.h
[modify] https://crrev.com/56ebfeb6267fcc0276ca5a103bb21e2f92ad4267/third_party/WebKit/Source/platform/graphics/paint/README.md
[add] https://crrev.com/56ebfeb6267fcc0276ca5a103bb21e2f92ad4267/third_party/WebKit/Source/platform/graphics/paint/ScrollHitTestDisplayItem.cpp
[add] https://crrev.com/56ebfeb6267fcc0276ca5a103bb21e2f92ad4267/third_party/WebKit/Source/platform/graphics/paint/ScrollHitTestDisplayItem.h
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 15 2017 

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

commit d56dfca81d4ad8167567c3c197e50b29ff5016bc
Author: pdr <pdr@chromium.org>
Date: Tue Aug 15 21:17:04 2017

Cleanup composite-scrollable-fixed-position-when-descendants-composite

When working on [1] I was confused by the exception being thrown
when composite-scrollable-fixed-position-when-descendants-composite
failed. This turned out to be because the layer count is different
with my patch, but that wasn't obvious from the test. This patch
rewrites the test to be simpler and have cleaner output if it fails.

[1] https://chromium-review.googlesource.com/c/609317

Bug:  738613 
Change-Id: Idd228bf8a6030f8d06466823fbadcc5fb8cda54f
Reviewed-on: https://chromium-review.googlesource.com/615041
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494549}
[delete] https://crrev.com/e99ddc1455117eba392cc9cd354a8f49fe3d6838/third_party/WebKit/LayoutTests/compositing/composite-scrollable-fixed-position-when-descendants-composite-expected.txt
[modify] https://crrev.com/d56dfca81d4ad8167567c3c197e50b29ff5016bc/third_party/WebKit/LayoutTests/compositing/composite-scrollable-fixed-position-when-descendants-composite.html
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 22 2017 

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

commit eb94e55a1c5c47b2f90177c43e92651d1ba9287d
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Tue Aug 22 22:54:38 2017

Add bug numbers for some SPv2 paint/invalidation layout test failures

BUG=757977, 738613 , 757938 , 648274 
TBR=pdr@chromium.org,chrishtr@chromium.org

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I163c04bddfd2922adf78b87cb18749b5f02e9b4a
Reviewed-on: https://chromium-review.googlesource.com/627024
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496480}
[modify] https://crrev.com/eb94e55a1c5c47b2f90177c43e92651d1ba9287d/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2

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

Blockedon: 758360
Project Member

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

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

commit eef5607b471f955590278d5e44762c5206272caa
Author: pdr <pdr@chromium.org>
Date: Thu Aug 24 01:23:50 2017

Create scrollable cc:Layers from ScrollHitTest display items

This patch changes PaintArtifactCompositor to create layers specifically
for scrolling that are sized to the scroll container's bounds. These
layers are inserted in the correct paint order because they are
created from ScrollHitTestDisplayItems.

This patch has a change in the property nodes used for the scroll hit
test layer so that the layer is no longer under the scroll or overflow
clip nodes. This ensures crbug.com/753124 will be fixable in the future
by not clipping the hit test region and including borders. To do this,
the scroll node is now passed on the scroll hit test display item.

Bug:  738613 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: If0ade80cfd233384f4b74923148b2d79eb4b41d6
Reviewed-on: https://chromium-review.googlesource.com/609317
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496907}
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/core/paint/BlockPainter.cpp
[add] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/core/paint/BlockPainterTest.cpp
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/core/paint/PaintLayer.cpp
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.h
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositorTest.cpp
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/platform/graphics/compositing/PaintChunksToCcLayer.cpp
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.h
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/platform/graphics/paint/PaintChunker.cpp
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/platform/graphics/paint/PaintChunker.h
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/platform/graphics/paint/PaintChunkerTest.cpp
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/platform/graphics/paint/PropertyTreeState.cpp
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/platform/graphics/paint/ScrollHitTestDisplayItem.cpp
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/platform/graphics/paint/ScrollHitTestDisplayItem.h
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.h
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/platform/graphics/paint/TransformPaintPropertyNode.cpp
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/platform/graphics/paint/TransformPaintPropertyNode.h
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/platform/testing/TestPaintArtifact.cpp
[modify] https://crrev.com/eef5607b471f955590278d5e44762c5206272caa/third_party/WebKit/Source/platform/testing/TestPaintArtifact.h

Comment 12 by pdr@chromium.org, Mar 9 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment