Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
animations: Don't always re-resolve the node style
When animations and transitions change don't always re-resolve node
style, just replace the animation and transition rules and re-cascade.
  • Loading branch information
mrobinson committed Jun 10, 2020
1 parent 364235a commit af0bb1f
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 108 deletions.
3 changes: 1 addition & 2 deletions components/style/animation.rs
Expand Up @@ -882,10 +882,9 @@ impl ElementAnimationSet {
}

fn cancel_active_transitions(&mut self) {
self.dirty = !self.transitions.is_empty();

for transition in self.transitions.iter_mut() {
if transition.state != AnimationState::Finished {
self.dirty = true;
transition.state = AnimationState::Canceled;
}
}
Expand Down
138 changes: 95 additions & 43 deletions components/style/matching.rs
Expand Up @@ -17,8 +17,10 @@ use crate::dom::TNode;
use crate::invalidation::element::restyle_hints::RestyleHint;
use crate::properties::longhands::display::computed_value::T as Display;
use crate::properties::ComputedValues;
use crate::properties::PropertyDeclarationBlock;
use crate::rule_tree::{CascadeLevel, StrongRuleNode};
use crate::selector_parser::{PseudoElement, RestyleDamage};
use crate::shared_lock::Locked;
use crate::style_resolver::ResolvedElementStyles;
use crate::style_resolver::{PseudoElementResolution, StyleResolverForElement};
use crate::stylist::RuleInclusion;
Expand Down Expand Up @@ -87,6 +89,29 @@ enum CascadeVisitedMode {
}

trait PrivateMatchMethods: TElement {
fn replace_single_rule_node(
context: &mut StyleContext<Self>,
level: CascadeLevel,
pdb: Option<ArcBorrow<Locked<PropertyDeclarationBlock>>>,
path: &mut StrongRuleNode,
) -> bool {
let stylist = &context.shared.stylist;
let guards = &context.shared.guards;

let mut important_rules_changed = false;
let new_node = stylist.rule_tree().update_rule_at_level(
level,
pdb,
path,
guards,
&mut important_rules_changed,
);
if let Some(n) = new_node {
*path = n;
}
important_rules_changed
}

/// Updates the rule nodes without re-running selector matching, using just
/// the rule tree, for a specific visited mode.
///
Expand All @@ -98,17 +123,11 @@ trait PrivateMatchMethods: TElement {
cascade_visited: CascadeVisitedMode,
cascade_inputs: &mut ElementCascadeInputs,
) -> bool {
use crate::properties::PropertyDeclarationBlock;
use crate::shared_lock::Locked;

debug_assert!(
replacements.intersects(RestyleHint::replacements()) &&
(replacements & !RestyleHint::replacements()).is_empty()
);

let stylist = &context.shared.stylist;
let guards = &context.shared.guards;

let primary_rules = match cascade_visited {
CascadeVisitedMode::Unvisited => cascade_inputs.primary.rules.as_mut(),
CascadeVisitedMode::Visited => cascade_inputs.primary.visited_rules.as_mut(),
Expand All @@ -119,34 +138,18 @@ trait PrivateMatchMethods: TElement {
None => return false,
};

let replace_rule_node = |level: CascadeLevel,
pdb: Option<ArcBorrow<Locked<PropertyDeclarationBlock>>>,
path: &mut StrongRuleNode|
-> bool {
let mut important_rules_changed = false;
let new_node = stylist.rule_tree().update_rule_at_level(
level,
pdb,
path,
guards,
&mut important_rules_changed,
);
if let Some(n) = new_node {
*path = n;
}
important_rules_changed
};

if !context.shared.traversal_flags.for_animation_only() {
let mut result = false;
if replacements.contains(RestyleHint::RESTYLE_STYLE_ATTRIBUTE) {
let style_attribute = self.style_attribute();
result |= replace_rule_node(
result |= Self::replace_single_rule_node(
context,
CascadeLevel::same_tree_author_normal(),
style_attribute,
primary_rules,
);
result |= replace_rule_node(
result |= Self::replace_single_rule_node(
context,
CascadeLevel::same_tree_author_important(),
style_attribute,
primary_rules,
Expand All @@ -166,15 +169,17 @@ trait PrivateMatchMethods: TElement {
debug_assert!(context.shared.traversal_flags.for_animation_only());

if replacements.contains(RestyleHint::RESTYLE_SMIL) {
replace_rule_node(
Self::replace_single_rule_node(
context,
CascadeLevel::SMILOverride,
self.smil_override(),
primary_rules,
);
}

if replacements.contains(RestyleHint::RESTYLE_CSS_TRANSITIONS) {
replace_rule_node(
Self::replace_single_rule_node(
context,
CascadeLevel::Transitions,
self.transition_rule(&context.shared)
.as_ref()
Expand All @@ -184,7 +189,8 @@ trait PrivateMatchMethods: TElement {
}

if replacements.contains(RestyleHint::RESTYLE_CSS_ANIMATIONS) {
replace_rule_node(
Self::replace_single_rule_node(
context,
CascadeLevel::Animations,
self.animation_rule(&context.shared)
.as_ref()
Expand Down Expand Up @@ -374,12 +380,13 @@ trait PrivateMatchMethods: TElement {
&self,
context: &mut StyleContext<Self>,
old_values: &mut Option<Arc<ComputedValues>>,
new_values: &mut Arc<ComputedValues>,
new_styles: &mut ResolvedElementStyles,
restyle_hint: RestyleHint,
important_rules_changed: bool,
) {
use crate::context::UpdateAnimationsTasks;

let new_values = new_styles.primary_style_mut();
if context.shared.traversal_flags.for_animation_only() {
self.handle_display_change_for_smil_if_needed(
context,
Expand Down Expand Up @@ -458,10 +465,65 @@ trait PrivateMatchMethods: TElement {
&self,
context: &mut StyleContext<Self>,
old_values: &mut Option<Arc<ComputedValues>>,
new_values: &mut Arc<ComputedValues>,
new_resolved_styles: &mut ResolvedElementStyles,
_restyle_hint: RestyleHint,
_important_rules_changed: bool,
) {
if !self.process_animations_for_style(
context,
old_values,
new_resolved_styles.primary_style_mut(),
) {
return;
}

// If we have modified animation or transitions, we recascade style for this node.
let mut rule_node = new_resolved_styles.primary_style().rules().clone();
Self::replace_single_rule_node(
context,
CascadeLevel::Transitions,
self.transition_rule(&context.shared)
.as_ref()
.map(|a| a.borrow_arc()),
&mut rule_node,
);
Self::replace_single_rule_node(
context,
CascadeLevel::Animations,
self.animation_rule(&context.shared)
.as_ref()
.map(|a| a.borrow_arc()),
&mut rule_node,
);

// If these animations haven't modified the rule now, we can just exit early.
if rule_node == *new_resolved_styles.primary_style().rules() {
return;
}

let inputs = CascadeInputs {
rules: Some(rule_node),
visited_rules: new_resolved_styles.primary_style().visited_rules().cloned(),
};

let style = StyleResolverForElement::new(
*self,
context,
RuleInclusion::All,
PseudoElementResolution::IfApplicable,
)
.cascade_style_and_visited_with_default_parents(inputs);

new_resolved_styles.primary.style = style;
}

#[cfg(feature = "servo")]
fn process_animations_for_style(
&self,
context: &mut StyleContext<Self>,
old_values: &mut Option<Arc<ComputedValues>>,
new_values: &mut Arc<ComputedValues>,
) -> bool {
use crate::animation::AnimationState;

// We need to call this before accessing the `ElementAnimationSet` from the
Expand Down Expand Up @@ -533,17 +595,7 @@ trait PrivateMatchMethods: TElement {
.insert(this_opaque, animation_set);
}

// If we have modified animation or transitions, we recascade style for this node.
if changed_animations {
let mut resolver = StyleResolverForElement::new(
*self,
context,
RuleInclusion::All,
PseudoElementResolution::IfApplicable,
);
let new_primary = resolver.resolve_style_with_default_parents();
*new_values = new_primary.primary.style.0;
}
changed_animations
}

/// Computes and applies non-redundant damage.
Expand Down Expand Up @@ -709,7 +761,7 @@ pub trait MatchMethods: TElement {
self.process_animations(
context,
&mut data.styles.primary,
&mut new_styles.primary.style.0,
&mut new_styles,
data.hint,
important_rules_changed,
);
Expand Down
12 changes: 12 additions & 0 deletions components/style/style_resolver.rs
Expand Up @@ -66,6 +66,18 @@ pub struct ResolvedElementStyles {
pub pseudos: EagerPseudoStyles,
}

impl ResolvedElementStyles {
/// Convenience accessor for the primary style.
pub fn primary_style(&self) -> &Arc<ComputedValues> {
&self.primary.style.0
}

/// Convenience mutable accessor for the style.
pub fn primary_style_mut(&mut self) -> &mut Arc<ComputedValues> {
&mut self.primary.style.0
}
}

impl PrimaryStyle {
/// Convenience accessor for the style.
pub fn style(&self) -> &ComputedValues {
Expand Down
Expand Up @@ -5,24 +5,9 @@
[Web Animations: property <opacity> from [0\] to [1\] at (0) should be [0\]]
expected: FAIL

[CSS Transitions with transition: all: property <opacity> from [inherit\] to [0.2\] at (1.5) should be [0\]]
expected: FAIL

[CSS Transitions with transition: all: property <opacity> from [initial\] to [0.2\] at (1.5) should be [0\]]
expected: FAIL

[CSS Transitions: property <opacity> from [inherit\] to [0.2\] at (1.5) should be [0\]]
expected: FAIL

[Web Animations: property <opacity> from [inherit\] to [0.2\] at (0.3) should be [0.62\]]
expected: FAIL

[CSS Transitions: property <opacity> from [0\] to [1\] at (1.5) should be [1\]]
expected: FAIL

[CSS Transitions with transition: all: property <opacity> from [unset\] to [0.2\] at (1.5) should be [0\]]
expected: FAIL

[Web Animations: property <opacity> from [initial\] to [0.2\] at (0.6) should be [0.52\]]
expected: FAIL

Expand All @@ -44,9 +29,6 @@
[Web Animations: property <opacity> from [inherit\] to [0.2\] at (-0.3) should be [0.98\]]
expected: FAIL

[CSS Animations: property <opacity> from [inherit\] to [0.2\] at (1.5) should be [0\]]
expected: FAIL

[Web Animations: property <opacity> from [initial\] to [0.2\] at (-0.3) should be [1\]]
expected: FAIL

Expand All @@ -56,15 +38,6 @@
[Web Animations: property <opacity> from [initial\] to [0.2\] at (0) should be [1\]]
expected: FAIL

[CSS Transitions: property <opacity> from [initial\] to [0.2\] at (1.5) should be [0\]]
expected: FAIL

[CSS Animations: property <opacity> from [unset\] to [0.2\] at (-0.3) should be [1\]]
expected: FAIL

[CSS Transitions: property <opacity> from [unset\] to [0.2\] at (1.5) should be [0\]]
expected: FAIL

[Web Animations: property <opacity> from [0\] to [1\] at (0.3) should be [0.3\]]
expected: FAIL

Expand All @@ -74,9 +47,6 @@
[Web Animations: property <opacity> from [inherit\] to [0.2\] at (1.5) should be [0\]]
expected: FAIL

[CSS Animations: property <opacity> from [0\] to [1\] at (1.5) should be [1\]]
expected: FAIL

[Web Animations: property <opacity> from [unset\] to [0.2\] at (1) should be [0.2\]]
expected: FAIL

Expand All @@ -86,12 +56,6 @@
[Web Animations: property <opacity> from [inherit\] to [0.2\] at (0.6) should be [0.44\]]
expected: FAIL

[CSS Animations: property <opacity> from [initial\] to [0.2\] at (1.5) should be [0\]]
expected: FAIL

[CSS Animations: property <opacity> from [0\] to [1\] at (-0.3) should be [0\]]
expected: FAIL

[Web Animations: property <opacity> from [unset\] to [0.2\] at (-0.3) should be [1\]]
expected: FAIL

Expand All @@ -101,12 +65,6 @@
[Web Animations: property <opacity> from [unset\] to [0.2\] at (1.5) should be [0\]]
expected: FAIL

[CSS Animations: property <opacity> from [unset\] to [0.2\] at (1.5) should be [0\]]
expected: FAIL

[CSS Transitions with transition: all: property <opacity> from [0\] to [1\] at (1.5) should be [1\]]
expected: FAIL

[Web Animations: property <opacity> from [0\] to [1\] at (-0.3) should be [0\]]
expected: FAIL

Expand All @@ -131,27 +89,6 @@
[Web Animations: property <opacity> from neutral to [0.2\] at (-0.3) should be [0.07\]]
expected: FAIL

[CSS Animations: property <opacity> from [initial\] to [0.2\] at (-0.3) should be [1\]]
expected: FAIL

[Web Animations: property <opacity> from [initial\] to [0.2\] at (0.3) should be [0.76\]]
expected: FAIL

[CSS Transitions with transition: all: property <opacity> from [0\] to [1\] at (-0.3) should be [0\]]
expected: FAIL

[CSS Transitions: property <opacity> from [initial\] to [0.2\] at (-0.3) should be [1\]]
expected: FAIL

[CSS Transitions with transition: all: property <opacity> from [initial\] to [0.2\] at (-0.3) should be [1\]]
expected: FAIL

[CSS Transitions: property <opacity> from [0\] to [1\] at (-0.3) should be [0\]]
expected: FAIL

[CSS Transitions: property <opacity> from [unset\] to [0.2\] at (-0.3) should be [1\]]
expected: FAIL

[CSS Transitions with transition: all: property <opacity> from [unset\] to [0.2\] at (-0.3) should be [1\]]
expected: FAIL

0 comments on commit af0bb1f

Please sign in to comment.