Bug 1643201 - servo: Remove AnimatedProperty.
authorMartin Robinson <mrobinson@igalia.com>
Thu, 04 Jun 2020 00:34:34 +0000
changeset 533805 99aa4226e81a6ad28a1a147e765296a80d8ce54d
parent 533804 e907368ef28594e98744c3353f5ddfb411ea8ffe
child 533806 46918ab447afbe6ccfdf51e5bd9a8559c4f05013
push id37478
push userabutkovits@mozilla.com
push dateThu, 04 Jun 2020 09:29:07 +0000
treeherdermozilla-central@e87e4800d332 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1643201
milestone79.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1643201 - servo: Remove AnimatedProperty. This removes an extra layer of abstraction and allows Servo to share more code with Gecko. In addition, we will need to handle raw `AnimationValue` structs soon in order to fully implement "faster reversing of interrupted transitions." Depends on D78192 Differential Revision: https://phabricator.services.mozilla.com/D78193
servo/components/style/animation.rs
servo/components/style/properties/helpers/animated_properties.mako.rs
--- a/servo/components/style/animation.rs
+++ b/servo/components/style/animation.rs
@@ -6,138 +6,138 @@
 
 // NOTE(emilio): This code isn't really executed in Gecko, but we don't want to
 // compile it out so that people remember it exists.
 
 use crate::bezier::Bezier;
 use crate::context::SharedStyleContext;
 use crate::dom::{OpaqueNode, TElement, TNode};
 use crate::font_metrics::FontMetricsProvider;
-use crate::properties::animated_properties::AnimatedProperty;
+use crate::properties::animated_properties::AnimationValue;
 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};
 #[cfg(feature = "servo")]
 use crate::properties::LonghandIdSet;
 use crate::stylesheets::keyframes_rule::{KeyframesAnimation, KeyframesStep, KeyframesStepValue};
 use crate::stylesheets::Origin;
+use crate::values::animated::{Animate, Procedure};
 use crate::values::computed::Time;
 use crate::values::computed::TimingFunction;
 use crate::values::generics::box_::AnimationIterationCount;
 use crate::values::generics::easing::{StepPosition, TimingFunction as GenericTimingFunction};
 use crate::Atom;
 use servo_arc::Arc;
 use std::fmt;
 
 /// Represents an animation for a given property.
 #[derive(Clone, Debug, MallocSizeOf)]
 pub struct PropertyAnimation {
-    /// An `AnimatedProperty` that this `PropertyAnimation` corresponds to.
-    property: AnimatedProperty,
+    /// The value we are animating from.
+    from: AnimationValue,
+
+    /// The value we are animating to.
+    to: AnimationValue,
 
     /// The timing function of this `PropertyAnimation`.
     timing_function: TimingFunction,
 
     /// The duration of this `PropertyAnimation` in seconds.
     pub duration: f64,
 }
 
 impl PropertyAnimation {
     /// Returns the given property longhand id.
     pub fn property_id(&self) -> LonghandId {
-        self.property.id()
-    }
-
-    /// Returns the given property name.
-    pub fn property_name(&self) -> &'static str {
-        self.property.name()
+        debug_assert_eq!(self.from.id(), self.to.id());
+        self.from.id()
     }
 
     fn from_longhand(
         longhand: LonghandId,
         timing_function: TimingFunction,
         duration: Time,
         old_style: &ComputedValues,
         new_style: &ComputedValues,
     ) -> Option<PropertyAnimation> {
-        let animated_property = AnimatedProperty::from_longhand(longhand, old_style, new_style)?;
+        // FIXME(emilio): Handle the case where old_style and new_style's writing mode differ.
+        let longhand = longhand.to_physical(new_style.writing_mode);
+        let from = AnimationValue::from_computed_values(longhand, old_style)?;
+        let to = AnimationValue::from_computed_values(longhand, new_style)?;
+        let duration = duration.seconds() as f64;
 
-        let property_animation = PropertyAnimation {
-            property: animated_property,
+        if from == to || duration == 0.0 {
+            return None;
+        }
+
+        Some(PropertyAnimation {
+            from,
+            to,
             timing_function,
-            duration: duration.seconds() as f64,
-        };
-
-        if property_animation.does_animate() {
-            Some(property_animation)
-        } else {
-            None
-        }
+            duration,
+        })
     }
 
-    /// Update the given animation at a given point of progress.
-    pub fn update(&self, style: &mut ComputedValues, time: f64) {
+    /// The output of the timing function given the progress ration of this animation.
+    fn timing_function_output(&self, progress: f64) -> f64 {
         let epsilon = 1. / (200. * self.duration);
-        let progress = match self.timing_function {
+        match self.timing_function {
             GenericTimingFunction::CubicBezier { x1, y1, x2, y2 } => {
-                Bezier::new(x1, y1, x2, y2).solve(time, epsilon)
+                Bezier::new(x1, y1, x2, y2).solve(progress, epsilon)
             },
             GenericTimingFunction::Steps(steps, pos) => {
-                let mut current_step = (time * (steps as f64)).floor() as i32;
+                let mut current_step = (progress * (steps as f64)).floor() as i32;
 
                 if pos == StepPosition::Start ||
                     pos == StepPosition::JumpStart ||
                     pos == StepPosition::JumpBoth
                 {
                     current_step = current_step + 1;
                 }
 
                 // FIXME: We should update current_step according to the "before flag".
                 // In order to get the before flag, we have to know the current animation phase
                 // and whether the iteration is reversed. For now, we skip this calculation.
                 // (i.e. Treat before_flag is unset,)
                 // https://drafts.csswg.org/css-easing/#step-timing-function-algo
 
-                if time >= 0.0 && current_step < 0 {
+                if progress >= 0.0 && current_step < 0 {
                     current_step = 0;
                 }
 
                 let jumps = match pos {
                     StepPosition::JumpBoth => steps + 1,
                     StepPosition::JumpNone => steps - 1,
                     StepPosition::JumpStart |
                     StepPosition::JumpEnd |
                     StepPosition::Start |
                     StepPosition::End => steps,
                 };
 
-                if time <= 1.0 && current_step > jumps {
+                if progress <= 1.0 && current_step > jumps {
                     current_step = jumps;
                 }
 
                 (current_step as f64) / (jumps as f64)
             },
             GenericTimingFunction::Keyword(keyword) => {
                 let (x1, x2, y1, y2) = keyword.to_bezier();
-                Bezier::new(x1, x2, y1, y2).solve(time, epsilon)
+                Bezier::new(x1, x2, y1, y2).solve(progress, epsilon)
             },
-        };
-
-        self.property.update(style, progress);
+        }
     }
 
-    #[inline]
-    fn does_animate(&self) -> bool {
-        self.property.does_animate() && self.duration != 0.0
-    }
-
-    /// Whether this animation has the same end value as another one.
-    #[inline]
-    pub fn has_the_same_end_value_as(&self, other: &Self) -> bool {
-        self.property.has_the_same_end_value_as(&other.property)
+    /// Update the given animation at a given point of progress.
+    fn update(&self, style: &mut ComputedValues, progress: f64) {
+        let procedure = Procedure::Interpolate {
+            progress: self.timing_function_output(progress),
+        };
+        if let Ok(new_value) = self.from.animate(&self.to, procedure) {
+            new_value.set_in_style_for_servo(style);
+        }
     }
 }
 
 /// This structure represents the state of an animation.
 #[derive(Clone, Debug, MallocSizeOf, PartialEq)]
 pub enum AnimationState {
     /// This animation is paused. The inner field is the percentage of progress
     /// when it was paused, from 0 to 1.
@@ -482,52 +482,33 @@ impl Animation {
             context,
             target_keyframe,
             &from_style,
             &self.cascade_style,
             font_metrics_provider,
         );
 
         let mut new_style = (*style).clone();
+        let mut update_style_for_longhand = |longhand| {
+            let from = AnimationValue::from_computed_values(longhand, &from_style)?;
+            let to = AnimationValue::from_computed_values(longhand, &target_style)?;
+            PropertyAnimation {
+                from,
+                to,
+                timing_function,
+                duration: relative_duration as f64,
+            }
+            .update(&mut new_style, relative_progress);
+            None::<()>
+        };
 
         for property in self.keyframes_animation.properties_changed.iter() {
-            debug!(
-                "Animation::update_style: scanning prop {:?} for animation \"{}\"",
-                property, self.name
-            );
-            let animation = PropertyAnimation::from_longhand(
-                property,
-                timing_function,
-                Time::from_seconds(relative_duration as f32),
-                &from_style,
-                &target_style,
-            );
-
-            match animation {
-                Some(property_animation) => {
-                    debug!(
-                        "Animation::update_style: got property animation for prop {:?}",
-                        property
-                    );
-                    debug!("Animation::update_style: {:?}", property_animation);
-                    property_animation.update(&mut new_style, relative_progress);
-                },
-                None => {
-                    debug!(
-                        "Animation::update_style: property animation {:?} not animating",
-                        property
-                    );
-                },
-            }
+            update_style_for_longhand(property);
         }
 
-        debug!(
-            "Animation::update_style: got style change in animation \"{}\"",
-            self.name
-        );
         *style = new_style;
     }
 }
 
 impl fmt::Debug for Animation {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         f.debug_struct("Animation")
             .field("name", &self.name)
@@ -569,34 +550,29 @@ impl Transition {
     /// not take into account canceling i.e. when an animation or transition is
     /// canceled due to changes in the style.
     pub fn has_ended(&self, time: f64) -> bool {
         time >= self.start_time + (self.property_animation.duration)
     }
 
     /// Whether this animation has the same end value as another one.
     #[inline]
-    fn has_same_end_value(&self, other_animation: &PropertyAnimation) -> bool {
-        if self.state == AnimationState::Canceled {
-            return false;
-        }
-        self.property_animation
-            .has_the_same_end_value_as(other_animation)
+    fn progress(&self, now: f64) -> f64 {
+        let progress = (now - self.start_time) / (self.property_animation.duration);
+        progress.min(1.0)
     }
 
     /// Update a style to the value specified by this `Transition` given a `SharedStyleContext`.
     fn update_style(&self, context: &SharedStyleContext, style: &mut ComputedValues) {
         // Never apply canceled transitions to a style.
         if self.state == AnimationState::Canceled {
             return;
         }
 
-        let now = context.current_time_for_animations;
-        let progress = (now - self.start_time) / (self.property_animation.duration);
-        let progress = progress.min(1.0);
+        let progress = self.progress(context.current_time_for_animations);
         if progress >= 0.0 {
             self.property_animation.update(style, progress);
         }
     }
 }
 
 /// Holds the animation state for a particular element.
 #[derive(Debug, Default, MallocSizeOf)]
@@ -778,52 +754,50 @@ pub fn start_transitions_if_applicable(
     context: &SharedStyleContext,
     opaque_node: OpaqueNode,
     old_style: &ComputedValues,
     new_style: &Arc<ComputedValues>,
     animation_state: &mut ElementAnimationSet,
 ) -> LonghandIdSet {
     // If the style of this element is display:none, then we don't start any transitions
     // and we cancel any currently running transitions by returning an empty LonghandIdSet.
-    if new_style.get_box().clone_display().is_none() {
+    let box_style = new_style.get_box();
+    if box_style.clone_display().is_none() {
         return LonghandIdSet::new();
     }
 
     let mut properties_that_transition = LonghandIdSet::new();
     for transition in new_style.transition_properties() {
         let physical_property = transition.longhand_id.to_physical(new_style.writing_mode);
         if properties_that_transition.contains(physical_property) {
             continue;
         } else {
             properties_that_transition.insert(physical_property);
         }
 
         let property_animation = match PropertyAnimation::from_longhand(
             transition.longhand_id,
-            new_style
-                .get_box()
-                .transition_timing_function_mod(transition.index),
-            new_style
-                .get_box()
-                .transition_duration_mod(transition.index),
+            box_style.transition_timing_function_mod(transition.index),
+            box_style.transition_duration_mod(transition.index),
             old_style,
             new_style,
         ) {
             Some(property_animation) => property_animation,
             None => continue,
         };
 
         // 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 running or
-        // completed.
+        // completed. We don't take into account any canceled animations.
         // [1]: https://drafts.csswg.org/css-transitions/#starting
         if animation_state
             .transitions
             .iter()
-            .any(|transition| transition.has_same_end_value(&property_animation))
+            .filter(|transition| transition.state != AnimationState::Canceled)
+            .any(|transition| transition.property_animation.to == property_animation.to)
         {
             continue;
         }
 
         // Kick off the animation.
         debug!("Kicking off transition of {:?}", property_animation);
         let box_style = new_style.get_box();
         let start_time = context.current_time_for_animations +
--- a/servo/components/style/properties/helpers/animated_properties.mako.rs
+++ b/servo/components/style/properties/helpers/animated_properties.mako.rs
@@ -52,156 +52,25 @@ impl From<nsCSSPropertyID> for Transitio
             }
             _ => {
                 panic!("non-convertible nsCSSPropertyID")
             }
         }
     }
 }
 
-/// An animated property interpolation between two computed values for that
-/// property.
-#[derive(Clone, Debug, PartialEq)]
-#[cfg_attr(feature = "servo", derive(MallocSizeOf))]
-pub enum AnimatedProperty {
-    % for prop in data.longhands:
-        % if prop.animatable and not prop.logical:
-            <%
-                value_type = "longhands::{}::computed_value::T".format(prop.ident)
-                if not prop.is_animatable_with_computed_value:
-                    value_type = "<{} as ToAnimatedValue>::AnimatedValue".format(value_type)
-            %>
-            /// ${prop.name}
-            ${prop.camel_case}(${value_type}, ${value_type}),
-        % endif
-    % endfor
-}
-
-impl AnimatedProperty {
-    /// Get the id of the property we're animating.
-    pub fn id(&self) -> LonghandId {
-        match *self {
-            % for prop in data.longhands:
-            % if prop.animatable and not prop.logical:
-            AnimatedProperty::${prop.camel_case}(..) => LonghandId::${prop.camel_case},
-            % endif
-            % endfor
-        }
-    }
-
-    /// Get the name of this property.
-    pub fn name(&self) -> &'static str {
-        self.id().name()
-    }
-
-    /// Whether this interpolation does animate, that is, whether the start and
-    /// end values are different.
-    pub fn does_animate(&self) -> bool {
-        match *self {
-            % for prop in data.longhands:
-                % if prop.animatable and not prop.logical:
-                    AnimatedProperty::${prop.camel_case}(ref from, ref to) => from != to,
-                % endif
-            % endfor
-        }
-    }
-
-    /// Whether an animated property has the same end value as another.
-    pub fn has_the_same_end_value_as(&self, other: &Self) -> bool {
-        match (self, other) {
-            % for prop in data.longhands:
-                % if prop.animatable and not prop.logical:
-                    (&AnimatedProperty::${prop.camel_case}(_, ref this_end_value),
-                     &AnimatedProperty::${prop.camel_case}(_, ref other_end_value)) => {
-                        this_end_value == other_end_value
-                    }
-                % endif
-            % endfor
-            _ => false,
-        }
-    }
-
-    /// Update `style` with the proper computed style corresponding to this
-    /// animation at `progress`.
-    #[cfg_attr(feature = "gecko", allow(unused))]
-    pub fn update(&self, style: &mut ComputedValues, progress: f64) {
-        #[cfg(feature = "servo")]
-        {
-            match *self {
-                % for prop in data.longhands:
-                % if prop.animatable and not prop.logical:
-                    AnimatedProperty::${prop.camel_case}(ref from, ref to) => {
-                        // https://drafts.csswg.org/web-animations/#discrete-animation-type
-                        % if prop.animation_value_type == "discrete":
-                            let value = if progress < 0.5 { from.clone() } else { to.clone() };
-                        % else:
-                            let value = match from.animate(to, Procedure::Interpolate { progress }) {
-                                Ok(value) => value,
-                                Err(()) => return,
-                            };
-                        % endif
-                        % if not prop.is_animatable_with_computed_value:
-                            let value: longhands::${prop.ident}::computed_value::T =
-                                ToAnimatedValue::from_animated_value(value);
-                        % endif
-                        style.mutate_${prop.style_struct.name_lower}().set_${prop.ident}(value);
-                    }
-                % endif
-                % endfor
-            }
-        }
-    }
-
-    /// Get an animatable value from a transition-property, an old style, and a
-    /// new style.
-    pub fn from_longhand(
-        property: LonghandId,
-        old_style: &ComputedValues,
-        new_style: &ComputedValues,
-    ) -> Option<AnimatedProperty> {
-        // FIXME(emilio): Handle the case where old_style and new_style's
-        // writing mode differ.
-        let property = property.to_physical(new_style.writing_mode);
-        Some(match property {
-            % for prop in data.longhands:
-            % if prop.animatable and not prop.logical:
-                LonghandId::${prop.camel_case} => {
-                    let old_computed = old_style.clone_${prop.ident}();
-                    let new_computed = new_style.clone_${prop.ident}();
-                    AnimatedProperty::${prop.camel_case}(
-                    % if prop.is_animatable_with_computed_value:
-                        old_computed,
-                        new_computed,
-                    % else:
-                        old_computed.to_animated_value(),
-                        new_computed.to_animated_value(),
-                    % endif
-                    )
-                }
-            % endif
-            % endfor
-            _ => return None,
-        })
-    }
-}
-
 /// A collection of AnimationValue that were composed on an element.
 /// This HashMap stores the values that are the last AnimationValue to be
 /// composed for each TransitionProperty.
 pub type AnimationValueMap = FxHashMap<LonghandId, AnimationValue>;
 
 /// An enum to represent a single computed value belonging to an animated
 /// property in order to be interpolated with another one. When interpolating,
 /// both values need to belong to the same property.
 ///
-/// This is different to AnimatedProperty in the sense that AnimatedProperty
-/// also knows the final value to be used during the animation.
-///
-/// This is to be used in Gecko integration code.
-///
 /// FIXME: We need to add a path for custom properties, but that's trivial after
 /// this (is a similar path to that of PropertyDeclaration).
 #[cfg_attr(feature = "servo", derive(MallocSizeOf))]
 #[derive(Debug)]
 #[repr(u16)]
 pub enum AnimationValue {
     % for prop in data.longhands:
     /// `${prop.name}`
@@ -541,16 +410,40 @@ impl AnimationValue {
                 % endif
                 )
             }
             % endif
             % endfor
             _ => return None,
         })
     }
+
+    /// Update `style` with the value of this `AnimationValue`.
+    ///
+    /// SERVO ONLY: This doesn't properly handle things like updating 'em' units
+    /// when animated font-size.
+    pub fn set_in_style_for_servo(&self, style: &mut ComputedValues) {
+        match self {
+            % for prop in data.longhands:
+            % if prop.animatable and not prop.logical:
+            AnimationValue::${prop.camel_case}(ref value) => {
+                % if not prop.is_animatable_with_computed_value:
+                let value: longhands::${prop.ident}::computed_value::T =
+                    ToAnimatedValue::from_animated_value(value.clone());
+                    style.mutate_${prop.style_struct.name_lower}().set_${prop.ident}(value);
+                % else:
+                    style.mutate_${prop.style_struct.name_lower}().set_${prop.ident}(value.clone());
+                % endif
+            }
+            % else:
+            AnimationValue::${prop.camel_case}(..) => unreachable!(),
+            % endif
+            % endfor
+        }
+    }
 }
 
 fn animate_discrete<T: Clone>(this: &T, other: &T, procedure: Procedure) -> Result<T, ()> {
     if let Procedure::Interpolate { progress } = procedure {
         Ok(if progress < 0.5 { this.clone() } else { other.clone() })
     } else {
         Err(())
     }