[BlinkGenPropertyTrees] fast/multicol/composited-layer-nested.html is failing |
||||
Issue descriptionThis test is failing with blink gen property trees: third_party/blink/tools/run_web_tests.py --debug --additional-driver-flag=--enable-blink-gen-property-trees fast/multicol/composited-layer-nested.html The composited layer positions are actually different. We now match Safari so this could be a progression. This needs investigation.
,
Jun 19 2018
This test failure may also be related: paint/pagination/composited-paginated-outlined-box.html
,
Aug 16
,
Aug 17
For fast/multicol/composited-layer-nested.html, there are quirks about the fact that SPv1 doesn't fragment composited and squashed layers. The failure of the test is caused by different methods of handling the quirks.
The test can be simplified as the following:
<div style="columns: 4; height: 100px; column-rule: 1px solid black; border: 1px solid black">
<div id="outer" style="will-change: transform">
<div style="height: 100px">B1</div>
<div style="height: 100px">B2</div>
<div id="inner" style="will-change: transform">
<div style="height: 100px">B3</div>
<div style="height: 100px">B4</div>
</div>
</div>
</div>
In pre-BGPT, the result is like:
---------------------
|B1 | |B3 | |
| | | | |
---------------------
B2 B4
Though we don't fragment "outer", we do place "inner" at its normal place regardless of compositing of "outer", so Block3 and Block4 appears in the third column.
However, PaintPropertyTreeBuilder handles the quicks differently. In the property trees, "inner" and their children are placed at different places. The PaintOffsetTranslation of "inner" is translate(0, 100) because PaintPropertyTreeBuilder now doesn't fragment "outer" as a whole including its descendants regardless of their compositing status.
In BGPT, we fully use the property trees to place the composited layers, so the result is like:
---------------------
|B1 | | | |
| | | | |
---------------------
B2
B3
B4
We may have 2 choices:
1. Change PaintPropertyTreeBuilder to mimic all the current quirks of Pre-BGPT to pass the test with the existing expectation.
2. Keep the current PaintPropertyTreeBuilder behavior and switch to the new quirks.
I'm inclined to 2 which is simpler. It may break some web pages that are based on the current quirks. I'll get stats to evaluate the impact. Do you know how to do a UMA experiment on cluster-telemetry or other ways to do such stat? (Anyway I could still use my ugly stat CL https://chromium-review.googlesource.com/c/chromium/src/+/864636.)
,
Aug 17
I think the following example can be another justification of choice 2 which produces "better" result:
<div style="columns: 4; height: 100px; column-rule: 1px solid black; border: 1px solid black">
<div id="outer" style="will-change: transform">
<div style="height: 100px">B1</div>
<div style="height: 100px">B2</div>
<div id="inner" style="will-change: transform">
<div style="height: 100px">B3</div>
<div style="height: 100px">B4</div>
</div>
<div style="height: 100px">B5</div>
<div style="height: 100px">B6</div>
</div>
</div>
For choice 1 (current behavior of pre-BGPT), the result is like:
-------------
|B1| |B3| |
-------------
B2 B4
B5
B6
For choice 2 (current behavior of BGPT), the result is like:
-------------
|B1| | | |
-------------
B2
B3
B4
B5
B6
,
Aug 17
Stat data is available for top 100k desktop sites (https://ct.skia.org/results/cluster-telemetry/tasks/benchmark_runs/wangxianzhu-20180817184053/consolidated_outputs/wangxianzhu-20180817184053.output): A. Percentage of LayoutObjects that are composited and under multicol: 0.002% Percentage of web pages containing A: 0.13% B. Percentage of LayoutObjects that are composited and under another composited LayoutObject under multicol: 0.00018% Percentage of web pages containing B: 0.012% C. Percentage of LayoutObjects that are actually affected by the different quirk methods: 0.00000014% (2 out of 17393871) Percentage of web pages containing C: 0.001% (1 out of 96135) Based on the low numbers, I think we should choose Choice 2.
,
Aug 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/429c5dcef93750aae315c80cdbfaea74d43c965c commit 429c5dcef93750aae315c80cdbfaea74d43c965c Author: Xianzhu Wang <wangxianzhu@chromium.org> Date: Tue Aug 21 02:58:05 2018 [BGPT] Fix paint/pagination/composited-paginated-outlined-box.html For SPv1, we don't fragment a composited or squashed layer, but should create a single fragment for it under the parent fragment where the layer's border box starts. Previously we used overflow box to determine fragments causing the composited/squashed layer to be placed in wrong parent fragment when the layer has overflows. Now use border box rect in the case. Bug: 854192 Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ib05e8629f423b3a1960de1fc8f3989c42f54766f Reviewed-on: https://chromium-review.googlesource.com/1180377 Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Cr-Commit-Position: refs/heads/master@{#584615} [modify] https://crrev.com/429c5dcef93750aae315c80cdbfaea74d43c965c/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-gen-property-trees [modify] https://crrev.com/429c5dcef93750aae315c80cdbfaea74d43c965c/third_party/blink/renderer/core/paint/paint_property_tree_builder.cc
,
Aug 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/380436db198ac23ee1a901f74407a58bf7829c64 commit 380436db198ac23ee1a901f74407a58bf7829c64 Author: Xianzhu Wang <wangxianzhu@chromium.org> Date: Tue Aug 21 05:40:46 2018 [BGPT] Rebaseline two multicol tests - fast/multicol/composited-layer-nested.html According to discussions and data in crbug.com/854192 , we should not fragment a composited/squashed layer as a whole no matter if there are nested composited layers under it (as current BGPT code does). - fast/multicol/vertical-rl/composited-relpos-overlapping-will-change.html The new result looks more correct (the composited layer is placed in the second column where the layer's border box rect starts). Also updated the test to show more clearly. Bug: 854192 Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ie1ad00980034c6c991fa82d2ee4f675010292cdb Reviewed-on: https://chromium-review.googlesource.com/1180405 Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#584661} [modify] https://crrev.com/380436db198ac23ee1a901f74407a58bf7829c64/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-gen-property-trees [modify] https://crrev.com/380436db198ac23ee1a901f74407a58bf7829c64/third_party/WebKit/LayoutTests/fast/multicol/vertical-rl/composited-relpos-overlapping-will-change.html [add] https://crrev.com/380436db198ac23ee1a901f74407a58bf7829c64/third_party/WebKit/LayoutTests/flag-specific/enable-blink-gen-property-trees/fast/multicol/composited-layer-nested-expected.png [add] https://crrev.com/380436db198ac23ee1a901f74407a58bf7829c64/third_party/WebKit/LayoutTests/flag-specific/enable-blink-gen-property-trees/fast/multicol/vertical-rl/composited-relpos-overlapping-will-change-expected.png [modify] https://crrev.com/380436db198ac23ee1a901f74407a58bf7829c64/third_party/WebKit/LayoutTests/platform/linux/fast/multicol/vertical-rl/composited-relpos-overlapping-will-change-expected.png [modify] https://crrev.com/380436db198ac23ee1a901f74407a58bf7829c64/third_party/WebKit/LayoutTests/platform/mac/fast/multicol/vertical-rl/composited-relpos-overlapping-will-change-expected.png [modify] https://crrev.com/380436db198ac23ee1a901f74407a58bf7829c64/third_party/WebKit/LayoutTests/platform/win/fast/multicol/vertical-rl/composited-relpos-overlapping-will-change-expected.png
,
Aug 21
|
||||
►
Sign in to add a comment |
||||
Comment 1 by pdr@chromium.org
, Jun 19 2018