Use AnimatableLonghand for AnimationValueMap and related code

In the next few patches we move all non-transition related code over to
using AnimatableLonghand instead of TransitionProperty. This will allow
us to re-purpose TransitionProperty to represent only properties that
can be transitioned (i.e. excluding discrete properties) as well as
simplifying the code by removing the need to deal with shorthands and
the "all" value in places that do not need to handle those values.

components/style/animation.rs

@@ -12,7 +12,7 @@ use dom::OpaqueNode;
 use euclid::Point2D;
 use font_metrics::FontMetricsProvider;
 use properties::{self, CascadeFlags, ComputedValues, Importance};
-use properties::animated_properties::{AnimatedProperty, TransitionProperty};
+use properties::animated_properties::{AnimatableLonghand, AnimatedProperty, TransitionProperty};
 use properties::longhands::animation_direction::computed_value::single_value::T as AnimationDirection;
 use properties::longhands::animation_iteration_count::single_value::computed_value::T as AnimationIterationCount;
 use properties::longhands::animation_play_state::computed_value::single_value::T as AnimationPlayState;
@@ -330,7 +330,27 @@ impl PropertyAnimation {
                                 -> Option<PropertyAnimation> {
         debug_assert!(!transition_property.is_shorthand() &&
                       transition_property != &TransitionProperty::All);
-        let animated_property = AnimatedProperty::from_transition_property(transition_property,
+
+        // We're not expecting |transition_property| to be a shorthand (including 'all') and
+        // all other transitionable properties should be animatable longhands (since transitionable
+        // is a subset of animatable).
+        let animatable_longhand =
+            AnimatableLonghand::from_transition_property(transition_property).unwrap();
+
+        PropertyAnimation::from_animatable_longhand(&animatable_longhand,
+                                                    timing_function,
+                                                    duration,
+                                                    old_style,
+                                                    new_style)
+    }
+
+    fn from_animatable_longhand(animatable_longhand: &AnimatableLonghand,
+                                timing_function: TimingFunction,
+                                duration: Time,
+                                old_style: &ComputedValues,
+                                new_style: &ComputedValues)
+                                -> Option<PropertyAnimation> {
+        let animated_property = AnimatedProperty::from_animatable_longhand(animatable_longhand,
                                                                            old_style,
                                                                            new_style);
 
@@ -743,22 +763,22 @@ pub fn update_style_for_animation(context: &SharedStyleContext,
 
             let mut new_style = (*style).clone();
 
-            for transition_property in &animation.properties_changed {
+            for property in &animation.properties_changed {
                 debug!("update_style_for_animation: scanning prop {:?} for animation \"{}\"",
-                       transition_property, name);
-                match PropertyAnimation::from_transition_property(transition_property,
+                       property, name);
+                match PropertyAnimation::from_animatable_longhand(property,
                                                                   timing_function,
                                                                   Time::from_seconds(relative_duration as f32),
                                                                   &from_style,
                                                                   &target_style) {
                     Some(property_animation) => {
-                        debug!("update_style_for_animation: got property animation for prop {:?}", transition_property);
+                        debug!("update_style_for_animation: got property animation for prop {:?}", property);
                         debug!("update_style_for_animation: {:?}", property_animation);
                         property_animation.update(Arc::make_mut(&mut new_style), relative_progress);
                     }
                     None => {
                         debug!("update_style_for_animation: property animation {:?} not animating",
-                               transition_property);
+                               property);
                     }
                 }
             }

components/style/gecko/wrapper.rs

@@ -68,7 +68,8 @@ use logical_geometry::WritingMode;
 use media_queries::Device;
 use properties::{ComputedValues, parse_style_attribute};
 use properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock};
-use properties::animated_properties::{AnimationValue, AnimationValueMap, TransitionProperty};
+use properties::animated_properties::{AnimatableLonghand, AnimationValue, AnimationValueMap};
+use properties::animated_properties::TransitionProperty;
 use properties::style_structs::Font;
 use rule_tree::CascadeLevel as ServoCascadeLevel;
 use selector_parser::{AttrValue, ElementExt, PseudoClassStringArg};
@@ -1143,8 +1144,11 @@ impl<'le> TElement for GeckoElement<'le> {
             return existing_transitions.get(property).unwrap() != &after_value;
         }
 
+        // |property| should be an animatable longhand
+        let animatable_longhand = AnimatableLonghand::from_transition_property(property).unwrap();
+
         combined_duration > 0.0f32 &&
-        AnimatedProperty::from_transition_property(property,
+        AnimatedProperty::from_animatable_longhand(&animatable_longhand,
                                                    before_change_style,
                                                    after_change_style).does_animate()
     }

components/style/properties/declaration_block.rs

@@ -126,7 +126,7 @@ impl<'a, 'cx, 'cx_a:'cx> AnimationValueIterator<'a, 'cx, 'cx_a> {
 }
 
 impl<'a, 'cx, 'cx_a:'cx> Iterator for AnimationValueIterator<'a, 'cx, 'cx_a> {
-    type Item = (TransitionProperty, AnimationValue);
+    type Item = (AnimatableLonghand, AnimationValue);
     #[inline]
     fn next(&mut self) -> Option<Self::Item> {
         use properties::Importance;
@@ -136,11 +136,11 @@ impl<'a, 'cx, 'cx_a:'cx> Iterator for AnimationValueIterator<'a, 'cx, 'cx_a> {
             match next {
                 Some(&(ref decl, importance)) => {
                     if importance == Importance::Normal {
-                        let property = TransitionProperty::from_declaration(decl);
+                        let property = AnimatableLonghand::from_declaration(decl);
                         let animation = AnimationValue::from_declaration(decl, &mut self.context,
                                                                          self.default_values);
                         debug_assert!(property.is_none() == animation.is_none(),
-                                      "The failure condition of TransitionProperty::from_declaration \
+                                      "The failure condition of AnimatableLonghand::from_declaration \
                                        and AnimationValue::from_declaration should be the same");
                         // Skip the property if either ::from_declaration fails.
                         match (property, animation) {
@@ -201,7 +201,7 @@ impl PropertyDeclarationBlock {
         }
     }
 
-    /// Return an iterator of (TransitionProperty, AnimationValue).
+    /// Return an iterator of (AnimatableLonghand, AnimationValue).
     pub fn to_animation_value_iter<'a, 'cx, 'cx_a:'cx>(&'a self,
                                                        context: &'cx mut Context<'cx_a>,
                                                        default_values: &'a Arc<ComputedValues>)
@@ -554,7 +554,7 @@ impl PropertyDeclarationBlock {
         let mut longhands = LonghandIdSet::new();
 
         for (property, animation_value) in animation_value_map.iter() {
-          longhands.set_transition_property_bit(property);
+          longhands.set_animatable_longhand_bit(property);
           declarations.push((animation_value.uncompute(), Importance::Normal));
         }
 

components/style/properties/helpers/animated_properties.mako.rs

@@ -51,6 +51,8 @@ use values::generics::position as generic_position;
 /// NOTE: This includes the 'display' property since it is animatable from SMIL even though it is
 /// not animatable from CSS animations or Web Animations. CSS transitions also does not allow
 /// animating 'display', but for CSS transitions we have the separate TransitionProperty type.
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
+#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub enum AnimatableLonghand {
     % for prop in data.longhands:
         % if prop.animatable:
@@ -60,6 +62,101 @@ pub enum AnimatableLonghand {
     % endfor
 }
 
+impl AnimatableLonghand {
+    /// 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)}
+                        => Some(AnimatableLonghand::${prop.camel_case}),
+                % endif
+            % endfor
+            _ => None
+        }
+    }
+
+    /// Converts from TransitionProperty. Returns None if the property is not an animatable
+    /// longhand.
+    pub fn from_transition_property(transition_property: &TransitionProperty) -> Option<Self> {
+        match *transition_property {
+            % for prop in data.longhands:
+                <%
+                    # TODO: Later in this patch series, once we introduce the 'transitionable'
+                    # definition, we will need to make the below test:
+                    #
+                    #    if prop.transitionable and prop.animatable:
+                %>
+                % if prop.animatable:
+                    TransitionProperty::${prop.camel_case}
+                        => Some(AnimatableLonghand::${prop.camel_case}),
+                % endif
+            % endfor
+            _ => None
+        }
+    }
+
+    /// Get an animatable longhand property from a property declaration.
+    pub fn from_declaration(declaration: &PropertyDeclaration) -> Option<Self> {
+        use properties::LonghandId;
+        match *declaration {
+            % for prop in data.longhands:
+                % if prop.animatable:
+                    PropertyDeclaration::${prop.camel_case}(..)
+                        => Some(AnimatableLonghand::${prop.camel_case}),
+                % endif
+            % endfor
+            PropertyDeclaration::CSSWideKeyword(id, _) |
+            PropertyDeclaration::WithVariables(id, _) => {
+                match id {
+                    % for prop in data.longhands:
+                        % if prop.animatable:
+                            LonghandId::${prop.camel_case} =>
+                                Some(AnimatableLonghand::${prop.camel_case}),
+                        % endif
+                    % endfor
+                    _ => None,
+                }
+            },
+            _ => None,
+        }
+    }
+}
+
+/// Convert to nsCSSPropertyID.
+#[cfg(feature = "gecko")]
+#[allow(non_upper_case_globals)]
+impl<'a> From< &'a AnimatableLonghand> for nsCSSPropertyID {
+    fn from(property: &'a AnimatableLonghand) -> nsCSSPropertyID {
+        match *property {
+            % for prop in data.longhands:
+                % if prop.animatable:
+                    AnimatableLonghand::${prop.camel_case}
+                        => ${helpers.to_nscsspropertyid(prop.ident)},
+                % endif
+            % endfor
+        }
+    }
+}
+
+/// Convert to PropertyDeclarationId.
+#[cfg(feature = "gecko")]
+#[allow(non_upper_case_globals)]
+impl<'a> From<AnimatableLonghand> for PropertyDeclarationId<'a> {
+    fn from(property: AnimatableLonghand) -> PropertyDeclarationId<'a> {
+        match property {
+            % for prop in data.longhands:
+                % if prop.animatable:
+                    AnimatableLonghand::${prop.camel_case}
+                        => PropertyDeclarationId::Longhand(LonghandId::${prop.camel_case}),
+                % endif
+            % endfor
+        }
+    }
+}
+
 /// A given transition property, that is either `All`, an animatable longhand property,
 /// a shorthand with at least one animatable longhand component, or an unsupported property.
 // NB: This needs to be here because it needs all the longhands generated
@@ -127,32 +224,6 @@ impl TransitionProperty {
         }).map_err(|()| SelectorParseError::UnexpectedIdent(ident.into()).into())
     }
 
-    /// Get a transition property from a property declaration.
-    pub fn from_declaration(declaration: &PropertyDeclaration) -> Option<Self> {
-        use properties::LonghandId;
-        match *declaration {
-            % for prop in data.longhands:
-                % if prop.animatable:
-                    PropertyDeclaration::${prop.camel_case}(..)
-                        => Some(TransitionProperty::${prop.camel_case}),
-                % endif
-            % endfor
-            PropertyDeclaration::CSSWideKeyword(id, _) |
-            PropertyDeclaration::WithVariables(id, _) => {
-                match id {
-                    % for prop in data.longhands:
-                        % if prop.animatable:
-                            LonghandId::${prop.camel_case} =>
-                                Some(TransitionProperty::${prop.camel_case}),
-                        % endif
-                    % endfor
-                    _ => None,
-                }
-            },
-            _ => None,
-        }
-    }
-
     /// 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 {
@@ -274,23 +345,6 @@ impl From<nsCSSPropertyID> for TransitionProperty {
     }
 }
 
-/// Convert to PropertyDeclarationId.
-#[cfg(feature = "gecko")]
-#[allow(non_upper_case_globals)]
-impl<'a> From<TransitionProperty> for PropertyDeclarationId<'a> {
-    fn from(transition_property: TransitionProperty) -> PropertyDeclarationId<'a> {
-        match transition_property {
-            % for prop in data.longhands:
-                % if prop.animatable:
-                    TransitionProperty::${prop.camel_case}
-                        => PropertyDeclarationId::Longhand(LonghandId::${prop.camel_case}),
-                % endif
-            % endfor
-            _ => panic!(),
-        }
-    }
-}
-
 /// An animated property interpolation between two computed values for that
 /// property.
 #[derive(Clone, Debug, PartialEq)]
@@ -377,22 +431,20 @@ impl AnimatedProperty {
 
     /// Get an animatable value from a transition-property, an old style, and a
     /// new style.
-    pub fn from_transition_property(transition_property: &TransitionProperty,
+    pub fn from_animatable_longhand(property: &AnimatableLonghand,
                                     old_style: &ComputedValues,
                                     new_style: &ComputedValues)
                                     -> AnimatedProperty {
-        match *transition_property {
-            TransitionProperty::All => panic!("Can't use TransitionProperty::All here."),
+        match *property {
             % for prop in data.longhands:
                 % if prop.animatable:
-                    TransitionProperty::${prop.camel_case} => {
+                    AnimatableLonghand::${prop.camel_case} => {
                         AnimatedProperty::${prop.camel_case}(
                             old_style.get_${prop.style_struct.ident.strip("_")}().clone_${prop.ident}().into(),
                             new_style.get_${prop.style_struct.ident.strip("_")}().clone_${prop.ident}().into())
                     }
                 % endif
             % endfor
-            ref other => panic!("Can't use TransitionProperty::{:?} here", other),
         }
     }
 }
@@ -401,7 +453,7 @@ impl AnimatedProperty {
 /// This HashMap stores the values that are the last AnimationValue to be
 /// composed for each TransitionProperty.
 #[cfg(feature = "gecko")]
-pub type AnimationValueMap = HashMap<TransitionProperty, AnimationValue>;
+pub type AnimationValueMap = HashMap<AnimatableLonghand, AnimationValue>;
 #[cfg(feature = "gecko")]
 unsafe impl HasFFI for AnimationValueMap {
     type FFIType = RawServoAnimationValueMap;

components/style/properties/properties.mako.rs

@@ -32,7 +32,7 @@ use font_metrics::FontMetricsProvider;
 use logical_geometry::WritingMode;
 use media_queries::Device;
 use parser::{Parse, ParserContext};
-use properties::animated_properties::TransitionProperty;
+use properties::animated_properties::AnimatableLonghand;
 #[cfg(feature = "gecko")] use properties::longhands::system_font::SystemFont;
 use selectors::parser::SelectorParseError;
 #[cfg(feature = "servo")] use servo_config::prefs::PREFS;
@@ -300,29 +300,25 @@ impl LonghandIdSet {
         }
     }
 
-    /// Set the corresponding bit of TransitionProperty.
-    /// This function will panic if TransitionProperty::All is given.
-    pub fn set_transition_property_bit(&mut self, property: &TransitionProperty) {
+    /// Set the corresponding bit of AnimatableLonghand.
+    pub fn set_animatable_longhand_bit(&mut self, property: &AnimatableLonghand) {
         match *property {
             % for prop in data.longhands:
                 % if prop.animatable:
-                    TransitionProperty::${prop.camel_case} => self.insert(LonghandId::${prop.camel_case}),
+                    AnimatableLonghand::${prop.camel_case} => self.insert(LonghandId::${prop.camel_case}),
                 % endif
             % endfor
-            ref other => unreachable!("Tried to set TransitionProperty::{:?} in a PropertyBitfield", other),
         }
     }
 
-    /// Return true if the corresponding bit of TransitionProperty is set.
-    /// This function will panic if TransitionProperty::All is given.
-    pub fn has_transition_property_bit(&self, property: &TransitionProperty) -> bool {
+    /// Return true if the corresponding bit of AnimatableLonghand is set.
+    pub fn has_animatable_longhand_bit(&self, property: &AnimatableLonghand) -> bool {
         match *property {
             % for prop in data.longhands:
                 % if prop.animatable:
-                    TransitionProperty::${prop.camel_case} => self.contains(LonghandId::${prop.camel_case}),
+                    AnimatableLonghand::${prop.camel_case} => self.contains(LonghandId::${prop.camel_case}),
                 % endif
             % endfor
-            ref other => unreachable!("Tried to get TransitionProperty::{:?} in a PropertyBitfield", other),
         }
     }
 }