Fix color space handling in SkImage_Gpu::getROPixels

The dstColorSpace is a badly named parameter. It's a hint about where/how
the returned pixels are going to be used. Raster and GPU are meant to
ignore that information, codecs use it to drive our decoding heuristic.

This is a re-land of https://skia-review.googlesource.com/c/10109/

Bug: skia:
Change-Id: Iee006a8e014e028b4f0f2471d7152b6bccd72cb2
Reviewed-on: https://skia-review.googlesource.com/14404
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>

1 files changed, 9 insertions, 9 deletions

diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp
index 0c5d4c7f48..7b7737dd74 100644
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -63,11 +63,13 @@ SkImageInfo SkImage_Gpu::onImageInfo() const {
     return SkImageInfo::Make(fProxy->width(), fProxy->height(), ct, fAlphaType, fColorSpace);
 }
 
-static SkImageInfo make_info(int w, int h, SkAlphaType at, sk_sp<SkColorSpace> colorSpace) {
-    return SkImageInfo::MakeN32(w, h, at, std::move(colorSpace));
-}
-
-bool SkImage_Gpu::getROPixels(SkBitmap* dst, SkColorSpace* dstColorSpace, CachingHint chint) const {
+bool SkImage_Gpu::getROPixels(SkBitmap* dst, SkColorSpace*, CachingHint chint) const {
+    // The SkColorSpace parameter "dstColorSpace" is really just a hint about how/where the bitmap
+    // will be used. The client doesn't expect that we convert to that color space, it's intended
+    // for codec-backed images, to drive our decoding heuristic. In theory we *could* read directly
+    // into that color space (to save the client some effort in whatever they're about to do), but
+    // that would make our use of the bitmap cache incorrect (or much less efficient, assuming we
+    // rolled the dstColorSpace into the key).
     const auto desc = SkBitmapCacheDesc::Make(this);
     if (SkBitmapCache::Find(desc, dst)) {
         SkASSERT(dst->getGenerationID() == this->uniqueID());
@@ -76,17 +78,15 @@ bool SkImage_Gpu::getROPixels(SkBitmap* dst, SkColorSpace* dstColorSpace, Cachin
         return true;
     }
 
-    SkImageInfo ii = make_info(this->width(), this->height(), this->alphaType(),
-                               sk_ref_sp(dstColorSpace));
     SkBitmapCache::RecPtr rec = nullptr;
     SkPixmap pmap;
     if (kAllow_CachingHint == chint) {
-        rec = SkBitmapCache::Alloc(desc, ii, &pmap);
+        rec = SkBitmapCache::Alloc(desc, this->onImageInfo(), &pmap);
         if (!rec) {
             return false;
         }
     } else {
-        if (!dst->tryAllocPixels(ii) || !dst->peekPixels(&pmap)) {
+        if (!dst->tryAllocPixels(this->onImageInfo()) || !dst->peekPixels(&pmap)) {
             return false;
         }
     }