Skip to content

Commit

Permalink
Auto merge of #18153 - emilio:restyle-damage-cleanup, r=heycam
Browse files Browse the repository at this point in the history 
stylo: Remove a lot of the restyle damage related complexity.

The only reason why we had the `existing_style_for_style_damage` bit is to apply
some optimizations that we don't have anymore.

I still want to reintroduce a few of them, at least for the non-eager
pseudo-element case... But I think I won't need this at all.

This allows us to remove a fair amount of Gecko code too.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/18153)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Aug 20, 2017
2 parents 4f3fb90 + 0f37b20 commit 87206bd
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 199 deletions.
7 changes: 4 additions & 3 deletions components/layout/animation.rs
View file
Expand Up @@ -161,9 +161,10 @@ pub fn recalc_style_for_animations(context: &LayoutContext,
&mut fragment.style,
&ServoMetricsProvider);
let difference =
RestyleDamage::compute_style_difference(&old_style,
&old_style,
&fragment.style);
RestyleDamage::compute_style_difference(
&old_style,
&fragment.style,
);
damage |= difference.damage;
}
}
Expand Down
8 changes: 0 additions & 8 deletions components/layout_thread/dom_wrapper.rs
View file
Expand Up @@ -444,14 +444,6 @@ impl<'le> TElement for ServoLayoutElement<'le> {
}
}

#[inline]
fn existing_style_for_restyle_damage<'a>(&'a self,
current_cv: &'a ComputedValues,
_pseudo_element: Option<&PseudoElement>)
-> Option<&'a ComputedValues> {
Some(current_cv)
}

fn has_dirty_descendants(&self) -> bool {
unsafe { self.as_node().node.get_flag(HAS_DIRTY_DESCENDANTS) }
}
Expand Down
14 changes: 1 addition & 13 deletions components/style/dom.rs
View file
Expand Up @@ -20,7 +20,7 @@ use properties::{AnimationRules, ComputedValues, PropertyDeclarationBlock};
#[cfg(feature = "gecko")] use properties::animated_properties::AnimationValue;
#[cfg(feature = "gecko")] use properties::animated_properties::TransitionProperty;
use rule_tree::CascadeLevel;
use selector_parser::{AttrValue, ElementExt, PreExistingComputedValues};
use selector_parser::{AttrValue, ElementExt};
use selector_parser::{PseudoClassStringArg, PseudoElement};
use selectors::matching::{ElementSelectorFlags, VisitedHandlingMode};
use selectors::sink::Push;
Expand Down Expand Up @@ -378,18 +378,6 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone +
/// Internal iterator for the classes of this element.
fn each_class<F>(&self, callback: F) where F: FnMut(&Atom);

/// Get the pre-existing style to calculate restyle damage (change hints).
///
/// This needs to be generic since it varies between Servo and Gecko.
///
/// XXX(emilio): It's a bit unfortunate we need to pass the current computed
/// values as an argument here, but otherwise Servo would crash due to
/// double borrows to return it.
fn existing_style_for_restyle_damage<'a>(&'a self,
current_computed_values: &'a ComputedValues,
pseudo: Option<&PseudoElement>)
-> Option<&'a PreExistingComputedValues>;

/// Whether a given element may generate a pseudo-element.
///
/// This is useful to avoid computing, for example, pseudo styles for
Expand Down
16 changes: 14 additions & 2 deletions components/style/gecko/pseudo_element.rs
View file
Expand Up @@ -75,10 +75,22 @@ impl PseudoElement {
EAGER_PSEUDOS[i].clone()
}

/// Whether this pseudo-element is ::before or ::after.
/// Whether the current pseudo element is ::before or ::after.
#[inline]
pub fn is_before_or_after(&self) -> bool {
matches!(*self, PseudoElement::Before | PseudoElement::After)
self.is_before() || self.is_after()
}

/// Whether this pseudo-element is the ::before pseudo.
#[inline]
pub fn is_before(&self) -> bool {
*self == PseudoElement::Before
}

/// Whether this pseudo-element is the ::after pseudo.
#[inline]
pub fn is_after(&self) -> bool {
*self == PseudoElement::After
}

/// Whether this pseudo-element is ::first-letter.
Expand Down
41 changes: 7 additions & 34 deletions components/style/gecko/restyle_damage.rs
View file
Expand Up @@ -6,7 +6,7 @@

use gecko_bindings::bindings;
use gecko_bindings::structs;
use gecko_bindings::structs::{nsChangeHint, nsStyleContext, nsStyleStructID};
use gecko_bindings::structs::nsChangeHint;
use matching::{StyleChange, StyleDifference};
use properties::ComputedValues;
use std::ops::{BitAnd, BitOr, BitOrAssign, Not};
Expand Down Expand Up @@ -46,49 +46,22 @@ impl GeckoRestyleDamage {
/// structs, so they effectively only diff structs that have ever been
/// accessed from layout.
pub fn compute_style_difference(
source: &nsStyleContext,
old_style: &ComputedValues,
new_style: &ComputedValues,
) -> StyleDifference {
let mut any_style_changed: bool = false;
let hint = unsafe {
bindings::Gecko_CalcStyleDifference(old_style,
new_style,
source.mBits,
&mut any_style_changed)
bindings::Gecko_CalcStyleDifference(
old_style,
new_style,
structs::NS_STYLE_INHERIT_MASK as u64,
&mut any_style_changed
)
};
let change = if any_style_changed { StyleChange::Changed } else { StyleChange::Unchanged };
StyleDifference::new(GeckoRestyleDamage(nsChangeHint(hint)), change)
}

/// Computes the `StyleDifference` between the two `ComputedValues` objects
/// for the case where the old and new style are both `display: none`.
///
/// In general we don't need to generate damage for such elements, but we
/// do need to generate a frame reconstruction for `-moz-binding` changes,
/// so that we can start loading the new binding.
pub fn compute_undisplayed_style_difference(
old_style: &ComputedValues,
new_style: &ComputedValues,
) -> StyleDifference {
let mut any_style_changed: bool = false;

// Just compute the Display struct's difference.
let display_struct_bit = 1 << (nsStyleStructID::eStyleStruct_Display as u32);
let hint = unsafe {
bindings::Gecko_CalcStyleDifference(old_style,
new_style,
display_struct_bit,
&mut any_style_changed)
};

// Only pay attention to a reconstruct change hint.
let damage = GeckoRestyleDamage(nsChangeHint(hint)) & Self::reconstruct();

let change = if damage.is_empty() { StyleChange::Changed } else { StyleChange::Unchanged };
StyleDifference::new(damage, change)
}

/// Returns true if this restyle damage contains all the damage of |other|.
pub fn contains(self, other: Self) -> bool {
self & other == other
Expand Down
15 changes: 1 addition & 14 deletions components/style/gecko/wrapper.rs
View file
Expand Up @@ -45,7 +45,6 @@ use gecko_bindings::bindings::Gecko_GetExtraContentStyleDeclarations;
use gecko_bindings::bindings::Gecko_GetHTMLPresentationAttrDeclarationBlock;
use gecko_bindings::bindings::Gecko_GetSMILOverrideDeclarationBlock;
use gecko_bindings::bindings::Gecko_GetStyleAttrDeclarationBlock;
use gecko_bindings::bindings::Gecko_GetStyleContext;
use gecko_bindings::bindings::Gecko_GetUnvisitedLinkAttrDeclarationBlock;
use gecko_bindings::bindings::Gecko_GetVisitedLinkAttrDeclarationBlock;
use gecko_bindings::bindings::Gecko_IsSignificantChild;
Expand All @@ -55,7 +54,7 @@ use gecko_bindings::bindings::Gecko_UnsetDirtyStyleAttr;
use gecko_bindings::bindings::Gecko_UpdateAnimations;
use gecko_bindings::structs;
use gecko_bindings::structs::{RawGeckoElement, RawGeckoNode, RawGeckoXBLBinding};
use gecko_bindings::structs::{nsIAtom, nsIContent, nsINode_BooleanFlag, nsStyleContext};
use gecko_bindings::structs::{nsIAtom, nsIContent, nsINode_BooleanFlag};
use gecko_bindings::structs::ELEMENT_HANDLED_SNAPSHOT;
use gecko_bindings::structs::ELEMENT_HAS_ANIMATION_ONLY_DIRTY_DESCENDANTS_FOR_SERVO;
use gecko_bindings::structs::ELEMENT_HAS_DIRTY_DESCENDANTS_FOR_SERVO;
Expand Down Expand Up @@ -1020,18 +1019,6 @@ impl<'le> TElement for GeckoElement<'le> {
Gecko_ClassOrClassList)
}

fn existing_style_for_restyle_damage<'a>(&'a self,
_existing_values: &'a ComputedValues,
pseudo: Option<&PseudoElement>)
-> Option<&'a nsStyleContext> {
// TODO(emilio): Migrate this to CSSPseudoElementType.
let atom_ptr = pseudo.map_or(ptr::null_mut(), |p| p.atom().as_ptr());
unsafe {
let context_ptr = Gecko_GetStyleContext(self.0, atom_ptr);
context_ptr.as_ref()
}
}

fn has_snapshot(&self) -> bool {
self.flags() & (ELEMENT_HAS_SNAPSHOT as u32) != 0
}
Expand Down
129 changes: 31 additions & 98 deletions components/style/matching.rs
View file
Expand Up @@ -14,7 +14,6 @@ use invalidation::element::restyle_hints::{RESTYLE_CSS_ANIMATIONS, RESTYLE_CSS_T
use invalidation::element::restyle_hints::{RESTYLE_SMIL, RESTYLE_STYLE_ATTRIBUTE};
use invalidation::element::restyle_hints::RestyleHint;
use properties::ComputedValues;
use properties::longhands::display::computed_value as display;
use rule_tree::{CascadeLevel, StrongRuleNode};
use selector_parser::{PseudoElement, RestyleDamage};
use selectors::matching::ElementSelectorFlags;
Expand Down Expand Up @@ -152,14 +151,16 @@ trait PrivateMatchMethods: TElement {
}

#[cfg(feature = "gecko")]
fn needs_animations_update(&self,
context: &mut StyleContext<Self>,
old_values: Option<&Arc<ComputedValues>>,
new_values: &ComputedValues)
-> bool {
fn needs_animations_update(
&self,
context: &mut StyleContext<Self>,
old_values: Option<&Arc<ComputedValues>>,
new_values: &ComputedValues,
) -> bool {
use properties::longhands::display::computed_value as display;

let new_box_style = new_values.get_box();
let has_new_animation_style = new_box_style.animation_name_count() >= 1 &&
new_box_style.animation_name_at(0).0.is_some();
let has_new_animation_style = new_box_style.specifies_animations();
let has_animations = self.has_css_animations();

old_values.map_or(has_new_animation_style, |old| {
Expand All @@ -175,7 +176,7 @@ trait PrivateMatchMethods: TElement {
// and update only animations corresponding to those @keyframes.
(context.shared.traversal_flags.contains(traversal_flags::ForCSSRuleChanges) &&
has_new_animation_style) ||
!old_box_style.animations_equals(&new_box_style) ||
!old_box_style.animations_equals(new_box_style) ||
(old_display_style == display::T::none &&
new_display_style != display::T::none &&
has_new_animation_style) ||
Expand All @@ -188,14 +189,17 @@ trait PrivateMatchMethods: TElement {
/// Create a SequentialTask for resolving descendants in a SMIL display property
/// animation if the display property changed from none.
#[cfg(feature = "gecko")]
fn handle_display_change_for_smil_if_needed(&self,
context: &mut StyleContext<Self>,
old_values: Option<&ComputedValues>,
new_values: &ComputedValues,
restyle_hints: RestyleHint) {
fn handle_display_change_for_smil_if_needed(
&self,
context: &mut StyleContext<Self>,
old_values: Option<&ComputedValues>,
new_values: &ComputedValues,
restyle_hints: RestyleHint
) {
use context::DISPLAY_CHANGED_FROM_NONE_FOR_SMIL;
use properties::longhands::display::computed_value as display;

let display_changed_from_none = old_values.as_ref().map_or(false, |old| {
let display_changed_from_none = old_values.map_or(false, |old| {
let old_display_style = old.get_box().clone_display();
let new_display_style = new_values.get_box().clone_display();
old_display_style == display::T::none &&
Expand Down Expand Up @@ -669,19 +673,22 @@ pub trait MatchMethods : TElement {
// damage calculation for themselves, when there's an actual pseudo.
let is_existing_before_or_after =
cfg!(feature = "gecko") &&
pseudo.map_or(false, |p| p.is_before_or_after()) &&
self.existing_style_for_restyle_damage(old_values, pseudo)
.is_some();
pseudo.map_or(false, |p| {
(p.is_before() && self.before_pseudo_element().is_some()) ||
(p.is_after() && self.after_pseudo_element().is_some())
});

if is_existing_before_or_after {
return ChildCascadeRequirement::CanSkipCascade;
}

self.accumulate_damage_for(shared_context,
restyle,
old_values,
new_values,
pseudo)
self.accumulate_damage_for(
shared_context,
restyle,
old_values,
new_values,
pseudo
)
}

/// Updates the rule nodes without re-running selector matching, using just
Expand Down Expand Up @@ -821,81 +828,7 @@ pub trait MatchMethods : TElement {
pseudo: Option<&PseudoElement>
) -> StyleDifference {
debug_assert!(pseudo.map_or(true, |p| p.is_eager()));
if let Some(source) = self.existing_style_for_restyle_damage(old_values, pseudo) {
return RestyleDamage::compute_style_difference(source, old_values, new_values)
}

let new_display = new_values.get_box().clone_display();
let old_display = old_values.get_box().clone_display();

let new_style_is_display_none = new_display == display::T::none;
let old_style_is_display_none = old_display == display::T::none;

// If there's no style source, that likely means that Gecko couldn't
// find a style context.
//
// This happens with display:none elements, and not-yet-existing
// pseudo-elements.
if new_style_is_display_none && old_style_is_display_none {
// The style remains display:none. The only case we need to care
// about is if -moz-binding changed, and to generate a reconstruct
// so that we can start the binding load. Otherwise, there is no
// need for damage.
return RestyleDamage::compute_undisplayed_style_difference(old_values, new_values);
}

if pseudo.map_or(false, |p| p.is_before_or_after()) {
// FIXME(bz) This duplicates some of the logic in
// PseudoElement::should_exist, but it's not clear how best to share
// that logic without redoing the "get the display" work.
let old_style_generates_no_pseudo =
old_style_is_display_none ||
old_values.ineffective_content_property();

let new_style_generates_no_pseudo =
new_style_is_display_none ||
new_values.ineffective_content_property();

if old_style_generates_no_pseudo != new_style_generates_no_pseudo {
return StyleDifference::new(RestyleDamage::reconstruct(), StyleChange::Changed)
}

// The pseudo-element will remain undisplayed, so just avoid
// triggering any change.
//
// NOTE(emilio): We will only arrive here for pseudo-elements that
// aren't generated (see the is_existing_before_or_after check in
// accumulate_damage).
//
// However, it may be the case that the style of this element would
// make us think we need a pseudo, but we don't, like for pseudos in
// replaced elements, that's why we need the old != new instead of
// just check whether the new style would generate a pseudo.
return StyleDifference::new(RestyleDamage::empty(), StyleChange::Unchanged)
}

if pseudo.map_or(false, |p| p.is_first_letter() || p.is_first_line()) {
// No one cares about this pseudo, and we've checked above that
// we're not switching from a "cares" to a "doesn't care" state
// or vice versa.
return StyleDifference::new(RestyleDamage::empty(),
StyleChange::Unchanged)
}

// If we are changing display property we need to accumulate
// reconstruction damage for the change.
// FIXME: Bug 1378972: This is a workaround for bug 1374175, we should
// generate more accurate restyle damage in fallback cases.
let needs_reconstruction = new_display != old_display;
let damage = if needs_reconstruction {
RestyleDamage::reconstruct()
} else {
RestyleDamage::empty()
};
// We don't really know if there was a change in any style (since we
// didn't actually call compute_style_difference) but we return
// StyleChange::Changed conservatively.
StyleDifference::new(damage, StyleChange::Changed)
RestyleDamage::compute_style_difference(old_values, new_values)
}
}

Expand Down
10 changes: 0 additions & 10 deletions components/style/selector_parser.rs
View file
Expand Up @@ -35,16 +35,6 @@ pub use servo::restyle_damage::ServoRestyleDamage as RestyleDamage;
#[cfg(feature = "gecko")]
pub use gecko::restyle_damage::GeckoRestyleDamage as RestyleDamage;

/// A type that represents the previous computed values needed for restyle
/// damage calculation.
#[cfg(feature = "servo")]
pub type PreExistingComputedValues = ::properties::ComputedValues;

/// A type that represents the previous computed values needed for restyle
/// damage calculation.
#[cfg(feature = "gecko")]
pub type PreExistingComputedValues = ::gecko_bindings::structs::nsStyleContext;

/// Servo's selector parser.
#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
pub struct SelectorParser<'a> {
Expand Down

0 comments on commit 87206bd

Please sign in to comment.