Revert "Revert "Add SkImage::makeColorTypeAndColorSpace""
This reverts commit 9a97c96c9c3a04374af1726d698ddc35e0d728c4.
This works like makeColorSpace, but allows changing color type as well.
Added a GM to test, although the GM demonstrates several ways this can
fail (especially when using this on lazy images).
For simple use-cases (8888 <-> F16), everything should be fine.
For the reland, add logic to the GM to guard against context abandon
failures, and to ensure that lazy images can be decoded (by calling
makeRasterImage) before trying to draw them. That prevents the DDL
recorder from seeing them.
Bug: skia:
Change-Id: Ibc7b07c3399979b1a44d85a38424e5487e606607
Reviewed-on: https://skia-review.googlesource.com/c/183800
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/gm/makecolorspace.cpp b/gm/makecolorspace.cpp
index 2f5acd6..aa28efd 100644
--- a/gm/makecolorspace.cpp
+++ b/gm/makecolorspace.cpp
@@ -8,6 +8,7 @@
#include "gm.h"
#include "Resources.h"
#include "SkCodec.h"
+#include "SkColorSpace.h"
#include "SkImage.h"
#include "SkImagePriv.h"
@@ -82,3 +83,50 @@
};
DEF_GM(return new MakeCSGM;)
+
+DEF_SIMPLE_GM_BG(makecolortypeandspace, canvas, 128 * 3, 128 * 4, SK_ColorWHITE) {
+ sk_sp<SkImage> images[] = {
+ GetResourceAsImage("images/mandrill_128.png"),
+ GetResourceAsImage("images/color_wheel.png"),
+ };
+ auto rec2020 = SkColorSpace::MakeRGB(SkNamedTransferFn::kSRGB, SkNamedGamut::kRec2020);
+
+ // Use the lazy images on the first iteration, and concrete (raster/GPU) images on the second
+ for (bool lazy : {true, false}) {
+ for (int j = 0; j < 2; ++j) {
+ const SkImage* image = images[j].get();
+ if (!image) {
+ // Can happen on bots that abandon the GPU context
+ continue;
+ }
+
+ // Unmodified
+ canvas->drawImage(image, 0, 0);
+
+ // Change the color type/space of the image in a couple ways. In both cases, codec
+ // may fail, because we refude to decode transparent sources to opaque color types.
+ // Guard against that, to avoid cascading failures in DDL.
+
+ // 565 in a wide color space (should be visibly quantized). Fails with the color_wheel,
+ // because of the codec issues mentioned above.
+ auto image565 = image->makeColorTypeAndColorSpace(kRGB_565_SkColorType, rec2020);
+ if (!lazy || image565->makeRasterImage()) {
+ canvas->drawImage(image565, 128, 0);
+ }
+
+ // Grayscale in the original color space. This fails in even more cases, due to the
+ // above opaque issue, and because Ganesh doesn't support drawing to gray, at all.
+ auto imageGray = image->makeColorTypeAndColorSpace(kGray_8_SkColorType,
+ image->refColorSpace());
+ if (!lazy || imageGray->makeRasterImage()) {
+ canvas->drawImage(imageGray, 256, 0);
+ }
+
+ images[j] = canvas->getGrContext()
+ ? image->makeTextureImage(canvas->getGrContext(), nullptr)
+ : image->makeRasterImage();
+
+ canvas->translate(0, 128);
+ }
+ }
+}
diff --git a/include/core/SkImage.h b/include/core/SkImage.h
index c35ae95..764447f 100644
--- a/include/core/SkImage.h
+++ b/include/core/SkImage.h
@@ -1020,6 +1020,19 @@
*/
sk_sp<SkImage> makeColorSpace(sk_sp<SkColorSpace> target) const;
+ /** Experimental.
+ Creates SkImage in target SkColorType and SkColorSpace.
+ Returns nullptr if SkImage could not be created.
+
+ Returns original SkImage if it is in target SkColorType and SkColorSpace.
+
+ @param targetColorType SkColorType of returned SkImage
+ @param targetColorSpace SkColorSpace of returned SkImage
+ @return created SkImage in target SkColorType and SkColorSpace
+ */
+ sk_sp<SkImage> makeColorTypeAndColorSpace(SkColorType targetColorType,
+ sk_sp<SkColorSpace> targetColorSpace) const;
+
private:
SkImage(int width, int height, uint32_t uniqueID);
friend class SkImage_Base;
diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp
index ccc258b..a28028e 100644
--- a/src/image/SkImage.cpp
+++ b/src/image/SkImage.cpp
@@ -313,12 +313,30 @@
if (!colorSpace) {
colorSpace = sk_srgb_singleton();
}
- if (SkColorSpace::Equals(colorSpace, target.get()) ||
- kAlpha_8_SkColorType == as_IB(this)->onImageInfo().colorType()) {
+ if (SkColorSpace::Equals(colorSpace, target.get()) || this->isAlphaOnly()) {
return sk_ref_sp(const_cast<SkImage*>(this));
}
- return as_IB(this)->onMakeColorSpace(std::move(target));
+ return as_IB(this)->onMakeColorTypeAndColorSpace(this->colorType(), std::move(target));
+}
+
+sk_sp<SkImage> SkImage::makeColorTypeAndColorSpace(SkColorType targetColorType,
+ sk_sp<SkColorSpace> targetColorSpace) const {
+ if (kUnknown_SkColorType == targetColorType || !targetColorSpace) {
+ return nullptr;
+ }
+
+ SkColorType colorType = this->colorType();
+ SkColorSpace* colorSpace = this->colorSpace();
+ if (!colorSpace) {
+ colorSpace = sk_srgb_singleton();
+ }
+ if (colorType == targetColorType &&
+ (SkColorSpace::Equals(colorSpace, targetColorSpace.get()) || this->isAlphaOnly())) {
+ return sk_ref_sp(const_cast<SkImage*>(this));
+ }
+
+ return as_IB(this)->onMakeColorTypeAndColorSpace(targetColorType, std::move(targetColorSpace));
}
sk_sp<SkImage> SkImage::makeNonTextureImage() const {
diff --git a/src/image/SkImage_Base.h b/src/image/SkImage_Base.h
index 8a73366..e6736eb 100644
--- a/src/image/SkImage_Base.h
+++ b/src/image/SkImage_Base.h
@@ -96,7 +96,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> onMakeColorTypeAndColorSpace(SkColorType, sk_sp<SkColorSpace>) 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 2b28f1a..5bec1cd 100644
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -60,10 +60,11 @@
return SkImageInfo::Make(fProxy->width(), fProxy->height(), colorType, fAlphaType, fColorSpace);
}
-sk_sp<SkImage> SkImage_Gpu::onMakeColorSpace(sk_sp<SkColorSpace> target) const {
+sk_sp<SkImage> SkImage_Gpu::onMakeColorTypeAndColorSpace(SkColorType targetCT,
+ sk_sp<SkColorSpace> targetCS) const {
auto xform = GrColorSpaceXformEffect::Make(fColorSpace.get(), fAlphaType,
- target.get(), fAlphaType);
- SkASSERT(xform);
+ targetCS.get(), fAlphaType);
+ SkASSERT(xform || targetCT != this->colorType());
sk_sp<GrTextureProxy> proxy = this->asTextureProxyRef();
@@ -75,7 +76,7 @@
sk_sp<GrRenderTargetContext> renderTargetContext(
fContext->contextPriv().makeDeferredRenderTargetContextWithFallback(
format, SkBackingFit::kExact, this->width(), this->height(),
- proxy->config(), nullptr));
+ SkColorType2GrPixelConfig(targetCT), nullptr));
if (!renderTargetContext) {
return nullptr;
}
@@ -83,7 +84,9 @@
GrPaint paint;
paint.setPorterDuffXPFactory(SkBlendMode::kSrc);
paint.addColorTextureProcessor(std::move(proxy), SkMatrix::I());
- paint.addColorFragmentProcessor(std::move(xform));
+ if (xform) {
+ paint.addColorFragmentProcessor(std::move(xform));
+ }
renderTargetContext->drawRect(GrNoClip(), std::move(paint), GrAA::kNo, SkMatrix::I(),
SkRect::MakeIWH(this->width(), this->height()));
@@ -93,7 +96,7 @@
// MDB: this call is okay bc we know 'renderTargetContext' was exact
return sk_make_sp<SkImage_Gpu>(fContext, kNeedNewImageUniqueID, fAlphaType,
- renderTargetContext->asTextureProxyRef(), std::move(target));
+ renderTargetContext->asTextureProxyRef(), std::move(targetCS));
}
///////////////////////////////////////////////////////////////////////////////////////////////////
diff --git a/src/image/SkImage_Gpu.h b/src/image/SkImage_Gpu.h
index 42ba0a5..3b13ec8 100644
--- a/src/image/SkImage_Gpu.h
+++ b/src/image/SkImage_Gpu.h
@@ -37,7 +37,7 @@
virtual bool onIsTextureBacked() const override { return SkToBool(fProxy.get()); }
- sk_sp<SkImage> onMakeColorSpace(sk_sp<SkColorSpace>) const final;
+ sk_sp<SkImage> onMakeColorTypeAndColorSpace(SkColorType, sk_sp<SkColorSpace>) const final;
/**
Create a new SkImage that is very similar to an SkImage created by MakeFromTexture. The main
diff --git a/src/image/SkImage_GpuYUVA.cpp b/src/image/SkImage_GpuYUVA.cpp
index 55411cc..e178894 100644
--- a/src/image/SkImage_GpuYUVA.cpp
+++ b/src/image/SkImage_GpuYUVA.cpp
@@ -143,15 +143,18 @@
//////////////////////////////////////////////////////////////////////////////////////////////////
-sk_sp<SkImage> SkImage_GpuYUVA::onMakeColorSpace(sk_sp<SkColorSpace> target) const {
+sk_sp<SkImage> SkImage_GpuYUVA::onMakeColorTypeAndColorSpace(SkColorType,
+ sk_sp<SkColorSpace> targetCS) const {
+ // We explicitly ignore color type changes, for now.
+
// we may need a mutex here but for now we expect usage to be in a single thread
if (fOnMakeColorSpaceTarget &&
- SkColorSpace::Equals(target.get(), fOnMakeColorSpaceTarget.get())) {
+ SkColorSpace::Equals(targetCS.get(), fOnMakeColorSpaceTarget.get())) {
return fOnMakeColorSpaceResult;
}
- sk_sp<SkImage> result = sk_sp<SkImage>(new SkImage_GpuYUVA(this, target));
+ sk_sp<SkImage> result = sk_sp<SkImage>(new SkImage_GpuYUVA(this, targetCS));
if (result) {
- fOnMakeColorSpaceTarget = target;
+ fOnMakeColorSpaceTarget = targetCS;
fOnMakeColorSpaceResult = result;
}
return result;
diff --git a/src/image/SkImage_GpuYUVA.h b/src/image/SkImage_GpuYUVA.h
index c8b9616..35bcbba 100644
--- a/src/image/SkImage_GpuYUVA.h
+++ b/src/image/SkImage_GpuYUVA.h
@@ -36,7 +36,7 @@
virtual bool onIsTextureBacked() const override { return SkToBool(fProxies[0].get()); }
- sk_sp<SkImage> onMakeColorSpace(sk_sp<SkColorSpace>) const final;
+ sk_sp<SkImage> onMakeColorTypeAndColorSpace(SkColorType, sk_sp<SkColorSpace>) const final;
virtual bool isYUVA() const override { return true; }
virtual bool asYUVATextureProxiesRef(sk_sp<GrTextureProxy> proxies[4],
diff --git a/src/image/SkImage_Lazy.cpp b/src/image/SkImage_Lazy.cpp
index a1557e1..3782974 100644
--- a/src/image/SkImage_Lazy.cpp
+++ b/src/image/SkImage_Lazy.cpp
@@ -53,7 +53,7 @@
///////////////////////////////////////////////////////////////////////////////
SkImage_Lazy::Validator::Validator(sk_sp<SharedGenerator> gen, const SkIRect* subset,
- sk_sp<SkColorSpace> colorSpace)
+ const SkColorType* colorType, sk_sp<SkColorSpace> colorSpace)
: fSharedGenerator(std::move(gen)) {
if (!fSharedGenerator) {
return;
@@ -84,8 +84,13 @@
fInfo = info.makeWH(subset->width(), subset->height());
fOrigin = SkIPoint::Make(subset->x(), subset->y());
- if (colorSpace) {
- fInfo = fInfo.makeColorSpace(colorSpace);
+ if (colorType || colorSpace) {
+ if (colorType) {
+ fInfo = fInfo.makeColorType(*colorType);
+ }
+ if (colorSpace) {
+ fInfo = fInfo.makeColorSpace(colorSpace);
+ }
fUniqueID = SkNextID::ImageID();
}
}
@@ -249,30 +254,33 @@
SkASSERT(fInfo.bounds() != subset);
const SkIRect generatorSubset = subset.makeOffset(fOrigin.x(), fOrigin.y());
- Validator validator(fSharedGenerator, &generatorSubset, fInfo.refColorSpace());
+ const SkColorType colorType = fInfo.colorType();
+ Validator validator(fSharedGenerator, &generatorSubset, &colorType, fInfo.refColorSpace());
return validator ? sk_sp<SkImage>(new SkImage_Lazy(&validator)) : nullptr;
}
-sk_sp<SkImage> SkImage_Lazy::onMakeColorSpace(sk_sp<SkColorSpace> target) const {
- SkAutoExclusive autoAquire(fOnMakeColorSpaceMutex);
- if (fOnMakeColorSpaceTarget &&
- SkColorSpace::Equals(target.get(), fOnMakeColorSpaceTarget.get())) {
- return fOnMakeColorSpaceResult;
+sk_sp<SkImage> SkImage_Lazy::onMakeColorTypeAndColorSpace(SkColorType targetCT,
+ sk_sp<SkColorSpace> targetCS) const {
+ SkAutoExclusive autoAquire(fOnMakeColorTypeAndSpaceMutex);
+ if (fOnMakeColorTypeAndSpaceResult &&
+ targetCT == fOnMakeColorTypeAndSpaceResult->colorType() &&
+ SkColorSpace::Equals(targetCS.get(), fOnMakeColorTypeAndSpaceResult->colorSpace())) {
+ return fOnMakeColorTypeAndSpaceResult;
}
const SkIRect generatorSubset =
SkIRect::MakeXYWH(fOrigin.x(), fOrigin.y(), fInfo.width(), fInfo.height());
- Validator validator(fSharedGenerator, &generatorSubset, target);
+ Validator validator(fSharedGenerator, &generatorSubset, &targetCT, targetCS);
sk_sp<SkImage> result = validator ? sk_sp<SkImage>(new SkImage_Lazy(&validator)) : nullptr;
if (result) {
- fOnMakeColorSpaceTarget = target;
- fOnMakeColorSpaceResult = result;
+ fOnMakeColorTypeAndSpaceResult = result;
}
return result;
}
sk_sp<SkImage> SkImage::MakeFromGenerator(std::unique_ptr<SkImageGenerator> generator,
const SkIRect* subset) {
- SkImage_Lazy::Validator validator(SharedGenerator::Make(std::move(generator)), subset, nullptr);
+ SkImage_Lazy::Validator
+ validator(SharedGenerator::Make(std::move(generator)), subset, nullptr, nullptr);
return validator ? sk_make_sp<SkImage_Lazy>(&validator) : nullptr;
}
@@ -427,9 +435,10 @@
ScopedGenerator generator(fSharedGenerator);
Generator_GrYUVProvider provider(generator);
- // The pixels in the texture will be in the generator's color space. If onMakeColorSpace
- // has been called then this will not match this image's color space. To correct this, apply
- // a color space conversion from the generator's color space to this image's color space.
+ // The pixels in the texture will be in the generator's color space.
+ // If onMakeColorTypeAndColorSpace has been called then this will not match this image's
+ // color space. To correct this, apply a color space conversion from the generator's color
+ // space to this image's color space.
SkColorSpace* generatorColorSpace = fSharedGenerator->fGenerator->getInfo().colorSpace();
SkColorSpace* thisColorSpace = fInfo.colorSpace();
diff --git a/src/image/SkImage_Lazy.h b/src/image/SkImage_Lazy.h
index dbec740..51b893a 100644
--- a/src/image/SkImage_Lazy.h
+++ b/src/image/SkImage_Lazy.h
@@ -20,7 +20,8 @@
class SkImage_Lazy : public SkImage_Base {
public:
struct Validator {
- Validator(sk_sp<SharedGenerator>, const SkIRect* subset, sk_sp<SkColorSpace> colorSpace);
+ Validator(sk_sp<SharedGenerator>, const SkIRect* subset, const SkColorType* colorType,
+ sk_sp<SkColorSpace> colorSpace);
operator bool() const { return fSharedGenerator.get(); }
@@ -55,7 +56,7 @@
sk_sp<SkImage> onMakeSubset(const SkIRect&) const override;
bool getROPixels(SkBitmap*, CachingHint) const override;
bool onIsLazyGenerated() const override { return true; }
- sk_sp<SkImage> onMakeColorSpace(sk_sp<SkColorSpace>) const override;
+ sk_sp<SkImage> onMakeColorTypeAndColorSpace(SkColorType, sk_sp<SkColorSpace>) const override;
bool onIsValid(GrContext*) const override;
@@ -77,17 +78,16 @@
sk_sp<SharedGenerator> fSharedGenerator;
// Note that fInfo is not necessarily the info from the generator. It may be cropped by
- // onMakeSubset and its color space may be changed by onMakeColorSpace.
+ // onMakeSubset and its color type/space may be changed by onMakeColorTypeAndColorSpace.
const SkImageInfo fInfo;
const SkIPoint fOrigin;
uint32_t fUniqueID;
- // Repeated calls to onMakeColorSpace will result in a proliferation of unique IDs and
- // SkImage_Lazy instances. Cache the result of the last successful onMakeColorSpace call.
- mutable SkMutex fOnMakeColorSpaceMutex;
- mutable sk_sp<SkColorSpace> fOnMakeColorSpaceTarget;
- mutable sk_sp<SkImage> fOnMakeColorSpaceResult;
+ // Repeated calls to onMakeColorTypeAndColorSpace will result in a proliferation of unique IDs
+ // and SkImage_Lazy instances. Cache the result of the last successful call.
+ mutable SkMutex fOnMakeColorTypeAndSpaceMutex;
+ mutable sk_sp<SkImage> fOnMakeColorTypeAndSpaceResult;
#if SK_SUPPORT_GPU
// When the SkImage_Lazy goes away, we will iterate over all the unique keys we've used and
diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp
index 78ecedb..806f366 100644
--- a/src/image/SkImage_Raster.cpp
+++ b/src/image/SkImage_Raster.cpp
@@ -101,7 +101,7 @@
SkASSERT(bitmapMayBeMutable || fBitmap.isImmutable());
}
- sk_sp<SkImage> onMakeColorSpace(sk_sp<SkColorSpace>) const override;
+ sk_sp<SkImage> onMakeColorTypeAndColorSpace(SkColorType, sk_sp<SkColorSpace>) const override;
bool onIsValid(GrContext* context) const override { return true; }
void notifyAddedToRasterCache() const override {
@@ -337,12 +337,13 @@
///////////////////////////////////////////////////////////////////////////////
-sk_sp<SkImage> SkImage_Raster::onMakeColorSpace(sk_sp<SkColorSpace> target) const {
+sk_sp<SkImage> SkImage_Raster::onMakeColorTypeAndColorSpace(SkColorType targetCT,
+ sk_sp<SkColorSpace> targetCS) const {
SkPixmap src;
SkAssertResult(fBitmap.peekPixels(&src));
SkBitmap dst;
- dst.allocPixels(fBitmap.info().makeColorSpace(target));
+ dst.allocPixels(fBitmap.info().makeColorType(targetCT).makeColorSpace(targetCS));
SkAssertResult(dst.writePixels(src));
dst.setImmutable();