Interpolated rotations should end cleanly where possible.

ui/gfx/interpolated_transform.cc

Index: ui/gfx/interpolated_transform.cc
diff --git a/ui/gfx/interpolated_transform.cc b/ui/gfx/interpolated_transform.cc
index 6989fdaff2528e330489beb6fc18f4d1cdb46e47..9209809c1db0ae2de75be6c8fd3f0bdee036ab7d 100644
--- a/ui/gfx/interpolated_transform.cc
+++ b/ui/gfx/interpolated_transform.cc
@@ -17,6 +17,48 @@ 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 {
@@ -157,7 +199,9 @@ 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;
 }
 
@@ -268,7 +312,6 @@ InterpolatedTranslation::InterpolateButDoNotCompose(float t) const {
   // TODO(vollick) 3d xforms.
   result.SetTranslate(ValueBetween(t, start_pos_.x(), end_pos_.x()),
                       ValueBetween(t, start_pos_.y(), end_pos_.y()));
-
   return result;
 }