diff --git a/components/layout/animation.rs b/components/layout/animation.rs index 5222d4ba70a..71a388dcf0c 100644 --- a/components/layout/animation.rs +++ b/components/layout/animation.rs @@ -135,7 +135,7 @@ pub fn recalc_style_for_animations(context: &SharedLayoutContext, update_style_for_animation(&context.style_context, animation, &mut fragment.style); - damage |= RestyleDamage::compute(Some(&old_style), &fragment.style); + damage |= RestyleDamage::compute(&old_style, &fragment.style); } } }); diff --git a/components/script_layout_interface/restyle_damage.rs b/components/script_layout_interface/restyle_damage.rs index 95722cc1a86..9e2cd1db514 100644 --- a/components/script_layout_interface/restyle_damage.rs +++ b/components/script_layout_interface/restyle_damage.rs @@ -51,7 +51,7 @@ impl TRestyleDamage for RestyleDamage { RestyleDamage::empty() } - fn compute(old: Option<&Arc>, + fn compute(old: &Arc, new: &Arc) -> RestyleDamage { compute_damage(old, new) } @@ -154,13 +154,7 @@ macro_rules! add_if_not_equal( }) ); -fn compute_damage(old: Option<&Arc>, new: &Arc) -> RestyleDamage { - let new = &**new; - let old: &ServoComputedValues = match old { - None => return RestyleDamage::rebuild_and_reflow(), - Some(cv) => &**cv, - }; - +fn compute_damage(old: &ServoComputedValues, new: &ServoComputedValues) -> RestyleDamage { let mut damage = RestyleDamage::empty(); // This should check every CSS property, as enumerated in the fields of diff --git a/components/style/dom.rs b/components/style/dom.rs index e21b0de6d4d..492d6d93cb6 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -45,7 +45,7 @@ impl OpaqueNode { } } -pub trait TRestyleDamage : Debug + BitOr + Copy { +pub trait TRestyleDamage : Debug + PartialEq + BitOr + Copy { /// The source for our current computed values in the cascade. This is a /// ComputedValues in Servo and a StyleContext in Gecko. /// @@ -56,7 +56,7 @@ pub trait TRestyleDamage : Debug + BitOr + Copy { /// This should be obtained via TNode::existing_style_for_restyle_damage type PreExistingComputedValues; - fn compute(old: Option<&Self::PreExistingComputedValues>, + fn compute(old: &Self::PreExistingComputedValues, new: &Arc) -> Self; fn empty() -> Self; diff --git a/components/style/matching.rs b/components/style/matching.rs index 3ac9929098d..b76a417005f 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -638,13 +638,17 @@ pub trait ElementMatchMethods : TElement { let style = &mut node.mutate_data().unwrap().style; - let damage = { - let source = - node.existing_style_for_restyle_damage((*style).as_ref()); - let damage = <::ConcreteNode as TNode> - ::ConcreteRestyleDamage::compute(source, &shared_style); - damage - }; + let damage = + match node.existing_style_for_restyle_damage((*style).as_ref()) { + Some(ref source) => { + <::ConcreteNode as TNode> + ::ConcreteRestyleDamage::compute(source, &shared_style) + } + None => { + <::ConcreteNode as TNode> + ::ConcreteRestyleDamage::rebuild_and_reflow() + } + }; let restyle_result = if shared_style.get_box().clone_display() == display::T::none { RestyleResult::Stop @@ -713,6 +717,43 @@ pub trait MatchMethods : TNode { } } + fn compute_restyle_damage(&self, + old_style: Option<&Arc>, + new_style: &Arc) -> Self::ConcreteRestyleDamage { + match self.existing_style_for_restyle_damage(old_style) { + Some(ref source) => { + Self::ConcreteRestyleDamage::compute(source, + new_style) + } + None => { + // If there's no style source, two things can happen: + // + // 1. This is not an incremental restyle (old_style is none). + // In this case we can't do too much than sending + // rebuild_and_reflow. + // + // 2. This is an incremental restyle, but the old display value + // is none, so there's no effective way for Gecko to get the + // style source. In this case, we could return either + // RestyleDamage::empty(), in the case both displays are + // none, or rebuild_and_reflow, otherwise. The first case + // should be already handled when calling this function, so + // we can assert that the new display value is not none. + // + // Also, this can be a text node (in which case we don't + // care of watching the new display value). + // + // Unfortunately we can't strongly assert part of this, since + // we style some nodes that in Gecko never generate a frame, + // like children of replaced content. Arguably, we shouldn't be + // styling those here, but until we implement that we'll have to + // stick without the assertions. + debug_assert!(new_style.get_box().clone_display() != display::T::none); + Self::ConcreteRestyleDamage::rebuild_and_reflow() + } + } + } + unsafe fn cascade_node<'a, Ctx>(&self, context: &Ctx, parent: Option, @@ -741,12 +782,8 @@ pub trait MatchMethods : TNode { let mut data = &mut *data_ref; let cloned_parent_style = ComputedValues::style_for_child_text_node(parent_style.unwrap()); - let damage = { - let existing_style = - self.existing_style_for_restyle_damage(data.style.as_ref()); - Self::ConcreteRestyleDamage::compute(existing_style, - &cloned_parent_style) - }; + let damage = + self.compute_restyle_damage(data.style.as_ref(), &cloned_parent_style); data.style = Some(cloned_parent_style); @@ -810,9 +847,7 @@ pub trait MatchMethods : TNode { Some(display) if display == this_display => { Self::ConcreteRestyleDamage::empty() } - _ => { - Self::ConcreteRestyleDamage::rebuild_and_reflow() - } + _ => Self::ConcreteRestyleDamage::rebuild_and_reflow() }; debug!("Short-circuiting traversal: {:?} {:?} {:?}", @@ -824,12 +859,8 @@ pub trait MatchMethods : TNode { // Otherwise, we just compute the damage normally, and sum up the damage // related to pseudo-elements. - let mut damage = { - let existing_style = - self.existing_style_for_restyle_damage(data.style.as_ref()); - - Self::ConcreteRestyleDamage::compute(existing_style, &final_style) - }; + let mut damage = + self.compute_restyle_damage(data.style.as_ref(), &final_style); data.style = Some(final_style); @@ -837,18 +868,23 @@ pub trait MatchMethods : TNode { // effectively comparing with the old computed values (given our style // source is the old nsStyleContext). // - // I... don't think any difference can arise from comparing with the old - // element restyle damage vs the new one, given that we're only summing - // the changes, and any change that we could miss would already have - // been caught by the parent's change. If for some reason I'm wrong on - // this, we'd have to compare computed values in Gecko too. + // We should be diffing the old pseudo-element styles with the new ones, + // see https://bugzilla.mozilla.org/show_bug.cgi?id=1293399. + // + // For now we do the following, which preserves the old Servo behavior. let existing_style = self.existing_style_for_restyle_damage(data.style.as_ref()); let data_per_pseudo = &mut data.per_pseudo; let new_style = data.style.as_ref(); + debug_assert!(new_style.is_some()); + let rebuild_and_reflow = + Self::ConcreteRestyleDamage::rebuild_and_reflow(); + + debug_assert!(existing_style.is_some() || damage == rebuild_and_reflow); + ::Impl::each_eagerly_cascaded_pseudo_element(|pseudo| { let applicable_declarations_for_this_pseudo = applicable_declarations.per_pseudo.get(&pseudo).unwrap(); @@ -868,9 +904,14 @@ pub trait MatchMethods : TNode { /* shareable = */ false, should_animate_properties); - let new_damage = - Self::ConcreteRestyleDamage::compute(existing_style, &new_pseudo_style); - damage = damage | new_damage; + // No point in computing more damage if we're already doing all + // the work. + if damage != rebuild_and_reflow { + let new_damage = + Self::ConcreteRestyleDamage::compute(existing_style.unwrap(), + &new_pseudo_style); + damage = damage | new_damage; + } data_per_pseudo.insert(pseudo, new_pseudo_style); } }); diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs index 272910ceb7a..9f9010b486e 100644 --- a/ports/geckolib/wrapper.rs +++ b/ports/geckolib/wrapper.rs @@ -95,7 +95,7 @@ impl<'ln> GeckoNode<'ln> { } } -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq)] pub struct GeckoRestyleDamage(nsChangeHint); impl TRestyleDamage for GeckoRestyleDamage { @@ -106,31 +106,10 @@ impl TRestyleDamage for GeckoRestyleDamage { GeckoRestyleDamage(unsafe { mem::transmute(0u32) }) } - fn compute(source: Option<&nsStyleContext>, + fn compute(source: &nsStyleContext, new_style: &Arc) -> Self { type Helpers = ArcHelpers; - let context = match source { - Some(ctx) => ctx as *const nsStyleContext as *mut nsStyleContext, - // If there's no style source (that is, no style context), there can - // be two reasons for it. - // - // The first one, is that this is not an incremental restyle (so we - // also don't have the old computed values). In that case the best - // we can do is return rebuild_and_reflow. - // - // The second one is that this is an incremental restyle, but the - // node has display: none. In this case, the old computed values - // should still be present, and we should be able to compare the new - // to the old display to see if it effectively needs a reflow, or we - // can do nothing on it because the old and the new display values - // are none. - // - // This is done outside of this method in servo itself, so we - // effectively only need to check for the first case. Ideally, we - // should be able to assert that in this case the display values - // differ or are not none, but we can't reach the node from here. - None => return Self::rebuild_and_reflow(), - }; + let context = source as *const nsStyleContext as *mut nsStyleContext; Helpers::borrow(new_style, |new_style| { let hint = unsafe { Gecko_CalcStyleDifference(context, new_style) };