style: Rewrite the animation representation to allow having state in layout

I have to make the appropriate changes in layout, but I'm running out of battery
in the bus.

components/constellation/constellation.rs

@@ -1136,7 +1136,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
                 let msg = LayoutControlMsg::TickAnimations;
                 match self.pipelines.get(&pipeline_id) {
                     Some(pipeline) => pipeline.layout_chan.send(msg),
-                    None => return warn!("Pipeline {:?} got script tick after closure.", pipeline_id),
+                    None => return warn!("Pipeline {:?} got layout tick after closure.", pipeline_id),
                 }
             }
         };

components/layout/animation.rs

@@ -23,7 +23,7 @@ pub fn update_animation_state(constellation_chan: &IpcSender<ConstellationMsg>,
                               expired_animations: &mut HashMap<OpaqueNode, Vec<Animation>>,
                               new_animations_receiver: &Receiver<Animation>,
                               pipeline_id: PipelineId) {
-    let mut new_running_animations = Vec::new();
+    let mut new_running_animations = vec![];
     while let Ok(animation) = new_animations_receiver.try_recv() {
         new_running_animations.push(animation)
     }
@@ -36,22 +36,24 @@ pub fn update_animation_state(constellation_chan: &IpcSender<ConstellationMsg>,
 
     // Expire old running animations.
     let now = time::precise_time_s();
-    let mut keys_to_remove = Vec::new();
+    let mut keys_to_remove = vec![];
     for (key, running_animations) in running_animations.iter_mut() {
         let mut animations_still_running = vec![];
         for running_animation in running_animations.drain(..) {
             if now < running_animation.end_time {
                 animations_still_running.push(running_animation);
                 continue
+            } else if running_animation.state.pending_iterations > 0 {
+                // if the animation should run again, just tick it...
+                let duration = running_animation.end_time - running_animation.start_time;
+                running_animation.start_time += duration;
             }
-            match expired_animations.entry(*key) {
+            expired_animations.entry(*key)
-                Entry::Vacant(entry) => {
+                              .or_insert_with(Vec::new)
-                    entry.insert(vec![running_animation]);
+                              .push(running_animation);
-                }
-                Entry::Occupied(mut entry) => entry.get_mut().push(running_animation),
-            }
         }
-        if animations_still_running.len() == 0 {
+
+        if animations_still_running.is_empty() {
             keys_to_remove.push(*key);
         } else {
             *running_animations = animations_still_running
@@ -84,12 +86,15 @@ pub fn update_animation_state(constellation_chan: &IpcSender<ConstellationMsg>,
 
 /// Recalculates style for a set of animations. This does *not* run with the DOM lock held.
 pub fn recalc_style_for_animations(flow: &mut Flow,
-                                   animations: &HashMap<OpaqueNode, Vec<Animation>>) {
+                                   animations: &mut HashMap<OpaqueNode, Vec<Animation>>) {
     let mut damage = RestyleDamage::empty();
     flow.mutate_fragments(&mut |fragment| {
-        if let Some(ref animations) = animations.get(&fragment.node) {
+        if let Some(ref animations) = animations.get_mut(&fragment.node) {
-            for animation in *animations {
+            for mut animation in *animations {
-                update_style_for_animation(animation, &mut fragment.style, Some(&mut damage));
+                if !animation.is_paused() {
+                    update_style_for_animation(animation, &mut fragment.style, Some(&mut damage));
+                    animation.increment_keyframe_if_applicable();
+                }
             }
         }
     });

components/layout_thread/lib.rs

@@ -1302,13 +1302,13 @@ impl LayoutThread {
 
         if let Some(mut root_flow) = self.root_flow.clone() {
             // Perform an abbreviated style recalc that operates without access to the DOM.
-            let animations = self.running_animations.read().unwrap();
+            let mut animations = self.running_animations.write().unwrap();
             profile(time::ProfilerCategory::LayoutStyleRecalc,
                     self.profiler_metadata(),
                     self.time_profiler_chan.clone(),
                     || {
                         animation::recalc_style_for_animations(flow_ref::deref_mut(&mut root_flow),
-                                                               &*animations)
+                                                               &mut animations)
                     });
         }
 

components/script/script_thread.rs

@@ -671,7 +671,7 @@ impl ScriptThread {
         }
 
         // Store new resizes, and gather all other events.
-        let mut sequential = vec!();
+        let mut sequential = vec![];
 
         // Receive at least one message so we don't spinloop.
         let mut event = {

components/style/animation.rs

@@ -13,6 +13,8 @@ use keyframes::KeyframesStep;
 use properties::animated_properties::{AnimatedProperty, TransitionProperty};
 use properties::longhands::transition_timing_function::computed_value::StartEnd;
 use properties::longhands::transition_timing_function::computed_value::TransitionTimingFunction;
+use properties::longhands::animation_play_state::computed_value::AnimationPlayState;
+use properties::longhands::animation_iteration_count::computed_value::AnimationIterationCount;
 use properties::style_struct_traits::Box;
 use properties::{ComputedValues, ServoComputedValues};
 use std::sync::mpsc::Sender;
@@ -22,43 +24,60 @@ use values::computed::Time;
 use selector_impl::SelectorImplExt;
 use context::SharedStyleContext;
 use selectors::matching::DeclarationBlock;
+use string_cache::Atom;
 use properties;
 
-#[derive(Clone, Debug)]
+/// This structure represents a keyframes animation current iteration state.
-pub enum AnimationKind {
+///
-    Transition,
+/// If the iteration count is infinite, there's no other state, otherwise we
-    Keyframe,
+/// have to keep track the current iteration and the max iteration count.
+#[derive(Debug, Clone)]
+pub enum KeyframesIterationState {
+    Infinite,
+    // current, max
+    Finite(u32, u32),
+}
+
+/// This structure represents the current keyframe animation state, i.e., the
+/// duration, the current and maximum iteration count, and the state (either
+/// playing or paused).
+#[derive(Debug, Clone)]
+pub struct KeyframesAnimationState {
+    pub started_at: f64,
+    pub duration: f64,
+    pub iteration_state: KeyframesIterationState,
+    pub paused: bool,
 }
 
 /// State relating to an animation.
-#[derive(Clone)]
+#[derive(Clone, Debug)]
-pub struct Animation {
+pub enum Animation {
-    /// The kind of animation, either a transition or a keyframe.
+    /// A transition is just a single frame triggered at a time, with a reflow.
-    pub kind: AnimationKind,
+    ///
-    /// An opaque reference to the DOM node participating in the animation.
+    /// the f64 field is the start time as returned by `time::precise_time_s()`.
-    pub node: OpaqueNode,
+    Transition(OpaqueNode, f64, AnimationFrame),
+    /// A keyframes animation is identified by a name, and can have a
+    /// node-dependent state (i.e. iteration count, etc.).
+    Keyframes(OpaqueNode, Atom, KeyframesAnimationState),
+}
+
+/// A keyframes animation previously sent to layout.
+
+/// A single animation frame of a single property.
+#[derive(Debug, Clone)]
+pub struct AnimationFrame {
     /// A description of the property animation that is occurring.
     pub property_animation: PropertyAnimation,
-    /// The start time of the animation, as returned by `time::precise_time_s()`.
+    /// The duration of the animation. This is either relative in the keyframes
-    pub start_time: f64,
+    /// case (a number between 0 and 1), or absolute in the transition case.
-    /// The end time of the animation, as returned by `time::precise_time_s()`.
+    pub duration: f64,
-    pub end_time: f64,
 }
 
-impl Animation {
+#[derive(Debug, Clone)]
-    /// Returns the duration of this animation in seconds.
-    #[inline]
-    pub fn duration(&self) -> f64 {
-        self.end_time - self.start_time
-    }
-}
-
-
-#[derive(Clone, Debug)]
 pub struct PropertyAnimation {
     property: AnimatedProperty,
     timing_function: TransitionTimingFunction,
-    duration: Time,
+    duration: Time, // TODO: isn't this just repeated?
 }
 
 impl PropertyAnimation {
@@ -167,9 +186,12 @@ impl<T> GetMod for Vec<T> {
     }
 }
 
-/// Inserts transitions into the queue of running animations as applicable for the given style
+/// Inserts transitions into the queue of running animations as applicable for
-/// difference. This is called from the layout worker threads. Returns true if any animations were
+/// the given style difference. This is called from the layout worker threads.
-/// kicked off and false otherwise.
+/// Returns true if any animations were kicked off and false otherwise.
+//
+// TODO(emilio): Take rid of this mutex splitting SharedLayoutContex into a
+// cloneable part and a non-cloneable part..
 pub fn start_transitions_if_applicable<C: ComputedValues>(new_animations_sender: &Mutex<Sender<Animation>>,
                                                           node: OpaqueNode,
                                                           old_style: &C,
@@ -188,16 +210,14 @@ pub fn start_transitions_if_applicable<C: ComputedValues>(new_animations_sender:
             let box_style = new_style.as_servo().get_box();
             let start_time =
                 now + (box_style.transition_delay.0.get_mod(i).seconds() as f64);
-            new_animations_sender.lock().unwrap().send(Animation {
+            new_animations_sender
-                kind: AnimationKind::Transition,
+                .lock().unwrap()
-                node: node,
+                .send(Animation::Transition(node, start_time, AnimationFrame {
-                property_animation: property_animation,
+                    duration: box_style.transition_duration.0.get_mod(i).seconds() as f64,
-                start_time: start_time,
+                    property_animation: property_animation,
-                end_time: start_time +
+                })).unwrap();
-                    (box_style.transition_duration.0.get_mod(i).seconds() as f64),
-            }).unwrap();
 
-            had_animations = true
+            had_animations = true;
         }
     }
 
@@ -227,95 +247,191 @@ pub fn maybe_start_animations<Impl: SelectorImplExt>(context: &SharedStyleContex
 {
     let mut had_animations = false;
 
-    for (i, name) in new_style.as_servo().get_box().animation_name.0.iter().enumerate() {
+    let box_style = new_style.as_servo().get_box();
+    for (i, name) in box_style.animation_name.0.iter().enumerate() {
         debug!("maybe_start_animations: name={}", name);
-        let total_duration = new_style.as_servo().get_box().animation_duration.0.get_mod(i).seconds();
+        let total_duration = box_style.animation_duration.0.get_mod(i).seconds();
         if total_duration == 0. {
             continue
         }
 
-        // TODO: This should be factored out, too much indentation.
         if let Some(ref animation) = context.stylist.animations().get(&name) {
-            debug!("maybe_start_animations: found animation {}", name);
+            let delay = box_style.animation_delay.0.get_mod(i).seconds();
-            had_animations = true;
-            let mut last_keyframe_style = compute_style_for_animation_step(context,
-                                                                           &animation.steps[0],
-                                                                           new_style);
-            // Apply the style inmediately. TODO: clone()...
-            // *new_style = last_keyframe_style.clone();
-
-            let mut ongoing_animation_percentage = animation.steps[0].duration_percentage.0;
-            let delay = new_style.as_servo().get_box().animation_delay.0.get_mod(i).seconds();
             let animation_start = time::precise_time_s() + delay as f64;
+            let duration = box_style.animation_duration.0.get_mod(i).seconds();
+            let iteration_state = match *box_style.animation_iteration_count.0.get_mod(i) {
+                AnimationIterationCount::Infinite => KeyframesIterationState::Infinite,
+                AnimationIterationCount::Number(n) => KeyframesIterationState::Finite(0, n),
+            };
+            let paused = *box_style.animation_play_state.0.get_mod(i) == AnimationPlayState::paused;
 
-            // TODO: We can probably be smarter here and batch steps out or
+            context.new_animations_sender
-            // something.
+                   .lock().unwrap()
-            for step in &animation.steps[1..] {
+                   .send(Animation::Keyframes(node, name.clone(), KeyframesAnimationState {
-                for transition_property in &animation.properties_changed {
+                       started_at: animation_start,
-                    debug!("maybe_start_animations: processing animation prop {:?} for animation {}", transition_property, name);
+                       duration: duration as f64,
-
+                       iteration_state: iteration_state,
-                    let new_keyframe_style = compute_style_for_animation_step(context,
+                       paused: paused,
-                                                                              step,
+                   })).unwrap();
-                                                                              &last_keyframe_style);
+            had_animations = true;
-                    // NB: This will get the previous frame timing function, or
-                    // the old one if caught, which is what the spec says.
-                    //
-                    // We might need to reset to the initial timing function
-                    // though.
-                    let timing_function =
-                        *last_keyframe_style.as_servo()
-                                            .get_box().animation_timing_function.0.get_mod(i);
-
-                    let percentage = step.duration_percentage.0;
-                    let this_keyframe_duration = total_duration * percentage;
-                    if let Some(property_animation) = PropertyAnimation::from_transition_property(*transition_property,
-                                                                                                  timing_function,
-                                                                                                  Time(this_keyframe_duration),
-                                                                                                  &last_keyframe_style,
-                                                                                                  &new_keyframe_style) {
-                        debug!("maybe_start_animations: got property animation for prop {:?}", transition_property);
-
-                        let relative_start_time = ongoing_animation_percentage * total_duration;
-                        let start_time = animation_start + relative_start_time as f64;
-                        let end_time = start_time + (relative_start_time + this_keyframe_duration) as f64;
-                        context.new_animations_sender.lock().unwrap().send(Animation {
-                            kind: AnimationKind::Keyframe,
-                            node: node,
-                            property_animation: property_animation,
-                            start_time: start_time,
-                            end_time: end_time,
-                        }).unwrap();
-                    }
-
-                    last_keyframe_style = new_keyframe_style;
-                    ongoing_animation_percentage += percentage;
-                }
-            }
         }
     }
 
     had_animations
 }
 
-/// Updates a single animation and associated style based on the current time. If `damage` is
+/// Updates a given computed style for a given animation frame. Returns a bool
-/// provided, inserts the appropriate restyle damage.
+/// representing if the style was indeed updated.
-pub fn update_style_for_animation<Damage: TRestyleDamage>(animation: &Animation,
+pub fn update_style_for_animation_frame<C: ComputedValues>(mut new_style: &mut Arc<C>,
-                                                          style: &mut Arc<Damage::ConcreteComputedValues>,
+                                                           now: f64,
-                                                          damage: Option<&mut Damage>) {
+                                                           start_time: f64,
-    let now = time::precise_time_s();
+                                                           frame: &AnimationFrame) -> bool {
-    let mut progress = (now - animation.start_time) / animation.duration();
+    let mut progress = (now - start_time) / frame.duration;
     if progress > 1.0 {
         progress = 1.0
     }
+
     if progress <= 0.0 {
-        return
+        return false;
     }
 
-    let mut new_style = (*style).clone();
+    frame.property_animation.update(Arc::make_mut(&mut new_style), progress);
-    animation.property_animation.update(Arc::make_mut(&mut new_style), progress);
-    if let Some(damage) = damage {
-        *damage = *damage | Damage::compute(Some(style), &new_style);
-    }
 
-    *style = new_style
+    true
+}
+/// Updates a single animation and associated style based on the current time. If `damage` is
+/// provided, inserts the appropriate restyle damage.
+pub fn update_style_for_animation<Damage, Impl>(context: &SharedStyleContext<Impl>,
+                                                animation: &Animation,
+                                                style: &mut Arc<Damage::ConcreteComputedValues>,
+                                                damage: Option<&mut Damage>)
+where Impl: SelectorImplExt,
+      Damage: TRestyleDamage<ConcreteComputedValues = Impl::ComputedValues> {
+    let now = time::precise_time_s();
+    match *animation {
+        Animation::Transition(_, start_time, ref frame) => {
+            let mut new_style = (*style).clone();
+            let updated_style = update_style_for_animation_frame(&mut new_style,
+                                                                 now, start_time,
+                                                                 frame);
+            if updated_style {
+                if let Some(damage) = damage {
+                    *damage = *damage | Damage::compute(Some(style), &new_style);
+                }
+
+                *style = new_style
+            }
+        }
+        Animation::Keyframes(_, ref name, ref state) => {
+            debug_assert!(!state.paused);
+            let duration = state.duration;
+            let started_at = state.started_at;
+
+            let animation = match context.stylist.animations().get(name) {
+                None => {
+                    warn!("update_style_for_animation: Animation {:?} not found", name);
+                    return;
+                }
+                Some(animation) => animation,
+            };
+
+            let maybe_index = style.as_servo()
+                                   .get_box().animation_name.0.iter()
+                                   .position(|animation_name| name == animation_name);
+
+            let index = match maybe_index {
+                Some(index) => index,
+                None => {
+                    warn!("update_style_for_animation: Animation {:?} not found in style", name);
+                    return;
+                }
+            };
+
+            let total_duration = style.as_servo().get_box().animation_duration.0.get_mod(index).seconds() as f64;
+            if total_duration == 0. {
+                debug!("update_style_for_animation: zero duration for animation {:?}", name);
+                return;
+            }
+
+            let mut total_progress = (now - started_at) / total_duration;
+            if total_progress < 0. {
+                warn!("Negative progress found for animation {:?}", name);
+            }
+            if total_progress > 1. {
+                total_progress = 1.;
+            }
+
+
+            let mut last_keyframe = None;
+            let mut target_keyframe = None;
+
+            // TODO: we could maybe binary-search this?
+            for i in 1..animation.steps.len() {
+                if total_progress as f32 <= animation.steps[i].start_percentage.0 {
+                    // We might have found our current keyframe.
+                    target_keyframe = Some(&animation.steps[i]);
+                    last_keyframe = target_keyframe;
+                }
+            }
+
+            let target_keyframe = match target_keyframe {
+                Some(current) => current,
+                None => {
+                    warn!("update_style_for_animation: No current keyframe found for animation {:?} at progress {}", name, total_progress);
+                    return;
+                }
+            };
+
+            let last_keyframe = match last_keyframe {
+                Some(last_keyframe) => last_keyframe,
+                None => {
+                    warn!("update_style_for_animation: No last keyframe found for animation {:?} at progress {}", name, total_progress);
+                    return;
+                }
+            };
+
+            let relative_duration = (target_keyframe.start_percentage.0 - last_keyframe.start_percentage.0) as f64 * duration;
+            let last_keyframe_ended_at = state.started_at + (total_duration * last_keyframe.start_percentage.0 as f64);
+            let relative_progress = (now - last_keyframe_ended_at) / relative_duration;
+
+            // TODO: How could we optimise it? Is it such a big deal?
+            let from_style = compute_style_for_animation_step(context,
+                                                              last_keyframe,
+                                                              &**style);
+
+            // NB: The spec says that the timing function can be overwritten
+            // from the keyframe style.
+            let mut timing_function = *style.as_servo().get_box().animation_timing_function.0.get_mod(index);
+            if !from_style.as_servo().get_box().animation_timing_function.0.is_empty() {
+                timing_function = from_style.as_servo().get_box().animation_timing_function.0[0];
+            }
+
+            let mut target_style = compute_style_for_animation_step(context,
+                                                                    target_keyframe,
+                                                                    &from_style);
+
+            let mut new_style = (*style).clone();
+            let mut style_changed = false;
+
+            for transition_property in &animation.properties_changed {
+                if let Some(property_animation) = PropertyAnimation::from_transition_property(*transition_property,
+                                                                                              timing_function,
+                                                                                              Time(relative_duration as f32),
+                                                                                              &from_style,
+                                                                                              &target_style) {
+                        debug!("update_style_for_animation: got property animation for prop {:?}", transition_property);
+                        property_animation.update(Arc::make_mut(&mut new_style), relative_progress);
+                        style_changed = true;
+                    }
+            }
+
+            if style_changed {
+                if let Some(damage) = damage {
+                    *damage = *damage | Damage::compute(Some(style), &new_style);
+                }
+
+                *style = new_style;
+            }
+        }
+    }
 }

components/style/keyframes.rs

@@ -113,9 +113,8 @@ impl Keyframe {
 /// A single step from a keyframe animation.
 #[derive(Debug, Clone, PartialEq, HeapSizeOf)]
 pub struct KeyframesStep {
-    /// The percentage of the animation duration that should be taken for this
+    /// The percentage of the animation duration when this step starts.
-    /// step.
+    pub start_percentage: KeyframePercentage,
-    pub duration_percentage: KeyframePercentage,
     /// Declarations that will determine the final style during the step.
     pub declarations: Arc<Vec<PropertyDeclaration>>,
 }
@@ -125,7 +124,7 @@ impl KeyframesStep {
     fn new(percentage: KeyframePercentage,
            declarations: Arc<Vec<PropertyDeclaration>>) -> Self {
         KeyframesStep {
-            duration_percentage: percentage,
+            start_percentage: percentage,
             declarations: declarations,
         }
     }
@@ -166,9 +165,6 @@ impl KeyframesAnimation {
         debug_assert!(keyframes.len() > 1);
         let mut steps = vec![];
 
-        // NB: we do two passes, first storing the steps in the order of
-        // appeareance, then sorting them, then updating with the real
-        // "duration_percentage".
         let mut animated_properties = get_animated_properties(&keyframes[0]);
         if animated_properties.is_empty() {
             return None;
@@ -181,24 +177,8 @@ impl KeyframesAnimation {
             }
         }
 
-        steps.sort_by_key(|step| step.duration_percentage);
+        // Sort by the start percentage, so we can easily find a frame.
-
+        steps.sort_by_key(|step| step.start_percentage);
-        if steps[0].duration_percentage != KeyframePercentage(0.0) {
-            // TODO: we could just insert a step from 0 and without declarations
-            // so we won't animate at the beginning. Seems like what other
-            // engines do, but might be a bit tricky so I'd rather leave it as a
-            // follow-up.
-            return None;
-        }
-
-        let mut remaining = 1.0;
-        let mut last_step_end = 0.0;
-        debug_assert!(steps.len() > 1);
-        for current_step in &mut steps[1..] {
-            let new_duration_percentage = KeyframePercentage(current_step.duration_percentage.0 - last_step_end);
-            last_step_end = current_step.duration_percentage.0;
-            current_step.duration_percentage = new_duration_percentage;
-        }
 
         Some(KeyframesAnimation {
             steps: steps,
@@ -206,3 +186,4 @@ impl KeyframesAnimation {
         })
     }
 }
+

components/style/matching.rs

@@ -6,7 +6,7 @@
 
 #![allow(unsafe_code)]
 
-use animation;
+use animation::{self, Animation};
 use context::{SharedStyleContext, LocalStyleContext};
 use data::PrivateStyleData;
 use dom::{TElement, TNode, TRestyleDamage};
@@ -471,7 +471,10 @@ trait PrivateMatchMethods: TNode
             had_animations_to_expire = animations_to_expire.is_some();
             if let Some(ref animations) = animations_to_expire {
                 for animation in *animations {
-                    animation.property_animation.update(Arc::make_mut(style), 1.0);
+                    // TODO: revisit this code for keyframes
+                    if let Animation::Transition(_, _, ref frame) = *animation {
+                        frame.property_animation.update(Arc::make_mut(style), 1.0);
+                    }
                 }
             }
         }
@@ -489,7 +492,8 @@ trait PrivateMatchMethods: TNode
         if had_running_animations {
             let mut all_running_animations = context.running_animations.write().unwrap();
             for running_animation in all_running_animations.get(&this_opaque).unwrap() {
-                animation::update_style_for_animation::<Self::ConcreteRestyleDamage>(running_animation, style, None);
+                animation::update_style_for_animation::<Self::ConcreteRestyleDamage,
+                    <Self::ConcreteElement as Element>::Impl>(context, running_animation, style, None);
             }
             all_running_animations.remove(&this_opaque);
         }