Animation: Allow CC per-curve timing functions to be steps timing functions

Source/core/animation/CompositorAnimations.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 155 matching lines @@
             default:
                 return false;
             }
         }
     }
 
     CompositorAnimationsImpl::CompositorTiming out;
     if (!CompositorAnimationsImpl::convertTimingForCompositor(timing, 0, out, playerPlaybackRate))
         return false;
 
-    if (timing.timingFunction->type() != TimingFunction::LinearFunction) {
-        // Checks the of size of KeyframeVector instead of PropertySpecificKeyframeVector.
+    if (timing.timingFunction->type() == TimingFunction::CubicBezierFunction) {
+        // FIXME: Fix compositor timing functions to accept inputs outside of

[dstockwell, 2015/01/05 23:13:24]

Does this fixme apply to line 187? Should be moved closer.

[loyso (OOO), 2015/01/06 00:45:29]

It's related to the whole block. See my comment below.

+        // [0,1].
+        const CubicBezierTimingFunction& cubic = toCubicBezierTimingFunction(*timing.timingFunction);
         const KeyframeVector& keyframes = keyframeEffect.getFrames();
-        if (keyframes.size() == 2 && keyframes.first()->easing().type() == TimingFunction::LinearFunction)
-            return true;
+        double startRange = 0;

[dstockwell, 2015/01/05 23:13:24]

start/end should be min/max

[loyso (OOO), 2015/01/06 00:45:29]

Agreed. In fact, this code is deleted in the upcoming CL (see the link below).
Should I fix this nit? :)

+        double endRange = 1;
+        cubic.range(&startRange, &endRange);
 
-        // FIXME: Support non-linear timing functions in the compositor for
-        // more than two keyframes.
-        return false;
+        ASSERT(keyframes.size() >= 2);
+        if ((startRange < 0 || endRange > 1) && (keyframes.first()->easing().type() != TimingFunction::LinearFunction || keyframes[keyframes.size() - 2]->easing().type() != TimingFunction::LinearFunction))

[dstockwell, 2015/01/05 23:13:24]

Can you add a comment to explain this? If it's related to the fixme from above,
why don't we care about the timing functions on the keyframes not tested here?

[loyso (OOO), 2015/01/06 00:45:29]

Yes, this whole block is related to FIXME from above. If per-curve timing
function produces time out of range [0,1] then it query the first key for
extrapolated values for t<0 and the last key for t>1.

I could put a comment, but we already have this FIXME fixed and under your
review :) It's here https://codereview.chromium.org/787353003/patch/1/10002
(removes this whole block)

[dstockwell, 2015/01/06 03:33:49]

Ahh, yes makes sense.
OK, no need then.

+            return false;
     }
 
     return true;
 }
 
 bool CompositorAnimations::canStartAnimationOnCompositor(const Element& element)
 {
     return element.renderer() && element.renderer()->compositingState() == PaintsIntoOwnBacking;
 }
 
@@ skipping 75 matching lines @@
     out.scaledTimeOffset = -timing.startDelay + timeOffset;
     out.playbackRate = timing.playbackRate * playerPlaybackRate;
     out.fillMode = timing.fillMode == Timing::FillModeAuto ? Timing::FillModeNone : timing.fillMode;
     out.iterationStart = timing.iterationStart;
     out.assertValid();
     return true;
 }
 
 namespace {
 
+void getCubicBezierTimingFunctionParameters(const TimingFunction& timingFunction, bool& outCustom,
+    WebCompositorAnimationCurve::TimingFunctionType& outEaseSubType,
+    double& outX1, double& outY1, double& outX2, double& outY2)
+{
+    const CubicBezierTimingFunction& cubic = toCubicBezierTimingFunction(timingFunction);
+    outCustom = false;
+
+    switch (cubic.subType()) {
+    case CubicBezierTimingFunction::Ease:
+        outEaseSubType = WebCompositorAnimationCurve::TimingFunctionTypeEase;
+        break;
+    case CubicBezierTimingFunction::EaseIn:
+        outEaseSubType = WebCompositorAnimationCurve::TimingFunctionTypeEaseIn;
+        break;
+    case CubicBezierTimingFunction::EaseOut:
+        outEaseSubType = WebCompositorAnimationCurve::TimingFunctionTypeEaseOut;
+        break;
+    case CubicBezierTimingFunction::EaseInOut:
+        outEaseSubType = WebCompositorAnimationCurve::TimingFunctionTypeEaseInOut;
+        break;
+    case CubicBezierTimingFunction::Custom:
+        outCustom = true;
+        outX1 = cubic.x1();
+        outY1 = cubic.y1();
+        outX2 = cubic.x2();
+        outY2 = cubic.y2();
+        break;
+    default:
+        ASSERT_NOT_REACHED();
+    }
+}
+
+void getStepsTimingFunctionParameters(const TimingFunction& timingFunction, int& outSteps, float& outStepsStartOffset)
+{
+    const StepsTimingFunction& steps = toStepsTimingFunction(timingFunction);
+
+    outSteps = steps.numberOfSteps();
+    switch (steps.stepAtPosition()) {
+    case StepsTimingFunction::Start:
+        outStepsStartOffset = 1;
+        break;
+    case StepsTimingFunction::Middle:
+        outStepsStartOffset = 0.5;
+        break;
+    case StepsTimingFunction::End:
+        outStepsStartOffset = 0;
+        break;
+    default:
+        ASSERT_NOT_REACHED();
+    }
+}
+
 template<typename PlatformAnimationCurveType, typename PlatformAnimationKeyframeType>
 void addKeyframeWithTimingFunction(PlatformAnimationCurveType& curve, const PlatformAnimationKeyframeType& keyframe, const TimingFunction* timingFunction)
 {
     if (!timingFunction) {
         curve.add(keyframe);
         return;
     }
 
     switch (timingFunction->type()) {
     case TimingFunction::LinearFunction:
         curve.add(keyframe, WebCompositorAnimationCurve::TimingFunctionTypeLinear);
-        return;
+        break;
 
     case TimingFunction::CubicBezierFunction: {
-        const CubicBezierTimingFunction* cubic = toCubicBezierTimingFunction(timingFunction);
+        bool custom;
+        WebCompositorAnimationCurve::TimingFunctionType easeSubType;
+        double x1, y1;
+        double x2, y2;
+        getCubicBezierTimingFunctionParameters(*timingFunction, custom, easeSubType, x1, y1, x2, y2);
 
-        if (cubic->subType() == CubicBezierTimingFunction::Custom) {
-            curve.add(keyframe, cubic->x1(), cubic->y1(), cubic->x2(), cubic->y2());
-        } else {
+        if (custom)
+            curve.add(keyframe, x1, y1, x2, y2);
+        else
+            curve.add(keyframe, easeSubType);

[dstockwell, 2015/01/05 23:13:24]

Why not always specify the control points?

[loyso (OOO), 2015/01/06 00:45:29]

That would be a lose of information. At this stage I want to keep the CC part in
structure as close as possible to blink implementation (to get rid of one of
them finally). blink::CubicBezierTimingFunction stores it's subtype. So I want
to store it the same way.

+        break;
+    }
 
-            WebCompositorAnimationCurve::TimingFunctionType easeType;
-            switch (cubic->subType()) {
-            case CubicBezierTimingFunction::Ease:
-                easeType = WebCompositorAnimationCurve::TimingFunctionTypeEase;
-                break;
-            case CubicBezierTimingFunction::EaseIn:
-                easeType = WebCompositorAnimationCurve::TimingFunctionTypeEaseIn;
-                break;
-            case CubicBezierTimingFunction::EaseOut:
-                easeType = WebCompositorAnimationCurve::TimingFunctionTypeEaseOut;
-                break;
-            case CubicBezierTimingFunction::EaseInOut:
-                easeType = WebCompositorAnimationCurve::TimingFunctionTypeEaseInOut;
-                break;
+    case TimingFunction::StepsFunction: {
+        int steps;
+        float stepsStartOffset;
+        getStepsTimingFunctionParameters(*timingFunction, steps, stepsStartOffset);
 
-            // Custom Bezier are handled seperately.
-            case CubicBezierTimingFunction::Custom:
-            default:
-                ASSERT_NOT_REACHED();
-                return;
-            }
+        curve.add(keyframe, steps, stepsStartOffset);
+        break;
+    }
 
-            curve.add(keyframe, easeType);
-        }
+    default:
+        ASSERT_NOT_REACHED();
+    }
+}
+
+template <typename PlatformAnimationCurveType>
+void setTimingFunctionOnCurve(PlatformAnimationCurveType& curve, TimingFunction* timingFunction)
+{
+    if (!timingFunction) {
+        curve.setLinearTimingFunction();
         return;
     }
 
-    case TimingFunction::StepsFunction:
-        const StepsTimingFunction* steps = toStepsTimingFunction(timingFunction);
+    switch (timingFunction->type()) {
+    case TimingFunction::LinearFunction:
+        curve.setLinearTimingFunction();
+        break;
 
+    case TimingFunction::CubicBezierFunction: {
+        bool custom;
+        WebCompositorAnimationCurve::TimingFunctionType easeSubType;
+        double x1, y1;
+        double x2, y2;
+        getCubicBezierTimingFunctionParameters(*timingFunction, custom, easeSubType, x1, y1, x2, y2);
+
+        if (custom)
+            curve.setCubicBezierTimingFunction(x1, y1, x2, y2);
+        else
+            curve.setCubicBezierTimingFunction(easeSubType);
+        break;
+    }
+
+    case TimingFunction::StepsFunction: {
+        int steps;
         float stepsStartOffset;
-        switch (steps->stepAtPosition()) {
-        case StepsTimingFunction::Start:
-            stepsStartOffset = 1;
-            break;
-        case StepsTimingFunction::Middle:
-            stepsStartOffset = 0.5;
-            break;
-        case StepsTimingFunction::End:
-            stepsStartOffset = 0;
-            break;
-        default:
-            ASSERT_NOT_REACHED();
-            return;
-        }
-        curve.add(keyframe, steps->numberOfSteps(), stepsStartOffset);
-        return;
+        getStepsTimingFunctionParameters(*timingFunction, steps, stepsStartOffset);
+
+        curve.setStepsTimingFunction(steps, stepsStartOffset);
+        break;
+    }
+
+    default:
+        ASSERT_NOT_REACHED();
     }
 }
 
 } // namespace anoymous
 
 void CompositorAnimationsImpl::addKeyframesToCurve(WebCompositorAnimationCurve& curve, const PropertySpecificKeyframeVector& keyframes, const Timing& timing)
 {
     auto* lastKeyframe = keyframes.last().get();
     for (const auto& keyframe : keyframes) {
         const TimingFunction* keyframeTimingFunction = 0;
         if (keyframe != lastKeyframe) { // Ignore timing function of last frame.
-            if (keyframes.size() == 2 && keyframes.first()->easing().type() == TimingFunction::LinearFunction)
-                keyframeTimingFunction = timing.timingFunction.get();
-            else
-                keyframeTimingFunction = &keyframe->easing();
+            keyframeTimingFunction = &keyframe->easing();
         }
 
         // FIXME: This relies on StringKeyframes being eagerly evaluated, which will
         // not happen eventually. Instead we should extract the CSSValue here
         // and convert using another set of toAnimatableXXXOperations functions.
         const AnimatableValue* value = keyframe->getAnimatableValue().get();
 
         switch (curve.type()) {
         case WebCompositorAnimationCurve::AnimationCurveTypeFilter: {
             OwnPtr<WebFilterOperations> ops = adoptPtr(Platform::current()->compositorSupport()->createFilterOperations());
@@ skipping 39 matching lines @@
         getKeyframeValuesForProperty(&effect, property, compositorTiming.scaledDuration, values);
 
         WebCompositorAnimation::TargetProperty targetProperty;
         OwnPtr<WebCompositorAnimationCurve> curve;
         switch (property) {
         case CSSPropertyOpacity: {
             targetProperty = WebCompositorAnimation::TargetPropertyOpacity;
 
             WebFloatAnimationCurve* floatCurve = Platform::current()->compositorSupport()->createFloatAnimationCurve();
             addKeyframesToCurve(*floatCurve, values, timing);
+            setTimingFunctionOnCurve(*floatCurve, timing.timingFunction.get());
             curve = adoptPtr(floatCurve);
             break;
         }
         case CSSPropertyWebkitFilter: {
             targetProperty = WebCompositorAnimation::TargetPropertyFilter;
             WebFilterAnimationCurve* filterCurve = Platform::current()->compositorSupport()->createFilterAnimationCurve();
             addKeyframesToCurve(*filterCurve, values, timing);
+            setTimingFunctionOnCurve(*filterCurve, timing.timingFunction.get());
             curve = adoptPtr(filterCurve);
             break;
         }
         case CSSPropertyTransform: {
             targetProperty = WebCompositorAnimation::TargetPropertyTransform;
             WebTransformAnimationCurve* transformCurve = Platform::current()->compositorSupport()->createTransformAnimationCurve();
             addKeyframesToCurve(*transformCurve, values, timing);
+            setTimingFunctionOnCurve(*transformCurve, timing.timingFunction.get());
             curve = adoptPtr(transformCurve);
             break;
         }
         default:
             ASSERT_NOT_REACHED();
             continue;
         }
         ASSERT(curve.get());
 
         OwnPtr<WebCompositorAnimation> animation = adoptPtr(Platform::current()->compositorSupport()->createAnimation(*curve, targetProperty, group, 0));
@@ skipping 38 matching lines @@
             break;
         default:
             ASSERT_NOT_REACHED();
         }
         animations.append(animation.release());
     }
     ASSERT(!animations.isEmpty());
 }
 
 } // namespace blink