color: Enable color conversion in image decoder caches

cc/tiles/software_image_decode_cache.cc

Index: cc/tiles/software_image_decode_cache.cc
diff --git a/cc/tiles/software_image_decode_cache.cc b/cc/tiles/software_image_decode_cache.cc
index d7c1059ba92be8f10f39bc1bed366b8a2cf0ca3b..716d1f45ef3d1df3b5b6ca2034301acae0201c39 100644
--- a/cc/tiles/software_image_decode_cache.cc
+++ b/cc/tiles/software_image_decode_cache.cc
@@ -446,7 +446,7 @@ SoftwareImageDecodeCache::DecodeImageInternal(const ImageKey& key,
     case kLow_SkFilterQuality:
       if (key.should_use_subrect())
         return GetSubrectImageDecode(key, std::move(image));
-      return GetOriginalImageDecode(std::move(image));
+      return GetOriginalSizeImageDecode(key, std::move(image));
     case kMedium_SkFilterQuality:
     case kHigh_SkFilterQuality:
       return GetScaledImageDecode(key, std::move(image));
@@ -563,22 +563,35 @@ DecodedDrawImage SoftwareImageDecodeCache::GetDecodedImageForDrawInternal(
 }
 
 std::unique_ptr<SoftwareImageDecodeCache::DecodedImage>
-SoftwareImageDecodeCache::GetOriginalImageDecode(sk_sp<const SkImage> image) {
+SoftwareImageDecodeCache::GetOriginalSizeImageDecode(
+    const ImageKey& key,
+    sk_sp<const SkImage> image) {
   SkImageInfo decoded_info =
       CreateImageInfo(image->width(), image->height(), format_);
+  sk_sp<SkColorSpace> target_color_space =
+      key.target_color_space().ToSkColorSpace();
+
   std::unique_ptr<base::DiscardableMemory> decoded_pixels;
   {
     TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
-                 "SoftwareImageDecodeCache::GetOriginalImageDecode - "
+                 "SoftwareImageDecodeCache::GetOriginalSizeImageDecode - "
                  "allocate decoded pixels");
     decoded_pixels =
         base::DiscardableMemoryAllocator::GetInstance()
             ->AllocateLockedDiscardableMemory(decoded_info.minRowBytes() *
                                               decoded_info.height());
   }
+  if (target_color_space) {

[vmpstr, 2017/04/27 22:49:45]

Is this the right spot to do this?

From my understanding doing this conversion will essentially decode the image
into malloc'ed memory and return a new image for that. In the block below, we'll
then call read pixels which will just copy the data into discardable.

Is there a way to first read the pixels and then convert them? Or is there a way
to specify our own memory in the makeColorSpace function? I'm trying to avoid a
full decoded copy here if possible.

[ccameron, 2017/05/03 21:17:13]

This understanding is correct.

So, if (target_color_space) then
- the call to makeColorSpace will trigger the decode and color conversion
- readPixels will copy that to the buffer

If (!target_color_space) then
- the call to readPixels will trigger the decode into the buffer

It's true, we're doing an extra copy. A more direct scheme would be to have
something like "SkImage::readPixelsIntoColorSpace".

I'll need to do a power analysis of all of this to see what the problematic
areas are. IIRC, the color conversion cost is 2-3x the decode (even when done at
the same time as the decode), so this may be more marginal.

Of note is that in the case of GPU raster, doing the color conversion this way
(uploading the original pixels to the GPU and then doing a conversion into a new
buffer on the GPU) proves cheaper.

Also of note is that we should be saying "if target_color_space and
target_color_space isn't approximately equal to the image color's space".

+    TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
+                 "SoftwareImageDecodeCache::GetOriginalSizeImageDecode - "
+                 "color conversion");
+    image = image->makeColorSpace(target_color_space,
+                                  SkTransferFunctionBehavior::kIgnore);
+    DCHECK(image);
+  }
   {
     TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
-                 "SoftwareImageDecodeCache::GetOriginalImageDecode - "
+                 "SoftwareImageDecodeCache::GetOriginalSizeImageDecode - "
                  "read pixels");
     bool result = image->readPixels(decoded_info, decoded_pixels->data(),
                                     decoded_info.minRowBytes(), 0, 0,
@@ -590,12 +603,10 @@ SoftwareImageDecodeCache::GetOriginalImageDecode(sk_sp<const SkImage> image) {
     }
   }
 
-  // TODO(ccameron,msarett): Convert image to target color space.
-  // http://crbug.com/706613
-
-  return base::MakeUnique<DecodedImage>(decoded_info, std::move(decoded_pixels),
-                                        SkSize::Make(0, 0),
-                                        next_tracing_id_.GetNext());
+  return base::MakeUnique<DecodedImage>(
+      decoded_info.makeColorSpace(target_color_space),
+      std::move(decoded_pixels), SkSize::Make(0, 0),
+      next_tracing_id_.GetNext());
 }
 
 std::unique_ptr<SoftwareImageDecodeCache::DecodedImage>
@@ -609,6 +620,9 @@ SoftwareImageDecodeCache::GetSubrectImageDecode(const ImageKey& key,
       kNone_SkFilterQuality, SkMatrix::I(), key.target_color_space());
   ImageKey original_size_key =
       ImageKey::FromDrawImage(original_size_draw_image);
+  sk_sp<SkColorSpace> target_color_space =
+      key.target_color_space().ToSkColorSpace();
+
   // Sanity checks.
   DCHECK(original_size_key.can_use_original_size_decode())
       << original_size_key.ToString();
@@ -621,6 +635,8 @@ SoftwareImageDecodeCache::GetSubrectImageDecode(const ImageKey& key,
   if (!decoded_draw_image.image())
     return nullptr;
 
+  DCHECK(SkColorSpace::Equals(decoded_draw_image.image()->colorSpace(),
+                              target_color_space.get()));
   SkImageInfo subrect_info = CreateImageInfo(
       key.target_size().width(), key.target_size().height(), format_);
   std::unique_ptr<base::DiscardableMemory> subrect_pixels;
@@ -649,11 +665,9 @@ SoftwareImageDecodeCache::GetSubrectImageDecode(const ImageKey& key,
     DCHECK(result);
   }
 
-  // TODO(ccameron,msarett): Convert image to target color space.
-  // http://crbug.com/706613
-
   return base::WrapUnique(
-      new DecodedImage(subrect_info, std::move(subrect_pixels),
+      new DecodedImage(subrect_info.makeColorSpace(target_color_space),
+                       std::move(subrect_pixels),
                        SkSize::Make(-key.src_rect().x(), -key.src_rect().y()),
                        next_tracing_id_.GetNext()));
 }
@@ -669,6 +683,9 @@ SoftwareImageDecodeCache::GetScaledImageDecode(const ImageKey& key,
       kNone_SkFilterQuality, SkMatrix::I(), key.target_color_space());
   ImageKey original_size_key =
       ImageKey::FromDrawImage(original_size_draw_image);
+  sk_sp<SkColorSpace> target_color_space =
+      key.target_color_space().ToSkColorSpace();
+
   // Sanity checks.
   DCHECK(original_size_key.can_use_original_size_decode())
       << original_size_key.ToString();
@@ -691,6 +708,8 @@ SoftwareImageDecodeCache::GetScaledImageDecode(const ImageKey& key,
   }
 
   DCHECK(!key.target_size().IsEmpty());
+  DCHECK(SkColorSpace::Equals(decoded_draw_image.image()->colorSpace(),
+                              target_color_space.get()));
   SkImageInfo scaled_info = CreateImageInfo(
       key.target_size().width(), key.target_size().height(), format_);
   std::unique_ptr<base::DiscardableMemory> scaled_pixels;
@@ -714,11 +733,9 @@ SoftwareImageDecodeCache::GetScaledImageDecode(const ImageKey& key,
     DCHECK(result) << key.ToString();
   }
 
-  // TODO(ccameron,msarett): Convert image to target color space.
-  // http://crbug.com/706613
-
   return base::MakeUnique<DecodedImage>(
-      scaled_info, std::move(scaled_pixels),
+      scaled_info.makeColorSpace(decoded_draw_image.image()->refColorSpace()),
+      std::move(scaled_pixels),
       SkSize::Make(-key.src_rect().x(), -key.src_rect().y()),
       next_tracing_id_.GetNext());
 }