From 21148c7e5183f0799745d67bfadf860c442baf3d Mon Sep 17 00:00:00 2001
From: Jeremy Chen <jeremychen@mozilla.com>
Date: Mon, 11 Sep 2017 16:04:12 +0800
Subject: [PATCH] stylo: do not handle the fallback discrete animation inside
 the Animate trait.

At present, we do the fallback discrete animation for non-invertible matrices in
ComputedMatrix.animate(). However, according to the spec, we should fallback to
discrete animation for cases like:

1. animation between transform with single non-invertible matrix
2. animation between transform with matched transform functions that have at least
   one non-invertible matrix
2. animation between transform with mismatched transform functions that have at
   least one non-invertible matrix.

The current implementation only handles the first case.

Moreover, we already have fallback discrete animation procedures in CSS Animation
and Web Animation, so we should be able to not doing any fallback inside the
Animate trait.

In this patch, we let the animation between non-invertible matrices to return Err().
So, we can propagate the Err() to the callers, and let the fallback discrete
animation procedure stay at the Servo_MatrixTransform_Operate, which is ouside
the Animate trait.
---
 .../helpers/animated_properties.mako.rs        | 18 ++++++++----------
 ports/geckolib/glue.rs                         |  8 ++++++--
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs
index 1e72fb1cbc9..ee3f601cdb9 100644
--- a/components/style/properties/helpers/animated_properties.mako.rs
+++ b/components/style/properties/helpers/animated_properties.mako.rs
@@ -1453,11 +1453,10 @@ impl Animate for ComputedMatrix {
                 (Ok(this), Ok(other)) => {
                     Ok(ComputedMatrix::from(this.animate(&other, procedure)?))
                 },
-                _ => {
-                    let (this_weight, other_weight) = procedure.weights();
-                    let result = if this_weight > other_weight { *self } else { *other };
-                    Ok(result)
-                },
+                // Matrices can be undecomposable due to couple reasons, e.g.,
+                // non-invertible matrices. In this case, we should report Err
+                // here, and let the caller do the fallback procedure.
+                _ => Err(())
             }
         } else {
             let this = MatrixDecomposed2D::from(*self);
@@ -1477,11 +1476,10 @@ impl Animate for ComputedMatrix {
             (Ok(from), Ok(to)) => {
                 Ok(ComputedMatrix::from(from.animate(&to, procedure)?))
             },
-            _ => {
-                let (this_weight, other_weight) = procedure.weights();
-                let result = if this_weight > other_weight { *self } else { *other };
-                Ok(result)
-            },
+            // Matrices can be undecomposable due to couple reasons, e.g.,
+            // non-invertible matrices. In this case, we should report Err here,
+            // and let the caller do the fallback procedure.
+            _ => Err(())
         }
     }
 }
diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs
index eccfc810cfe..ed5bd40c338 100644
--- a/ports/geckolib/glue.rs
+++ b/ports/geckolib/glue.rs
@@ -2218,9 +2218,13 @@ pub extern "C" fn Servo_MatrixTransform_Operate(matrix_operator: MatrixTransform
     };
 
     let output = unsafe { output.as_mut() }.expect("not a valid 'output' matrix");
-    if let Ok(result) =  result {
+    if let Ok(result) = result {
         *output = result.into();
-    };
+    } else if progress < 0.5 {
+        *output = from.clone().into();
+    } else {
+        *output = to.clone().into();
+    }
 }
 
 #[no_mangle]