From 0ef70d52f22a0f9a3398866a5db9447dd5407c3a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Thu, 3 May 2018 10:15:15 +0200
Subject: [PATCH] style: Fix perspective interpolation.

It's not sound to insert random matrices in random positions in the transform
operation list.

I cannot make any sense of what the old code was trying to do.

Bug: 1458715
Reviewed-by: hiro
MozReview-Commit-ID: 5BtCiueEPlR
---
 .../helpers/animated_properties.mako.rs       | 25 ++++---------------
 components/style/values/computed/transform.rs |  6 ++---
 2 files changed, 8 insertions(+), 23 deletions(-)

diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs
index 57a8ed99880..387507516e8 100644
--- a/components/style/properties/helpers/animated_properties.mako.rs
+++ b/components/style/properties/helpers/animated_properties.mako.rs
@@ -1291,16 +1291,8 @@ impl Animate for ComputedTransformOperation {
                 &TransformOperation::Perspective(ref fd),
                 &TransformOperation::Perspective(ref td),
             ) => {
-                let mut fd_matrix = Matrix3D::identity();
-                let mut td_matrix = Matrix3D::identity();
-                if fd.px() > 0. {
-                    fd_matrix.m34 = -1. / fd.px();
-                }
-                if td.px() > 0. {
-                    td_matrix.m34 = -1. / td.px();
-                }
-                Ok(TransformOperation::Matrix3D(
-                    fd_matrix.animate(&td_matrix, procedure)?,
+                Ok(TransformOperation::Perspective(
+                    fd.animate(td, procedure)?
                 ))
             },
             _ if self.is_translate() && other.is_translate() => {
@@ -2640,16 +2632,7 @@ impl ComputeSquaredDistance for ComputedTransformOperation {
                 &TransformOperation::Perspective(ref fd),
                 &TransformOperation::Perspective(ref td),
             ) => {
-                let mut fd_matrix = Matrix3D::identity();
-                let mut td_matrix = Matrix3D::identity();
-                if fd.px() > 0. {
-                    fd_matrix.m34 = -1. / fd.px();
-                }
-
-                if td.px() > 0. {
-                    td_matrix.m34 = -1. / td.px();
-                }
-                fd_matrix.compute_squared_distance(&td_matrix)
+                fd.compute_squared_distance(td)
             }
             (
                 &TransformOperation::Perspective(ref p),
@@ -2658,6 +2641,8 @@ impl ComputeSquaredDistance for ComputedTransformOperation {
                 &TransformOperation::Matrix3D(ref m),
                 &TransformOperation::Perspective(ref p),
             ) => {
+                // FIXME(emilio): Is this right? Why interpolating this with
+                // Perspective but not with anything else?
                 let mut p_matrix = Matrix3D::identity();
                 if p.px() > 0. {
                     p_matrix.m34 = -1. / p.px();
diff --git a/components/style/values/computed/transform.rs b/components/style/values/computed/transform.rs
index 38b45f83669..fd7a7cf5c2d 100644
--- a/components/style/values/computed/transform.rs
+++ b/components/style/values/computed/transform.rs
@@ -251,11 +251,11 @@ impl ToAnimatedZero for TransformOperation {
             generic::TransformOperation::Rotate(_) => {
                 Ok(generic::TransformOperation::Rotate(Angle::zero()))
             },
-            generic::TransformOperation::Perspective(..) |
+            generic::TransformOperation::Perspective(ref l) => {
+                Ok(generic::TransformOperation::Perspective(l.to_animated_zero()?))
+            },
             generic::TransformOperation::AccumulateMatrix { .. } |
             generic::TransformOperation::InterpolateMatrix { .. } => {
-                // Perspective: We convert a perspective function into an equivalent
-                //     ComputedMatrix, and then decompose/interpolate/recompose these matrices.
                 // AccumulateMatrix/InterpolateMatrix: We do interpolation on
                 //     AccumulateMatrix/InterpolateMatrix by reading it as a ComputedMatrix
                 //     (with layout information), and then do matrix interpolation.