Skip to content

Commit

Permalink
Removing the kInlineTransform compositing reason
Browse files Browse the repository at this point in the history 
This optimization was the cause of at least one bug (crbug.com/812166), and will hopefully not be needed once Composite After Paint is implemented.

Bug: 812166
Change-Id: I2024b887949904bad5515d76b2fa29607dae3ff9
Reviewed-on: https://chromium-review.googlesource.com/c/1354572
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612670}(cherry picked from commit f6e075c)
Reviewed-on: https://chromium-review.googlesource.com/c/1359144
Reviewed-by: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#13}
Cr-Branched-From: d897fb1-refs/heads/master@{#612437}
  • Loading branch information
@mfreed7
mfreed7 committed Dec 3, 2018
1 parent 07bb924 commit 58bbcb0
Show file tree
Hide file tree
Showing 24 changed files with 54 additions and 221 deletions.
3 changes: 0 additions & 3 deletions third_party/blink/renderer/core/paint/compositing/compositing_reason_finder.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,6 @@ CompositingReasonFinder::PotentialCompositingReasonsFromStyle(
!style.SubtreeWillChangeContents())
reasons |= CompositingReason::kWillChangeCompositingHint;

if (style.HasInlineTransform())
reasons |= CompositingReason::kInlineTransform;

if (style.UsedTransformStyle3D() == ETransformStyle3D::kPreserve3d)
reasons |= CompositingReason::kPreserve3DWith3DDescendants;

Expand Down
10 changes: 1 addition & 9 deletions third_party/blink/renderer/core/paint/compositing/compositing_requirements_updater.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -608,11 +608,6 @@ void CompositingRequirementsUpdater::UpdateRecursive(
will_be_composited_or_squashed = true;
}

if (will_be_composited_or_squashed) {
reasons_to_composite |= layer->PotentialCompositingReasonsFromStyle() &
CompositingReason::kInlineTransform;
}

if (will_be_composited_or_squashed &&
layer->GetLayoutObject().StyleRef().HasBlendMode()) {
current_recursion_data.has_unisolated_composited_blending_descendant_ =
Expand All @@ -631,12 +626,9 @@ void CompositingRequirementsUpdater::UpdateRecursive(
bool is_composited_clipping_layer =
can_be_composited && (reasons_to_composite &
CompositingReason::kClipsCompositingDescendants);
bool is_composited_with_inline_transform =
reasons_to_composite & CompositingReason::kInlineTransform;
if ((!child_recursion_data.testing_overlap_ &&
!is_composited_clipping_layer) ||
layer->GetLayoutObject().StyleRef().HasCurrentTransformAnimation() ||
is_composited_with_inline_transform)
layer->GetLayoutObject().StyleRef().HasCurrentTransformAnimation())
current_recursion_data.testing_overlap_ = false;

if (child_recursion_data.compositing_ancestor_ == layer)
Expand Down
27 changes: 0 additions & 27 deletions third_party/blink/renderer/core/paint/compositing/compositing_requirements_updater_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,33 +76,6 @@ TEST_F(CompositingRequirementsUpdaterTest,
EXPECT_EQ(CompositingReason::kOverlap, target->GetCompositingReasons());
}

TEST_F(CompositingRequirementsUpdaterTest,
NoAssumedOverlapReasonForNonSelfPaintingLayer) {
SetBodyInnerHTML(R"HTML(
<style>
#target {
overflow: auto;
width: 100px;
height: 100px;
}
</style>
<div style="position: relative; width: 500px; height: 300px;
transform: translateZ(0)"></div>
<div id=target></div>
)HTML");

PaintLayer* target =
ToLayoutBoxModelObject(GetLayoutObjectByElementId("target"))->Layer();
EXPECT_FALSE(target->GetCompositingReasons());

// Now make |target| self-painting.
GetDocument().getElementById("target")->setAttribute(html_names::kStyleAttr,
"position: relative");
UpdateAllLifecyclePhasesForTest();
EXPECT_EQ(CompositingReason::kAssumedOverlap,
target->GetCompositingReasons());
}

TEST_F(CompositingRequirementsUpdaterTest,
NoDescendantReasonForNonSelfPaintingLayer) {
SetBodyInnerHTML(R"HTML(
Expand Down
11 changes: 2 additions & 9 deletions third_party/blink/renderer/core/paint/paint_layer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3064,15 +3064,8 @@ bool PaintLayer::AttemptDirectCompositingUpdate(
if (!rare_data_ || !rare_data_->composited_layer_mapping)
return false;

// To cut off almost all the work in the compositing update for
// this case, we treat inline transforms has having assumed overlap
// (similar to how we treat animated transforms). Notice that we read
// CompositingReasonInlineTransform from the m_compositingReasons, which
// means that the inline transform actually triggered assumed overlap in
// the overlap map.
if (diff.TransformChanged() &&
(!rare_data_ || !(rare_data_->compositing_reasons &
CompositingReason::kInlineTransform)))
// If a transform changed, we can't use the fast path.
if (diff.TransformChanged())
return false;

// We composite transparent Layers differently from non-transparent
Expand Down
3 changes: 0 additions & 3 deletions third_party/blink/renderer/platform/graphics/compositing_reasons.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,6 @@ constexpr CompositingReasonStringMap kCompositingReasonsStringMap[] = {
"with no scrolling contents"},
{CompositingReason::kLayerForDecoration, "layerForDecoration",
"Layer painted on top of other layers as decoration"},
{CompositingReason::kInlineTransform, "inlineTransform",
"Has an inline transform, which causes subsequent layers to assume "
"overlap"},

};

Expand Down
8 changes: 2 additions & 6 deletions third_party/blink/renderer/platform/graphics/compositing_reasons.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,7 @@ using CompositingReasons = uint64_t;
V(LayerForAncestorClippingMask) \
V(LayerForScrollingBlockSelection) \
/* Composited layer painted on top of all other layers as decoration. */ \
V(LayerForDecoration) \
\
/* Composited elements with inline transforms trigger assumed overlap so \
that we can update their transforms quickly. */ \
V(InlineTransform)
V(LayerForDecoration)

class PLATFORM_EXPORT CompositingReason {
private:
Expand Down Expand Up @@ -147,7 +143,7 @@ class PLATFORM_EXPORT CompositingReason {

kComboAllStyleDeterminedReasons = kComboAllDirectStyleDeterminedReasons |
kComboCompositedDescendants |
kCombo3DDescendants | kInlineTransform,
kCombo3DDescendants,

kComboSquashableReasons =
kOverlap | kAssumedOverlap | kOverflowScrollingParent,
Expand Down
1 change: 0 additions & 1 deletion third_party/blink/web_tests/SmokeTests
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ compositing/geometry/limit-layer-bounds-positioned-transition.html
compositing/gestures/gesture-tapHighlight-no-graphics-layer-region-based-multicol.html
compositing/iframes/visibility-hidden-transformed-content.html
compositing/images/clip-on-directly-composited-image.html
compositing/layer-creation/assumed-overlap-for-inline-transform.html
compositing/layer-creation/fixed-position-out-of-view-dynamic.html
compositing/layer-creation/fixed-position-out-of-view.html
compositing/reflections/empty-reflection-with-mask.html
Expand Down
10 changes: 5 additions & 5 deletions ...siting/gestures/gesture-tapHighlight-1-overflow-div-scrolled-late-composite-expected.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
<div style="transform: translateZ(0); position: relative; left: 10px; top: 10px"></div>
<div id="targetDiv" style="position: relative; left: 10px; top: 40px; width: 200px; height: 100px; overflow-y: scroll; overflow-x: scroll;">
<div id="divToForceCompositedLayer">
<a href="">Link 1</a><br>
<a href="">Link 2</a><br>
<a href="">Link 3</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
<a href="" class="activeLink" id="targetLink">Target Link.</a><br>
<a href="">Link 4</a><br>
<a href="">Link 5</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
</div></div>
<div style="position: relative; left: 10px; top: 80px">
This test is successful if "Target Link" above is covered in a green rectangle with square corners.
Expand Down
10 changes: 5 additions & 5 deletions ...sts/compositing/gestures/gesture-tapHighlight-1-overflow-div-scrolled-late-composite.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
<div style="transform: translateZ(0); position: relative; left: 10px; top: 10px"></div>
<div id="targetDiv" style="position: relative; left: 10px; top: 40px; width: 200px; height: 100px; overflow-y: scroll; overflow-x: scroll;">
<div id="divToForceCompositedLayer">
<a href="">Link 1</a><br>
<a href="">Link 2</a><br>
<a href="">Link 3</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
<a class="opaqueHighlight" href="" id="targetLink">Target Link.</a><br>
<a href="">Link 4</a><br>
<a href="">Link 5</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
</div></div>
<div style="position: relative; left: 10px; top: 80px">
This test is successful if "Target Link" above is covered in a green rectangle with square corners.
Expand Down
10 changes: 5 additions & 5 deletions ...ing/gestures/gesture-tapHighlight-1-overflow-div-scrolled-late-noncomposite-expected.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@
<div style="transform: translateZ(0); position: relative; left: 10px; top: 10px"></div>
<div id="targetDiv" style="position: relative; left: 10px; top: 40px; width: 200px; height: 100px; overflow-y: scroll; overflow-x: scroll;">
<div id="divToControlCompositedLayer" style="transform: translateZ(0);">
<a href="">Link 1</a><br>
<a href="">Link 2</a><br>
<a href="">Link 3</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
<span class="fauxHighlight" id="targetLink">Target Link.</span><br>
<a href="">Link 4</a><br>
<a href="">Link 5</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
</div></div>
<div style="position: relative; left: 10px; top: 80px">
This test is successful if "Target Link" above is covered in a green rectangle with square corners.
Expand Down
10 changes: 5 additions & 5 deletions .../compositing/gestures/gesture-tapHighlight-1-overflow-div-scrolled-late-noncomposite.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@
<div style="transform: translateZ(0); position: relative; left: 10px; top: 10px"></div>
<div id="targetDiv" style="position: relative; left: 10px; top: 40px; width: 200px; height: 100px; overflow-y: scroll; overflow-x: scroll;">
<div id="divToControlCompositedLayer" style="transform: translateZ(0);">
<a href="">Link 1</a><br>
<a href="">Link 2</a><br>
<a href="">Link 3</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
<a class="opaqueHighlight needsFix" href="" id="targetLink">Target Link.</a><br>
<a href="">Link 4</a><br>
<a href="">Link 5</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
</div></div>
<div style="position: relative; left: 10px; top: 80px">
This test is successful if "Target Link" above is covered in a green rectangle with square corners.
Expand Down
10 changes: 5 additions & 5 deletions ...compositing/gestures/gesture-tapHighlight-on-promoted-overflow-div-scrolled-expected.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
<body onload="runTest();">
<div style="transform: translateZ(0); position: relative; left: 10px; top: 10px"></div>
<div id="targetDiv" style="position: relative; left: 10px; top: 40px; width: 200px; height: 100px; overflow-y: scroll; overflow-x: scroll;">
<a href="">Link 1</a><br>
<a href="">Link 2</a><br>
<a href="">Link 3</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
<a href="" class="activeLink" id="targetLink">Target Link.</a><br>
<a href="">Link 4</a><br>
<a href="">Link 5</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
</div>
<div style="position: relative; left: 10px; top: 80px">
This test is successful if "Target Link" above is covered in a green
Expand Down
10 changes: 5 additions & 5 deletions ...eb_tests/compositing/gestures/gesture-tapHighlight-on-promoted-overflow-div-scrolled.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@
<body onload="runTest();">
<div style="transform: translateZ(0); position: relative; left: 10px; top: 10px"></div>
<div id="targetDiv" style="position: relative; left: 10px; top: 40px; width: 200px; height: 100px; overflow-y: scroll; overflow-x: scroll;">
<a href="">Link 1</a><br>
<a href="">Link 2</a><br>
<a href="">Link 3</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
<a class="opaqueHighlight" href="" id="targetLink">Target Link.</a><br>
<a href="">Link 4</a><br>
<a href="">Link 5</a><br>
<a href="">AAAAAA</a><br>
<a href="">AAAAAA</a><br>
</div>
<div style="position: relative; left: 10px; top: 80px">
This test is successful if "Target Link" above is covered in a green
Expand Down
16 changes: 0 additions & 16 deletions ...arty/blink/web_tests/compositing/layer-creation/assumed-overlap-for-inline-transform.html
View file

This file was deleted.

2 changes: 1 addition & 1 deletion ...d_party/blink/web_tests/compositing/overflow/avoid-ancestor-clip-for-scroll-children.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
<div class="translated">
<div class="scrolled"></div>
</div>
<div class="translated" style="transform: translate(0,430px)">
<div class="translated" style="transform: translate3D(0px,430px,0px)">
<div class="scrolled"></div>
</div>
</div>
Expand Down
14 changes: 1 addition & 13 deletions third_party/blink/web_tests/compositing/squashing/frame-clip-squashed-scrolled-expected.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,11 @@
"backgroundColor": "#FFFFFF",
"transform": 1
},
{
"name": "Squashing Containment Layer",
"drawsContent": false,
"transform": 1
},
{
"name": "LayoutBlockFlow DIV",
"contentsOpaque": true,
"drawsContent": false,
"transform": 2
},
{
"name": "Squashing Layer (first squashed layer: LayoutBlockFlow (relative positioned) DIV)",
"position": [8, 8],
"bounds": [102, 2002],
"transform": 1
}
],
"transforms": [
Expand All @@ -55,8 +44,7 @@
[0, 1, 0, 0],
[0, 0, 1, 0],
[8, 8, 0, 1]
],
"flattenInheritedTransform": false
]
}
]
}
Expand Down
12 changes: 1 addition & 11 deletions third_party/blink/web_tests/compositing/squashing/squash-onto-distant-relative-expected.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,12 @@
"contentsOpaque": true,
"backgroundColor": "#FFFFFF"
},
{
"name": "Squashing Containment Layer",
"drawsContent": false
},
{
"name": "LayoutBlockFlow (positioned) DIV id='squashing'",
"bounds": [1, 1],
"contentsOpaque": true,
"backgroundColor": "#FFFFFF",
"transform": 1
},
{
"name": "Squashing Layer (first squashed layer: LayoutBlockFlow (positioned) DIV id='squashed')",
"position": [600, 0],
"bounds": [200, 200]
}
],
"transforms": [
Expand All @@ -42,8 +33,7 @@
[0, 1, 0, 0],
[0, 0, 1, 0],
[-33554430, 0, 0, 1]
],
"flattenInheritedTransform": false
]
}
]
}
Expand Down
9 changes: 0 additions & 9 deletions third_party/blink/web_tests/compositing/squashing/squashing-inside-perspective-expected.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,21 +28,12 @@
"drawsContent": false,
"transform": 2
},
{
"name": "Squashing Containment Layer",
"drawsContent": false,
"transform": 2
},
{
"name": "LayoutBlockFlow (positioned) DIV",
"bounds": [200, 200],
"contentsOpaque": true,
"backgroundColor": "#00008B",
"transform": 3
},
{
"name": "Squashing Layer (first squashed layer: LayoutBlockFlow (positioned) DIV)",
"transform": 2
}
],
"transforms": [
Expand Down
Binary file modified ...link/web_tests/css3/blending/effect-background-blend-mode-stacking-expected.png
View file
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion ...rty/blink/web_tests/paint/invalidation/compositing/repaint-via-layout-offset-expected.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"transform": 1
},
{
"name": "Squashing Layer (first squashed layer: LayoutBlockFlow (positioned) DIV id='container')",
"name": "Squashing Layer (first squashed layer: LayoutBlockFlow (positioned) SPAN class='child')",
"position": [50, 50],
"bounds": [50, 50],
"paintInvalidations": [
Expand Down
17 changes: 0 additions & 17 deletions ...ests/paint/invalidation/scroll/overflow-scroll-composited-non-stacking-child-expected.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,23 +40,6 @@
"contentsOpaque": true,
"backgroundColor": "#FFDDBB",
"transform": 1
},
{
"name": "Ancestor Clipping Layer",
"position": [23, 65],
"bounds": [285, 175],
"drawsContent": false
},
{
"name": "LayoutBlockFlow (relative positioned) DIV class='list'",
"position": [43, 35],
"bounds": [180, 250],
"drawsContent": false
},
{
"name": "Squashing Layer (first squashed layer: LayoutBlockFlow (relative positioned) DIV class='commit')",
"position": [43, 35],
"bounds": [180, 250]
}
],
"transforms": [
Expand Down

0 comments on commit 58bbcb0

Please sign in to comment.