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

 // 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/base/resource/resource_bundle.h"
 
 #include <vector>
 
 #include "base/command_line.h"
 #include "base/file_util.h"
@@ skipping 29 matching lines @@
 const int kMediumFontSizeDelta = 3;
 const int kLargeFontSizeDelta = 8;
 
 ResourceBundle* g_shared_instance_ = NULL;
 
 bool ShouldHighlightMissingScaledResources() {
   return CommandLine::ForCurrentProcess()->HasSwitch(
       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
 // ResourceBundle on demand for given resource_id. It falls back
 // to the 1x bitmap if the bitmap for the requested scale factor does not
 // exist. If the resource for the requested scale factor is not exactly
 // |scale_factor| * the size of the 1x resource, it will end up with
 // broken UI because it will be drawn as if the bitmap was the correct size.
 // When --highlight-missing-scaled-resources flag is specified, it
 // 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);
+    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 (scale_factor != SCALE_FACTOR_100P &&
-        (!result.get() ||
-         result->width() != size_in_pixel.width() ||
-         result->height() != size_in_pixel.height())) {
+    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 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.
+    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);
 };
 
 // static
 std::string ResourceBundle::InitSharedInstanceWithLocale(
     const std::string& pref_locale, Delegate* delegate) {
   DCHECK(g_shared_instance_ == NULL) << "ResourceBundle initialized twice";
   g_shared_instance_ = new ResourceBundle(delegate);
 
@@ skipping 193 matching lines @@
   }
 
   gfx::Image image;
   if (delegate_)
     image = delegate_->GetImageNamed(resource_id);
 
   if (image.IsEmpty()) {
     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);
   }
 
   // The load was successful, so cache the image.
   base::AutoLock lock_scope(*images_and_fonts_lock_);
 
   // Another thread raced the load and has already cached the image.
   if (images_.count(resource_id))
     return images_[resource_id];
@@ skipping 207 matching lines @@
 
     if (!large_bold_font_.get()) {
        large_bold_font_.reset(new gfx::Font());
       *large_bold_font_ =
           base_font_->DeriveFont(kLargeFontSizeDelta,
                                  base_font_->GetStyle() | gfx::Font::BOLD);
     }
   }
 }
 
-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() {
   base::AutoLock lock(*images_and_fonts_lock_);
 
   if (empty_image_.IsEmpty()) {
     // The placeholder bitmap is bright red so people notice the problem.
     SkBitmap bitmap;
     bitmap.setConfig(SkBitmap::kARGB_8888_Config, 32, 32);
     bitmap.allocPixels();
     bitmap.eraseARGB(255, 255, 0, 0);
     empty_image_ = gfx::Image(bitmap);
   }
   return empty_image_;
 }
 
 }  // namespace ui