[BGPT] Don't collect non-animation non-scroll element ids
Collecting non-animation element ids gave compositor wrong information
that they need to be handled by the compositor, causing non-composited
animation (caused by will-change: contents) on composited-layer failed
to start in BGPT mode.
Also add virtual/disable-blink-gen-property-trees/transitions.
Bug: 936912
Change-Id: I3ff64a732920ef884d838064d2d3a005e8a50860
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1512765
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#641429}diff --git a/third_party/blink/renderer/core/paint/paint_property_tree_builder_test.cc b/third_party/blink/renderer/core/paint/paint_property_tree_builder_test.cc
index 03722e9..283b59f 100644
--- a/third_party/blink/renderer/core/paint/paint_property_tree_builder_test.cc
+++ b/third_party/blink/renderer/core/paint/paint_property_tree_builder_test.cc
@@ -686,7 +686,8 @@
TEST_P(PaintPropertyTreeBuilderTest,
TransformNodeWithActiveAnimationHasDirectCompositingReason) {
- if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
+ if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled() &&
+ !RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled())
return;
LoadTestData("transform-animation.html");
@@ -707,7 +708,8 @@
TEST_P(PaintPropertyTreeBuilderTest,
EffectNodeWithActiveAnimationHasDirectCompositingReason) {
- if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
+ if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled() &&
+ !RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled())
return;
LoadTestData("opacity-animation.html");
@@ -4857,7 +4859,8 @@
TEST_P(PaintPropertyTreeBuilderTest,
TransformNodeNotAnimatedStillHasCompositorElementId) {
- if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
+ if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled() &&
+ !RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled())
return;
SetBodyInnerHTML("<div id='target' style='transform: translateX(2em)'></div");
@@ -4869,7 +4872,8 @@
TEST_P(PaintPropertyTreeBuilderTest,
EffectNodeNotAnimatedStillHasCompositorElementId) {
- if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
+ if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled() &&
+ !RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled())
return;
SetBodyInnerHTML("<div id='target' style='opacity: 0.5'></div");
@@ -4884,7 +4888,8 @@
TEST_P(PaintPropertyTreeBuilderTest,
TransformNodeAnimatedHasCompositorElementId) {
- if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
+ if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled() &&
+ !RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled())
return;
LoadTestData("transform-animation.html");
@@ -4896,7 +4901,8 @@
}
TEST_P(PaintPropertyTreeBuilderTest, EffectNodeAnimatedHasCompositorElementId) {
- if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
+ if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled() &&
+ !RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled())
return;
LoadTestData("opacity-animation.html");
@@ -4935,7 +4941,8 @@
}
TEST_P(PaintPropertyTreeBuilderTest, ScrollNodeHasCompositorElementId) {
- if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
+ if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled() &&
+ !RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled())
return;
SetBodyInnerHTML(R"HTML(
@@ -5645,7 +5652,8 @@
)HTML");
// When the root scrolls, there should be direct compositing reasons.
- if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
+ if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled() ||
+ RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled())
EXPECT_TRUE(DocScrollTranslation()->HasDirectCompositingReasons());
// Remove scrolling from the root.
@@ -5673,7 +5681,8 @@
)HTML");
UpdateAllLifecyclePhasesForTest();
- if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
+ if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled() ||
+ RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled())
EXPECT_TRUE(DocScrollTranslation()->HasDirectCompositingReasons());
// When the child iframe scrolls, there should not be direct compositing
diff --git a/third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor.cc b/third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor.cc
index 5b39453..cd76d1c1 100644
--- a/third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor.cc
+++ b/third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor.cc
@@ -35,15 +35,6 @@
namespace {
-// Returns true if the given element namespace is one of the ones needed for
-// running animations on the compositor. These are the only element_ids the
-// compositor needs to track existence of in the element id set.
-bool IsAnimationNamespace(CompositorElementIdNamespace element_namespace) {
- return element_namespace == CompositorElementIdNamespace::kPrimaryTransform ||
- element_namespace == CompositorElementIdNamespace::kPrimaryEffect ||
- element_namespace == CompositorElementIdNamespace::kEffectFilter;
-}
-
// Inserts the element ids of the given node and all of its ancestors into the
// given |composited_element_ids| set. Filters out specifically element ids
// which are needed for animations. Returns once it finds an id which already
@@ -55,8 +46,7 @@
CompositorElementIdSet& composited_element_ids) {
for (const auto* n = &node; n; n = SafeUnalias(n->Parent())) {
const CompositorElementId& element_id = n->GetCompositorElementId();
- if (element_id &&
- IsAnimationNamespace(NamespaceFromCompositorElementId(element_id))) {
+ if (element_id && n->RequiresCompositingForAnimation()) {
if (composited_element_ids.count(element_id)) {
// Once we reach a node already counted we can stop traversing the
// parent chain.
diff --git a/third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor_test.cc b/third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor_test.cc
index d33c2291..1346f66 100644
--- a/third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor_test.cc
+++ b/third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor_test.cc
@@ -933,14 +933,14 @@
}
TEST_P(PaintArtifactCompositorTest, EffectTreeConversionWithAlias) {
- EffectPaintPropertyNode::State effect1_state;
- effect1_state.local_transform_space = &t0();
- effect1_state.output_clip = &c0();
- effect1_state.opacity = 0.5;
- effect1_state.direct_compositing_reasons = CompositingReason::kAll;
- effect1_state.compositor_element_id = CompositorElementId(2);
+ Update(TestPaintArtifact()
+ .Chunk()
+ .RectDrawing(FloatRect(0, 0, 100, 100), Color::kWhite)
+ .Build());
+ auto root_stable_id = GetPropertyTrees().effect_tree.Node(1)->stable_id;
+
auto real_effect1 =
- EffectPaintPropertyNode::Create(e0(), std::move(effect1_state));
+ CreateOpacityEffect(e0(), t0(), &c0(), 0.5, CompositingReason::kAll);
auto effect1 = EffectPaintPropertyNode::CreateAlias(*real_effect1);
auto real_effect2 =
CreateOpacityEffect(*effect1, 0.3, CompositingReason::kAll);
@@ -960,20 +960,19 @@
ASSERT_EQ(3u, ContentLayerCount());
const cc::EffectTree& effect_tree = GetPropertyTrees().effect_tree;
- // Node #0 reserved for null; #1 for root render surface; #2 for
- // e0(), plus 3 nodes for those created by
- // this test.
+ // Node #0 reserved for null; #1 for root render surface; #2 for e0(),
+ // plus 3 nodes for those created by this test.
ASSERT_EQ(5u, effect_tree.size());
const cc::EffectNode& converted_root_effect = *effect_tree.Node(1);
EXPECT_EQ(-1, converted_root_effect.parent_id);
- EXPECT_EQ(CompositorElementIdFromUniqueObjectId(1).GetInternalValue(),
- converted_root_effect.stable_id);
+ EXPECT_EQ(root_stable_id, converted_root_effect.stable_id);
const cc::EffectNode& converted_effect1 = *effect_tree.Node(2);
EXPECT_EQ(converted_root_effect.id, converted_effect1.parent_id);
EXPECT_FLOAT_EQ(0.5, converted_effect1.opacity);
- EXPECT_EQ(2u, converted_effect1.stable_id);
+ EXPECT_EQ(real_effect1->GetCompositorElementId().GetInternalValue(),
+ converted_effect1.stable_id);
const cc::EffectNode& converted_effect2 = *effect_tree.Node(3);
EXPECT_EQ(converted_effect1.id, converted_effect2.parent_id);
@@ -2348,8 +2347,8 @@
}
TEST_P(PaintArtifactCompositorTest, UpdatePopulatesCompositedElementIds) {
- auto transform = CreateSampleTransformNodeWithElementId();
- auto effect = CreateSampleEffectNodeWithElementId();
+ auto transform = CreateAnimatingTransform(t0());
+ auto effect = CreateAnimatingOpacityEffect(e0());
TestPaintArtifact artifact;
artifact.Chunk(*transform, c0(), e0())
.RectDrawing(FloatRect(0, 0, 100, 100), Color::kBlack)
@@ -2369,35 +2368,26 @@
// included in the composited element id set returned from
// |PaintArtifactCompositor::Update(...)|.
TEST_P(PaintArtifactCompositorTest, UniqueAnimationCompositedElementIds) {
- TransformPaintPropertyNode::State transform_state;
- transform_state.direct_compositing_reasons =
- CompositingReason::kActiveTransformAnimation;
- transform_state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
- 31, CompositorElementIdNamespace::kPrimaryTransform);
- auto transform =
- TransformPaintPropertyNode::Create(t0(), std::move(transform_state));
-
- EffectPaintPropertyNode::State effect_state;
- effect_state.local_transform_space = transform;
- effect_state.output_clip = &c0();
- effect_state.opacity = 2.0 / 255.0;
- effect_state.direct_compositing_reasons =
- CompositingReason::kActiveOpacityAnimation;
- effect_state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
- 41, CompositorElementIdNamespace::kPrimaryEffect);
- auto effect = EffectPaintPropertyNode::Create(e0(), std::move(effect_state));
-
- TestPaintArtifact artifact;
- artifact.Chunk(*transform, c0(), *effect)
- .RectDrawing(FloatRect(0, 0, 100, 100), Color::kBlack);
+ auto animating_transform = CreateAnimatingTransform(t0());
+ auto non_animating_transform = CreateTransform(
+ *animating_transform, TransformationMatrix().Translate(10, 20));
+ auto animating_effect = CreateAnimatingOpacityEffect(e0());
+ auto non_animating_effect = CreateOpacityEffect(*animating_effect, 0.5f);
CompositorElementIdSet composited_element_ids;
- Update(artifact.Build(), composited_element_ids);
+ Update(TestPaintArtifact()
+ .Chunk(*animating_transform, c0(), *animating_effect)
+ .RectDrawing(FloatRect(0, 0, 100, 100), Color::kBlack)
+ .Chunk(*non_animating_transform, c0(), *non_animating_effect)
+ .RectDrawing(FloatRect(0, 0, 100, 100), Color::kBlack)
+ .Build(),
+ composited_element_ids);
EXPECT_EQ(2u, composited_element_ids.size());
- EXPECT_TRUE(
- composited_element_ids.count(transform->GetCompositorElementId()));
- EXPECT_TRUE(composited_element_ids.count(effect->GetCompositorElementId()));
+ EXPECT_EQ(1u, composited_element_ids.count(
+ animating_transform->GetCompositorElementId()));
+ EXPECT_EQ(1u, composited_element_ids.count(
+ animating_effect->GetCompositorElementId()));
}
TEST_P(PaintArtifactCompositorTest, SkipChunkWithOpacityZero) {
@@ -2551,7 +2541,7 @@
}
TEST_P(PaintArtifactCompositorTest, UpdateManagesLayerElementIds) {
- auto transform = CreateSampleTransformNodeWithElementId();
+ auto transform = CreateAnimatingTransform(t0());
CompositorElementId element_id = transform->GetCompositorElementId();
{
diff --git a/third_party/blink/renderer/platform/graphics/paint/effect_paint_property_node.h b/third_party/blink/renderer/platform/graphics/paint/effect_paint_property_node.h
index 27d6210a..ed45226 100644
--- a/third_party/blink/renderer/platform/graphics/paint/effect_paint_property_node.h
+++ b/third_party/blink/renderer/platform/graphics/paint/effect_paint_property_node.h
@@ -167,6 +167,12 @@
return DirectCompositingReasons() != CompositingReason::kNone;
}
+ // TODO(crbug.com/900241): Use HaveActiveXXXAnimation() instead of this
+ // function when we can track animations for each property type.
+ bool RequiresCompositingForAnimation() const {
+ return DirectCompositingReasons() &
+ CompositingReason::kComboActiveAnimation;
+ }
bool HasActiveOpacityAnimation() const {
return DirectCompositingReasons() &
CompositingReason::kActiveOpacityAnimation;
diff --git a/third_party/blink/renderer/platform/graphics/paint/transform_paint_property_node.h b/third_party/blink/renderer/platform/graphics/paint/transform_paint_property_node.h
index 9e0b1f8..be0a19f 100644
--- a/third_party/blink/renderer/platform/graphics/paint/transform_paint_property_node.h
+++ b/third_party/blink/renderer/platform/graphics/paint/transform_paint_property_node.h
@@ -251,6 +251,12 @@
return DirectCompositingReasons() != CompositingReason::kNone;
}
+ // TODO(crbug.com/900241): Use HaveActiveTransformAnimation() instead of this
+ // function when we can track animations for each property type.
+ bool RequiresCompositingForAnimation() const {
+ return DirectCompositingReasons() &
+ CompositingReason::kComboActiveAnimation;
+ }
bool HasActiveTransformAnimation() const {
return DirectCompositingReasons() &
CompositingReason::kActiveTransformAnimation;
diff --git a/third_party/blink/renderer/platform/testing/paint_property_test_helpers.h b/third_party/blink/renderer/platform/testing/paint_property_test_helpers.h
index ba98245..e91bc24 100644
--- a/third_party/blink/renderer/platform/testing/paint_property_test_helpers.h
+++ b/third_party/blink/renderer/platform/testing/paint_property_test_helpers.h
@@ -34,6 +34,8 @@
state.output_clip = output_clip;
state.opacity = opacity;
state.direct_compositing_reasons = compositing_reasons;
+ state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
+ NewUniqueObjectId(), CompositorElementIdNamespace::kPrimary);
return EffectPaintPropertyNode::Create(parent, std::move(state));
}
@@ -55,6 +57,8 @@
state.output_clip = output_clip;
state.opacity = opacity;
state.direct_compositing_reasons = CompositingReason::kActiveOpacityAnimation;
+ state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
+ NewUniqueObjectId(), CompositorElementIdNamespace::kPrimaryEffect);
return EffectPaintPropertyNode::Create(parent, std::move(state));
}
@@ -71,6 +75,8 @@
state.filter = std::move(filter);
state.filters_origin = filters_origin;
state.direct_compositing_reasons = compositing_reasons;
+ state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
+ NewUniqueObjectId(), CompositorElementIdNamespace::kEffectFilter);
return EffectPaintPropertyNode::Create(parent, std::move(state));
}
@@ -93,6 +99,8 @@
state.output_clip = output_clip;
state.filter = std::move(filter);
state.direct_compositing_reasons = CompositingReason::kActiveFilterAnimation;
+ state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
+ NewUniqueObjectId(), CompositorElementIdNamespace::kEffectFilter);
return EffectPaintPropertyNode::Create(parent, std::move(state));
}
@@ -109,6 +117,8 @@
state.backdrop_filter = std::move(backdrop_filter);
state.filters_origin = filters_origin;
state.direct_compositing_reasons = compositing_reasons;
+ state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
+ NewUniqueObjectId(), CompositorElementIdNamespace::kPrimary);
return EffectPaintPropertyNode::Create(parent, std::move(state));
}
@@ -134,6 +144,8 @@
state.backdrop_filter = std::move(backdrop_filter);
state.direct_compositing_reasons =
CompositingReason::kActiveBackdropFilterAnimation;
+ state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
+ NewUniqueObjectId(), CompositorElementIdNamespace::kPrimaryEffect);
return EffectPaintPropertyNode::Create(parent, std::move(state));
}
@@ -179,6 +191,19 @@
return TransformPaintPropertyNode::Create(parent, std::move(state));
}
+inline scoped_refptr<TransformPaintPropertyNode> CreateAnimatingTransform(
+ const TransformPaintPropertyNode& parent,
+ const TransformationMatrix& matrix = TransformationMatrix(),
+ const FloatPoint3D& origin = FloatPoint3D()) {
+ TransformPaintPropertyNode::State state{
+ TransformPaintPropertyNode::TransformAndOrigin(matrix, origin)};
+ state.direct_compositing_reasons =
+ CompositingReason::kActiveTransformAnimation;
+ state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
+ NewUniqueObjectId(), CompositorElementIdNamespace::kPrimaryTransform);
+ return TransformPaintPropertyNode::Create(parent, std::move(state));
+}
+
inline scoped_refptr<TransformPaintPropertyNode> CreateScrollTranslation(
const TransformPaintPropertyNode& parent,
float offset_x,
diff --git a/third_party/blink/web_tests/VirtualTestSuites b/third_party/blink/web_tests/VirtualTestSuites
index 97c5dee..8963d3e 100644
--- a/third_party/blink/web_tests/VirtualTestSuites
+++ b/third_party/blink/web_tests/VirtualTestSuites
@@ -127,6 +127,12 @@
},
{
"prefix": "disable-blink-gen-property-trees",
+ "base": "transitions",
+ "args": ["--disable-blink-features=BlinkGenPropertyTrees",
+ "--enable-threaded-compositing"]
+ },
+ {
+ "prefix": "disable-blink-gen-property-trees",
"base": "compositing",
"args": ["--disable-blink-features=BlinkGenPropertyTrees"]
},
diff --git a/third_party/blink/web_tests/transitions/transition-3d-transform-will-change-contents-expected.html b/third_party/blink/web_tests/transitions/transition-3d-transform-will-change-contents-expected.html
new file mode 100644
index 0000000..0fbfa749
--- /dev/null
+++ b/third_party/blink/web_tests/transitions/transition-3d-transform-will-change-contents-expected.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<style>
+#target {
+ width: 400px;
+ height: 100px;
+ background: blue;
+ position: relative;
+ left: 20px;
+}
+</style>
+<div id="target"></div>
diff --git a/third_party/blink/web_tests/transitions/transition-3d-transform-will-change-contents.html b/third_party/blink/web_tests/transitions/transition-3d-transform-will-change-contents.html
new file mode 100644
index 0000000..bb168e9
--- /dev/null
+++ b/third_party/blink/web_tests/transitions/transition-3d-transform-will-change-contents.html
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<style>
+#target {
+ width:100px;
+ height: 100px;
+ background: blue;
+ transition-duration: 0.1s;
+ transform: translate3d(10px, 0, 0);
+ will-change: contents;
+}
+#target.changed {
+ width: 400px;
+ transform: translate3d(20px, 0, 0);
+}
+</style>
+<div id="target"></div>
+<script src="../resources/run-after-layout-and-paint.js"></script>
+<script>
+if (window.testRunner)
+ testRunner.waitUntilDone();
+runAfterLayoutAndPaint(() => {
+ target.addEventListener('transitionend', () => {
+ if (window.testRunner)
+ testRunner.notifyDone();
+ });
+ target.classList.add('changed');
+});
+</script>
diff --git a/third_party/blink/web_tests/virtual/disable-blink-gen-property-trees/transitions/README.txt b/third_party/blink/web_tests/virtual/disable-blink-gen-property-trees/transitions/README.txt
new file mode 100644
index 0000000..ef8bdb24
--- /dev/null
+++ b/third_party/blink/web_tests/virtual/disable-blink-gen-property-trees/transitions/README.txt
@@ -0,0 +1,2 @@
+# This suite runs tests with --disable-blink-features=BlinkGenPropertyTrees
+# and --enable-threaded-compositing.
diff --git a/third_party/blink/web_tests/virtual/disable-blink-gen-property-trees/transitions/opacity-transform-transitions-inside-iframe-expected.png b/third_party/blink/web_tests/virtual/disable-blink-gen-property-trees/transitions/opacity-transform-transitions-inside-iframe-expected.png
new file mode 100644
index 0000000..0cc6e8a
--- /dev/null
+++ b/third_party/blink/web_tests/virtual/disable-blink-gen-property-trees/transitions/opacity-transform-transitions-inside-iframe-expected.png
Binary files differ
