Implement unit tests and temporary MathUtil wrappers for transform functionality

cc/math_util.cc

Index: cc/math_util.cc
diff --git a/cc/math_util.cc b/cc/math_util.cc
index 606f67bf482b34e7ffbb9cdf35ffb2d039ccb401..091a5e99df420ac5cddb23de2a7fdc5b87880860 100644
--- a/cc/math_util.cc
+++ b/cc/math_util.cc
@@ -395,4 +395,165 @@ gfx::Vector2dF MathUtil::projectVector(gfx::Vector2dF source, gfx::Vector2dF des
     return gfx::Vector2dF(projectedLength * destination.x(), projectedLength * destination.y());
 }
 
+bool MathUtil::isInvertible(const gfx::Transform& transform)
+{
+    // TODO (shawnsingh): improve this function. We don't need to perform a
+    // full matrix inversion when the matrix was actually invertible.

[Ian Vollick, 2012/11/16 13:44:46]

Now that SkMatrix44::determinant is public, would could easily write an
efficient gfx::Transform::IsInvertible().

+    gfx::Transform inverse;
+    bool invertible = transform.GetInverse(&inverse);
+    return invertible;
+}
+
+bool MathUtil::isBackFaceVisible(const gfx::Transform&)
+{
+    DCHECK(false); // NOT YET IMPLEMENTED!!
+    return false;
+}
+
+bool MathUtil::isIdentity(const gfx::Transform& transform)
+{
+    return transform.matrix().isIdentity();
+}
+
+bool MathUtil::isIdentityOrTranslation(const gfx::Transform& transform)
+{
+    const SkMatrix44& matrix = transform.matrix();
+

[Ian Vollick, 2012/11/16 13:44:46]

SkMatrix44 now has getDouble for accessing matrix elements as doubles rather
than get(..) which returns SkMScalars. Perhaps those would be more appropriate
here? Same comment goes for the other get(..)'s in this file.

+    bool hasNoPerspective = !matrix.get(3, 0) && !matrix.get(3, 1) && !matrix.get(3, 2) && (matrix.get(3, 3) == 1);
+    bool hasNoRotationOrSkew = !matrix.get(0, 1) && !matrix.get(0, 2) && !matrix.get(1, 0) &&
+        !matrix.get(1, 2) && !matrix.get(2, 0) && !matrix.get(2, 1);
+    bool hasNoScale = matrix.get(0, 0) == 1
+        && matrix.get(1, 1) == 1
+        && matrix.get(2, 2) == 1;
+
+    return hasNoPerspective && hasNoRotationOrSkew && hasNoScale;
+}
+
+bool MathUtil::hasPerspective(const gfx::Transform& transform)
+{
+    // Mathematically it is a bit too strict to expect the 4th element to be
+    // equal to 1. However, the only non-perspective case where this element
+    // becomes non-1 is when it was explicitly initialized. In that case it
+    // still causes us to have a nontrivial divide-by-w, so we count it as
+    // being perspective here.
+    const SkMatrix44& matrix = transform.matrix();
+    return matrix.get(3, 0) || matrix.get(3, 1) || matrix.get(3, 2) || (matrix.get(3, 3) != 1);
+}
+
+void MathUtil::makeIdentity(gfx::Transform* transform)
+{
+    transform->matrix().setIdentity();
+}
+
+void MathUtil::rotateEulerAngles(gfx::Transform* transform, float eulerX, float eulerY, float eulerZ)
+{
+    // TODO (shawnsingh): make this implementation faster and more accurate by
+    // hard-coding each matrix instead of calling rotateAxisAngle().

[Ian Vollick, 2012/11/16 13:44:46]

Actually, it would be nice if SkMatrix44 noticed when you passed it one of the
canonical basis vectors and did something clever so you didn't have to do
something smart here. That said, it would be nice if we could write out each of
these matrices and multiply them symbolically. We could probably construct their
product directly rather than multiplying these three components.

[shawnsingh, 2012/11/17 00:38:31]

I think I understand you're saying that we should make skia recognize the
special degenerate axes.  But I don't understand the second half of what you're
saying?   are you suggesting we should construct the product of 3 or 4 matrices
by computing each element manually rather than using the multiplifaction
operator?

+    gfx::Transform rotationAboutX;
+    gfx::Transform rotationAboutY;
+    gfx::Transform rotationAboutZ;
+
+    MathUtil::rotateAxisAngle(&rotationAboutX, 1, 0, 0, eulerX);
+    MathUtil::rotateAxisAngle(&rotationAboutY, 0, 1, 0, eulerY);
+    MathUtil::rotateAxisAngle(&rotationAboutZ, 0, 0, 1, eulerZ);
+
+    gfx::Transform composite = rotationAboutZ * rotationAboutY * rotationAboutX;
+    transform->PreconcatTransform(composite);
+}
+
+void MathUtil::rotateAxisAngle(gfx::Transform* transform, float i, float j, float k, float degrees)
+{
+    // TODO (shawnsingh): fix gfx::Transform API to receive vector instead of
+    // point for the axis.

[Ian Vollick, 2012/11/16 13:44:46]

This is a good point, but this would ideally be a vector backed by doubles. It
would also be nice if we could construct vectors and points that are backed by
doubles in general. This would simplify much of the code in transform_util which
exists simply because we don't have these classes.

[shawnsingh, 2012/11/17 00:38:31]

Sounds like musings for follow-up patch, right?  aside from making these values
double for now, seems like this is the way we can do it for now.  Please let me
know if you intended me to change this code somehow, other than float-->double
which is done now.

+    gfx::Point3F axis(i, j, k);
+    transform->PreconcatRotateAbout(axis, degrees);
+}
+
+gfx::Transform MathUtil::inverse(const gfx::Transform& transform)
+{
+    gfx::Transform result;
+    transform.GetInverse(&result);

[Ian Vollick, 2012/11/16 13:44:46]

GetInverse should be decorated with WARN_UNUSED_RESULT -- this code really
shouldn't compile.

+    return result;
+}
+
+gfx::Transform MathUtil::to2dTransform(const gfx::Transform& transform)
+{
+    gfx::Transform result = transform;
+    SkMatrix44& matrix = result.matrix();
+    matrix.set(0, 2, 0);

[Ian Vollick, 2012/11/16 13:44:46]

setDouble

+    matrix.set(1, 2, 0);
+    matrix.set(2, 2, 1);
+    matrix.set(3, 2, 0);
+
+    matrix.set(2, 0, 0);
+    matrix.set(2, 1, 0);
+    matrix.set(2, 3, 0);
+
+    return result;
+}
+
+gfx::Transform MathUtil::toGfxTransform(const WebKit::WebTransformationMatrix&)
+{
+    DCHECK(false); // NOT YET IMPLEMENTED!!
+    return gfx::Transform();
+}
+
+gfx::Transform MathUtil::createGfxTransform(float m11, float m12, float m13, float m14,
+                                            float m21, float m22, float m23, float m24,
+                                            float m31, float m32, float m33, float m34,
+                                            float m41, float m42, float m43, float m44)
+{
+    gfx::Transform result;
+    SkMatrix44& matrix = result.matrix();
+
+    // Initialize column 1
+    matrix.set(0, 0, m11);

[Ian Vollick, 2012/11/16 13:44:46]

setDouble.

+    matrix.set(1, 0, m12);
+    matrix.set(2, 0, m13);
+    matrix.set(3, 0, m14);
+
+    // Initialize column 2
+    matrix.set(0, 1, m21);
+    matrix.set(1, 1, m22);
+    matrix.set(2, 1, m23);
+    matrix.set(3, 1, m24);
+
+    // Initialize column 2

[Ian Vollick, 2012/11/16 13:44:46]

Copy pasta -- column 3. Here and below.

+    matrix.set(0, 2, m31);
+    matrix.set(1, 2, m32);
+    matrix.set(2, 2, m33);
+    matrix.set(3, 2, m34);
+
+    // Initialize column 2
+    matrix.set(0, 3, m41);
+    matrix.set(1, 3, m42);
+    matrix.set(2, 3, m43);
+    matrix.set(3, 3, m44);
+
+    return result;
+}
+
+gfx::Transform MathUtil::createGfxTransform(float a, float b, float c,
+                                            float d, float e, float f)
+{
+    gfx::Transform result;
+    SkMatrix44& matrix = result.matrix();
+    matrix.set(0, 0, a);

[Ian Vollick, 2012/11/16 13:44:46]

setDouble

+    matrix.set(1, 0, b);
+    matrix.set(0, 1, c);
+    matrix.set(1, 1, d);
+    matrix.set(0, 3, e);
+    matrix.set(1, 3, f);
+
+    return result;
+}
+
+gfx::Transform operator*(const gfx::Transform& A, const gfx::Transform& B)
+{
+    // Compute A * B.
+    gfx::Transform result = A;
+    result.PreconcatTransform(B);
+    return result;
+}
+
 }  // namespace cc