Skip to content

Commit

Permalink
layout: Don't restart a transition if the end value for the new
Browse files Browse the repository at this point in the history 
transition is the same as the end value for a running transition per
CSS-TRANSITIONS § 3.

Besides being contrary to spec, the old behavior could cause a cascade
of CSS transitions incorrectly triggering themselves when calls to
`getComputedStyle()` were intermingled with them.

Improves performance of nytimes.com.
  • Loading branch information
@pcwalton
pcwalton committed Dec 1, 2016
1 parent f378f28 commit 89dff2d
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 25 deletions.
18 changes: 17 additions & 1 deletion components/style/animation.rs
View file
Expand Up @@ -335,6 +335,11 @@ impl PropertyAnimation {
fn does_animate(&self) -> bool {
self.property.does_animate() && self.duration != Time(0.0)
}

#[inline]
pub fn has_the_same_end_value_as(&self, other: &PropertyAnimation) -> bool {
self.property.has_the_same_end_value_as(&other.property)
}
}

/// Inserts transitions into the queue of running animations as applicable for
Expand All @@ -348,13 +353,24 @@ pub fn start_transitions_if_applicable(new_animations_sender: &Sender<Animation>
unsafe_node: UnsafeNode,
old_style: &ComputedValues,
new_style: &mut Arc<ComputedValues>,
timer: &Timer)
timer: &Timer,
possibly_expired_animations: &[PropertyAnimation])
-> bool {
let mut had_animations = false;
for i in 0..new_style.get_box().transition_property_count() {
// Create any property animations, if applicable.
let property_animations = PropertyAnimation::from_transition(i, old_style, Arc::make_mut(new_style));
for property_animation in property_animations {
// Per [1], don't trigger a new transition if the end state for that transition is
// the same as that of a transition that's already running on the same node.
//
// [1]: https://drafts.csswg.org/css-transitions/#starting
if possibly_expired_animations.iter().any(|animation| {
animation.has_the_same_end_value_as(&property_animation)
}) {
continue
}

// Set the property to the initial value.
// NB: get_mut is guaranteed to succeed since we called make_mut()
// above.
Expand Down
59 changes: 35 additions & 24 deletions components/style/matching.rs
View file
Expand Up @@ -7,7 +7,7 @@
#![allow(unsafe_code)]

use {Atom, LocalName};
use animation;
use animation::{self, Animation, PropertyAnimation};
use atomic_refcell::AtomicRefMut;
use cache::LRUCache;
use cascade_info::CascadeInfo;
Expand Down Expand Up @@ -395,10 +395,10 @@ trait PrivateMatchMethods: TElement {
parent_style: Option<&Arc<ComputedValues>>,
old_style: Option<&Arc<ComputedValues>>,
rule_node: &StrongRuleNode,
possibly_expired_animations: &[PropertyAnimation],
booleans: CascadeBooleans)
-> Arc<ComputedValues>
where Ctx: StyleContext<'a>
{
where Ctx: StyleContext<'a> {
let shared_context = context.shared_context();
let mut cascade_info = CascadeInfo::new();
let mut cascade_flags = CascadeFlags::empty();
Expand Down Expand Up @@ -445,7 +445,8 @@ trait PrivateMatchMethods: TElement {
self.as_node().to_unsafe(),
&**style,
&mut this_style,
&shared_context.timer);
&shared_context.timer,
&possibly_expired_animations);
}
}

Expand All @@ -454,11 +455,11 @@ trait PrivateMatchMethods: TElement {

fn update_animations_for_cascade(&self,
context: &SharedStyleContext,
style: &mut Arc<ComputedValues>) -> bool {
style: &mut Arc<ComputedValues>,
possibly_expired_animations: &mut Vec<PropertyAnimation>) {
// Finish any expired transitions.
let this_opaque = self.as_node().opaque();
let had_animations_to_expire =
animation::complete_expired_transitions(this_opaque, style, context);
animation::complete_expired_transitions(this_opaque, style, context);

// Merge any running transitions into the current style, and cancel them.
let had_running_animations = context.running_animations
Expand All @@ -467,7 +468,7 @@ trait PrivateMatchMethods: TElement {
.is_some();
if had_running_animations {
let mut all_running_animations = context.running_animations.write();
for mut running_animation in all_running_animations.get_mut(&this_opaque).unwrap() {
for running_animation in all_running_animations.get_mut(&this_opaque).unwrap() {
// This shouldn't happen frequently, but under some
// circumstances mainly huge load or debug builds, the
// constellation might be delayed in sending the
Expand All @@ -483,12 +484,12 @@ trait PrivateMatchMethods: TElement {
animation::update_style_for_animation(context,
running_animation,
style);
running_animation.mark_as_expired();
if let Animation::Transition(_, _, _, ref frame, _) = *running_animation {
possibly_expired_animations.push(frame.property_animation.clone())
}
}
}
}

had_animations_to_expire || had_running_animations
}

fn share_style_with_candidate_if_possible(&self,
Expand Down Expand Up @@ -729,6 +730,7 @@ pub trait MatchMethods : TElement {
let parent_style = parent_data.as_ref().map(|x| &x.current_styles().primary.values);

let mut new_styles;
let mut possibly_expired_animations = vec![];

let damage = {
let (old_primary, old_pseudos) = match data.previous_styles_mut() {
Expand All @@ -737,7 +739,8 @@ pub trait MatchMethods : TElement {
// Update animations before the cascade. This may modify the
// value of the old primary style.
self.update_animations_for_cascade(context.shared_context(),
&mut previous.primary.values);
&mut previous.primary.values,
&mut possibly_expired_animations);
(Some(&previous.primary.values), Some(&mut previous.pseudos))
}
};
Expand All @@ -747,6 +750,7 @@ pub trait MatchMethods : TElement {
parent_style,
old_primary,
&primary_rule_node,
&possibly_expired_animations,
CascadeBooleans {
shareable: primary_is_shareable,
animate: true,
Expand All @@ -761,7 +765,8 @@ pub trait MatchMethods : TElement {
&new_styles.primary.values,
&mut new_styles.pseudos,
context,
pseudo_rule_nodes);
pseudo_rule_nodes,
&mut possibly_expired_animations);

self.as_node().set_can_be_fragmented(parent.map_or(false, |p| {
p.as_node().can_be_fragmented() ||
Expand All @@ -774,14 +779,16 @@ pub trait MatchMethods : TElement {
data.finish_styling(new_styles, damage);
}

fn compute_damage_and_cascade_pseudos<'a, Ctx>(&self,
old_primary: Option<&Arc<ComputedValues>>,
mut old_pseudos: Option<&mut PseudoStyles>,
new_primary: &Arc<ComputedValues>,
new_pseudos: &mut PseudoStyles,
context: &Ctx,
mut pseudo_rule_nodes: PseudoRuleNodes)
-> RestyleDamage
fn compute_damage_and_cascade_pseudos<'a, Ctx>(
&self,
old_primary: Option<&Arc<ComputedValues>>,
mut old_pseudos: Option<&mut PseudoStyles>,
new_primary: &Arc<ComputedValues>,
new_pseudos: &mut PseudoStyles,
context: &Ctx,
mut pseudo_rule_nodes: PseudoRuleNodes,
possibly_expired_animations: &mut Vec<PropertyAnimation>)
-> RestyleDamage
where Ctx: StyleContext<'a>
{
// Here we optimise the case of the style changing but both the
Expand Down Expand Up @@ -838,14 +845,18 @@ pub trait MatchMethods : TElement {
// Update animations before the cascade. This may modify
// the value of old_pseudo_style.
self.update_animations_for_cascade(context.shared_context(),
&mut old_pseudo_style.values);
&mut old_pseudo_style.values,
possibly_expired_animations);
}
}

let new_pseudo_values =
self.cascade_node_pseudo_element(context, Some(new_primary),
maybe_old_pseudo_style.as_ref().map(|s| &s.values),
self.cascade_node_pseudo_element(context,
Some(new_primary),
maybe_old_pseudo_style.as_ref()
.map(|s| &s.values),
&new_rule_node,
&possibly_expired_animations,
CascadeBooleans {
shareable: false,
animate: animate,
Expand Down
14 changes: 14 additions & 0 deletions components/style/properties/helpers/animated_properties.mako.rs
View file
Expand Up @@ -124,6 +124,20 @@ impl AnimatedProperty {
}
}

pub fn has_the_same_end_value_as(&self, other: &AnimatedProperty) -> bool {
match (self, other) {
% for prop in data.longhands:
% if prop.animatable:
(&AnimatedProperty::${prop.camel_case}(_, ref this_end_value),
&AnimatedProperty::${prop.camel_case}(_, ref other_end_value)) => {
this_end_value == other_end_value
}
% endif
% endfor
_ => false,
}
}

pub fn update(&self, style: &mut ComputedValues, progress: f64) {
match *self {
% for prop in data.longhands:
Expand Down

0 comments on commit 89dff2d

Please sign in to comment.