Bug 1496619 - part 7: Implement steps(jump-*) functions r=birtles

1. Add a new preference, layout.css.step-position-jump.enabled, for
   step(_, jump-*) timing functions.
2. We still keep JumpEnd and End tags, even though there is no difference
   between them. Therefore, we could disable the preference if needed.
3. Update the calculation of StepTiming to match the algorithm in the spec.
4. For servo, we implement the correct step function algorithm except
   for the handling of before_flag. This could be fixed later.

Depends on D9313

Differential Revision: https://phabricator.services.mozilla.com/D9314

--HG--
extra : moz-landing-system : lando

dom/animation/ComputedTimingFunction.cpp

@@ -55,34 +55,47 @@ StepTiming(const ComputedTimingFunction::StepFunc& aStepFunc,
            double aPortion,
            ComputedTimingFunction::BeforeFlag aBeforeFlag)
 {
-  // Calculate current step using step-end behavior
+  // Use the algorithm defined in the spec:
-  int32_t step = floor(aPortion * aStepFunc.mSteps);
+  // https://drafts.csswg.org/css-easing-1/#step-timing-function-algo
 
-  // step-start is one step ahead
+  // Calculate current step.
-  if (aStepFunc.mPos == StyleStepPosition::Start) {
+  int32_t currentStep = floor(aPortion * aStepFunc.mSteps);
-    step++;
+
+  // Increment current step if it is jump-start or start.
+  if (aStepFunc.mPos == StyleStepPosition::Start ||
+      aStepFunc.mPos == StyleStepPosition::JumpStart ||
+      aStepFunc.mPos == StyleStepPosition::JumpBoth) {
+    ++currentStep;
   }
 
   // If the "before flag" is set and we are at a transition point,
   // drop back a step
   if (aBeforeFlag == ComputedTimingFunction::BeforeFlag::Set &&
       fmod(aPortion * aStepFunc.mSteps, 1) == 0) {
-    step--;
+    --currentStep;
   }
 
-  // Convert to a progress value
-  double result = double(step) / double(aStepFunc.mSteps);
-
   // 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) {
+  if (aPortion >= 0.0 && currentStep < 0) {
-    return 0.0;
+    currentStep = 0;
   }
-  if (result > 1.0 && aPortion <= 1.0) {
+
-    return 1.0;
+  int32_t jumps = aStepFunc.mSteps;
+  if (aStepFunc.mPos == StyleStepPosition::JumpBoth) {
+    ++jumps;
+  } else if (aStepFunc.mPos == StyleStepPosition::JumpNone) {
+    --jumps;
   }
-  return result;
+
+  if (aPortion <= 1.0 && currentStep > jumps) {
+    currentStep = jumps;
+  }
+
+  // Convert to the output progress value.
+  MOZ_ASSERT(jumps > 0, "`jumps` should be a positive integer");
+  return double(currentStep) / double(jumps);
 }
 
 double

layout/style/nsStyleUtil.cpp

@@ -282,12 +282,24 @@ nsStyleUtil::AppendStepsTimingFunction(uint32_t aStepNumber,
   aResult.AppendLiteral("steps(");
   aResult.AppendInt(aStepNumber);
   switch (aStepPos) {
+    case StyleStepPosition::JumpStart:
+      aResult.AppendLiteral(", jump-start)");
+      break;
+    case StyleStepPosition::JumpNone:
+      aResult.AppendLiteral(", jump-none)");
+      break;
+    case StyleStepPosition::JumpBoth:
+      aResult.AppendLiteral(", jump-both)");
+      break;
     case StyleStepPosition::Start:
       aResult.AppendLiteral(", start)");
       break;
+    case StyleStepPosition::JumpEnd:
     case StyleStepPosition::End:
-    default:
       aResult.AppendLiteral(")");
+      break;
+    default:
+      MOZ_ASSERT_UNREACHABLE("Unsupported timing function");
   }
 }
 

layout/style/test/property_database.js

@@ -8164,6 +8164,34 @@ if (IsCSSPropertyPrefEnabled("layout.css.clip-path-path.enabled")) {
   );
 }
 
+if (IsCSSPropertyPrefEnabled("layout.css.step-position-jump.enabled")) {
+  gCSSProperties["animation-timing-function"].other_values.push(
+    "steps(1, jump-start)",
+    "steps(1, jump-end)",
+    "steps(2, jump-none)",
+    "steps(1, jump-both)",
+  );
+  gCSSProperties["animation-timing-function"].invalid_values.push(
+    "steps(0, jump-start)",
+    "steps(0, jump-end)",
+    "steps(1, jump-none)",
+    "steps(0, jump-both)",
+  );
+
+  gCSSProperties["transition-timing-function"].other_values.push(
+    "steps(1, jump-start)",
+    "steps(1, jump-end)",
+    "steps(2, jump-none)",
+    "steps(1, jump-both)",
+  );
+  gCSSProperties["transition-timing-function"].invalid_values.push(
+    "steps(0, jump-start)",
+    "steps(0, jump-end)",
+    "steps(1, jump-none)",
+    "steps(0, jump-both)",
+  );
+}
+
 const OVERFLOW_MOZKWS = [
   "-moz-scrollbars-none",
   "-moz-scrollbars-horizontal",

modules/libpref/init/StaticPrefList.h

@@ -803,6 +803,13 @@ VARCACHE_PREF(
   bool, false
 )
 
+// Is steps(jump-*) supported in easing functions?
+VARCACHE_PREF(
+  "layout.css.step-position-jump.enabled",
+   layout_css_step_position_jump_enabled,
+  bool, true
+)
+
 //---------------------------------------------------------------------------
 // JavaScript prefs
 //---------------------------------------------------------------------------

servo/components/style/animation.rs

@@ -363,11 +363,39 @@ impl PropertyAnimation {
             GenericTimingFunction::CubicBezier { x1, y1, x2, y2 } => {
                 Bezier::new(x1, y1, x2, y2).solve(time, epsilon)
             },
-            GenericTimingFunction::Steps(steps, StepPosition::Start) => {
+            GenericTimingFunction::Steps(steps, pos) => {
-                (time * (steps as f64)).ceil() / (steps as f64)
+                let mut current_step = (time * (steps as f64)).floor() as i32;
-            },
+
-            GenericTimingFunction::Steps(steps, StepPosition::End) => {
+                if pos == StepPosition::Start ||
-                (time * (steps as f64)).floor() / (steps as f64)
+                   pos == StepPosition::JumpStart ||
+                   pos == StepPosition::JumpBoth {
+                    current_step = current_step + 1;
+                }
+
+                // FIXME: We should update current_step according to the "before flag".
+                // In order to get the before flag, we have to know the current animation phase
+                // and whether the iteration is reversed. For now, we skip this calculation.
+                // (i.e. Treat before_flag is unset,)
+                // https://drafts.csswg.org/css-easing/#step-timing-function-algo
+
+                if time >= 0.0 && current_step < 0 {
+                    current_step = 0;
+                }
+
+                let jumps = match pos {
+                    StepPosition::JumpBoth => steps + 1,
+                    StepPosition::JumpNone => steps - 1,
+                    StepPosition::JumpStart |
+                    StepPosition::JumpEnd |
+                    StepPosition::Start |
+                    StepPosition::End => steps,
+                };
+
+                if time <= 1.0 && current_step > jumps {
+                    current_step = jumps;
+                }
+
+                (current_step as f64) / (jumps as f64)
             },
             GenericTimingFunction::Keyword(keyword) => {
                 let (x1, x2, y1, y2) = keyword.to_bezier();

servo/components/style/values/generics/easing.rs

@@ -6,6 +6,7 @@
 //! https://drafts.csswg.org/css-easing/#timing-functions
 
 use values::CSSFloat;
+use parser::ParserContext;
 
 /// A generic easing function.
 #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss)]
@@ -23,7 +24,8 @@ pub enum TimingFunction<Integer, Number> {
         x2: Number,
         y2: Number,
     },
-    /// `step-start | step-end | steps(<integer>, [ start | end ]?)`
+    /// `step-start | step-end | steps(<integer>, [ <step-position> ]?)`
+    /// `<step-position> = jump-start | jump-end | jump-none | jump-both | start | end`
     #[css(comma, function)]
     #[value_info(other_values = "step-start,step-end")]
     Steps(Integer, #[css(skip_if = "is_end")] StepPosition),
@@ -52,18 +54,37 @@ pub enum TimingKeyword {
     EaseInOut,
 }
 
+#[cfg(feature = "gecko")]
+fn step_position_jump_enabled(_context: &ParserContext) -> bool {
+    use gecko_bindings::structs;
+    unsafe { structs::StaticPrefs_sVarCache_layout_css_step_position_jump_enabled }
+}
+
+#[cfg(feature = "servo")]
+fn step_position_jump_enabled(_context: &ParserContext) -> bool {
+    false
+}
+
 #[allow(missing_docs)]
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
 #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq, ToComputedValue, ToCss)]
 #[repr(u8)]
 pub enum StepPosition {
+    #[parse(condition = "step_position_jump_enabled")]
+    JumpStart,
+    #[parse(condition = "step_position_jump_enabled")]
+    JumpEnd,
+    #[parse(condition = "step_position_jump_enabled")]
+    JumpNone,
+    #[parse(condition = "step_position_jump_enabled")]
+    JumpBoth,
     Start,
     End,
 }
 
 #[inline]
 fn is_end(position: &StepPosition) -> bool {
-    *position == StepPosition::End
+    *position == StepPosition::JumpEnd || *position == StepPosition::End
 }
 
 impl<Integer, Number> TimingFunction<Integer, Number> {

servo/components/style/values/specified/easing.rs

@@ -59,8 +59,18 @@ impl Parse for TimingFunction {
                     let steps = Integer::parse_positive(context, i)?;
                     let position = i.try(|i| {
                         i.expect_comma()?;
-                        StepPosition::parse(i)
+                        StepPosition::parse(context, i)
                     }).unwrap_or(StepPosition::End);
+
+                    // jump-none accepts a positive integer greater than 1.
+                    // FIXME(emilio): The spec asks us to avoid rejecting it at parse
+                    // time except until computed value time.
+                    //
+                    // It's not totally clear it's worth it though, and no other browser
+                    // does this.
+                    if position == StepPosition::JumpNone && 2 > steps.value() {
+                        return Err(i.new_custom_error(StyleParseErrorKind::UnspecifiedError));
+                    }
                     Ok(GenericTimingFunction::Steps(steps, position))
                 },
                 _ => Err(()),