From d7a5f224082341f7dbb8cb66c9bdc0f5cacc64ff Mon Sep 17 00:00:00 2001
From: Jeremy Chen <jeremychen@mozilla.com>
Date: Wed, 18 Oct 2017 19:02:10 +0800
Subject: [PATCH] stylo: Avoid using InterpolateMatrix as a fallback for
 matched transform function pair.

In the current implementation, if there is any interpolation error in a matched
transform function pair, we fall-back to use InterpolateMatrix unconditionally.
However, the error could be caused by:

1. mismatched transform function pair
2. matched transform function pair within at least one undecomposable matrix.

Using InterpolateMatrix for case 1 makes sense, however, using InterpolateMatrix
for case 2 does not. According to the spec, we should just report error for
case 2, and let the caller do the fallback procedure. Using InterpolateMatrix
for case 2 will go through more unnecessary code path, and produce more memory
usage and calculation cost, which should be avoidable.

In this patch, we add an extra pass to check if a transform function pair have
matched operations in advance. With this information, we can easily tell whether
the interpolation error in a equal-length transform function pair is caused by
case 1 or case 2. So, we can avoid the unnecessary cost.

Gecko bug: Bug 1399049
---
 .../helpers/animated_properties.mako.rs       | 53 +++++++++++++++----
 1 file changed, 44 insertions(+), 9 deletions(-)

diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs
index 422447d582d..b7223d4b616 100644
--- a/components/style/properties/helpers/animated_properties.mako.rs
+++ b/components/style/properties/helpers/animated_properties.mako.rs
@@ -1157,6 +1157,27 @@ impl Animate for TransformOperation {
     }
 }
 
+fn is_matched_operation(first: &TransformOperation, second: &TransformOperation) -> bool {
+    match (first, second) {
+        (&TransformOperation::Matrix(..),
+         &TransformOperation::Matrix(..)) |
+        (&TransformOperation::MatrixWithPercents(..),
+         &TransformOperation::MatrixWithPercents(..)) |
+        (&TransformOperation::Skew(..),
+         &TransformOperation::Skew(..)) |
+        (&TransformOperation::Translate(..),
+         &TransformOperation::Translate(..)) |
+        (&TransformOperation::Scale(..),
+         &TransformOperation::Scale(..)) |
+        (&TransformOperation::Rotate(..),
+         &TransformOperation::Rotate(..)) |
+        (&TransformOperation::Perspective(..),
+         &TransformOperation::Perspective(..)) => true,
+        // InterpolateMatrix and AccumulateMatrix are for mismatched transform.
+        _ => false
+    }
+}
+
 /// <https://www.w3.org/TR/css-transforms-1/#Rotate3dDefined>
 fn rotate_to_matrix(x: f32, y: f32, z: f32, a: Angle) -> ComputedMatrix {
     let half_rad = a.radians() / 2.0;
@@ -2138,23 +2159,37 @@ impl Animate for TransformList {
             Cow::Owned(self.to_animated_zero()?)
         };
 
+        // For matched transform lists.
         {
             let this = (*this).0.as_ref().map_or(&[][..], |l| l);
             let other = (*other).0.as_ref().map_or(&[][..], |l| l);
             if this.len() == other.len() {
-                let result = this.iter().zip(other).map(|(this, other)| {
-                    this.animate(other, procedure)
-                }).collect::<Result<Vec<_>, _>>();
-                if let Ok(list) = result {
-                    return Ok(TransformList(if list.is_empty() {
-                        None
-                    } else {
-                        Some(list)
-                    }));
+                let is_matched_transforms = this.iter().zip(other).all(|(this, other)| {
+                    is_matched_operation(this, other)
+                });
+
+                if is_matched_transforms {
+                    let result = this.iter().zip(other).map(|(this, other)| {
+                        this.animate(other, procedure)
+                    }).collect::<Result<Vec<_>, _>>();
+                    if let Ok(list) = result {
+                        return Ok(TransformList(if list.is_empty() {
+                            None
+                        } else {
+                            Some(list)
+                        }));
+                    }
+
+                    // Can't animate for a pair of matched transform lists?
+                    // This means we have at least one undecomposable matrix,
+                    // so we should report Err here, and let the caller do
+                    // the fallback procedure.
+                    return Err(());
                 }
             }
         }
 
+        // For mismatched transform lists.
         match procedure {
             Procedure::Add => Err(()),
             Procedure::Interpolate { progress } => {