Commit 1c5b9bab authored by Philip Rogers's avatar Philip Rogers Committed by Commit Bot

Composite will-change: filter

This patch makes will-change: filter a compositing reason. The approach
is similar to will-change: transform where filter changes do not cause
DiffNeedsFullLayoutAndPaintInvalidation (see change in
computed_style_diff_functions.json5). LBMO::StyleDidChange is
used to invalidate when the will-change property changes.

Bug: 960953
Change-Id: I7600c141baf11d1e10fae229a6261926c44923fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2145466
Commit-Queue: Philip Rogers <pdr@chromium.org>
Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758501}
parent 22f96f3f
......@@ -261,6 +261,7 @@ DISABLE_CFI_PERF
void LayoutBoxModelObject::StyleDidChange(StyleDifference diff,
const ComputedStyle* old_style) {
bool had_transform_related_property = HasTransformRelatedProperty();
bool had_filter_inducing_property = HasFilterInducingProperty();
bool had_layer = HasLayer();
bool layer_was_self_painting = had_layer && Layer()->IsSelfPaintingLayer();
bool was_horizontal_writing_mode = IsHorizontalWritingMode();
......@@ -314,7 +315,7 @@ void LayoutBoxModelObject::StyleDidChange(StyleDifference diff,
Layer()->RemoveOnlyThisLayerAfterStyleChange(old_style);
if (EverHadLayout())
SetChildNeedsLayout();
if (had_transform_related_property) {
if (had_transform_related_property || had_filter_inducing_property) {
SetNeedsLayoutAndIntrinsicWidthsRecalcAndFullPaintInvalidation(
layout_invalidation_reason::kStyleChange);
}
......@@ -335,9 +336,10 @@ void LayoutBoxModelObject::StyleDidChange(StyleDifference diff,
AddSubtreePaintPropertyUpdateReason(
SubtreePaintPropertyUpdateReason::kContainerChainMayChange);
} else if (had_layer == HasLayer() &&
had_transform_related_property != HasTransformRelatedProperty()) {
// This affects whether to create transform node. Note that if the
// HasLayer() value changed, then all of this was already set in
(had_transform_related_property != HasTransformRelatedProperty() ||
had_filter_inducing_property != HasFilterInducingProperty())) {
// This affects whether to create transform or filter nodes. Note that if
// the HasLayer() value changed, then all of this was already set in
// CreateLayerAfterStyleChange() or DestroyLayer().
SetNeedsPaintPropertyUpdate();
if (Layer())
......
......@@ -1173,6 +1173,71 @@ TEST_F(LayoutBoxModelObjectTest, BackfaceVisibilityChange) {
EXPECT_FALSE(target_layer->SelfNeedsRepaint());
}
TEST_F(LayoutBoxModelObjectTest, ChangingFilterWithWillChange) {
SetBodyInnerHTML(R"HTML(
<style>
#target {
width: 100px;
height: 100px;
will-change: filter;
}
</style>
<div id="target"></div>
)HTML");
// Adding a filter should not need to check for paint invalidation because
// will-change: filter is present.
auto* target = GetDocument().getElementById("target");
target->setAttribute(html_names::kStyleAttr, "filter: grayscale(1)");
GetDocument().UpdateStyleAndLayoutTree();
EXPECT_FALSE(target->GetLayoutObject()->ShouldCheckForPaintInvalidation());
UpdateAllLifecyclePhasesForTest();
EXPECT_FALSE(target->GetLayoutObject()->ShouldCheckForPaintInvalidation());
// Removing a filter should not need to check for paint invalidation because
// will-change: filter is present.
target->removeAttribute(html_names::kStyleAttr);
GetDocument().UpdateStyleAndLayoutTree();
EXPECT_FALSE(target->GetLayoutObject()->ShouldCheckForPaintInvalidation());
UpdateAllLifecyclePhasesForTest();
EXPECT_FALSE(target->GetLayoutObject()->ShouldCheckForPaintInvalidation());
}
TEST_F(LayoutBoxModelObjectTest, ChangingWillChangeFilter) {
SetBodyInnerHTML(R"HTML(
<style>
.willChange {
will-change: filter;
}
#filter {
width: 100px;
height: 100px;
}
</style>
<div id="target"></div>
)HTML");
// Adding will-change: filter should check for paint invalidation and create
// a PaintLayer.
auto* target = GetDocument().getElementById("target");
target->classList().Add("willChange");
GetDocument().UpdateStyleAndLayoutTree();
EXPECT_TRUE(target->GetLayoutObject()->ShouldCheckForPaintInvalidation());
EXPECT_TRUE(ToLayoutBoxModelObject(target->GetLayoutObject())->Layer());
// A lifecycle update should clear dirty bits.
UpdateAllLifecyclePhasesForTest();
EXPECT_FALSE(target->GetLayoutObject()->ShouldCheckForPaintInvalidation());
EXPECT_TRUE(ToLayoutBoxModelObject(target->GetLayoutObject())->Layer());
// Removing will-change: filter should check for paint invalidation and remove
// the PaintLayer.
target->classList().Remove("willChange");
GetDocument().UpdateStyleAndLayoutTree();
EXPECT_TRUE(target->GetLayoutObject()->ShouldCheckForPaintInvalidation());
EXPECT_FALSE(ToLayoutBoxModelObject(target->GetLayoutObject())->Layer());
}
TEST_F(LayoutBoxModelObjectTest, UpdateStackingContextForOption) {
// We do not create LayoutObject for option elements inside multiple selects
// on platforms where DelegatesMenuListRendering() returns true like Android.
......
......@@ -285,9 +285,11 @@ CompositingReasons CompositingReasonFinder::CompositingReasonsForWillChange(
reasons |= CompositingReason::kWillChangeTransform;
if (style.HasWillChangeOpacityHint())
reasons |= CompositingReason::kWillChangeOpacity;
if (style.HasWillChangeFilterHint())
reasons |= CompositingReason::kWillChangeFilter;
// kWillChangeOther is needed only when neither kWillChangeTransform nor
// kWillChangeOpacity is set.
// kWillChangeOther is needed only when none of the explicit kWillChange*
// reasons are set.
if (reasons == CompositingReason::kNone &&
style.HasWillChangeCompositingHint())
reasons |= CompositingReason::kWillChangeOther;
......
......@@ -702,11 +702,11 @@ static CompositingReasons CompositingReasonsForTransformProperty() {
// property instead of creating all nodes and only create a transform/
// effect/filter node if needed.
reasons |= CompositingReason::kComboActiveAnimation;
// We also need to create transform node if the opacity node is created for
// will-change:opacity to avoid raster invalidation (caused by otherwise a
// created/deleted effect node) when we start/stop an opacity animation.
// https://crbug.com/942681
// We also need to create a transform node if will-change creates other nodes,
// to avoid raster invalidation caused by creating/deleting those nodes when
// starting/stopping an animation. See: https://crbug.com/942681.
reasons |= CompositingReason::kWillChangeOpacity;
reasons |= CompositingReason::kWillChangeFilter;
return reasons;
}
......@@ -875,13 +875,13 @@ static CompositingReasons CompositingReasonsForEffectProperty() {
// property instead of creating all nodes and only create a transform/
// effect/filter node if needed.
reasons |= CompositingReason::kComboActiveAnimation;
// We also need to create effect node if the transform node is created for
// will-change:transform to avoid raster invalidation (caused by otherwise a
// created/deleted effect node) when we start/stop a transform animation.
// https://crbug.com/942681
// We also need to create an effect node if will-change creates other nodes,
// to avoid raster invalidation caused by creating/deleting those nodes when
// starting/stopping an animation. See: https://crbug.com/942681.
// In CompositeAfterPaint, this also avoids decomposition of the effect when
// the object is forced compositing with will-change:transform.
reasons |= CompositingReason::kWillChangeTransform;
reasons |= CompositingReason::kWillChangeFilter;
return reasons;
}
......@@ -1197,12 +1197,12 @@ static CompositingReasons CompositingReasonsForFilterProperty() {
// property instead of creating all nodes and only create a transform/
// effect/filter node if needed.
reasons |= CompositingReason::kComboActiveAnimation;
// We also need to create filter node if the transform/effect node is
// created for will-change:transform/opacity to avoid raster invalidation
// (caused by otherwise a created/deleted filter node) when we start/stop a
// transform/opacity animation. https://crbug.com/942681
// We also need to create a filter node if will-change creates other nodes,
// to avoid raster invalidation caused by creating/deleting those nodes when
// starting/stopping an animation. See: https://crbug.com/942681.
// In CompositeAfterPaint, this also avoids decomposition of the filter when
// the object is forced compositing with will-change:transform/opacity.
// the object is forced compositing with will-change.
reasons |= CompositingReason::kWillChangeTransform |
CompositingReason::kWillChangeOpacity;
return reasons;
......
......@@ -1102,6 +1102,8 @@ static bool IsWillChangeCompositingHintProperty(CSSPropertyID property) {
return true;
switch (property) {
case CSSPropertyID::kOpacity:
case CSSPropertyID::kFilter:
case CSSPropertyID::kAliasWebkitFilter:
case CSSPropertyID::kTop:
case CSSPropertyID::kLeft:
case CSSPropertyID::kBottom:
......
......@@ -129,10 +129,6 @@
method: "BorderRightWidth()",
field_dependencies: ["border-right-width"]
},
{
method: "HasFilters()",
field_dependencies: ["filter"]
},
{
method: "HasPseudoElementStyle(kPseudoIdScrollbar)",
field_dependencies: ["StyleType"]
......
......@@ -52,6 +52,8 @@ constexpr CompositingReasonStringMap kCompositingReasonsStringMap[] = {
"Has a will-change: transform compositing hint"},
{CompositingReason::kWillChangeOpacity, "willChangeOpacity",
"Has a will-change: opacity compositing hint"},
{CompositingReason::kWillChangeFilter, "willChangeFilter",
"Has a will-change: filter compositing hint"},
{CompositingReason::kWillChangeOther, "willChangeOther",
"Has a will-change compositing hint other than transform and opacity"},
{CompositingReason::kBackdropFilter, "backdropFilter",
......
......@@ -34,8 +34,9 @@ using CompositingReasons = uint64_t;
V(VideoOverlay) \
V(WillChangeTransform) \
V(WillChangeOpacity) \
/* This flag is needed only when neither kWillChangeTransform nor \
kWillChangeOpacity is set */ \
V(WillChangeFilter) \
/* This flag is needed only when none of the explicit kWillChange* reasons \
are set. */ \
V(WillChangeOther) \
V(BackdropFilter) \
V(RootScroller) \
......@@ -118,8 +119,8 @@ class PLATFORM_EXPORT CompositingReason {
kComboAllDirectStyleDeterminedReasons =
k3DTransform | kBackfaceVisibilityHidden | kComboActiveAnimation |
kWillChangeTransform | kWillChangeOpacity | kWillChangeOther |
kBackdropFilter,
kWillChangeTransform | kWillChangeOpacity | kWillChangeFilter |
kWillChangeOther | kBackdropFilter,
kComboAllDirectNonStyleDeterminedReasons =
kVideo | kCanvas | kPlugin | kIFrame | kOverflowScrollingParent |
......@@ -160,7 +161,8 @@ class PLATFORM_EXPORT CompositingReason {
kDirectReasonsForEffectProperty = kActiveOpacityAnimation |
kWillChangeOpacity | kBackdropFilter |
kActiveBackdropFilterAnimation,
kDirectReasonsForFilterProperty = kActiveFilterAnimation,
kDirectReasonsForFilterProperty =
kActiveFilterAnimation | kWillChangeFilter,
};
};
......
......@@ -49,11 +49,18 @@
"transform": 6
},
{
"name": "LayoutNGBlockFlow DIV id='willChangeCombinationThatComposites' class='shouldComposite'",
"name": "LayoutNGBlockFlow DIV id='willChangeFilter' class='shouldComposite'",
"bounds": [30, 30],
"contentsOpaque": true,
"backgroundColor": "#008000",
"transform": 7
},
{
"name": "LayoutNGBlockFlow DIV id='willChangeCombinationThatComposites' class='shouldComposite'",
"bounds": [30, 30],
"contentsOpaque": true,
"backgroundColor": "#008000",
"transform": 8
}
],
"transforms": [
......@@ -113,6 +120,15 @@
},
{
"id": 7,
"transform": [
[1, 0, 0, 0],
[0, 1, 0, 0],
[0, 0, 1, 0],
[12, 382, 0, 1]
]
},
{
"id": 8,
"transform": [
[1, 0, 0, 0],
[0, 1, 0, 0],
......
......@@ -82,7 +82,7 @@
<div id="willChangeTop" class="positioned shouldComposite"></div>
<div id="willChangeBottom" class="shouldNotComposite"></div>
<div id="willChangeBottom" class="positioned shouldComposite"></div>
<div id="willChangeFilter" class="shouldNotComposite"></div>
<div id="willChangeFilter" class="shouldComposite"></div>
<div id="willChangeCombinationThatComposites" class="shouldComposite"></div>
<div id="willChangeZIndex" class="shouldNotComposite"></div>
......
......@@ -49,11 +49,18 @@
"transform": 6
},
{
"name": "LayoutNGBlockFlow DIV id='willChangeCombinationThatComposites' class='shouldComposite'",
"name": "LayoutNGBlockFlow DIV id='willChangeFilter' class='shouldComposite'",
"bounds": [30, 30],
"contentsOpaque": true,
"backgroundColor": "#008000",
"transform": 7
},
{
"name": "LayoutNGBlockFlow DIV id='willChangeCombinationThatComposites' class='shouldComposite'",
"bounds": [30, 30],
"contentsOpaque": true,
"backgroundColor": "#008000",
"transform": 8
}
],
"transforms": [
......@@ -113,6 +120,15 @@
},
{
"id": 7,
"transform": [
[1, 0, 0, 0],
[0, 1, 0, 0],
[0, 0, 1, 0],
[12, 382, 0, 1]
]
},
{
"id": 8,
"transform": [
[1, 0, 0, 0],
[0, 1, 0, 0],
......
......@@ -49,11 +49,18 @@
"transform": 6
},
{
"name": "LayoutBlockFlow DIV id='willChangeCombinationThatComposites' class='shouldComposite'",
"name": "LayoutBlockFlow DIV id='willChangeFilter' class='shouldComposite'",
"bounds": [30, 30],
"contentsOpaque": true,
"backgroundColor": "#008000",
"transform": 7
},
{
"name": "LayoutBlockFlow DIV id='willChangeCombinationThatComposites' class='shouldComposite'",
"bounds": [30, 30],
"contentsOpaque": true,
"backgroundColor": "#008000",
"transform": 8
}
],
"transforms": [
......@@ -113,6 +120,15 @@
},
{
"id": 7,
"transform": [
[1, 0, 0, 0],
[0, 1, 0, 0],
[0, 0, 1, 0],
[12, 382, 0, 1]
]
},
{
"id": 8,
"transform": [
[1, 0, 0, 0],
[0, 1, 0, 0],
......
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