Makes Transform concrete. Fixes bug in coordinate conversion and makes all conversion routines calculate the transform in the same way. Lastly fixes bug in touch_factory.cc that was causing crashes on my machine when running views_unittests. Oh, and adds some tests of conversion methods.

BUG=none
TEST=none
R=ben@chromium.org,sadrul@chromium.org

Review URL: http://codereview.chromium.org/7033002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@85635 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 142 insertions, 150 deletions

diff --git a/views/touchui/touch_factory.cc b/views/touchui/touch_factory.cc
index fefe35f..dcf88c8 100644
--- a/views/touchui/touch_factory.cc
+++ b/views/touchui/touch_factory.cc
@@ -179,10 +179,12 @@ void TouchFactory::UpdateDeviceList(Display* display) {
   touch_device_list_.clear();
   XDeviceInfo* devlist = XListInputDevices(display, &count);
   for (int i = 0; i < count; i++) {
-    const char* devtype = XGetAtomName(display, devlist[i].type);
-    if (devtype && !strcmp(devtype, XI_TOUCHSCREEN)) {
-      touch_device_lookup_[devlist[i].id] = true;
-      touch_device_list_.push_back(devlist[i].id);
+    if (devlist[i].type) {
+      const char* devtype = XGetAtomName(display, devlist[i].type);
+      if (devtype && !strcmp(devtype, XI_TOUCHSCREEN)) {
+        touch_device_lookup_[devlist[i].id] = true;
+        touch_device_list_.push_back(devlist[i].id);
+      }
     }
   }
   if (devlist)
diff --git a/views/view.cc b/views/view.cc
index 67ac560..9fdd27e 100644
--- a/views/view.cc
+++ b/views/view.cc
@@ -400,73 +400,30 @@ bool View::IsEnabled() const {
 // Transformations -------------------------------------------------------------
 
 const ui::Transform& View::GetTransform() const {
-  static const ui::Transform* no_op = ui::Transform::Create();
+  static const ui::Transform* no_op = new ui::Transform;
   if (transform_.get())
     return *transform_.get();
   return *no_op;
 }
 
-void View::SetRotation(float degree) {
-  InitTransform();
-  transform_->SetRotate(degree);
-}
-
-void View::SetScaleX(float x) {
-  InitTransform();
-  transform_->SetScaleX(x);
-}
-
-void View::SetScaleY(float y) {
-  InitTransform();
-  transform_->SetScaleY(y);
-}
-
-void View::SetScale(float x, float y) {
-  InitTransform();
-  transform_->SetScale(x, y);
-}
-
-void View::SetTranslateX(float x) {
-  InitTransform();
-  transform_->SetTranslateX(x);
-}
-
-void View::SetTranslateY(float y) {
-  InitTransform();
-  transform_->SetTranslateY(y);
-}
-
-void View::SetTranslate(float x, float y) {
-  InitTransform();
-  transform_->SetTranslate(x, y);
-}
-
-void View::ConcatRotation(float degree) {
-  InitTransform();
-  transform_->ConcatRotate(degree);
-}
-
-void View::ConcatScale(float x, float y) {
-  InitTransform();
-  transform_->ConcatScale(x, y);
-}
-
-void View::ConcatTranslate(float x, float y) {
-  InitTransform();
-  transform_->ConcatTranslate(x, y);
-}
-
-void View::ResetTransform() {
-  transform_.reset(NULL);
-  clip_x_ = clip_y_ = 0.0;
+void View::SetTransform(const ui::Transform& transform) {
+  if (!transform.HasChange()) {
+    if (!transform_.get())
+      return;
+    transform_.reset(NULL);
 #if !defined(COMPOSITOR_2)
-  canvas_.reset();
+    canvas_.reset();
 #else
-  texture_.reset();
+    texture_.reset();
 #endif
+    SchedulePaint();
+  } else {
+    transform_.reset(new ui::Transform(transform));
+    // TODO: this needs to trigger a paint on the widget. It shouldn't use
+    // SchedulePaint as we don't want to mark the views dirty.
+  }
 }
 
-
 // RTL positioning -------------------------------------------------------------
 
 gfx::Rect View::GetMirroredBounds() const {
@@ -711,6 +668,8 @@ void View::Paint(gfx::Canvas* canvas) {
 
     texture_canvas.reset(gfx::Canvas::CreateCanvas(dirty_rect.width(),
         dirty_rect.height(), false));
+    texture_canvas->AsCanvasSkia()->drawColor(
+        SK_ColorBLACK, SkXfermode::kClear_Mode);
     texture_canvas->TranslateInt(-dirty_rect.x(), -dirty_rect.y());
     canvas = texture_canvas.get();
     // TODO: set texture_needs_updating_ to false.
@@ -1201,9 +1160,9 @@ void View::PaintComposite() {
 
   if (texture_.get()) {
     // TODO: if dirty_region doesn't itersect bounds, return.
-    scoped_ptr<ui::Transform> transform(ui::Transform::Create());
-    GetTransformRelativeToRoot(transform.get());
-    texture_->Draw(*transform);
+    ui::Transform transform;
+    GetTransformRelativeTo(NULL, &transform);
+    texture_->Draw(transform);
   }
 
   for (int i = 0, count = child_count(); i < count; ++i)
@@ -1556,22 +1515,21 @@ void View::RemoveDescendantToNotify(View* view) {
 
 // Transformations -------------------------------------------------------------
 
-void View::InitTransform() {
-  if (!transform_.get())
-    transform_.reset(ui::Transform::Create());
-}
-
-void View::GetTransformRelativeToRoot(ui::Transform* transform) {
-  // TODO: the direction of transformation is likely wrong here.
+bool View::GetTransformRelativeTo(const View* ancestor,
+                                  ui::Transform* transform) const {
+  if (this == ancestor)
+    return true;
+  bool ret_value = false;
   if (parent_) {
-    parent_->GetTransformRelativeToRoot(transform);
+    ret_value = parent_->GetTransformRelativeTo(ancestor, transform);
   } else if (transform_.get()) {
-    transform->Copy(*transform_);
+    *transform = *transform_;
   }
   transform->ConcatTranslate(static_cast<float>(GetMirroredX()),
                              static_cast<float>(y()));
   if (transform_.get())
     transform->ConcatTransform(*transform_);
+  return ret_value;
 }
 
 // Coordinate conversion -------------------------------------------------------
@@ -1606,46 +1564,19 @@ void View::ConvertPointToView(const View* src,
 
 bool View::ConvertPointForAncestor(const View* ancestor,
                                    gfx::Point* point) const {
-  scoped_ptr<ui::Transform> trans(ui::Transform::Create());
-
+  ui::Transform trans;
   // TODO(sad): Have some way of caching the transformation results.
-
-  const View* v = this;
-  for (; v && v != ancestor; v = v->parent()) {
-    if (v->GetTransform().HasChange()) {
-      if (!trans->ConcatTransform(v->GetTransform()))
-        return false;
-    }
-    trans->ConcatTranslate(static_cast<float>(v->GetMirroredX()),
-                           static_cast<float>(v->y()));
-  }
-
-  if (trans->HasChange()) {
-    trans->TransformPoint(point);
-  }
-
-  return v == ancestor;
+  bool result = GetTransformRelativeTo(ancestor, &trans);
+  trans.TransformPoint(point);
+  return result;
 }
 
 bool View::ConvertPointFromAncestor(const View* ancestor,
                                     gfx::Point* point) const {
-  scoped_ptr<ui::Transform> trans(ui::Transform::Create());
-
-  const View* v = this;
-  for (; v && v != ancestor; v = v->parent()) {
-    if (v->GetTransform().HasChange()) {
-      if (!trans->ConcatTransform(v->GetTransform()))
-        return false;
-    }
-    trans->ConcatTranslate(static_cast<float>(v->GetMirroredX()),
-                           static_cast<float>(v->y()));
-  }
-
-  if (trans->HasChange()) {
-    trans->TransformPointReverse(point);
-  }
-
-  return v == ancestor;
+  ui::Transform trans;
+  bool result = GetTransformRelativeTo(ancestor, &trans);
+  trans.TransformPointReverse(point);
+  return result;
 }
 
 // Accelerated painting --------------------------------------------------------
diff --git a/views/view.h b/views/view.h
index 98337a5..ee979d6 100644
--- a/views/view.h
+++ b/views/view.h
@@ -348,24 +348,8 @@ class View : public AcceleratorTarget {
   void set_clip_y(float y) { clip_y_ = y; }
   void set_clip(float x, float y) { clip_x_ = x; clip_y_ = y; }
 
-  void SetRotation(float degree);
-
-  void SetScaleX(float x);
-  void SetScaleY(float y);
-  void SetScale(float x, float y);
-
-  void SetTranslateX(float x);
-  void SetTranslateY(float y);
-  void SetTranslate(float x, float y);
-
-  // The following functions apply the transformations on top of the existing
-  // transform.
-  void ConcatRotation(float degree);
-  void ConcatScale(float x, float y);
-  void ConcatTranslate(float x, float y);
-
-  // Reset the transformation matrix.
-  void ResetTransform();
+  // Sets the transform to the supplied transform.
+  void SetTransform(const ui::Transform& transform);
 
   // RTL positioning -----------------------------------------------------------
 
@@ -1234,12 +1218,11 @@ class View : public AcceleratorTarget {
 
   // Transformations -----------------------------------------------------------
 
-  // Initialize the transform matrix when necessary.
-  void InitTransform();
-
-  // Returns in |transform| the transform to get from root view coordinates to
-  // this views coordinates.
-  void GetTransformRelativeToRoot(ui::Transform* transform);
+  // Returns in |transform| the transform to get from coordinates of |ancestor|
+  // to this. Returns true if |ancestor| is found. If |ancestor| is not found,
+  // or NULL, |transform| is set to convert from root view coordinates to this.
+  bool GetTransformRelativeTo(const View* ancestor,
+                              ui::Transform* transform) const;
 
   // Coordinate conversion -----------------------------------------------------
 
diff --git a/views/view_unittest.cc b/views/view_unittest.cc
index 97d32fa..f98f5b9 100644
--- a/views/view_unittest.cc
+++ b/views/view_unittest.cc
@@ -12,6 +12,7 @@
 #include "ui/base/models/simple_menu_model.h"
 #include "ui/gfx/canvas_skia.h"
 #include "ui/gfx/path.h"
+#include "ui/gfx/transform.h"
 #include "views/background.h"
 #include "views/controls/button/button_dropdown.h"
 #include "views/controls/button/checkbox.h"
@@ -1638,8 +1639,10 @@ TEST_F(ViewTest, TransformPaint) {
   EXPECT_EQ(gfx::Rect(100, 100, 200, 100), v1->scheduled_paint_rect());
 
   // Rotate |v1| counter-clockwise.
-  v1->SetRotation(-90.0);
-  v1->SetTranslateY(500);
+  ui::Transform transform;
+  transform.SetRotate(-90.0f);
+  transform.SetTranslateY(500.0f);
+  v1->SetTransform(transform);
 
   // |v2| now occupies (100, 200) to (200, 400) in |root|.
 
@@ -1670,8 +1673,10 @@ TEST_F(ViewTest, TransformEvent) {
   // At this moment, |v2| occupies (100, 100) to (300, 200) in |root|.
 
   // Rotate |v1| counter-clockwise.
-  v1->SetRotation(-90.0);
-  v1->SetTranslateY(500);
+  ui::Transform transform(v1->GetTransform());
+  transform.SetRotate(-90.0f);
+  transform.SetTranslateY(500.0f);
+  v1->SetTransform(transform);
 
   // |v2| now occupies (100, 200) to (200, 400) in |root|.
   v1->Reset();
@@ -1690,8 +1695,10 @@ TEST_F(ViewTest, TransformEvent) {
   root->OnMouseReleased(released);
 
   // Now rotate |v2| inside |v1| clockwise.
-  v2->SetRotation(90.0);
-  v2->SetTranslateX(100);
+  transform = v2->GetTransform();
+  transform.SetRotate(90.0f);
+  transform.SetTranslateX(100.0f);
+  v2->SetTransform(transform);
 
   // Now, |v2| occupies (100, 100) to (200, 300) in |v1|, and (100, 300) to
   // (300, 400) in |root|.
@@ -1710,19 +1717,23 @@ TEST_F(ViewTest, TransformEvent) {
 
   root->OnMouseReleased(released);
 
-  v1->ResetTransform();
-  v2->ResetTransform();
+  v1->SetTransform(ui::Transform());
+  v2->SetTransform(ui::Transform());
 
   TestView* v3 = new TestView();
   v3->SetBounds(10, 10, 20, 30);
   v2->AddChildView(v3);
 
   // Rotate |v3| clockwise with respect to |v2|.
-  v3->SetRotation(90.0);
-  v3->SetTranslateX(30);
+  transform = v1->GetTransform();
+  transform.SetRotate(90.0f);
+  transform.SetTranslateX(30.0f);
+  v3->SetTransform(transform);
 
   // Scale |v2| with respect to |v1| along both axis.
-  v2->SetScale(0.8f, 0.5f);
+  transform = v2->GetTransform();
+  transform.SetScale(0.8f, 0.5f);
+  v2->SetTransform(transform);
 
   // |v3| occupies (108, 105) to (132, 115) in |root|.
 
@@ -1741,24 +1752,28 @@ TEST_F(ViewTest, TransformEvent) {
 
   root->OnMouseReleased(released);
 
-  v1->ResetTransform();
-  v2->ResetTransform();
-  v3->ResetTransform();
+  v1->SetTransform(ui::Transform());
+  v2->SetTransform(ui::Transform());
+  v3->SetTransform(ui::Transform());
 
   v1->Reset();
   v2->Reset();
   v3->Reset();
 
   // Rotate |v3| clockwise with respect to |v2|, and scale it along both axis.
-  v3->SetRotation(90.0);
-  v3->SetTranslateX(30);
+  transform = v3->GetTransform();
+  transform.SetRotate(90.0f);
+  transform.SetTranslateX(30.0f);
   // Rotation sets some scaling transformation. Using SetScale would overwrite
   // that and pollute the rotation. So combine the scaling with the existing
   // transforamtion.
-  v3->ConcatScale(0.8f, 0.5f);
+  transform.ConcatScale(0.8f, 0.5f);
+  v3->SetTransform(transform);
 
   // Translate |v2| with respect to |v1|.
-  v2->SetTranslate(10, 10);
+  transform = v2->GetTransform();
+  transform.SetTranslate(10, 10);
+  v2->SetTransform(transform);
 
   // |v3| now occupies (120, 120) to (144, 130) in |root|.
 
@@ -1875,6 +1890,67 @@ TEST_F(ViewTest, SetBoundsPaint) {
   EXPECT_EQ(gfx::Rect(10, 10, 40, 40), paint_rect);
 }
 
+// Tests conversion methods with a transform.
+TEST_F(ViewTest, ConvertPointToViewWithTransform) {
+  TestView top_view;
+  TestView* child = new TestView;
+  TestView* child_child = new TestView;
+
+  top_view.AddChildView(child);
+  child->AddChildView(child_child);
+
+  top_view.SetBounds(0, 0, 1000, 1000);
+
+  child->SetBounds(10, 10, 500, 500);
+  ui::Transform transform;
+  transform.SetScale(5.0f, 5.0f);
+  child->SetTransform(transform);
+
+  child_child->SetBounds(10, 10, 100, 100);
+  transform = ui::Transform();
+  transform.SetScale(2.0f, 2.0f);
+  child_child->SetTransform(transform);
+
+  // Conversions from child->top and top->child.
+  {
+    gfx::Point point(5, 5);
+    View::ConvertPointToView(child, &top_view, &point);
+    EXPECT_EQ(35, point.x());
+    EXPECT_EQ(35, point.y());
+
+    point.SetPoint(35, 35);
+    View::ConvertPointToView(&top_view, child, &point);
+    EXPECT_EQ(5, point.x());
+    EXPECT_EQ(5, point.y());
+  }
+
+  // Conversions from child_child->top and top->child_child.
+  {
+    gfx::Point point(5, 5);
+    View::ConvertPointToView(child_child, &top_view, &point);
+    EXPECT_EQ(110, point.x());
+    EXPECT_EQ(110, point.y());
+
+    point.SetPoint(110, 110);
+    View::ConvertPointToView(&top_view, child_child, &point);
+    EXPECT_EQ(5, point.x());
+    EXPECT_EQ(5, point.y());
+  }
+
+  // Conversions from child_child->child and child->child_child
+  {
+    gfx::Point point(5, 5);
+    View::ConvertPointToView(child_child, child, &point);
+    EXPECT_EQ(20, point.x());
+    EXPECT_EQ(20, point.y());
+
+    point.SetPoint(20, 20);
+    View::ConvertPointToView(child, child_child, &point);
+    EXPECT_EQ(5, point.x());
+    EXPECT_EQ(5, point.y());
+  }
+}
+
 TEST_F(ViewTest, Contains) {
   TestView v1;
   TestView* v2 = new TestView();