Fix rounding rules for skia operations to work with non-integer scaling factors.

ui/gfx/image/image_skia_operations.cc

Index: ui/gfx/image/image_skia_operations.cc
diff --git a/ui/gfx/image/image_skia_operations.cc b/ui/gfx/image/image_skia_operations.cc
index 4b5612ea063bc1376556b58dee5e03e80b9ffe23..428fcf039d9f7eaf543de61b345591e0dccce316 100644
--- a/ui/gfx/image/image_skia_operations.cc
+++ b/ui/gfx/image/image_skia_operations.cc
@@ -14,6 +14,8 @@
 #include "ui/gfx/image/image_skia_rep.h"
 #include "ui/gfx/image/image_skia_source.h"
 #include "ui/gfx/insets.h"
+#include "ui/gfx/point.h"
+#include "ui/gfx/point_conversions.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/rect_conversions.h"
 #include "ui/gfx/size.h"
@@ -24,6 +26,24 @@
 namespace gfx {
 namespace {
 
+gfx::Size DIPToPixelSize(gfx::Size dip_size, float scale) {
+  return ToCeiledSize(ScaleSize(dip_size, scale));
+}
+
+gfx::Rect DIPToPixelBounds(gfx::Rect dip_bounds, float scale) {
+  return gfx::Rect(ToFlooredPoint(ScalePoint(dip_bounds.origin(), scale)),
+                   DIPToPixelSize(dip_bounds.size(), scale));
+}
+
+ImageSkiaRep CropImageSkiaRep(ImageSkiaRep& source,
+                              gfx::Rect& bounds_in_pixel) {

[oshima, 2013/03/14 18:12:20]

Did you define this intentionally? Or can be in ExtractSubsetImageSource?

[oshima, 2013/03/14 19:51:17]

you probably missed this comment.

[kevers, 2013/03/14 20:18:28]

Oops..I did miss it.  It was pulled out since it was being used elsewhere.  Can
move it back now that cropping is done in only one spot.

[kevers, 2013/03/14 20:41:42]

Move back to ExtractSubsetImageSource.

+  SkBitmap result;
+  bool success = source.sk_bitmap().extractSubset(&result,
+      RectToSkIRect(bounds_in_pixel));
+  DCHECK(success);
+  return ImageSkiaRep(result, source.scale_factor());
+}
+
 // Returns an image rep for the ImageSkiaSource to return to visually indicate
 // an error.
 ImageSkiaRep GetErrorImageRep(ui::ScaleFactor scale_factor,
@@ -216,10 +236,12 @@ class TiledImageSource : public gfx::ImageSkiaSource {
   virtual ImageSkiaRep GetImageForScale(ui::ScaleFactor scale_factor) OVERRIDE {
     ImageSkiaRep source_rep = source_.GetRepresentation(scale_factor);
     float scale = ui::GetScaleFactorScale(source_rep.scale_factor());
+    gfx::Rect bounds = DIPToPixelBounds(gfx::Rect(src_x_, src_y_, dst_w_,
+                                                  dst_h_), scale);
     return ImageSkiaRep(
         SkBitmapOperations::CreateTiledBitmap(
             source_rep.sk_bitmap(),
-            src_x_ * scale, src_y_ * scale, dst_w_ * scale, dst_h_ * scale),
+            bounds.x(), bounds.y(), bounds.width(), bounds.height()),
         source_rep.scale_factor());
   }
 
@@ -260,37 +282,30 @@ class HSLImageSource : public gfx::ImageSkiaSource {
 
 // ImageSkiaSource which uses SkBitmapOperations::CreateButtonBackground
 // to generate image reps for the target image.
-class ButtonImageSource: public gfx::ImageSkiaSource {
+class ButtonImageSource: public BinaryImageSource {

[oshima, 2013/03/13 22:08:53]

I vaguely remember that doing this caused some issues on 2x DSF/Pixel.

I can't remember now (Sorry, it's my fault. I should have
documented it) but could you please test this on 2x DSF
and check if this doesn't cause any issue?

[kevers, 2013/03/14 15:49:46]

Tested on Linux Chromeos with a forced scale factor.  Do not see any visual
defects.

[oshima, 2013/03/14 18:12:20]

I now remember the reason. crbug.com/171725

In short, the image and mask can be different size. Can you handle this
separately?

[kevers, 2013/03/14 19:31:39]

Reverted ButtonImageSource change and added a comment.

  public:
   ButtonImageSource(SkColor color,
                     const ImageSkia& image,
                     const ImageSkia& mask)
-      : color_(color),
-        image_(image),
-        mask_(mask) {
+      : BinaryImageSource(image, mask, "ButtonImageSource"),
+        color_(color) {
   }
 
   virtual ~ButtonImageSource() {
   }
 
-  // gfx::ImageSkiaSource overrides:
-  virtual ImageSkiaRep GetImageForScale(ui::ScaleFactor scale_factor) OVERRIDE {
-    ImageSkiaRep image_rep = image_.GetRepresentation(scale_factor);
-    ImageSkiaRep mask_rep = mask_.GetRepresentation(scale_factor);
-    if (image_rep.scale_factor() != mask_rep.scale_factor()) {
-      image_rep = image_.GetRepresentation(ui::SCALE_FACTOR_100P);
-      mask_rep = mask_.GetRepresentation(ui::SCALE_FACTOR_100P);
-    }
+  // BinaryImageSource overrides:
+  virtual ImageSkiaRep CreateImageSkiaRep(
+      const ImageSkiaRep& first_rep,
+      const ImageSkiaRep& second_rep) const OVERRIDE {
     return gfx::ImageSkiaRep(
         SkBitmapOperations::CreateButtonBackground(color_,
-              image_rep.sk_bitmap(), mask_rep.sk_bitmap()),
-          image_rep.scale_factor());
+              first_rep.sk_bitmap(), second_rep.sk_bitmap()),
+          first_rep.scale_factor());
   }
 
  private:
   const SkColor color_;
-  const ImageSkia image_;
-  const ImageSkia mask_;
 
   DISALLOW_COPY_AND_ASSIGN(ButtonImageSource);
 };
@@ -312,13 +327,9 @@ class ExtractSubsetImageSource: public gfx::ImageSkiaSource {
   virtual ImageSkiaRep GetImageForScale(ui::ScaleFactor scale_factor) OVERRIDE {
     ImageSkiaRep image_rep = image_.GetRepresentation(scale_factor);
     float scale_to_pixel = ui::GetScaleFactorScale(image_rep.scale_factor());
-    SkIRect subset_bounds_in_pixel = RectToSkIRect(ToFlooredRectDeprecated(
-        gfx::ScaleRect(subset_bounds_, scale_to_pixel)));
-    SkBitmap dst;
-    bool success = image_rep.sk_bitmap().extractSubset(&dst,
-                                                       subset_bounds_in_pixel);
-    DCHECK(success);
-    return gfx::ImageSkiaRep(dst, image_rep.scale_factor());
+    Rect subset_bounds_in_pixel = DIPToPixelBounds(subset_bounds_,
+                                                   scale_to_pixel);
+    return CropImageSkiaRep(image_rep, subset_bounds_in_pixel);
   }
 
  private:
@@ -349,8 +360,7 @@ class ResizeSource : public ImageSkiaSource {
       return image_rep;
 
     const float scale = ui::GetScaleFactorScale(scale_factor);
-    const Size target_pixel_size = gfx::ToFlooredSize(
-        gfx::ScaleSize(target_dip_size_, scale));
+    const Size target_pixel_size = DIPToPixelSize(target_dip_size_, scale);
     const SkBitmap resized = skia::ImageOperations::Resize(
         image_rep.sk_bitmap(),
         resize_method_,