Loading/Creating images for mutiple scale factors on the fly

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 e5fb8e3c617ce18b81fb4c00aea2918c07cd49d2..15e44a6456923acc9124039004252154e41e2c0a 100644
--- a/ui/gfx/image/image_skia.cc
+++ b/ui/gfx/image/image_skia.cc
@@ -4,30 +4,117 @@
 
 #include "ui/gfx/image/image_skia.h"
 
-#include <limits>
+#include <algorithm>
 #include <cmath>
+#include <limits>
 
 #include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "ui/gfx/image/image_skia_source.h"
 #include "ui/gfx/size.h"
 #include "ui/gfx/skia_util.h"
 
 namespace gfx {
+namespace {
+
+// static
+gfx::ImageSkiaRep& NullImageRep() {
+  CR_DEFINE_STATIC_LOCAL(ImageSkiaRep, null_image_rep, ());
+  return null_image_rep;
+}
+
+}  // namespace
 
 namespace internal {
+namespace {
+
+class Matcher {
+ public:
+  explicit Matcher(ui::ScaleFactor scale_factor) : scale_factor_(scale_factor) {
+  }
+
+  bool operator()(const ImageSkiaRep& rep) const {
+    return rep.scale_factor() == scale_factor_;
+  }
+
+ private:
+  ui::ScaleFactor scale_factor_;
+};
+
+}  // namespace
 
 // 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() {
+  ImageSkiaStorage(ImageSkiaSource* source, const gfx::Size& size)
+      : source_(source),
+        size_(size) {
   }
 
   std::vector<gfx::ImageSkiaRep>& image_reps() { return image_reps_; }
 
-  void set_size(const gfx::Size& size) { size_ = size; }
   const gfx::Size& size() const { return size_; }
 
+  // Returns the iterator of the image rep whose density best matches
+  // |scale_factor|. If the image for the |scale_factor| doesn't exist
+  // in the storage and |storage| is set, it fetches new image by calling
+  // |ImageSkiaSource::GetImageForScale|. If the source returns the
+  // image with different scale factor (if the image doesn't exist in
+  // resource, for example), it will fallback to closest image rep.
+  std::vector<ImageSkiaRep>::iterator FindRepresentation(
+      ui::ScaleFactor scale_factor, bool fetch_new_image) const {
+    ImageSkiaStorage* non_const = const_cast<ImageSkiaStorage*>(this);
+
+    float scale = ui::GetScaleFactorScale(scale_factor);
+    ImageSkia::ImageSkiaReps::iterator closest_iter =
+        non_const->image_reps().end();
+    ImageSkia::ImageSkiaReps::iterator exact_iter =
+        non_const->image_reps().end();
+    float smallest_diff = std::numeric_limits<float>::max();
+    for (ImageSkia::ImageSkiaReps::iterator it =
+             non_const->image_reps().begin();
+         it < image_reps_.end(); ++it) {
+      if (it->GetScale() == scale) {
+        // found exact match
+        fetch_new_image = false;
+        if (it->is_null())
+          continue;
+        exact_iter = it;
+        break;
+      }
+      float diff = std::abs(it->GetScale() - scale);
+      if (diff < smallest_diff && !it->is_null()) {
+        closest_iter = it;
+        smallest_diff = diff;
+      }
+    }
+
+    if (fetch_new_image && source_.get()) {
+      ImageSkiaRep image = source_->GetImageForScale(scale_factor);
+
+      // If the source returned the new image, store it.
+      if (!image.is_null() &&
+          std::find_if(image_reps_.begin(), image_reps_.end(),
+                       Matcher(image.scale_factor())) == image_reps_.end()) {
+        non_const->image_reps().push_back(image);
+      }
+
+      // If the result image's scale factor isn't same as the expected
+      // scale factor, create null ImageSkiaRep with the |scale_factor|
+      // so that the next lookup will fallback to the closest scale.
+      if (image.is_null() || image.scale_factor() != scale_factor) {
+        non_const->image_reps().push_back(
+            ImageSkiaRep(SkBitmap(), scale_factor));
+      }
+
+      // image_reps_ must have the exact much now, so find again.
+      return FindRepresentation(scale_factor, false);
+    }
+    return exact_iter != image_reps_.end() ? exact_iter : closest_iter;
+  }

[pkotwicz, 2012/07/01 22:22:23]

This code is ok. However, I would rather that if the image representation at the
desired scale factor is not available that it would be generated via a low
quality scale as I did here: http://codereview.chromium.org/10575039/

When a user does something like this
gfx::Canvas canvas(image.size(), ui::SCALE_FACTOR_200P, false);
SkBitmapOperations(c.ExtractImageRep(),
image1.GetRepresentation(ui::SCALE_FACTOR_200P))
It is much simpler if this call always succeeds.
It can fail if image1 only has a representation for ui::SCALE_FACTOR_100P.
The reason why I think it makes sense to do a low quality scale in
GetRepresentation is that we are just moving the time at which the scale is
applied from Canvas::DrawImageInt to ImageSkia::GetRepresentation.
The benefit is a cleaner API
I am ok if you do this in a separate CL though

[oshima, 2012/07/02 16:43:34]

ImageSkia::GetRepresentation doesn't guarantee that it will
return the image rep for given scale factor, and the caller must either 
do CHECK/DCHECK or handle it gracefully.

If scaling is what a user wants, we can provide such option as an utility
source, and
daisy chain them. Here is pseudo code:

class AutoScaleSoruce : public gfx::ImagSkiaSource {
 public:
  ImageSkiaRep GetImageForScale(ui::ScaleFactor scale_factor) {
    ImageSkiaRep rep = source_->GetImageForScale(scale_factor);
    if (rep.scale_factor() != scale_factor) {
      rep = ScaleImageSkiaRep(rep, cale_factor);
    return rep;
  }
  scoped_ptr<ImageSkiaSource> source_;
}

new ImageSkia(new AutoScaleSoruce(new YourImageSkiaSource());

Otherwise, it should fail early (either programmatically, or visually) so that
we can detect such issue sooner than later.

[pkotwicz, 2012/07/02 17:13:53]

I think there are two aspects.
1) We currently sometimes only have 1x images. (This is a bug because we are
missing assets). It might be useful to scale the images directly in the resource
bundle.
2) I don't know of any situation where the user would not want scaling. From
implementing the tab strip, I ran into a number of places where we were missing
2x assets. So I had to be really careful about which scale factor I requested
assets which went through SkBitmapOperations. However, I am fine if you want to
tackle this problem in a separate patch.
Btw, we currently already support HDA. (Though I should write some code to make
this slightly more explicit) So we already are not scaling for HDA code.

[pkotwicz, 2012/07/02 17:26:21]

Btw, it might be sufficient to scale the assets which do not have 2x versions to
2x. (Only do 1)

[oshima, 2012/07/02 17:26:47]

grit will scale if necessary. Even if it doesn't 1x image will be scaled in
canvas
appropriately, right?
We don't want to scale in most case and want to use correct 2x images instead.
This is transition issue and we should fix it in right way. I will handle these
issues
when I convert them.

+
  private:
   ~ImageSkiaStorage() {
   }
@@ -35,8 +122,10 @@ class ImageSkiaStorage : public base::RefCounted<ImageSkiaStorage> {
   // Vector of bitmaps and their associated scale factor.
   std::vector<gfx::ImageSkiaRep> image_reps_;
 
+  scoped_ptr<ImageSkiaSource> source_;
+
   // Size of the image in DIP.
-  gfx::Size size_;
+  const gfx::Size size_;
 
   friend class base::RefCounted<ImageSkiaStorage>;
 };
@@ -46,6 +135,10 @@ class ImageSkiaStorage : public base::RefCounted<ImageSkiaStorage> {
 ImageSkia::ImageSkia() : storage_(NULL) {
 }
 
+ImageSkia::ImageSkia(ImageSkiaSource* source, const gfx::Size& size)
+    : storage_(new internal::ImageSkiaStorage(source, size)) {
+}
+
 ImageSkia::ImageSkia(const SkBitmap& bitmap) {
   Init(ImageSkiaRep(bitmap));
 }
@@ -103,7 +196,8 @@ void ImageSkia::RemoveRepresentation(ui::ScaleFactor scale_factor) {
     return;
 
   ImageSkiaReps& image_reps = storage_->image_reps();
-  ImageSkiaReps::iterator it = FindRepresentation(scale_factor);
+  ImageSkiaReps::iterator it =
+      storage_->FindRepresentation(scale_factor, false);
   if (it != image_reps.end() && it->scale_factor() == scale_factor)
     image_reps.erase(it);
 }
@@ -112,7 +206,8 @@ bool ImageSkia::HasRepresentation(ui::ScaleFactor scale_factor) {
   if (isNull())
     return false;
 
-  ImageSkiaReps::iterator it = FindRepresentation(scale_factor);
+  ImageSkiaReps::iterator it =
+      storage_->FindRepresentation(scale_factor, false);
   return (it != storage_->image_reps().end() &&
           it->scale_factor() == scale_factor);
 }
@@ -122,7 +217,7 @@ const ImageSkiaRep& ImageSkia::GetRepresentation(
   if (isNull())
     return NullImageRep();
 
-  ImageSkiaReps::iterator it = FindRepresentation(scale_factor);
+  ImageSkiaReps::iterator it = storage_->FindRepresentation(scale_factor, true);
   if (it == storage_->image_reps().end())
     return NullImageRep();
 
@@ -137,6 +232,10 @@ int ImageSkia::width() const {
   return isNull() ? 0 : storage_->size().width();
 }
 
+gfx::Size ImageSkia::size() const {
+  return gfx::Size(width(), height());
+}
+
 int ImageSkia::height() const {
   return isNull() ? 0 : storage_->size().height();
 }
@@ -191,36 +290,9 @@ void ImageSkia::Init(const ImageSkiaRep& image_rep) {
     storage_ = NULL;
     return;
   }
-  storage_ = new internal::ImageSkiaStorage();
-  storage_->set_size(gfx::Size(image_rep.GetWidth(), image_rep.GetHeight()));
+  storage_ = new internal::ImageSkiaStorage(
+      NULL, gfx::Size(image_rep.GetWidth(), image_rep.GetHeight()));
   storage_->image_reps().push_back(image_rep);
 }
 
-// static
-ImageSkiaRep& ImageSkia::NullImageRep() {
-  CR_DEFINE_STATIC_LOCAL(ImageSkiaRep, null_image_rep, ());
-  return null_image_rep;
-}
-
-std::vector<ImageSkiaRep>::iterator ImageSkia::FindRepresentation(
-    ui::ScaleFactor scale_factor) const {
-  DCHECK(!isNull());
-
-  float scale = ui::GetScaleFactorScale(scale_factor);
-  ImageSkiaReps& image_reps = storage_->image_reps();
-  ImageSkiaReps::iterator closest_iter = image_reps.end();
-  float smallest_diff = std::numeric_limits<float>::max();
-  for (ImageSkiaReps::iterator it = image_reps.begin();
-       it < image_reps.end();
-       ++it) {
-    float diff = std::abs(it->GetScale() - scale);
-    if (diff < smallest_diff) {
-      closest_iter = it;
-      smallest_diff = diff;
-    }
-  }
-
-  return closest_iter;
-}
-
 }  // namespace gfx