Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-03 20:50:07 +01:00
Code Issues Projects Releases Packages Wiki Activity

layout: Don't restart a transition if the end value for the new

Browse source 
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.
This commit is contained in:
Patrick Walton 2016-11-29 17:48:33 -08:00
parent f378f2807e
commit 89dff2d77f
3 changed files with 66 additions and 25 deletions
Download patch file Download diff file Expand all files Collapse all files

18
components/style/animation.rs
View file

@ -335,6 +335,11 @@ impl PropertyAnimation {
fn does_animate(&self) -> bool { fn does_animate(&self) -> bool {
self.property.does_animate() && self.duration != Time(0.0) 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 /// Inserts transitions into the queue of running animations as applicable for
@ -348,13 +353,24 @@ pub fn start_transitions_if_applicable(new_animations_sender: &Sender<Animation>
unsafe_node: UnsafeNode, unsafe_node: UnsafeNode,
old_style: &ComputedValues, old_style: &ComputedValues,
new_style: &mut Arc<ComputedValues>, new_style: &mut Arc<ComputedValues>,
timer: &Timer) timer: &Timer,
possibly_expired_animations: &[PropertyAnimation])
-> bool { -> bool {
let mut had_animations = false; let mut had_animations = false;
for i in 0..new_style.get_box().transition_property_count() { for i in 0..new_style.get_box().transition_property_count() {
// Create any property animations, if applicable. // Create any property animations, if applicable.
let property_animations = PropertyAnimation::from_transition(i, old_style, Arc::make_mut(new_style)); let property_animations = PropertyAnimation::from_transition(i, old_style, Arc::make_mut(new_style));
for property_animation in property_animations { 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. // Set the property to the initial value.
// NB: get_mut is guaranteed to succeed since we called make_mut() // NB: get_mut is guaranteed to succeed since we called make_mut()
// above. // above.

59
components/style/matching.rs
View file

@ -7,7 +7,7 @@
#![allow(unsafe_code)] #![allow(unsafe_code)]
use {Atom, LocalName}; use {Atom, LocalName};
use animation; use animation::{self, Animation, PropertyAnimation};
use atomic_refcell::AtomicRefMut; use atomic_refcell::AtomicRefMut;
use cache::LRUCache; use cache::LRUCache;
use cascade_info::CascadeInfo; use cascade_info::CascadeInfo;
@ -395,10 +395,10 @@ trait PrivateMatchMethods: TElement {
parent_style: Option<&Arc<ComputedValues>>, parent_style: Option<&Arc<ComputedValues>>,
old_style: Option<&Arc<ComputedValues>>, old_style: Option<&Arc<ComputedValues>>,
rule_node: &StrongRuleNode, rule_node: &StrongRuleNode,
possibly_expired_animations: &[PropertyAnimation],
booleans: CascadeBooleans) booleans: CascadeBooleans)
-> Arc<ComputedValues> -> Arc<ComputedValues>
where Ctx: StyleContext<'a> where Ctx: StyleContext<'a> {
{
let shared_context = context.shared_context(); let shared_context = context.shared_context();
let mut cascade_info = CascadeInfo::new(); let mut cascade_info = CascadeInfo::new();
let mut cascade_flags = CascadeFlags::empty(); let mut cascade_flags = CascadeFlags::empty();
@ -445,7 +445,8 @@ trait PrivateMatchMethods: TElement {
self.as_node().to_unsafe(), self.as_node().to_unsafe(),
&**style, &**style,
&mut this_style, &mut this_style,
&shared_context.timer); &shared_context.timer,
&possibly_expired_animations);
} }
} }
@ -454,11 +455,11 @@ trait PrivateMatchMethods: TElement {
fn update_animations_for_cascade(&self, fn update_animations_for_cascade(&self,
context: &SharedStyleContext, context: &SharedStyleContext,
style: &mut Arc<ComputedValues>) -> bool { style: &mut Arc<ComputedValues>,
possibly_expired_animations: &mut Vec<PropertyAnimation>) {
// Finish any expired transitions. // Finish any expired transitions.
let this_opaque = self.as_node().opaque(); 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. // Merge any running transitions into the current style, and cancel them.
let had_running_animations = context.running_animations let had_running_animations = context.running_animations
@ -467,7 +468,7 @@ trait PrivateMatchMethods: TElement {
.is_some(); .is_some();
if had_running_animations { if had_running_animations {
let mut all_running_animations = context.running_animations.write(); 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 // This shouldn't happen frequently, but under some
// circumstances mainly huge load or debug builds, the // circumstances mainly huge load or debug builds, the
// constellation might be delayed in sending the // constellation might be delayed in sending the
@ -483,12 +484,12 @@ trait PrivateMatchMethods: TElement {
animation::update_style_for_animation(context, animation::update_style_for_animation(context,
running_animation, running_animation,
style); 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, fn share_style_with_candidate_if_possible(&self,
@ -729,6 +730,7 @@ pub trait MatchMethods : TElement {
let parent_style = parent_data.as_ref().map(|x| &x.current_styles().primary.values); let parent_style = parent_data.as_ref().map(|x| &x.current_styles().primary.values);
let mut new_styles; let mut new_styles;
let mut possibly_expired_animations = vec![];
let damage = { let damage = {
let (old_primary, old_pseudos) = match data.previous_styles_mut() { let (old_primary, old_pseudos) = match data.previous_styles_mut() {
@ -737,7 +739,8 @@ pub trait MatchMethods : TElement {
// Update animations before the cascade. This may modify the // Update animations before the cascade. This may modify the
// value of the old primary style. // value of the old primary style.
self.update_animations_for_cascade(context.shared_context(), 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)) (Some(&previous.primary.values), Some(&mut previous.pseudos))
} }
}; };
@ -747,6 +750,7 @@ pub trait MatchMethods : TElement {
parent_style, parent_style,
old_primary, old_primary,
&primary_rule_node, &primary_rule_node,
&possibly_expired_animations,
CascadeBooleans { CascadeBooleans {
shareable: primary_is_shareable, shareable: primary_is_shareable,
animate: true, animate: true,
@ -761,7 +765,8 @@ pub trait MatchMethods : TElement {
&new_styles.primary.values, &new_styles.primary.values,
&mut new_styles.pseudos, &mut new_styles.pseudos,
context, context,
pseudo_rule_nodes); pseudo_rule_nodes,
&mut possibly_expired_animations);
self.as_node().set_can_be_fragmented(parent.map_or(false, |p| { self.as_node().set_can_be_fragmented(parent.map_or(false, |p| {
p.as_node().can_be_fragmented() || p.as_node().can_be_fragmented() ||
@ -774,14 +779,16 @@ pub trait MatchMethods : TElement {
data.finish_styling(new_styles, damage); data.finish_styling(new_styles, damage);
} }
fn compute_damage_and_cascade_pseudos<'a, Ctx>(&self, fn compute_damage_and_cascade_pseudos<'a, Ctx>(
old_primary: Option<&Arc<ComputedValues>>, &self,
mut old_pseudos: Option<&mut PseudoStyles>, old_primary: Option<&Arc<ComputedValues>>,
new_primary: &Arc<ComputedValues>, mut old_pseudos: Option<&mut PseudoStyles>,
new_pseudos: &mut PseudoStyles, new_primary: &Arc<ComputedValues>,
context: &Ctx, new_pseudos: &mut PseudoStyles,
mut pseudo_rule_nodes: PseudoRuleNodes) context: &Ctx,
-> RestyleDamage mut pseudo_rule_nodes: PseudoRuleNodes,
possibly_expired_animations: &mut Vec<PropertyAnimation>)
-> RestyleDamage
where Ctx: StyleContext<'a> where Ctx: StyleContext<'a>
{ {
// Here we optimise the case of the style changing but both the // Here we optimise the case of the style changing but both the
@ -838,14 +845,18 @@ pub trait MatchMethods : TElement {
// Update animations before the cascade. This may modify // Update animations before the cascade. This may modify
// the value of old_pseudo_style. // the value of old_pseudo_style.
self.update_animations_for_cascade(context.shared_context(), 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 = let new_pseudo_values =
self.cascade_node_pseudo_element(context, Some(new_primary), self.cascade_node_pseudo_element(context,
maybe_old_pseudo_style.as_ref().map(|s| &s.values), Some(new_primary),
maybe_old_pseudo_style.as_ref()
.map(|s| &s.values),
&new_rule_node, &new_rule_node,
&possibly_expired_animations,
CascadeBooleans { CascadeBooleans {
shareable: false, shareable: false,
animate: animate, animate: animate,

14
components/style/properties/helpers/animated_properties.mako.rs
View file

@ -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) { pub fn update(&self, style: &mut ComputedValues, progress: f64) {
match *self { match *self {
% for prop in data.longhands: % for prop in data.longhands: