Explicitly convert Point to PointF for event code

Explicitly convert Point to PointF for event code

Implicit conversions are going away, so explicitly
pass PointF to things that want a PointF.

BUG=342848

[danakj, 2015-10-22 00:32:43]

Description was changed from

==========
pointfconvert-prod

BUG=
==========

to

==========
pointfconvert-prod

BUG=342848
==========

[danakj, 2015-10-22 21:53:40]

Description was changed from

==========
pointfconvert-prod

BUG=342848
==========

to

==========
Explicitly convert Point to PointF for event code

Implicit conversions are going away, so explicitly
pass PointF to things that want a PointF.

BUG=342848
==========

[danakj, 2015-10-22 21:53:41]

danakj@chromium.org changed reviewers:
+ sky@chromium.org

[danakj, 2015-10-22 21:53:46]

Patchset #2 (id:20001) has been deleted

[danakj, 2015-10-22 21:53:51]

Patchset #1 (id:1) has been deleted

[danakj, 2015-10-22 21:55:24]

So now we finally get to "pass PointFs to events". In production there's not
that many callsites that are this way. We discussed maybe adding a static
factory method for events, but then we'd have to change all these same places to
call that instead of the constructor. Just casting to PointF when needed seemed
simple enough to understand and hopefully this is easy to review now.

[sky, 2015-10-22 23:12:54]

I'm curious. Do you have a list of the call sites that actually supply
a float point that comes from floating point values?

On Thu, Oct 22, 2015 at 2:55 PM,  <danakj@chromium.org> wrote:
> So now we finally get to "pass PointFs to events". In production there's not
> that many callsites that are this way. We discussed maybe adding a static
> factory method for events, but then we'd have to change all these same
> places to
> call that instead of the constructor. Just casting to PointF when needed
> seemed
> simple enough to understand and hopefully this is easy to review now.
>
> https://codereview.chromium.org/1421713002/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[danakj, 2015-10-22 23:34:56]

On 2015/10/22 23:12:54, sky wrote:
> I'm curious. Do you have a list of the call sites that actually supply
> a float point that comes from floating point values?

Good question! There's fewer places though non-zero implies the float part is
important? I tried changing the constructor to take a Point and fish out errors,
these are the ones in the linux desktop chrome target:

https://code.google.com/p/chromium/codesearch#chromium/src/ui/events/gestures...
https://code.google.com/p/chromium/codesearch#chromium/src/ui/chromeos/touch_...
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...
(This one drops the original float part when it does the math I noticed, it
doesn't use location_f()).

https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...

[danakj, 2015-10-22 23:37:12]

On 2015/10/22 23:34:56, danakj wrote:
> these are the ones in the linux desktop chrome target:

Oops, that was on a chromeos build of course. :)

[danakj, 2015-10-22 23:39:41]

On 2015/10/22 23:37:12, danakj wrote:
> On 2015/10/22 23:34:56, danakj wrote:
> > these are the ones in the linux desktop chrome target:
> 
> Oops, that was on a chromeos build of course. :)

Oh ya, I left out GestureEvent in that, but it takes 2 floats not a PointF and
changing it to 2 ints the compiler just silently drops the floats to ints in
those targets. :| So that's harder to find.

[sky, 2015-10-23 21:13:13]

https://codereview.chromium.org/1421713002/diff/40001/ash/drag_drop/drag_drop...
File ash/drag_drop/drag_drop_controller.cc (right):

https://codereview.chromium.org/1421713002/diff/40001/ash/drag_drop/drag_drop...
ash/drag_drop/drag_drop_controller.cc:54: gfx::PointF
final_origin(drag_image_bounds.origin());
You don't need the floating point until the RectF constructor below. So, keep
this a Point until then.

https://codereview.chromium.org/1421713002/diff/40001/ash/host/ash_window_tre...
File ash/host/ash_window_tree_host.cc (right):

https://codereview.chromium.org/1421713002/diff/40001/ash/host/ash_window_tre...
ash/host/ash_window_tree_host.cc:28: if (screen_position_client &&
!local.Contains(event->location())) {
I got to hear and got sad that in most cases we effectively drop the floating
point values. I guess this is no surprise given my comment that views and aura
is integer based. I sent an email asking if we could nuke the floating point
values from LocatedEvent. We'll see where that goes.

[danakj, 2015-10-23 21:46:11]

I looked through for more of similar things and changed them to defer casting
where it looked like it made sense based on your request. PTAL

https://codereview.chromium.org/1421713002/diff/40001/ash/drag_drop/drag_drop...
File ash/drag_drop/drag_drop_controller.cc (right):

https://codereview.chromium.org/1421713002/diff/40001/ash/drag_drop/drag_drop...
ash/drag_drop/drag_drop_controller.cc:54: gfx::PointF
final_origin(drag_image_bounds.origin());
On 2015/10/23 21:13:13, sky wrote:
> You don't need the floating point until the RectF constructor below. So, keep
> this a Point until then.

Done.

https://codereview.chromium.org/1421713002/diff/40001/ash/drag_drop/drag_drop...
ash/drag_drop/drag_drop_controller.cc:59: float total_x_offset =
drag_image_offset->x();
made these ints too

https://codereview.chromium.org/1421713002/diff/40001/ui/events/test/event_ge...
File ui/events/test/event_generator.cc (right):

https://codereview.chromium.org/1421713002/diff/40001/ui/events/test/event_ge...
ui/events/test/event_generator.cc:365: gfx::PointF location(start);
I could cast |start| in the TouchEvent directly, but this variable already
exists (and needs to be float) down below, so moving up here seems ok?

https://codereview.chromium.org/1421713002/diff/40001/ui/views/controls/textf...
File ui/views/controls/textfield/textfield.cc (right):

https://codereview.chromium.org/1421713002/diff/40001/ui/views/controls/textf...
ui/views/controls/textfield/textfield.cc:1157: gfx::RectF
r1(render_text->GetCursorBounds(start_sel, true));
This one is about SelectionBounds, which are floats. I could cast in the SetEdge
instead here too, left it for now.