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 5616c3c35d2a53d748d906a2c43003cdca9117af..fa7aa24fee689dbf99023469b985be557934697c 100644
--- a/cc/tiles/software_image_decode_cache.cc
+++ b/cc/tiles/software_image_decode_cache.cc
@@ -158,10 +158,11 @@ SkFilterQuality GetDecodedFilterQuality(const ImageDecodeCacheKey& key) {
 
 SkImageInfo CreateImageInfo(size_t width,
                             size_t height,
-                            ResourceFormat format) {
+                            ResourceFormat format,
+                            sk_sp<SkColorSpace> color_space) {
   return SkImageInfo::Make(width, height,
                            ResourceFormatToClosestSkColorType(format),
-                           kPremul_SkAlphaType);
+                           kPremul_SkAlphaType, color_space);

[msarett1, 2017/04/10 21:02:53]

Ignorable nit/question not related to this CL:

Is there always alpha?  Or can we sometimes mark the image as opaque?

[ccameron, 2017/04/10 23:18:30]

I'm not sure ... vmpstr may know. I believe that we do have some image metadata
and perhaps could mark some images as opaque here.

[msarett1, 2017/04/12 12:59:17]

Marking as opaque would definitely improve the performance of the color space
conversion.

 }
 
 void RecordLockExistingCachedImageHistogram(TilePriority::PriorityBin bin,
@@ -454,7 +455,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));
@@ -574,13 +575,17 @@ 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_);
+      CreateImageInfo(image->width(), image->height(), format_,
+                      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()
@@ -589,7 +594,7 @@ SoftwareImageDecodeCache::GetOriginalImageDecode(sk_sp<const SkImage> image) {
   }
   {
     TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
-                 "SoftwareImageDecodeCache::GetOriginalImageDecode - "
+                 "SoftwareImageDecodeCache::GetOriginalSizeImageDecode - "
                  "read pixels");
     bool result = image->readPixels(decoded_info, decoded_pixels->data(),

[ccameron, 2017/04/10 23:18:29]

Yes, I was hoping to just call readPixels here... calling
image->makeColorSpace->readPixels means making an extra copy of the data long
the way, which may be an unacceptable penalty.

But ... I'll put the readPixels in for now, and we'll see what happens after
that.

[msarett1, 2017/04/12 12:59:17]

Acknowledged.

                                     decoded_info.minRowBytes(), 0, 0,
@@ -601,9 +606,6 @@ 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());
@@ -632,8 +634,11 @@ SoftwareImageDecodeCache::GetSubrectImageDecode(const ImageKey& key,
   if (!decoded_draw_image.image())
     return nullptr;
 
-  SkImageInfo subrect_info = CreateImageInfo(
-      key.target_size().width(), key.target_size().height(), format_);
+  DCHECK(SkColorSpace::Equals(decoded_draw_image.image()->colorSpace(),
+                              key.target_color_space().ToSkColorSpace().get()));
+  SkImageInfo subrect_info =
+      CreateImageInfo(key.target_size().width(), key.target_size().height(),
+                      format_, decoded_draw_image.image()->refColorSpace());
   std::unique_ptr<base::DiscardableMemory> subrect_pixels;
   {
     TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
@@ -660,9 +665,6 @@ 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),
                        SkSize::Make(-key.src_rect().x(), -key.src_rect().y()),
@@ -702,8 +704,17 @@ SoftwareImageDecodeCache::GetScaledImageDecode(const ImageKey& key,
   }
 
   DCHECK(!key.target_size().IsEmpty());
+
+  // We cannot specify the color space for the scalePixels call, because the
+  // color space may not be renderable. Just declare that that the pixels had
+  // been in the target color space the whole time when we create the image at
+  // the very end.
+  DCHECK(SkColorSpace::Equals(decoded_draw_image.image()->colorSpace(),
+                              key.target_color_space().ToSkColorSpace().get()));
   SkImageInfo scaled_info = CreateImageInfo(
-      key.target_size().width(), key.target_size().height(), format_);
+      key.target_size().width(), key.target_size().height(), format_, nullptr);
+  SkImageInfo scaled_info_with_color_space =
+      scaled_info.makeColorSpace(decoded_draw_image.image()->refColorSpace());
   std::unique_ptr<base::DiscardableMemory> scaled_pixels;
   {
     TRACE_EVENT0(
@@ -725,11 +736,8 @@ 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_with_color_space, std::move(scaled_pixels),
       SkSize::Make(-key.src_rect().x(), -key.src_rect().y()),
       next_tracing_id_.GetNext());
 }