Add medium image quality to software predecode.

cc/tiles/software_image_decode_controller.cc

Index: cc/tiles/software_image_decode_controller.cc
diff --git a/cc/tiles/software_image_decode_controller.cc b/cc/tiles/software_image_decode_controller.cc
index c61a0c6b5732448a0e6d9c85536183851e3f8849..33b3454b0bef6680bfd16d9cef0b71a3f3b62ce3 100644
--- a/cc/tiles/software_image_decode_controller.cc
+++ b/cc/tiles/software_image_decode_controller.cc
@@ -6,7 +6,9 @@
 
 #include <stdint.h>
 
+#include <algorithm>
 #include <functional>
+#include <limits>
 
 #include "base/macros.h"
 #include "base/memory/discardable_memory.h"
@@ -174,25 +176,6 @@ bool SoftwareImageDecodeController::GetTaskForImageAndRef(
     return false;
   }
 
-  // If we're not going to do a scale, we will just create a task to preroll the
-  // image the first time we see it. This doesn't need to account for memory.
-  // TODO(vmpstr): We can also lock the original sized image, in which case it
-  // does require memory bookkeeping.
-  if (!CanHandleImage(key)) {
-    base::AutoLock lock(lock_);
-    if (prerolled_images_.count(key.image_id()) == 0) {
-      scoped_refptr<ImageDecodeTask>& existing_task = pending_image_tasks_[key];
-      if (!existing_task) {
-        existing_task = make_scoped_refptr(
-            new ImageDecodeTaskImpl(this, key, image, prepare_tiles_id));
-      }
-      *task = existing_task;
-    } else {
-      *task = nullptr;
-    }
-    return false;
-  }
-
   base::AutoLock lock(lock_);
 
   // If we already have the image in cache, then we can return it.
@@ -264,7 +247,6 @@ void SoftwareImageDecodeController::UnrefImage(const DrawImage& image) {
   //       it yet (or failed to decode it).
   //   2b. Unlock the image but keep it in list.
   const ImageKey& key = ImageKey::FromDrawImage(image);
-  DCHECK(CanHandleImage(key));
   TRACE_EVENT1("disabled-by-default-cc.debug",
                "SoftwareImageDecodeController::UnrefImage", "key",
                key.ToString());
@@ -295,20 +277,6 @@ void SoftwareImageDecodeController::DecodeImage(const ImageKey& key,
                                                 const DrawImage& image) {
   TRACE_EVENT1("cc", "SoftwareImageDecodeController::DecodeImage", "key",
                key.ToString());
-  if (!CanHandleImage(key)) {
-    image.image()->preroll();
-
-    base::AutoLock lock(lock_);
-    prerolled_images_.insert(key.image_id());
-    // Erase the pending task from the queue, since the task won't be doing
-    // anything useful after this function terminates. Since we don't preroll
-    // images twice, this is actually not necessary but it behaves similar to
-    // the other code path: when this function finishes, the task isn't in the
-    // pending_image_tasks_ list.
-    pending_image_tasks_.erase(key);
-    return;
-  }
-
   base::AutoLock lock(lock_);
   AutoRemoveKeyFromTaskMap remove_key_from_task_map(&pending_image_tasks_, key);
 
@@ -365,120 +333,99 @@ void SoftwareImageDecodeController::DecodeImage(const ImageKey& key,
 }
 
 scoped_ptr<SoftwareImageDecodeController::DecodedImage>
-SoftwareImageDecodeController::DecodeImageInternal(
-    const ImageKey& key,
-    const DrawImage& draw_image) {
-  TRACE_EVENT1("disabled-by-default-cc.debug",
-               "SoftwareImageDecodeController::DecodeImageInternal", "key",
-               key.ToString());
-  const SkImage* image = draw_image.image();
-
-  // If we can use the original decode, then we don't need to do scaling. We can
-  // just read pixels into the final memory.
-  if (key.can_use_original_decode()) {
-    SkImageInfo decoded_info =
-        CreateImageInfo(image->width(), image->height(), format_);
-    scoped_ptr<base::DiscardableMemory> decoded_pixels;
-    {
-      TRACE_EVENT0(
-          "disabled-by-default-cc.debug",
-          "SoftwareImageDecodeController::DecodeImageInternal - allocate "
-          "decoded pixels");
-      decoded_pixels =
-          base::DiscardableMemoryAllocator::GetInstance()
-              ->AllocateLockedDiscardableMemory(decoded_info.minRowBytes() *
-                                                decoded_info.height());
-    }
-    {
-      TRACE_EVENT0(
-          "disabled-by-default-cc.debug",
-          "SoftwareImageDecodeController::DecodeImageInternal - read pixels");
-      bool result = image->readPixels(decoded_info, decoded_pixels->data(),
-                                      decoded_info.minRowBytes(), 0, 0,
-                                      SkImage::kDisallow_CachingHint);
+SoftwareImageDecodeController::DecodeImageMediumQuality(const ImageKey& key,
+                                                        const SkImage& image) {
+  // We need an original decode to either do a scale or extract a subrect
+  // from the image.
+  auto decoded_image_result = DecodeImageOrUseCache(key, image);
+  if (!decoded_image_result.decoded_pixmap_.addr()) {
+    return nullptr;
+  }
 
-      if (!result) {
-        decoded_pixels->Unlock();
-        return nullptr;
+  // Generate a key and scaled image for each mipmap level.
+  gfx::Rect src_rect = key.src_rect();
+  int src_height = src_rect.height();
+  int src_width = src_rect.width();
+
+  std::unique_ptr<DecodedImage> last_mip_scaled_image;
+  std::unique_ptr<DecodedImage> target_mip_image;
+  for (int current_mip_level = 0;; current_mip_level++) {
+    int mip_height = std::max(1, src_height / (1 << current_mip_level));
+    int mip_width = std::max(1, src_width / (1 << current_mip_level));
+    SkScalar y_scale = static_cast<float>(mip_height) / src_height;
+    SkScalar x_scale = static_cast<float>(mip_width) / src_width;
+
+    // Generate an image and key to cache it
+    DrawImage mip_image(&image, gfx::RectToSkIRect(src_rect),
+                        kMedium_SkFilterQuality,

[vmpstr, 2016/03/29 21:48:04]

Passing medium here would make skia also generate mipmaps during scaling, no?
I'm not sure what pixmap scalePixels would do with medium quality otherwise.

[cblume, 2016/03/30 00:31:08]

Yeah. This is sort of a placeholder until we figure out what we need from Skia.

If we only want to generate 1 mip level then that changes significantly what we
need from Skia. And I think I might like the idea of generating them on demand
(as opposed to generating them all in advance).

+                        SkMatrix::MakeScale(x_scale, y_scale));
+    auto mip_key = ImageKey::FromDrawImage(mip_image);
+    auto mip_scaled_image = ScaleImage(mip_key, decoded_image_result);
+
+    // Check if this is the first mipmap smaller than target.
+    // If so, use the mip level below on the mip stack.
+    // This effectively always uses the larger image and always scales down.
+    if (mip_height < key.target_size().height() ||
+        mip_width < key.target_size().width()) {
+      if (target_mip_image == nullptr) {
+        target_mip_image = std::move(last_mip_scaled_image);

[vmpstr, 2016/03/29 21:48:04]

Should we just break here?

[cblume, 2016/03/30 00:31:08]

If we are only generating one level (which I like), then yes.

       }
     }
+    last_mip_scaled_image =
+        std::unique_ptr<DecodedImage>(mip_scaled_image.release());

[vmpstr, 2016/03/29 21:48:04]

is this just a std::move?

 
-    return make_scoped_ptr(new DecodedImage(
-        decoded_info, std::move(decoded_pixels), SkSize::Make(0, 0)));
+    if (mip_height == 1 && mip_width == 1) {
+      // We have reached the final mip level
+      break;
+    }
   }
 
-  // If we get here, that means we couldn't use the original sized decode for
-  // whatever reason. However, in all cases we do need an original decode to
-  // either do a scale or to extract a subrect from the image. So, what we can
-  // do is construct a key that would require a full sized decode, then get that
-  // decode via GetDecodedImageForDrawInternal(), use it, and unref it. This
-  // ensures that if the original sized decode is already available in any of
-  // the caches, we reuse that. We also ensure that all the proper locking takes
-  // place. If, on the other hand, the decode was not available,
-  // GetDecodedImageForDrawInternal() would decode the image, and unreffing it
-  // later ensures that we will store the discardable memory unlocked in the
-  // cache to be used by future requests.
-  gfx::Rect full_image_rect(image->width(), image->height());
-  DrawImage original_size_draw_image(image, gfx::RectToSkIRect(full_image_rect),
-                                     kNone_SkFilterQuality, SkMatrix::I());
-  ImageKey original_size_key =
-      ImageKey::FromDrawImage(original_size_draw_image);
-  // Sanity checks.
-  DCHECK(original_size_key.can_use_original_decode());
-  DCHECK(full_image_rect.size() == original_size_key.target_size());
+  return make_scoped_ptr<DecodedImage>(target_mip_image.release());

[vmpstr, 2016/03/29 21:48:04]

just return target_mip_image;?

+}
 
-  auto decoded_draw_image = GetDecodedImageForDrawInternal(
-      original_size_key, original_size_draw_image);
-  if (!decoded_draw_image.image()) {
-    DrawWithImageFinished(original_size_draw_image, decoded_draw_image);
+scoped_ptr<SoftwareImageDecodeController::DecodedImage>
+SoftwareImageDecodeController::DecodeImageHighQuality(const ImageKey& key,
+                                                      const SkImage& image) {
+  // We need an original decode to either do a scale or to extract a subrect
+  // from the image.
+  auto decoded_image_result = DecodeImageOrUseCache(key, image);
+  if (!decoded_image_result.decoded_pixmap_.addr()) {
     return nullptr;
   }
 
-  SkPixmap decoded_pixmap;
-  bool result = decoded_draw_image.image()->peekPixels(&decoded_pixmap);
-  DCHECK(result);
-  if (key.src_rect() != full_image_rect) {
-    result = decoded_pixmap.extractSubset(&decoded_pixmap,
-                                          gfx::RectToSkIRect(key.src_rect()));
-    DCHECK(result);
-  }
 
   // Now we have a decoded_pixmap which represents the src_rect at the
   // original scale. All we need to do is scale it.
-  DCHECK(!key.target_size().IsEmpty());
-  SkImageInfo scaled_info = CreateImageInfo(
-      key.target_size().width(), key.target_size().height(), format_);
-  scoped_ptr<base::DiscardableMemory> scaled_pixels;
-  {
-    TRACE_EVENT0(
-        "disabled-by-default-cc.debug",
-        "SoftwareImageDecodeController::DecodeImageInternal - allocate "
-        "scaled pixels");
-    scaled_pixels = base::DiscardableMemoryAllocator::GetInstance()
-                        ->AllocateLockedDiscardableMemory(
-                            scaled_info.minRowBytes() * scaled_info.height());
-  }
-  SkPixmap scaled_pixmap(scaled_info, scaled_pixels->data(),
-                         scaled_info.minRowBytes());
-  // TODO(vmpstr): Start handling more than just high filter quality.
-  DCHECK_EQ(kHigh_SkFilterQuality, key.filter_quality());
-  {
-    TRACE_EVENT0(
-        "disabled-by-default-cc.debug",
-        "SoftwareImageDecodeController::DecodeImageInternal - scale pixels");
-    bool result =
-        decoded_pixmap.scalePixels(scaled_pixmap, key.filter_quality());
-    DCHECK(result);
+  return ScaleImage(key, decoded_image_result);
+}
+
+scoped_ptr<SoftwareImageDecodeController::DecodedImage>
+SoftwareImageDecodeController::DecodeImageInternal(
+    const ImageKey& key,
+    const DrawImage& draw_image) {
+  TRACE_EVENT1("disabled-by-default-cc.debug",
+               "SoftwareImageDecodeController::DecodeImageInternal", "key",
+               key.ToString());
+  const SkImage* image = draw_image.image();
+  if (!image) {
+    return nullptr;
   }
 
-  // Release the original sized decode. Any other intermediate result to release
-  // would be the subrect memory. However, that's in a scoped_ptr and will be
-  // deleted automatically when we return.
-  DrawWithImageFinished(original_size_draw_image, decoded_draw_image);
+  switch (key.filter_quality()) {
+    case kNone_SkFilterQuality:
+    // fall through
+    case kLow_SkFilterQuality:
+      DCHECK(key.can_use_original_decode());
 
-  return make_scoped_ptr(
-      new DecodedImage(scaled_info, std::move(scaled_pixels),
-                       SkSize::Make(-key.src_rect().x(), -key.src_rect().y())));
+      return AttemptToUseOriginalImage(key, *image);
+    case kMedium_SkFilterQuality:
+      return DecodeImageMediumQuality(key, *image);
+    case kHigh_SkFilterQuality:
+      return DecodeImageHighQuality(key, *image);
+    default:
+      NOTREACHED();
+      return nullptr;
+  }
 }
 
 DecodedDrawImage SoftwareImageDecodeController::GetDecodedImageForDraw(
@@ -491,9 +438,6 @@ DecodedDrawImage SoftwareImageDecodeController::GetDecodedImageForDraw(
   if (key.target_size().IsEmpty())
     return DecodedDrawImage(nullptr, kNone_SkFilterQuality);
 
-  if (!CanHandleImage(key))
-    return DecodedDrawImage(draw_image.image(), draw_image.filter_quality());
-
   return GetDecodedImageForDrawInternal(key, draw_image);
 }
 
@@ -549,7 +493,11 @@ DecodedDrawImage SoftwareImageDecodeController::GetDecodedImageForDrawInternal(
     // on the compositor thread. This means holding on to the lock might stall
     // the compositor thread for the duration of the decode!
     base::AutoUnlock unlock(lock_);
-    scoped_decoded_image = DecodeImageInternal(key, draw_image);
+    const SkImage* image = draw_image.image();
+    if (!image) {
+      return DecodedDrawImage(nullptr, kNone_SkFilterQuality);
+    }
+    scoped_decoded_image = AttemptToUseOriginalImage(key, *image);
     decoded_image = scoped_decoded_image.get();
 
     // Skip the image if we couldn't decode it.
@@ -590,6 +538,128 @@ DecodedDrawImage SoftwareImageDecodeController::GetDecodedImageForDrawInternal(
   return decoded_draw_image;
 }
 
+SoftwareImageDecodeController::DecodedImageResult::DecodedImageResult(
+    SkPixmap decoded_pixmap,
+    DrawImage original_size_draw_image,
+    DecodedDrawImage decoded_draw_image)
+    : decoded_pixmap_(decoded_pixmap),
+      original_size_draw_image_(original_size_draw_image),
+      decoded_draw_image_(decoded_draw_image) {}
+
+scoped_ptr<SoftwareImageDecodeController::DecodedImage>
+SoftwareImageDecodeController::AttemptToUseOriginalImage(const ImageKey& key,
+                                                         const SkImage& image) {
+  TRACE_EVENT1("disabled-by-default-cc.debug",
+               "SoftwareImageDecodeController::AttemptToUseOriginalImage",
+               "key", key.ToString());
+  if (!key.can_use_original_decode()) {
+    return nullptr;
+  } else {
+    SkImageInfo decoded_info =
+        CreateImageInfo(image.width(), image.height(), format_);
+    scoped_ptr<base::DiscardableMemory> decoded_pixels;
+    {
+      TRACE_EVENT0("disabled-by-default-cc.debug",
+                   "SoftwareImageDecodeController::AttemptToUseOriginalImage - "
+                   "allocate decoded pixels");
+      decoded_pixels =
+          base::DiscardableMemoryAllocator::GetInstance()
+              ->AllocateLockedDiscardableMemory(decoded_info.minRowBytes() *
+                                                decoded_info.height());
+    }
+    {
+      TRACE_EVENT0("disabled-by-default-cc.debug",
+                   "SoftwareImageDecodeController::AttemptToUseOriginalImage - "
+                   "read pixels");
+      bool result = image.readPixels(decoded_info, decoded_pixels->data(),
+                                     decoded_info.minRowBytes(), 0, 0,
+                                     SkImage::kDisallow_CachingHint);
+
+      if (!result) {
+        decoded_pixels->Unlock();
+        return nullptr;
+      }
+    }
+
+    return make_scoped_ptr(new DecodedImage(
+        decoded_info, std::move(decoded_pixels), SkSize::Make(0, 0)));
+  }
+}
+
+SoftwareImageDecodeController::DecodedImageResult
+SoftwareImageDecodeController::DecodeImageOrUseCache(const ImageKey& key,
+                                                     const SkImage& image) {
+  // Construct a key to use in GetDecodedImageForDrawInternal().
+  // This allows us to reuse an image in any cache if available.
+  gfx::Rect full_image_rect(image.width(), image.height());
+  DrawImage original_size_draw_image(&image,
+                                     gfx::RectToSkIRect(full_image_rect),
+                                     kNone_SkFilterQuality, SkMatrix::I());
+  ImageKey original_size_key =
+      ImageKey::FromDrawImage(original_size_draw_image);
+  // Sanity checks.
+  DCHECK(original_size_key.can_use_original_decode());
+  DCHECK(full_image_rect.size() == original_size_key.target_size());
+
+  auto decoded_draw_image = GetDecodedImageForDrawInternal(
+      original_size_key, original_size_draw_image);
+
+  if (!decoded_draw_image.image()) {
+    DrawWithImageFinished(original_size_draw_image, decoded_draw_image);
+    return DecodedImageResult(SkPixmap(), DrawImage(),
+                              DecodedDrawImage(nullptr, kNone_SkFilterQuality));
+  }
+
+  SkPixmap decoded_pixmap;
+  bool result = decoded_draw_image.image()->peekPixels(&decoded_pixmap);
+  DCHECK(result);
+  if (key.src_rect() != full_image_rect) {
+    result = decoded_pixmap.extractSubset(&decoded_pixmap,
+                                          gfx::RectToSkIRect(key.src_rect()));
+    DCHECK(result);
+  }
+
+  return DecodedImageResult(decoded_pixmap, original_size_draw_image,
+                            decoded_draw_image);
+}
+
+scoped_ptr<SoftwareImageDecodeController::DecodedImage>
+SoftwareImageDecodeController::ScaleImage(
+    const ImageKey& key,
+    const DecodedImageResult& decoded_image_result) {
+  DCHECK(!key.target_size().IsEmpty());
+  SkImageInfo scaled_info = CreateImageInfo(
+      key.target_size().width(), key.target_size().height(), format_);
+  scoped_ptr<base::DiscardableMemory> scaled_pixels;
+  {
+    TRACE_EVENT0(
+        "disabled-by-default-cc.debug",
+        "SoftwareImageDecodeController::ScaleImage - allocate scaled pixels");
+    scaled_pixels = base::DiscardableMemoryAllocator::GetInstance()
+                        ->AllocateLockedDiscardableMemory(
+                            scaled_info.minRowBytes() * scaled_info.height());
+  }
+  SkPixmap scaled_pixmap(scaled_info, scaled_pixels->data(),
+                         scaled_info.minRowBytes());
+  {
+    TRACE_EVENT0("disabled-by-default-cc.debug",
+                 "SoftwareImageDecodeController::ScaleImage - scale pixels");
+    bool result = decoded_image_result.decoded_pixmap_.scalePixels(
+        scaled_pixmap, key.filter_quality());
+    DCHECK(result);
+  }
+
+  // Release the original sized decode. Any other intermediate result to release
+  // would be the subrect memory. However, that's in a scoped_ptr and will be
+  // deleted automatically when we return.
+  DrawWithImageFinished(decoded_image_result.original_size_draw_image_,
+                        decoded_image_result.decoded_draw_image_);
+
+  return make_scoped_ptr(new SoftwareImageDecodeController::DecodedImage(
+      scaled_info, std::move(scaled_pixels),
+      SkSize::Make(-key.src_rect().x(), -key.src_rect().y())));
+}
+
 void SoftwareImageDecodeController::DrawWithImageFinished(
     const DrawImage& image,
     const DecodedDrawImage& decoded_image) {
@@ -597,7 +667,7 @@ void SoftwareImageDecodeController::DrawWithImageFinished(
                "SoftwareImageDecodeController::DrawWithImageFinished", "key",
                ImageKey::FromDrawImage(image).ToString());
   ImageKey key = ImageKey::FromDrawImage(image);
-  if (!decoded_image.image() || !CanHandleImage(key))
+  if (!decoded_image.image())
     return;
 
   if (decoded_image.is_at_raster_decode())
@@ -664,11 +734,6 @@ void SoftwareImageDecodeController::UnrefAtRasterImage(const ImageKey& key) {
   }
 }
 
-bool SoftwareImageDecodeController::CanHandleImage(const ImageKey& key) {
-  // TODO(vmpstr): Start handling medium filter quality as well.
-  return key.filter_quality() != kMedium_SkFilterQuality;
-}
-
 void SoftwareImageDecodeController::ReduceCacheUsage() {
   TRACE_EVENT0("cc", "SoftwareImageDecodeController::ReduceCacheUsage");
   base::AutoLock lock(lock_);