Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include animations and transitions in the cascade #26806

Merged
merged 2 commits into from Jun 10, 2020
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Prev

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
commit af0bb1f7281cc029ba276c51ddc0d4d4abd34052
@@ -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;
}
}
@@ -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;
@@ -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.
///
@@ -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(),
@@ -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,
@@ -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()
@@ -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()
@@ -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,
@@ -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,
);

This comment has been minimized.

Copy link
@emilio

emilio Jun 10, 2020

Member

May be worth either asserting that rule_node is not the primary style's rule node anymore, or check for that and bail out.

This comment has been minimized.

Copy link
@mrobinson

mrobinson Jun 10, 2020

Author Member

This makes sense. I've changed it to return early if the rule nodes are equal after replacing them.


// 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
@@ -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.
@@ -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,
);
@@ -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 {
@@ -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

@@ -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

@@ -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

@@ -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

@@ -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

@@ -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

@@ -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

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.