Bug 1394284 - stylo: do not handle the fallback discrete animation inside the Animate trait. draft
authorJeremy Chen <jeremychen@mozilla.com>
Mon, 11 Sep 2017 16:04:12 +0800
changeset 667567 c3450b5b131bcccab6831258ac6b95c1f8dba8e5
parent 667303 a0eb21bf55e1c1ae0ba311e6f2273da05c712799
child 667568 59c361ec88003070b386df20e7251eb853fe9c52
child 667622 07a7d9597e278ac6cd40db7e7efa88a7f9b0d36f
push id80761
push userbmo:jeremychen@mozilla.com
push dateWed, 20 Sep 2017 09:36:52 +0000
bugs1394284
milestone57.0a1
Bug 1394284 - 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. MozReview-Commit-ID: D1ObSj5kP5q
servo/components/style/properties/helpers/animated_properties.mako.rs
servo/ports/geckolib/glue.rs
--- a/servo/components/style/properties/helpers/animated_properties.mako.rs
+++ b/servo/components/style/properties/helpers/animated_properties.mako.rs
@@ -1448,21 +1448,20 @@ impl Animate for ComputedMatrix {
     fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> {
         if self.is_3d() || other.is_3d() {
             let decomposed_from = decompose_3d_matrix(*self);
             let decomposed_to = decompose_3d_matrix(*other);
             match (decomposed_from, decomposed_to) {
                 (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);
             let other = MatrixDecomposed2D::from(*other);
             Ok(ComputedMatrix::from(this.animate(&other, procedure)?))
         }
     }
 
@@ -1472,21 +1471,20 @@ impl Animate for ComputedMatrix {
             (decompose_3d_matrix(*self), decompose_3d_matrix(*other))
         } else {
             (decompose_2d_matrix(self), decompose_2d_matrix(other))
         };
         match (from, to) {
             (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(())
         }
     }
 }
 
 impl ComputeSquaredDistance for ComputedMatrix {
     #[inline]
     #[cfg(feature = "servo")]
     fn compute_squared_distance(&self, other: &Self) -> Result<SquaredDistance, ()> {
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -2200,19 +2200,23 @@ pub extern "C" fn Servo_MatrixTransform_
     let from = ComputedMatrix::from(unsafe { from.as_ref() }.expect("not a valid 'from' matrix"));
     let to = ComputedMatrix::from(unsafe { to.as_ref() }.expect("not a valid 'to' matrix"));
     let result = match matrix_operator {
         Interpolate => from.animate(&to, Procedure::Interpolate { progress }),
         Accumulate => from.animate(&to, Procedure::Accumulate { count: progress as u64 }),
     };
 
     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]
 pub extern "C" fn Servo_ParseStyleAttribute(data: *const nsACString,
                                             raw_extra_data: *mut URLExtraData,
                                             quirks_mode: nsCompatibility,
                                             loader: *mut Loader)
                                             -> RawServoDeclarationBlockStrong {