[web-animations] account for iterationComposite when interpolating cu…

…stom properties https://bugs.webkit.org/show_bug.cgi?id=249394 Reviewed by Antti Koivisto. In order to correctly handle iterationComposite when interpolating custom properties, we update CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration() to take a CSSPropertyAnimation::Property as a parameter such that we can accept a custom property. In the implementation we can check for the custom property type to identify whether it requires blending for iterationComposite. In KeyframeEffect::setAnimatedPropertiesInStyle(), we can then remove the code specific to standard CSS properties when dealing with iterationComposite. * LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/animation/custom-property-animation-color.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/animation/custom-property-animation-length-expected.txt: * Source/WebCore/animation/CSSPropertyAnimation.cpp: (WebCore::CSSPropertyAnimation::blendProperty): (WebCore::CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration): * Source/WebCore/animation/CSSPropertyAnimation.h: * Source/WebCore/animation/KeyframeEffect.cpp: (WebCore::KeyframeEffect::setAnimatedPropertiesInStyle): Canonical link: https://commits.webkit.org/257925@main
WebKit · Dec 15, 2022 · f391c2f826f214bec6c97c56da49016b5d68dfa2 · f391c2f
1 parent fa664e0
commit f391c2f826f214bec6c97c56da49016b5d68dfa2
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 23 deletions.
diff --git a/...atform-tests/css/css-properties-values-api/animation/custom-property-animation-color.html b/...atform-tests/css/css-properties-values-api/animation/custom-property-animation-color.html
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<link rel="help" href="https://drafts.css-houdini.org/css-properties-values-api-1">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="../resources/utils.js"></script>
+<div id="target"></div>
+<script>
+
+animation_test({
+  syntax: "<color>",
+  inherits: false,
+  initialValue: "red"
+}, {
+  keyframes: ["blue", "green"],
+  expected: "rgb(0, 64, 128)"
+}, 'Animating a custom property of type <color>');
+
+// Need to test accumulation, iterationComposite, etc.
+
+</script>
diff --git a/...sts/css/css-properties-values-api/animation/custom-property-animation-length-expected.txt b/...sts/css/css-properties-values-api/animation/custom-property-animation-length-expected.txt
@@ -3,5 +3,5 @@ PASS Animating a custom property of type <length>
 PASS Animating a custom property of type <length> with a single keyframe
 PASS Animating a custom property of type <length> with additivity
 PASS Animating a custom property of type <length> with a single keyframe and additivity
-FAIL Animating a custom property of type <length> with iterationComposite assert_equals: expected "250px" but got "50px"
+PASS Animating a custom property of type <length> with iterationComposite
 
diff --git a/Source/WebCore/animation/CSSPropertyAnimation.cpp b/Source/WebCore/animation/CSSPropertyAnimation.cpp
@@ -3976,8 +3976,7 @@ void CSSPropertyAnimation::blendProperty(const CSSPropertyBlendingClient& client
         [&] (CSSPropertyID propertyId) {
             blendStandardProperty(client, propertyId, destination, from, to, progress, compositeOperation, iterationCompositeOperation, currentIteration);
         }, [&] (AtomString customProperty) {
-            // FIXME: we don't deal with iterationCompositeOpertion for custom properties yet.
-            blendCustomProperty(client, customProperty, destination, from, to, progress, compositeOperation, IterationCompositeOperation::Replace, 0);
+            blendCustomProperty(client, customProperty, destination, from, to, progress, compositeOperation, iterationCompositeOperation, currentIteration);
         }
     );
 }
@@ -3998,10 +3997,33 @@ bool CSSPropertyAnimation::isPropertyAdditiveOrCumulative(Property property)
     );
 }
 
-bool CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration(CSSPropertyID property, const RenderStyle& a, const RenderStyle& b)
+bool CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration(const CSSPropertyBlendingClient& client, Property property, const RenderStyle& a, const RenderStyle& b)
 {
-    auto* wrapper = CSSPropertyAnimationWrapperMap::singleton().wrapperForProperty(property);
-    return wrapper && wrapper->requiresBlendingForAccumulativeIteration(a, b);
+    return WTF::switchOn(property,
+        [&] (CSSPropertyID propertyId) {
+            if (auto* wrapper = CSSPropertyAnimationWrapperMap::singleton().wrapperForProperty(propertyId))
+                return wrapper->requiresBlendingForAccumulativeIteration(a, b);
+            return false;
+        }, [&] (AtomString customProperty) {
+            auto [from, to] = customPropertyValuesForBlending(client, customProperty, a.getCustomProperty(customProperty), b.getCustomProperty(customProperty));
+            if (!from || !to)
+                return false;
+
+            if (!std::holds_alternative<CSSCustomPropertyValue::SyntaxValue>(from->value()) || !std::holds_alternative<CSSCustomPropertyValue::SyntaxValue>(to->value()))
+                return false;
+
+            auto& fromSyntaxValue = std::get<CSSCustomPropertyValue::SyntaxValue>(from->value());
+            auto& toSyntaxValue = std::get<CSSCustomPropertyValue::SyntaxValue>(to->value());
+
+            // FIXME: we need to also ensure we blend for iterationComposite for <color>, <transform-list>, <filter-value-list> and <shadow>.
+            return WTF::switchOn(fromSyntaxValue, [toSyntaxValue](const Length& fromLength) {
+                ASSERT(std::holds_alternative<Length>(toSyntaxValue));
+                return lengthsRequireBlendingForAccumulativeIteration(fromLength, std::get<Length>(toSyntaxValue) );
+            }, [] (auto&) {
+                return false;
+            });
+        }
+    );
 }
 
 bool CSSPropertyAnimation::animationOfPropertyIsAccelerated(CSSPropertyID property)

diff --git a/Source/WebCore/animation/CSSPropertyAnimation.h b/Source/WebCore/animation/CSSPropertyAnimation.h
@@ -44,7 +44,7 @@ class CSSPropertyAnimation {
 
     static bool isPropertyAnimatable(CSSPropertyID);
     static bool isPropertyAdditiveOrCumulative(Property);
-    static bool propertyRequiresBlendingForAccumulativeIteration(CSSPropertyID, const RenderStyle& a, const RenderStyle& b);
+    static bool propertyRequiresBlendingForAccumulativeIteration(const CSSPropertyBlendingClient&, Property, const RenderStyle& a, const RenderStyle& b);
     static bool animationOfPropertyIsAccelerated(CSSPropertyID);
     static bool propertiesEqual(CSSPropertyID, const RenderStyle& a, const RenderStyle& b);
     static bool canPropertyBeInterpolated(CSSPropertyID, const RenderStyle& a, const RenderStyle& b);

diff --git a/Source/WebCore/animation/KeyframeEffect.cpp b/Source/WebCore/animation/KeyframeEffect.cpp
@@ -1546,22 +1546,18 @@ void KeyframeEffect::setAnimatedPropertiesInStyle(RenderStyle& targetStyle, doub
                     CSSPropertyAnimation::blendProperty(*this, property, endKeyframeStyle, targetStyle, *endKeyframe.style(), 1, endKeyframeCompositeOperation);
             }
 
-            // FIXME: handle custom properties here as well.
-            if (std::holds_alternative<CSSPropertyID>(property)) {
-                auto cssPropertyId = std::get<CSSPropertyID>(property);
-                // If this keyframe effect has an iteration composite operation of accumulate,
-                if (m_iterationCompositeOperation == IterationCompositeOperation::Accumulate && currentIteration && CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration(cssPropertyId, startKeyframeStyle, endKeyframeStyle)) {
-                    usedBlendingForAccumulativeIteration = true;
-                    // apply the following step current iteration times:
-                    for (auto i = 0; i < currentIteration; ++i) {
-                        // replace the property value of target property on keyframe with the result of combining the
-                        // property value on the final keyframe in property-specific keyframes (Va) with the property
-                        // value on keyframe (Vb) using the accumulation procedure defined for target property.
-                        if (!startKeyframe.key() && !hasImplicitZeroKeyframe)
-                            CSSPropertyAnimation::blendProperty(*this, cssPropertyId, startKeyframeStyle, *endKeyframe.style(), startKeyframeStyle, 1, CompositeOperation::Accumulate);
-                        if (endKeyframe.key() == 1 && !hasImplicitOneKeyframe)
-                            CSSPropertyAnimation::blendProperty(*this, cssPropertyId, endKeyframeStyle, *endKeyframe.style(), endKeyframeStyle, 1, CompositeOperation::Accumulate);
-                    }
+            // If this keyframe effect has an iteration composite operation of accumulate,
+            if (m_iterationCompositeOperation == IterationCompositeOperation::Accumulate && currentIteration && CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration(*this, property, startKeyframeStyle, endKeyframeStyle)) {
+                usedBlendingForAccumulativeIteration = true;
+                // apply the following step current iteration times:
+                for (auto i = 0; i < currentIteration; ++i) {
+                    // replace the property value of target property on keyframe with the result of combining the
+                    // property value on the final keyframe in property-specific keyframes (Va) with the property
+                    // value on keyframe (Vb) using the accumulation procedure defined for target property.
+                    if (!startKeyframe.key() && !hasImplicitZeroKeyframe)
+                        CSSPropertyAnimation::blendProperty(*this, property, startKeyframeStyle, *endKeyframe.style(), startKeyframeStyle, 1, CompositeOperation::Accumulate);
+                    if (endKeyframe.key() == 1 && !hasImplicitOneKeyframe)
+                        CSSPropertyAnimation::blendProperty(*this, property, endKeyframeStyle, *endKeyframe.style(), endKeyframeStyle, 1, CompositeOperation::Accumulate);
                 }
             }
         }