[Web Animations] Correctly obtain the timing function for a given keyframe
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Mar 2018 05:41:47 +0000 (05:41 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Mar 2018 05:41:47 +0000 (05:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184146

Reviewed by Dean Jackson.

Source/WebCore:

The way we would get the timing function for a given KeyframeValue stored in a KeyframeList was really suboptimal.
When keyframes were created, we would set the animated element's style on each keyframe, and set keyframe-specific
properties and values on top. When figuring out the timing function for a KeyframeValue, we would look at its render
style, go through its list of animations, which could include animations that are irrelevant to this specific keyframe
list since all animations from the animated element are referenced, and we would have to look up the correct animation
by name and get the timing function, even though the timing function stored on the animation was now specific to this
particular keyframe.

We now simply set a m_timingFunction member on a KeyframeValue, which is null if no explicit animation-timing-function
was provided for this keyframe in CSS, and otherwise set to a valid TimingFunction.

This fixes our behavior for a 4 existing animation tests when opted into the CSS Animations and CSS Transitions as
Web Animations feature.

* animation/KeyframeEffectReadOnly.cpp:
(WebCore::KeyframeEffectReadOnly::timingFunctionForKeyframeAtIndex):
* css/StyleResolver.cpp:
(WebCore::StyleResolver::keyframeStylesForAnimation):
* page/animation/KeyframeAnimation.cpp:
(WebCore::KeyframeAnimation::fetchIntervalEndpointsForProperty const):
* platform/animation/TimingFunction.cpp:
(WebCore::TimingFunction::createFromCSSText):
(WebCore::TimingFunction::createFromCSSValue):
* platform/animation/TimingFunction.h:
* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::startAnimation):
* rendering/style/KeyframeList.cpp:
(WebCore::KeyframeValue::timingFunction const): Deleted.
* rendering/style/KeyframeList.h:
(WebCore::KeyframeValue::timingFunction const):
(WebCore::KeyframeValue::setTimingFunction):

LayoutTests:

Make 4 tests opt into CSS Animations and CSS Transitions as Web Animations.

* animations/keyframe-timing-functions-transform.html:
* animations/keyframe-timing-functions.html:
* animations/keyframe-timing-functions2.html:
* animations/missing-keyframe-properties-timing-function.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230100 268f45cc-cd09-0410-ab3c-d52691b4dbfc

14 files changed:
LayoutTests/ChangeLog
LayoutTests/animations/keyframe-timing-functions-transform.html
LayoutTests/animations/keyframe-timing-functions.html
LayoutTests/animations/keyframe-timing-functions2.html
LayoutTests/animations/missing-keyframe-properties-timing-function.html
Source/WebCore/ChangeLog
Source/WebCore/animation/KeyframeEffectReadOnly.cpp
Source/WebCore/css/StyleResolver.cpp
Source/WebCore/page/animation/KeyframeAnimation.cpp
Source/WebCore/platform/animation/TimingFunction.cpp
Source/WebCore/platform/animation/TimingFunction.h
Source/WebCore/rendering/RenderLayerBacking.cpp
Source/WebCore/rendering/style/KeyframeList.cpp
Source/WebCore/rendering/style/KeyframeList.h

index 50e4aa4..7207a83 100644 (file)
@@ -1,3 +1,17 @@
+2018-03-29  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Correctly obtain the timing function for a given keyframe
+        https://bugs.webkit.org/show_bug.cgi?id=184146
+
+        Reviewed by Dean Jackson.
+
+        Make 4 tests opt into CSS Animations and CSS Transitions as Web Animations.
+
+        * animations/keyframe-timing-functions-transform.html:
+        * animations/keyframe-timing-functions.html:
+        * animations/keyframe-timing-functions2.html:
+        * animations/missing-keyframe-properties-timing-function.html:
+
 2018-03-29  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r230087.
index c142d79..e89b82d 100644 (file)
@@ -1,4 +1,4 @@
-<!DOCTYPE html>
+<!DOCTYPE html><!-- webkit-test-runner [ enableCSSAnimationsAndCSSTransitionsBackedByWebAnimations=true ] -->
 
 <html>
 <head>
index 05efc40..420ea07 100644 (file)
@@ -1,5 +1,4 @@
-<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
-   "http://www.w3.org/TR/html4/loose.dtd">
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd"><!-- webkit-test-runner [ enableCSSAnimationsAndCSSTransitionsBackedByWebAnimations=true ] -->
 
 <html lang="en">
 <head>
index 18ae4a7..6e80101 100644 (file)
@@ -1,4 +1,4 @@
-<!DOCTYPE html>
+<!DOCTYPE html><!-- webkit-test-runner [ enableCSSAnimationsAndCSSTransitionsBackedByWebAnimations=true ] -->
 
 <html>
 <head>
index 53927c2..d1e2925 100644 (file)
@@ -1,4 +1,4 @@
-<!DOCTYPE html>
+<!DOCTYPE html><!-- webkit-test-runner [ enableCSSAnimationsAndCSSTransitionsBackedByWebAnimations=true ] -->
 <html>
 <head>
   <style type="text/css" media="screen">
index 5de34cf..3627f41 100644 (file)
@@ -1,3 +1,42 @@
+2018-03-29  Antoine Quint  <graouts@apple.com>
+
+        [Web Animations] Correctly obtain the timing function for a given keyframe
+        https://bugs.webkit.org/show_bug.cgi?id=184146
+
+        Reviewed by Dean Jackson.
+
+        The way we would get the timing function for a given KeyframeValue stored in a KeyframeList was really suboptimal.
+        When keyframes were created, we would set the animated element's style on each keyframe, and set keyframe-specific
+        properties and values on top. When figuring out the timing function for a KeyframeValue, we would look at its render
+        style, go through its list of animations, which could include animations that are irrelevant to this specific keyframe
+        list since all animations from the animated element are referenced, and we would have to look up the correct animation
+        by name and get the timing function, even though the timing function stored on the animation was now specific to this
+        particular keyframe.
+
+        We now simply set a m_timingFunction member on a KeyframeValue, which is null if no explicit animation-timing-function
+        was provided for this keyframe in CSS, and otherwise set to a valid TimingFunction.
+
+        This fixes our behavior for a 4 existing animation tests when opted into the CSS Animations and CSS Transitions as
+        Web Animations feature.
+
+        * animation/KeyframeEffectReadOnly.cpp:
+        (WebCore::KeyframeEffectReadOnly::timingFunctionForKeyframeAtIndex):
+        * css/StyleResolver.cpp:
+        (WebCore::StyleResolver::keyframeStylesForAnimation):
+        * page/animation/KeyframeAnimation.cpp:
+        (WebCore::KeyframeAnimation::fetchIntervalEndpointsForProperty const):
+        * platform/animation/TimingFunction.cpp:
+        (WebCore::TimingFunction::createFromCSSText):
+        (WebCore::TimingFunction::createFromCSSValue):
+        * platform/animation/TimingFunction.h:
+        * rendering/RenderLayerBacking.cpp:
+        (WebCore::RenderLayerBacking::startAnimation):
+        * rendering/style/KeyframeList.cpp:
+        (WebCore::KeyframeValue::timingFunction const): Deleted.
+        * rendering/style/KeyframeList.h:
+        (WebCore::KeyframeValue::timingFunction const):
+        (WebCore::KeyframeValue::setTimingFunction):
+
 2018-03-29  Ryosuke Niwa  <rniwa@webkit.org>
 
         Copying a list from Microsoft Word to TinyMCE fails when mso-list is on tags other than P
index 7dd00f5..016be0a 100644 (file)
@@ -1130,7 +1130,7 @@ TimingFunction* KeyframeEffectReadOnly::timingFunctionForKeyframeAtIndex(size_t
     // If we're dealing with a CSS Animation, the timing function is specified either on the keyframe itself,
     // or failing that on the backing Animation object which defines the default for all keyframes.
     if (is<CSSAnimation>(effectAnimation)) {
-        if (auto* timingFunction = m_blendingKeyframes[index].timingFunction(downcast<CSSAnimation>(effectAnimation)->animationName()))
+        if (auto* timingFunction = m_blendingKeyframes[index].timingFunction())
             return timingFunction;
     }
 
index 159f681..051ed38 100644 (file)
@@ -522,6 +522,8 @@ void StyleResolver::keyframeStylesForAnimation(const Element& element, const Ren
             KeyframeValue keyframeValue(0, nullptr);
             keyframeValue.setStyle(styleForKeyframe(elementStyle, keyframe.ptr(), keyframeValue));
             keyframeValue.setKey(key);
+            if (auto timingFunctionCSSValue = keyframe->properties().getPropertyCSSValue(CSSPropertyAnimationTimingFunction))
+                keyframeValue.setTimingFunction(TimingFunction::createFromCSSValue(*timingFunctionCSSValue.get()));
             list.insert(WTFMove(keyframeValue));
         }
     }
index f86e9f8..091c91c 100644 (file)
@@ -151,7 +151,7 @@ void KeyframeAnimation::fetchIntervalEndpointsForProperty(CSSPropertyID property
     double offset = prevKeyframe.key();
     double scale = 1.0 / (nextIndex == prevIndex ?  1 : (nextKeyframe.key() - prevKeyframe.key()));
 
-    prog = progress(scale, offset, prevKeyframe.timingFunction(name()));
+    prog = progress(scale, offset, prevKeyframe.timingFunction());
 }
 
 bool KeyframeAnimation::animate(CompositeAnimation& compositeAnimation, const RenderStyle& targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle)
index 941d4ad..8c972d6 100644 (file)
@@ -130,51 +130,56 @@ ExceptionOr<RefPtr<TimingFunction>> TimingFunction::createFromCSSText(const Stri
     cssString.append(cssText);
     auto styleProperties = MutableStyleProperties::create();
     styleProperties->parseDeclaration(cssString.toString(), CSSParserContext(HTMLStandardMode));
-    auto cssValue = styleProperties->getPropertyCSSValue(CSSPropertyAnimationTimingFunction);
 
-    if (!cssValue)
-        return Exception { TypeError };
+    if (auto cssValue = styleProperties->getPropertyCSSValue(CSSPropertyAnimationTimingFunction)) {
+        if (auto timingFunction = createFromCSSValue(*cssValue.get()))
+            return WTFMove(timingFunction);
+    }
+    
+    return Exception { TypeError };
+}
 
-    auto& value = *cssValue.get();
+RefPtr<TimingFunction> TimingFunction::createFromCSSValue(const CSSValue& value)
+{
     if (is<CSSPrimitiveValue>(value)) {
         switch (downcast<CSSPrimitiveValue>(value).valueID()) {
         case CSSValueLinear:
-            return { LinearTimingFunction::create() };
+            return LinearTimingFunction::create();
         case CSSValueEase:
-            return { CubicBezierTimingFunction::create() };
+            return CubicBezierTimingFunction::create();
         case CSSValueEaseIn:
-            return { CubicBezierTimingFunction::create(CubicBezierTimingFunction::EaseIn) };
+            return CubicBezierTimingFunction::create(CubicBezierTimingFunction::EaseIn);
         case CSSValueEaseOut:
-            return { CubicBezierTimingFunction::create(CubicBezierTimingFunction::EaseOut) };
+            return CubicBezierTimingFunction::create(CubicBezierTimingFunction::EaseOut);
         case CSSValueEaseInOut:
-            return { CubicBezierTimingFunction::create(CubicBezierTimingFunction::EaseInOut) };
+            return CubicBezierTimingFunction::create(CubicBezierTimingFunction::EaseInOut);
         case CSSValueStepStart:
-            return { StepsTimingFunction::create(1, true) };
+            return StepsTimingFunction::create(1, true);
         case CSSValueStepEnd:
-            return { StepsTimingFunction::create(1, false) };
+            return StepsTimingFunction::create(1, false);
         default:
-            return Exception { TypeError };
+            return nullptr;
         }
     }
 
     if (is<CSSCubicBezierTimingFunctionValue>(value)) {
         auto& cubicTimingFunction = downcast<CSSCubicBezierTimingFunctionValue>(value);
-        return { CubicBezierTimingFunction::create(cubicTimingFunction.x1(), cubicTimingFunction.y1(), cubicTimingFunction.x2(), cubicTimingFunction.y2()) };
+        return CubicBezierTimingFunction::create(cubicTimingFunction.x1(), cubicTimingFunction.y1(), cubicTimingFunction.x2(), cubicTimingFunction.y2());
     }
     if (is<CSSStepsTimingFunctionValue>(value)) {
         auto& stepsTimingFunction = downcast<CSSStepsTimingFunctionValue>(value);
-        return { StepsTimingFunction::create(stepsTimingFunction.numberOfSteps(), stepsTimingFunction.stepAtStart()) };
+        return StepsTimingFunction::create(stepsTimingFunction.numberOfSteps(), stepsTimingFunction.stepAtStart());
     }
     if (is<CSSFramesTimingFunctionValue>(value)) {
         auto& framesTimingFunction = downcast<CSSFramesTimingFunctionValue>(value);
-        return { FramesTimingFunction::create(framesTimingFunction.numberOfFrames()) };
+        return FramesTimingFunction::create(framesTimingFunction.numberOfFrames());
     }
     if (is<CSSSpringTimingFunctionValue>(value)) {
         auto& springTimingFunction = downcast<CSSSpringTimingFunctionValue>(value);
-        return { SpringTimingFunction::create(springTimingFunction.mass(), springTimingFunction.stiffness(), springTimingFunction.damping(), springTimingFunction.initialVelocity()) };
+        return SpringTimingFunction::create(springTimingFunction.mass(), springTimingFunction.stiffness(), springTimingFunction.damping(), springTimingFunction.initialVelocity());
     }
 
-    return Exception { TypeError };
+    return nullptr;
 }
 
 String TimingFunction::cssText() const
index b588edd..f09a593 100644 (file)
@@ -24,6 +24,7 @@
 
 #pragma once
 
+#include "CSSValue.h"
 #include "ExceptionOr.h"
 #include <wtf/Ref.h>
 #include <wtf/RefCounted.h>
@@ -53,6 +54,7 @@ public:
     bool operator!=(const TimingFunction& other) const { return !(*this == other); }
 
     static ExceptionOr<RefPtr<TimingFunction>> createFromCSSText(const String&);
+    static RefPtr<TimingFunction> createFromCSSValue(const CSSValue&);
     double transformTime(double, double, bool before = false) const;
     String cssText() const;
 
index d7dc176..b5bbf3e 100644 (file)
@@ -2732,7 +2732,7 @@ bool RenderLayerBacking::startAnimation(double timeOffset, const Animation* anim
         if (!keyframeStyle)
             continue;
             
-        auto* tf = currentKeyframe.timingFunction(keyframes.animationName());
+        auto* tf = currentKeyframe.timingFunction();
         
         bool isFirstOrLastKeyframe = key == 0 || key == 1;
         if ((hasTransform && isFirstOrLastKeyframe) || currentKeyframe.containsProperty(CSSPropertyTransform))
index 803c80d..3533e1d 100644 (file)
 
 namespace WebCore {
 
-TimingFunction* KeyframeValue::timingFunction(const AtomicString& name) const
-{
-    auto* keyframeStyle = style();
-    if (!keyframeStyle || !keyframeStyle->animations())
-        return nullptr;
-
-    for (size_t i = 0; i < keyframeStyle->animations()->size(); ++i) {
-        const Animation& animation = keyframeStyle->animations()->animation(i);
-        if (name == animation.name())
-            return animation.timingFunction();
-    }
-
-    return nullptr;
-}
-
 KeyframeList::~KeyframeList() = default;
 
 void KeyframeList::clear()
index 85979a9..d66f298 100644 (file)
@@ -52,12 +52,14 @@ public:
     const RenderStyle* style() const { return m_style.get(); }
     void setStyle(std::unique_ptr<RenderStyle> style) { m_style = WTFMove(style); }
 
-    TimingFunction* timingFunction(const AtomicString& name) const;
-
+    TimingFunction* timingFunction() const { return m_timingFunction.get(); }
+    void setTimingFunction(const RefPtr<TimingFunction>& timingFunction) { m_timingFunction = timingFunction; }
+    
 private:
     double m_key;
     HashSet<CSSPropertyID> m_properties; // The properties specified in this keyframe.
     std::unique_ptr<RenderStyle> m_style;
+    RefPtr<TimingFunction> m_timingFunction;
 };
 
 class KeyframeList {