Bug 1631154 - Add an iterator for transition properties. r=emilio

This simplifies the code a bit and also will allow us to more easily
make improvements to servo's animation implementation in the future.

servo/components/style/animation.rs

@@ -12,14 +12,13 @@ use crate::bezier::Bezier;
 use crate::context::SharedStyleContext;
 use crate::dom::{OpaqueNode, TElement};
 use crate::font_metrics::FontMetricsProvider;
-use crate::properties::animated_properties::AnimatedProperty;
+use crate::properties::animated_properties::{AnimatedProperty, TransitionPropertyIteration};
 use crate::properties::longhands::animation_direction::computed_value::single_value::T as AnimationDirection;
 use crate::properties::longhands::animation_play_state::computed_value::single_value::T as AnimationPlayState;
 use crate::properties::{self, CascadeMode, ComputedValues, LonghandId};
 use crate::stylesheets::keyframes_rule::{KeyframesAnimation, KeyframesStep, KeyframesStepValue};
 use crate::stylesheets::Origin;
 use crate::timer::Timer;
-use crate::values::computed::box_::TransitionProperty;
 use crate::values::computed::Time;
 use crate::values::computed::TimingFunction;
 use crate::values::generics::box_::AnimationIterationCount;
@@ -271,52 +270,6 @@ impl PropertyAnimation {
         self.property.name()
     }
 
-    /// Creates a new property animation for the given transition index and old
-    /// and new styles.  Any number of animations may be returned, from zero (if
-    /// the property did not animate) to one (for a single transition property)
-    /// to arbitrarily many (for `all`).
-    pub fn from_transition(
-        transition_index: usize,
-        old_style: &ComputedValues,
-        new_style: &mut ComputedValues,
-    ) -> Vec<PropertyAnimation> {
-        let mut result = vec![];
-        let box_style = new_style.get_box();
-        let transition_property = box_style.transition_property_at(transition_index);
-        let timing_function = box_style.transition_timing_function_mod(transition_index);
-        let duration = box_style.transition_duration_mod(transition_index);
-
-        match transition_property {
-            TransitionProperty::Custom(..) | TransitionProperty::Unsupported(..) => result,
-            TransitionProperty::Shorthand(ref shorthand_id) => shorthand_id
-                .longhands()
-                .filter_map(|longhand| {
-                    PropertyAnimation::from_longhand(
-                        longhand,
-                        timing_function,
-                        duration,
-                        old_style,
-                        new_style,
-                    )
-                })
-                .collect(),
-            TransitionProperty::Longhand(longhand_id) => {
-                let animation = PropertyAnimation::from_longhand(
-                    longhand_id,
-                    timing_function,
-                    duration,
-                    old_style,
-                    new_style,
-                );
-
-                if let Some(animation) = animation {
-                    result.push(animation);
-                }
-                result
-            },
-        }
-    }
-
     fn from_longhand(
         longhand: LonghandId,
         timing_function: TimingFunction,
@@ -414,56 +367,70 @@ pub fn start_transitions_if_applicable(
     running_and_expired_transitions: &[PropertyAnimation],
 ) -> bool {
     let mut had_animations = false;
-    for i in 0..new_style.get_box().transition_property_count() {
+    let transitions: Vec<TransitionPropertyIteration> = new_style.transition_properties().collect();
-        // Create any property animations, if applicable.
+    for transition in &transitions {
-        let property_animations =
+        let property_animation = match PropertyAnimation::from_longhand(
-            PropertyAnimation::from_transition(i, old_style, Arc::make_mut(new_style));
+            transition.longhand_id,
-        for property_animation in property_animations {
+            new_style
-            // Set the property to the initial value.
+                .get_box()
-            //
+                .transition_timing_function_mod(transition.index),
-            // NB: get_mut is guaranteed to succeed since we called make_mut()
+            new_style
-            // above.
+                .get_box()
-            property_animation.update(Arc::get_mut(new_style).unwrap(), 0.0);
+                .transition_duration_mod(transition.index),
+            old_style,
+            Arc::make_mut(new_style),
+        ) {
+            Some(property_animation) => property_animation,
+            None => continue,
+        };
 
-            // Per [1], don't trigger a new transition if the end state for that
+        // Set the property to the initial value.
-            // transition is the same as that of a transition that's already
+        //
-            // running on the same node.
+        // NB: get_mut is guaranteed to succeed since we called make_mut()
-            //
+        // above.
-            // [1]: https://drafts.csswg.org/css-transitions/#starting
+        property_animation.update(Arc::get_mut(new_style).unwrap(), 0.0);
+
+        // Per [1], don't trigger a new transition if the end state for that
+        // transition is the same as that of a transition that's already
+        // running on the same node.
+        //
+        // [1]: https://drafts.csswg.org/css-transitions/#starting
+        debug!(
+            "checking {:?} for matching end value",
+            running_and_expired_transitions
+        );
+        if running_and_expired_transitions
+            .iter()
+            .any(|animation| animation.has_the_same_end_value_as(&property_animation))
+        {
             debug!(
-                "checking {:?} for matching end value",
+                "Not initiating transition for {}, other transition \
-                running_and_expired_transitions
+                 found with the same end value",
+                property_animation.property_name()
             );
-            if running_and_expired_transitions
+            continue;
-                .iter()
-                .any(|animation| animation.has_the_same_end_value_as(&property_animation))
-            {
-                debug!(
-                    "Not initiating transition for {}, other transition \
-                     found with the same end value",
-                    property_animation.property_name()
-                );
-                continue;
-            }
-
-            // Kick off the animation.
-            debug!("Kicking off transition of {:?}", property_animation);
-            let box_style = new_style.get_box();
-            let now = timer.seconds();
-            let start_time = now + (box_style.transition_delay_mod(i).seconds() as f64);
-            new_animations_sender
-                .send(Animation::Transition(
-                    opaque_node,
-                    start_time,
-                    AnimationFrame {
-                        duration: box_style.transition_duration_mod(i).seconds() as f64,
-                        property_animation,
-                    },
-                ))
-                .unwrap();
-
-            had_animations = true;
         }
+
+        // Kick off the animation.
+        debug!("Kicking off transition of {:?}", property_animation);
+        let box_style = new_style.get_box();
+        let now = timer.seconds();
+        let start_time = now + (box_style.transition_delay_mod(transition.index).seconds() as f64);
+        let duration = box_style
+            .transition_duration_mod(transition.index)
+            .seconds() as f64;
+        new_animations_sender
+            .send(Animation::Transition(
+                opaque_node,
+                start_time,
+                AnimationFrame {
+                    duration,
+                    property_animation,
+                },
+            ))
+            .unwrap();
+
+        had_animations = true;
     }
 
     had_animations

servo/components/style/gecko/wrapper.rs

@@ -1560,9 +1560,7 @@ impl<'le> TElement for GeckoElement<'le> {
         before_change_style: &ComputedValues,
         after_change_style: &ComputedValues,
     ) -> bool {
-        use crate::gecko_bindings::structs::nsCSSPropertyID;
         use crate::properties::LonghandIdSet;
-        use crate::values::computed::TransitionProperty;
 
         debug_assert!(
             self.might_need_transitions_update(Some(before_change_style), after_change_style),
@@ -1571,53 +1569,21 @@ impl<'le> TElement for GeckoElement<'le> {
         );
 
         let after_change_box_style = after_change_style.get_box();
-        let transitions_count = after_change_box_style.transition_property_count();
         let existing_transitions = self.css_transitions_info();
-
-        // 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;
-        };
-
         let mut transitions_to_keep = LonghandIdSet::new();
-
+        for transition_property in after_change_style.transition_properties() {
-        for i in 0..transitions_count {
+            let physical_longhand = transition_property
-            let property = after_change_box_style.transition_nscsspropertyid_at(i);
+                .longhand_id
-            let combined_duration = after_change_box_style.transition_combined_duration_at(i);
+                .to_physical(after_change_style.writing_mode);
-
+            transitions_to_keep.insert(physical_longhand);
-            // We don't need to update transition for none/custom properties.
+            if self.needs_transitions_update_per_property(
-            if is_none_or_custom_property(property) {
+                physical_longhand,
-                continue;
+                after_change_box_style.transition_combined_duration_at(transition_property.index),
-            }
+                before_change_style,
-
+                after_change_style,
-            let transition_property: TransitionProperty = property.into();
+                &existing_transitions,
-
+            ) {
-            let mut property_check_helper = |property: LonghandId| -> bool {
+                return true;
-                let property = property.to_physical(after_change_style.writing_mode);
-                transitions_to_keep.insert(property);
-                self.needs_transitions_update_per_property(
-                    property,
-                    combined_duration,
-                    before_change_style,
-                    after_change_style,
-                    &existing_transitions,
-                )
-            };
-
-            match transition_property {
-                TransitionProperty::Custom(..) | TransitionProperty::Unsupported(..) => {},
-                TransitionProperty::Shorthand(ref shorthand) => {
-                    if shorthand.longhands().any(property_check_helper) {
-                        return true;
-                    }
-                },
-                TransitionProperty::Longhand(longhand_id) => {
-                    if property_check_helper(longhand_id) {
-                        return true;
-                    }
-                },
             }
         }
 

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

@@ -11,7 +11,7 @@
 
 #[cfg(feature = "gecko")] use crate::gecko_bindings::structs::nsCSSPropertyID;
 use itertools::{EitherOrBoth, Itertools};
-use crate::properties::{CSSWideKeyword, PropertyDeclaration};
+use crate::properties::{CSSWideKeyword, PropertyDeclaration, NonCustomPropertyIterator};
 use crate::properties::longhands;
 use crate::properties::longhands::visibility::computed_value::T as Visibility;
 use crate::properties::LonghandId;
@@ -883,3 +883,66 @@ impl ToAnimatedZero for AnimatedFilter {
         }
     }
 }
+
+/// An iterator over all the properties that transition on a given style.
+pub struct TransitionPropertyIterator<'a> {
+    style: &'a ComputedValues,
+    index_range: core::ops::Range<usize>,
+    longhand_iterator: Option<NonCustomPropertyIterator<LonghandId>>,
+}
+
+impl<'a> TransitionPropertyIterator<'a> {
+    /// Create a `TransitionPropertyIterator` for the given style.
+    pub fn from_style(style: &'a ComputedValues) -> Self {
+        Self {
+            style,
+            index_range: 0..style.get_box().transition_property_count(),
+            longhand_iterator: None,
+        }
+    }
+}
+
+/// A single iteration of the TransitionPropertyIterator.
+pub struct TransitionPropertyIteration {
+    /// The id of the longhand for this property.
+    pub longhand_id: LonghandId,
+
+    /// The index of this property in the list of transition properties for this
+    /// iterator's style.
+    pub index: usize,
+}
+
+impl<'a> Iterator for TransitionPropertyIterator<'a> {
+    type Item = TransitionPropertyIteration;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        use crate::values::computed::TransitionProperty;
+        loop {
+            if let Some(ref mut longhand_iterator) = self.longhand_iterator {
+                if let Some(longhand_id) = longhand_iterator.next() {
+                    return Some(TransitionPropertyIteration {
+                        longhand_id,
+                        index: self.index_range.start,
+                    });
+                }
+                self.longhand_iterator = None;
+            }
+
+            let index = self.index_range.next()?;
+            match self.style.get_box().transition_property_at(index) {
+                TransitionProperty::Longhand(longhand_id) => {
+                    return Some(TransitionPropertyIteration {
+                        longhand_id,
+                        index,
+                    })
+                }
+                // In the other cases, we set up our state so that we are ready to
+                // compute the next value of the iterator and then loop (equivalent
+                // to calling self.next()).
+                TransitionProperty::Shorthand(ref shorthand_id) =>
+                    self.longhand_iterator = Some(shorthand_id.longhands()),
+                TransitionProperty::Custom(..) | TransitionProperty::Unsupported(..) => {}
+            }
+        }
+    }
+}

servo/components/style/properties/properties.mako.rs

@@ -3020,6 +3020,13 @@ impl ComputedValues {
         % endfor
         set
     }
+
+    /// Create a `TransitionPropertyIterator` for this styles transition properties.
+    pub fn transition_properties<'a>(
+        &'a self
+    ) -> animated_properties::TransitionPropertyIterator<'a> {
+        animated_properties::TransitionPropertyIterator::from_style(self)
+    }
 }
 
 #[cfg(feature = "servo")]