Auto merge of #14418 - pcwalton:infinite-reflows, r=notriddle

layout: Fix some particularly bad cases of spurious reflows leading to script thread unresponsiveness.

See commits for details. This improves nytimes.com quite a bit.

r? @notriddle

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14418)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2016-12-01 10:16:38 -08:00 committed by GitHub
commit d2d62267a1
6 changed files with 86 additions and 34 deletions

View file

@ -1191,10 +1191,13 @@ impl<Window: WindowMethods> IOCompositor<Window> {
} }
} }
// We still need to tick layout unfortunately, see things like #12749. // We may need to tick animations in layout. (See #12749.)
let msg = ConstellationMsg::TickAnimation(pipeline_id, AnimationTickType::Layout); let animations_running = self.pipeline_details(pipeline_id).animations_running;
if let Err(e) = self.constellation_chan.send(msg) { if animations_running {
warn!("Sending tick to constellation failed ({}).", e); let msg = ConstellationMsg::TickAnimation(pipeline_id, AnimationTickType::Layout);
if let Err(e) = self.constellation_chan.send(msg) {
warn!("Sending tick to constellation failed ({}).", e);
}
} }
} }

View file

@ -88,7 +88,8 @@ pub fn update_animation_state(constellation_chan: &IpcSender<ConstellationMsg>,
if let Animation::Transition(_, unsafe_node, _, ref frame, _) = running_animation { if let Animation::Transition(_, unsafe_node, _, ref frame, _) = running_animation {
script_chan.send(ConstellationControlMsg::TransitionEnd(unsafe_node, script_chan.send(ConstellationControlMsg::TransitionEnd(unsafe_node,
frame.property_animation.property_name(), frame.property_animation
.property_name(),
frame.duration)) frame.duration))
.unwrap(); .unwrap();
} }
@ -113,7 +114,7 @@ pub fn update_animation_state(constellation_chan: &IpcSender<ConstellationMsg>,
for new_running_animation in new_running_animations { for new_running_animation in new_running_animations {
running_animations.entry(*new_running_animation.node()) running_animations.entry(*new_running_animation.node())
.or_insert_with(Vec::new) .or_insert_with(Vec::new)
.push(new_running_animation); .push(new_running_animation)
} }
let animation_state = if running_animations.is_empty() { let animation_state = if running_animations.is_empty() {
@ -122,7 +123,8 @@ pub fn update_animation_state(constellation_chan: &IpcSender<ConstellationMsg>,
AnimationState::AnimationsPresent AnimationState::AnimationsPresent
}; };
constellation_chan.send(ConstellationMsg::ChangeRunningAnimationsState(pipeline_id, animation_state)) constellation_chan.send(ConstellationMsg::ChangeRunningAnimationsState(pipeline_id,
animation_state))
.unwrap(); .unwrap();
} }

View file

@ -1093,7 +1093,7 @@ impl LayoutThread {
} }
let restyles = document.drain_pending_restyles(); let restyles = document.drain_pending_restyles();
debug!("Draining restyles: {}", restyles.len()); debug!("Draining restyles: {} (needs dirtying? {:?})", restyles.len(), needs_dirtying);
if !needs_dirtying { if !needs_dirtying {
for (el, restyle) in restyles { for (el, restyle) in restyles {
// Propagate the descendant bit up the ancestors. Do this before // Propagate the descendant bit up the ancestors. Do this before
@ -1284,6 +1284,10 @@ impl LayoutThread {
} }
fn tick_animations(&mut self, rw_data: &mut LayoutThreadData) { fn tick_animations(&mut self, rw_data: &mut LayoutThreadData) {
if opts::get().relayout_event {
println!("**** pipeline={}\tForDisplay\tSpecial\tAnimationTick", self.id);
}
let reflow_info = Reflow { let reflow_info = Reflow {
goal: ReflowGoal::ForDisplay, goal: ReflowGoal::ForDisplay,
page_clip_rect: max_rect(), page_clip_rect: max_rect(),

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,26 @@ 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.

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,

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: