Bug 1876266 - Do less work on the ConsiderInitiatingTransition loop. r=firefox-style-system-reviewers,zrhoffman

Differential Revision: https://phabricator.services.mozilla.com/D199483

layout/style/nsTransitionManager.cpp

@@ -103,9 +103,16 @@ bool nsTransitionManager::DoUpdateTransitions(
   bool startedAny = false;
   AnimatedPropertyIDSet propertiesChecked;
   for (uint32_t i = aStyle.mTransitionPropertyCount; i--;) {
-    // We're not going to look at any further transitions, so we can just avoid
-    // looking at this if we know it will not start any transitions.
-    if (i == 0 && aStyle.GetTransitionCombinedDuration(i).seconds <= 0.0f) {
+    const float delay = aStyle.GetTransitionDelay(i).ToMilliseconds();
+
+    // The spec says a negative duration is treated as zero.
+    const float duration =
+        std::max(aStyle.GetTransitionDuration(i).ToMilliseconds(), 0.0f);
+
+    // If the combined duration of this transition is 0 or less we won't start a
+    // transition, we can avoid even looking at transition-property if we're the
+    // last one.
+    if (i == 0 && delay + duration <= 0.0f) {
       continue;
     }
 
@@ -115,9 +122,9 @@ bool nsTransitionManager::DoUpdateTransitions(
                                // any of the properties in question changed and
                                // are animatable.
                                startedAny |= ConsiderInitiatingTransition(
-                                   aProperty, aStyle, i, aElement, aPseudoType,
-                                   aElementTransitions, aOldStyle, aNewStyle,
-                                   propertiesChecked);
+                                   aProperty, aStyle, i, delay, duration,
+                                   aElement, aPseudoType, aElementTransitions,
+                                   aOldStyle, aNewStyle, propertiesChecked);
                              });
   }
 
@@ -192,10 +199,6 @@ static nsTArray<Keyframe> GetTransitionKeyframes(
   return keyframes;
 }
 
-static bool IsTransitionable(const AnimatedPropertyID& aProperty) {
-  return Servo_Property_IsTransitionable(&aProperty);
-}
-
 static Maybe<CSSTransition::ReplacedTransitionProperties>
 GetReplacedTransitionProperties(const CSSTransition* aTransition,
                                 const DocumentTimeline* aTimelineToMatch) {
@@ -244,7 +247,8 @@ GetReplacedTransitionProperties(const CSSTransition* aTransition,
 
 bool nsTransitionManager::ConsiderInitiatingTransition(
     const AnimatedPropertyID& aProperty, const nsStyleUIReset& aStyle,
-    uint32_t transitionIdx, dom::Element* aElement, PseudoStyleType aPseudoType,
+    uint32_t aTransitionIndex, float aDelay, float aDuration,
+    dom::Element* aElement, PseudoStyleType aPseudoType,
     CSSTransitionCollection*& aElementTransitions,
     const ComputedStyle& aOldStyle, const ComputedStyle& aNewStyle,
     AnimatedPropertyIDSet& aPropertiesChecked) {
@@ -267,19 +271,7 @@ bool nsTransitionManager::ConsiderInitiatingTransition(
 
   aPropertiesChecked.AddProperty(property);
 
-  if (!IsTransitionable(property)) {
-    return false;
-  }
-
-  float delay = aStyle.GetTransitionDelay(transitionIdx).ToMilliseconds();
-
-  // The spec says a negative duration is treated as zero.
-  float duration = std::max(
-      aStyle.GetTransitionDuration(transitionIdx).ToMilliseconds(), 0.0f);
-
-  // If the combined duration of this transition is 0 or less don't start a
-  // transition.
-  if (delay + duration <= 0.0f) {
+  if (aDuration + aDelay <= 0.0f) {
     return false;
   }
 
@@ -370,12 +362,12 @@ bool nsTransitionManager::ConsiderInitiatingTransition(
     // Negative delays are essentially part of the transition
     // function, so reduce them along with the duration, but don't
     // reduce positive delays.
-    if (delay < 0.0f && std::isfinite(delay)) {
-      delay *= valuePortion;
+    if (aDelay < 0.0f && std::isfinite(aDelay)) {
+      aDelay *= valuePortion;
     }
 
-    if (std::isfinite(duration)) {
-      duration *= valuePortion;
+    if (std::isfinite(aDuration)) {
+      aDuration *= valuePortion;
     }
 
     startForReversingTest = oldTransition->ToValue();
@@ -383,11 +375,11 @@ bool nsTransitionManager::ConsiderInitiatingTransition(
   }
 
   TimingParams timing = TimingParamsFromCSSParams(
-      duration, delay, 1.0 /* iteration count */,
+      aDuration, aDelay, 1.0 /* iteration count */,
       StyleAnimationDirection::Normal, StyleAnimationFillMode::Backwards);
 
   const StyleComputedTimingFunction& tf =
-      aStyle.GetTransitionTimingFunction(transitionIdx);
+      aStyle.GetTransitionTimingFunction(aTransitionIndex);
   if (!tf.IsLinearKeyword()) {
     timing.SetTimingFunction(Some(tf));
   }

layout/style/nsTransitionManager.h

@@ -62,8 +62,8 @@ class nsTransitionManager final
   // Returns whether the transition actually started.
   bool ConsiderInitiatingTransition(
       const mozilla::AnimatedPropertyID&, const nsStyleUIReset& aStyle,
-      uint32_t transitionIdx, mozilla::dom::Element* aElement,
-      mozilla::PseudoStyleType aPseudoType,
+      uint32_t aTransitionIndex, float aDelay, float aDuration,
+      mozilla::dom::Element* aElement, mozilla::PseudoStyleType aPseudoType,
       CSSTransitionCollection*& aElementTransitions,
       const mozilla::ComputedStyle& aOldStyle,
       const mozilla::ComputedStyle& aNewStyle,

servo/components/style/properties/data.py

@@ -442,9 +442,6 @@ class Longhand(Property):
         self.animation_value_type = animation_value_type
 
         self.animatable = animation_value_type != "none"
-        self.transitionable = (
-            animation_value_type != "none" and animation_value_type != "discrete"
-        )
         self.is_animatable_with_computed_value = (
             animation_value_type == "ComputedValue"
             or animation_value_type == "discrete"
@@ -692,16 +689,7 @@ class Shorthand(Property):
                 return True
         return False
 
-    def get_transitionable(self):
-        transitionable = False
-        for sub in self.sub_properties:
-            if sub.transitionable:
-                transitionable = True
-                break
-        return transitionable
-
     animatable = property(get_animatable)
-    transitionable = property(get_transitionable)
 
     @staticmethod
     def type():
@@ -719,7 +707,6 @@ class Alias(object):
         self.servo_2013_pref = original.servo_2013_pref
         self.servo_2020_pref = original.servo_2020_pref
         self.gecko_pref = gecko_pref
-        self.transitionable = original.transitionable
         self.rule_types_allowed = original.rule_types_allowed
         self.flags = original.flags
 

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

@@ -389,7 +389,7 @@ impl AnimationValue {
         Some(animatable)
     }
 
-    /// Get an AnimationValue for an AnimatableLonghand from a given computed values.
+    /// Get an AnimationValue for an declaration id from a given computed values.
     pub fn from_computed_values(
         property: PropertyDeclarationId,
         style: &ComputedValues,

servo/components/style/properties/mod.rs

@@ -367,14 +367,6 @@ impl PropertyId {
         }
     }
 
-    /// Returns true if this property is one of the transitionable properties.
-    pub fn is_transitionable(&self) -> bool {
-        match self {
-            Self::NonCustom(id) => id.is_transitionable(),
-            Self::Custom(..) => true,
-        }
-    }
-
     /// Returns a given property from the given name, _regardless of whether it is enabled or
     /// not_, or Err(()) for unknown properties.
     ///

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

@@ -476,13 +476,6 @@ impl NonCustomPropertyId {
         MAP[self.0 as usize]
     }
 
-    /// Returns whether this property is transitionable.
-    #[inline]
-    pub fn is_transitionable(self) -> bool {
-        ${static_non_custom_property_id_set("TRANSITIONABLE", lambda p: p.transitionable)}
-        TRANSITIONABLE.contains(self)
-    }
-
     /// Returns whether this property is animatable.
     #[inline]
     pub fn is_animatable(self) -> bool {

servo/ports/geckolib/glue.rs

@@ -1203,6 +1203,11 @@ pub struct ShouldTransitionResult {
     old_transition_value_matches: bool,
 }
 
+#[inline]
+fn is_transitionable(prop: PropertyDeclarationId) -> bool {
+    prop.is_animatable() && !prop.is_discrete_animatable()
+}
+
 #[no_mangle]
 pub extern "C" fn Servo_ComputedValues_ShouldTransition(
     old: &ComputedValues,
@@ -1216,9 +1221,7 @@ pub extern "C" fn Servo_ComputedValues_ShouldTransition(
         return Default::default();
     };
     let prop = prop.as_borrowed();
-    if prop.is_discrete_animatable() &&
-        prop != PropertyDeclarationId::Longhand(LonghandId::Visibility)
-    {
+    if !is_transitionable(prop) {
         return Default::default();
     }
 
@@ -1261,9 +1264,7 @@ pub extern "C" fn Servo_ComputedValues_TransitionValueMatches(
         return false;
     };
     let prop = prop.as_borrowed();
-    if prop.is_discrete_animatable() &&
-        prop != PropertyDeclarationId::Longhand(LonghandId::Visibility)
-    {
+    if !is_transitionable(prop) {
         return false;
     }
     let Some(value) = AnimationValue::from_computed_values(prop, style) else {
@@ -1423,11 +1424,6 @@ pub extern "C" fn Servo_Property_IsAnimatable(prop: &structs::AnimatedPropertyID
     PropertyId::from_gecko_animated_property_id(prop).map_or(false, |p| p.is_animatable())
 }
 
-#[no_mangle]
-pub extern "C" fn Servo_Property_IsTransitionable(prop: &structs::AnimatedPropertyID) -> bool {
-    PropertyId::from_gecko_animated_property_id(prop).map_or(false, |p| p.is_transitionable())
-}
-
 #[no_mangle]
 pub extern "C" fn Servo_Property_IsDiscreteAnimatable(property: nsCSSPropertyID) -> bool {
     match LonghandId::from_nscsspropertyid(property) {