Bug 1332206 - Don't clamp steps timing functions outside [0, 1] range; r=hiro
authorBrian Birtles <birtles@gmail.com>
Wed, 08 Feb 2017 09:25:31 +0900
changeset 341324 e624d16a7d8f10549f0598be9819d60be1d65c9a
parent 341323 f85b73bd0790056fa17c5fdc5b148ee98d2ba896
child 341325 876e2bc7fc06bfa461d527de7262072cb014ec82
push id86684
push usercbook@mozilla.com
push dateWed, 08 Feb 2017 10:31:03 +0000
treeherdermozilla-inbound@c5b88e4e70f4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershiro
bugs1332206
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1332206 - Don't clamp steps timing functions outside [0, 1] range; r=hiro MozReview-Commit-ID: AiSdxQvU2mj
dom/animation/ComputedTimingFunction.cpp
testing/web-platform/meta/web-animations/animation-model/keyframe-effects/effect-value-transformed-distance.html.ini
--- a/dom/animation/ComputedTimingFunction.cpp
+++ b/dom/animation/ComputedTimingFunction.cpp
@@ -23,43 +23,48 @@ ComputedTimingFunction::Init(const nsTim
 }
 
 static inline double
 StepTiming(uint32_t aSteps,
            double aPortion,
            ComputedTimingFunction::BeforeFlag aBeforeFlag,
            nsTimingFunction::Type aType)
 {
-  MOZ_ASSERT(0.0 <= aPortion && aPortion <= 1.0, "out of range");
   MOZ_ASSERT(aType == nsTimingFunction::Type::StepStart ||
              aType == nsTimingFunction::Type::StepEnd, "invalid type");
 
-  if (aPortion == 1.0) {
-    return 1.0;
-  }
-
   // Calculate current step using step-end behavior
-  uint32_t step = uint32_t(aPortion * aSteps); // floor
+  int32_t step = floor(aPortion * aSteps);
 
   // step-start is one step ahead
   if (aType == nsTimingFunction::Type::StepStart) {
     step++;
   }
 
   // If the "before flag" is set and we are at a transition point,
-  // drop back a step (but only if we are not already at the zero point--
-  // we do this clamping here since |step| is an unsigned integer)
-  if (step != 0 &&
-      aBeforeFlag == ComputedTimingFunction::BeforeFlag::Set &&
+  // drop back a step
+  if (aBeforeFlag == ComputedTimingFunction::BeforeFlag::Set &&
       fmod(aPortion * aSteps, 1) == 0) {
     step--;
   }
 
   // Convert to a progress value
-  return double(step) / double(aSteps);
+  double result = double(step) / double(aSteps);
+
+  // We should not produce a result outside [0, 1] unless we have an
+  // input outside that range. This takes care of steps that would otherwise
+  // occur at boundaries.
+  if (result < 0.0 && aPortion >= 0.0) {
+    return 0.0;
+  }
+  if (result > 1.0 && aPortion <= 1.0) {
+    return 1.0;
+  }
+
+  return result;
 }
 
 double
 ComputedTimingFunction::GetValue(
     double aPortion,
     ComputedTimingFunction::BeforeFlag aBeforeFlag) const
 {
   if (HasSpline()) {
@@ -103,28 +108,16 @@ ComputedTimingFunction::GetValue(
       }
       // If we can't calculate a sensible tangent, don't extrapolate at all.
       return 1.0;
     }
 
     return mTimingFunction.GetSplineValue(aPortion);
   }
 
-  // Since we use endpoint-exclusive timing, the output of a steps(start) timing
-  // function when aPortion = 0.0 is the top of the first step. When aPortion is
-  // negative, however, we should use the bottom of the first step. We handle
-  // negative values of aPortion specially here since once we clamp aPortion
-  // to [0,1] below we will no longer be able to distinguish to the two cases.
-  if (aPortion < 0.0) {
-    return 0.0;
-  }
-
-  // Clamp in case of steps(end) and steps(start) for values greater than 1.
-  aPortion = clamped(aPortion, 0.0, 1.0);
-
   return StepTiming(mSteps, aPortion, aBeforeFlag, mType);
 }
 
 int32_t
 ComputedTimingFunction::Compare(const ComputedTimingFunction& aRhs) const
 {
   if (mType != aRhs.mType) {
     return int32_t(mType) - int32_t(aRhs.mType);
deleted file mode 100644
--- a/testing/web-platform/meta/web-animations/animation-model/keyframe-effects/effect-value-transformed-distance.html.ini
+++ /dev/null
@@ -1,13 +0,0 @@
-[effect-value-transformed-distance.html]
-  type: testharness
-  [step-start easing with input progress greater than 1]
-    expected: FAIL
-
-  [step-end easing with input progress greater than 2]
-    expected: FAIL
-
-  [step-start easing with input progress less than -1]
-    expected: FAIL
-
-  [step-end easing with input progress less than 0]
-    expected: FAIL