diff --git a/components/layout/animation.rs b/components/layout/animation.rs index 7f0eaf140f9..18e185aca84 100644 --- a/components/layout/animation.rs +++ b/components/layout/animation.rs @@ -17,12 +17,14 @@ use style::animation::{Animation, update_style_for_animation}; use time; /// Processes any new animations that were discovered after style recalculation. -/// Also expire any old animations that have completed, inserting them into `expired_animations`. +/// Also expire any old animations that have completed, inserting them into +/// `expired_animations`. pub fn update_animation_state(constellation_chan: &IpcSender, running_animations: &mut HashMap>, expired_animations: &mut HashMap>, new_animations_receiver: &Receiver, pipeline_id: PipelineId) { + let now = time::precise_time_s(); let mut new_running_animations = vec![]; while let Ok(animation) = new_animations_receiver.try_recv() { let should_push = match animation { @@ -33,6 +35,7 @@ pub fn update_animation_state(constellation_chan: &IpcSender, // run. if let Some(ref mut animations) = running_animations.get_mut(node) { // TODO: This being linear is probably not optimal. + // Also, we should move this logic somehow. match animations.iter_mut().find(|anim| match **anim { Animation::Keyframes(_, ref anim_name, _) => *name == *anim_name, Animation::Transition(..) => false, @@ -41,12 +44,7 @@ pub fn update_animation_state(constellation_chan: &IpcSender, debug!("update_animation_state: Found other animation {}", name); match *anim { Animation::Keyframes(_, _, ref mut anim_state) => { - // NB: The important part is not touching - // the started_at field, since we don't want - // to restart the animation. - let old_started_at = anim_state.started_at; - *anim_state = state.clone(); - anim_state.started_at = old_started_at; + anim_state.update_from_other(&state); false } _ => unreachable!(), @@ -72,13 +70,12 @@ pub fn update_animation_state(constellation_chan: &IpcSender, } // Expire old running animations. - let now = time::precise_time_s(); let mut keys_to_remove = vec![]; for (key, running_animations) in running_animations.iter_mut() { let mut animations_still_running = vec![]; for mut running_animation in running_animations.drain(..) { - let still_running = match running_animation { - Animation::Transition(_, started_at, ref frame) => { + let still_running = !running_animation.is_expired() && match running_animation { + Animation::Transition(_, started_at, ref frame, _expired) => { now < started_at + frame.duration } Animation::Keyframes(_, _, ref mut state) => { @@ -126,20 +123,19 @@ pub fn update_animation_state(constellation_chan: &IpcSender, .unwrap(); } -/// Recalculates style for a set of animations. This does *not* run with the DOM lock held. +/// Recalculates style for a set of animations. This does *not* run with the DOM +/// lock held. pub fn recalc_style_for_animations(context: &SharedLayoutContext, flow: &mut Flow, - animations: &mut HashMap>) { + animations: &HashMap>) { let mut damage = RestyleDamage::empty(); flow.mutate_fragments(&mut |fragment| { - if let Some(ref mut animations) = animations.get_mut(&fragment.node) { - for ref mut animation in animations.iter_mut() { - if !animation.is_paused() { - update_style_for_animation(&context.style_context, - animation, - &mut fragment.style, - Some(&mut damage)); - } + if let Some(ref animations) = animations.get(&fragment.node) { + for animation in animations.iter() { + update_style_for_animation(&context.style_context, + animation, + &mut fragment.style, + Some(&mut damage)); } } }); diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index b37d35e905a..0f5e558305a 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -1290,7 +1290,7 @@ impl LayoutThread { self.tick_animations(&mut rw_data); } - pub fn tick_animations(&mut self, rw_data: &mut LayoutThreadData) { + fn tick_animations(&mut self, rw_data: &mut LayoutThreadData) { let reflow_info = Reflow { goal: ReflowGoal::ForDisplay, page_clip_rect: MAX_RECT, @@ -1302,14 +1302,14 @@ 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 mut animations = self.running_animations.write().unwrap(); + let animations = self.running_animations.read().unwrap(); profile(time::ProfilerCategory::LayoutStyleRecalc, self.profiler_metadata(), self.time_profiler_chan.clone(), || { animation::recalc_style_for_animations(&layout_context, flow_ref::deref_mut(&mut root_flow), - &mut animations) + &animations) }); } diff --git a/components/style/animation.rs b/components/style/animation.rs index 1ad50cbc939..28dfb2b7a92 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -37,6 +37,18 @@ pub enum KeyframesIterationState { Finite(u32, u32), } +/// This structure represents wether an animation is actually running. +/// +/// An animation can be running, or paused at a given time. +#[derive(Debug, Clone)] +pub enum KeyframesRunningState { + /// This animation is paused. The inner field is the percentage of progress + /// when it was paused, from 0 to 1. + Paused(f64), + /// This animation is actually running. + Running, +} + /// 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). @@ -52,11 +64,13 @@ pub struct KeyframesAnimationState { /// The current iteration state for the animation. pub iteration_state: KeyframesIterationState, /// Werther this animation is paused. - pub paused: bool, + pub running_state: KeyframesRunningState, /// The declared animation direction of this animation. pub direction: AnimationDirection, /// The current animation direction. This can only be `normal` or `reverse`. pub current_direction: AnimationDirection, + /// Werther this keyframe animation is outdated due to a restyle. + pub expired: bool, } impl KeyframesAnimationState { @@ -67,17 +81,22 @@ impl KeyframesAnimationState { /// Returns true if the animation should keep running. pub fn tick(&mut self) -> bool { debug!("KeyframesAnimationState::tick"); - let still_running = match self.iteration_state { - KeyframesIterationState::Finite(ref mut current, ref max) => { - *current += 1; - debug!("KeyframesAnimationState::tick: current={}, max={}", current, max); - *current < *max - } - KeyframesIterationState::Infinite => true, - }; - // Just tick it again updating the started_at field. self.started_at += self.duration + self.delay; + match self.running_state { + // If it's paused, don't update direction or iteration count. + KeyframesRunningState::Paused(_) => return true, + KeyframesRunningState::Running => {}, + } + + if let KeyframesIterationState::Finite(ref mut current, ref max) = self.iteration_state { + *current += 1; + // NB: This prevent us from updating the direction, which might be + // needed for the correct handling of animation-fill-mode. + if *current >= *max { + return false; + } + } // Update the next iteration direction if applicable. match self.direction { @@ -92,7 +111,57 @@ impl KeyframesAnimationState { _ => {}, } - still_running + true + } + + /// Updates the appropiate state from other animation. + /// + /// This happens when an animation is re-submitted to layout, presumably + /// because of an state change. + /// + /// There are some bits of state we can't just replace, over all taking in + /// account times, so here's that logic. + pub fn update_from_other(&mut self, other: &Self) { + use self::KeyframesRunningState::*; + + debug!("KeyframesAnimationState::update_from_other({:?}, {:?})", self, other); + + // NB: We shall not touch the started_at field, since we don't want to + // restart the animation. + let old_started_at = self.started_at; + let old_duration = self.duration; + let old_direction = self.current_direction; + let old_running_state = self.running_state.clone(); + *self = other.clone(); + + let mut new_started_at = old_started_at; + + // If we're unpausing the animation, fake the start time so we seem to + // restore it. + // + // If the animation keeps paused, keep the old value. + // + // If we're pausing the animation, compute the progress value. + match (&mut self.running_state, old_running_state) { + (&mut Running, Paused(progress)) + => new_started_at = time::precise_time_s() - (self.duration * progress), + (&mut Paused(ref mut new), Paused(old)) + => *new = old, + (&mut Paused(ref mut progress), Running) + => *progress = (time::precise_time_s() - old_started_at) / old_duration, + _ => {}, + } + + self.current_direction = old_direction; + self.started_at = new_started_at; + } + + #[inline] + fn is_paused(&self) -> bool { + match self.running_state { + KeyframesRunningState::Paused(..) => true, + KeyframesRunningState::Running => false, + } } } @@ -102,24 +171,45 @@ pub enum Animation { /// A transition is just a single frame triggered at a time, with a reflow. /// /// the f64 field is the start time as returned by `time::precise_time_s()`. - Transition(OpaqueNode, f64, AnimationFrame), + /// + /// The `bool` field is werther this animation should no longer run. + Transition(OpaqueNode, f64, AnimationFrame, bool), /// A keyframes animation is identified by a name, and can have a /// node-dependent state (i.e. iteration count, etc.). Keyframes(OpaqueNode, Atom, KeyframesAnimationState), } impl Animation { + #[inline] + pub fn mark_as_expired(&mut self) { + debug_assert!(!self.is_expired()); + match *self { + Animation::Transition(_, _, _, ref mut expired) => *expired = true, + Animation::Keyframes(_, _, ref mut state) => state.expired = true, + } + } + + #[inline] + pub fn is_expired(&self) -> bool { + match *self { + Animation::Transition(_, _, _, expired) => expired, + Animation::Keyframes(_, _, ref state) => state.expired, + } + } + + #[inline] pub fn node(&self) -> &OpaqueNode { match *self { - Animation::Transition(ref node, _, _) => node, + Animation::Transition(ref node, _, _, _) => node, Animation::Keyframes(ref node, _, _) => node, } } + #[inline] pub fn is_paused(&self) -> bool { match *self { Animation::Transition(..) => false, - Animation::Keyframes(_, _, ref state) => state.paused, + Animation::Keyframes(_, _, ref state) => state.is_paused(), } } } @@ -277,7 +367,7 @@ pub fn start_transitions_if_applicable(new_animations_sender: .send(Animation::Transition(node, start_time, AnimationFrame { duration: box_style.transition_duration.0.get_mod(i).seconds() as f64, property_animation: property_animation, - })).unwrap(); + }, /* is_expired = */ false)).unwrap(); had_animations = true; } @@ -320,7 +410,8 @@ pub fn maybe_start_animations(context: &SharedStyleContex if context.stylist.animations().get(&name).is_some() { debug!("maybe_start_animations: animation {} found", name); let delay = box_style.animation_delay.0.get_mod(i).seconds(); - let animation_start = time::precise_time_s() + delay as f64; + let now = time::precise_time_s(); + let animation_start = now + 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, @@ -336,7 +427,11 @@ pub fn maybe_start_animations(context: &SharedStyleContex AnimationDirection::alternate_reverse => AnimationDirection::reverse, }; - let paused = *box_style.animation_play_state.0.get_mod(i) == AnimationPlayState::paused; + let running_state = match *box_style.animation_play_state.0.get_mod(i) { + AnimationPlayState::paused => KeyframesRunningState::Paused(0.), + AnimationPlayState::running => KeyframesRunningState::Running, + }; + context.new_animations_sender .lock().unwrap() @@ -345,9 +440,10 @@ pub fn maybe_start_animations(context: &SharedStyleContex duration: duration as f64, delay: delay as f64, iteration_state: iteration_state, - paused: paused, + running_state: running_state, direction: animation_direction, current_direction: initial_direction, + expired: false, })).unwrap(); had_animations = true; } @@ -375,8 +471,8 @@ pub fn update_style_for_animation_frame(mut new_style: &mut A true } -/// Updates a single animation and associated style based on the current time. If `damage` is -/// provided, inserts the appropriate restyle damage. +/// 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(context: &SharedStyleContext, animation: &Animation, style: &mut Arc, @@ -384,10 +480,11 @@ pub fn update_style_for_animation(context: &SharedStyleContext { debug!("update_style_for_animation: entering"); - let now = time::precise_time_s(); match *animation { - Animation::Transition(_, start_time, ref frame) => { + Animation::Transition(_, start_time, ref frame, expired) => { + debug_assert!(!expired); debug!("update_style_for_animation: transition found"); + let now = time::precise_time_s(); let mut new_style = (*style).clone(); let updated_style = update_style_for_animation_frame(&mut new_style, now, start_time, @@ -401,11 +498,16 @@ where Impl: SelectorImplExt, } } Animation::Keyframes(_, ref name, ref state) => { - debug!("update_style_for_animation: animation found {:?}", name); - debug_assert!(!state.paused); + debug_assert!(!state.expired); + debug!("update_style_for_animation: animation found: \"{}\", {:?}", name, state); let duration = state.duration; let started_at = state.started_at; + let now = match state.running_state { + KeyframesRunningState::Running => time::precise_time_s(), + KeyframesRunningState::Paused(progress) => started_at + duration * progress, + }; + let animation = match context.stylist.animations().get(name) { None => { warn!("update_style_for_animation: Animation {:?} not found", name); @@ -456,7 +558,7 @@ where Impl: SelectorImplExt, last_keyframe_position = target_keyframe_position.and_then(|pos| { if pos != 0 { Some(pos - 1) } else { None } - }); + }).unwrap_or(0); } AnimationDirection::reverse => { target_keyframe_position = @@ -466,7 +568,7 @@ where Impl: SelectorImplExt, last_keyframe_position = target_keyframe_position.and_then(|pos| { if pos != animation.steps.len() - 1 { Some(pos + 1) } else { None } - }); + }).unwrap_or(animation.steps.len() - 1); } _ => unreachable!(), } @@ -485,14 +587,7 @@ where Impl: SelectorImplExt, } }; - let last_keyframe = match last_keyframe_position { - Some(last) => &animation.steps[last], - None => { - warn!("update_style_for_animation: No last keyframe found for animation \"{}\" at progress {}", - name, total_progress); - return; - } - }; + let last_keyframe = &animation.steps[last_keyframe_position]; let relative_timespan = (target_keyframe.start_percentage.0 - last_keyframe.start_percentage.0).abs(); let relative_duration = relative_timespan as f64 * duration; @@ -524,7 +619,6 @@ where Impl: SelectorImplExt, &from_style); let mut new_style = (*style).clone(); - let mut style_changed = false; for transition_property in &animation.properties_changed { debug!("update_style_for_animation: scanning prop {:?} for animation \"{}\"", @@ -538,7 +632,6 @@ where Impl: SelectorImplExt, debug!("update_style_for_animation: got property animation for prop {:?}", transition_property); debug!("update_style_for_animation: {:?}", property_animation); property_animation.update(Arc::make_mut(&mut new_style), relative_progress); - style_changed = true; } None => { debug!("update_style_for_animation: property animation {:?} not animating", @@ -547,14 +640,12 @@ where Impl: SelectorImplExt, } } - if style_changed { - debug!("update_style_for_animation: got style change in animation \"{}\"", name); - if let Some(damage) = damage { - *damage = *damage | Damage::compute(Some(style), &new_style); - } - - *style = new_style; + debug!("update_style_for_animation: got style change in animation \"{}\"", name); + if let Some(damage) = damage { + *damage = *damage | Damage::compute(Some(style), &new_style); } + + *style = new_style; } } } diff --git a/components/style/matching.rs b/components/style/matching.rs index 3d6d2bd147e..7b13e34c1bc 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -469,8 +469,9 @@ trait PrivateMatchMethods: TNode had_animations_to_expire = animations_to_expire.is_some(); if let Some(ref animations) = animations_to_expire { for animation in *animations { - // TODO: revisit this code for keyframes - if let Animation::Transition(_, _, ref frame) = *animation { + // NB: Expiring a keyframes animation is the same as not + // applying the keyframes style to it, so we're safe. + if let Animation::Transition(_, _, ref frame, _) = *animation { frame.property_animation.update(Arc::make_mut(style), 1.0); } } @@ -489,11 +490,11 @@ trait PrivateMatchMethods: TNode .is_some(); 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() { + for mut running_animation in all_running_animations.get_mut(&this_opaque).unwrap() { animation::update_style_for_animation::::Impl>(context, running_animation, style, None); + running_animation.mark_as_expired(); } - all_running_animations.remove(&this_opaque); } had_animations_to_expire || had_running_animations diff --git a/components/style/parallel.rs b/components/style/parallel.rs index e967c3839fd..55f15f1d80f 100644 --- a/components/style/parallel.rs +++ b/components/style/parallel.rs @@ -58,7 +58,7 @@ fn top_down_dom(unsafe_nodes: UnsafeNodeList, where N: TNode, C: DomTraversalContext { let context = C::new(proxy.user_data(), unsafe_nodes.1); - let mut discovered_child_nodes = Vec::new(); + let mut discovered_child_nodes = vec![]; for unsafe_node in *unsafe_nodes.0 { // Get a real layout node. let node = unsafe { N::from_unsafe(&unsafe_node) };