Skip to content

Commit

Permalink
style: Cleanup unused style traversal flags.
Browse files Browse the repository at this point in the history
Some of these were unused, some of them were only used in combination with
others, so I've unified them.

In particular, Forgetful and ClearAnimationOnlyDirtyDescendants were used only
together for a very specific task (the final animation traversal), so I merged
them into something that has that name.

ClearDirtyBits was unused, so I removed along with some code that would no
longer be called.

Differential Revision: https://phabricator.services.mozilla.com/D25454
  • Loading branch information
emilio committed Apr 12, 2019
1 parent a763601 commit fce5801
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 60 deletions.
9 changes: 0 additions & 9 deletions components/style/dom.rs
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -642,15 +642,6 @@ pub trait TElement:
self.unset_dirty_descendants(); self.unset_dirty_descendants();
} }


/// Clear all element flags related to dirtiness.
///
/// In Gecko, this corresponds to the regular dirty descendants bit, the
/// animation-only dirty descendants bit, the lazy frame construction bit,
/// and the lazy frame construction descendants bit.
unsafe fn clear_dirty_bits(&self) {
self.unset_dirty_descendants();
}

/// Returns true if this element is a visited link. /// Returns true if this element is a visited link.
/// ///
/// Servo doesn't support visited styles yet. /// Servo doesn't support visited styles yet.
Expand Down
10 changes: 0 additions & 10 deletions components/style/gecko/wrapper.rs
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -1423,16 +1423,6 @@ impl<'le> TElement for GeckoElement<'le> {
) )
} }


#[inline]
unsafe fn clear_dirty_bits(&self) {
self.unset_flags(
ELEMENT_HAS_DIRTY_DESCENDANTS_FOR_SERVO as u32 |
ELEMENT_HAS_ANIMATION_ONLY_DIRTY_DESCENDANTS_FOR_SERVO as u32 |
NODE_DESCENDANTS_NEED_FRAMES as u32 |
NODE_NEEDS_FRAME as u32,
)
}

fn is_visited_link(&self) -> bool { fn is_visited_link(&self) -> bool {
self.state().intersects(ElementState::IN_VISITED_STATE) self.state().intersects(ElementState::IN_VISITED_STATE)
} }
Expand Down
6 changes: 3 additions & 3 deletions components/style/matching.rs
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ trait PrivateMatchMethods: TElement {
debug!("accumulate_damage_for: {:?}", self); debug!("accumulate_damage_for: {:?}", self);
debug_assert!(!shared_context debug_assert!(!shared_context
.traversal_flags .traversal_flags
.contains(TraversalFlags::Forgetful)); .contains(TraversalFlags::FinalAnimationTraversal));


let difference = self.compute_style_difference(old_values, new_values, pseudo); let difference = self.compute_style_difference(old_values, new_values, pseudo);


Expand Down Expand Up @@ -723,11 +723,11 @@ pub trait MatchMethods: TElement {
} }
} }


// Don't accumulate damage if we're in a forgetful traversal. // Don't accumulate damage if we're in the final animation traversal.
if context if context
.shared .shared
.traversal_flags .traversal_flags
.contains(TraversalFlags::Forgetful) .contains(TraversalFlags::FinalAnimationTraversal)
{ {
return ChildCascadeRequirement::MustCascadeChildren; return ChildCascadeRequirement::MustCascadeChildren;
} }
Expand Down
28 changes: 3 additions & 25 deletions components/style/traversal.rs
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -535,40 +535,18 @@ pub fn recalc_style_at<E, D, F>(
debug_assert!(!element.has_animation_only_dirty_descendants()); debug_assert!(!element.has_animation_only_dirty_descendants());
} }


debug_assert!(
flags.for_animation_only() ||
!flags.contains(TraversalFlags::ClearDirtyBits) ||
!element.has_animation_only_dirty_descendants(),
"Should have cleared animation bits already"
);
clear_state_after_traversing(element, data, flags); clear_state_after_traversing(element, data, flags);
} }


fn clear_state_after_traversing<E>(element: E, data: &mut ElementData, flags: TraversalFlags) fn clear_state_after_traversing<E>(element: E, data: &mut ElementData, flags: TraversalFlags)
where where
E: TElement, E: TElement,
{ {
// If we are in a forgetful traversal, drop the existing restyle if flags.intersects(TraversalFlags::FinalAnimationTraversal) {
// data here, since we won't need to perform a post-traversal to pick up debug_assert!(flags.for_animation_only());
// any change hints.
if flags.contains(TraversalFlags::Forgetful) {
data.clear_restyle_flags_and_damage(); data.clear_restyle_flags_and_damage();
}

// Clear dirty bits as appropriate.
if flags.for_animation_only() {
if flags.intersects(
TraversalFlags::ClearDirtyBits | TraversalFlags::ClearAnimationOnlyDirtyDescendants,
) {
unsafe {
element.unset_animation_only_dirty_descendants();
}
}
} else if flags.contains(TraversalFlags::ClearDirtyBits) {
// The animation traversal happens first, so we don't need to guard against
// clearing the animation bit on the regular traversal.
unsafe { unsafe {
element.clear_dirty_bits(); element.unset_animation_only_dirty_descendants();
} }
} }
} }
Expand Down
17 changes: 4 additions & 13 deletions components/style/traversal_flags.rs
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -16,15 +16,9 @@ bitflags! {
/// Traverse and update all elements with CSS animations since /// Traverse and update all elements with CSS animations since
/// @keyframes rules may have changed. Triggered by CSS rule changes. /// @keyframes rules may have changed. Triggered by CSS rule changes.
const ForCSSRuleChanges = 1 << 1; const ForCSSRuleChanges = 1 << 1;
/// A forgetful traversal ignores the previous state of the frame tree, and /// The final animation-only traversal, which shouldn't really care about other
/// thus does not compute damage or maintain other state describing the styles /// style changes anymore.
/// pre-traversal. A forgetful traversal is usually the right thing if you const FinalAnimationTraversal = 1 << 2;
/// aren't going to do a post-traversal.
const Forgetful = 1 << 3;
/// Clears all the dirty bits on the elements traversed.
const ClearDirtyBits = 1 << 5;
/// Clears the animation-only dirty descendants bit in the subtree.
const ClearAnimationOnlyDirtyDescendants = 1 << 6;
/// Allows the traversal to run in parallel if there are sufficient cores on /// Allows the traversal to run in parallel if there are sufficient cores on
/// the machine. /// the machine.
const ParallelTraversal = 1 << 7; const ParallelTraversal = 1 << 7;
Expand Down Expand Up @@ -58,10 +52,7 @@ pub fn assert_traversal_flags_match() {
check_traversal_flags! { check_traversal_flags! {
ServoTraversalFlags_AnimationOnly => TraversalFlags::AnimationOnly, ServoTraversalFlags_AnimationOnly => TraversalFlags::AnimationOnly,
ServoTraversalFlags_ForCSSRuleChanges => TraversalFlags::ForCSSRuleChanges, ServoTraversalFlags_ForCSSRuleChanges => TraversalFlags::ForCSSRuleChanges,
ServoTraversalFlags_Forgetful => TraversalFlags::Forgetful, ServoTraversalFlags_FinalAnimationTraversal => TraversalFlags::FinalAnimationTraversal,
ServoTraversalFlags_ClearDirtyBits => TraversalFlags::ClearDirtyBits,
ServoTraversalFlags_ClearAnimationOnlyDirtyDescendants =>
TraversalFlags::ClearAnimationOnlyDirtyDescendants,
ServoTraversalFlags_ParallelTraversal => TraversalFlags::ParallelTraversal, ServoTraversalFlags_ParallelTraversal => TraversalFlags::ParallelTraversal,
ServoTraversalFlags_FlushThrottledAnimations => TraversalFlags::FlushThrottledAnimations, ServoTraversalFlags_FlushThrottledAnimations => TraversalFlags::FlushThrottledAnimations,
} }
Expand Down

0 comments on commit fce5801

Please sign in to comment.