Commit 3f6cbe40 authored by Xida Chen's avatar Xida Chen Committed by Commit Bot

[OT-PW] Composite a custom property animation only if used by OT-PW

In our current implementation, a custom property animation is composited
regardless whether it is used by paint worklet or not.

This CL makes changes such that a custom property animation is composited
only when it may be used by off-thread paint worklet.

Bug: 1002586
Change-Id: I305c1396495fb12477756c5e2d0e56404983a39d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1798889
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: default avatarRune Lillesveen <futhark@chromium.org>
Reviewed-by: default avatarStephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699730}
parent fadac507
...@@ -1106,6 +1106,7 @@ jumbo_source_set("unit_tests") { ...@@ -1106,6 +1106,7 @@ jumbo_source_set("unit_tests") {
"css/css_test_helpers.h", "css/css_test_helpers.h",
"css/css_uri_value_test.cc", "css/css_uri_value_test.cc",
"css/css_value_test_helper.h", "css/css_value_test_helper.h",
"css/mock_css_paint_image_generator.h",
"display_lock/display_lock_budget_test.cc", "display_lock/display_lock_budget_test.cc",
"display_lock/display_lock_context_test.cc", "display_lock/display_lock_context_test.cc",
"display_lock/display_lock_utilities_test.cc", "display_lock/display_lock_utilities_test.cc",
......
...@@ -267,6 +267,11 @@ CompositorAnimations::CheckCanStartEffectOnCompositor( ...@@ -267,6 +267,11 @@ CompositorAnimations::CheckCanStartEffectOnCompositor(
if (keyframe_value) { if (keyframe_value) {
DCHECK(RuntimeEnabledFeatures::OffMainThreadCSSPaintEnabled()); DCHECK(RuntimeEnabledFeatures::OffMainThreadCSSPaintEnabled());
DCHECK(keyframe_value->IsDouble() || keyframe_value->IsColor()); DCHECK(keyframe_value->IsDouble() || keyframe_value->IsColor());
// If a custom property is not used by CSS Paint, then we should not
// support that on the compositor thread.
if (!layout_object->Style()->HasCSSPaintImagesUsingCustomProperty(
property.CustomPropertyName()))
reasons |= kUnsupportedCSSProperty;
// TODO: Add support for keyframes containing different types // TODO: Add support for keyframes containing different types
if (keyframes.front()->GetCompositorKeyframeValue()->GetType() != if (keyframes.front()->GetCompositorKeyframeValue()->GetType() !=
keyframe_value->GetType()) { keyframe_value->GetType()) {
......
...@@ -47,7 +47,11 @@ ...@@ -47,7 +47,11 @@
#include "third_party/blink/renderer/core/animation/element_animations.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/keyframe_effect.h"
#include "third_party/blink/renderer/core/animation/pending_animations.h" #include "third_party/blink/renderer/core/animation/pending_animations.h"
#include "third_party/blink/renderer/core/css/css_custom_ident_value.h"
#include "third_party/blink/renderer/core/css/css_paint_value.h"
#include "third_party/blink/renderer/core/css/css_syntax_definition.h"
#include "third_party/blink/renderer/core/css/css_test_helpers.h" #include "third_party/blink/renderer/core/css/css_test_helpers.h"
#include "third_party/blink/renderer/core/css/mock_css_paint_image_generator.h"
#include "third_party/blink/renderer/core/dom/document.h" #include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/dom/node_computed_style.h" #include "third_party/blink/renderer/core/dom/node_computed_style.h"
#include "third_party/blink/renderer/core/frame/frame_test_helpers.h" #include "third_party/blink/renderer/core/frame/frame_test_helpers.h"
...@@ -58,6 +62,7 @@ ...@@ -58,6 +62,7 @@
#include "third_party/blink/renderer/core/paint/paint_layer.h" #include "third_party/blink/renderer/core/paint/paint_layer.h"
#include "third_party/blink/renderer/core/style/computed_style.h" #include "third_party/blink/renderer/core/style/computed_style.h"
#include "third_party/blink/renderer/core/style/filter_operations.h" #include "third_party/blink/renderer/core/style/filter_operations.h"
#include "third_party/blink/renderer/core/style/style_generated_image.h"
#include "third_party/blink/renderer/core/testing/core_unit_test_helper.h" #include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
#include "third_party/blink/renderer/core/testing/dummy_page_holder.h" #include "third_party/blink/renderer/core/testing/dummy_page_holder.h"
#include "third_party/blink/renderer/platform/animation/compositor_color_animation_curve.h" #include "third_party/blink/renderer/platform/animation/compositor_color_animation_curve.h"
...@@ -77,8 +82,27 @@ ...@@ -77,8 +82,27 @@
#include "third_party/blink/renderer/platform/wtf/hash_functions.h" #include "third_party/blink/renderer/platform/wtf/hash_functions.h"
#include "third_party/skia/include/core/SkColor.h" #include "third_party/skia/include/core/SkColor.h"
using testing::_;
using testing::NiceMock;
using testing::Return;
using testing::ReturnRef;
using testing::Values;
namespace blink { namespace blink {
namespace {
// CSSPaintImageGenerator requires that CSSPaintImageGeneratorCreateFunction be
// a static method. As such, it cannot access a class member and so instead we
// store a pointer to the overriding generator globally.
MockCSSPaintImageGenerator* g_override_generator = nullptr;
CSSPaintImageGenerator* ProvideOverrideGenerator(
const String&,
const Document&,
CSSPaintImageGenerator::Observer*) {
return g_override_generator;
}
} // namespace
using namespace css_test_helpers; using namespace css_test_helpers;
class AnimationCompositorAnimationsTest : public PaintTestConfigurations, class AnimationCompositorAnimationsTest : public PaintTestConfigurations,
...@@ -588,9 +612,11 @@ TEST_P(AnimationCompositorAnimationsTest, ...@@ -588,9 +612,11 @@ TEST_P(AnimationCompositorAnimationsTest,
RegisterProperty(GetDocument(), "--foo", "<number>", "0", false); RegisterProperty(GetDocument(), "--foo", "<number>", "0", false);
RegisterProperty(GetDocument(), "--bar", "<length>", "10px", false); RegisterProperty(GetDocument(), "--bar", "<length>", "10px", false);
RegisterProperty(GetDocument(), "--loo", "<color>", "rgb(0, 0, 0)", false); RegisterProperty(GetDocument(), "--loo", "<color>", "rgb(0, 0, 0)", false);
RegisterProperty(GetDocument(), "--x", "<number>", "0", false);
SetCustomProperty("--foo", "10"); SetCustomProperty("--foo", "10");
SetCustomProperty("--bar", "10px"); SetCustomProperty("--bar", "10px");
SetCustomProperty("--loo", "rgb(0, 255, 0)"); SetCustomProperty("--loo", "rgb(0, 255, 0)");
SetCustomProperty("--x", "5");
auto style = GetDocument().EnsureStyleResolver().StyleForElement(element_); auto style = GetDocument().EnsureStyleResolver().StyleForElement(element_);
EXPECT_TRUE(style->NonInheritedVariables()); EXPECT_TRUE(style->NonInheritedVariables());
...@@ -603,7 +629,41 @@ TEST_P(AnimationCompositorAnimationsTest, ...@@ -603,7 +629,41 @@ TEST_P(AnimationCompositorAnimationsTest,
EXPECT_TRUE(style->NonInheritedVariables() EXPECT_TRUE(style->NonInheritedVariables()
->GetData(AtomicString("--loo")) ->GetData(AtomicString("--loo"))
.value_or(nullptr)); .value_or(nullptr));
EXPECT_TRUE(style->NonInheritedVariables()
->GetData(AtomicString("--x"))
.value_or(nullptr));
NiceMock<MockCSSPaintImageGenerator>* mock_generator =
MakeGarbageCollected<NiceMock<MockCSSPaintImageGenerator>>();
base::AutoReset<MockCSSPaintImageGenerator*> scoped_override_generator(
&g_override_generator, mock_generator);
base::AutoReset<CSSPaintImageGenerator::CSSPaintImageGeneratorCreateFunction>
scoped_create_function(
CSSPaintImageGenerator::GetCreateFunctionForTesting(),
ProvideOverrideGenerator);
mock_generator->AddCustomProperty("--foo");
mock_generator->AddCustomProperty("--bar");
mock_generator->AddCustomProperty("--loo");
auto* ident = MakeGarbageCollected<CSSCustomIdentValue>("foopainter");
CSSPaintValue* paint_value = MakeGarbageCollected<CSSPaintValue>(ident);
paint_value->CreateGeneratorForTesting(GetDocument());
StyleGeneratedImage* style_image =
MakeGarbageCollected<StyleGeneratedImage>(*paint_value);
style->AddPaintImage(style_image);
element_->GetLayoutObject()->SetStyle(style);
// The image is added for testing off-thread paint worklet supporting
// custom property animation case. The style doesn't have a real
// PaintImage, so we cannot call UpdateAllLifecyclePhasesForTest. But the
// PaintArtifactCompositor requires NeedsUpdate to be false.
// In the real world when a PaintImage does exist in the style, the life
// cycle will be updated automatically and we don't have to worry about
// this.
auto* paint_artifact_compositor =
GetDocument().View()->GetPaintArtifactCompositor();
paint_artifact_compositor->ClearNeedsUpdateForTesting();
ON_CALL(*mock_generator, IsImageGeneratorReady()).WillByDefault(Return(true));
StringKeyframe* keyframe = CreateReplaceOpKeyframe("--foo", "10"); StringKeyframe* keyframe = CreateReplaceOpKeyframe("--foo", "10");
EXPECT_EQ(DuplicateSingleKeyframeAndTestIsCandidateOnResult(keyframe), EXPECT_EQ(DuplicateSingleKeyframeAndTestIsCandidateOnResult(keyframe),
CompositorAnimations::kNoFailure); CompositorAnimations::kNoFailure);
...@@ -625,6 +685,12 @@ TEST_P(AnimationCompositorAnimationsTest, ...@@ -625,6 +685,12 @@ TEST_P(AnimationCompositorAnimationsTest,
SetCustomProperty("opacity", "var(--foo)"); SetCustomProperty("opacity", "var(--foo)");
EXPECT_TRUE(DuplicateSingleKeyframeAndTestIsCandidateOnResult(keyframe) & EXPECT_TRUE(DuplicateSingleKeyframeAndTestIsCandidateOnResult(keyframe) &
CompositorAnimations::kUnsupportedCSSProperty); CompositorAnimations::kUnsupportedCSSProperty);
// Cannot composite because "--x" is not used by the paint worklet.
StringKeyframe* non_used_keyframe = CreateReplaceOpKeyframe("--x", "5");
EXPECT_EQ(
DuplicateSingleKeyframeAndTestIsCandidateOnResult(non_used_keyframe),
CompositorAnimations::kUnsupportedCSSProperty);
} }
TEST_P(AnimationCompositorAnimationsTest, TEST_P(AnimationCompositorAnimationsTest,
......
...@@ -159,6 +159,13 @@ scoped_refptr<Image> CSSImageGeneratorValue::GetImage( ...@@ -159,6 +159,13 @@ scoped_refptr<Image> CSSImageGeneratorValue::GetImage(
return nullptr; return nullptr;
} }
bool CSSImageGeneratorValue::IsUsingCustomProperty(
const AtomicString& custom_property_name) const {
if (GetClassType() == kPaintClass)
return To<CSSPaintValue>(this)->IsUsingCustomProperty(custom_property_name);
return false;
}
bool CSSImageGeneratorValue::IsFixedSize() const { bool CSSImageGeneratorValue::IsFixedSize() const {
switch (GetClassType()) { switch (GetClassType()) {
case kCrossfadeClass: case kCrossfadeClass:
......
...@@ -105,6 +105,8 @@ class CORE_EXPORT CSSImageGeneratorValue : public CSSValue { ...@@ -105,6 +105,8 @@ class CORE_EXPORT CSSImageGeneratorValue : public CSSValue {
CSSImageGeneratorValue* ComputedCSSValue(const ComputedStyle&, CSSImageGeneratorValue* ComputedCSSValue(const ComputedStyle&,
bool allow_visited_style); bool allow_visited_style);
bool IsUsingCustomProperty(const AtomicString& custom_property_name) const;
void TraceAfterDispatch(blink::Visitor* visitor) { void TraceAfterDispatch(blink::Visitor* visitor) {
CSSValue::TraceAfterDispatch(visitor); CSSValue::TraceAfterDispatch(visitor);
} }
......
...@@ -55,6 +55,21 @@ String CSSPaintValue::GetName() const { ...@@ -55,6 +55,21 @@ String CSSPaintValue::GetName() const {
return name_->Value(); return name_->Value();
} }
bool CSSPaintValue::IsUsingCustomProperty(
const AtomicString& custom_property_name) const {
if (!generator_ || !generator_->IsImageGeneratorReady())
return false;
return generator_->CustomInvalidationProperties().Contains(
custom_property_name);
}
void CSSPaintValue::CreateGeneratorForTesting(const Document& document) {
if (!generator_) {
generator_ = CSSPaintImageGenerator::Create(
GetName(), document, paint_image_generator_observer_);
}
}
scoped_refptr<Image> CSSPaintValue::GetImage( scoped_refptr<Image> CSSPaintValue::GetImage(
const ImageResourceObserver& client, const ImageResourceObserver& client,
const Document& document, const Document& document,
......
...@@ -64,6 +64,10 @@ class CORE_EXPORT CSSPaintValue : public CSSImageGeneratorValue { ...@@ -64,6 +64,10 @@ class CORE_EXPORT CSSPaintValue : public CSSImageGeneratorValue {
return this; return this;
} }
bool IsUsingCustomProperty(const AtomicString& custom_property_name) const;
void CreateGeneratorForTesting(const Document& document);
void TraceAfterDispatch(blink::Visitor*); void TraceAfterDispatch(blink::Visitor*);
private: private:
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/renderer/core/css/css_custom_ident_value.h" #include "third_party/blink/renderer/core/css/css_custom_ident_value.h"
#include "third_party/blink/renderer/core/css/css_syntax_definition.h" #include "third_party/blink/renderer/core/css/css_syntax_definition.h"
#include "third_party/blink/renderer/core/css/mock_css_paint_image_generator.h"
#include "third_party/blink/renderer/core/dom/document.h" #include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/frame/frame_test_helpers.h" #include "third_party/blink/renderer/core/frame/frame_test_helpers.h"
#include "third_party/blink/renderer/core/frame/web_local_frame_impl.h" #include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
...@@ -55,41 +56,6 @@ INSTANTIATE_TEST_SUITE_P(, ...@@ -55,41 +56,6 @@ INSTANTIATE_TEST_SUITE_P(,
kCSSPaintAPIArguments | kCSSPaintAPIArguments |
kOffMainThreadCSSPaint)); kOffMainThreadCSSPaint));
class MockCSSPaintImageGenerator : public CSSPaintImageGenerator {
public:
MockCSSPaintImageGenerator() {
// These methods return references, so setup a default ON_CALL to make them
// easier to use. They can be overridden by a specific test if desired.
ON_CALL(*this, NativeInvalidationProperties())
.WillByDefault(ReturnRef(native_properties_));
ON_CALL(*this, CustomInvalidationProperties())
.WillByDefault(ReturnRef(custom_properties_));
ON_CALL(*this, InputArgumentTypes())
.WillByDefault(ReturnRef(input_argument_types_));
}
MOCK_METHOD4(Paint,
scoped_refptr<Image>(const ImageResourceObserver&,
const FloatSize& container_size,
const CSSStyleValueVector*,
float device_scale_factor));
MOCK_CONST_METHOD0(NativeInvalidationProperties, Vector<CSSPropertyID>&());
MOCK_CONST_METHOD0(CustomInvalidationProperties, Vector<AtomicString>&());
MOCK_CONST_METHOD0(HasAlpha, bool());
MOCK_CONST_METHOD0(InputArgumentTypes, Vector<CSSSyntaxDefinition>&());
MOCK_CONST_METHOD0(IsImageGeneratorReady, bool());
MOCK_CONST_METHOD0(WorkletId, int());
void AddNativeProperty() {
native_properties_.push_back(CSSPropertyID::kBorderImageSource);
}
private:
Vector<CSSPropertyID> native_properties_;
Vector<AtomicString> custom_properties_;
Vector<CSSSyntaxDefinition> input_argument_types_;
};
// CSSPaintImageGenerator requires that CSSPaintImageGeneratorCreateFunction be // CSSPaintImageGenerator requires that CSSPaintImageGeneratorCreateFunction be
// a static method. As such, it cannot access a class member and so instead we // a static method. As such, it cannot access a class member and so instead we
// store a pointer to the overriding generator globally. // store a pointer to the overriding generator globally.
......
// Copyright 2019 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.
#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_CSS_MOCK_CSS_PAINT_IMAGE_GENERATOR_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_CSS_MOCK_CSS_PAINT_IMAGE_GENERATOR_H_
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/renderer/core/css/css_paint_image_generator.h"
using testing::ReturnRef;
namespace blink {
class MockCSSPaintImageGenerator : public CSSPaintImageGenerator {
public:
MockCSSPaintImageGenerator() {
// These methods return references, so setup a default ON_CALL to make them
// easier to use. They can be overridden by a specific test if desired.
ON_CALL(*this, NativeInvalidationProperties())
.WillByDefault(ReturnRef(native_properties_));
ON_CALL(*this, CustomInvalidationProperties())
.WillByDefault(ReturnRef(custom_properties_));
ON_CALL(*this, InputArgumentTypes())
.WillByDefault(ReturnRef(input_argument_types_));
}
MOCK_METHOD4(Paint,
scoped_refptr<Image>(const ImageResourceObserver&,
const FloatSize& container_size,
const CSSStyleValueVector*,
float device_scale_factor));
MOCK_CONST_METHOD0(NativeInvalidationProperties, Vector<CSSPropertyID>&());
MOCK_CONST_METHOD0(CustomInvalidationProperties, Vector<AtomicString>&());
MOCK_CONST_METHOD0(HasAlpha, bool());
MOCK_CONST_METHOD0(InputArgumentTypes, Vector<CSSSyntaxDefinition>&());
MOCK_CONST_METHOD0(IsImageGeneratorReady, bool());
MOCK_CONST_METHOD0(WorkletId, int());
void AddCustomProperty(const AtomicString& custom_property) {
custom_properties_.push_back(custom_property);
}
void AddNativeProperty() {
native_properties_.push_back(CSSPropertyID::kBorderImageSource);
}
private:
Vector<CSSPropertyID> native_properties_;
Vector<AtomicString> custom_properties_;
Vector<CSSSyntaxDefinition> input_argument_types_;
};
} // namespace blink
#endif // THIRD_PARTY_BLINK_RENDERER_CORE_CSS_MOCK_CSS_PAINT_IMAGE_GENERATOR_H_
...@@ -54,6 +54,7 @@ ...@@ -54,6 +54,7 @@
#include "third_party/blink/renderer/core/style/shadow_list.h" #include "third_party/blink/renderer/core/style/shadow_list.h"
#include "third_party/blink/renderer/core/style/style_difference.h" #include "third_party/blink/renderer/core/style/style_difference.h"
#include "third_party/blink/renderer/core/style/style_fetched_image.h" #include "third_party/blink/renderer/core/style/style_fetched_image.h"
#include "third_party/blink/renderer/core/style/style_generated_image.h"
#include "third_party/blink/renderer/core/style/style_image.h" #include "third_party/blink/renderer/core/style/style_image.h"
#include "third_party/blink/renderer/core/style/style_inherited_variables.h" #include "third_party/blink/renderer/core/style/style_inherited_variables.h"
#include "third_party/blink/renderer/core/style/style_non_inherited_variables.h" #include "third_party/blink/renderer/core/style/style_non_inherited_variables.h"
...@@ -943,6 +944,22 @@ void ComputedStyle::AddPaintImage(StyleImage* image) { ...@@ -943,6 +944,22 @@ void ComputedStyle::AddPaintImage(StyleImage* image) {
MutablePaintImagesInternal()->push_back(image); MutablePaintImagesInternal()->push_back(image);
} }
bool ComputedStyle::HasCSSPaintImagesUsingCustomProperty(
const AtomicString& custom_property_name) const {
if (PaintImagesInternal()) {
for (const auto& image : *PaintImagesInternal()) {
DCHECK(image);
// IsPaintImage is true for CSS Paint images only, please refer to the
// constructor of StyleGeneratedImage.
if (image->IsPaintImage()) {
return To<StyleGeneratedImage>(image.Get())
->IsUsingCustomProperty(custom_property_name);
}
}
}
return false;
}
void ComputedStyle::AddCursor(StyleImage* image, void ComputedStyle::AddCursor(StyleImage* image,
bool hot_spot_specified, bool hot_spot_specified,
const IntPoint& hot_spot) { const IntPoint& hot_spot) {
......
...@@ -2302,7 +2302,12 @@ class ComputedStyle : public ComputedStyleBase, ...@@ -2302,7 +2302,12 @@ class ComputedStyle : public ComputedStyleBase,
} }
// Paint utility functions. // Paint utility functions.
void AddPaintImage(StyleImage*); CORE_EXPORT void AddPaintImage(StyleImage*);
// Returns true if any property has an <image> value that is a CSS paint
// function that is using a given custom property.
bool HasCSSPaintImagesUsingCustomProperty(
const AtomicString& custom_property_name) const;
// FIXME: reflections should belong to this helper function but they are // FIXME: reflections should belong to this helper function but they are
// currently handled through their self-painting layers. So the layout code // currently handled through their self-painting layers. So the layout code
......
...@@ -78,6 +78,11 @@ void StyleGeneratedImage::RemoveClient(ImageResourceObserver* observer) { ...@@ -78,6 +78,11 @@ void StyleGeneratedImage::RemoveClient(ImageResourceObserver* observer) {
image_generator_value_->RemoveClient(observer); image_generator_value_->RemoveClient(observer);
} }
bool StyleGeneratedImage::IsUsingCustomProperty(
const AtomicString& custom_property_name) const {
return image_generator_value_->IsUsingCustomProperty(custom_property_name);
}
scoped_refptr<Image> StyleGeneratedImage::GetImage( scoped_refptr<Image> StyleGeneratedImage::GetImage(
const ImageResourceObserver& observer, const ImageResourceObserver& observer,
const Document& document, const Document& document,
......
...@@ -60,6 +60,8 @@ class CORE_EXPORT StyleGeneratedImage final : public StyleImage { ...@@ -60,6 +60,8 @@ class CORE_EXPORT StyleGeneratedImage final : public StyleImage {
const FloatSize& target_size) const override; const FloatSize& target_size) const override;
bool KnownToBeOpaque(const Document&, const ComputedStyle&) const override; bool KnownToBeOpaque(const Document&, const ComputedStyle&) const override;
bool IsUsingCustomProperty(const AtomicString& custom_property_name) const;
void Trace(blink::Visitor*) override; void Trace(blink::Visitor*) override;
private: private:
......
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