Use SkMatrix44 for the underlying implementation of ui::Transform

ui/gfx/transform.cc

Index: ui/gfx/transform.cc
diff --git a/ui/gfx/transform.cc b/ui/gfx/transform.cc
index 03e2873d27c5ca1e795c3d4c5049854bf87c0c42..2a123584947f38f983bd0d138ff9601fda2beb70 100644
--- a/ui/gfx/transform.cc
+++ b/ui/gfx/transform.cc
@@ -10,105 +10,174 @@
 #include "ui/gfx/rect.h"
 #include "ui/gfx/skia_util.h"
 
+namespace {
+
+// Should be in a standard header
+static inline int SymmetricRound(float x) {
+  return static_cast<int>(
+    x > 0.0f
+      ? std::floor(x + 0.5f)
+      : std::ceil(x - 0.5f));
+}
+
+} // namespace
+
 namespace ui {
 
-Transform::Transform() {
+Transform::Transform()
+  : hasInverse_(false) {
   matrix_.reset();
+  inverse_.reset();
 }
 
 Transform::~Transform() {}
 
 void Transform::SetRotate(float degree) {
-  matrix_.setRotate(SkFloatToScalar(degree));
+  matrix_.setRotateDegreesAbout(0, 0, 1, SkFloatToScalar(degree));
+  hasInverse_ = false;
 }
 
 void Transform::SetScaleX(float x) {
-  matrix_.setScaleX(SkFloatToScalar(x));
+  matrix_.setScale(
+    SkFloatToScalar(x),
+    matrix_.get(1,1),
+    matrix_.get(2,2));
+  hasInverse_ = false;
 }
 
 void Transform::SetScaleY(float y) {
-  matrix_.setScaleY(SkFloatToScalar(y));
+  matrix_.setScale(
+    matrix_.get(0,0),
+    SkFloatToScalar(y),
+    matrix_.get(2,2));
+  hasInverse_ = false;
 }
 
 void Transform::SetScale(float x, float y) {
-  matrix_.setScale(SkFloatToScalar(x), SkFloatToScalar(y));
+  matrix_.setScale(
+    SkFloatToScalar(x),
+    SkFloatToScalar(y),
+    matrix_.get(2, 2));
+  hasInverse_ = false;
 }
 
 void Transform::SetTranslateX(float x) {
-  matrix_.setTranslateX(SkFloatToScalar(x));
+  matrix_.setTranslate(
+    SkFloatToScalar(x),
+    matrix_.get(1,3),
+    matrix_.get(2,3));
+  hasInverse_ = false;
 }
 
 void Transform::SetTranslateY(float y) {
-  matrix_.setTranslateY(SkFloatToScalar(y));
+  matrix_.setTranslate(
+    matrix_.get(0,3),
+    SkFloatToScalar(y),
+    matrix_.get(2,3));
+  hasInverse_ = false;
 }
 
 void Transform::SetTranslate(float x, float y) {
-  matrix_.setTranslate(SkFloatToScalar(x), SkFloatToScalar(y));
+  matrix_.setTranslate(
+    SkFloatToScalar(x),
+    SkFloatToScalar(y),
+    matrix_.get(2, 3));
+  hasInverse_ = false;
 }
 
 void Transform::ConcatRotate(float degree) {
-  matrix_.postRotate(SkFloatToScalar(degree));
+  SkMatrix44 rot;

[rjkroege, 2011/06/10 17:52:14]

rot()? Make it clear you're invoking the default constructor? Same comment
elsewhere.

+  rot.reset();
+  rot.setRotateDegreesAbout(0, 0, 1, SkFloatToScalar(degree));
+  matrix_.postConcat(rot);
+  hasInverse_ = false;
 }
 
 void Transform::ConcatScale(float x, float y) {
-  matrix_.postScale(SkFloatToScalar(x), SkFloatToScalar(y));
+  SkMatrix44 scale;
+  scale.reset();
+  scale.setScale(SkFloatToScalar(x), SkFloatToScalar(y), 1);
+  matrix_.postConcat(scale);
+  hasInverse_ = false;
 }
 
 void Transform::ConcatTranslate(float x, float y) {
-  matrix_.postTranslate(SkFloatToScalar(x), SkFloatToScalar(y));
+  SkMatrix44 translate;
+  translate.reset();
+  translate.setTranslate(SkFloatToScalar(x), SkFloatToScalar(y), 0);
+  matrix_.postConcat(translate);
+  hasInverse_ = false;
 }
 
-bool Transform::PreconcatTransform(const Transform& transform) {
-  return matrix_.setConcat(matrix_, transform.matrix_);
+void Transform::PreconcatTransform(const Transform& transform) {
+  if (!transform.matrix_.isIdentity()) {
+    matrix_.preConcat(transform.matrix_);
+    hasInverse_ = false;
+  }
 }
 
-bool Transform::ConcatTransform(const Transform& transform) {
-  return matrix_.setConcat(transform.matrix_, matrix_);
+void Transform::ConcatTransform(const Transform& transform) {
+  if (!transform.matrix_.isIdentity()) {
+    matrix_.postConcat(transform.matrix_);
+    hasInverse_ = false;
+  }
 }
 
 bool Transform::HasChange() const {
   return !matrix_.isIdentity();
 }
 
-bool Transform::TransformPoint(gfx::Point* point) {
-  SkPoint skp;
-  matrix_.mapXY(SkIntToScalar(point->x()), SkIntToScalar(point->y()), &skp);
-  point->SetPoint(static_cast<int>(std::floor(skp.fX)),
-                  static_cast<int>(std::floor(skp.fY)));
-  return true;
+void Transform::TransformPoint(gfx::Point* point) {
+  TransformPointInternal(matrix_, point);
 }
 
 bool Transform::TransformPointReverse(gfx::Point* point) {
-  SkMatrix inverse;
   // TODO(sad): Try to avoid trying to invert the matrix.
-  if (matrix_.invert(&inverse)) {
-    SkPoint skp;
-    inverse.mapXY(SkIntToScalar(point->x()), SkIntToScalar(point->y()), &skp);
-    point->SetPoint(static_cast<int>(std::floor(skp.fX)),
-                    static_cast<int>(std::floor(skp.fY)));
-    return true;
-  }
-  return false;
+  if (!ComputeInverse())
+    return false;
+
+  TransformPointInternal(inverse_, point);
+  return true;
 }
 
 bool Transform::TransformRect(gfx::Rect* rect) {
   SkRect src = gfx::RectToSkRect(*rect);
-  if (!matrix_.mapRect(&src))
+  SkMatrix matrix = matrix_;

[rjkroege, 2011/06/10 17:52:14]

does this invoke the copy constructor? does it need to? Is mapRect const? Please
adjust its cont-ness as necessary

+  if (!matrix.mapRect(&src))
     return false;
   *rect = gfx::SkRectToRect(src);
   return true;
 }
 
 bool Transform::TransformRectReverse(gfx::Rect* rect) {
-  SkMatrix inverse;
-  if (!matrix_.invert(&inverse))
+  if (!ComputeInverse())
     return false;
-
+  SkMatrix matrix = inverse_;
   SkRect src = gfx::RectToSkRect(*rect);
-  if (!inverse.mapRect(&src))
+  if (!matrix.mapRect(&src))
     return false;
   *rect = gfx::SkRectToRect(src);
   return true;
 }
 
+void Transform::TransformPointInternal(const SkMatrix44& xform,
+                                       gfx::Point* point) {
+  SkScalar p[4] = {
+    SkIntToScalar(point->x()),
+    SkIntToScalar(point->y()),
+    0,
+    1 };
+
+  xform.map(p);
+  point->SetPoint(SymmetricRound(p[0]),
+                  SymmetricRound(p[1]));
+}
+
+bool Transform::ComputeInverse() {
+  if (!hasInverse_) {
+    hasInverse_ = matrix_.invert(&inverse_);
+  }
+  return hasInverse_;
+}
+
 }  // namespace ui