Bug 1371518 - Move is_discrete from TransitionProperty to AnimatableLonghand; r?hiro draft
authorBrian Birtles <birtles@gmail.com>
Wed, 14 Jun 2017 14:50:17 +0900
changeset 593856 88bb0120ce4ffeacda44ccf22903aa92943774b0
parent 593855 8d33d231ff0739beec28618e17402573b3b67221
child 593857 273a82324c9aa35eb3efa84bc6ae53c7fb330a84
push id63840
push userbbirtles@mozilla.com
push dateWed, 14 Jun 2017 07:29:38 +0000
reviewershiro
bugs1371518
milestone56.0a1
Bug 1371518 - Move is_discrete from TransitionProperty to AnimatableLonghand; r?hiro MozReview-Commit-ID: Khxw0st9rxk
servo/components/style/gecko/wrapper.rs
servo/components/style/properties/helpers/animated_properties.mako.rs
servo/ports/geckolib/glue.rs
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -1120,24 +1120,24 @@ impl<'le> TElement for GeckoElement<'le>
                                              combined_duration: f32,
                                              before_change_style: &ComputedValues,
                                              after_change_style: &ComputedValues,
                                              existing_transitions: &HashMap<TransitionProperty,
                                                                             Arc<AnimationValue>>)
                                              -> bool {
         use properties::animated_properties::AnimatedProperty;
 
+        // |property| should be an animatable longhand
+        let animatable_longhand = AnimatableLonghand::from_transition_property(property).unwrap();
+
         // We don't allow transitions on properties that are not interpolable.
-        if property.is_discrete() {
+        if animatable_longhand.is_discrete() {
             return false;
         }
 
-        // |property| should be an animatable longhand
-        let animatable_longhand = AnimatableLonghand::from_transition_property(property).unwrap();
-
         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(&animatable_longhand,
                                                               after_change_style));
             return existing_transitions.get(property).unwrap() != &after_value;
--- a/servo/components/style/properties/helpers/animated_properties.mako.rs
+++ b/servo/components/style/properties/helpers/animated_properties.mako.rs
@@ -58,16 +58,28 @@ pub enum AnimatableLonghand {
         % if prop.animatable:
             /// ${prop.name}
             ${prop.camel_case},
         % endif
     % endfor
 }
 
 impl AnimatableLonghand {
+    /// Returns true if this AnimatableLonghand is one of the discretely animatable properties.
+    pub fn is_discrete(&self) -> bool {
+        match *self {
+            % for prop in data.longhands:
+                % if prop.animation_value_type == "discrete":
+                    AnimatableLonghand::${prop.camel_case} => true,
+                % endif
+            % endfor
+            _ => false
+        }
+    }
+
     /// Converts from an nsCSSPropertyID. Returns None if nsCSSPropertyID is not an animatable
     /// longhand in Servo.
     #[cfg(feature = "gecko")]
     pub fn from_nscsspropertyid(css_property: nsCSSPropertyID) -> Option<Self> {
         match css_property {
             % for prop in data.longhands:
                 % if prop.animatable:
                     ${helpers.to_nscsspropertyid(prop.ident)}
@@ -219,29 +231,16 @@ impl TransitionProperty {
                 match CSSWideKeyword::from_ident(&ident) {
                     Some(_) => Err(()),
                     None => Ok(TransitionProperty::Unsupported((&*ident).into()))
                 }
             }
         }).map_err(|()| SelectorParseError::UnexpectedIdent(ident.into()).into())
     }
 
-    /// Returns true if this TransitionProperty is one of the discrete animatable properties and
-    /// this TransitionProperty should be a longhand property.
-    pub fn is_discrete(&self) -> bool {
-        match *self {
-            % for prop in data.longhands:
-                % if prop.animation_value_type == "discrete":
-                    TransitionProperty::${prop.camel_case} => true,
-                % endif
-            % endfor
-            _ => false
-        }
-    }
-
     /// Return animatable longhands of this shorthand TransitionProperty, except for "all".
     pub fn longhands(&self) -> &'static [TransitionProperty] {
         % for prop in data.shorthands_except_all():
             % if prop.animatable:
                 static ${prop.ident.upper()}: &'static [TransitionProperty] = &[
                     % for sub in prop.sub_properties:
                         % if sub.animatable:
                             TransitionProperty::${sub.camel_case},
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -91,17 +91,16 @@ use style::gecko_properties::{self, styl
 use style::invalidation::element::restyle_hints::{self, RestyleHint};
 use style::media_queries::{MediaList, parse_media_query_list};
 use style::parallel;
 use style::parser::{PARSING_MODE_DEFAULT, ParserContext};
 use style::properties::{CascadeFlags, ComputedValues, Importance, SourcePropertyDeclaration};
 use style::properties::{LonghandIdSet, PropertyDeclaration, PropertyDeclarationBlock, PropertyId, StyleBuilder};
 use style::properties::SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP;
 use style::properties::animated_properties::{Animatable, AnimatableLonghand, AnimationValue};
-use style::properties::animated_properties::TransitionProperty;
 use style::properties::parse_one_declaration_into;
 use style::rule_tree::StyleSource;
 use style::selector_parser::PseudoElementCascadeType;
 use style::sequential;
 use style::shared_lock::{SharedRwLock, SharedRwLockReadGuard, StylesheetGuards, ToCssWithGuard, Locked};
 use style::string_cache::Atom;
 use style::style_adjuster::StyleAdjuster;
 use style::stylearc::Arc;
@@ -700,18 +699,20 @@ pub extern "C" fn Servo_ComputedValues_E
 #[no_mangle]
 pub extern "C" fn Servo_Property_IsAnimatable(property: nsCSSPropertyID) -> bool {
     use style::properties::animated_properties;
     animated_properties::nscsspropertyid_is_animatable(property)
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_Property_IsDiscreteAnimatable(property: nsCSSPropertyID) -> bool {
-    let property: TransitionProperty = property.into();
-    property.is_discrete()
+    match AnimatableLonghand::from_nscsspropertyid(property) {
+        Some(longhand) => longhand.is_discrete(),
+        None => false
+    }
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_StyleWorkerThreadCount() -> u32 {
     GLOBAL_STYLE_DATA.num_threads as u32
 }
 
 #[no_mangle]