Auto merge of #27032 - mrobinson:fractional-iteration, r=jdm

animations: Finish support for fractional iteration counts

This change also improves support for creating animations with negative
delays, as that is necessary to test support for fractional iteration
lengths.

Fixes: #14858

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #14858
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

components/script/animations.rs

@@ -386,7 +386,7 @@ impl Animations {
         now: f64,
         pipeline_id: PipelineId,
     ) {
-        let num_iterations = match animation.iteration_state {
+        let iteration_index = match animation.iteration_state {
             KeyframesIterationState::Finite(current, _) |
             KeyframesIterationState::Infinite(current) => current,
         };
@@ -402,10 +402,14 @@ impl Animations {
             TransitionOrAnimationEventType::AnimationStart => {
                 (-animation.delay).max(0.).min(active_duration)
             },
-            TransitionOrAnimationEventType::AnimationIteration |
+            TransitionOrAnimationEventType::AnimationIteration => {
-            TransitionOrAnimationEventType::AnimationEnd => num_iterations * animation.duration,
+                iteration_index * animation.duration
+            },
+            TransitionOrAnimationEventType::AnimationEnd => {
+                (iteration_index * animation.duration) + animation.current_iteration_duration()
+            },
             TransitionOrAnimationEventType::AnimationCancel => {
-                (num_iterations * animation.duration) + (now - animation.started_at).max(0.)
+                (iteration_index * animation.duration) + (now - animation.started_at).max(0.)
             },
             _ => unreachable!(),
         }

components/style/animation.rs

@@ -27,7 +27,9 @@ use crate::stylesheets::keyframes_rule::{KeyframesAnimation, KeyframesStep, Keyf
 use crate::values::animated::{Animate, Procedure};
 use crate::values::computed::{Time, TimingFunction};
 use crate::values::generics::box_::AnimationIterationCount;
-use crate::values::generics::easing::{StepPosition, TimingFunction as GenericTimingFunction};
+use crate::values::generics::easing::{
+    StepPosition, TimingFunction as GenericTimingFunction, TimingKeyword,
+};
 use crate::Atom;
 use fxhash::FxHashMap;
 use parking_lot::RwLock;
@@ -125,8 +127,14 @@ impl PropertyAnimation {
                 (current_step as f64) / (jumps as f64)
             },
             GenericTimingFunction::Keyword(keyword) => {
-                let (x1, x2, y1, y2) = keyword.to_bezier();
+                let bezier = match keyword {
-                Bezier::new(x1, x2, y1, y2).solve(progress, epsilon)
+                    TimingKeyword::Linear => return progress,
+                    TimingKeyword::Ease => Bezier::new(0.25, 0.1, 0.25, 1.),
+                    TimingKeyword::EaseIn => Bezier::new(0.42, 0., 1., 1.),
+                    TimingKeyword::EaseOut => Bezier::new(0., 0., 0.58, 1.),
+                    TimingKeyword::EaseInOut => Bezier::new(0.42, 0., 0.58, 1.),
+                };
+                bezier.solve(progress, epsilon)
             },
         }
     }
@@ -470,19 +478,27 @@ impl Animation {
             return false;
         }
 
+        if self.on_last_iteration() {
+            return false;
+        }
+
+        self.iterate();
+        true
+    }
+
+    fn iterate(&mut self) {
+        debug_assert!(!self.on_last_iteration());
+
         if let KeyframesIterationState::Finite(ref mut current, max) = self.iteration_state {
-            // If we are already on the final iteration, just exit now. This prevents
-            // us from updating the direction, which might be needed for the correct
-            // handling of animation-fill-mode and also firing animationiteration events
-            // at the end of animations.
             *current = (*current + 1.).min(max);
-            if *current == max {
+        }
-                return false;
+
-            }
+        if let AnimationState::Paused(ref mut progress) = self.state {
+            debug_assert!(*progress > 1.);
+            *progress -= 1.;
         }
 
         // Update the next iteration direction if applicable.
-        // TODO(mrobinson): The duration might now be wrong for floating point iteration counts.
         self.started_at += self.duration;
         match self.direction {
             AnimationDirection::Alternate | AnimationDirection::AlternateReverse => {
@@ -494,36 +510,55 @@ impl Animation {
             },
             _ => {},
         }
-
-        true
     }
 
+    /// A number (> 0 and <= 1) which represents the fraction of a full iteration
+    /// that the current iteration of the animation lasts. This will be less than 1
+    /// if the current iteration is the fractional remainder of a non-integral
+    /// iteration count.
+    pub fn current_iteration_end_progress(&self) -> f64 {
+        match self.iteration_state {
+            KeyframesIterationState::Finite(current, max) => (max - current).min(1.),
+            KeyframesIterationState::Infinite(_) => 1.,
+        }
+    }
+
+    /// The duration of the current iteration of this animation which may be less
+    /// than the animation duration if it has a non-integral iteration count.
+    pub fn current_iteration_duration(&self) -> f64 {
+        self.current_iteration_end_progress() * self.duration
+    }
+
+    /// Whether or not the current iteration is over. Note that this method assumes that
+    /// the animation is still running.
     fn iteration_over(&self, time: f64) -> bool {
-        time > (self.started_at + self.duration)
+        time > (self.started_at + self.current_iteration_duration())
+    }
+
+    /// Assuming this animation is running, whether or not it is on the last iteration.
+    fn on_last_iteration(&self) -> bool {
+        match self.iteration_state {
+            KeyframesIterationState::Finite(current, max) => current >= (max - 1.),
+            KeyframesIterationState::Infinite(_) => false,
+        }
     }
 
     /// Whether or not this animation has finished at the provided time. This does
     /// not take into account canceling i.e. when an animation or transition is
     /// canceled due to changes in the style.
     pub fn has_ended(&self, time: f64) -> bool {
-        match self.state {
+        if !self.on_last_iteration() {
-            AnimationState::Running => {},
-            AnimationState::Finished => return true,
-            AnimationState::Pending | AnimationState::Canceled | AnimationState::Paused(_) => {
-                return false
-            },
-        }
-
-        if !self.iteration_over(time) {
             return false;
         }
 
-        // If we have a limited number of iterations and we cannot advance to another
+        let progress = match self.state {
-        // iteration, then we have ended.
+            AnimationState::Finished => return true,
-        return match self.iteration_state {
+            AnimationState::Paused(progress) => progress,
-            KeyframesIterationState::Finite(current, max) => max == current,
+            AnimationState::Running => (time - self.started_at) / self.duration,
-            KeyframesIterationState::Infinite(..) => false,
+            AnimationState::Pending | AnimationState::Canceled => return false,
         };
+
+        progress >= self.current_iteration_end_progress()
     }
 
     /// Updates the appropiate state from other animation.
@@ -601,38 +636,36 @@ impl Animation {
     /// Fill in an `AnimationValueMap` with values calculated from this animation at
     /// the given time value.
     fn get_property_declaration_at_time(&self, now: f64, map: &mut AnimationValueMap) {
-        let duration = self.duration;
+        debug_assert!(!self.computed_steps.is_empty());
-        let started_at = self.started_at;
 
-        let now = match self.state {
+        let total_progress = match self.state {
-            AnimationState::Running | AnimationState::Pending | AnimationState::Finished => now,
+            AnimationState::Running | AnimationState::Pending | AnimationState::Finished => {
-            AnimationState::Paused(progress) => started_at + duration * progress,
+                (now - self.started_at) / self.duration
+            },
+            AnimationState::Paused(progress) => progress,
             AnimationState::Canceled => return,
         };
 
-        debug_assert!(!self.computed_steps.is_empty());
-
-        let mut total_progress = (now - started_at) / duration;
         if total_progress < 0. &&
             self.fill_mode != AnimationFillMode::Backwards &&
             self.fill_mode != AnimationFillMode::Both
         {
             return;
         }
-
+        if self.has_ended(now) &&
-        if total_progress > 1. &&
             self.fill_mode != AnimationFillMode::Forwards &&
             self.fill_mode != AnimationFillMode::Both
         {
             return;
         }
-        total_progress = total_progress.min(1.0).max(0.0);
+        let total_progress = total_progress
+            .min(self.current_iteration_end_progress())
+            .max(0.0);
 
         // Get the indices of the previous (from) keyframe and the next (to) keyframe.
         let next_keyframe_index;
         let prev_keyframe_index;
         let num_steps = self.computed_steps.len();
-        debug_assert!(num_steps > 0);
         match self.current_direction {
             AnimationDirection::Normal => {
                 next_keyframe_index = self
@@ -674,45 +707,43 @@ impl Animation {
             None => return,
         };
 
+        // If we only need to take into account one keyframe, then exit early
+        // in order to avoid doing more work.
         let mut add_declarations_to_map = |keyframe: &ComputedKeyframe| {
             for value in keyframe.values.iter() {
                 map.insert(value.id(), value.clone());
             }
         };
-
         if total_progress <= 0.0 {
             add_declarations_to_map(&prev_keyframe);
             return;
         }
-
         if total_progress >= 1.0 {
             add_declarations_to_map(&next_keyframe);
             return;
         }
 
-        let relative_timespan =
+        let percentage_between_keyframes =
-            (next_keyframe.start_percentage - prev_keyframe.start_percentage).abs();
+            (next_keyframe.start_percentage - prev_keyframe.start_percentage).abs() as f64;
-        let relative_duration = relative_timespan as f64 * duration;
+        let duration_between_keyframes = percentage_between_keyframes * self.duration;
-        let last_keyframe_ended_at = match self.current_direction {
+        let direction_aware_prev_keyframe_start_percentage = match self.current_direction {
-            AnimationDirection::Normal => {
+            AnimationDirection::Normal => prev_keyframe.start_percentage as f64,
-                self.started_at + (duration * prev_keyframe.start_percentage as f64)
+            AnimationDirection::Reverse => 1. - prev_keyframe.start_percentage as f64,
-            },
-            AnimationDirection::Reverse => {
-                self.started_at + (duration * (1. - prev_keyframe.start_percentage as f64))
-            },
             _ => unreachable!(),
         };
+        let progress_between_keyframes = (total_progress -
+            direction_aware_prev_keyframe_start_percentage) /
+            percentage_between_keyframes;
 
-        let relative_progress = (now - last_keyframe_ended_at) / relative_duration;
         for (from, to) in prev_keyframe.values.iter().zip(next_keyframe.values.iter()) {
             let animation = PropertyAnimation {
                 from: from.clone(),
                 to: to.clone(),
                 timing_function: prev_keyframe.timing_function,
-                duration: relative_duration as f64,
+                duration: duration_between_keyframes as f64,
             };
 
-            if let Ok(value) = animation.calculate_value(relative_progress) {
+            if let Ok(value) = animation.calculate_value(progress_between_keyframes) {
                 map.insert(value.id(), value);
             }
         }
@@ -1319,7 +1350,7 @@ pub fn maybe_start_animations<E>(
         };
 
         debug!("maybe_start_animations: name={}", name);
-        let duration = box_style.animation_duration_mod(i).seconds();
+        let duration = box_style.animation_duration_mod(i).seconds() as f64;
         if duration == 0. {
             continue;
         }
@@ -1339,8 +1370,11 @@ pub fn maybe_start_animations<E>(
             continue;
         }
 
+        // NB: This delay may be negative, meaning that the animation may be created
+        // in a state where we have advanced one or more iterations or even that the
+        // animation begins in a finished state.
         let delay = box_style.animation_delay_mod(i).seconds();
-        let animation_start = context.current_time_for_animations + delay as f64;
+
         let iteration_state = match box_style.animation_iteration_count_mod(i) {
             AnimationIterationCount::Infinite => KeyframesIterationState::Infinite(0.0),
             AnimationIterationCount::Number(n) => KeyframesIterationState::Finite(0.0, n.into()),
@@ -1357,8 +1391,11 @@ pub fn maybe_start_animations<E>(
             },
         };
 
+        let now = context.current_time_for_animations;
+        let started_at = now + delay as f64;
+        let mut starting_progress = (now - started_at) / duration;
         let state = match box_style.animation_play_state_mod(i) {
-            AnimationPlayState::Paused => AnimationState::Paused(0.),
+            AnimationPlayState::Paused => AnimationState::Paused(starting_progress),
             AnimationPlayState::Running => AnimationState::Pending,
         };
 
@@ -1371,12 +1408,12 @@ pub fn maybe_start_animations<E>(
             resolver,
         );
 
-        let new_animation = Animation {
+        let mut new_animation = Animation {
             name: name.clone(),
             properties_changed: keyframe_animation.properties_changed,
             computed_steps,
-            started_at: animation_start,
+            started_at,
-            duration: duration as f64,
+            duration,
             fill_mode: box_style.animation_fill_mode_mod(i),
             delay: delay as f64,
             iteration_state,
@@ -1387,6 +1424,13 @@ pub fn maybe_start_animations<E>(
             is_new: true,
         };
 
+        // If we started with a negative delay, make sure we iterate the animation if
+        // the delay moves us past the first iteration.
+        while starting_progress > 1. && !new_animation.on_last_iteration() {
+            new_animation.iterate();
+            starting_progress -= 1.;
+        }
+
         animation_state.dirty = true;
 
         // If the animation was already present in the list for the node, just update its state.

components/style/values/generics/easing.rs

@@ -6,7 +6,6 @@
 //! https://drafts.csswg.org/css-easing/#timing-functions
 
 use crate::parser::ParserContext;
-use crate::values::CSSFloat;
 
 /// A generic easing function.
 #[derive(
@@ -118,18 +117,3 @@ impl<Integer, Number> TimingFunction<Integer, Number> {
         TimingFunction::Keyword(TimingKeyword::Ease)
     }
 }
-
-impl TimingKeyword {
-    /// Returns the keyword as a quadruplet of Bezier point coordinates
-    /// `(x1, y1, x2, y2)`.
-    #[inline]
-    pub fn to_bezier(self) -> (CSSFloat, CSSFloat, CSSFloat, CSSFloat) {
-        match self {
-            TimingKeyword::Linear => (0., 0., 1., 1.),
-            TimingKeyword::Ease => (0.25, 0.1, 0.25, 1.),
-            TimingKeyword::EaseIn => (0.42, 0., 1., 1.),
-            TimingKeyword::EaseOut => (0., 0., 0.58, 1.),
-            TimingKeyword::EaseInOut => (0.42, 0., 0.58, 1.),
-        }
-    }
-}

tests/wpt/metadata-layout-2020/css/css-animations/animation-delay-011.html.ini

@@ -1,2 +0,0 @@
-[animation-delay-011.html]
-  expected: FAIL

tests/wpt/metadata-layout-2020/css/filter-effects/css-filters-animation-blur.html.ini

@@ -1,2 +0,0 @@
-[css-filters-animation-blur.html]
-  expected: FAIL

tests/wpt/metadata-layout-2020/css/filter-effects/css-filters-animation-brightness.html.ini

@@ -1,2 +0,0 @@
-[css-filters-animation-brightness.html]
-  expected: FAIL

tests/wpt/metadata-layout-2020/css/filter-effects/css-filters-animation-combined-001.html.ini

@@ -1,2 +0,0 @@
-[css-filters-animation-combined-001.html]
-  expected: FAIL

tests/wpt/metadata-layout-2020/css/filter-effects/css-filters-animation-contrast.html.ini

@@ -1,2 +0,0 @@
-[css-filters-animation-contrast.html]
-  expected: FAIL

tests/wpt/metadata-layout-2020/css/filter-effects/css-filters-animation-grayscale.html.ini

@@ -1,2 +0,0 @@
-[css-filters-animation-grayscale.html]
-  expected: FAIL

tests/wpt/metadata-layout-2020/css/filter-effects/css-filters-animation-invert.html.ini

@@ -1,2 +0,0 @@
-[css-filters-animation-invert.html]
-  expected: FAIL

tests/wpt/metadata-layout-2020/css/filter-effects/css-filters-animation-opacity.html.ini

@@ -1,2 +0,0 @@
-[css-filters-animation-opacity.html]
-  expected: FAIL

tests/wpt/metadata-layout-2020/css/filter-effects/css-filters-animation-saturate.html.ini

@@ -1,2 +0,0 @@
-[css-filters-animation-saturate.html]
-  expected: FAIL

tests/wpt/metadata-layout-2020/css/filter-effects/css-filters-animation-sepia.html.ini

@@ -1,2 +0,0 @@
-[css-filters-animation-sepia.html]
-  expected: FAIL

tests/wpt/metadata/MANIFEST.json

@@ -386051,6 +386051,13 @@
        {}
       ]
      ],
+     "animation-iteration-count-009.html": [
+      "da86c81b9337a99841977acd8e2ffe8b8e858190",
+      [
+       null,
+       {}
+      ]
+     ],
      "animation-iteration-count-calc.html": [
       "44e1e96a589a4e1c5b98e919e7246d05097b0604",
       [

tests/wpt/metadata/css/css-ui/outline-017.html.ini

@@ -1,10 +0,0 @@
-[outline-017.html]
-  [outline-color is animated as a color]
-    expected: FAIL
-
-  [outline-width is animated as a length]
-    expected: FAIL
-
-  [outline-offset is animated as a length]
-    expected: FAIL
-

tests/wpt/metadata/css/css-ui/outline-018.html.ini

@@ -1,4 +0,0 @@
-[outline-018.html]
-  [outline-style is animated as a discrete type]
-    expected: FAIL
-

tests/wpt/mozilla/meta/MANIFEST.json

@@ -12871,7 +12871,7 @@
    "css": {
     "animations": {
      "animation-delay.html": [
-      "0d2053a9134d8ff0ade7b5dc37ecfce305557c44",
+      "f54cf2b9bca93e82b177243b9d754e4ae3bf15fa",
       [
        null,
        {}
@@ -12887,7 +12887,7 @@
       ]
      ],
      "animation-fill-mode.html": [
-      "9602ec9f0e0eb1f6efcc2e7bd95181ef65339bae",
+      "ac3062879af9836768890d653f4b29b6165b6a45",
       [
        null,
        {}

tests/wpt/mozilla/tests/css/animations/animation-delay.html

@@ -31,6 +31,7 @@ test(function() {
   element.style.animationIterationCount = 1;
   element.style.animationName = "width-animation";
   element.style.animationTimingFunction = "linear";
+  element.style.animationFillMode = "forwards";
 
   document.body.appendChild(element);
   assert_equals(getComputedStyle(element).getPropertyValue("width"), "50px");
@@ -48,7 +49,7 @@ test(function() {
   assert_equals(getComputedStyle(element).getPropertyValue("width"), "500px");
 
   testBinding.advanceClock(500);
-  assert_equals(getComputedStyle(element).getPropertyValue("width"), "50px");
+  assert_equals(getComputedStyle(element).getPropertyValue("width"), "500px");
 }, "animation-delay should function correctly");
 
 test(function() {
@@ -61,6 +62,7 @@ test(function() {
   element.style.animationIterationCount = 2;
   element.style.animationName = "width-animation";
   element.style.animationTimingFunction = "linear";
+  element.style.animationFillMode = "forwards";
 
   document.body.appendChild(element);
   assert_equals(getComputedStyle(element).getPropertyValue("width"), "50px");
@@ -85,7 +87,7 @@ test(function() {
   assert_equals(getComputedStyle(element).getPropertyValue("width"), "500px");
 
   testBinding.advanceClock(500);
-  assert_equals(getComputedStyle(element).getPropertyValue("width"), "50px");
+  assert_equals(getComputedStyle(element).getPropertyValue("width"), "500px");
 }, "animation-delay should function correctly with multiple iterations");
 
 </script>

tests/wpt/mozilla/tests/css/animations/animation-fill-mode.html

@@ -48,8 +48,9 @@ function runThroughAnimation(testBinding, element, waitForDelay = true) {
   testBinding.advanceClock(500);
   assert_equals(getComputedStyle(element).getPropertyValue("width"), "250px");
 
+  // After advancing another 500 milliseconds the animation should finished and
+  // width value will depend on the value of `animation-fill-mode`.
   testBinding.advanceClock(500);
-  assert_equals(getComputedStyle(element).getPropertyValue("width"), "500px");
 }
 
 test(function() {

tests/wpt/web-platform-tests/css/css-animations/animation-iteration-count-009.html

@@ -0,0 +1,46 @@
+<!doctype html>
+<meta charset=utf-8>
+<title>CSS Animation Test: fractional animation-iteration-count</title>
+<link rel="help" href="https://drafts.csswg.org/css-animations/#animation-iteration-count">
+<link rel="author" title="Martin Robinson" href="mailto:mrobinson@igalia.com">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="support/testcommon.js"></script>
+<style>
+@keyframes margin-animation {
+  from {
+    margin-left: 0px;
+  }
+  to {
+    margin-left: 100px;
+  }
+}
+</style>
+<div id="log"></div>
+<script>
+'use strict';
+
+promise_test(async t => {
+  const div = addDiv(t);
+  div.style.animation = 'margin-animation 1s -10s linear 1.5 normal forwards paused';
+  assert_equals(getComputedStyle(div).marginLeft, '50px');
+}, 'Basic floating point iteration count');
+
+promise_test(async t => {
+  const div = addDiv(t);
+  div.style.animation = 'margin-animation 1s -10s linear 3.25 normal forwards paused';
+  assert_equals(getComputedStyle(div).marginLeft, '25px');
+}, 'Floating point iteration count after multiple iterations');
+
+promise_test(async t => {
+  const div = addDiv(t);
+  div.style.animation = 'margin-animation 1s -10s linear 0.75 normal forwards paused';
+  assert_equals(getComputedStyle(div).marginLeft, '75px');
+}, 'Floating point iteration count during first iteration');
+
+promise_test(async t => {
+  const div = addDiv(t);
+  div.style.animation = 'margin-animation 1s -10s linear 1.75 alternate forwards paused';
+  assert_equals(getComputedStyle(div).marginLeft, '25px');
+}, 'Floating point iteration count with alternating directions');
+</script>