Remove ConvertPointToPixel() methods.

The Convert*ToDIP() and Convert*ToPixel() functions are problematic in
that they hide what decision is being taken in the underlying code
when moving from floating point to integer, and consumers should be
thinking about what they want for correctness. They also act in
inconsistent ways when converting a Rect vs its components. This
removes the functions that perform float->int conversions in order to
have callers show what they intend to happen and make it clear that
data is being lost in some fashion.

R=sky@chromium.org

Bug: 1130050
Change-Id: I2bbe5ff2b85cc4c643ec66fe09a8208e643c3329
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2418568
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809815}

ash/magnifier/magnification_controller.h

@@ -172,9 +172,7 @@ class ASH_EXPORT MagnificationController : public ui::EventHandler,
   // given scale. Returns true if the window is changed; otherwise, false.
   // These methods should be called internally just after the scale and/or
   // the position are changed to redraw the window.
-  bool Redraw(const gfx::PointF& position_in_physical_pixels,
-              float scale,
-              bool animate);
+  bool Redraw(const gfx::PointF& position_in_pixels, float scale, bool animate);
 
   // Redraws the magnification window with the given origin position in dip and
   // the given scale. Returns true if the window is changed; otherwise, false.

content/browser/renderer_host/render_widget_host_input_event_router.cc

@@ -556,7 +556,7 @@ RenderWidgetTargetResult RenderWidgetHostInputEventRouter::FindViewAtLocation(
   float device_scale_factor = root_view->GetDeviceScaleFactor();
   DCHECK_GT(device_scale_factor, 0.0f);
   gfx::PointF point_in_pixels =
-      gfx::ConvertPointToPixel(device_scale_factor, point);
+      gfx::ConvertPointToPixels(point, device_scale_factor);
   viz::Target target = query->FindTargetForLocationStartingFrom(
       source, point_in_pixels, root_view->GetFrameSinkId());
   frame_sink_id = target.frame_sink_id;

content/browser/renderer_host/render_widget_host_view_base.cc

@@ -793,8 +793,8 @@ bool RenderWidgetHostViewBase::TransformPointToTargetCoordSpace(
 
   float device_scale_factor = original_view->GetDeviceScaleFactor();
   DCHECK_GT(device_scale_factor, 0.0f);
-  gfx::Point3F point_in_pixels(
-      gfx::ConvertPointToPixel(device_scale_factor, point));
+  gfx::Point3F point_in_pixels =
+      gfx::Point3F(gfx::ConvertPointToPixels(point, device_scale_factor));
   // TODO(crbug.com/966995): Optimize so that |point_in_pixels| doesn't need to
   // be in the coordinate space of the root surface in HitTestQuery.
   gfx::Transform transform_root_to_original;

ui/compositor/dip_util.cc

@@ -33,11 +33,6 @@ gfx::Rect ConvertRectToDIP(const Layer* layer,
   return gfx::ConvertRectToDIP(layer->device_scale_factor(), rect_in_pixel);
 }
 
-gfx::Point ConvertPointToPixel(const Layer* layer,
-                               const gfx::Point& point_in_dip) {
-  return gfx::ConvertPointToPixel(layer->device_scale_factor(), point_in_dip);
-}
-
 gfx::Size ConvertSizeToPixel(const Layer* layer,
                              const gfx::Size& size_in_dip) {
   return gfx::ConvertSizeToPixel(layer->device_scale_factor(), size_in_dip);

ui/compositor/dip_util.h

@@ -9,7 +9,6 @@
 #include "ui/gfx/geometry/point_f.h"
 
 namespace gfx {
-class Point;
 class Size;
 class Rect;
 }  // namespace gfx
@@ -25,9 +24,6 @@ COMPOSITOR_EXPORT gfx::Size ConvertSizeToDIP(
 COMPOSITOR_EXPORT gfx::Rect ConvertRectToDIP(
     const Layer* layer,
     const gfx::Rect& rect_in_pixel);
-COMPOSITOR_EXPORT gfx::Point ConvertPointToPixel(
-    const Layer* layer,
-    const gfx::Point& point_in_dip);
 COMPOSITOR_EXPORT gfx::Size ConvertSizeToPixel(
     const Layer* layer,
     const gfx::Size& size_in_dip);

ui/gfx/geometry/dip_util.cc

@@ -6,7 +6,6 @@
 
 #include "ui/gfx/geometry/insets.h"
 #include "ui/gfx/geometry/point.h"
-#include "ui/gfx/geometry/point_conversions.h"
 #include "ui/gfx/geometry/point_f.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/rect_conversions.h"
@@ -25,6 +24,16 @@ PointF ConvertPointToDips(const PointF& point_in_pixels,
   return ScalePoint(point_in_pixels, 1.f / device_scale_factor);
 }
 
+PointF ConvertPointToPixels(const Point& point_in_dips,
+                            float device_scale_factor) {
+  return ScalePoint(PointF(point_in_dips), device_scale_factor);
+}
+
+PointF ConvertPointToPixels(const PointF& point_in_dips,
+                            float device_scale_factor) {
+  return ScalePoint(point_in_dips, device_scale_factor);
+}
+
 Insets ConvertInsetsToDIP(float scale_factor,
                           const gfx::Insets& insets_in_pixel) {
   if (scale_factor == 1.f)
@@ -52,18 +61,6 @@ Insets ConvertInsetsToPixel(float scale_factor,
   return insets_in_dip.Scale(scale_factor);
 }
 
-Point ConvertPointToPixel(float scale_factor, const Point& point_in_dip) {
-  if (scale_factor == 1.f)
-    return point_in_dip;
-  return ScaleToFlooredPoint(point_in_dip, scale_factor);
-}
-
-PointF ConvertPointToPixel(float scale_factor, const PointF& point_in_dip) {
-  if (scale_factor == 1.f)
-    return point_in_dip;
-  return ScalePoint(point_in_dip, scale_factor);
-}
-
 Size ConvertSizeToPixel(float scale_factor, const Size& size_in_dip) {
   if (scale_factor == 1.f)
     return size_in_dip;

ui/gfx/geometry/dip_util.h

@@ -26,6 +26,7 @@ class Size;
 // point values, which can itself be a lossy operation for large integers. The
 // intention of these methods is to be used for UI values which are relatively
 // small.
+
 GEOMETRY_EXPORT gfx::PointF ConvertPointToDips(
     const gfx::Point& point_in_pixels,
     float device_scale_factor);
@@ -33,6 +34,13 @@ GEOMETRY_EXPORT gfx::PointF ConvertPointToDips(
     const gfx::PointF& point_in_pixels,
     float device_scale_factor);
 
+GEOMETRY_EXPORT gfx::PointF ConvertPointToPixels(
+    const gfx::Point& point_in_dips,
+    float device_scale_factor);
+GEOMETRY_EXPORT gfx::PointF ConvertPointToPixels(
+    const gfx::PointF& point_in_dips,
+    float device_scale_factor);
+
 GEOMETRY_EXPORT gfx::Insets ConvertInsetsToDIP(
     float scale_factor,
     const gfx::Insets& insets_in_pixel);
@@ -44,11 +52,6 @@ GEOMETRY_EXPORT gfx::Rect ConvertRectToDIP(float scale_factor,
 GEOMETRY_EXPORT gfx::Insets ConvertInsetsToPixel(
     float scale_factor,
     const gfx::Insets& insets_in_dip);
-GEOMETRY_EXPORT gfx::Point ConvertPointToPixel(float scale_factor,
-                                               const gfx::Point& point_in_dip);
-GEOMETRY_EXPORT gfx::PointF ConvertPointToPixel(
-    float scale_factor,
-    const gfx::PointF& point_in_dip);
 GEOMETRY_EXPORT gfx::Size ConvertSizeToPixel(float scale_factor,
                                              const gfx::Size& size_in_dip);
 GEOMETRY_EXPORT gfx::Rect ConvertRectToPixel(float scale_factor,

ui/views/widget/desktop_aura/desktop_screen_x11.cc

@@ -69,8 +69,11 @@ bool DesktopScreenX11::IsWindowUnderCursor(gfx::NativeWindow window) {
 
 gfx::NativeWindow DesktopScreenX11::GetWindowAtScreenPoint(
     const gfx::Point& point) {
+  // TODO(danakj): Should this be rounded?
+  gfx::Point point_in_pixels = gfx::ToFlooredPoint(
+      gfx::ConvertPointToPixels(point, GetXDisplayScaleFactor()));
   auto window = ui::X11TopmostWindowFinder().FindLocalProcessWindowAt(
-      gfx::ConvertPointToPixel(GetXDisplayScaleFactor(), point), {});
+      point_in_pixels, {});
   return window != x11::Window::None
              ? views::DesktopWindowTreeHostPlatform::GetContentWindowForWidget(
                    static_cast<gfx::AcceleratedWidget>(window))
@@ -83,9 +86,11 @@ gfx::NativeWindow DesktopScreenX11::GetLocalProcessWindowAtPoint(
   std::set<gfx::AcceleratedWidget> ignore_widgets;
   for (auto* const window : ignore)
     ignore_widgets.emplace(window->GetHost()->GetAcceleratedWidget());
+  // TODO(danakj): Should this be rounded?
+  gfx::Point point_in_pixels = gfx::ToFlooredPoint(
+      gfx::ConvertPointToPixels(point, GetXDisplayScaleFactor()));
   auto window = ui::X11TopmostWindowFinder().FindLocalProcessWindowAt(
-      gfx::ConvertPointToPixel(GetXDisplayScaleFactor(), point),
-      ignore_widgets);
+      point_in_pixels, ignore_widgets);
   return window != x11::Window::None
              ? views::DesktopWindowTreeHostPlatform::GetContentWindowForWidget(
                    static_cast<gfx::AcceleratedWidget>(window))