Skip to content

GitLab

T
tangled
Commit bea4d986 authored by Philip Rogers's avatar Philip Rogers Committed by Commit Bot
Browse files
Options

Reland "Fix background painting on scrolling contents layer with foregrounds" 

This reverts commit e06da142.

Reason for revert: Was not the root cause of test failures.

Original change's description:
> Revert "Fix background painting on scrolling contents layer with foregrounds"
> 
> This reverts commit 7834ee21.
> 
> Reason for revert: Suspected as causing failing tests in 874162
> 
> Original change's description:
> > Fix background painting on scrolling contents layer with foregrounds
> > 
> > This patch removes a negative z-index children check from
> > PaintLayer::GetBackgroundPaintLocation. This lets backgrounds paint in
> > the scrolling contents layer in the presence of negative z-index
> > children, fixing the painting of background-attachment: local. This
> > patch also removes a DCHECK that was hit when a foreground layer and
> > kBackgroundPaintInScrollingContents coexisted. This DCHECK[1] could be
> > hit for root layers without the functional change in this patch. As the
> > test in this patch shows, kBackgroundPaintInScrollingContents and
> > foreground layers work together, so this patch makes non-root-layers use
> > both, and removes the DCHECK.
> > 
> > What are foreground layers? A foreground layer is created if a stacked
> > PaintLayer has composited negative z-index children that should wedge
> > between the layer's background and the layer's contents. The foreground
> > layer will have the normal-flow contents in this case. See the comment
> > above foreground_layer_ in composited_layer_mapping.h.
> > 
> > [1] DCHECK originally added in: https://crrev.com/9a2cdc8d96d. Reading
> > the comments, I think the DCHECK was intended to ensure the background
> > would not be painted into the foreground layer.
> > 
> > Bug: 861948
> > Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
> > Change-Id: I7409cbe9ac61fed444856df342d0707d6babab09
> > Reviewed-on: https://chromium-review.googlesource.com/1173532
> > Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> > Commit-Queue: Philip Rogers <pdr@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#584802}
> 
> TBR=pdr@chromium.org,chrishtr@chromium.org
> 
> Change-Id: Ifd22a963dbd5ce288dfb31ab1e8ac41ebe63062d
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 861948
> Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
> Reviewed-on: https://chromium-review.googlesource.com/1184109
> Reviewed-by: Philip Rogers <pdr@chromium.org>
> Commit-Queue: Philip Rogers <pdr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#584895}

TBR=pdr@chromium.org,chrishtr@chromium.org

Change-Id: I569313a9d5e1dd0b79e541cc3e3ae8816c4c3e05
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 861948
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1184181Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584911}
parent 074a70f9
Hide whitespace changes
Inline Side-by-side
Showing with 96 additions and 13 deletions
third_party/WebKit/LayoutTests/paint/background/scrolling-background-with-negative-z-child-expected.html 0 → 100644
View file @ bea4d986
<!doctype html>
<style>
#container {
overflow: scroll;
width: 400px;
height: 300px;
background: linear-gradient(black, white);
background-attachment: local;
will-change: transform;
}
#child {
height: 1000px;
width: 10px;
}
</style>
<div id="container">
<div id="child"></div>
</div>
<script>
document.getElementById('container').scrollTop = 1000;
</script>
third_party/WebKit/LayoutTests/paint/background/scrolling-background-with-negative-z-child.html 0 → 100644
View file @ bea4d986
<!doctype html>
<style>
#container {
overflow: scroll;
width: 400px;
height: 300px;
background: linear-gradient(black, white);
background-attachment: local;
position: relative;
z-index: 2;
will-change: transform;
}
#compositedChild {
height: 1000px;
width: 10px;
will-change: transform;
z-index: -1;
position: absolute;
}
</style>
<div id="container">
<div id="compositedChild"></div>
</div>
<script>
if (window.testRunner) {
testRunner.waitUntilDone();
testRunner.layoutAndPaintAsyncThen(function() {
document.getElementById('container').scrollTop = 1000;
testRunner.notifyDone();
});
} else {
// For manual testing.
setTimeout(function() {
document.getElementById('container').scrollTop = 1000;
}, 500);
}
</script>
third_party/WebKit/LayoutTests/paint/invalidation/invalidation-on-foreground-graphics-layer-expected.txt
View file @ bea4d986
......@@ -21,7 +21,6 @@
"name": "LayoutBlockFlow DIV",
"position": [8, 8],
"bounds": [200, 200],
"contentsOpaque": true,
"backgroundColor": "#D3D3D3"
},
{
......@@ -34,6 +33,7 @@
"name": "Scrolling Contents Layer",
"position": [8, 8],
"bounds": [300, 300],
"contentsOpaque": true,
"backgroundColor": "#D3D3D3"
},
{
......
third_party/WebKit/LayoutTests/paint/stacking/html-with-negative-z-index-expected.html 0 → 100644
View file @ bea4d986
<!doctype html>
<style> * { margin: 0; } </style>
<div style="width: 100px; height: 100px; background: green;"></div>
third_party/WebKit/LayoutTests/paint/stacking/html-with-negative-z-index.html 0 → 100644
View file @ bea4d986
<!doctype html>
<style>
* {
margin: 0;
}
html {
background: white;
width: 200px;
height: 200px;
position: relative;
z-index: -1;
}
.compDesc {
will-change: transform;
position: absolute;
top: 0;
left: 50px;
width: 50px;
height: 100px;
background: green;
z-index: -2;
}
.desc {
width: 50px;
height: 100px;
background: green;
}
</style>
<div class="compDesc"></div>
<div class="desc"></div>
third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc
View file @ bea4d986
......@@ -3432,11 +3432,7 @@ void CompositedLayerMapping::PaintContents(
graphics_layer == scrolling_contents_layer_.get() ||
graphics_layer == decoration_outline_layer_.get() ||
graphics_layer == ancestor_clipping_mask_layer_.get()) {
bool paint_root_background_onto_scrolling_contents_layer =
background_paints_onto_scrolling_contents_layer_;
DCHECK(!paint_root_background_onto_scrolling_contents_layer ||
!foreground_layer_);
if (paint_root_background_onto_scrolling_contents_layer) {
if (background_paints_onto_scrolling_contents_layer_) {
if (graphics_layer == scrolling_contents_layer_.get())
paint_layer_flags &= ~kPaintLayerPaintingSkipRootBackground;
else if (!background_paints_onto_graphics_layer_)
......
third_party/blink/renderer/core/paint/paint_layer.cc
View file @ bea4d986
......@@ -2750,10 +2750,6 @@ BackgroundPaintLocation PaintLayer::GetBackgroundPaintLocation(
else
location = GetLayoutObject().GetBackgroundPaintLocation(reasons);
}
if (!IsRootLayer() && stacking_node_) {
if (StackingNode()->HasNegativeZOrderList())
location = kBackgroundPaintInGraphicsLayer;
}
return location;
}
......
third_party/blink/renderer/core/paint/paint_layer_scrollable_area_test.cc
View file @ bea4d986
......@@ -154,9 +154,9 @@ TEST_F(PaintLayerScrollableAreaTest,
</div>
)HTML");
// #scroller1 cannot paint background into scrolling contents layer because it
// has a negative z-index child.
EXPECT_EQ(kBackgroundPaintInGraphicsLayer,
// #scroller1 can paint background into scrolling contents layer even with a
// negative z-index child.
EXPECT_EQ(kBackgroundPaintInScrollingContents,
GetBackgroundPaintLocation("scroller1"));
// #scroller2 cannot paint background into scrolling contents layer because it
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment