Interpolated rotations should end cleanly where possible.

ui/gfx/interpolated_transform.cc

 // Copyright (c) 2012 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.
 
 #include "ui/gfx/interpolated_transform.h"
 
 #include <cmath>
 
 #ifndef M_PI
 #define M_PI 3.14159265358979323846
 #endif
 
 #include "base/logging.h"
 #include "ui/base/animation/tween.h"
 
 namespace {
 
 static const float EPSILON = 1e-6f;
 
+bool IsAMultipleOfNinetyDegrees(float degrees)

[James Cook, 2012/04/24 17:18:18]

nit: Maybe just IsMultipleOfNinetyDegrees()?

+{
+  float remainder = fabs(fmod(degrees, 90.0f));
+  return remainder < EPSILON || 90.0f - remainder < EPSILON;
+}
+
+// Returns false if |degrees| is not a multiple of ninety degrees or if
+// |rotation| is NULL. It does not affect |rotation| in this case. Otherwise
+// *rotation is set to be the appropriate sanitized rotation matrix. That is,
+// the rotation matrix corresponding to |degrees| which has entries that are all
+// either 0, 1 or -1.
+bool MassageRotationIfMultipleOfNinetyDegrees(ui::Transform* rotation,
+                                              float degrees)
+{
+  if (!IsAMultipleOfNinetyDegrees(degrees) || !rotation)
+    return false;
+
+  ui::Transform transform;
+  transform.SetRotate(degrees);
+  SkMatrix44& m = transform.matrix();
+
+  // A rotation that is a multiple of ninety degrees will have entries that are
+  // either 0, 1, or -1.
+  for (int row = 0; row < 4; ++row) {
+    for (int col = 0; col < 4; ++col) {
+      float entry = m.get(row, col);
+      if (fabs(entry) < EPSILON)

[piman, 2012/04/24 17:15:43]

I worry that we may miss this, especially for large values of degrees.
In https://code.google.com/p/chrome-os-partner/issues/detail?id=9166 there is an
error that is larger than your epsilon.

For a rotation of n*90 degrees, there's only 4 matrices:
1, 0
0, 1

0, 1
-1, 0

-1, 0
0, -1

0, -1
1, 0

Could we snap degrees/90 to the nearest integer, mod 4, and pick the exact
matrix?

+        entry = 0;
+      else if (fabs(entry + 1) < EPSILON)
+        entry = -1;
+      else if (fabs(entry - 1) < EPSILON)
+        entry = 1;
+      else
+        NOTREACHED();
+      m.set(row, col, entry);
+    }
+  }
+
+  *rotation = transform;
+  return true;
+}
+
 } // namespace
 
 namespace ui {
 
 ///////////////////////////////////////////////////////////////////////////////
 // InterpolatedTransform
 //
 
 InterpolatedTransform::InterpolatedTransform()
     : start_time_(0.0f),
@@ skipping 120 matching lines @@
                                            float end_time)
     : InterpolatedTransform(start_time, end_time),
       start_degrees_(start_degrees),
       end_degrees_(end_degrees) {
 }
 
 InterpolatedRotation::~InterpolatedRotation() {}
 
 ui::Transform InterpolatedRotation::InterpolateButDoNotCompose(float t) const {
   ui::Transform result;
-  result.SetRotate(ValueBetween(t, start_degrees_, end_degrees_));
+  float interpolated_degrees = ValueBetween(t, start_degrees_, end_degrees_);
+  result.SetRotate(interpolated_degrees);
+  MassageRotationIfMultipleOfNinetyDegrees(&result, interpolated_degrees);

[James Cook, 2012/04/24 17:18:18]

Do we want to always do this or just when t == 0 or t == 1?

   return result;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // InterpolatedAxisAngleRotation
 //
 
 InterpolatedAxisAngleRotation::InterpolatedAxisAngleRotation(
     gfx::Point3f axis,
     float start_degrees,
@@ skipping 90 matching lines @@
 }
 
 InterpolatedTranslation::~InterpolatedTranslation() {}
 
 ui::Transform
 InterpolatedTranslation::InterpolateButDoNotCompose(float t) const {
   ui::Transform result;
   // TODO(vollick) 3d xforms.
   result.SetTranslate(ValueBetween(t, start_pos_.x(), end_pos_.x()),
                       ValueBetween(t, start_pos_.y(), end_pos_.y()));
-
   return result;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // InterpolatedConstantTransform
 //
 
 InterpolatedConstantTransform::InterpolatedConstantTransform(
   const ui::Transform& transform)
     : InterpolatedTransform(),
@@ skipping 107 matching lines @@
 
     rotation->SetChild(translation.release());
     scale->SetChild(rotation.release());
     transform_.reset(scale.release());
   } else {
     transform_.reset(new InterpolatedConstantTransform(end_transform));
   }
 }
 
 } // namespace ui