Fix makeColorSpace on YUV images
This is a second attempt at https://skia-review.googlesource.com/c/skia/+/182816
This version ensures that SkImage::colorSpace() returns the target after
makeColorSpace has been called (to match user expectations, and match
behavior with lazy images). Given that, the xform is baked into the FP
within the maker, rather than externally (again, this matches the lazy
image behavior).
Additionally, the target color space needs to be taken into account when
flattening into the RGB proxy, and some base-class methods need to use
this->colorSpace() rather than fColorSpace to tag the output.
Added a GM that tests quite a few different scenarios. All images have
makeColorSpace() applied:
- Raster image (for reference)
- yuvImage
- yuvImage->makeSubset()
- yuvImage->makeNonTextureImage()
- readPixels(yuvImage)
All images should look the same as the top row. Verified that they do
match, in both untagged (gl) and tagged (glsrgb) configs.
I think there may still be some cases where we transform too many or too
few times, or incorrectly tag the result of an image operation, but this
is much more correct than before, and should (I hope) address Chrome's
immediate needs.
Bug: skia:8740
Change-Id: I5d501879866861a5ba91240f688d3f95711f7595
Reviewed-on: https://skia-review.googlesource.com/c/189494
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
diff --git a/src/image/SkImage_GpuBase.cpp b/src/image/SkImage_GpuBase.cpp
index 9c86d29..35d5a2e 100644
--- a/src/image/SkImage_GpuBase.cpp
+++ b/src/image/SkImage_GpuBase.cpp
@@ -135,7 +135,7 @@
// MDB: this call is okay bc we know 'sContext' was kExact
return sk_make_sp<SkImage_Gpu>(fContext, kNeedNewImageUniqueID, fAlphaType,
- sContext->asTextureProxyRef(), fColorSpace);
+ sContext->asTextureProxyRef(), this->refColorSpace());
}
static void apply_premul(const SkImageInfo& info, void* pixels, size_t rowBytes) {
@@ -185,7 +185,7 @@
}
sk_sp<GrSurfaceContext> sContext = fContext->priv().makeWrappedSurfaceContext(
- this->asTextureProxyRef(), fColorSpace);
+ this->asTextureProxyRef(), this->refColorSpace());
if (!sContext) {
return false;
}
@@ -343,6 +343,7 @@
bool SkImage_GpuBase::RenderYUVAToRGBA(GrContext* ctx, GrRenderTargetContext* renderTargetContext,
const SkRect& rect, SkYUVColorSpace yuvColorSpace,
+ sk_sp<GrColorSpaceXform> colorSpaceXform,
const sk_sp<GrTextureProxy> proxies[4],
const SkYUVAIndex yuvaIndices[4]) {
SkASSERT(renderTargetContext);
@@ -353,9 +354,12 @@
GrPaint paint;
paint.setPorterDuffXPFactory(SkBlendMode::kSrc);
- paint.addColorFragmentProcessor(GrYUVtoRGBEffect::Make(proxies, yuvaIndices,
- yuvColorSpace,
- GrSamplerState::Filter::kNearest));
+ auto fp = GrYUVtoRGBEffect::Make(proxies, yuvaIndices, yuvColorSpace,
+ GrSamplerState::Filter::kNearest);
+ if (colorSpaceXform) {
+ fp = GrColorSpaceXformEffect::Make(std::move(fp), std::move(colorSpaceXform));
+ }
+ paint.addColorFragmentProcessor(std::move(fp));
renderTargetContext->drawRect(GrNoClip(), std::move(paint), GrAA::kNo, SkMatrix::I(), rect);