From 1c1e487491c5de4f6d9c686d82b1d3049568336c Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Mon, 17 Apr 2017 14:22:48 +0800 Subject: [PATCH 1/7] Bug 1341372 - Part 1: Let animation-only restyle include css-transition. Animation-only restyle should include both Animation and Transition cascade levels. MozReview-Commit-ID: 5l6gaJKbixM --- components/style/data.rs | 10 +++---- components/style/dom.rs | 9 ++++++ components/style/gecko/wrapper.rs | 11 +++++++ components/style/matching.rs | 49 +++++++++++++++++++++---------- components/style/restyle_hints.rs | 13 +++++++- ports/geckolib/glue.rs | 9 ++++-- 6 files changed, 77 insertions(+), 24 deletions(-) diff --git a/components/style/data.rs b/components/style/data.rs index 7e42511b559..a1cc8b59f65 100644 --- a/components/style/data.rs +++ b/components/style/data.rs @@ -9,7 +9,7 @@ use dom::TElement; use properties::ComputedValues; use properties::longhands::display::computed_value as display; -use restyle_hints::{RESTYLE_CSS_ANIMATIONS, RESTYLE_DESCENDANTS, RESTYLE_LATER_SIBLINGS, RESTYLE_SELF, RestyleHint}; +use restyle_hints::{RESTYLE_DESCENDANTS, RESTYLE_LATER_SIBLINGS, RESTYLE_SELF, RestyleHint}; use rule_tree::StrongRuleNode; use selector_parser::{EAGER_PSEUDO_COUNT, PseudoElement, RestyleDamage, Snapshot}; #[cfg(feature = "servo")] use std::collections::HashMap; @@ -199,13 +199,13 @@ impl StoredRestyleHint { // In the middle of an animation only restyle, we don't need to // propagate any restyle hints, and we need to remove ourselves. if traversal_flags.for_animation_only() { - if self.0.contains(RESTYLE_CSS_ANIMATIONS) { - self.0.remove(RESTYLE_CSS_ANIMATIONS); + if self.0.intersects(RestyleHint::for_animations()) { + self.0.remove(RestyleHint::for_animations()); } return Self::empty(); } - debug_assert!(!self.0.contains(RESTYLE_CSS_ANIMATIONS), + debug_assert!(!self.0.intersects(RestyleHint::for_animations()), "There should not be any animation restyle hints \ during normal traversal"); @@ -259,7 +259,7 @@ impl StoredRestyleHint { /// Returns true if the hint has animation-only restyle. pub fn has_animation_hint(&self) -> bool { - self.0.contains(RESTYLE_CSS_ANIMATIONS) + self.0.intersects(RestyleHint::for_animations()) } } diff --git a/components/style/dom.rs b/components/style/dom.rs index ea2383f409e..0221160b3e9 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -14,6 +14,7 @@ use data::ElementData; use element_state::ElementState; use font_metrics::FontMetricsProvider; use properties::{ComputedValues, PropertyDeclarationBlock}; +use rule_tree::CascadeLevel; use selector_parser::{ElementExt, PreExistingComputedValues, PseudoElement}; use selectors::matching::ElementSelectorFlags; use shared_lock::Locked; @@ -320,6 +321,14 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone + AnimationRules(None, None) } + /// Get this element's animation rule by the cascade level. + fn get_animation_rule_by_cascade(&self, + _pseudo: Option<&PseudoElement>, + _cascade_level: CascadeLevel) + -> Option>> { + None + } + /// Get this element's animation rule. fn get_animation_rule(&self, _pseudo: Option<&PseudoElement>) -> Option>> { diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 610686ab6de..45dce1d304a 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -528,6 +528,17 @@ impl<'le> TElement for GeckoElement<'le> { self.get_transition_rule(pseudo)) } + fn get_animation_rule_by_cascade(&self, + pseudo: Option<&PseudoElement>, + cascade_level: ServoCascadeLevel) + -> Option>> { + match cascade_level { + ServoCascadeLevel::Animations => self.get_animation_rule(pseudo), + ServoCascadeLevel::Transitions => self.get_transition_rule(pseudo), + _ => panic!("Unsupported cascade level for getting the animation rule") + } + } + fn get_animation_rule(&self, pseudo: Option<&PseudoElement>) -> Option>> { get_animation_rule(self, pseudo, CascadeLevel::Animations) diff --git a/components/style/matching.rs b/components/style/matching.rs index 595f7a10eb9..80459785733 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -19,7 +19,7 @@ use dom::{AnimationRules, SendElement, TElement, TNode}; use font_metrics::FontMetricsProvider; use properties::{CascadeFlags, ComputedValues, SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP, cascade}; use properties::longhands::display::computed_value as display; -use restyle_hints::{RESTYLE_STYLE_ATTRIBUTE, RESTYLE_CSS_ANIMATIONS, RestyleHint}; +use restyle_hints::{RESTYLE_STYLE_ATTRIBUTE, RESTYLE_CSS_ANIMATIONS, RESTYLE_CSS_TRANSITIONS, RestyleHint}; use rule_tree::{CascadeLevel, RuleTree, StrongRuleNode}; use selector_parser::{PseudoElement, RestyleDamage, SelectorImpl}; use selectors::bloom::BloomFilter; @@ -984,24 +984,43 @@ pub trait MatchMethods : TElement { } }; - // RESTYLE_CSS_ANIMATIONS is processed prior to other restyle hints - // in the name of animation-only traversal. Rest of restyle hints + // RESTYLE_CSS_ANIMATIONS or RESTYLE_CSS_TRANSITIONS is processed prior to other + // restyle hints in the name of animation-only traversal. Rest of restyle hints // will be processed in a subsequent normal traversal. - if hint.contains(RESTYLE_CSS_ANIMATIONS) { + if hint.intersects(RestyleHint::for_animations()) { debug_assert!(context.shared.traversal_flags.for_animation_only()); - let animation_rule = self.get_animation_rule(None); - replace_rule_node(CascadeLevel::Animations, - animation_rule.as_ref(), - primary_rules); - - let pseudos = &mut element_styles.pseudos; - for pseudo in pseudos.keys().iter().filter(|p| p.is_before_or_after()) { - let animation_rule = self.get_animation_rule(Some(&pseudo)); - let pseudo_rules = &mut pseudos.get_mut(&pseudo).unwrap().rules; - replace_rule_node(CascadeLevel::Animations, + use data::EagerPseudoStyles; + let mut replace_rule_node_for_animation = |level: CascadeLevel, + primary_rules: &mut StrongRuleNode, + pseudos: &mut EagerPseudoStyles| { + let animation_rule = self.get_animation_rule_by_cascade(None, level); + replace_rule_node(level, animation_rule.as_ref(), - pseudo_rules); + primary_rules); + + for pseudo in pseudos.keys().iter().filter(|p| p.is_before_or_after()) { + let animation_rule = self.get_animation_rule_by_cascade(Some(&pseudo), level); + let pseudo_rules = &mut pseudos.get_mut(&pseudo).unwrap().rules; + replace_rule_node(level, + animation_rule.as_ref(), + pseudo_rules); + } + }; + + // Apply Transition rules and Animation rules if the corresponding restyle hint + // is contained. + let pseudos = &mut element_styles.pseudos; + if hint.contains(RESTYLE_CSS_TRANSITIONS) { + replace_rule_node_for_animation(CascadeLevel::Transitions, + primary_rules, + pseudos); + } + + if hint.contains(RESTYLE_CSS_ANIMATIONS) { + replace_rule_node_for_animation(CascadeLevel::Animations, + primary_rules, + pseudos); } } else if hint.contains(RESTYLE_STYLE_ATTRIBUTE) { let style_attribute = self.style_attribute(); diff --git a/components/style/restyle_hints.rs b/components/style/restyle_hints.rs index 8256722494a..406de50b157 100644 --- a/components/style/restyle_hints.rs +++ b/components/style/restyle_hints.rs @@ -48,6 +48,11 @@ bitflags! { /// of their descendants. const RESTYLE_LATER_SIBLINGS = 0x08, + /// Replace the style data coming from CSS transitions without updating + /// any other style data. This hint is only processed in animation-only + /// traversal which is prior to normal traversal. + const RESTYLE_CSS_TRANSITIONS = 0x10, + /// Replace the style data coming from CSS animations without updating /// any other style data. This hint is only processed in animation-only /// traversal which is prior to normal traversal. @@ -87,6 +92,7 @@ pub fn assert_restyle_hints_match() { // (RESTYLE_SELF | RESTYLE_DESCENDANTS). nsRestyleHint_eRestyle_Subtree => RESTYLE_DESCENDANTS, nsRestyleHint_eRestyle_LaterSiblings => RESTYLE_LATER_SIBLINGS, + nsRestyleHint_eRestyle_CSSTransitions => RESTYLE_CSS_TRANSITIONS, nsRestyleHint_eRestyle_CSSAnimations => RESTYLE_CSS_ANIMATIONS, nsRestyleHint_eRestyle_StyleAttribute => RESTYLE_STYLE_ATTRIBUTE, } @@ -96,7 +102,12 @@ impl RestyleHint { /// The subset hints that affect the styling of a single element during the /// traversal. pub fn for_self() -> Self { - RESTYLE_SELF | RESTYLE_STYLE_ATTRIBUTE | RESTYLE_CSS_ANIMATIONS + RESTYLE_SELF | RESTYLE_STYLE_ATTRIBUTE | RESTYLE_CSS_ANIMATIONS | RESTYLE_CSS_TRANSITIONS + } + + /// The subset hints that are used for animation restyle. + pub fn for_animations() -> Self { + RESTYLE_CSS_ANIMATIONS | RESTYLE_CSS_TRANSITIONS } } diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index d16fab6695a..8c7d2175a16 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -1788,12 +1788,15 @@ pub extern "C" fn Servo_NoteExplicitHints(element: RawGeckoElementBorrowed, debug!("Servo_NoteExplicitHints: {:?}, restyle_hint={:?}, change_hint={:?}", element, restyle_hint, change_hint); debug_assert!(restyle_hint == structs::nsRestyleHint_eRestyle_CSSAnimations || - (restyle_hint.0 & structs::nsRestyleHint_eRestyle_CSSAnimations.0) == 0, - "eRestyle_CSSAnimations should only appear by itself"); + restyle_hint == structs::nsRestyleHint_eRestyle_CSSTransitions || + (restyle_hint.0 & (structs::nsRestyleHint_eRestyle_CSSAnimations.0 | + structs::nsRestyleHint_eRestyle_CSSTransitions.0)) == 0, + "eRestyle_CSSAnimations or eRestyle_CSSTransitions should only appear by itself"); let mut maybe_data = element.mutate_data(); let maybe_restyle_data = maybe_data.as_mut().and_then(|d| unsafe { - maybe_restyle(d, element, restyle_hint == structs::nsRestyleHint_eRestyle_CSSAnimations) + maybe_restyle(d, element, restyle_hint == structs::nsRestyleHint_eRestyle_CSSAnimations || + restyle_hint == structs::nsRestyleHint_eRestyle_CSSTransitions) }); if let Some(restyle_data) = maybe_restyle_data { let restyle_hint: RestyleHint = restyle_hint.into(); From b1476f1f81ced4b6e1b23a08f1a216c3b90e80f8 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Mon, 17 Apr 2017 14:25:10 +0800 Subject: [PATCH 2/7] Bug 1341372 - Part 2: Support has_css_transitions. Add one FFI to check if there is any transition in CSSTransitionCollection. This will be used to check if we need to update transition and if we should compute the after-change style. MozReview-Commit-ID: 6HpVAtrx6Rc --- components/script/layout_wrapper.rs | 4 ++++ components/style/dom.rs | 3 +++ components/style/gecko/wrapper.rs | 6 ++++++ components/style/gecko_bindings/bindings.rs | 5 +++++ 4 files changed, 18 insertions(+) diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index e4df1b77ec3..12d830e5fad 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -468,6 +468,10 @@ impl<'le> TElement for ServoLayoutElement<'le> { fn has_css_animations(&self, _pseudo: Option<&PseudoElement>) -> bool { panic!("this should be only called on gecko"); } + + fn has_css_transitions(&self, _pseudo: Option<&PseudoElement>) -> bool { + panic!("this should be only called on gecko"); + } } impl<'le> PartialEq for ServoLayoutElement<'le> { diff --git a/components/style/dom.rs b/components/style/dom.rs index 0221160b3e9..f56194fb14b 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -472,6 +472,9 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone + /// Returns true if the element has a CSS animation. fn has_css_animations(&self, _pseudo: Option<&PseudoElement>) -> bool; + /// Returns true if the element has a CSS transition. + fn has_css_transitions(&self, _pseudo: Option<&PseudoElement>) -> bool; + /// Returns true if the element has animation restyle hints. fn has_animation_restyle_hints(&self) -> bool { let data = match self.borrow_data() { diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 45dce1d304a..d429c905b71 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -34,6 +34,7 @@ use gecko_bindings::bindings::{Gecko_SetNodeFlags, Gecko_UnsetNodeFlags}; use gecko_bindings::bindings::Gecko_ClassOrClassList; use gecko_bindings::bindings::Gecko_ElementHasAnimations; use gecko_bindings::bindings::Gecko_ElementHasCSSAnimations; +use gecko_bindings::bindings::Gecko_ElementHasCSSTransitions; use gecko_bindings::bindings::Gecko_GetAnimationRule; use gecko_bindings::bindings::Gecko_GetExtraContentStyleDeclarations; use gecko_bindings::bindings::Gecko_GetHTMLPresentationAttrDeclarationBlock; @@ -709,6 +710,11 @@ impl<'le> TElement for GeckoElement<'le> { let atom_ptr = PseudoElement::ns_atom_or_null_from_opt(pseudo); unsafe { Gecko_ElementHasCSSAnimations(self.0, atom_ptr) } } + + fn has_css_transitions(&self, pseudo: Option<&PseudoElement>) -> bool { + let atom_ptr = PseudoElement::ns_atom_or_null_from_opt(pseudo); + unsafe { Gecko_ElementHasCSSTransitions(self.0, atom_ptr) } + } } impl<'le> PartialEq for GeckoElement<'le> { diff --git a/components/style/gecko_bindings/bindings.rs b/components/style/gecko_bindings/bindings.rs index 2d66af833c4..bd7b434c5d8 100644 --- a/components/style/gecko_bindings/bindings.rs +++ b/components/style/gecko_bindings/bindings.rs @@ -648,6 +648,11 @@ extern "C" { aPseudoTagOrNull: *mut nsIAtom) -> bool; } +extern "C" { + pub fn Gecko_ElementHasCSSTransitions(aElement: RawGeckoElementBorrowed, + aPseudoTagOrNull: *mut nsIAtom) + -> bool; +} extern "C" { pub fn Gecko_GetProgressFromComputedTiming(aComputedTiming: RawGeckoComputedTimingBorrowed) From 145e1b6bff35917f4fd400eb97f66a145b338c5f Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Tue, 4 Apr 2017 17:54:30 +0800 Subject: [PATCH 3/7] Bug 1341372 - Part 3: Factor out need_update_animations. We will add another function, needs_update_transitions, to check if we need to update transitions, so factor this out. MozReview-Commit-ID: 5LYkyi4aDri --- components/style/matching.rs | 46 ++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/components/style/matching.rs b/components/style/matching.rs index 80459785733..5b3774b8862 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -524,6 +524,31 @@ trait PrivateMatchMethods: TElement { pseudo_style.is_some()) } + #[cfg(feature = "gecko")] + fn needs_update_animations(&self, + old_values: &Option>, + new_values: &Arc, + pseudo: Option<&PseudoElement>) -> bool { + let ref 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.len() != 0; + let has_animations = self.has_css_animations(pseudo); + + old_values.as_ref().map_or(has_new_animation_style, |ref old| { + let ref old_box_style = old.get_box(); + let old_display_style = old_box_style.clone_display(); + let new_display_style = new_box_style.clone_display(); + // FIXME: Bug 1344581: We still need to compare keyframe rules. + !old_box_style.animations_equals(&new_box_style) || + (old_display_style == display::T::none && + new_display_style != display::T::none && + has_new_animation_style) || + (old_display_style != display::T::none && + new_display_style == display::T::none && + has_animations) + }) + } + #[cfg(feature = "gecko")] fn process_animations(&self, context: &mut StyleContext, @@ -533,27 +558,8 @@ trait PrivateMatchMethods: TElement { use context::{CSS_ANIMATIONS, EFFECT_PROPERTIES}; use context::UpdateAnimationsTasks; - let ref 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.len() != 0; - let has_animations = self.has_css_animations(pseudo); - let mut tasks = UpdateAnimationsTasks::empty(); - let needs_update_animations = - old_values.as_ref().map_or(has_new_animation_style, |ref old| { - let ref old_box_style = old.get_box(); - let old_display_style = old_box_style.clone_display(); - let new_display_style = new_box_style.clone_display(); - // FIXME: Bug 1344581: We still need to compare keyframe rules. - !old_box_style.animations_equals(&new_box_style) || - (old_display_style == display::T::none && - new_display_style != display::T::none && - has_new_animation_style) || - (old_display_style != display::T::none && - new_display_style == display::T::none && - has_animations) - }); - if needs_update_animations { + if self.needs_update_animations(old_values, new_values, pseudo) { tasks.insert(CSS_ANIMATIONS); } if self.has_animations(pseudo) { From c88ca7dcd01fe6feb56ec4ca2c8ec4a363c031e5 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Tue, 4 Apr 2017 18:45:11 +0800 Subject: [PATCH 4/7] Bug 1341372 - Part 4: Let get_after_change_style return Option. It is possible to call get_after_change_style if there is no transition rule. In order to avoid cloning the token computed values, we just return None. The caller can use this Option to know which computed values should be used. MozReview-Commit-ID: 7fcgSVEtXWh --- components/style/matching.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/components/style/matching.rs b/components/style/matching.rs index 5b3774b8862..88daa198d57 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -496,32 +496,34 @@ trait PrivateMatchMethods: TElement { } } + /// get_after_change_style removes the transition rules from the ComputedValues. + /// If there is no transition rule in the ComputedValues, it returns None. #[cfg(feature = "gecko")] fn get_after_change_style(&self, context: &mut StyleContext, primary_style: &ComputedStyle, pseudo_style: &Option<(&PseudoElement, &ComputedStyle)>) - -> Arc { + -> Option> { let style = &pseudo_style.as_ref().map_or(primary_style, |p| &*p.1); let rule_node = &style.rules; let without_transition_rules = context.shared.stylist.rule_tree.remove_transition_rule_if_applicable(rule_node); if without_transition_rules == *rule_node { - // Note that unwrapping here is fine, because the style is - // only incomplete during the styling process. - return style.values.as_ref().unwrap().clone(); + // We don't have transition rule in this case, so return None to let the caller + // use the original ComputedValues. + return None; } let mut cascade_flags = CascadeFlags::empty(); if self.skip_root_and_item_based_display_fixup() { - cascade_flags.insert(SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP) + cascade_flags.insert(SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP); } - self.cascade_with_rules(context.shared, - &context.thread_local.font_metrics_provider, - &without_transition_rules, - primary_style, - cascade_flags, - pseudo_style.is_some()) + Some(self.cascade_with_rules(context.shared, + &context.thread_local.font_metrics_provider, + &without_transition_rules, + primary_style, + cascade_flags, + pseudo_style.is_some())) } #[cfg(feature = "gecko")] From 2399cde5048d8eedd5683bf9c30268755846112e Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Fri, 14 Apr 2017 10:41:04 +0800 Subject: [PATCH 5/7] Bug 1341372 - Part 5: Add TransitionProperty::any. TransitionProperty::any returns true if one of its closure returns true. MozReview-Commit-ID: 4YsKkHaWCYq --- .../properties/helpers/animated_properties.mako.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index ff11c9d5930..51f2dc69393 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -66,6 +66,19 @@ impl TransitionProperty { % endfor } + /// Iterates over every property that is not TransitionProperty::All, stopping and returning + /// true when the provided callback returns true for the first time. + pub fn any bool>(mut cb: F) -> bool { + % for prop in data.longhands: + % if prop.animatable: + if cb(TransitionProperty::${prop.camel_case}) { + return true; + } + % endif + % endfor + false + } + /// Parse a transition-property value. pub fn parse(input: &mut Parser) -> Result { match_ignore_ascii_case! { &try!(input.expect_ident()), From eb8db7b89216558396e1b24da54246b970a35d75 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Mon, 17 Apr 2017 14:32:41 +0800 Subject: [PATCH 6/7] Bug 1341372 - Part 6: Trigger transitions. 1. We need to call get_after_change_style, which is the computed styles without transition rules, while process_animations. 2. If we have after-change style, we may replace the new computed values with after-change style, according to whether we really need to update transitions. 3. There are some cases we don't update transitions, so we need to early return. might_needs_transitions_update() will check it first and it will filter out most common cases. 4. needs_transitions_update() will check each property and existing running transitions to make sure we really don't need to update transitions. The logic of this function is similar with that of nsTransitionManager::DoUpdateTransitions(). MozReview-Commit-ID: 2ccdPjgrxKz --- components/style/context.rs | 35 ++++- components/style/dom.rs | 45 +++++- components/style/gecko/wrapper.rs | 166 +++++++++++++++++++- components/style/gecko_bindings/bindings.rs | 19 +++ components/style/matching.rs | 56 ++++++- components/style/properties/gecko.mako.rs | 10 ++ 6 files changed, 315 insertions(+), 16 deletions(-) diff --git a/components/style/context.rs b/components/style/context.rs index f89674e8d04..bf12841f8a4 100644 --- a/components/style/context.rs +++ b/components/style/context.rs @@ -19,6 +19,7 @@ use font_metrics::FontMetricsProvider; #[cfg(feature = "gecko")] use gecko_bindings::structs; use matching::StyleSharingCandidateCache; use parking_lot::RwLock; +#[cfg(feature = "gecko")] use properties::ComputedValues; #[cfg(feature = "gecko")] use selector_parser::PseudoElement; use selectors::matching::ElementSelectorFlags; #[cfg(feature = "servo")] use servo_config::opts; @@ -285,10 +286,22 @@ bitflags! { pub enum SequentialTask { /// Entry to avoid an unused type parameter error on servo. Unused(SendElement), + + /// Performs one of a number of possible tasks related to updating animations based on the + /// |tasks| field. These include updating CSS animations/transitions that changed as part + /// of the non-animation style traversal, and updating the computed effect properties. #[cfg(feature = "gecko")] - /// Marks that we need to update CSS animations, update effect properties of - /// any type of animations after the normal traversal. - UpdateAnimations(SendElement, Option, UpdateAnimationsTasks), + UpdateAnimations { + /// The target element. + el: SendElement, + /// The target pseudo element. + pseudo: Option, + /// The before-change style for transitions. We use before-change style as the initial + /// value of its Keyframe. Required if |tasks| includes CSSTransitions. + before_change_style: Option>, + /// The tasks which are performed in this SequentialTask. + tasks: UpdateAnimationsTasks + }, } impl SequentialTask { @@ -299,18 +312,24 @@ impl SequentialTask { match self { Unused(_) => unreachable!(), #[cfg(feature = "gecko")] - UpdateAnimations(el, pseudo, tasks) => { - unsafe { el.update_animations(pseudo.as_ref(), tasks) }; + UpdateAnimations { el, pseudo, before_change_style, tasks } => { + unsafe { el.update_animations(pseudo.as_ref(), before_change_style, tasks) }; } } } + /// Creates a task to update various animation-related state on + /// a given (pseudo-)element. #[cfg(feature = "gecko")] - /// Creates a task to update various animation state on a given (pseudo-)element. - pub fn update_animations(el: E, pseudo: Option, + pub fn update_animations(el: E, + pseudo: Option, + before_change_style: Option>, tasks: UpdateAnimationsTasks) -> Self { use self::SequentialTask::*; - UpdateAnimations(unsafe { SendElement::new(el) }, pseudo, tasks) + UpdateAnimations { el: unsafe { SendElement::new(el) }, + pseudo: pseudo, + before_change_style: before_change_style, + tasks: tasks } } } diff --git a/components/style/dom.rs b/components/style/dom.rs index f56194fb14b..ffc9f67ef8d 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -14,12 +14,15 @@ use data::ElementData; use element_state::ElementState; use font_metrics::FontMetricsProvider; use properties::{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::{ElementExt, PreExistingComputedValues, PseudoElement}; use selectors::matching::ElementSelectorFlags; use shared_lock::Locked; use sink::Push; use std::fmt; +#[cfg(feature = "gecko")] use std::collections::HashMap; use std::fmt::Debug; use std::hash::Hash; use std::ops::Deref; @@ -462,6 +465,7 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone + /// Creates a task to update various animation state on a given (pseudo-)element. #[cfg(feature = "gecko")] fn update_animations(&self, _pseudo: Option<&PseudoElement>, + before_change_style: Option>, tasks: UpdateAnimationsTasks); /// Returns true if the element has relevant animations. Relevant @@ -472,7 +476,8 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone + /// Returns true if the element has a CSS animation. fn has_css_animations(&self, _pseudo: Option<&PseudoElement>) -> bool; - /// Returns true if the element has a CSS transition. + /// Returns true if the element has a CSS transition (including running transitions and + /// completed transitions). fn has_css_transitions(&self, _pseudo: Option<&PseudoElement>) -> bool; /// Returns true if the element has animation restyle hints. @@ -484,6 +489,44 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone + return data.get_restyle() .map_or(false, |r| r.hint.has_animation_hint()); } + + /// Gets the current existing CSS transitions, by |property, end value| pairs in a HashMap. + #[cfg(feature = "gecko")] + fn get_css_transitions_info(&self, + pseudo: Option<&PseudoElement>) + -> HashMap>; + + /// Does a rough (and cheap) check for whether or not transitions might need to be updated that + /// will quickly return false for the common case of no transitions specified or running. If + /// this returns false, we definitely don't need to update transitions but if it returns true + /// we can perform the more thoroughgoing check, needs_transitions_update, to further + /// reduce the possibility of false positives. + #[cfg(feature = "gecko")] + fn might_need_transitions_update(&self, + old_values: &Option<&Arc>, + new_values: &Arc, + pseudo: Option<&PseudoElement>) -> bool; + + /// Returns true if one of the transitions needs to be updated on this element. We check all + /// the transition properties to make sure that updating transitions is necessary. + /// This method should only be called if might_needs_transitions_update returns true when + /// passed the same parameters. + #[cfg(feature = "gecko")] + fn needs_transitions_update(&self, + before_change_style: &Arc, + after_change_style: &Arc, + pseudo: Option<&PseudoElement>) -> bool; + + /// Returns true if we need to update transitions for the specified property on this element. + #[cfg(feature = "gecko")] + fn needs_transitions_update_per_property(&self, + property: TransitionProperty, + combined_duration: f32, + before_change_style: &Arc, + after_change_style: &Arc, + existing_transitions: &HashMap>) + -> bool; } /// Trait abstracting over different kinds of dirty-descendants bits. diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index d429c905b71..f64703dfd9c 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -57,7 +57,7 @@ use media_queries::Device; use parking_lot::RwLock; use properties::{ComputedValues, parse_style_attribute}; use properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock}; -use properties::animated_properties::AnimationValueMap; +use properties::animated_properties::{AnimationValue, AnimationValueMap, TransitionProperty}; use properties::style_structs::Font; use rule_tree::CascadeLevel as ServoCascadeLevel; use selector_parser::{ElementExt, Snapshot}; @@ -67,6 +67,7 @@ use selectors::parser::{AttrSelector, NamespaceConstraint}; use shared_lock::Locked; use sink::Push; use std::cell::RefCell; +use std::collections::HashMap; use std::fmt; use std::hash::{Hash, Hasher}; use std::ptr; @@ -665,7 +666,9 @@ impl<'le> TElement for GeckoElement<'le> { (self.flags() & node_flags) == node_flags } - fn update_animations(&self, pseudo: Option<&PseudoElement>, + fn update_animations(&self, + pseudo: Option<&PseudoElement>, + before_change_style: Option>, tasks: UpdateAnimationsTasks) { // We have to update animations even if the element has no computed style // since it means the element is in a display:none subtree, we should destroy @@ -692,9 +695,10 @@ impl<'le> TElement for GeckoElement<'le> { ); let atom_ptr = PseudoElement::ns_atom_or_null_from_opt(pseudo); + let before_change_values = before_change_style.as_ref().map(|v| *HasArcFFI::arc_as_borrowed(v)); unsafe { Gecko_UpdateAnimations(self.0, atom_ptr, - None, + before_change_values, computed_values_opt, parent_values_opt, tasks.bits()); @@ -715,6 +719,162 @@ impl<'le> TElement for GeckoElement<'le> { let atom_ptr = PseudoElement::ns_atom_or_null_from_opt(pseudo); unsafe { Gecko_ElementHasCSSTransitions(self.0, atom_ptr) } } + + fn get_css_transitions_info(&self, + pseudo: Option<&PseudoElement>) + -> HashMap> { + use gecko_bindings::bindings::Gecko_ElementTransitions_EndValueAt; + use gecko_bindings::bindings::Gecko_ElementTransitions_Length; + use gecko_bindings::bindings::Gecko_ElementTransitions_PropertyAt; + + let atom_ptr = PseudoElement::ns_atom_or_null_from_opt(pseudo); + let collection_length = unsafe { Gecko_ElementTransitions_Length(self.0, atom_ptr) }; + let mut map = HashMap::with_capacity(collection_length); + for i in 0..collection_length { + let (property, raw_end_value) = unsafe { + (Gecko_ElementTransitions_PropertyAt(self.0, atom_ptr, i as usize).into(), + Gecko_ElementTransitions_EndValueAt(self.0, atom_ptr, i as usize)) + }; + let end_value = AnimationValue::arc_from_borrowed(&raw_end_value); + debug_assert!(end_value.is_some()); + map.insert(property, end_value.unwrap().clone()); + } + map + } + + fn might_need_transitions_update(&self, + old_values: &Option<&Arc>, + new_values: &Arc, + pseudo: Option<&PseudoElement>) -> bool { + use properties::longhands::display::computed_value as display; + + if old_values.is_none() { + return false; + } + + let ref new_box_style = new_values.get_box(); + let transition_not_running = !self.has_css_transitions(pseudo) && + new_box_style.transition_property_count() == 1 && + new_box_style.transition_combined_duration_at(0) <= 0.0f32; + let new_display_style = new_box_style.clone_display(); + let old_display_style = old_values.map(|ref old| old.get_box().clone_display()).unwrap(); + + new_box_style.transition_property_count() > 0 && + !transition_not_running && + (new_display_style != display::T::none && + old_display_style != display::T::none) + } + + // Detect if there are any changes that require us to update transitions. This is used as a + // more thoroughgoing check than the, cheaper might_need_transitions_update check. + // The following logic shadows the logic used on the Gecko side + // (nsTransitionManager::DoUpdateTransitions) where we actually perform the update. + // https://drafts.csswg.org/css-transitions/#starting + fn needs_transitions_update(&self, + before_change_style: &Arc, + after_change_style: &Arc, + pseudo: Option<&PseudoElement>) -> bool { + use gecko_bindings::structs::nsCSSPropertyID; + use properties::animated_properties; + use std::collections::HashSet; + + debug_assert!(self.might_need_transitions_update(&Some(before_change_style), + after_change_style, + pseudo), + "We should only call needs_transitions_update if \ + might_need_transitions_update returns true"); + + let ref after_change_box_style = after_change_style.get_box(); + let transitions_count = after_change_box_style.transition_property_count(); + let existing_transitions = self.get_css_transitions_info(pseudo); + let mut transitions_to_keep = if !existing_transitions.is_empty() && + (after_change_box_style.transition_nscsspropertyid_at(0) != + nsCSSPropertyID::eCSSPropertyExtra_all_properties) { + Some(HashSet::::with_capacity(transitions_count)) + } else { + None + }; + + // Check if this property is none, custom or unknown. + let is_none_or_custom_property = |property: nsCSSPropertyID| -> bool { + return property == nsCSSPropertyID::eCSSPropertyExtra_no_properties || + property == nsCSSPropertyID::eCSSPropertyExtra_variable || + property == nsCSSPropertyID::eCSSProperty_UNKNOWN; + }; + + for i in 0..transitions_count { + let property = after_change_box_style.transition_nscsspropertyid_at(i); + let combined_duration = after_change_box_style.transition_combined_duration_at(i); + + // We don't need to update transition for none/custom properties. + if is_none_or_custom_property(property) { + continue; + } + + let mut property_check_helper = |property: TransitionProperty| -> bool { + if self.needs_transitions_update_per_property(property, + combined_duration, + before_change_style, + after_change_style, + &existing_transitions) { + return true; + } + + if let Some(set) = transitions_to_keep.as_mut() { + set.insert(property); + } + false + }; + // FIXME: Bug 1353628: Shorthand properties are parsed failed now, so after fixing + // that, we have to handle shorthand. + if property == nsCSSPropertyID::eCSSPropertyExtra_all_properties { + if TransitionProperty::any(property_check_helper) { + return true; + } + } else { + if animated_properties::nscsspropertyid_is_animatable(property) && + property_check_helper(property.into()) { + return true; + } + } + } + + // Check if we have to cancel the running transition because this is not a matching + // transition-property value. + transitions_to_keep.map_or(false, |set| { + existing_transitions.keys().any(|property| !set.contains(property)) + }) + } + + fn needs_transitions_update_per_property(&self, + property: TransitionProperty, + combined_duration: f32, + before_change_style: &Arc, + after_change_style: &Arc, + existing_transitions: &HashMap>) + -> bool { + use properties::animated_properties::AnimatedProperty; + + // We don't allow transitions on properties that are not interpolable. + if property.is_discrete() { + return false; + } + + if existing_transitions.contains_key(&property) { + // If there is an existing transition, update only if the end value differs. + // If the end value has not changed, we should leave the currently running + // transition as-is since we don't want to interrupt its timing function. + let after_value = + Arc::new(AnimationValue::from_computed_values(&property, after_change_style)); + return existing_transitions.get(&property).unwrap() != &after_value; + } + + combined_duration > 0.0f32 && + AnimatedProperty::from_transition_property(&property, + before_change_style, + after_change_style).does_animate() + } } impl<'le> PartialEq for GeckoElement<'le> { diff --git a/components/style/gecko_bindings/bindings.rs b/components/style/gecko_bindings/bindings.rs index bd7b434c5d8..2ea1f286db5 100644 --- a/components/style/gecko_bindings/bindings.rs +++ b/components/style/gecko_bindings/bindings.rs @@ -653,6 +653,25 @@ extern "C" { aPseudoTagOrNull: *mut nsIAtom) -> bool; } +extern "C" { + pub fn Gecko_ElementTransitions_Length(aElement: RawGeckoElementBorrowed, + aPseudoTagOrNull: *mut nsIAtom) + -> usize; +} +extern "C" { + pub fn Gecko_ElementTransitions_PropertyAt(aElement: + RawGeckoElementBorrowed, + aPseudoTagOrNull: *mut nsIAtom, + aIndex: usize) + -> nsCSSPropertyID; +} +extern "C" { + pub fn Gecko_ElementTransitions_EndValueAt(aElement: + RawGeckoElementBorrowed, + aPseudoTagOrNull: *mut nsIAtom, + aIndex: usize) + -> RawServoAnimationValueBorrowedOrNull; +} extern "C" { pub fn Gecko_GetProgressFromComputedTiming(aComputedTiming: RawGeckoComputedTimingBorrowed) diff --git a/components/style/matching.rs b/components/style/matching.rs index 88daa198d57..c986b5825c7 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -477,10 +477,14 @@ trait PrivateMatchMethods: TElement { // Handle animations. if animate && !context.shared.traversal_flags.for_animation_only() { + let pseudo_style = pseudo_style.as_ref().map(|&(ref pseudo, ref computed_style)| { + (*pseudo, &**computed_style) + }); self.process_animations(context, &mut old_values, &mut new_values, - pseudo); + primary_style, + &pseudo_style); } // Accumulate restyle damage. @@ -556,20 +560,63 @@ trait PrivateMatchMethods: TElement { context: &mut StyleContext, old_values: &mut Option>, new_values: &mut Arc, - pseudo: Option<&PseudoElement>) { - use context::{CSS_ANIMATIONS, EFFECT_PROPERTIES}; + primary_style: &ComputedStyle, + pseudo_style: &Option<(&PseudoElement, &ComputedStyle)>) { + use context::{CSS_ANIMATIONS, CSS_TRANSITIONS, EFFECT_PROPERTIES}; use context::UpdateAnimationsTasks; + let pseudo = pseudo_style.map(|(p, _)| p); let mut tasks = UpdateAnimationsTasks::empty(); if self.needs_update_animations(old_values, new_values, pseudo) { tasks.insert(CSS_ANIMATIONS); } + + let before_change_style = if self.might_need_transitions_update(&old_values.as_ref(), + new_values, + pseudo) { + let after_change_style = if self.has_css_transitions(pseudo) { + self.get_after_change_style(context, primary_style, pseudo_style) + } else { + None + }; + + // In order to avoid creating a SequentialTask for transitions which may not be updated, + // we check it per property to make sure Gecko side will really update transition. + let needs_transitions_update = { + // We borrow new_values here, so need to add a scope to make sure we release it + // before assigning a new value to it. + let after_change_style_ref = match after_change_style { + Some(ref value) => value, + None => &new_values + }; + self.needs_transitions_update(old_values.as_ref().unwrap(), + after_change_style_ref, + pseudo) + }; + + if needs_transitions_update { + if let Some(values_without_transitions) = after_change_style { + *new_values = values_without_transitions; + } + tasks.insert(CSS_TRANSITIONS); + + // We need to clone old_values into SequentialTask, so we can use it later. + old_values.clone() + } else { + None + } + } else { + None + }; + if self.has_animations(pseudo) { tasks.insert(EFFECT_PROPERTIES); } + if !tasks.is_empty() { let task = ::context::SequentialTask::update_animations(*self, pseudo.cloned(), + before_change_style, tasks); context.thread_local.tasks.push(task); } @@ -580,7 +627,8 @@ trait PrivateMatchMethods: TElement { context: &mut StyleContext, old_values: &mut Option>, new_values: &mut Arc, - _pseudo: Option<&PseudoElement>) { + _primary_style: &ComputedStyle, + _pseudo_style: &Option<(&PseudoElement, &ComputedStyle)>) { let possibly_expired_animations = &mut context.thread_local.current_element_info.as_mut().unwrap() .possibly_expired_animations; diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 02e3ace6f44..bbf0dec5682 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -44,6 +44,7 @@ use gecko_bindings::bindings::ServoComputedValuesBorrowedOrNull; use gecko_bindings::bindings::{Gecko_ResetFilters, Gecko_CopyFiltersFrom}; use gecko_bindings::bindings::RawGeckoPresContextBorrowed; use gecko_bindings::structs::{self, StyleComplexColor}; +use gecko_bindings::structs::nsCSSPropertyID; use gecko_bindings::structs::nsStyleVariables; use gecko_bindings::sugar::ns_style_coord::{CoordDataValue, CoordData, CoordDataMut}; use gecko_bindings::sugar::ownership::HasArcFFI; @@ -1985,6 +1986,11 @@ fn static_assert() { ${impl_transition_time_value('duration', 'Duration')} ${impl_transition_timing_function()} + pub fn transition_combined_duration_at(&self, index: usize) -> f32 { + // https://drafts.csswg.org/css-transitions/#transition-combined-duration + self.gecko.mTransitions[index].mDuration.max(0.0) + self.gecko.mTransitions[index].mDelay + } + pub fn set_transition_property(&mut self, v: longhands::transition_property::computed_value::T) { use gecko_bindings::structs::nsCSSPropertyID::eCSSPropertyExtra_no_properties; @@ -2018,6 +2024,10 @@ fn static_assert() { self.gecko.mTransitions[index].mProperty.into() } + pub fn transition_nscsspropertyid_at(&self, index: usize) -> nsCSSPropertyID { + self.gecko.mTransitions[index].mProperty + } + pub fn copy_transition_property_from(&mut self, other: &Self) { unsafe { self.gecko.mTransitions.ensure_len(other.gecko.mTransitions.len()) }; From 5a1582ca4d2c00a377382f22ca09939327d90c4f Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Fri, 14 Apr 2017 10:44:38 +0800 Subject: [PATCH 7/7] Bug 1341372 - Part 7: Rename needs_update_animations. MozReview-Commit-ID: 2bJlBbdX543 --- components/style/matching.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/style/matching.rs b/components/style/matching.rs index c986b5825c7..90a92a4ea5e 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -531,7 +531,7 @@ trait PrivateMatchMethods: TElement { } #[cfg(feature = "gecko")] - fn needs_update_animations(&self, + fn needs_animations_update(&self, old_values: &Option>, new_values: &Arc, pseudo: Option<&PseudoElement>) -> bool { @@ -567,7 +567,7 @@ trait PrivateMatchMethods: TElement { let pseudo = pseudo_style.map(|(p, _)| p); let mut tasks = UpdateAnimationsTasks::empty(); - if self.needs_update_animations(old_values, new_values, pseudo) { + if self.needs_animations_update(old_values, new_values, pseudo) { tasks.insert(CSS_ANIMATIONS); }