Skip to content

Commit

Permalink
Bug 1500260 - Remove unused expired boolean in Animation::Transition.…
Browse files Browse the repository at this point in the history
… r=emilio

The last caller who used was #14418, which did fix a problem but introduced
multiple. In particular, now transitions don't get expired ever, until they
finish running of course.

That is not ok, given you can have something that the user can trigger to change
the style (hi, :hover, for example), and right now that triggers new
transitions, getting this into a really funny state.

I should give fixing this a shot, but it's non-trivial at all.

This cherry-picks part of servo/servo#20757.
  • Loading branch information
emilio committed Oct 18, 2018
1 parent 3f29fd8 commit 9ee318b
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 19 deletions.
23 changes: 5 additions & 18 deletions servo/components/style/animation.rs
Expand Up @@ -214,9 +214,7 @@ 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()`.
///
/// The `bool` field is werther this animation should no longer run.
Transition(OpaqueNode, f64, AnimationFrame, bool),
Transition(OpaqueNode, f64, AnimationFrame),
/// A keyframes animation is identified by a name, and can have a
/// node-dependent state (i.e. iteration count, etc.).
///
Expand All @@ -230,21 +228,11 @@ pub enum Animation {
}

impl Animation {
/// Mark this animation as expired.
#[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,
}
}

/// Whether this animation is expired.
#[inline]
pub fn is_expired(&self) -> bool {
match *self {
Animation::Transition(_, _, _, expired) => expired,
Animation::Transition(..) => false,
Animation::Keyframes(_, _, _, ref state) => state.expired,
}
}
Expand All @@ -253,7 +241,7 @@ impl Animation {
#[inline]
pub fn node(&self) -> &OpaqueNode {
match *self {
Animation::Transition(ref node, _, _, _) => node,
Animation::Transition(ref node, _, _) => node,
Animation::Keyframes(ref node, _, _, _) => node,
}
}
Expand Down Expand Up @@ -465,7 +453,6 @@ pub fn start_transitions_if_applicable(
duration: box_style.transition_duration_mod(i).seconds() as f64,
property_animation: property_animation,
},
/* is_expired = */ false,
)).unwrap();

had_animations = true;
Expand Down Expand Up @@ -656,7 +643,7 @@ pub fn update_style_for_animation<E>(
debug_assert!(!animation.is_expired());

match *animation {
Animation::Transition(_, start_time, ref frame, _) => {
Animation::Transition(_, start_time, ref frame) => {
debug!("update_style_for_animation: transition found");
let now = context.timer.seconds();
let mut new_style = (*style).clone();
Expand Down Expand Up @@ -868,7 +855,7 @@ pub fn complete_expired_transitions(
if let Some(ref animations) = animations_to_expire {
for animation in *animations {
// TODO: support animation-fill-mode
if let Animation::Transition(_, _, ref frame, _) = *animation {
if let Animation::Transition(_, _, ref frame) = *animation {
frame.property_animation.update(Arc::make_mut(style), 1.0);
}
}
Expand Down
2 changes: 1 addition & 1 deletion servo/components/style/matching.rs
Expand Up @@ -625,7 +625,7 @@ trait PrivateMatchMethods: TElement {
font_metrics,
);

if let Animation::Transition(_, _, ref frame, _) = *running_animation {
if let Animation::Transition(_, _, ref frame) = *running_animation {
possibly_expired_animations.push(frame.property_animation.clone())
}
}
Expand Down

0 comments on commit 9ee318b

Please sign in to comment.