Resize images for hi-dpi based on a custom PNG chunk added by GRIT r78, and roll GRIT r78

ui/base/resource/resource_bundle.cc

Index: ui/base/resource/resource_bundle.cc
diff --git a/ui/base/resource/resource_bundle.cc b/ui/base/resource/resource_bundle.cc
index f8d214fe81f4509781175a1cebe6a0a4498ba46f..81d5b3d0615f38dd3d4342b2157d101cd166af71 100644
--- a/ui/base/resource/resource_bundle.cc
+++ b/ui/base/resource/resource_bundle.cc
@@ -47,6 +47,13 @@ bool ShouldHighlightMissingScaledResources() {
       switches::kHighlightMissingScaledResources);
 }
 
+int ScaleDimension(int in, int numerator, int denominator) {
+  DCHECK(0 < in && in < 65536);
+  DCHECK(100 <= numerator && numerator <= 200);
+  DCHECK(100 <= denominator && denominator <= 200);
+  return (in * numerator + (denominator / 2)) / denominator;
+}
+
 }  // namespace
 
 // An ImageSkiaSource that loads bitmaps for requested scale factor from
@@ -59,78 +66,59 @@ bool ShouldHighlightMissingScaledResources() {
 // will show the scaled image blended with red instead.
 class ResourceBundle::ResourceBundleImageSource : public gfx::ImageSkiaSource {
  public:
-  ResourceBundleImageSource(ResourceBundle* rb,
-                            int resource_id,
-                            const gfx::Size& size_in_dip)
-      : rb_(rb),
-        resource_id_(resource_id),
-        size_in_dip_(size_in_dip) {
-  }
+  ResourceBundleImageSource(ResourceBundle* rb, int resource_id)
+      : rb_(rb), resource_id_(resource_id) {}
   virtual ~ResourceBundleImageSource() {}
 
   // gfx::ImageSkiaSource overrides:
   virtual gfx::ImageSkiaRep GetImageForScale(
       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);
-
-    if (scale_factor != SCALE_FACTOR_100P &&
-        (!result.get() ||
-         result->width() != size_in_pixel.width() ||
-         result->height() != size_in_pixel.height())) {
-
-      // If non 1x resource is missing from |image| or is the incorrect
-      // size and --highlight-missing-scaled-resources is specified, logs
-      // the resource id and creates a version of the resource at the correct
-      // size. Blends the created resource with red to make it
-      // distinguishable from bitmaps in the resource pak.
+    int percent = static_cast<int>(
+        GetScaleFactorScale(scale_factor) * 100 + 0.5);
+    SkBitmap image;
+    int image_percent;
+    bool found = rb_->LoadBitmap(
+        resource_id_, scale_factor, &image, &image_percent);

[oshima, 2012/10/05 20:33:11]

We need scale info only in the images that fell back, don't we?

[oshima, 2012/10/07 02:42:21]

And in that case, all we need is a simple marker instead of
scale info.

[benrg, 2012/10/07 04:36:11]

Yes, when we add a csCl chunk the scale is always 100% because we only fall back
to 100%. I chose to store the scale anyway, but I could change that if you want.

[oshima, 2012/10/08 17:32:15]

Since we'll never store scale other than 100, let's keep it simple and store a
bit.

[benrg, 2012/10/09 00:07:06]

Done.

+
+    if (!found) {
+      // Some images (e.g. wallpapers) exist only at 100%. Return the unscaled
+      // image with the correct scale factor.

[oshima, 2012/10/08 17:32:15]

you can just return empty ImageSkiaReps and it will fallback to 100P

[benrg, 2012/10/09 00:07:06]

I left this as is for now because the new code below depends on the current
behavior.

+      DCHECK_NE(scale_factor, SCALE_FACTOR_100P);
+      found = rb_->LoadBitmap(
+          resource_id_, SCALE_FACTOR_100P, &image, &image_percent);
+      DCHECK(found);

[oshima, 2012/10/08 17:32:15]

I don't think this DCHECK is useful.

+      return gfx::ImageSkiaRep(image, SCALE_FACTOR_100P);
+    }
+
+    if (image_percent > 0 && percent != image_percent) {
+      // The scale stored in the image (normally 100%) doesn't match the
+      // requested scale: rescale the image.
+      image = skia::ImageOperations::Resize(
+          image,
+          skia::ImageOperations::RESIZE_LANCZOS3,
+          ScaleDimension(image.width(), percent, image_percent),
+          ScaleDimension(image.height(), percent, image_percent));
+      // If --highlight-missing-scaled-resources is specified, log the resource
+      // id and blend the created resource with red.
       if (ShouldHighlightMissingScaledResources()) {
-        if (!result.get()) {
-          LOG(ERROR) << "Missing " << scale << "x resource. id="
-                     << resource_id_;
-        } else {
-          LOG(ERROR) << "Incorrectly sized " << scale << "x resource. id="
-                     << resource_id_;
-        }
-
-        scoped_ptr<SkBitmap> bitmap1x(
-            rb_->LoadBitmap(resource_id_, SCALE_FACTOR_100P));
-        DCHECK(bitmap1x.get());
-        SkBitmap bitmap_scaled = skia::ImageOperations::Resize(
-            *bitmap1x,
-            skia::ImageOperations::RESIZE_LANCZOS3,
-            size_in_pixel.width(),
-            size_in_pixel.height());
+        LOG(ERROR) << "Missing " << percent << "% scaled resource. id="
+                   << resource_id_;
 
         SkBitmap mask;
         mask.setConfig(SkBitmap::kARGB_8888_Config,
-                       bitmap_scaled.width(),
-                       bitmap_scaled.height());
+                       image.width(), image.height());
         mask.allocPixels();
         mask.eraseColor(SK_ColorRED);
-        result.reset(new SkBitmap());
-        *result.get() = SkBitmapOperations::CreateBlendedBitmap(
-            bitmap_scaled, mask, 0.2);
-      } else if (!result.get() || result->width() == size_in_dip_.width()) {
-        // The scaled resource pack may have the 1x image if its grd file
-        // points to 1x image. Fallback to 1x by returning empty image
-        // in this case. This 1x image will be scaled when drawn.
-        return gfx::ImageSkiaRep();
+        image = SkBitmapOperations::CreateBlendedBitmap(image, mask, 0.2);
       }
-      // If the size of scaled image isn't exactly |scale| * 1x version,
-      // create ImageSkia as usual. This will end up with
-      // corrupted visual representation as the size of image doesn't
-      // match the expected size.
     }
-    DCHECK(result.get());
-    return gfx::ImageSkiaRep(*result.get(), scale_factor);
+
+    return gfx::ImageSkiaRep(image, scale_factor);
   }
 
  private:
   ResourceBundle* rb_;
   const int resource_id_;
-  const gfx::Size size_in_dip_;
 
   DISALLOW_COPY_AND_ASSIGN(ResourceBundleImageSource);
 };
@@ -344,24 +332,27 @@ gfx::Image& ResourceBundle::GetImageNamed(int resource_id) {
     DCHECK(!delegate_ && !data_packs_.empty()) <<
         "Missing call to SetResourcesDataDLL?";
 
-    // TODO(oshima): Pick the scale factor from currently used scale factors.
-    scoped_ptr<SkBitmap> bitmap(LoadBitmap(resource_id, SCALE_FACTOR_100P));
-    if (!bitmap.get()) {
+    // TODO(benrg): We only need this image for its dimensions. Do something
+    // more efficient.

[oshima, 2012/10/07 02:42:21]

I wanted to know if the image has fallen back to 1x in.
http://codereview.chromium.org/10914092

Let's keep other resouce_bundle.cc change separate,
and finish marker change first.

[benrg, 2012/10/07 04:36:11]

Are you saying that my changes here are okay as is?

[oshima, 2012/10/08 17:32:15]

I want to change so that it will load at the target scale here, (as in
http://codereview.chromium.org/10914092), so I'd like to keep this method
as is for now.

[benrg, 2012/10/09 00:07:06]

I folded some of the code from that CL into this one. Let me know what you
think. The call to GetPrimaryDisplay() is commented out because it crashes at
startup and I'm not sure how to fix that, but the code works as written and
should be easier to fix than what I had before.

[oshima, 2012/10/09 18:28:35]

looks ok. Can you send me stack trace of the crash you had?

+    SkBitmap image1x;
+    int image1x_percent;
+    bool found_1x = LoadBitmap(resource_id, SCALE_FACTOR_100P,
+                               &image1x, &image1x_percent);
+    if (!found_1x) {
       LOG(WARNING) << "Unable to load image with id " << resource_id;
       NOTREACHED();  // Want to assert in debug mode.
       // The load failed to retrieve the image; show a debugging red square.
       return GetEmptyImage();
     }
+    DCHECK(image1x_percent == -1 || image1x_percent == 100);
+    gfx::Size size_in_dip(image1x.width(), image1x.height());
 
     // ResourceBundle::GetSharedInstance() is destroyed after the
     // BrowserMainLoop has finished running. |image_skia| is guaranteed to be
     // destroyed before the resource bundle is destroyed.
-    gfx::Size size_in_dip(bitmap->width(), bitmap->height());
-    gfx::ImageSkia image_skia(
-        new ResourceBundleImageSource(this, resource_id, size_in_dip),
-        size_in_dip);
-    image_skia.AddRepresentation(gfx::ImageSkiaRep(*bitmap.get(),
-                                                   SCALE_FACTOR_100P));
+    gfx::ImageSkia image_skia(new ResourceBundleImageSource(this, resource_id),
+                              size_in_dip);
+    image_skia.AddRepresentation(gfx::ImageSkiaRep(image1x, SCALE_FACTOR_100P));
     image_skia.SetReadOnly();
     image = gfx::Image(image_skia);
   }
@@ -589,37 +580,42 @@ void ResourceBundle::LoadFontsIfNecessary() {
   }
 }
 
-SkBitmap* ResourceBundle::LoadBitmap(const ResourceHandle& data_handle,
-                                     int resource_id) const {
+bool ResourceBundle::LoadBitmap(const ResourceHandle& data_handle,
+                                int resource_id,
+                                SkBitmap* bitmap,
+                                int* percent) const {
   scoped_refptr<base::RefCountedMemory> memory(
       data_handle.GetStaticMemory(resource_id));
   if (!memory)
-    return NULL;
+    return false;
 
-  SkBitmap bitmap;
-  if (gfx::PNGCodec::Decode(memory->front(), memory->size(), &bitmap))
-    return new SkBitmap(bitmap);
+  if (gfx::PNGCodec::Decode(memory->front(), memory->size(), bitmap, percent))
+    return true;
 
   // 99% of our assets are PNGs, however fallback to JPEG.
-  SkBitmap* allocated_bitmap =
-      gfx::JPEGCodec::Decode(memory->front(), memory->size());
-  if (allocated_bitmap)
-    return allocated_bitmap;
+  scoped_ptr<SkBitmap> jpeg_bitmap(
+      gfx::JPEGCodec::Decode(memory->front(), memory->size()));
+  if (jpeg_bitmap.get()) {
+    bitmap->swap(*jpeg_bitmap.get());
+    *percent = -1;
+    return true;
+  }
 
   NOTREACHED() << "Unable to decode theme image resource " << resource_id;
-  return NULL;
+  return false;
 }
 
-SkBitmap* ResourceBundle::LoadBitmap(int resource_id,
-                                     ScaleFactor scale_factor) const {
+bool ResourceBundle::LoadBitmap(int resource_id,
+                                ScaleFactor scale_factor,
+                                SkBitmap* bitmap,
+                                int* percent) const {

[oshima, 2012/10/05 20:33:11]

I think it's better to use ScaleFactor rather than int, and rejects scales that
we don't support.
We can use scale_factor in/out parameter too.

[benrg, 2012/10/07 04:36:11]

Done.

   for (size_t i = 0; i < data_packs_.size(); ++i) {
     if (data_packs_[i]->GetScaleFactor() == scale_factor) {
-      SkBitmap* bitmap = LoadBitmap(*data_packs_[i], resource_id);
-      if (bitmap)
-        return bitmap;
+      if (LoadBitmap(*data_packs_[i], resource_id, bitmap, percent))
+        return true;
     }
   }
-  return NULL;
+  return false;
 }
 
 gfx::Image& ResourceBundle::GetEmptyImage() {