Skip to content

Commit

Permalink
[CompositeClipPathAnimations]Fall back to main thread if clip-path an…
Browse files Browse the repository at this point in the history
…imation is not compositable

Integrate fall back logic implemented for composited background-color
animations in

https://chromium-review.googlesource.com/c/chromium/src/+/2588636

Bug: 1245361
Change-Id: I0fa917330a83b1b3e7d14bbfa9efd23a846eb851
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3133755
Commit-Queue: Olga Gerchikov <gerchiko@microsoft.com>
Reviewed-by: Xida Chen <xidachen@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#918328}
  • Loading branch information
ogerchikov authored and Chromium LUCI CQ committed Sep 4, 2021
1 parent d0980af commit c56accc
Show file tree
Hide file tree
Showing 15 changed files with 146 additions and 88 deletions.
43 changes: 0 additions & 43 deletions third_party/blink/renderer/core/animation/animation_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1661,49 +1661,6 @@ TEST_P(AnimationAnimationTestCompositing,
EXPECT_FALSE(animation->HasActiveAnimationsOnCompositor());
}

// This test ensures that a clip-path animation can start on compositor.
TEST_P(AnimationAnimationTestCompositing, ClipPathComposited) {
ScopedCompositeClipPathAnimationForTest composite_clip_path_animation(true);
SetBodyInnerHTML(R"HTML(
<div id ="target" style="width: 100px; height: 100px; clip-path: circle(50% at 50% 50%)">
</div>
)HTML");

// Create KeyframeEffect
Timing timing;
timing.iteration_duration = AnimationTimeDelta::FromSecondsD(30);

Persistent<StringKeyframe> start_keyframe =
MakeGarbageCollected<StringKeyframe>();
start_keyframe->SetCSSPropertyValue(
CSSPropertyID::kClipPath, "circle(50% at 50% 50%)",
SecureContextMode::kInsecureContext, nullptr);
Persistent<StringKeyframe> end_keyframe =
MakeGarbageCollected<StringKeyframe>();
end_keyframe->SetCSSPropertyValue(
CSSPropertyID::kClipPath, "(20% at 20% 20%)",
SecureContextMode::kInsecureContext, nullptr);

StringKeyframeVector keyframes;
keyframes.push_back(start_keyframe);
keyframes.push_back(end_keyframe);

Element* element = GetElementById("target");
auto* model = MakeGarbageCollected<StringKeyframeEffectModel>(keyframes);

NonThrowableExceptionState exception_state;
DocumentTimeline* timeline =
MakeGarbageCollected<DocumentTimeline>(&GetDocument());
Animation* animation = Animation::Create(
MakeGarbageCollected<KeyframeEffect>(element, model, timing), timeline,
exception_state);

UpdateAllLifecyclePhasesForTest();
animation->play();
EXPECT_EQ(animation->CheckCanStartAnimationOnCompositor(nullptr),
CompositorAnimations::kNoFailure);
}

TEST_P(AnimationAnimationTestCompositing,
ScrollLinkedAnimationCanBeComposited) {
ResetWithCompositedAnimation();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
#include "third_party/blink/renderer/core/animation/element_animations.h"
#include "third_party/blink/renderer/core/animation/keyframe_effect_model.h"
#include "third_party/blink/renderer/core/css/background_color_paint_image_generator.h"
#include "third_party/blink/renderer/core/css/clip_path_paint_image_generator.h"
#include "third_party/blink/renderer/core/css/properties/computed_style_utils.h"
#include "third_party/blink/renderer/core/dom/dom_node_ids.h"
#include "third_party/blink/renderer/core/dom/node_computed_style.h"
Expand Down Expand Up @@ -367,7 +368,21 @@ CompositorAnimations::CheckCanStartEffectOnCompositor(
break;
}
case CSSPropertyID::kClipPath: {
if (!RuntimeEnabledFeatures::CompositeClipPathAnimationEnabled()) {
Animation* compositable_animation = nullptr;
if (RuntimeEnabledFeatures::CompositeClipPathAnimationEnabled()) {
ClipPathPaintImageGenerator* generator =
target_element.GetDocument()
.GetFrame()
->GetClipPathPaintImageGenerator();
// TODO(crbug.com/686074): The generator may be null in tests.
// Fix and remove this test-only branch.
if (generator) {
compositable_animation =
generator->GetAnimationIfCompositable(&target_element);
}
}
if (!RuntimeEnabledFeatures::CompositeClipPathAnimationEnabled() ||
!compositable_animation) {
DefaultToUnsupportedProperty(unsupported_properties, property,
&reasons);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace blink {

class Animation;
class Element;
class Image;
class LocalFrame;
class Node;
Expand All @@ -30,6 +32,7 @@ class CORE_EXPORT ClipPathPaintImageGenerator
virtual scoped_refptr<Image> Paint(float zoom,
const FloatRect& reference_box,
const Node&) = 0;
virtual Animation* GetAnimationIfCompositable(const Element* element) = 0;
};

} // namespace blink
Expand Down
47 changes: 27 additions & 20 deletions third_party/blink/renderer/core/paint/clip_path_clipper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,27 @@ static bool UsesZoomedReferenceBox(const LayoutObject& clip_path_owner) {
return !clip_path_owner.IsSVGChild() || clip_path_owner.IsSVGForeignObject();
}

static bool HasCompositeClipPathAnimation(const LayoutObject& layout_object) {
if (!RuntimeEnabledFeatures::CompositeClipPathAnimationEnabled() ||
!layout_object.StyleRef().HasCurrentClipPathAnimation())
return false;

ClipPathPaintImageGenerator* generator =
layout_object.GetFrame()->GetClipPathPaintImageGenerator();
// TODO(crbug.com/686074): The generator may be null in tests.
// Fix and remove this test-only branch.
if (generator) {
const Element* element = To<Element>(layout_object.GetNode());
return generator->GetAnimationIfCompositable(element);
}
return false;
}

static void PaintWorkletBasedClip(GraphicsContext& context,
const LayoutObject& clip_path_owner,
const FloatRect& reference_box,
bool uses_zoomed_reference_box) {
DCHECK(RuntimeEnabledFeatures::CompositeClipPathAnimationEnabled());
DCHECK(HasCompositeClipPathAnimation(clip_path_owner));
DCHECK_EQ(clip_path_owner.StyleRef().ClipPath()->GetType(),
ClipPathOperation::SHAPE);

Expand Down Expand Up @@ -207,12 +223,8 @@ void ClipPathClipper::PaintClipPathAsMaskImage(

bool uses_zoomed_reference_box = UsesZoomedReferenceBox(layout_object);
FloatRect reference_box = LocalReferenceBox(layout_object);
// TODO(crbug.com/1223975): Currently for CompositeClipPathAnimation feature
// to be activated a node must have clip-path attribute.
if (RuntimeEnabledFeatures::CompositeClipPathAnimationEnabled() &&
layout_object.StyleRef().HasCurrentClipPathAnimation() &&
layout_object.StyleRef().ClipPath()->GetType() ==
ClipPathOperation::SHAPE) {

if (HasCompositeClipPathAnimation(layout_object)) {
if (!layout_object.GetFrame())
return;
PaintWorkletBasedClip(context, layout_object, reference_box,
Expand Down Expand Up @@ -265,11 +277,7 @@ void ClipPathClipper::PaintClipPathAsMaskImage(
bool ClipPathClipper::ShouldUseMaskBasedClip(const LayoutObject& object) {
if (object.IsText() || !object.StyleRef().HasClipPath())
return false;
// TODO(crbug.com/1223975): Currently for CompositeClipPathAnimation feature
// to be activated a node must have clip-path attribute.
if (RuntimeEnabledFeatures::CompositeClipPathAnimationEnabled() &&
object.StyleRef().ClipPath()->GetType() == ClipPathOperation::SHAPE &&
object.StyleRef().HasCurrentClipPathAnimation())
if (HasCompositeClipPathAnimation(object))
return true;
const auto* reference_clip =
DynamicTo<ReferenceClipPathOperation>(object.StyleRef().ClipPath());
Expand All @@ -284,14 +292,13 @@ bool ClipPathClipper::ShouldUseMaskBasedClip(const LayoutObject& object) {

absl::optional<Path> ClipPathClipper::PathBasedClip(
const LayoutObject& clip_path_owner) {
// TODO(crbug.com/1223975): Currently for CompositeClipPathAnimation feature
// to be activated a node must have clip-path attribute.
if (RuntimeEnabledFeatures::CompositeClipPathAnimationEnabled() &&
clip_path_owner.StyleRef().HasCurrentClipPathAnimation()) {
const ClipPathOperation& clip_path = *clip_path_owner.StyleRef().ClipPath();
if (clip_path.GetType() == ClipPathOperation::SHAPE)
return absl::nullopt;
}
// TODO(crbug.com/1223975): Currently HasCompositeClipPathAnimation is called
// multiple times, which is not efficient. Cache
// HasCompositeClipPathAnimation value as part of fragment_data, similarly to
// FragmentData::ClipPathPath().
if (HasCompositeClipPathAnimation(clip_path_owner))
return absl::nullopt;

return PathBasedClipInternal(clip_path_owner,
UsesZoomedReferenceBox(clip_path_owner),
LocalReferenceBox(clip_path_owner));
Expand Down
1 change: 1 addition & 0 deletions third_party/blink/renderer/modules/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,7 @@ source_set("unit_tests") {
"credentialmanager/password_credential_test.cc",
"csspaint/document_paint_definition_test.cc",
"csspaint/nativepaint/background_color_paint_definition_test.cc",
"csspaint/nativepaint/clip_path_paint_definition_test.cc",
"csspaint/paint_rendering_context_2d_test.cc",
"csspaint/paint_worklet_global_scope_test.cc",
"csspaint/paint_worklet_proxy_client_test.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,25 @@ bool CanGetValueFromKeyframe(const PropertySpecificKeyframe* frame,
return true;
}

} // namespace

template <>
struct DowncastTraits<ClipPathPaintWorkletInput> {
static bool AllowFrom(const cc::PaintWorkletInput& worklet_input) {
auto* input = DynamicTo<PaintWorkletInput>(worklet_input);
return input && AllowFrom(*input);
}

static bool AllowFrom(const PaintWorkletInput& worklet_input) {
return worklet_input.GetType() ==
PaintWorkletInput::PaintWorkletInputType::kClipPath;
}
};

// TODO(crbug.com/686074): Introduce helper functions commonly used by
// background-color and clip-path animations.
Animation* GetAnimationIfCompositable(const Element* element) {
Animation* ClipPathPaintDefinition::GetAnimationIfCompositable(
const Element* element) {
if (!element->GetElementAnimations())
return nullptr;
Animation* compositable_animation = nullptr;
Expand Down Expand Up @@ -205,21 +221,6 @@ Animation* GetAnimationIfCompositable(const Element* element) {
return compositable_animation;
}

} // namespace

template <>
struct DowncastTraits<ClipPathPaintWorkletInput> {
static bool AllowFrom(const cc::PaintWorkletInput& worklet_input) {
auto* input = DynamicTo<PaintWorkletInput>(worklet_input);
return input && AllowFrom(*input);
}

static bool AllowFrom(const PaintWorkletInput& worklet_input) {
return worklet_input.GetType() ==
PaintWorkletInput::PaintWorkletInputType::kClipPath;
}
};

// static
ClipPathPaintDefinition* ClipPathPaintDefinition::Create(
LocalFrame& local_root) {
Expand Down Expand Up @@ -330,9 +331,9 @@ scoped_refptr<Image> ClipPathPaintDefinition::Paint(
Vector<double> offsets;
absl::optional<double> progress;

// TODO(crbug.com/1223975): implement main-thread fall back logic for
// animations that we cannot handle.
Animation* animation = GetAnimationIfCompositable(element);
// If we are here the animation must be compositable.
DCHECK(animation);

const AnimationEffect* effect = animation->effect();
DCHECK(effect->IsKeyframeEffect());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

namespace blink {

class Animation;
class FloatRect;
class Image;
class LocalFrame;
Expand All @@ -34,7 +35,8 @@ class MODULES_EXPORT ClipPathPaintDefinition final
scoped_refptr<Image> Paint(float zoom,
const FloatRect& reference_box,
const Node&);

// Shared code that is being called in multiple places.
static Animation* GetAnimationIfCompositable(const Element* element);
void Trace(Visitor* visitor) const override;
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// Copyright 2021 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "third_party/blink/renderer/modules/csspaint/nativepaint/clip_path_paint_definition.h"

#include "testing/gmock/include/gmock/gmock.h"
#include "third_party/blink/renderer/core/animation/document_timeline.h"
#include "third_party/blink/renderer/core/animation/element_animations.h"
#include "third_party/blink/renderer/core/animation/keyframe_effect.h"
#include "third_party/blink/renderer/core/animation/string_keyframe.h"
#include "third_party/blink/renderer/core/animation/timing.h"
#include "third_party/blink/renderer/core/dom/element.h"
#include "third_party/blink/renderer/core/testing/page_test_base.h"

namespace blink {

class ClipPathPaintDefinitionTest : public PageTestBase {
public:
ClipPathPaintDefinitionTest() = default;
~ClipPathPaintDefinitionTest() override = default;
};

// Test the case where there is a background-color animation with two simple
// keyframes that will not fall back to main.
TEST_F(ClipPathPaintDefinitionTest, SimpleClipPathAnimationNotFallback) {
ScopedCompositeClipPathAnimationForTest composite_clip_path_animation(true);
SetBodyInnerHTML(R"HTML(
<div id ="target" style="width: 100px; height: 100px">
</div>
)HTML");

Timing timing;
timing.iteration_duration = AnimationTimeDelta::FromSecondsD(30);

CSSPropertyID property_id = CSSPropertyID::kClipPath;
Persistent<StringKeyframe> start_keyframe =
MakeGarbageCollected<StringKeyframe>();
start_keyframe->SetCSSPropertyValue(property_id, "circle(50% at 50% 50%)",
SecureContextMode::kInsecureContext,
nullptr);
Persistent<StringKeyframe> end_keyframe =
MakeGarbageCollected<StringKeyframe>();
end_keyframe->SetCSSPropertyValue(property_id, "circle(30% at 30% 30%)",
SecureContextMode::kInsecureContext,
nullptr);

StringKeyframeVector keyframes;
keyframes.push_back(start_keyframe);
keyframes.push_back(end_keyframe);

auto* model = MakeGarbageCollected<StringKeyframeEffectModel>(keyframes);
model->SetComposite(EffectModel::kCompositeReplace);

Element* element = GetElementById("target");
NonThrowableExceptionState exception_state;
DocumentTimeline* timeline =
MakeGarbageCollected<DocumentTimeline>(&GetDocument());
Animation* animation = Animation::Create(
MakeGarbageCollected<KeyframeEffect>(element, model, timing), timeline,
exception_state);
UpdateAllLifecyclePhasesForTest();
animation->play();

EXPECT_TRUE(element->GetElementAnimations());
EXPECT_EQ(element->GetElementAnimations()->Animations().size(), 1u);
EXPECT_EQ(ClipPathPaintDefinition::GetAnimationIfCompositable(element),
animation);
}

} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ scoped_refptr<Image> ClipPathPaintImageGeneratorImpl::Paint(
return clip_path_paint_definition_->Paint(zoom, reference_box, node);
}

Animation* ClipPathPaintImageGeneratorImpl::GetAnimationIfCompositable(
const Element* element) {
return ClipPathPaintDefinition::GetAnimationIfCompositable(element);
}

void ClipPathPaintImageGeneratorImpl::Shutdown() {
clip_path_paint_definition_->UnregisterProxyClient();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class MODULES_EXPORT ClipPathPaintImageGeneratorImpl final
scoped_refptr<Image> Paint(float zoom,
const FloatRect& reference_box,
const Node&) final;
Animation* GetAnimationIfCompositable(const Element* element) final;

void Shutdown() final;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
width: 100px;
height: 100px;
background-color: green;
clip-path: ellipse(50% 40% at 50% 50%);
/* Use a long animation that start at 30% progress where the slope of the
selected timing function is zero. By setting up the animation in this way,
we accommodate lengthy delays in running the test without a potential drift
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
width: 300px;
height: 300px;
background-color: green;
clip-path: ellipse(50% 40% at 50% 50%);
/* Use a long animation that start at 60% progress where the slope of the
selected timing function is zero. By setting up the animation in this way,
we accommodate lengthy delays in running the test without a potential drift
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
width: 100px;
height: 100px;
background-color: green;
clip-path: ellipse(50% 40% at 50% 50%);
/* Use a long animation that start at 5% progress where the slope of the
selected timing function is zero. By setting up the animation in this way,
we accommodate lengthy delays in running the test without a potential drift
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
width: 300px;
height: 300px;
background-color: green;
clip-path: ellipse(50% 40% at 50% 50%);
/* Use a long animation that start at 50% progress where the slope of the
selected timing function is zero. By setting up the animation in this way,
we accommodate lengthy delays in running the test without a potential drift
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
width: 100px;
height: 100px;
background-color: green;
clip-path: ellipse(50% 40% at 50% 50%);
/* Use a long animation that start at 50% progress where the slope of the
selected timing function is zero. By setting up the animation in this way,
we accommodate lengthy delays in running the test without a potential drift
Expand Down

0 comments on commit c56accc

Please sign in to comment.