Bug 1496619 - part 7: Implement steps(jump-*) functions r=birtles
authorBoris Chiou <boris.chiou@gmail.com>
Fri, 26 Oct 2018 18:03:37 +0000
changeset 443236 77d9e79ed3b9e2a52d635068107d1f2c28369bf9
parent 443235 79675a8112d6e66dd4c1d17985ff2c70f0378ee2
child 443237 24936aee75434cfe3b1551a1818fd4bc2b40f0aa
push id34944
push userncsoregi@mozilla.com
push dateSat, 27 Oct 2018 09:49:55 +0000
treeherdermozilla-central@49d47a692ca4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1496619
milestone65.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 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
dom/animation/ComputedTimingFunction.cpp
layout/style/nsStyleUtil.cpp
layout/style/test/property_database.js
modules/libpref/init/StaticPrefList.h
servo/components/style/animation.rs
servo/components/style/values/generics/easing.rs
servo/components/style/values/specified/easing.rs
--- a/dom/animation/ComputedTimingFunction.cpp
+++ b/dom/animation/ComputedTimingFunction.cpp
@@ -50,44 +50,57 @@ ComputedTimingFunction::Init(const nsTim
   }
 }
 
 static inline double
 StepTiming(const ComputedTimingFunction::StepFunc& aStepFunc,
            double aPortion,
            ComputedTimingFunction::BeforeFlag aBeforeFlag)
 {
-  // Calculate current step using step-end behavior
-  int32_t step = floor(aPortion * aStepFunc.mSteps);
+  // Use the algorithm defined in the spec:
+  // https://drafts.csswg.org/css-easing-1/#step-timing-function-algo
+
+  // Calculate current step.
+  int32_t currentStep = floor(aPortion * aStepFunc.mSteps);
 
-  // step-start is one step ahead
-  if (aStepFunc.mPos == StyleStepPosition::Start) {
-    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) {
-    return 0.0;
+  if (aPortion >= 0.0 && currentStep < 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
 ComputedTimingFunction::GetValue(
     double aPortion,
     ComputedTimingFunction::BeforeFlag aBeforeFlag) const
 {
   if (HasSpline()) {
--- a/layout/style/nsStyleUtil.cpp
+++ b/layout/style/nsStyleUtil.cpp
@@ -277,22 +277,34 @@ nsStyleUtil::AppendPaintOrderValue(uint8
 /* static */ void
 nsStyleUtil::AppendStepsTimingFunction(uint32_t aStepNumber,
                                        mozilla::StyleStepPosition aStepPos,
                                        nsAString& aResult)
 {
   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:
+      aResult.AppendLiteral(")");
+      break;
     default:
-      aResult.AppendLiteral(")");
+      MOZ_ASSERT_UNREACHABLE("Unsupported timing function");
   }
 }
 
 /* static */ void
 nsStyleUtil::AppendCubicBezierTimingFunction(float aX1, float aY1,
                                              float aX2, float aY2,
                                              nsAString& aResult)
 {
--- a/layout/style/test/property_database.js
+++ b/layout/style/test/property_database.js
@@ -8159,16 +8159,44 @@ if (IsCSSPropertyPrefEnabled("layout.css
 
   gCSSProperties["clip-path"].invalid_values.push(
     "path(nonzero)",
     "path(evenodd, '')",
     "path(abs, 'M 10 10 L 10 10 z')",
   );
 }
 
+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",
   "-moz-scrollbars-vertical",
 ];
 if (IsCSSPropertyPrefEnabled("layout.css.overflow.moz-scrollbars.enabled")) {
   gCSSProperties["overflow"].other_values.push(...OVERFLOW_MOZKWS);
 } else {
--- a/modules/libpref/init/StaticPrefList.h
+++ b/modules/libpref/init/StaticPrefList.h
@@ -798,16 +798,23 @@ VARCACHE_PREF(
 
 // Is support for CSS column-span enabled?
 VARCACHE_PREF(
   "layout.css.column-span.enabled",
    layout_css_column_span_enabled,
   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
 //---------------------------------------------------------------------------
 
 // nsJSEnvironmentObserver observes the memory-pressure notifications and
 // forces a garbage collection and cycle collection when it happens, if the
 // appropriate pref is set.
 #ifdef ANDROID
--- a/servo/components/style/animation.rs
+++ b/servo/components/style/animation.rs
@@ -358,21 +358,49 @@ impl PropertyAnimation {
 
     /// Update the given animation at a given point of progress.
     pub fn update(&self, style: &mut ComputedValues, time: f64) {
         let epsilon = 1. / (200. * (self.duration.seconds() as f64));
         let progress = match self.timing_function {
             GenericTimingFunction::CubicBezier { x1, y1, x2, y2 } => {
                 Bezier::new(x1, y1, x2, y2).solve(time, epsilon)
             },
-            GenericTimingFunction::Steps(steps, StepPosition::Start) => {
-                (time * (steps as f64)).ceil() / (steps as f64)
-            },
-            GenericTimingFunction::Steps(steps, StepPosition::End) => {
-                (time * (steps as f64)).floor() / (steps as f64)
+            GenericTimingFunction::Steps(steps, pos) => {
+                let mut current_step = (time * (steps as f64)).floor() as i32;
+
+                if pos == StepPosition::Start ||
+                   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();
                 Bezier::new(x1, x2, y1, y2).solve(time, epsilon)
             },
         };
 
         self.property.update(style, progress);
--- a/servo/components/style/values/generics/easing.rs
+++ b/servo/components/style/values/generics/easing.rs
@@ -1,16 +1,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! Generic types for CSS Easing Functions.
 //! 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)]
 #[value_info(ty = "TIMING_FUNCTION")]
 #[repr(u8, C)]
 pub enum TimingFunction<Integer, Number> {
     /// `linear | ease | ease-in | ease-out | ease-in-out`
     Keyword(TimingKeyword),
@@ -18,17 +19,18 @@ pub enum TimingFunction<Integer, Number>
     #[allow(missing_docs)]
     #[css(comma, function)]
     CubicBezier {
         x1: Number,
         y1: 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),
 }
 
 #[allow(missing_docs)]
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
 #[derive(
@@ -47,28 +49,47 @@ pub enum TimingFunction<Integer, Number>
 pub enum TimingKeyword {
     Linear,
     Ease,
     EaseIn,
     EaseOut,
     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> {
     /// `ease`
     #[inline]
     pub fn ease() -> Self {
         TimingFunction::Keyword(TimingKeyword::Ease)
     }
--- a/servo/components/style/values/specified/easing.rs
+++ b/servo/components/style/values/specified/easing.rs
@@ -54,18 +54,28 @@ impl Parse for TimingFunction {
                     }
 
                     Ok(GenericTimingFunction::CubicBezier { x1, y1, x2, y2 })
                 },
                 "steps" => {
                     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(()),
             }).map_err(|()| {
                 location.new_custom_error(
                     StyleParseErrorKind::UnexpectedFunction(function.clone())
                 )
             })