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 <brianosman@google.com>
> > > Reviewed-by: Mike Klein <mtklein@google.com>
> >
> > 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 <brianosman@google.com>
> > Commit-Queue: Brian Osman <brianosman@google.com>
>
> 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 <brianosman@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
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 <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
diff --git a/src/effects/SkToSRGBColorFilter.cpp b/src/effects/SkToSRGBColorFilter.cpp
index b24ca8d..4b5558d 100644
--- a/src/effects/SkToSRGBColorFilter.cpp
+++ b/src/effects/SkToSRGBColorFilter.cpp
@@ -53,6 +53,6 @@
std::unique_ptr<GrFragmentProcessor> 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 fdcc0cd..f7b924a 100644
--- a/src/gpu/GrColorSpaceXform.cpp
+++ b/src/gpu/GrColorSpaceXform.cpp
@@ -144,10 +144,9 @@
std::unique_ptr<GrFragmentProcessor> 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 4542d5c..d824dcd 100644
--- a/src/gpu/GrColorSpaceXform.h
+++ b/src/gpu/GrColorSpaceXform.h
@@ -52,7 +52,7 @@
* Returns a fragment processor that converts the input's color space from src to dst.
*/
static std::unique_ptr<GrFragmentProcessor> 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 f5bc7ea..a5b71dd 100644
--- a/src/gpu/GrYUVProvider.cpp
+++ b/src/gpu/GrYUVProvider.cpp
@@ -141,8 +141,7 @@
// 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<GrFragmentProcessor> 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 509f607..00b7117 100644
--- a/src/image/SkImage.cpp
+++ b/src/image/SkImage.cpp
@@ -300,7 +300,8 @@
}
sk_sp<SkImage> SkImage::makeColorSpace(sk_sp<SkColorSpace> target) const {
- if (!target) {
+ SkColorSpaceTransferFn fn;
+ if (!target || !target->isNumericalTransferFn(&fn)) {
return nullptr;
}
@@ -312,8 +313,11 @@
return sk_ref_sp(const_cast<SkImage*>(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> SkImage::makeNonTextureImage() const {
diff --git a/src/image/SkImage_Base.h b/src/image/SkImage_Base.h
index 2682d32..3adf51e 100644
--- a/src/image/SkImage_Base.h
+++ b/src/image/SkImage_Base.h
@@ -93,7 +93,7 @@
virtual bool onPinAsTexture(GrContext*) const { return false; }
virtual void onUnpinAsTexture(GrContext*) const {}
- virtual sk_sp<SkImage> onMakeColorSpace(sk_sp<SkColorSpace>) const = 0;
+ virtual sk_sp<SkImage> onMakeColorSpace(sk_sp<SkColorSpace>, 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 c2bfeac..5e81474 100644
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -1012,9 +1012,17 @@
///////////////////////////////////////////////////////////////////////////////////////////////////
-sk_sp<SkImage> SkImage_Gpu::onMakeColorSpace(sk_sp<SkColorSpace> target) const {
- auto xform = GrColorSpaceXformEffect::Make(fColorSpace.get(), this->alphaType(),
- target.get(), this->alphaType());
+sk_sp<SkImage> SkImage_Gpu::onMakeColorSpace(sk_sp<SkColorSpace> target, SkColorType) const {
+ sk_sp<SkColorSpace> srcSpace = fColorSpace;
+ if (!fColorSpace) {
+ if (target->isSRGB()) {
+ return sk_ref_sp(const_cast<SkImage*>((SkImage*)this));
+ }
+
+ srcSpace = SkColorSpace::MakeSRGB();
+ }
+
+ auto xform = GrColorSpaceXformEffect::Make(srcSpace.get(), this->alphaType(), target.get());
if (!xform) {
return sk_ref_sp(const_cast<SkImage_Gpu*>(this));
}
@@ -1040,10 +1048,13 @@
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<SkImage_Gpu>(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 1a7f323..b7febb5 100644
--- a/src/image/SkImage_Gpu.h
+++ b/src/image/SkImage_Gpu.h
@@ -59,7 +59,7 @@
sk_sp<SkColorSpace> refColorSpace() { return fColorSpace; }
- sk_sp<SkImage> onMakeColorSpace(sk_sp<SkColorSpace>) const override;
+ sk_sp<SkImage> onMakeColorSpace(sk_sp<SkColorSpace>, 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 9a7bead..6aa0ca0 100644
--- a/src/image/SkImage_Lazy.cpp
+++ b/src/image/SkImage_Lazy.cpp
@@ -311,7 +311,8 @@
return validator ? sk_sp<SkImage>(new SkImage_Lazy(&validator)) : nullptr;
}
-sk_sp<SkImage> SkImage_Lazy::onMakeColorSpace(sk_sp<SkColorSpace> target) const {
+sk_sp<SkImage> SkImage_Lazy::onMakeColorSpace(sk_sp<SkColorSpace> 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 95d78cd..0e047be 100644
--- a/src/image/SkImage_Lazy.h
+++ b/src/image/SkImage_Lazy.h
@@ -61,7 +61,7 @@
sk_sp<SkImage> onMakeSubset(const SkIRect&) const override;
bool getROPixels(SkBitmap*, SkColorSpace* dstColorSpace, CachingHint) const override;
bool onIsLazyGenerated() const override { return true; }
- sk_sp<SkImage> onMakeColorSpace(sk_sp<SkColorSpace>) const override;
+ sk_sp<SkImage> onMakeColorSpace(sk_sp<SkColorSpace>, SkColorType) const override;
bool onIsValid(GrContext*) const override;
diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp
index ac3ddb7..859014f 100644
--- a/src/image/SkImage_Raster.cpp
+++ b/src/image/SkImage_Raster.cpp
@@ -110,7 +110,7 @@
SkASSERT(bitmapMayBeMutable || fBitmap.isImmutable());
}
- sk_sp<SkImage> onMakeColorSpace(sk_sp<SkColorSpace>) const override;
+ sk_sp<SkImage> onMakeColorSpace(sk_sp<SkColorSpace>, SkColorType) const override;
bool onIsValid(GrContext* context) const override { return true; }
@@ -341,7 +341,8 @@
///////////////////////////////////////////////////////////////////////////////
-sk_sp<SkImage> SkImage_Raster::onMakeColorSpace(sk_sp<SkColorSpace> target) const {
+sk_sp<SkImage> SkImage_Raster::onMakeColorSpace(sk_sp<SkColorSpace> target,
+ SkColorType targetColorType) const {
SkPixmap src;
SkAssertResult(fBitmap.peekPixels(&src));
@@ -354,7 +355,7 @@
src.setColorSpace(SkColorSpace::MakeSRGB());
}
- SkImageInfo dstInfo = fBitmap.info().makeColorSpace(target);
+ SkImageInfo dstInfo = fBitmap.info().makeColorType(targetColorType).makeColorSpace(target);
SkBitmap dst;
dst.allocPixels(dstInfo);