[BlinkGenPropertyTrees] Scrollbars not clipped correctly |
|||||||||
Issue descriptionThis causes the border-box-rect-clips-scrollbars.html LayoutTest to fail: ./third_party/WebKit/Tools/Scripts/run-webkit-tests -t ChromeDebug --additional-driver-flag="--enable-blink-gen-property-trees" scrollbars/border-box-rect-clips-scrollbars.html
,
May 9 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2b167e9924ea45519128db9b589994c8f98df330 commit 2b167e9924ea45519128db9b589994c8f98df330 Author: David Bokan <bokan@chromium.org> Date: Wed May 09 19:39:54 2018 [BlinkGenPropertyTrees] Add failing scrollbar expectation https://crrev.com/5ed7b147c2631389d601d99c831b56f17e0511be made scrollbars appear in this test and removed the expectation but the clipping is incorrect so the test fails. Bug: 841342 Change-Id: Ic4efa5b648b288857c8b47878da1c57060303453 Reviewed-on: https://chromium-review.googlesource.com/1052212 Commit-Queue: Tien-Ren Chen <trchen@chromium.org> Reviewed-by: Tien-Ren Chen <trchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#557281} [modify] https://crrev.com/2b167e9924ea45519128db9b589994c8f98df330/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-gen-property-trees
,
May 10 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b6d006bf5eb32c19eccac5fd0272a098572b55fb commit b6d006bf5eb32c19eccac5fd0272a098572b55fb Author: David Bokan <bokan@chromium.org> Date: Thu May 10 15:52:54 2018 [BlinkGenPropertyTrees] Add more scrolling tests expectations Most "scrolling" tests in Blink are in the fast/scrolling and fast/events directories, the latter due to testing touch, wheel, keyboard, etc. events. Many of these will use eventSender and so avoid the composited path entirely so they provide just some mildly useful main-thread scrolling coverage. Some do use gpuBenchmarking and are thus more useful. Bug: 836913 , 840017 , 841342 , 841423 Change-Id: Idc4c2fae7ee7a48f074df8b3df5a027a75c82f71 Reviewed-on: https://chromium-review.googlesource.com/1052823 Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#557533} [modify] https://crrev.com/b6d006bf5eb32c19eccac5fd0272a098572b55fb/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-gen-property-trees [modify] https://crrev.com/b6d006bf5eb32c19eccac5fd0272a098572b55fb/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/b6d006bf5eb32c19eccac5fd0272a098572b55fb/third_party/WebKit/LayoutTests/VirtualTestSuites [add] https://crrev.com/b6d006bf5eb32c19eccac5fd0272a098572b55fb/third_party/WebKit/LayoutTests/virtual/threaded/fast/scrolling/README.txt
,
May 14 2018
I can reproduce this issue by running chrome with --enable-blink-gen-property-trees on http://ht.chaopeng.me/scrollbar-clip.html I print out the clip tree. It seems correct with --enable-blink-gen-property-trees. And I don't know how it works without --enable-blink-gen-property-trees. pdr@ could you please help me take a look for the log. Thank you.
,
May 14 2018
,
May 14 2018
I think we need a little more debug output.
<!DOCTYPE html>
<style>
* { margin: 0; }
.scroller {
margin-left: 10px;
height: 50px;
overflow-y: scroll;
overflow-x: hidden;
width: 8px;
will-change: transform;
}
.space {
width: 20px;
height: 100px;
background: blue;
}
</style>
<div class="scroller"><div class="space"></div></div>
(I'm not sure how you debug the blink side, but I find putting the following line in local_frame_view.cc as the last line of PrePaint works well: showAllPropertyTrees(*this);)
I'm getting the following clip tree:
root 0x17233c8290 {"localTransformSpace":"0x17232e0590","rect":"InfiniteIntRect"}
OverflowClip (LayoutView #document) 0x17233c8510 {"parent":"0x17233c8290","localTransformSpace":"0x17232e0c70","rect":"0,0 800x600","rectExcludingOverlayScrollbars":"0,0 800x600"}
OverflowControlsClip (LayoutBlockFlow DIV class='scroller composited') 0x17233c8650 {"parent":"0x17233c8510","localTransformSpace":"0x17232e09b0","rect":"0,0 8x50"}
OverflowClip (LayoutBlockFlow DIV class='scroller composited') 0x17233c8790 {"parent":"0x17233c8510","localTransformSpace":"0x17232e09b0","rect":"0,0 -7x50","rectExcludingOverlayScrollbars":"0,0 -7x50"}
The OverflowControlsClip of size 8x50 should be clipping the scrollbar. I think the issue is that the scrollbar paint chunk has the wrong clip node. Can you try printing the paint chunks and seeing if the scrollbar paint chunk is associated with the OverflowControlsClip clip node?
,
May 15 2018
It seems scrollbar paint chunk has the wrong clip node. I added log to GraphicsLayer::PaintContentsToDisplayList. We assign the root clip node to all paint chunk.
[1:1:0515/114443.933521:ERROR:pre_paint_tree_walk.cc(57)] - blink begin -----------------------------------------------
[1:1:0515/114443.933770:ERROR:paint_property_tree_printer.cc(206)] Transform tree:
root 0x3e5d3a3a8170 {"flattensInheritedTransform":false,"backface":"visible","scroll":"0x3e5d3a3043d0"}
PaintOffsetTranslation (LayoutView #document) 0x3e5d3a3a8430 {"parent":"0x3e5d3a3a8170"}
PaintOffsetTranslation (LayoutBlockFlow DIV class='scroller composited') 0x3e5d3a3a8590 {"parent":"0x3e5d3a3a8430","matrix":"translation(8,8,0)"}
Transform (LayoutBlockFlow DIV class='scroller composited') 0x3e5d3a3a86f0 {"parent":"0x3e5d3a3a8590","backface":"visible","directCompositingReasons":"willChange","compositorElementId":"(72)"}
ScrollTranslation (LayoutBlockFlow DIV class='scroller composited') 0x3e5d3a3a8850 {"parent":"0x3e5d3a3a86f0","scroll":"0x3e5d3a304490"}
[1:1:0515/114443.934334:ERROR:paint_property_tree_printer.cc(211)] Clip tree:
root 0x3e5d3a39c250 {"localTransformSpace":"0x3e5d3a3a8170","rect":"InfiniteIntRect"}
OverflowClip (LayoutView #document) 0x3e5d3a39c5b0 {"parent":"0x3e5d3a39c250","localTransformSpace":"0x3e5d3a3a8430","rect":"0,0 1270x1438","rectExcludingOverlayScrollbars":"0,0 1270x1438"}
OverflowControlsClip (LayoutBlockFlow DIV class='scroller composited') 0x3e5d3a39c6d0 {"parent":"0x3e5d3a39c5b0","localTransformSpace":"0x3e5d3a3a86f0","rect":"0,0 12x54"}
OverflowClip (LayoutBlockFlow DIV class='scroller composited') 0x3e5d3a39c7f0 {"parent":"0x3e5d3a39c5b0","localTransformSpace":"0x3e5d3a3a86f0","rect":"2,2 -7x35","rectExcludingOverlayScrollbars":"2,2 -7x35"}
[1:1:0515/114443.934943:ERROR:paint_property_tree_printer.cc(216)] Effect tree:
[1:1:0515/114443.935122:ERROR:paint_property_tree_printer.cc(221)] Scroll tree:
root 0x3e5d3a3043d0 {}
Scroll (LayoutBlockFlow DIV class='scroller composited') 0x3e5d3a304490 {"parent":"0x3e5d3a3043d0","containerRect":"2,2 -7x35","contentsRect":"2,2 1x100","userScrollable":"both","compositorElementId":"(74)"}
[1:1:0515/114443.935488:ERROR:pre_paint_tree_walk.cc(59)] - blink end -----------------------------------------------
[1:1:0515/114443.942479:ERROR:graphics_layer.cc(1435)] chunk begin -----------------------
[1:1:0515/114443.942684:ERROR:graphics_layer.cc(1437)] chunk "PaintChunk(begin=0, end=1, id=0x3e5d3a345458:DrawingDocumentBackground:0 cacheable=1 props=(t:0x3e5d3a3a8430 c:0x3e5d3a39c5b0 e:0x3e5d3a3501f0) bounds=0,0 1270x1438 known_to_be_opaque=0)"
[1:1:0515/114443.943093:ERROR:graphics_layer.cc(1439)] chunk end -------------------------
[1:1:0515/114443.943361:ERROR:graphics_layer.cc(1435)] chunk begin -----------------------
[1:1:0515/114443.943497:ERROR:graphics_layer.cc(1437)] chunk "PaintChunk(begin=0, end=1, id=0x37d032a081c0:DrawingPaintPhaseSelfBlockBackground:0 cacheable=1 props=(t:0x3e5d3a3a86f0 c:0x3e5d3a39c5b0 e:0x3e5d3a3501f0) bounds=0,0 12x54 known_to_be_opaque=0)"
[1:1:0515/114443.943877:ERROR:graphics_layer.cc(1439)] chunk end -------------------------
[1:1:0515/114443.944106:ERROR:graphics_layer.cc(1435)] chunk begin -----------------------
[1:1:0515/114443.944264:ERROR:graphics_layer.cc(1439)] chunk end -------------------------
[1:1:0515/114443.945759:ERROR:graphics_layer.cc(1435)] chunk begin -----------------------
[1:1:0515/114443.945981:ERROR:graphics_layer.cc(1437)] chunk "PaintChunk(begin=0, end=1, id=0x3e5d3a346898:DrawingScrollbarCorner:0 cacheable=1 props=(t:0x3e5d3a3a86f0 c:0x3e5d3a39c5b0 e:0x3e5d3a3501f0) bounds=-5,37 15x15 known_to_be_opaque=0)"
[1:1:0515/114443.946352:ERROR:graphics_layer.cc(1439)] chunk end -------------------------
,
May 15 2018
When painting the scrollbar, this line should pick up the overflow controls clip: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/scrollable_area_painter.cc?type=cs&q=OverflowControlsClip&sq=package:chromium&g=0&l=183 When painting the scrollbar, are we not going through that code?
,
May 15 2018
It go through "scoped_paint_chunk_properties", but scrollbar has layer. So scrollbars do not paint here.
,
May 15 2018
Can you try adding the scoped paint chunk properties to the code that paints the scrollbar?
,
May 16 2018
This scrollbar is same as overlay scrollbar, they are painted in cc side. What should I do in this case? #2 0x7fda4016c306 blink::ScrollbarThemeAura::PaintTrackPiece() #3 0x7fda4016c2bf blink::ScrollbarThemeAura::PaintTrackBackground() #4 0x7fda3fd5ab02 blink::WebScrollbarThemePainter::PaintTrackBackground() #5 0x7fda40168ef6 blink::ScrollbarLayerDelegate::PaintPart() #6 0x7fda4f8a97e2 cc::PaintedScrollbarLayer::RasterizeScrollbarPart() #7 0x7fda4f8a8b97 cc::PaintedScrollbarLayer::Update() #8 0x7fda4fa7fe1f cc::LayerTreeHost::PaintContent() #9 0x7fda4fa7ed94 cc::LayerTreeHost::DoUpdateLayers() #10 0x7fda4fa7de2d cc::LayerTreeHost::UpdateLayers() #11 0x7fda4fb6d9e6 cc::ProxyMain::BeginMainFrame()
,
May 16 2018
The cc::Layer should be created in Blink from an associated GraphicsLayer. Can you ensure that GraphicsLayer has the overflow controls clip set via SetLayerState?
,
May 16 2018
It seems SetLayerState set the wrong clip. I think I need to investigate at https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/compositing/compositing_layer_property_updater.cc?type=cs&g=0&l=45-68 [1:1:0516/164328.178158:ERROR:composited_layer_mapping.cc(2131)] create leyer V 0x1f63ead46410 [1:1:0516/164328.178585:ERROR:composited_layer_mapping.cc(2131)] create leyer H 0x1f63ead46650 [1:1:0516/164328.178865:ERROR:composited_layer_mapping.cc(2131)] create leyer V 0x1f63ead46890 [1:1:0516/164328.179063:ERROR:composited_layer_mapping.cc(2131)] create leyer V 0x1f63ead46ad0 [1:1:0516/164328.180417:ERROR:graphics_layer.cc(1388)] GraphicsLayer::SetLayerState "t:0x1f63eada8430 c:0x1f63ead9c250 e:0x1f63ead501f0" 0x1f63ead45210 [1:1:0516/164328.180727:ERROR:graphics_layer.cc(1388)] GraphicsLayer::SetLayerState "t:0x1f63eada8430 c:0x1f63ead9c5b0 e:0x1f63ead501f0" 0x1f63ead45450 [1:1:0516/164328.181594:ERROR:graphics_layer.cc(1388)] GraphicsLayer::SetLayerState "t:0x1f63eada86f0 c:0x1f63ead9c5b0 e:0x1f63ead501f0" 0x1f63ead45690 [1:1:0516/164328.181857:ERROR:graphics_layer.cc(1388)] GraphicsLayer::SetLayerState "t:0x1f63eada86f0 c:0x1f63ead9c5b0 e:0x1f63ead501f0" 0x1f63ead46410 [1:1:0516/164328.182071:ERROR:graphics_layer.cc(1388)] GraphicsLayer::SetLayerState "t:0x1f63eada86f0 c:0x1f63ead9c5b0 e:0x1f63ead501f0" 0x1f63ead46650 [1:1:0516/164328.182253:ERROR:graphics_layer.cc(1388)] GraphicsLayer::SetLayerState "t:0x1f63eada86f0 c:0x1f63ead9c5b0 e:0x1f63ead501f0" 0x1f63ead46890 [1:1:0516/164328.182451:ERROR:graphics_layer.cc(1388)] GraphicsLayer::SetLayerState "t:0x1f63eada8850 c:0x1f63ead9c7f0 e:0x1f63ead501f0" 0x1f63ead461d0 [1:1:0516/164328.183279:ERROR:paint_property_tree_printer.cc(211)] Clip tree: root 0x1f63ead9c250 {"localTransformSpace":"0x1f63eada8170","rect":"InfiniteIntRect"} OverflowClip (LayoutView #document) 0x1f63ead9c5b0 {"parent":"0x1f63ead9c250","localTransformSpace":"0x1f63eada8430","rect":"0,0 1270x1487","rectExcludingOverlayScrollbars":"0,0 1270x1487"} OverflowControlsClip (LayoutBlockFlow DIV class='scroller composited') 0x1f63ead9c6d0 {"parent":"0x1f63ead9c5b0","localTransformSpace":"0x1f63eada86f0","rect":"0,0 12x54"} OverflowClip (LayoutBlockFlow DIV class='scroller composited') 0x1f63ead9c7f0 {"parent":"0x1f63ead9c5b0","localTransformSpace":"0x1f63eada86f0","rect":"2,2 -7x35","rectExcludingOverlayScrollbars":"2,2 -7x35"}
,
May 17 2018
We create 2 clips for scrollable area:
1. OverflowControlsClip (LayoutBlockFlow DIV class='scroller composited') 0x14518ad9c6d0 {"parent":"0x14518ad9c5b0","localTransformSpace":"0x14518ada86f0","rect":"0,0 12x54"}
2. OverflowClip (LayoutBlockFlow DIV class='scroller composited') 0x14518ad9c7f0 {"parent":"0x14518ad9c5b0","localTransformSpace":"0x14518ada86f0","rect":"2,2 -7x35","rectExcludingOverlayScrollbars":"2,2 -7x35"}
OverflowControlsClip includes scrollbar and OverflowClip excludes.
When we walk through the layout tree. I don't see we walk through OverflowControlsClip. But we set PropertyTreeState in CompositingLayerPropertyUpdater::Update when walk through OverflowClip. Also LocalBorderBoxProperties seems not include clip.
I think we need to assign OverflowControlsClip as the scrollbar's clip. pdr@ could you please give me some help? Thank you.
,
May 17 2018
Re: "I think we need to assign OverflowControlsClip as the scrollbar's clip.", I agree completely. Please give it a try. I am not sure LocalBorderBoxProperties should be associated with the OverflowControlsClip. One way forward: find whatever SetLayerState call is associated with the scrollbar and make that use the OverflowControlsClip. This looks like a small change in CompositingLayerPropertyUpdater. Btw, this hierarchy description may be useful for you: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/object_paint_properties.h?q=object_paint_prop&sq=package:chromium&g=0&l=119
,
May 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bcd51e1bd9608026939396e7d808583cf492905d commit bcd51e1bd9608026939396e7d808583cf492905d Author: chaopeng <chaopeng@chromium.org> Date: Wed May 23 20:12:33 2018 Use OverflowControlsClip for scrollbars We create 2 clips for scrollable area: 1. OverflowControlsClip 2. OverflowClip OverflowControlsClip includes scrollbar and OverflowClip excludes. Currently, we set LocalBorderBoxProperties or cssclip as clip to PropertyTreeState in CompositingLayerPropertyUpdater::Update when walk through layers which is incorrect for scrollbars. In this patch, we set OverflowControlsClip as clip to scrollbars and scroll corner in CompositingLayerPropertyUpdater::Update. Bug: 841342 Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;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: I3bafb7bd542bacaa998897867249861122df9516 Reviewed-on: https://chromium-review.googlesource.com/1069493 Commit-Queue: Jianpeng Chao <chaopeng@chromium.org> Reviewed-by: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#561226} [modify] https://crrev.com/bcd51e1bd9608026939396e7d808583cf492905d/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-gen-property-trees [modify] https://crrev.com/bcd51e1bd9608026939396e7d808583cf492905d/third_party/blink/renderer/core/paint/compositing/compositing_layer_property_updater.cc
,
May 24 2018
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by bokan@chromium.org
, May 9 2018Labels: -Pri-3 Pri-2
Owner: chaopeng@chromium.org
Status: Assigned (was: Available)