Add Vector2d classes that represent offsets, instead of using Point.

ui/aura/test/event_generator.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/aura/test/event_generator.h"
 
 #include "base/memory/scoped_ptr.h"
 #include "ui/aura/root_window.h"
 #include "ui/base/events/event.h"
 
@@ skipping 108 matching lines @@
                            current_location_, flags_);
     Dispatch(mouseev);
     flags_ ^= ui::EF_RIGHT_MOUSE_BUTTON;
   }
 }
 
 void EventGenerator::MoveMouseTo(const gfx::Point& point, int count) {
   DCHECK_GT(count, 0);
   const ui::EventType event_type = (flags_ & ui::EF_LEFT_MOUSE_BUTTON) ?
       ui::ET_MOUSE_DRAGGED : ui::ET_MOUSE_MOVED;
-  const gfx::Point diff = point.Subtract(current_location_);
+  const gfx::Vector2d diff = point - current_location_;
   for (int i = 1; i <= count; i++) {
     const gfx::Point move_point = current_location_.Add(
-        gfx::Point(diff.x() / count * i, diff.y() / count * i));
+        gfx::Vector2d(diff.x() / count * i, diff.y() / count * i));

[Peter Kasting, 2012/10/30 01:14:14]

Do you think this suggests we should support multiplication and division
operators on vectors to scale them?

[danakj, 2012/10/30 19:21:21]

Yeh, that sounds nice. We have a Scale() method on them now. I wanted to change
this code minimally to avoid hard-to-spot bugs in such a large CL.

[Peter Kasting, 2012/10/30 20:24:37]

You're welcome to tack Scale() -> *,/ onto the list of operators to add on that
bug, then :)

As for the code here, it looks like it suffers from some pretty severe precision
issues.  It seems like we should at least upcast diff.x() and diff.y() to
floats.  If we're going to do that anyway, we might as well go ahead and just
call Scale().  Which points out that Scale() is missing on Vector2d.  Seems like
it should be there and should do the calculations in floating-point internally,
then round.

[danakj, 2012/10/30 21:14:52]

Actually, I'm less sure about this after trying it out. Suddenly you have points
and scalars all appearing in the same statement and it gets very unclear.

Vector2d v = (point1 - point2) * scalar1 / scalar2;

I was going to do here, but I think I'll use the Scale() method, and we can
revisit this later. Also, there is Scale(float) and Scale(float, float) so at
least one of the Scale() methods needs to stay, leading to inconsistency.
I strongly dislike implicit rounding from Float to Int. You should work with a
Vector2dF if you want to scale by a float. Then do an explicit round/floor/ceil
to get back an int.

I've switched this code to scaling vectors, and added vector2d_conversions.h/cc
to support it.

[Peter Kasting, 2012/10/30 21:41:23]

Commented on bug.  I'll note that having operator* for a uniform scaling and
Scale(float, float) for if you need to scale differently in different dimensions
seems like an OK disparity to me.  Although, much like with the setters and
Grow(), I'm a bit leery of allowing nonuniform scaling between dimensions. 
However, unlike with Grow(), I'm not sure how to fall back on something else if
code really needs to do that.  Does the compositor need this sort of nonuniform
scaling?

[danakj, 2012/10/30 21:49:10]

We do non-uniform scaling between different spaces, yes. For instance in
layer_tree_host_impl.cc:

localStartPoint.scale(widthScale, heightScale);

For image layers that are XxY and drawing into a space MxN, we need different
scales for each axis.

[Peter Kasting, 2012/10/30 21:56:35]

Sure, but is it vectors you need to scale, rather than points or rects?

Ah well, if you need it, keep it, and whether to use the operator for uniform
scaling is up to you.

[danakj, 2012/10/30 21:59:34]

I'm not entirely sure, since these types are going to all change. If we need to
scale a scroll delta between spaces then it'll be a vector. There is one such
scale in pinchGestureUpdate() in the same file for an IntSize, so I'd say it
seems likely. Maybe refactoring could make us only need to move end-points
around though?

     ui::MouseEvent mouseev(event_type, move_point, move_point, flags_);
     Dispatch(mouseev);
   }
   current_location_ = point;
 }
 
 void EventGenerator::MoveMouseRelativeTo(const Window* window,
                                          const gfx::Point& point) {
   gfx::Point root_point(point);
   Window::ConvertPointToTarget(window, root_window_, &root_point);
@@ skipping 192 matching lines @@
   TestKeyEvent keyev(native_event.get(), flags, false);
 #else
   ui::KeyEvent keyev(type, key_code, flags, false);
 #endif  // USE_X11
 #endif  // OS_WIN
   Dispatch(keyev);
 }
 
 }  // namespace test
 }  // namespace aura