[webkit-changes] [230100] trunk

Thu, 29 Mar 2018 22:44:19 -0700

Title: [230100] trunk
Revision
230100
Author
grao...@webkit.org
Date
2018-03-29 22:41:47 -0700 (Thu, 29 Mar 2018)

Log Message

[Web Animations] Correctly obtain the timing function for a given keyframe
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:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (230099 => 230100)


--- trunk/LayoutTests/ChangeLog	2018-03-30 05:33:17 UTC (rev 230099)
+++ trunk/LayoutTests/ChangeLog	2018-03-30 05:41:47 UTC (rev 230100)
@@ -1,3 +1,17 @@
+2018-03-29  Antoine Quint  <grao...@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  <ryanhad...@apple.com>
 
         Unreviewed, rolling out r230087.

Modified: trunk/LayoutTests/animations/keyframe-timing-functions-transform.html (230099 => 230100)


--- trunk/LayoutTests/animations/keyframe-timing-functions-transform.html	2018-03-30 05:33:17 UTC (rev 230099)
+++ trunk/LayoutTests/animations/keyframe-timing-functions-transform.html	2018-03-30 05:41:47 UTC (rev 230100)
@@ -1,4 +1,4 @@
-<!DOCTYPE html>
+<!DOCTYPE html><!-- webkit-test-runner [ enableCSSAnimationsAndCSSTransitionsBackedByWebAnimations=true ] -->
 
 <html>
 <head>

Modified: trunk/LayoutTests/animations/keyframe-timing-functions.html (230099 => 230100)


--- trunk/LayoutTests/animations/keyframe-timing-functions.html	2018-03-30 05:33:17 UTC (rev 230099)
+++ trunk/LayoutTests/animations/keyframe-timing-functions.html	2018-03-30 05:41:47 UTC (rev 230100)
@@ -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>

Modified: trunk/LayoutTests/animations/keyframe-timing-functions2.html (230099 => 230100)


--- trunk/LayoutTests/animations/keyframe-timing-functions2.html	2018-03-30 05:33:17 UTC (rev 230099)
+++ trunk/LayoutTests/animations/keyframe-timing-functions2.html	2018-03-30 05:41:47 UTC (rev 230100)
@@ -1,4 +1,4 @@
-<!DOCTYPE html>
+<!DOCTYPE html><!-- webkit-test-runner [ enableCSSAnimationsAndCSSTransitionsBackedByWebAnimations=true ] -->
 
 <html>
 <head>

Modified: trunk/LayoutTests/animations/missing-keyframe-properties-timing-function.html (230099 => 230100)


--- trunk/LayoutTests/animations/missing-keyframe-properties-timing-function.html	2018-03-30 05:33:17 UTC (rev 230099)
+++ trunk/LayoutTests/animations/missing-keyframe-properties-timing-function.html	2018-03-30 05:41:47 UTC (rev 230100)
@@ -1,4 +1,4 @@
-<!DOCTYPE html>
+<!DOCTYPE html><!-- webkit-test-runner [ enableCSSAnimationsAndCSSTransitionsBackedByWebAnimations=true ] -->
 <html>
 <head>
   <style type="text/css" media="screen">

Modified: trunk/Source/WebCore/ChangeLog (230099 => 230100)


--- trunk/Source/WebCore/ChangeLog	2018-03-30 05:33:17 UTC (rev 230099)
+++ trunk/Source/WebCore/ChangeLog	2018-03-30 05:41:47 UTC (rev 230100)
@@ -1,3 +1,42 @@
+2018-03-29  Antoine Quint  <grao...@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  <rn...@webkit.org>
 
         Copying a list from Microsoft Word to TinyMCE fails when mso-list is on tags other than P

Modified: trunk/Source/WebCore/animation/KeyframeEffectReadOnly.cpp (230099 => 230100)


--- trunk/Source/WebCore/animation/KeyframeEffectReadOnly.cpp	2018-03-30 05:33:17 UTC (rev 230099)
+++ trunk/Source/WebCore/animation/KeyframeEffectReadOnly.cpp	2018-03-30 05:41:47 UTC (rev 230100)
@@ -1130,7 +1130,7 @@
     // 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;
     }

Modified: trunk/Source/WebCore/css/StyleResolver.cpp (230099 => 230100)


--- trunk/Source/WebCore/css/StyleResolver.cpp	2018-03-30 05:33:17 UTC (rev 230099)
+++ trunk/Source/WebCore/css/StyleResolver.cpp	2018-03-30 05:41:47 UTC (rev 230100)
@@ -522,6 +522,8 @@
             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));
         }
     }

Modified: trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp (230099 => 230100)


--- trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp	2018-03-30 05:33:17 UTC (rev 230099)
+++ trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp	2018-03-30 05:41:47 UTC (rev 230100)
@@ -151,7 +151,7 @@
     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)

Modified: trunk/Source/WebCore/platform/animation/TimingFunction.cpp (230099 => 230100)


--- trunk/Source/WebCore/platform/animation/TimingFunction.cpp	2018-03-30 05:33:17 UTC (rev 230099)
+++ trunk/Source/WebCore/platform/animation/TimingFunction.cpp	2018-03-30 05:41:47 UTC (rev 230100)
@@ -130,51 +130,56 @@
     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

Modified: trunk/Source/WebCore/platform/animation/TimingFunction.h (230099 => 230100)


--- trunk/Source/WebCore/platform/animation/TimingFunction.h	2018-03-30 05:33:17 UTC (rev 230099)
+++ trunk/Source/WebCore/platform/animation/TimingFunction.h	2018-03-30 05:41:47 UTC (rev 230100)
@@ -24,6 +24,7 @@
 
 #pragma once
 
+#include "CSSValue.h"
 #include "ExceptionOr.h"
 #include <wtf/Ref.h>
 #include <wtf/RefCounted.h>
@@ -53,6 +54,7 @@
     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;

Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.cpp (230099 => 230100)


--- trunk/Source/WebCore/rendering/RenderLayerBacking.cpp	2018-03-30 05:33:17 UTC (rev 230099)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.cpp	2018-03-30 05:41:47 UTC (rev 230100)
@@ -2732,7 +2732,7 @@
         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))

Modified: trunk/Source/WebCore/rendering/style/KeyframeList.cpp (230099 => 230100)


--- trunk/Source/WebCore/rendering/style/KeyframeList.cpp	2018-03-30 05:33:17 UTC (rev 230099)
+++ trunk/Source/WebCore/rendering/style/KeyframeList.cpp	2018-03-30 05:41:47 UTC (rev 230100)
@@ -27,21 +27,6 @@
 
 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()

Modified: trunk/Source/WebCore/rendering/style/KeyframeList.h (230099 => 230100)


--- trunk/Source/WebCore/rendering/style/KeyframeList.h	2018-03-30 05:33:17 UTC (rev 230099)
+++ trunk/Source/WebCore/rendering/style/KeyframeList.h	2018-03-30 05:41:47 UTC (rev 230100)
@@ -52,12 +52,14 @@
     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 {

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to