Remove implicit flooring Scale() method from Point and Size.

When scaling an integer point or size, return a floating point
result. Implicitly flooring hides design problems and bugs. Add
conversion functions to floor or ceil a SizeF or PointF
into an integer format again.

All existing behaviour has been preserved by replacing uses of
foo.Scale() with ToFlooredFoo(foo.Scale()).

R=sky
BUG=147395


Review URL: https://chromiumcodereview.appspot.com/11081007

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

20 files changed, 165 insertions, 39 deletions

diff --git a/ui/aura/root_window.cc b/ui/aura/root_window.cc
index b82ab03..c9cdbbd 100644
--- a/ui/aura/root_window.cc
+++ b/ui/aura/root_window.cc
@@ -38,6 +38,7 @@
 #include "ui/compositor/layer_animator.h"
 #include "ui/gfx/display.h"
 #include "ui/gfx/point3.h"
+#include "ui/gfx/point_conversions.h"
 #include "ui/gfx/screen.h"
 
 using std::vector;
@@ -387,7 +388,7 @@ void RootWindow::PostNativeEvent(const base::NativeEvent& native_event) {
 
 void RootWindow::ConvertPointToNativeScreen(gfx::Point* point) const {
   // TODO(oshima): Take the root window's transform into account.
-  *point = point->Scale(ui::GetDeviceScaleFactor(layer()));
+  *point = gfx::ToFlooredPoint(point->Scale(ui::GetDeviceScaleFactor(layer())));
   gfx::Point location = host_->GetLocationOnNativeScreen();
   point->Offset(location.x(), location.y());
 }
@@ -395,7 +396,8 @@ void RootWindow::ConvertPointToNativeScreen(gfx::Point* point) const {
 void RootWindow::ConvertPointFromNativeScreen(gfx::Point* point) const {
   gfx::Point location = host_->GetLocationOnNativeScreen();
   point->Offset(-location.x(), -location.y());
-  *point = point->Scale(1 / ui::GetDeviceScaleFactor(layer()));
+  *point = gfx::ToFlooredPoint(
+      point->Scale(1 / ui::GetDeviceScaleFactor(layer())));
 }
 
 void RootWindow::AdvanceQueuedTouchEvent(Window* window, bool processed) {
diff --git a/ui/base/resource/resource_bundle.cc b/ui/base/resource/resource_bundle.cc
index 5a77c12..294b48a 100644
--- a/ui/base/resource/resource_bundle.cc
+++ b/ui/base/resource/resource_bundle.cc
@@ -29,6 +29,7 @@
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_skia_source.h"
 #include "ui/gfx/screen.h"
+#include "ui/gfx/size_conversions.h"
 #include "ui/gfx/skbitmap_operations.h"
 
 namespace ui {
@@ -73,7 +74,7 @@ class ResourceBundle::ResourceBundleImageSource : public gfx::ImageSkiaSource {
       ui::ScaleFactor scale_factor) OVERRIDE {
     scoped_ptr<SkBitmap> result(rb_->LoadBitmap(resource_id_, scale_factor));
     float scale = ui::GetScaleFactorScale(scale_factor);
-    gfx::Size size_in_pixel = size_in_dip_.Scale(scale);
+    gfx::Size size_in_pixel = gfx::ToFlooredSize(size_in_dip_.Scale(scale));
 
     if (scale_factor != SCALE_FACTOR_100P &&
         (!result.get() ||
diff --git a/ui/base/x/x11_util.cc b/ui/base/x/x11_util.cc
index d6e53f1..ffe827c6 100644
--- a/ui/base/x/x11_util.cc
+++ b/ui/base/x/x11_util.cc
@@ -34,6 +34,7 @@
 #include "base/threading/thread.h"
 #include "ui/base/keycodes/keyboard_code_conversion_x.h"
 #include "ui/base/x/x11_util_internal.h"
+#include "ui/gfx/point_conversions.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/size.h"
 
@@ -472,7 +473,7 @@ XcursorImage* SkBitmapToXcursorImage(const SkBitmap* cursor_image,
         skia::ImageOperations::RESIZE_BETTER,
         static_cast<int>(cursor_image->width() * scale),
         static_cast<int>(cursor_image->height() * scale));
-    hotspot_point = hotspot.Scale(scale);
+    hotspot_point = gfx::ToFlooredPoint(hotspot.Scale(scale));
     needs_scale = true;
   }
 
diff --git a/ui/compositor/dip_util.cc b/ui/compositor/dip_util.cc
index eff86e2..b7731d4 100644
--- a/ui/compositor/dip_util.cc
+++ b/ui/compositor/dip_util.cc
@@ -11,8 +11,10 @@
 #include "ui/compositor/layer.h"
 #include "ui/gfx/display.h"
 #include "ui/gfx/point.h"
+#include "ui/gfx/point_conversions.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/size.h"
+#include "ui/gfx/size_conversions.h"
 
 namespace ui {
 
@@ -22,35 +24,38 @@ float GetDeviceScaleFactor(const Layer* layer) {
 
 gfx::Point ConvertPointToDIP(const Layer* layer,
                              const gfx::Point& point_in_pixel) {
-  return point_in_pixel.Scale(1.0f / GetDeviceScaleFactor(layer));
+  return gfx::ToFlooredPoint(
+      point_in_pixel.Scale(1.0f / GetDeviceScaleFactor(layer)));
 }
 
 gfx::Size ConvertSizeToDIP(const Layer* layer,
                            const gfx::Size& size_in_pixel) {
-  return size_in_pixel.Scale(1.0f / GetDeviceScaleFactor(layer));
+  return gfx::ToFlooredSize(
+      size_in_pixel.Scale(1.0f / GetDeviceScaleFactor(layer)));
 }
 
 gfx::Rect ConvertRectToDIP(const Layer* layer,
                            const gfx::Rect& rect_in_pixel) {
   float scale = 1.0f / GetDeviceScaleFactor(layer);
-  return gfx::Rect(rect_in_pixel.origin().Scale(scale),
-                   rect_in_pixel.size().Scale(scale));
+  return gfx::Rect(gfx::ToFlooredPoint(rect_in_pixel.origin().Scale(scale)),
+                   gfx::ToFlooredSize(rect_in_pixel.size().Scale(scale)));
 }
 
 gfx::Point ConvertPointToPixel(const Layer* layer,
                                const gfx::Point& point_in_dip) {
-  return point_in_dip.Scale(GetDeviceScaleFactor(layer));
+  return gfx::ToFlooredPoint(
+      point_in_dip.Scale(GetDeviceScaleFactor(layer)));
 }
 
 gfx::Size ConvertSizeToPixel(const Layer* layer,
                              const gfx::Size& size_in_dip) {
-  return size_in_dip.Scale(GetDeviceScaleFactor(layer));
+  return gfx::ToFlooredSize(size_in_dip.Scale(GetDeviceScaleFactor(layer)));
 }
 
 gfx::Rect ConvertRectToPixel(const Layer* layer,
                              const gfx::Rect& rect_in_dip) {
   float scale = GetDeviceScaleFactor(layer);
-  return gfx::Rect(rect_in_dip.origin().Scale(scale),
-                   rect_in_dip.size().Scale(scale));
+  return gfx::Rect(gfx::ToFlooredPoint(rect_in_dip.origin().Scale(scale)),
+                   gfx::ToFlooredSize(rect_in_dip.size().Scale(scale)));
 }
 }  // namespace ui
diff --git a/ui/compositor/layer.cc b/ui/compositor/layer.cc
index bbca75a..e71cbd5 100644
--- a/ui/compositor/layer.cc
+++ b/ui/compositor/layer.cc
@@ -28,6 +28,7 @@
 #include "ui/gfx/display.h"
 #include "ui/gfx/interpolated_transform.h"
 #include "ui/gfx/point3.h"
+#include "ui/gfx/size_conversions.h"
 
 namespace {
 
@@ -754,8 +755,8 @@ void Layer::RecomputeDrawsContentAndUVRect() {
     if (scale_content_) {
       texture_size = texture_->size();
     } else {
-      texture_size =
-          texture_->size().Scale(1.0f / texture_->device_scale_factor());
+      texture_size = gfx::ToFlooredSize(
+          texture_->size().Scale(1.0f / texture_->device_scale_factor()));
     }
 
     gfx::Size size(std::min(bounds().width(), texture_size.width()),
diff --git a/ui/gfx/canvas.cc b/ui/gfx/canvas.cc
index c4f98d6..78badf1 100644
--- a/ui/gfx/canvas.cc
+++ b/ui/gfx/canvas.cc
@@ -13,6 +13,7 @@
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/font.h"
 #include "ui/gfx/rect.h"
+#include "ui/gfx/size_conversions.h"
 #include "ui/gfx/skia_util.h"
 #include "ui/gfx/transform.h"
 
@@ -28,7 +29,8 @@ Canvas::Canvas(const gfx::Size& size,
       : scale_factor_(scale_factor),
         owned_canvas_(NULL),
         canvas_(NULL) {
-  gfx::Size pixel_size = size.Scale(ui::GetScaleFactorScale(scale_factor));
+  gfx::Size pixel_size = gfx::ToFlooredSize(size.Scale(
+      ui::GetScaleFactorScale(scale_factor)));
   owned_canvas_.reset(new skia::PlatformCanvas(pixel_size.width(),
                                                pixel_size.height(),
                                                is_opaque));
@@ -74,7 +76,8 @@ void Canvas::RecreateBackingCanvas(const gfx::Size& size,
                                    ui::ScaleFactor scale_factor,
                                    bool is_opaque) {
   scale_factor_ = scale_factor;
-  gfx::Size pixel_size = size.Scale(ui::GetScaleFactorScale(scale_factor));
+  gfx::Size pixel_size = gfx::ToFlooredSize(
+      size.Scale(ui::GetScaleFactorScale(scale_factor)));
   owned_canvas_.reset(new skia::PlatformCanvas(pixel_size.width(),
                                                pixel_size.height(),
                                                is_opaque));
diff --git a/ui/gfx/display.cc b/ui/gfx/display.cc
index 9fcf156..e647f65 100644
--- a/ui/gfx/display.cc
+++ b/ui/gfx/display.cc
@@ -10,6 +10,7 @@
 #include "base/stringprintf.h"
 #include "ui/base/ui_base_switches.h"
 #include "ui/gfx/insets.h"
+#include "ui/gfx/size_conversions.h"
 
 namespace gfx {
 namespace {
@@ -87,8 +88,8 @@ void Display::SetScaleAndBounds(
 #if defined(USE_AURA)
   bounds_in_pixel_ = bounds_in_pixel;
 #endif
-  bounds_ = gfx::Rect(
-      bounds_in_pixel.size().Scale(1.0f / device_scale_factor_));
+  bounds_ = gfx::Rect(gfx::ToFlooredSize(
+      bounds_in_pixel.size().Scale(1.0f / device_scale_factor_)));
   UpdateWorkAreaFromInsets(insets);
 }
 
@@ -108,7 +109,7 @@ void Display::UpdateWorkAreaFromInsets(const gfx::Insets& insets) {
 }
 
 gfx::Size Display::GetSizeInPixel() const {
-  return size().Scale(device_scale_factor_);
+  return gfx::ToFlooredSize(size().Scale(device_scale_factor_));
 }
 
 std::string Display::ToString() const {
diff --git a/ui/gfx/image/image_skia_operations.cc b/ui/gfx/image/image_skia_operations.cc
index f190d8c..8afaf87 100644
--- a/ui/gfx/image/image_skia_operations.cc
+++ b/ui/gfx/image/image_skia_operations.cc
@@ -19,6 +19,7 @@
 #include "ui/gfx/rect.h"
 #include "ui/gfx/rect_conversions.h"
 #include "ui/gfx/size.h"
+#include "ui/gfx/size_conversions.h"
 #include "ui/gfx/skbitmap_operations.h"
 #include "ui/gfx/skia_util.h"
 
@@ -325,7 +326,8 @@ class ResizeSource : public ImageSkiaSource {
       return image_rep;
 
     const float scale = ui::GetScaleFactorScale(scale_factor);
-    const Size target_pixel_size(target_dip_size_.Scale(scale));
+    const Size target_pixel_size = gfx::ToFlooredSize(
+        target_dip_size_.Scale(scale));
     const SkBitmap resized = skia::ImageOperations::Resize(
         image_rep.sk_bitmap(),
         resize_method_,
diff --git a/ui/gfx/point.h b/ui/gfx/point.h
index 4bf9d55..1f437cd 100644
--- a/ui/gfx/point.h
+++ b/ui/gfx/point.h
@@ -48,6 +48,14 @@ class UI_EXPORT Point : public PointBase<Point, int> {
     return PointF(x(), y());
   }
 
+  PointF Scale(float scale) const WARN_UNUSED_RESULT {
+    return Scale(scale, scale);
+  }
+
+  PointF Scale(float x_scale, float y_scale) const WARN_UNUSED_RESULT {
+    return PointF(x() * x_scale, y() * y_scale);
+  }
+
   // Returns a string representation of point.
   std::string ToString() const;
 };
diff --git a/ui/gfx/point_base.h b/ui/gfx/point_base.h
index e6574d4..fcb2289 100644
--- a/ui/gfx/point_base.h
+++ b/ui/gfx/point_base.h
@@ -33,15 +33,6 @@ class UI_EXPORT PointBase {
     y_ += delta_y;
   }
 
-  Class Scale(float scale) const WARN_UNUSED_RESULT {
-    return Scale(scale, scale);
-  }
-
-  Class Scale(float x_scale, float y_scale) const WARN_UNUSED_RESULT {
-    return Class(static_cast<Type>(x_ * x_scale),
-                 static_cast<Type>(y_ * y_scale));
-  }
-
   Class Add(const Class& other) const WARN_UNUSED_RESULT {
     const Class* orig = static_cast<const Class*>(this);
     Class copy = *orig;
diff --git a/ui/gfx/point_conversions.cc b/ui/gfx/point_conversions.cc
new file mode 100644
index 0000000..0788ef7
--- /dev/null
+++ b/ui/gfx/point_conversions.cc
@@ -0,0 +1,24 @@
+// 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/gfx/point_conversions.h"
+
+#include "ui/gfx/safe_floor_ceil.h"
+
+namespace gfx {
+
+Point ToFlooredPoint(const PointF& point) {
+  int x = ToFlooredInt(point.x());
+  int y = ToFlooredInt(point.y());
+  return Point(x, y);
+}
+
+Point ToCeiledPoint(const PointF& point) {
+  int x = ToCeiledInt(point.x());
+  int y = ToCeiledInt(point.y());
+  return Point(x, y);
+}
+
+}  // namespace gfx
+
diff --git a/ui/gfx/point_conversions.h b/ui/gfx/point_conversions.h
new file mode 100644
index 0000000..6fb4ca0
--- /dev/null
+++ b/ui/gfx/point_conversions.h
@@ -0,0 +1,21 @@
+// 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.
+
+#ifndef UI_GFX_POINT_CONVERSIONS_H_
+#define UI_GFX_POINT_CONVERSIONS_H_
+
+#include "ui/gfx/point.h"
+#include "ui/gfx/point_f.h"
+
+namespace gfx {
+
+// Returns a Point with each component from the input PointF floored.
+UI_EXPORT Point ToFlooredPoint(const PointF& point);
+
+// Returns a Point with each component from the input PointF ceiled.
+UI_EXPORT Point ToCeiledPoint(const PointF& point);
+
+}  // namespace gfx
+
+#endif  // UI_GFX_RECT_CONVERSIONS_H_
diff --git a/ui/gfx/point_f.h b/ui/gfx/point_f.h
index ff1c4df..a0b367f 100644
--- a/ui/gfx/point_f.h
+++ b/ui/gfx/point_f.h
@@ -19,6 +19,14 @@ class UI_EXPORT PointF : public PointBase<PointF, float> {
   PointF(float x, float y);
   ~PointF();
 
+  PointF Scale(float scale) const WARN_UNUSED_RESULT {
+    return Scale(scale, scale);
+  }
+
+  PointF Scale(float x_scale, float y_scale) const WARN_UNUSED_RESULT {
+    return PointF(x() * x_scale, y() * y_scale);
+  }
+
   // Returns a string representation of point.
   std::string ToString() const;
 };
diff --git a/ui/gfx/shadow_value.cc b/ui/gfx/shadow_value.cc
index 09127ee..0602c2d 100644
--- a/ui/gfx/shadow_value.cc
+++ b/ui/gfx/shadow_value.cc
@@ -8,6 +8,7 @@
 
 #include "base/stringprintf.h"
 #include "ui/gfx/insets.h"
+#include "ui/gfx/point_conversions.h"
 
 namespace gfx {
 
@@ -28,7 +29,8 @@ ShadowValue::~ShadowValue() {
 }
 
 ShadowValue ShadowValue::Scale(float scale) const {
-  return ShadowValue(offset_.Scale(scale), blur_ * scale, color_);
+  gfx::Point scaled_offset = gfx::ToFlooredPoint(offset_.Scale(scale));
+  return ShadowValue(scaled_offset, blur_ * scale, color_);
 }
 
 std::string ShadowValue::ToString() const {
diff --git a/ui/gfx/size.h b/ui/gfx/size.h
index 8a53b97..a2cdf3e 100644
--- a/ui/gfx/size.h
+++ b/ui/gfx/size.h
@@ -47,6 +47,14 @@ class UI_EXPORT Size : public SizeBase<Size, int> {
     return SizeF(width(), height());
   }
 
+  SizeF Scale(float scale) const WARN_UNUSED_RESULT {
+    return Scale(scale, scale);
+  }
+
+  SizeF Scale(float x_scale, float y_scale) const WARN_UNUSED_RESULT {
+    return SizeF(width() * x_scale, height() * y_scale);
+  }
+
   std::string ToString() const;
 };
 
diff --git a/ui/gfx/size_base.h b/ui/gfx/size_base.h
index 788120b..d66ca67 100644
--- a/ui/gfx/size_base.h
+++ b/ui/gfx/size_base.h
@@ -32,15 +32,6 @@ class UI_EXPORT SizeBase {
     set_height(height_ + height);
   }
 
-  Class Scale(float scale) const WARN_UNUSED_RESULT {
-    return Scale(scale, scale);
-  }
-
-  Class Scale(float x_scale, float y_scale) const WARN_UNUSED_RESULT {
-    return Class(static_cast<Type>(width_ * x_scale),
-                 static_cast<Type>(height_ * y_scale));
-  }
-
   void set_width(Type width) { width_ = width; }
   void set_height(Type height) { height_ = height; }
 
diff --git a/ui/gfx/size_conversions.cc b/ui/gfx/size_conversions.cc
new file mode 100644
index 0000000..9668aaa
--- /dev/null
+++ b/ui/gfx/size_conversions.cc
@@ -0,0 +1,24 @@
+// 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/gfx/size_conversions.h"
+
+#include "ui/gfx/safe_floor_ceil.h"
+
+namespace gfx {
+
+Size ToFlooredSize(const SizeF& size) {
+  int w = ToFlooredInt(size.width());
+  int h = ToFlooredInt(size.height());
+  return Size(w, h);
+}
+
+Size ToCeiledSize(const SizeF& size) {
+  int w = ToCeiledInt(size.width());
+  int h = ToCeiledInt(size.height());
+  return Size(w, h);
+}
+
+}  // namespace gfx
+
diff --git a/ui/gfx/size_conversions.h b/ui/gfx/size_conversions.h
new file mode 100644
index 0000000..03197b3
--- /dev/null
+++ b/ui/gfx/size_conversions.h
@@ -0,0 +1,21 @@
+// 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.
+
+#ifndef UI_GFX_SIZE_CONVERSIONS_H_
+#define UI_GFX_SIZE_CONVERSIONS_H_
+
+#include "ui/gfx/size.h"
+#include "ui/gfx/size_f.h"
+
+namespace gfx {
+
+// Returns a Size with each component from the input SizeF floored.
+UI_EXPORT Size ToFlooredSize(const SizeF& size);
+
+// Returns a Size with each component from the input SizeF ceiled.
+UI_EXPORT Size ToCeiledSize(const SizeF& size);
+
+}  // namespace gfx
+
+#endif  // UI_GFX_RECT_CONVERSIONS_H_
diff --git a/ui/gfx/size_f.h b/ui/gfx/size_f.h
index c377395..f3ad7db 100644
--- a/ui/gfx/size_f.h
+++ b/ui/gfx/size_f.h
@@ -19,6 +19,14 @@ class UI_EXPORT SizeF : public SizeBase<SizeF, float> {
   SizeF(float width, float height);
   ~SizeF();
 
+  SizeF Scale(float scale) const WARN_UNUSED_RESULT {
+    return Scale(scale, scale);
+  }
+
+  SizeF Scale(float x_scale, float y_scale) const WARN_UNUSED_RESULT {
+    return SizeF(width() * x_scale, height() * y_scale);
+  }
+
   std::string ToString() const;
 };
 
diff --git a/ui/ui.gyp b/ui/ui.gyp
index 63bc0ee..9d99b41 100644
--- a/ui/ui.gyp
+++ b/ui/ui.gyp
@@ -140,6 +140,8 @@
         'gfx/point.h',
         'gfx/point3.h',
         'gfx/point_base.h',
+        'gfx/point_conversions.cc',
+        'gfx/point_conversions.h',
         'gfx/point_f.cc',
         'gfx/point_f.h',
         'gfx/rect.cc',
@@ -167,6 +169,8 @@
         'gfx/size.cc',
         'gfx/size.h',
         'gfx/size_base.h',
+        'gfx/size_conversions.cc',
+        'gfx/size_conversions.h',
         'gfx/size_f.cc',
         'gfx/size_f.h',
         'gfx/skbitmap_operations.cc',