Makes ImageSkia more like SkBitmap

ui/gfx/image/image_skia.cc

Index: ui/gfx/image/image_skia.cc
diff --git a/ui/gfx/image/image_skia.cc b/ui/gfx/image/image_skia.cc
index 3c2e94f867221b4240065aef0652376daca0b435..0385f574cbcf7ffb12b6d7e9f43d315bb85df0d6 100644
--- a/ui/gfx/image/image_skia.cc
+++ b/ui/gfx/image/image_skia.cc
@@ -8,139 +8,175 @@
 #include <cmath>
 
 #include "base/logging.h"
+#include "ui/gfx/size.h"
 #include "base/stl_util.h"
 
 namespace gfx {
 
-ImageSkia::ImageSkia(const SkBitmap* bitmap)
-    : size_(bitmap->width(), bitmap->height()),
-      mip_map_build_pending_(false) {
-  CHECK(bitmap);
-  // TODO(pkotwicz): Add a CHECK to ensure that !bitmap->isNull()
-  bitmaps_.push_back(bitmap);
-}
+namespace internal {
 
-ImageSkia::ImageSkia(const std::vector<const SkBitmap*>& bitmaps)
-    : bitmaps_(bitmaps),
-      mip_map_build_pending_(false) {
-  CHECK(!bitmaps_.empty());
-  // TODO(pkotwicz): Add a CHECK to ensure that !bitmap->isNull() for each
-  // vector element.
-  // Assume that the smallest bitmap represents 1x scale factor.
-  for (size_t i = 0; i < bitmaps_.size(); ++i) {
-    gfx::Size bitmap_size(bitmaps_[i]->width(), bitmaps_[i]->height());
-    if (size_.IsEmpty() || bitmap_size.GetArea() < size_.GetArea())
-      size_ = bitmap_size;
+// A helper class such that ImageSkia can be cheaply copied. ImageSkia holds a
+// refptr instance of ImageSkiaStorage, which in turn holds all of ImageSkia's
+// information.
+class ImageSkiaStorage : public base::RefCounted<ImageSkiaStorage> {
+ public:
+  ImageSkiaStorage() : build_mip_map_(false) {
   }
-}
 
-ImageSkia::~ImageSkia() {
-  STLDeleteElements(&bitmaps_);
-}
-
-void ImageSkia::BuildMipMap() {
-  mip_map_build_pending_ = true;
-}
+  void add_bitmap(const SkBitmap* bitmap) { bitmaps_.push_back(bitmap); }
+  void set_bitmaps(const std::vector<const SkBitmap*>& bitmaps) {
+    bitmaps_ = bitmaps;
+  }
+  const std::vector<const SkBitmap*>& bitmaps() const { return bitmaps_; }
 
-void ImageSkia::DrawToCanvasInt(gfx::Canvas* canvas, int x, int y) {
-  SkPaint p;
-  DrawToCanvasInt(canvas, x, y, p);
-}
+  void set_size(gfx::Size size) { size_ = size; }
+  gfx::Size size() const { return size_; }
 
-void ImageSkia::DrawToCanvasInt(gfx::Canvas* canvas,
-                                int x, int y,
-                                const SkPaint& paint) {
+  void set_build_mip_map() { build_mip_map_ = true; }
+  bool build_mip_map() const { return build_mip_map_; }
 
-  if (IsZeroSized())
-    return;
+ private:
+  ~ImageSkiaStorage() {
+    STLDeleteElements(&bitmaps_);
+  }
 
-  SkMatrix m = canvas->sk_canvas()->getTotalMatrix();
-  float scale_x = std::abs(SkScalarToFloat(m.getScaleX()));
-  float scale_y = std::abs(SkScalarToFloat(m.getScaleY()));
+  std::vector<const SkBitmap*> bitmaps_;

[sky, 2012/04/27 23:33:58]

Why does this need to be pointers? It would be a lot safer to make it by value.

[pkotwicz, 2012/04/30 20:27:07]

I think this needs to be pointers as long as we have SkBitmap*
Image::ToSkBitmap()

[sky, 2012/04/30 21:03:32]

Is the long term plan to get rid of ToSkBitmap? If not, can it return a value?

Either way, why does the need for ToSkbitmap returning a pointer imply these
have to be pointers? Couldn't places that need a pointer use & ?

+  gfx::Size size_;

[sky, 2012/04/27 23:33:58]

Document what this is.

+  bool build_mip_map_;

[sky, 2012/04/27 23:33:58]

Shouldn't the mip-map be per-image?

 
-  const SkBitmap* bitmap = GetBitmapForScale(scale_x, scale_y);
+  friend class base::RefCounted<ImageSkiaStorage>;
+};
 
-  if (mip_map_build_pending_) {
-    const_cast<SkBitmap*>(bitmap)->buildMipMap();
-    mip_map_build_pending_ = false;
-  }
+}  // internal
 
-  float bitmap_scale_x = static_cast<float>(bitmap->width()) / width();
-  float bitmap_scale_y = static_cast<float>(bitmap->height()) / height();
+ImageSkia::ImageSkia() : storage_(NULL) {
+}
 
-  canvas->Save();
-  canvas->sk_canvas()->scale(1.0f / bitmap_scale_x,
-                             1.0f / bitmap_scale_y);
-  canvas->sk_canvas()->drawBitmap(*bitmap, SkFloatToScalar(x * bitmap_scale_x),
-                                  SkFloatToScalar(y * bitmap_scale_y));
-  canvas->Restore();
+ImageSkia::ImageSkia(const SkBitmap& bitmap)
+    : storage_(new internal::ImageSkiaStorage()) {
+  storage_->set_size(gfx::Size(bitmap.width(), bitmap.height()));
+  // TODO(pkotwicz): Add a CHECK to ensure that !bitmap.isNull()
+  storage_->add_bitmap(new SkBitmap(bitmap));
 }
 
-void ImageSkia::DrawToCanvasInt(gfx::Canvas* canvas,
-                                int src_x, int src_y, int src_w, int src_h,
-                                int dest_x, int dest_y, int dest_w, int dest_h,
-                                bool filter) {
-  SkPaint p;
-  DrawToCanvasInt(canvas, src_x, src_y, src_w, src_h, dest_x, dest_y,
-                  dest_w, dest_h, filter, p);
+ImageSkia::ImageSkia(const SkBitmap& bitmap, float scale_factor)
+    : storage_(new internal::ImageSkiaStorage()) {
+  storage_->set_size(gfx::Size(
+      static_cast<int>(bitmap.width() / scale_factor),
+      static_cast<int>(bitmap.height() / scale_factor)));
+  // TODO(pkotwicz): Add a CHECK to ensure that !bitmap.isNull()
+  storage_->add_bitmap(new SkBitmap(bitmap));
 }
 
-void ImageSkia::DrawToCanvasInt(gfx::Canvas* canvas,
-                                int src_x, int src_y, int src_w, int src_h,
-                                int dest_x, int dest_y, int dest_w, int dest_h,
-                                bool filter,
-                                const SkPaint& paint) {
-  if (IsZeroSized())
-    return;
+ImageSkia::ImageSkia(const SkBitmap* bitmap)
+    : storage_(new internal::ImageSkiaStorage()) {
+  storage_->set_size(gfx::Size(bitmap->width(), bitmap->height()));
+  // TODO(pkotwicz): Add a CHECK to ensure that !bitmap.isNull()
+  storage_->add_bitmap(bitmap);
+}
 
-  SkMatrix m = canvas->sk_canvas()->getTotalMatrix();
-  float scale_x = std::abs(SkScalarToFloat(m.getScaleX()));
-  float scale_y = std::abs(SkScalarToFloat(m.getScaleY()));
+ImageSkia::ImageSkia(const std::vector<const SkBitmap*>& bitmaps)
+    : storage_(new internal::ImageSkiaStorage()) {
+  storage_->set_bitmaps(bitmaps);
+  // TODO(pkotwicz): Add a CHECK to ensure that !bitmap->isNull() for each
+  // vector element.
+  // Assume that the smallest bitmap represents 1x scale factor.
+  gfx::Size smallest_bitmap_size;
+  for (size_t i = 0; i < bitmaps.size(); ++i) {
+    gfx::Size bitmap_size(bitmaps[i]->width(), bitmaps[i]->height());
+    if (smallest_bitmap_size.IsEmpty() ||
+        bitmap_size.GetArea() < smallest_bitmap_size.GetArea())
+      smallest_bitmap_size = bitmap_size;
+  }
+  storage_->set_size(smallest_bitmap_size);
+}
 
-  const SkBitmap* bitmap = GetBitmapForScale(scale_x, scale_y);
+ImageSkia::ImageSkia(const ImageSkia& other) : storage_(other.storage_) {
+}
 
-  if (mip_map_build_pending_) {
-    const_cast<SkBitmap*>(bitmap)->buildMipMap();
-    mip_map_build_pending_ = false;
-  }
+ImageSkia& ImageSkia::operator=(const ImageSkia& other) {
+  storage_ = other.storage_;
+  return *this;
+}
 
-  float bitmap_scale_x = static_cast<float>(bitmap->width()) / width();
-  float bitmap_scale_y = static_cast<float>(bitmap->height()) / height();
+ImageSkia& ImageSkia::operator=(const SkBitmap& other) {
+  storage_ = new internal::ImageSkiaStorage();
+  storage_->set_size(gfx::Size(other.width(), other.height()));
+  // TODO(pkotwicz): Add a CHECK to ensure that !other.isNull()
+  storage_->add_bitmap(new SkBitmap(other));
+  return *this;
+}
 
-  canvas->Save();
-  canvas->sk_canvas()->scale(1.0f / bitmap_scale_x,
-                             1.0f / bitmap_scale_y);
-  canvas->DrawBitmapFloat(*bitmap,
-                        src_x * bitmap_scale_x, src_y * bitmap_scale_x,
-                        src_w * bitmap_scale_x, src_h * bitmap_scale_y,
-                        dest_x * bitmap_scale_x, dest_y * bitmap_scale_y,
-                        dest_w * bitmap_scale_x, dest_h * bitmap_scale_y,
-                        filter, paint);
+ImageSkia::operator SkBitmap() const {
+  return isNull() ? SkBitmap() : *bitmaps()[0];
+}
 
-  canvas->Restore();
+ImageSkia::~ImageSkia() {
 }
 
 const SkBitmap* ImageSkia::GetBitmapForScale(float x_scale_factor,
                                              float y_scale_factor) const {
+  if (isNull())
+    return NULL;
+
   // Get the desired bitmap width and height given |x_scale_factor|,
-  // |y_scale_factor| and |size_| at 1x density.
-  float desired_width = size_.width() * x_scale_factor;
-  float desired_height = size_.height() * y_scale_factor;
+  // |y_scale_factor| and size at 1x density.
+  float desired_width = width() * x_scale_factor;
+  float desired_height = height() * y_scale_factor;
 
-  size_t closest_index = 0;
+  const std::vector<const SkBitmap*>& bitmaps = storage_->bitmaps();
+  size_t closest_index = -1;
   float smallest_diff = std::numeric_limits<float>::max();
-  for (size_t i = 0; i < bitmaps_.size(); ++i) {
-    if (bitmaps_[i]->isNull())
+  for (size_t i = 0; i < bitmaps.size(); ++i) {
+    if (bitmaps[i]->isNull())

[sky, 2012/04/27 23:33:58]

Is there a reason to allow null/empty images into the storage?

[pkotwicz, 2012/04/30 20:27:07]

Changed semantics such that if bitmap->isNull(), then ImageSkia->isNull().
Unfortunately, we do want to allow gfx::ImageSkia to wrap a null SkBitmap. There
are a number of places where we use gfx::Image to wrap a bitmap which can be
null. The case I know about is in favicon_tab_helper.cc

       continue;
 
-    float diff = std::abs(bitmaps_[i]->width() - desired_width) +
-        std::abs(bitmaps_[i]->height() - desired_height);
+    float diff = std::abs(bitmaps[i]->width() - desired_width) +
+        std::abs(bitmaps[i]->height() - desired_height);
     if (diff < smallest_diff) {
       closest_index = i;
       smallest_diff = diff;
     }
   }
-  return bitmaps_[closest_index];
+  return closest_index < 0 ? NULL : bitmaps[closest_index];
+}
+
+bool ImageSkia::empty() const {
+  return isNull() || storage_->size().IsEmpty();
+}
+
+int ImageSkia::width() const {
+  return isNull() ? 0 : storage_->size().width();
+}
+
+int ImageSkia::height() const {
+  return isNull() ? 0 : storage_->size().height();
+}
+
+bool ImageSkia::extractSubset(ImageSkia* dst, SkIRect& subset) const {
+  if (isNull())
+    return false;
+  SkBitmap dst_bitmap;
+  bool return_value  = bitmaps()[0]->extractSubset(&dst_bitmap, subset);
+  *dst = ImageSkia(dst_bitmap);
+  return return_value;
+}
+
+void ImageSkia::BuildMipMap() {
+  if (isNull())
+    return;
+
+  storage_->set_build_mip_map();
+  // |build_mip_map_| does not need to be turned off as SkBitmap will not
+  // regenerate a mipmap if one exists.
+}
+
+bool ImageSkia::ShouldBuildMipMap() const {
+  return storage_->build_mip_map();
+}
+
+const std::vector<const SkBitmap*>& ImageSkia::bitmaps() const {
+  return storage_->bitmaps();
 }
 
 }  // namespace gfx