Add medium image quality to software predecode.

cc/tiles/software_image_decode_controller.cc

 // Copyright 2015 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 "cc/tiles/software_image_decode_controller.h"
 
 #include <stdint.h>
 
+#include <algorithm>
 #include <functional>
 
 #include "base/format_macros.h"
 #include "base/macros.h"
 #include "base/memory/discardable_memory.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/trace_event/memory_dump_manager.h"
 #include "cc/debug/devtools_instrumentation.h"
@@ skipping 70 matching lines @@
  private:
   SoftwareImageDecodeController* controller_;
   SoftwareImageDecodeController::ImageKey image_key_;
   DrawImage image_;
   skia::RefPtr<const SkImage> image_ref_;
   uint64_t source_prepare_tiles_id_;
 
   DISALLOW_COPY_AND_ASSIGN(ImageDecodeTaskImpl);
 };
 
+SkMatrix GetMipMapScale(const SoftwareImageDecodeController::ImageKey& key) {

[vmpstr, 2016/04/21 20:11:30]

Can you add a comment here to describe what the function does and what it
returns?

Should this be return SkSize and then the caller builds a matrix out of it if
they want?

Maybe GetMipMapScaleAdjustment or something?

[cblume, 2016/04/22 01:40:14]

Done.

+  gfx::Rect src_rect = key.src_rect();
+  int src_height = src_rect.height();
+  int src_width = src_rect.width();
+
+  int next_mip_height = src_height;
+  int next_mip_width = src_width;
+  for (int current_mip_level = 0;; current_mip_level++) {
+    int mip_height = next_mip_height;
+    int mip_width = next_mip_width;
+
+    next_mip_height = std::max(1, src_height / (1 << (current_mip_level + 1)));
+    next_mip_width = std::max(1, src_width / (1 << (current_mip_level + 1)));
+
+    // Check if an axis on the next mip level would be smaller than the target.
+    // If so, use the current mip level.
+    // This effectively always uses the larger image and always scales down.
+    if (next_mip_height < key.target_size().height() ||
+        next_mip_width < key.target_size().width()) {
+      SkScalar y_scale = 1.f;
+      SkScalar x_scale = 1.f;
+      if (current_mip_level != 0) {
+        y_scale = static_cast<float>(mip_height) / src_height;
+        x_scale = static_cast<float>(mip_width) / src_width;
+      }
+
+      return SkMatrix::MakeScale(x_scale, y_scale);
+    }
+
+    if (mip_height == 1 && mip_width == 1) {
+      // We have reached the final mip level
+      break;
+    }
+  }
+
+  return SkMatrix::MakeScale(0, 0);
+}
+
 SkSize GetScaleAdjustment(const ImageDecodeControllerKey& key) {
   // If the requested filter quality did not require scale, then the adjustment
   // is identity.
-  if (key.can_use_original_decode())

[vmpstr, 2016/04/21 20:11:30]

I'd kind of prefer if we keep this type of logic:

if (foo)
  return blah;
...
...

instead of

if (!foo) {
  ...
  ...
}
return blah;


but it's really up to you.

[cblume, 2016/04/22 01:40:14]

Done.

-    return SkSize::Make(1.f, 1.f);
+  float x_scale = 1.f;
+  float y_scale = 1.f;
 
-  float x_scale =
-      key.target_size().width() / static_cast<float>(key.src_rect().width());
-  float y_scale =
-      key.target_size().height() / static_cast<float>(key.src_rect().height());
+  if (!key.can_use_original_decode()) {
+    if (key.filter_quality() == kMedium_SkFilterQuality) {
+      SkMatrix mipmap_scale = GetMipMapScale(key);
+      x_scale = mipmap_scale.getScaleX();
+      y_scale = mipmap_scale.getScaleY();
+    } else {
+      x_scale = key.target_size().width() /
+                static_cast<float>(key.src_rect().width());
+      y_scale = key.target_size().height() /
+                static_cast<float>(key.src_rect().height());
+    }
+  }
   return SkSize::Make(x_scale, y_scale);
 }
 
 SkFilterQuality GetDecodedFilterQuality(const ImageDecodeControllerKey& key) {
   return std::min(key.filter_quality(), kLow_SkFilterQuality);
 }
 
 SkImageInfo CreateImageInfo(size_t width,
                             size_t height,
                             ResourceFormat format) {
   return SkImageInfo::Make(width, height,
                            ResourceFormatToClosestSkColorType(format),
                            kPremul_SkAlphaType);
 }
 
 }  // namespace
-

[vmpstr, 2016/04/21 20:11:31]

keep whitespace

[cblume, 2016/04/22 01:40:14]

Done.

 SoftwareImageDecodeController::SoftwareImageDecodeController(
     ResourceFormat format)
     : decoded_images_(ImageMRUCache::NO_AUTO_EVICT),
       at_raster_decoded_images_(ImageMRUCache::NO_AUTO_EVICT),
       locked_images_budget_(kLockedMemoryLimitBytes),
       format_(format) {
   // In certain cases, ThreadTaskRunnerHandle isn't set (Android Webview).
   // Don't register a dump provider in these cases.
   if (base::ThreadTaskRunnerHandle::IsSet()) {
     base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
@@ skipping 30 matching lines @@
                "SoftwareImageDecodeController::GetTaskForImageAndRef", "key",
                key.ToString());
 
   // If the target size is empty, we can skip this image during draw (and thus
   // we don't need to decode it or ref it).
   if (key.target_size().IsEmpty()) {
     *task = nullptr;
     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<TileTask>& 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.
   auto decoded_it = decoded_images_.Get(key);
   bool new_image_fits_in_memory =
       locked_images_budget_.AvailableMemoryBytes() >= key.locked_bytes();
   if (decoded_it != decoded_images_.end()) {
     if (decoded_it->second->is_locked() ||
         (new_image_fits_in_memory && decoded_it->second->Lock())) {
       RefImage(key);
@@ skipping 51 matching lines @@
 }
 
 void SoftwareImageDecodeController::UnrefImage(const DrawImage& image) {
   // When we unref the image, there are several situations we need to consider:
   // 1. The ref did not reach 0, which means we have to keep the image locked.
   // 2. The ref reached 0, we should unlock it.
   //   2a. The image isn't in the locked cache because we didn't get to decode
   //       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)) << key.ToString();
   TRACE_EVENT1("disabled-by-default-cc.debug",
                "SoftwareImageDecodeController::UnrefImage", "key",
                key.ToString());
 
   base::AutoLock lock(lock_);
   auto ref_count_it = decoded_images_ref_counts_.find(key);
   DCHECK(ref_count_it != decoded_images_ref_counts_.end());
 
   --ref_count_it->second;
   if (ref_count_it->second == 0) {
@@ skipping 10 matching lines @@
     DCHECK(decoded_image_it->second->is_locked());
     decoded_image_it->second->Unlock();
   }
   SanityCheckState(__LINE__, true);
 }
 
 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);
 
   // We could have finished all of the raster tasks (cancelled) while the task
   // was just starting to run. Since this task already started running, it
   // wasn't cancelled. So, if the ref count for the image is 0 then we can just
   // abort.
   if (decoded_images_ref_counts_.find(key) ==
       decoded_images_ref_counts_.end()) {
     return;
@@ skipping 36 matching lines @@
   if (decoded_images_ref_counts_.find(key) ==
       decoded_images_ref_counts_.end()) {
     decoded_image->Unlock();
   }
 
   decoded_images_.Put(key, std::move(decoded_image));
   SanityCheckState(__LINE__, true);
 }
 
 std::unique_ptr<SoftwareImageDecodeController::DecodedImage>
+SoftwareImageDecodeController::DecodeImageMediumQuality(const ImageKey& key,
+                                                        const SkImage& image) {
+  SkMatrix mipmap_scale = GetMipMapScale(key);
+  if (mipmap_scale.getScaleX() == 0 || mipmap_scale.getScaleY() == 0) {

[vmpstr, 2016/04/21 20:11:30]

This should be doing an epsilon comparison since it's a float?

[cblume, 2016/04/22 01:40:14]

This started a good conversation in my cube.
Our consensus is that we don't need to compare against epsilon. But I want to
provide the thought that got there:

I actually changed it to -1 since close to 0 is a possible scale. I wanted to
get away from any possibly-good value.

-1 can be represented perfectly in floating point, no matter the size of the
float. There must always be at least 1 sign bit and 1 mantissa bit.

However, there were other things discussed which are important to note. First,
there were no operations to get to -1. It was a literal. If there were
operations, we would need the epsilon.

Additionally, there is a case where the number couldn't be perfectly represented
like say comparing -1.3f == -1.3f. The compiler will generate sort of literal.
But it is possible that the compiler generates say a 80-bit literal. When the
original literal was stored into a float, it will involve rounding from the
80-bits.

Now then, all that said, didn't we just approve std::optional? In general I
dislike invalid values...

+    return nullptr;
+  }
+
+  gfx::Rect src_rect = key.src_rect();
+  DrawImage mip_image(&image, gfx::RectToSkIRect(src_rect),

[vmpstr, 2016/04/21 20:11:31]

nit: key.src_rect() directly here

[cblume, 2016/04/22 01:40:14]

Done.

+                      kMedium_SkFilterQuality, mipmap_scale);
+  auto mip_key = ImageKey::FromDrawImage(mip_image);

[vmpstr, 2016/04/21 20:11:30]

Interesting, so you create a new DrawImage that is at the exact mipmap level and
then use kMedium_SkFilterQuality but other than that it's going down the same
path that high filter quality would go... That's pretty cool!

Do we have guarantees that the mip size is exactly the same as what skia would
pick? That is to say, would there still be a bilerp to adjust the size by one
pixel or so maybe?

[cblume, 2016/04/22 01:40:14]

I asked the Skia team about if it runs a bilerp if I requested the exact scale
of the mipmap. I was told it would not.

Unfortunately, I do not have a good way to guarantee that the mip size is
exactly the same as the one used by Skia. That would require Skia exporting such
information. So I cannot DCHECK it or the likes. Right now, I just happen to
know that medium quality is mipmaps and I happen to know Skia uses SkMipMap. And
I happen to know SkMipMap fairly well.

A better solution would probably be to export SkMipMap, but that requires
exporting other dependencies as well. SkMipMap allows access to each mip level,
including that mip level's size.

+  return GetScaledImageDecode(mip_key, image);
+}
+
+std::unique_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;
 
   switch (key.filter_quality()) {
     case kNone_SkFilterQuality:
     case kLow_SkFilterQuality:
       return GetOriginalImageDecode(key, *image);
     case kMedium_SkFilterQuality:
-      NOTIMPLEMENTED();
-      return nullptr;
+      return DecodeImageMediumQuality(key, *image);

[vmpstr, 2016/04/21 20:11:30]

To be consistent, can you have this GetMediumQualityImageDecode()

Or something like GetMipMap...Decode? I don't know :)

[cblume, 2016/04/22 01:40:14]

Done.

     case kHigh_SkFilterQuality:
       return GetScaledImageDecode(key, *image);
     default:
       NOTREACHED();
       return nullptr;
   }
 }
 
 DecodedDrawImage SoftwareImageDecodeController::GetDecodedImageForDraw(
     const DrawImage& draw_image) {
   ImageKey key = ImageKey::FromDrawImage(draw_image);
   TRACE_EVENT1("disabled-by-default-cc.debug",
                "SoftwareImageDecodeController::GetDecodedImageForDraw", "key",
                key.ToString());
   // If the target size is empty, we can skip this image draw.
   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);
 }
 
 DecodedDrawImage SoftwareImageDecodeController::GetDecodedImageForDrawInternal(
     const ImageKey& key,
     const DrawImage& draw_image) {
   TRACE_EVENT1("disabled-by-default-cc.debug",
                "SoftwareImageDecodeController::GetDecodedImageForDrawInternal",
                "key", key.ToString());
   base::AutoLock lock(lock_);
@@ skipping 155 matching lines @@
   {
     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());
-  // TODO(vmpstr): Start handling more than just high filter quality.
-  DCHECK_EQ(kHigh_SkFilterQuality, key.filter_quality());

[vmpstr, 2016/04/21 20:11:31]

Can you add a

DCHECK(key.filter_quality() == kHigh_SkFilterQuality || k.filter_quality() ==
kMedium_SkFilterQuality)?

[cblume, 2016/04/22 01:40:14]

Done.

   {
     TRACE_EVENT0("disabled-by-default-cc.debug",
                  "SoftwareImageDecodeController::ScaleImage - scale pixels");
     bool result =
         decoded_pixmap.scalePixels(scaled_pixmap, key.filter_quality());
     DCHECK(result) << key.ToString();
   }
 
   // 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);
 
   return base::WrapUnique(
       new DecodedImage(scaled_info, std::move(scaled_pixels),
                        SkSize::Make(-key.src_rect().x(), -key.src_rect().y()),
                        next_tracing_id_.GetNext()));
 }
 
 void SoftwareImageDecodeController::DrawWithImageFinished(
     const DrawImage& image,
     const DecodedDrawImage& decoded_image) {
   TRACE_EVENT1("disabled-by-default-cc.debug",
                "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())
     UnrefAtRasterImage(key);
   else
     UnrefImage(image);
   SanityCheckState(__LINE__, false);
 }
 
 void SoftwareImageDecodeController::RefAtRasterImage(const ImageKey& key) {
@@ skipping 46 matching lines @@
       DCHECK(decoded_images_ref_counts_.find(key) ==
              decoded_images_ref_counts_.end());
       at_raster_image_it->second->Unlock();
       decoded_images_.Erase(image_it);
       decoded_images_.Put(key, std::move(at_raster_image_it->second));
     }
     at_raster_decoded_images_.Erase(at_raster_image_it);
   }
 }
 
-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_);
   size_t num_to_remove = (decoded_images_.size() > kMaxItemsInCache)
                              ? (decoded_images_.size() - kMaxItemsInCache)
                              : 0;
   for (auto it = decoded_images_.rbegin();
        num_to_remove != 0 && it != decoded_images_.rend();) {
     if (it->second->is_locked()) {
       ++it;
@@ skipping 243 matching lines @@
 void SoftwareImageDecodeController::MemoryBudget::ResetUsage() {
   current_usage_bytes_ = 0;
 }
 
 size_t SoftwareImageDecodeController::MemoryBudget::GetCurrentUsageSafe()
     const {
   return current_usage_bytes_.ValueOrDie();
 }
 
 }  // namespace cc