Commit 3dc40bd2 authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot

[BGPT] Don't create render surface for extra effect nodes for animation

For now we create both effect and filter nodes when there is any type of
effect animation. Previously we created render surface for both nodes,
causing memory and CPU regressions. Now don't create render surface for
the extra nodes.

Bug: 943826
Change-Id: Id26088b1cd58d95f44b040b099f56bd2fb1c335e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1536706Reviewed-by: default avatarRobert Flack <flackr@chromium.org>
Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644588}
parent 4ff11719
......@@ -235,6 +235,7 @@ void VisualViewport::UpdatePaintPropertyNodesIfNeeded(
state.local_transform_space = transform_parent;
state.direct_compositing_reasons =
CompositingReason::kActiveOpacityAnimation;
state.has_active_opacity_animation = true;
state.compositor_element_id =
GetScrollbarElementId(ScrollbarOrientation::kHorizontalScrollbar);
if (!horizontal_scrollbar_effect_node_) {
......@@ -256,6 +257,7 @@ void VisualViewport::UpdatePaintPropertyNodesIfNeeded(
state.local_transform_space = transform_parent;
state.direct_compositing_reasons =
CompositingReason::kActiveOpacityAnimation;
state.has_active_opacity_animation = true;
state.compositor_element_id =
GetScrollbarElementId(ScrollbarOrientation::kVerticalScrollbar);
if (!vertical_scrollbar_effect_node_) {
......
......@@ -106,6 +106,7 @@ LinkHighlightImpl::LinkHighlightImpl(Node* node)
state.local_transform_space = &TransformPaintPropertyNode::Root();
state.compositor_element_id = element_id_;
state.direct_compositing_reasons = CompositingReason::kActiveOpacityAnimation;
state.has_active_opacity_animation = true;
effect_ = EffectPaintPropertyNode::Create(EffectPaintPropertyNode::Root(),
std::move(state));
#if DCHECK_IS_ON()
......
......@@ -973,6 +973,12 @@ void FragmentPaintPropertyTreeBuilder::UpdateEffect() {
state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
object_.UniqueId(), CompositorElementIdNamespace::kPrimary);
}
// TODO(crbug.com/900241): Remove these setters when we can use
// state.direct_compositing_reasons to check for active animations.
state.has_active_opacity_animation = style.HasCurrentOpacityAnimation();
state.has_active_backdrop_filter_animation =
style.HasCurrentBackdropFilterAnimation();
}
EffectPaintPropertyNode::AnimationState animation_state;
......@@ -1136,6 +1142,11 @@ void FragmentPaintPropertyTreeBuilder::UpdateFilter() {
CompositingReason::kDirectReasonsForFilterProperty;
state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
object_.UniqueId(), CompositorElementIdNamespace::kEffectFilter);
// TODO(crbug.com/900241): Remove the setter when we can use
// state.direct_compositing_reasons to check for active animations.
state.has_active_filter_animation =
object_.StyleRef().HasCurrentFilterAnimation();
}
EffectPaintPropertyNode::AnimationState animation_state;
......@@ -1754,6 +1765,7 @@ void FragmentPaintPropertyTreeBuilder::UpdateScrollAndScrollTranslation() {
effect_state.local_transform_space = context_.current.transform;
effect_state.direct_compositing_reasons =
CompositingReason::kActiveOpacityAnimation;
effect_state.has_active_opacity_animation = true;
effect_state.compositor_element_id =
scrollable_area->GetScrollbarElementId(
ScrollbarOrientation::kVerticalScrollbar);
......@@ -1769,6 +1781,7 @@ void FragmentPaintPropertyTreeBuilder::UpdateScrollAndScrollTranslation() {
effect_state.local_transform_space = context_.current.transform;
effect_state.direct_compositing_reasons =
CompositingReason::kActiveOpacityAnimation;
effect_state.has_active_opacity_animation = true;
effect_state.compositor_element_id =
scrollable_area->GetScrollbarElementId(
ScrollbarOrientation::kHorizontalScrollbar);
......
......@@ -3587,6 +3587,26 @@ TEST_P(PaintArtifactCompositorTest,
EXPECT_OPACITY(opacity_id, 1.f, kNoRenderSurface);
}
TEST_P(PaintArtifactCompositorTest,
ActiveAnimationCompositingReasonWithoutActiveAnimationFlag) {
// TODO(crbug.com/900241): This test tests no render surface should be created
// for an effect node with kActiveFilterAnimation compositing reason without
// active animation flag. This simulates the extra effect node created for
// filter animation, which should not create render surface.
// Remove this test when we fix the bug.
EffectPaintPropertyNode::State state;
state.local_transform_space = &t0();
state.direct_compositing_reasons = CompositingReason::kActiveFilterAnimation;
auto e1 = EffectPaintPropertyNode::Create(e0(), std::move(state));
Update(TestPaintArtifact()
.Chunk(t0(), c0(), *e1)
.RectDrawing(FloatRect(150, 150, 100, 100), Color::kWhite)
.Build());
ASSERT_EQ(1u, ContentLayerCount());
EXPECT_OPACITY(ContentLayerAt(0)->effect_tree_index(), 1.f, kNoRenderSurface);
}
TEST_P(PaintArtifactCompositorTest, FilterCreatesRenderSurface) {
CompositorFilterOperations filter;
filter.AppendBlurFilter(5);
......
......@@ -58,6 +58,11 @@ class PLATFORM_EXPORT EffectPaintPropertyNode
// === End of effects ===
CompositingReasons direct_compositing_reasons = CompositingReason::kNone;
CompositorElementId compositor_element_id;
// TODO(crbug.com/900241): Use direct_compositing_reasons to check for
// active animations when we can track animations for each property type.
bool has_active_opacity_animation = false;
bool has_active_filter_animation = false;
bool has_active_backdrop_filter_animation = false;
// The offset of the origin of filters in local_transform_space.
FloatPoint filters_origin;
......@@ -205,16 +210,25 @@ class PLATFORM_EXPORT EffectPaintPropertyNode
CompositingReason::kComboActiveAnimation;
}
bool HasActiveOpacityAnimation() const {
return DirectCompositingReasons() &
CompositingReason::kActiveOpacityAnimation;
return state_.has_active_opacity_animation;
// TODO(crbug.com/900241): Use the following code when we can track
// animations for each property type.
// return DirectCompositingReasons() &
// CompositingReason::kActiveOpacityAnimation;
}
bool HasActiveFilterAnimation() const {
return DirectCompositingReasons() &
CompositingReason::kActiveFilterAnimation;
return state_.has_active_filter_animation;
// TODO(crbug.com/900241): Use the following code when we can track
// animations for each property type.
// return DirectCompositingReasons() &
// CompositingReason::kActiveFilterAnimation;
}
bool HasActiveBackdropFilterAnimation() const {
return DirectCompositingReasons() &
CompositingReason::kActiveBackdropFilterAnimation;
return state_.has_active_backdrop_filter_animation;
// TODO(crbug.com/900241): Use the following code when we can track
// animations for each property type.
// return DirectCompositingReasons() &
// CompositingReason::kActiveBackdropFilterAnimation;
}
const CompositorElementId& GetCompositorElementId() const {
......
......@@ -57,6 +57,7 @@ inline scoped_refptr<EffectPaintPropertyNode> CreateAnimatingOpacityEffect(
state.output_clip = output_clip;
state.opacity = opacity;
state.direct_compositing_reasons = CompositingReason::kActiveOpacityAnimation;
state.has_active_opacity_animation = true;
state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
NewUniqueObjectId(), CompositorElementIdNamespace::kPrimaryEffect);
return EffectPaintPropertyNode::Create(parent, std::move(state));
......@@ -99,6 +100,7 @@ inline scoped_refptr<EffectPaintPropertyNode> CreateAnimatingFilterEffect(
state.output_clip = output_clip;
state.filter = std::move(filter);
state.direct_compositing_reasons = CompositingReason::kActiveFilterAnimation;
state.has_active_filter_animation = true;
state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
NewUniqueObjectId(), CompositorElementIdNamespace::kEffectFilter);
return EffectPaintPropertyNode::Create(parent, std::move(state));
......@@ -144,6 +146,7 @@ CreateAnimatingBackdropFilterEffect(
state.backdrop_filter = std::move(backdrop_filter);
state.direct_compositing_reasons =
CompositingReason::kActiveBackdropFilterAnimation;
state.has_active_backdrop_filter_animation = true;
state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
NewUniqueObjectId(), CompositorElementIdNamespace::kPrimaryEffect);
return EffectPaintPropertyNode::Create(parent, std::move(state));
......
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