Bug 1746084 - Avoid generating InterpolateMatrix operations if there are no size dependencies. r=hiro

The issue here is that we end up with a transition between mismatched
transform lists that ends up generating an InterpolateMatrix {}
operation. So far so good, but we end up interpolating that a lot of
times and generating an unboundedly-deep operation list.

This implementas an optimization that flattens them to a single matrix
when possible (when there's no dependencies on the containing box).

This is similar to:

  2b89cc4df4

We fix the to_pixel_length() behavior for LenghtPercentage to be
correct (and update callers to preserve behavior).

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

servo/components/style/values/animated/transform.rs

@@ -891,25 +891,8 @@ impl Animate for ComputedTransform {
         match (this_remainder, other_remainder) {
             // If there is a remainder from *both* lists we must have had mismatched functions.
             // => Add the remainders to a suitable ___Matrix function.
-            (Some(this_remainder), Some(other_remainder)) => match procedure {
-                Procedure::Add => {
-                    debug_assert!(false, "Should have already dealt with add by the point");
-                    return Err(());
-                },
-                Procedure::Interpolate { progress } => {
-                    result.push(TransformOperation::InterpolateMatrix {
-                        from_list: Transform(this_remainder.to_vec().into()),
-                        to_list: Transform(other_remainder.to_vec().into()),
-                        progress: Percentage(progress as f32),
-                    });
-                },
-                Procedure::Accumulate { count } => {
-                    result.push(TransformOperation::AccumulateMatrix {
-                        from_list: Transform(this_remainder.to_vec().into()),
-                        to_list: Transform(other_remainder.to_vec().into()),
-                        count: cmp::min(count, i32::max_value() as u64) as i32,
-                    });
-                },
+            (Some(this_remainder), Some(other_remainder)) => {
+                result.push(TransformOperation::animate_mismatched_transforms(this_remainder, other_remainder, procedure)?);
             },
             // If there is a remainder from just one list, then one list must be shorter but
             // completely match the type of the corresponding functions in the longer list.
@@ -923,36 +906,19 @@ impl Animate for ComputedTransform {
                             let identity = transform.to_animated_zero().unwrap();
 
                             match transform {
-                                // We can't interpolate/accumulate ___Matrix types directly with a
-                                // matrix. Instead we need to wrap it in another ___Matrix type.
                                 TransformOperation::AccumulateMatrix { .. } |
                                 TransformOperation::InterpolateMatrix { .. } => {
-                                    let transform_list = Transform(vec![transform.clone()].into());
-                                    let identity_list = Transform(vec![identity].into());
-                                    let (from_list, to_list) = if fill_right {
-                                        (transform_list, identity_list)
+                                    let (from, to) = if fill_right {
+                                        (transform, &identity)
                                     } else {
-                                        (identity_list, transform_list)
+                                        (&identity, transform)
                                     };
 
-                                    match procedure {
-                                        Procedure::Add => Err(()),
-                                        Procedure::Interpolate { progress } => {
-                                            Ok(TransformOperation::InterpolateMatrix {
-                                                from_list,
-                                                to_list,
-                                                progress: Percentage(progress as f32),
-                                            })
-                                        },
-                                        Procedure::Accumulate { count } => {
-                                            Ok(TransformOperation::AccumulateMatrix {
-                                                from_list,
-                                                to_list,
-                                                count: cmp::min(count, i32::max_value() as u64)
-                                                    as i32,
-                                            })
-                                        },
-                                    }
+                                    TransformOperation::animate_mismatched_transforms(
+                                        &[from.clone()],
+                                        &[to.clone()],
+                                        procedure,
+                                    )
                                 },
                                 _ => {
                                     let (lhs, rhs) = if fill_right {
@@ -981,9 +947,13 @@ impl ComputeSquaredDistance for ComputedTransform {
 
         // Roll back to matrix interpolation if there is any Err(()) in the
         // transform lists, such as mismatched transform functions.
+        //
+        // FIXME: Using a zero size here seems a bit sketchy but matches the
+        // previous behavior.
         if squared_dist.is_err() {
-            let matrix1: Matrix3D = self.to_transform_3d_matrix(None)?.0.into();
-            let matrix2: Matrix3D = other.to_transform_3d_matrix(None)?.0.into();
+            let rect = euclid::Rect::zero();
+            let matrix1: Matrix3D = self.to_transform_3d_matrix(Some(&rect))?.0.into();
+            let matrix2: Matrix3D = other.to_transform_3d_matrix(Some(&rect))?.0.into();
             return matrix1.compute_squared_distance(&matrix2);
         }
 
@@ -1141,6 +1111,52 @@ impl Animate for ComputedTransformOperation {
     }
 }
 
+impl ComputedTransformOperation {
+    /// If there are no size dependencies, we try to animate in-place, to avoid
+    /// creating deeply nested Interpolate* operations.
+    fn try_animate_mismatched_transforms_in_place(
+        left: &[Self],
+        right: &[Self],
+        procedure: Procedure,
+    ) -> Result<Self, ()> {
+        let (left, _left_3d) = Transform::components_to_transform_3d_matrix(left, None)?;
+        let (right, _right_3d) = Transform::components_to_transform_3d_matrix(right, None)?;
+        ComputedTransformOperation::Matrix3D(left.into()).animate(&ComputedTransformOperation::Matrix3D(right.into()), procedure)
+    }
+
+    fn animate_mismatched_transforms(
+        left: &[Self],
+        right: &[Self],
+        procedure: Procedure,
+    ) -> Result<Self, ()> {
+        if let Ok(op) = Self::try_animate_mismatched_transforms_in_place(left, right, procedure) {
+            return Ok(op);
+        }
+        let from_list = Transform(left.to_vec().into());
+        let to_list = Transform(right.to_vec().into());
+        Ok(match procedure {
+            Procedure::Add => {
+                debug_assert!(false, "Addition should've been handled earlier");
+                return Err(())
+            },
+            Procedure::Interpolate { progress } => {
+                Self::InterpolateMatrix {
+                    from_list,
+                    to_list,
+                    progress: Percentage(progress as f32),
+                }
+            }
+            Procedure::Accumulate { count } => {
+                Self::AccumulateMatrix {
+                    from_list,
+                    to_list,
+                    count: cmp::min(count, i32::max_value() as u64) as i32,
+                }
+            }
+        })
+    }
+}
+
 // This might not be the most useful definition of distance. It might be better, for example,
 // to trace the distance travelled by a point as its transform is interpolated between the two
 // lists. That, however, proves to be quite complicated so we take a simple approach for now.

servo/components/style/values/generics/transform.rs

@@ -404,15 +404,7 @@ impl ToAbsoluteLength for ComputedLength {
 impl ToAbsoluteLength for ComputedLengthPercentage {
     #[inline]
     fn to_pixel_length(&self, containing_len: Option<ComputedLength>) -> Result<CSSFloat, ()> {
-        match containing_len {
-            Some(relative_len) => Ok(self.resolve(relative_len).px()),
-            // If we don't have reference box, we cannot resolve the used value,
-            // so only retrieve the length part. This will be used for computing
-            // distance without any layout info.
-            //
-            // FIXME(emilio): This looks wrong.
-            None => Ok(self.resolve(Zero::zero()).px()),
-        }
+        Ok(self.maybe_percentage_relative_to(containing_len).ok_or(())?.px())
     }
 }
 
@@ -572,12 +564,21 @@ impl<T> Transform<T> {
 
 impl<T: ToMatrix> Transform<T> {
     /// Return the equivalent 3d matrix of this transform list.
+    ///
     /// We return a pair: the first one is the transform matrix, and the second one
     /// indicates if there is any 3d transform function in this transform list.
     #[cfg_attr(rustfmt, rustfmt_skip)]
     pub fn to_transform_3d_matrix(
         &self,
         reference_box: Option<&Rect<ComputedLength>>
+    ) -> Result<(Transform3D<CSSFloat>, bool), ()> {
+        Self::components_to_transform_3d_matrix(&self.0, reference_box)
+    }
+
+    /// Converts a series of components to a 3d matrix.
+    pub fn components_to_transform_3d_matrix(
+        ops: &[T],
+        reference_box: Option<&Rect<ComputedLength>>
     ) -> Result<(Transform3D<CSSFloat>, bool), ()> {
         let cast_3d_transform = |m: Transform3D<f64>| -> Transform3D<CSSFloat> {
             use std::{f32, f64};
@@ -590,26 +591,27 @@ impl<T: ToMatrix> Transform<T> {
             )
         };
 
-        let (m, is_3d) = self.to_transform_3d_matrix_f64(reference_box)?;
+        let (m, is_3d) = Self::components_to_transform_3d_matrix_f64(ops, reference_box)?;
         Ok((cast_3d_transform(m), is_3d))
     }
 
     /// Same as Transform::to_transform_3d_matrix but a f64 version.
-    pub fn to_transform_3d_matrix_f64(
-        &self,
+    fn components_to_transform_3d_matrix_f64(
+        ops: &[T],
         reference_box: Option<&Rect<ComputedLength>>,
     ) -> Result<(Transform3D<f64>, bool), ()> {
-        // We intentionally use Transform3D<f64> during computation to avoid error propagation
-        // because using f32 to compute triangle functions (e.g. in rotation()) is not
-        // accurate enough. In Gecko, we also use "double" to compute the triangle functions.
-        // Therefore, let's use Transform3D<f64> during matrix computation and cast it into f32
-        // in the end.
+        // We intentionally use Transform3D<f64> during computation to avoid
+        // error propagation because using f32 to compute triangle functions
+        // (e.g. in rotation()) is not accurate enough. In Gecko, we also use
+        // "double" to compute the triangle functions. Therefore, let's use
+        // Transform3D<f64> during matrix computation and cast it into f32 in
+        // the end.
         let mut transform = Transform3D::<f64>::identity();
         let mut contain_3d = false;
 
-        for operation in &*self.0 {
+        for operation in ops {
             let matrix = operation.to_3d_matrix(reference_box)?;
-            contain_3d |= operation.is_3d();
+            contain_3d = contain_3d || operation.is_3d();
             transform = matrix.then(&transform);
         }