From 6251771ebc5c4d7c98af3e2f3f2e7b22e490c1d5 Mon Sep 17 00:00:00 2001 From: Brian Osman Date: Wed, 26 Apr 2017 16:26:39 -0400 Subject: [PATCH] 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 Reviewed-by: Robert Phillips --- src/image/SkImage_Gpu.cpp | 18 +++++++++--------- 1 file 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 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; } }