From 7f3ceba5dc7f89d013f39da0b5ce14807588e121 Mon Sep 17 00:00:00 2001 From: Mike Klein Date: Sun, 30 Sep 2018 22:23:46 +0000 Subject: [PATCH] Revert "Reland "Reland "Preserve colorType and alphaType in SkImage::makeColorSpace""" This reverts commit 41576877bd1900e7c965b92c0acc439f335191c5. Reason for revert: best guess for pixel test failures Original change's description: > Reland "Reland "Preserve colorType and alphaType in SkImage::makeColorSpace"" > > This reverts commit 11f4994b84e19512c11bb2c675bef5bd93ea5692. > > Reason for revert: Codec change should fix the layout test failures? > > Original change's description: > > Revert "Reland "Preserve colorType and alphaType in SkImage::makeColorSpace"" > > > > This reverts commit 893052ea5191a73fd127bf8668ce54e8162b250a. > > > > Reason for revert: Chrome roll layout failures. > > > > Original change's description: > > > Reland "Preserve colorType and alphaType in SkImage::makeColorSpace" > > > > > > This reverts commit 0d08b3e4b112fb10a07d223a48fa0a40091f88b1. > > > > > > Reason for revert: Fixed Chrome test. > > > > > > Original change's description: > > > > Revert "Preserve colorType and alphaType in SkImage::makeColorSpace" > > > > > > > > This reverts commit d842557c07246546ff15aede9d3b4e078c62b7e4. > > > > > > > > Reason for revert: Chrome roll failing CanvasAsyncBlobCreatorTest.ColorManagedConvertToBlob test. > > > > > > > > Original change's description: > > > > > Preserve colorType and alphaType in SkImage::makeColorSpace > > > > > > > > > > Raster images were always converting to N32, and GPU images were > > > > > always converting to premul. These were unexpected and inconsistent. > > > > > > > > > > Bug: skia:8382 > > > > > Change-Id: I78fe6cff1208eef077a71d08e676cf8f8d5fed9a > > > > > Reviewed-on: https://skia-review.googlesource.com/156142 > > > > > Commit-Queue: Brian Osman > > > > > Reviewed-by: Mike Klein > > > > > > > > TBR=mtklein@google.com,brianosman@google.com > > > > > > > > Change-Id: I366b097644ac1fa920fc9addcad3e09c2a0a63cc > > > > No-Presubmit: true > > > > No-Tree-Checks: true > > > > No-Try: true > > > > Bug: skia:8382 > > > > Reviewed-on: https://skia-review.googlesource.com/156184 > > > > Reviewed-by: Brian Osman > > > > Commit-Queue: Brian Osman > > > > > > TBR=mtklein@google.com,brianosman@google.com > > > > > > Change-Id: I860f33a1d57e0e77ce63b78d66d49a1082d2b4dd > > > No-Presubmit: true > > > No-Tree-Checks: true > > > No-Try: true > > > Bug: skia:8382 > > > Reviewed-on: https://skia-review.googlesource.com/156188 > > > Reviewed-by: Brian Osman > > > Commit-Queue: Brian Osman > > > > TBR=mtklein@google.com,brianosman@google.com > > > > Change-Id: Ib53912d014916374e5d0ee3d224ba69d41a5018c > > No-Presubmit: true > > No-Tree-Checks: true > > No-Try: true > > Bug: skia:8382 > > Reviewed-on: https://skia-review.googlesource.com/156360 > > Reviewed-by: Brian Osman > > Commit-Queue: Brian Osman > > TBR=mtklein@google.com,brianosman@google.com > > # Not skipping CQ checks because original CL landed > 1 day ago. > > Bug: skia:8382 > Change-Id: I3bd7109dd380b6ef213883074c3b07d19ba6ca37 > Reviewed-on: https://skia-review.googlesource.com/157743 > Reviewed-by: Brian Osman > Commit-Queue: Brian Osman TBR=mtklein@google.com,brianosman@google.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: skia:8382 Change-Id: I587820c6e6c981025f3a2a5a20d8ba4df4b65634 Reviewed-on: https://skia-review.googlesource.com/158100 Reviewed-by: Mike Klein Commit-Queue: Mike Klein --- src/effects/SkToSRGBColorFilter.cpp | 2 +- src/gpu/GrColorSpaceXform.cpp | 5 ++--- src/gpu/GrColorSpaceXform.h | 2 +- src/gpu/GrYUVProvider.cpp | 3 +-- src/image/SkImage.cpp | 8 ++++++-- src/image/SkImage_Base.h | 2 +- src/image/SkImage_Gpu.cpp | 19 +++++++++++++++---- src/image/SkImage_Gpu.h | 2 +- src/image/SkImage_Lazy.cpp | 3 ++- src/image/SkImage_Lazy.h | 2 +- src/image/SkImage_Raster.cpp | 7 ++++--- 11 files changed, 35 insertions(+), 20 deletions(-) diff --git a/src/effects/SkToSRGBColorFilter.cpp b/src/effects/SkToSRGBColorFilter.cpp index d80595c51b..510db128ca 100644 --- a/src/effects/SkToSRGBColorFilter.cpp +++ b/src/effects/SkToSRGBColorFilter.cpp @@ -52,6 +52,6 @@ void SkToSRGBColorFilter::flatten(SkWriteBuffer& buffer) const { std::unique_ptr SkToSRGBColorFilter::asFragmentProcessor( GrContext*, const GrColorSpaceInfo&) const { return GrColorSpaceXformEffect::Make(fSrcColorSpace.get(), kPremul_SkAlphaType, - sk_srgb_singleton(), kPremul_SkAlphaType); + sk_srgb_singleton()); } #endif diff --git a/src/gpu/GrColorSpaceXform.cpp b/src/gpu/GrColorSpaceXform.cpp index fdcc0cde82..f7b924a641 100644 --- a/src/gpu/GrColorSpaceXform.cpp +++ b/src/gpu/GrColorSpaceXform.cpp @@ -144,10 +144,9 @@ GrFragmentProcessor::OptimizationFlags GrColorSpaceXformEffect::OptFlags( std::unique_ptr GrColorSpaceXformEffect::Make(SkColorSpace* src, SkAlphaType srcAT, - SkColorSpace* dst, - SkAlphaType dstAT) { + SkColorSpace* dst) { auto xform = GrColorSpaceXform::Make(src, srcAT, - dst, dstAT); + dst, kPremul_SkAlphaType); if (!xform) { return nullptr; } diff --git a/src/gpu/GrColorSpaceXform.h b/src/gpu/GrColorSpaceXform.h index 4542d5cb72..d824dcd55b 100644 --- a/src/gpu/GrColorSpaceXform.h +++ b/src/gpu/GrColorSpaceXform.h @@ -52,7 +52,7 @@ public: * Returns a fragment processor that converts the input's color space from src to dst. */ static std::unique_ptr Make(SkColorSpace* src, SkAlphaType srcAT, - SkColorSpace* dst, SkAlphaType dstAT); + SkColorSpace* dst); /** * Returns a fragment processor that calls the passed in fragment processor, and then converts diff --git a/src/gpu/GrYUVProvider.cpp b/src/gpu/GrYUVProvider.cpp index f5bc7ea6e8..a5b71dd651 100644 --- a/src/gpu/GrYUVProvider.cpp +++ b/src/gpu/GrYUVProvider.cpp @@ -141,8 +141,7 @@ sk_sp GrYUVProvider::refAsTextureProxy(GrContext* ctx, const GrS // If the caller expects the pixels in a different color space than the one from the image, // apply a color conversion to do this. std::unique_ptr colorConversionProcessor = - GrColorSpaceXformEffect::Make(srcColorSpace, kOpaque_SkAlphaType, - dstColorSpace, kOpaque_SkAlphaType); + GrColorSpaceXformEffect::Make(srcColorSpace, kPremul_SkAlphaType, dstColorSpace); if (colorConversionProcessor) { paint.addColorFragmentProcessor(std::move(colorConversionProcessor)); } diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp index f20b64ae35..4bc7b15fc6 100644 --- a/src/image/SkImage.cpp +++ b/src/image/SkImage.cpp @@ -300,7 +300,8 @@ bool SkImage::isAlphaOnly() const { } sk_sp SkImage::makeColorSpace(sk_sp target) const { - if (!target) { + SkColorSpaceTransferFn fn; + if (!target || !target->isNumericalTransferFn(&fn)) { return nullptr; } @@ -312,8 +313,11 @@ sk_sp SkImage::makeColorSpace(sk_sp target) const { return sk_ref_sp(const_cast(this)); } + // TODO: Re-visit this! Keep existing color type? + SkColorType targetColorType = kN32_SkColorType; + // TODO: We might consider making this a deferred conversion? - return as_IB(this)->onMakeColorSpace(std::move(target)); + return as_IB(this)->onMakeColorSpace(std::move(target), targetColorType); } sk_sp SkImage::makeNonTextureImage() const { diff --git a/src/image/SkImage_Base.h b/src/image/SkImage_Base.h index 9229c0d264..f996072937 100644 --- a/src/image/SkImage_Base.h +++ b/src/image/SkImage_Base.h @@ -91,7 +91,7 @@ public: virtual bool onPinAsTexture(GrContext*) const { return false; } virtual void onUnpinAsTexture(GrContext*) const {} - virtual sk_sp onMakeColorSpace(sk_sp) const = 0; + virtual sk_sp onMakeColorSpace(sk_sp, SkColorType) const = 0; protected: SkImage_Base(int width, int height, uint32_t uniqueID); diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp index 735874ab01..bb025f093a 100644 --- a/src/image/SkImage_Gpu.cpp +++ b/src/image/SkImage_Gpu.cpp @@ -1170,9 +1170,17 @@ bool SkImage::MakeBackendTextureFromSkImage(GrContext* ctx, /////////////////////////////////////////////////////////////////////////////////////////////////// -sk_sp SkImage_Gpu::onMakeColorSpace(sk_sp target) const { - auto xform = GrColorSpaceXformEffect::Make(fColorSpace.get(), this->alphaType(), - target.get(), this->alphaType()); +sk_sp SkImage_Gpu::onMakeColorSpace(sk_sp target, SkColorType) const { + sk_sp srcSpace = fColorSpace; + if (!fColorSpace) { + if (target->isSRGB()) { + return sk_ref_sp(const_cast((SkImage*)this)); + } + + srcSpace = SkColorSpace::MakeSRGB(); + } + + auto xform = GrColorSpaceXformEffect::Make(srcSpace.get(), this->alphaType(), target.get()); if (!xform) { return sk_ref_sp(const_cast(this)); } @@ -1198,10 +1206,13 @@ sk_sp SkImage_Gpu::onMakeColorSpace(sk_sp target) const { return nullptr; } + SkAlphaType newAlphaType = (kUnpremul_SkAlphaType == fAlphaType) ? kPremul_SkAlphaType + : fAlphaType; // MDB: this call is okay bc we know 'renderTargetContext' was exact return sk_make_sp(fContext, kNeedNewImageUniqueID, - fAlphaType, renderTargetContext->asTextureProxyRef(), + newAlphaType, renderTargetContext->asTextureProxyRef(), std::move(target), fBudgeted); + } bool SkImage_Gpu::onIsValid(GrContext* context) const { diff --git a/src/image/SkImage_Gpu.h b/src/image/SkImage_Gpu.h index 32928b0cef..fac15d2301 100644 --- a/src/image/SkImage_Gpu.h +++ b/src/image/SkImage_Gpu.h @@ -56,7 +56,7 @@ public: sk_sp refColorSpace() { return fColorSpace; } - sk_sp onMakeColorSpace(sk_sp) const override; + sk_sp onMakeColorSpace(sk_sp, SkColorType) const override; typedef ReleaseContext TextureContext; typedef void (*TextureFulfillProc)(TextureContext textureContext, GrBackendTexture* outTexture); diff --git a/src/image/SkImage_Lazy.cpp b/src/image/SkImage_Lazy.cpp index 4bf9674a9d..e1f987b1ae 100644 --- a/src/image/SkImage_Lazy.cpp +++ b/src/image/SkImage_Lazy.cpp @@ -311,7 +311,8 @@ sk_sp SkImage_Lazy::onMakeSubset(const SkIRect& subset) const { return validator ? sk_sp(new SkImage_Lazy(&validator)) : nullptr; } -sk_sp SkImage_Lazy::onMakeColorSpace(sk_sp target) const { +sk_sp SkImage_Lazy::onMakeColorSpace(sk_sp target, + SkColorType targetColorType) const { SkAutoExclusive autoAquire(fOnMakeColorSpaceMutex); if (target && fOnMakeColorSpaceTarget && SkColorSpace::Equals(target.get(), fOnMakeColorSpaceTarget.get())) { diff --git a/src/image/SkImage_Lazy.h b/src/image/SkImage_Lazy.h index 3b8cfc5465..a0454c65e3 100644 --- a/src/image/SkImage_Lazy.h +++ b/src/image/SkImage_Lazy.h @@ -55,7 +55,7 @@ public: sk_sp onMakeSubset(const SkIRect&) const override; bool getROPixels(SkBitmap*, SkColorSpace* dstColorSpace, CachingHint) const override; bool onIsLazyGenerated() const override { return true; } - sk_sp onMakeColorSpace(sk_sp) const override; + sk_sp onMakeColorSpace(sk_sp, SkColorType) const override; bool onIsValid(GrContext*) const override; diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp index 8269cbca45..994bb4b7db 100644 --- a/src/image/SkImage_Raster.cpp +++ b/src/image/SkImage_Raster.cpp @@ -103,7 +103,7 @@ public: SkASSERT(bitmapMayBeMutable || fBitmap.isImmutable()); } - sk_sp onMakeColorSpace(sk_sp) const override; + sk_sp onMakeColorSpace(sk_sp, SkColorType) const override; bool onIsValid(GrContext* context) const override { return true; } @@ -334,7 +334,8 @@ bool SkImage_Raster::onAsLegacyBitmap(SkBitmap* bitmap) const { /////////////////////////////////////////////////////////////////////////////// -sk_sp SkImage_Raster::onMakeColorSpace(sk_sp target) const { +sk_sp SkImage_Raster::onMakeColorSpace(sk_sp target, + SkColorType targetColorType) const { SkPixmap src; SkAssertResult(fBitmap.peekPixels(&src)); @@ -347,7 +348,7 @@ sk_sp SkImage_Raster::onMakeColorSpace(sk_sp target) cons src.setColorSpace(SkColorSpace::MakeSRGB()); } - SkImageInfo dstInfo = fBitmap.info().makeColorSpace(target); + SkImageInfo dstInfo = fBitmap.info().makeColorType(targetColorType).makeColorSpace(target); SkBitmap dst; dst.allocPixels(dstInfo);