Fix for bug where composited rotate3d should SLERP

cc/animation/transform_operation.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Needed on Windows to get |M_PI| from <cmath>
 #ifdef _WIN32
 #define _USE_MATH_DEFINES
 #endif
 
 #include <algorithm>
@@ skipping 14 matching lines @@
 namespace cc {
 
 bool TransformOperation::IsIdentity() const {
   return matrix.IsIdentity();
 }
 
 static bool IsOperationIdentity(const TransformOperation* operation) {
   return !operation || operation->IsIdentity();
 }
 
+static bool Is3DRotation(const TransformOperation* operation) {
+  if (IsOperationIdentity(operation))
+    return false;
+  return operation->rotate.axis.x != 0 || operation->rotate.axis.y != 0;
+}
+
 static bool ShareSameAxis(const TransformOperation* from,
                           const TransformOperation* to,
                           SkMScalar* axis_x,
                           SkMScalar* axis_y,
                           SkMScalar* axis_z,
                           SkMScalar* angle_from) {
   if (IsOperationIdentity(from) && IsOperationIdentity(to))
     return false;
 
   if (IsOperationIdentity(from) && !IsOperationIdentity(to)) {
@@ skipping 72 matching lines @@
                         BlendSkMScalars(from_y, to_y, progress),
                         BlendSkMScalars(from_z, to_z, progress));
     break;
   }
   case TransformOperation::TRANSFORM_OPERATION_ROTATE: {
     SkMScalar axis_x = 0;
     SkMScalar axis_y = 0;
     SkMScalar axis_z = 1;
     SkMScalar from_angle = 0;
     SkMScalar to_angle = IsOperationIdentity(to) ? 0 : to->rotate.angle;
-    if (ShareSameAxis(from, to, &axis_x, &axis_y, &axis_z, &from_angle)) {
+    if (ShareSameAxis(from, to, &axis_x, &axis_y, &axis_z, &from_angle) &&
+        !Is3DRotation(from) && !Is3DRotation(to)) {

[ajuma, 2015/06/23 13:24:58]

I'm not sure I follow how this corresponds to the spec. This says that 3d
rotations should never be interpolated numerically, but the spec seems to say
that they should be interpolated numerically when they have the same normalized
direction vector. So maybe it's ShareSameAxis that needs fixing? Please add
tests (to transform_operations_unittest.cc) that show that we're interpolating
according to the spec in both cases (numerical interpolation for 3d rotations
with the same direction vector, and matrix interpolation when the direction
vectors are different).

[soonm, 2015/06/23 23:43:42]

Thanks ajuma, I think you're right. I misunderstood the spec, thinking that
rotate3d should always be interpolated with SLERP. With reference to the bug,
the error is on the main thread RotateTransformOperation.cpp because that does
not check the normalized axis correctly.

Closing this patch.

       result->RotateAbout(gfx::Vector3dF(axis_x, axis_y, axis_z),
                           BlendSkMScalars(from_angle, to_angle, progress));
     } else {
       gfx::Transform to_matrix;
       if (!IsOperationIdentity(to))
         to_matrix = to->matrix;
       gfx::Transform from_matrix;
       if (!IsOperationIdentity(from))
         from_matrix = from->matrix;
       *result = to_matrix;
@@ skipping 292 matching lines @@
       return true;
     }
     case TransformOperation::TRANSFORM_OPERATION_MATRIX:
       return false;
   }
   NOTREACHED();
   return false;
 }
 
 }  // namespace cc