cc: Add color space to image decode caches

cc/tiles/gpu_image_decode_cache.cc

 // Copyright 2016 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/gpu_image_decode_cache.h"
 
 #include <inttypes.h>
 
 #include "base/auto_reset.h"
 #include "base/debug/alias.h"
@@ skipping 91 matching lines @@
   return MipMapUtil::GetScaleAdjustmentForLevel(base_size, mip_level);
 }
 
 // Calculates the size of a given mip level.
 gfx::Size CalculateSizeForMipLevel(const DrawImage& draw_image, int mip_level) {
   gfx::Size base_size(draw_image.image()->width(),
                       draw_image.image()->height());
   return MipMapUtil::GetSizeForLevel(base_size, mip_level);
 }
 
-// Generates a uint64_t which uniquely identifies a DrawImage for the purposes
-// of the |in_use_cache_|. The key is generated as follows:
-//   ╔══════════════════════╤═══════════╤═══════════╗
-//   ║       image_id       │ mip_level │  quality  ║

[vmpstr, 2017/04/04 01:16:43]

Oh no, the fancy comment is gone!

[ericrk, 2017/04/04 02:38:04]

;_;

-//   ╚════════32═bits═══════╧══16═bits══╧══16═bits══╝
-uint64_t GenerateInUseCacheKey(const DrawImage& draw_image) {
+}  // namespace
+
+// Extract the information to uniquely identify a DrawImage for the purposes of
+// the |in_use_cache_|.
+GpuImageDecodeCache::InUseCacheKey::InUseCacheKey(const DrawImage& draw_image)
+    : image_id(draw_image.image()->uniqueID()),
+      mip_level(CalculateUploadScaleMipLevel(draw_image)),
+      filter_quality(CalculateUploadScaleFilterQuality(draw_image)),
+      target_color_space(draw_image.target_color_space()) {
   static_assert(

[ericrk, 2017/04/04 02:38:04]

this static_assert isn't needed now that we aren't squishing SkFilterQuality
into a uint16_t.

actually, the rest of the function is sort of not needed any more (comments and
DCHECKs mattered when we were packing things into a uin64_t)... collapse to {}?

[ccameron, 2017/04/04 06:41:10]

Done.

       kLast_SkFilterQuality <= std::numeric_limits<uint16_t>::max(),
       "InUseCacheKey depends on SkFilterQuality fitting in a uint16_t.");
-
-  SkFilterQuality filter_quality =
-      CalculateUploadScaleFilterQuality(draw_image);
   DCHECK_LE(filter_quality, kLast_SkFilterQuality);
 
   // An image has at most log_2(max(width, height)) mip levels, so given our
   // usage of 32-bit sizes for images, key.mip_level is at most 31.
-  int32_t mip_level = CalculateUploadScaleMipLevel(draw_image);
   DCHECK_LT(mip_level, 32);
-
-  return (static_cast<uint64_t>(draw_image.image()->uniqueID()) << 32) |
-         (mip_level << 16) | filter_quality;
 }
 
-}  // namespace
+bool GpuImageDecodeCache::InUseCacheKey::operator==(
+    const InUseCacheKey& other) const {
+  return image_id == other.image_id && mip_level == other.mip_level &&
+         filter_quality == other.filter_quality &&
+         target_color_space == other.target_color_space;
+}
+
+size_t GpuImageDecodeCache::InUseCacheKeyHash::operator()(
+    const InUseCacheKey& cache_key) const {
+  return static_cast<size_t>(cache_key.image_id) ^ (cache_key.mip_level << 16) ^

[vmpstr, 2017/04/04 01:16:43]

There has base hash functions that I think you should use here instead (see
base::HashInts). I'm not 100% sure what's better, but it seems a bit awkward to
have your own hashing. 

[ccameron, 2017/04/04 06:41:10]

Sounds reasonable -- added a nested HashInts(w, HashInts(x, HashInts(y, z)))

+         (cache_key.filter_quality << 24) ^
+         cache_key.target_color_space.GetHash();
+}
 
 GpuImageDecodeCache::InUseCacheEntry::InUseCacheEntry(
     scoped_refptr<ImageData> image_data)
     : image_data(std::move(image_data)) {}
 GpuImageDecodeCache::InUseCacheEntry::InUseCacheEntry(const InUseCacheEntry&) =
     default;
 GpuImageDecodeCache::InUseCacheEntry::InUseCacheEntry(InUseCacheEntry&&) =
     default;
 GpuImageDecodeCache::InUseCacheEntry::~InUseCacheEntry() = default;
 
@@ skipping 173 matching lines @@
 void GpuImageDecodeCache::UploadedImageData::ReportUsageStats() const {
   UMA_HISTOGRAM_BOOLEAN("Renderer4.GpuImageUploadState.Used",
                         usage_stats_.used);
   UMA_HISTOGRAM_BOOLEAN("Renderer4.GpuImageUploadState.FirstRefWasted",
                         usage_stats_.first_ref_wasted);
 }
 
 GpuImageDecodeCache::ImageData::ImageData(
     DecodedDataMode mode,
     size_t size,
+    const gfx::ColorSpace& target_color_space,
     const SkImage::DeferredTextureImageUsageParams& upload_params)
-    : mode(mode), size(size), upload_params(upload_params) {}
+    : mode(mode),
+      size(size),
+      target_color_space(target_color_space),
+      upload_params(upload_params) {}
 
 GpuImageDecodeCache::ImageData::~ImageData() {
   // We should never delete ImageData while it is in use or before it has been
   // cleaned up.
   DCHECK_EQ(0u, upload.ref_count);
   DCHECK_EQ(0u, decode.ref_count);
   DCHECK_EQ(false, decode.is_locked());
   // This should always be cleaned up before deleting the image, as it needs to
   // be freed with the GL context lock held.
   DCHECK(!upload.image());
@@ skipping 418 matching lines @@
     existing_task = make_scoped_refptr(
         new ImageDecodeTaskImpl(this, draw_image, tracing_info, task_type));
   }
   return existing_task;
 }
 
 void GpuImageDecodeCache::RefImageDecode(const DrawImage& draw_image) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
                "GpuImageDecodeCache::RefImageDecode");
   lock_.AssertAcquired();
-  auto found = in_use_cache_.find(GenerateInUseCacheKey(draw_image));
+  auto found = in_use_cache_.find(InUseCacheKey(draw_image));
   DCHECK(found != in_use_cache_.end());
   ++found->second.ref_count;
   ++found->second.image_data->decode.ref_count;
   OwnershipChanged(draw_image, found->second.image_data.get());
 }
 
 void GpuImageDecodeCache::UnrefImageDecode(const DrawImage& draw_image) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
                "GpuImageDecodeCache::UnrefImageDecode");
   lock_.AssertAcquired();
-  auto found = in_use_cache_.find(GenerateInUseCacheKey(draw_image));
+  auto found = in_use_cache_.find(InUseCacheKey(draw_image));
   DCHECK(found != in_use_cache_.end());
   DCHECK_GT(found->second.image_data->decode.ref_count, 0u);
   DCHECK_GT(found->second.ref_count, 0u);
   --found->second.ref_count;
   --found->second.image_data->decode.ref_count;
   OwnershipChanged(draw_image, found->second.image_data.get());
   if (found->second.ref_count == 0u) {
     in_use_cache_.erase(found);
   }
 }
 
 void GpuImageDecodeCache::RefImage(const DrawImage& draw_image) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
                "GpuImageDecodeCache::RefImage");
   lock_.AssertAcquired();
-  InUseCacheKey key = GenerateInUseCacheKey(draw_image);
+  InUseCacheKey key = InUseCacheKey(draw_image);
   auto found = in_use_cache_.find(key);
 
   // If no secondary cache entry was found for the given |draw_image|, then
   // the draw_image only exists in the |persistent_cache_|. Create an in-use
   // cache entry now.
   if (found == in_use_cache_.end()) {
     auto found_image = persistent_cache_.Peek(draw_image.image()->uniqueID());
     DCHECK(found_image != persistent_cache_.end());
     DCHECK(found_image->second->upload_params.fPreScaleMipLevel <=

[ericrk, 2017/04/04 02:38:04]

Can you replace this DCHECK with one that covers color-space (and quality) as
well?

DCHECK(IsCompatible(found_image->second.get(), draw_image));

[ccameron, 2017/04/04 06:41:10]

Done (I just put this line here).

            CalculateUploadScaleMipLevel(draw_image));
     found = in_use_cache_
                 .insert(InUseCache::value_type(
                     key, InUseCacheEntry(found_image->second)))
                 .first;
   }
 
   DCHECK(found != in_use_cache_.end());
   ++found->second.ref_count;
   ++found->second.image_data->upload.ref_count;
   OwnershipChanged(draw_image, found->second.image_data.get());
 }
 
 void GpuImageDecodeCache::UnrefImageInternal(const DrawImage& draw_image) {
   lock_.AssertAcquired();
-  auto found = in_use_cache_.find(GenerateInUseCacheKey(draw_image));
+  auto found = in_use_cache_.find(InUseCacheKey(draw_image));
   DCHECK(found != in_use_cache_.end());
   DCHECK_GT(found->second.image_data->upload.ref_count, 0u);
   DCHECK_GT(found->second.ref_count, 0u);
   --found->second.ref_count;
   --found->second.image_data->upload.ref_count;
   OwnershipChanged(draw_image, found->second.image_data.get());
   if (found->second.ref_count == 0u) {
     in_use_cache_.erase(found);
   }
 }
@@ skipping 246 matching lines @@
                 SkImage::kDisallow_CachingHint)) {
           backing_memory->Unlock();
           backing_memory.reset();
         }
         break;
       }
       case DecodedDataMode::GPU: {
         // TODO(crbug.com/649167): Params should not have changed since initial
         // sizing. Somehow this still happens. We should investigate and re-add
         // DCHECKs here to enforce this.
-
         if (!draw_image.image()->getDeferredTextureImageData(
                 *context_threadsafe_proxy_.get(), &image_data->upload_params, 1,
-                backing_memory->data())) {
+                backing_memory->data(),
+                draw_image.target_color_space().ToSkColorSpace().get())) {
+          DLOG(ERROR) << "getDeferredTextureImageData failed despite params "
+                      << "having validated.";
           backing_memory->Unlock();
           backing_memory.reset();
         }
         break;
       }
     }
   }
 
   if (image_data->decode.data()) {
     // An at-raster task decoded this before us. Ingore our decode.
@@ skipping 69 matching lines @@
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
                "GpuImageDecodeCache::CreateImageData");
   lock_.AssertAcquired();
 
   DecodedDataMode mode;
   int upload_scale_mip_level = CalculateUploadScaleMipLevel(draw_image);
   auto params = SkImage::DeferredTextureImageUsageParams(
       draw_image.matrix(), CalculateUploadScaleFilterQuality(draw_image),
       upload_scale_mip_level);
   size_t data_size = draw_image.image()->getDeferredTextureImageData(
-      *context_threadsafe_proxy_.get(), &params, 1, nullptr);
+      *context_threadsafe_proxy_.get(), &params, 1, nullptr,
+      draw_image.target_color_space().ToSkColorSpace().get());
 
   if (data_size == 0) {
     // Can't upload image, too large or other failure. Try to use SW fallback.
     SkImageInfo image_info =
         CreateImageInfoForDrawImage(draw_image, upload_scale_mip_level);
     data_size = image_info.getSafeSize(image_info.minRowBytes());
     mode = DecodedDataMode::CPU;
   } else {
     mode = DecodedDataMode::GPU;
   }
 
-  return make_scoped_refptr(new ImageData(mode, data_size, params));
+  return make_scoped_refptr(
+      new ImageData(mode, data_size, draw_image.target_color_space(), params));
 }
 
 void GpuImageDecodeCache::DeletePendingImages() {
   context_->GetLock()->AssertAcquired();
   lock_.AssertAcquired();
   images_pending_deletion_.clear();
 }
 
 SkImageInfo GpuImageDecodeCache::CreateImageInfoForDrawImage(
     const DrawImage& draw_image,
     int upload_scale_mip_level) const {
   gfx::Size mip_size =
       CalculateSizeForMipLevel(draw_image, upload_scale_mip_level);
   return SkImageInfo::Make(mip_size.width(), mip_size.height(),
                            ResourceFormatToClosestSkColorType(format_),
                            kPremul_SkAlphaType);
 }
 
 // Tries to find an ImageData that can be used to draw the provided
 // |draw_image|. First looks for an exact entry in our |in_use_cache_|. If one
 // cannot be found, it looks for a compatible entry in our |persistent_cache_|.
 GpuImageDecodeCache::ImageData* GpuImageDecodeCache::GetImageDataForDrawImage(
     const DrawImage& draw_image) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
                "GpuImageDecodeCache::GetImageDataForDrawImage");
   lock_.AssertAcquired();
-  auto found_in_use = in_use_cache_.find(GenerateInUseCacheKey(draw_image));
+  auto found_in_use = in_use_cache_.find(InUseCacheKey(draw_image));
   if (found_in_use != in_use_cache_.end())
     return found_in_use->second.image_data.get();
 
   auto found_persistent = persistent_cache_.Get(draw_image.image()->uniqueID());
   if (found_persistent != persistent_cache_.end()) {
     ImageData* image_data = found_persistent->second.get();
     if (IsCompatible(image_data, draw_image)) {
       return image_data;
     } else {
       found_persistent->second->is_orphaned = true;
@@ skipping 12 matching lines @@
 // |image_data|. This is true if the |image_data| is not scaled, or if it
 // is scaled at an equal or larger scale and equal or larger quality to
 // the provided |draw_image|.
 bool GpuImageDecodeCache::IsCompatible(const ImageData* image_data,
                                        const DrawImage& draw_image) const {
   bool is_scaled = image_data->upload_params.fPreScaleMipLevel != 0;
   bool scale_is_compatible = CalculateUploadScaleMipLevel(draw_image) >=
                              image_data->upload_params.fPreScaleMipLevel;
   bool quality_is_compatible = CalculateUploadScaleFilterQuality(draw_image) <=
                                image_data->upload_params.fQuality;
-  return !is_scaled || (scale_is_compatible && quality_is_compatible);
+  bool color_is_compatible =
+      image_data->target_color_space == draw_image.target_color_space();
+  if (!color_is_compatible)
+    return false;
+  if (is_scaled && (!scale_is_compatible || !quality_is_compatible))
+    return false;
+  return true;
 }
 
 size_t GpuImageDecodeCache::GetDrawImageSizeForTesting(const DrawImage& image) {
   base::AutoLock lock(lock_);
   scoped_refptr<ImageData> data = CreateImageData(image);
   return data->size;
 }
 
 void GpuImageDecodeCache::SetImageDecodingFailedForTesting(
     const DrawImage& image) {
   base::AutoLock lock(lock_);
   auto found = persistent_cache_.Peek(image.image()->uniqueID());
   DCHECK(found != persistent_cache_.end());
   ImageData* image_data = found->second.get();
   image_data->decode.decode_failure = true;
 }
 
 bool GpuImageDecodeCache::DiscardableIsLockedForTesting(
     const DrawImage& image) {
   base::AutoLock lock(lock_);
   auto found = persistent_cache_.Peek(image.image()->uniqueID());
   DCHECK(found != persistent_cache_.end());
   ImageData* image_data = found->second.get();
   return image_data->decode.is_locked();
 }
 
 bool GpuImageDecodeCache::IsInInUseCacheForTesting(
     const DrawImage& image) const {
-  auto found = in_use_cache_.find(GenerateInUseCacheKey(image));
+  auto found = in_use_cache_.find(InUseCacheKey(image));
   return found != in_use_cache_.end();
 }
 
 void GpuImageDecodeCache::OnMemoryStateChange(base::MemoryState state) {
   memory_state_ = state;
 }
 
 void GpuImageDecodeCache::OnPurgeMemory() {
   base::AutoLock lock(lock_);
   // Temporary changes |memory_state_| to free up cache as much as possible.
   base::AutoReset<base::MemoryState> reset(&memory_state_,
                                            base::MemoryState::SUSPENDED);
   EnsureCapacity(0);
 }
 
 }  // namespace cc