Replace nearly all kRespect with kIgnore
- Encoders and decoders always assume kIgnore.
- They are less opinionated about F16 and color space,
we just trust the color space that's passed in, and
put that directly in the image (no sRGB encoding).
- SkBitmap and SkPixmap read/write pixels functions were
defaulting to kResepct, those are now always kIgnore.
- Many other bits of plumbing are simplified, and I
added a default of kIgnore to SkImage::makeColorSpace,
so we can phase out that argument entirely.
- Still need to add defaults to other public APIs that
take SkTransferFunctionBehavior.
- This makes gold think that we've dramatically changed
the contents of all F16 images, but that's because
it doesn't understand the (now linear) color space
that's embedded. Once we triage them all once, they
will work fine (and they'll look perfect in the browser).
Bug: skia:
Change-Id: I62fa090f96cae1b67d181ce14bd91f34ff2ed747
Reviewed-on: https://skia-review.googlesource.com/140570
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index d9992c0..130fcd1 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -174,7 +174,7 @@
return srcIsOpaque;
case kGray_8_SkColorType:
return kGray_8_SkColorType == srcColor && srcIsOpaque &&
- !needs_color_xform(dst, srcCS, false);
+ !needs_color_xform(dst, srcCS);
case kAlpha_8_SkColorType:
// conceptually we can convert anything into alpha_8, but we haven't actually coded
// all of those other conversions yet, so only return true for the case we have codec.
@@ -247,7 +247,7 @@
if (0 == index) {
if (!this->conversionSupported(info, fSrcInfo.colorType(), fEncodedInfo.opaque(),
fSrcInfo.colorSpace())
- || !this->initializeColorXform(info, fEncodedInfo.alpha(), options.fPremulBehavior))
+ || !this->initializeColorXform(info, fEncodedInfo.alpha()))
{
return kInvalidConversion;
}
@@ -324,8 +324,7 @@
}
}
- return this->initializeColorXform(info, frame->reportedAlpha(), options.fPremulBehavior)
- ? kSuccess : kInvalidConversion;
+ return this->initializeColorXform(info, frame->reportedAlpha()) ? kSuccess : kInvalidConversion;
}
SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
@@ -648,8 +647,7 @@
}
}
-bool SkCodec::initializeColorXform(const SkImageInfo& dstInfo, SkEncodedInfo::Alpha encodedAlpha,
- SkTransferFunctionBehavior premulBehavior) {
+bool SkCodec::initializeColorXform(const SkImageInfo& dstInfo, SkEncodedInfo::Alpha encodedAlpha) {
fColorXform = nullptr;
fXformOnDecode = false;
if (!this->usesColorXform()) {
@@ -659,12 +657,8 @@
// a colorXform to do a color correct premul, since the blend step will handle
// premultiplication. But there is no way to know whether we need to blend from
// inside this call.
- bool needsColorCorrectPremul = needs_premul(dstInfo.alphaType(), encodedAlpha) &&
- SkTransferFunctionBehavior::kRespect == premulBehavior;
- if (needs_color_xform(dstInfo, fSrcInfo.colorSpace(), needsColorCorrectPremul)) {
- fColorXform = SkMakeColorSpaceXform(fSrcInfo.colorSpace(),
- dstInfo.colorSpace(),
- premulBehavior);
+ if (needs_color_xform(dstInfo, fSrcInfo.colorSpace())) {
+ fColorXform = SkMakeColorSpaceXform(fSrcInfo.colorSpace(), dstInfo.colorSpace());
if (!fColorXform) {
return false;
}
diff --git a/src/codec/SkCodecImageGenerator.cpp b/src/codec/SkCodecImageGenerator.cpp
index 6727dcf..55a6575 100644
--- a/src/codec/SkCodecImageGenerator.cpp
+++ b/src/codec/SkCodecImageGenerator.cpp
@@ -44,9 +44,7 @@
SkPixmap dst(requestInfo, requestPixels, requestRowBytes);
auto decode = [this](const SkPixmap& pm) {
- SkCodec::Options codecOpts;
- codecOpts.fPremulBehavior = SkTransferFunctionBehavior::kIgnore;
- SkCodec::Result result = fCodec->getPixels(pm, &codecOpts);
+ SkCodec::Result result = fCodec->getPixels(pm);
switch (result) {
case SkCodec::kSuccess:
case SkCodec::kIncompleteInput:
diff --git a/src/codec/SkCodecPriv.h b/src/codec/SkCodecPriv.h
index 1251d8d..65d4575 100644
--- a/src/codec/SkCodecPriv.h
+++ b/src/codec/SkCodecPriv.h
@@ -276,20 +276,20 @@
return kPremul_SkAlphaType == dstAT && SkEncodedInfo::kUnpremul_Alpha == encodedAlpha;
}
-static inline bool needs_color_xform(const SkImageInfo& dstInfo, const SkColorSpace* srcCS,
- bool needsColorCorrectPremul) {
+static inline bool needs_color_xform(const SkImageInfo& dstInfo, const SkColorSpace* srcCS) {
// We never perform a color xform in legacy mode.
if (!dstInfo.colorSpace()) {
return false;
}
- // F16 is by definition a linear space, so we always must perform a color xform.
+ // None of the codecs we have output F16 natively, so if we're trying to decode to F16,
+ // we'll have to use SkColorSpaceXform to get there.
bool isF16 = kRGBA_F16_SkColorType == dstInfo.colorType();
// Need a color xform when dst space does not match the src.
bool srcDstNotEqual = !SkColorSpace::Equals(srcCS, dstInfo.colorSpace());
- return needsColorCorrectPremul || isF16 || srcDstNotEqual;
+ return isF16 || srcDstNotEqual;
}
static inline SkAlphaType select_xform_alpha(SkAlphaType dstAlphaType, SkAlphaType srcAlphaType) {
diff --git a/src/codec/SkSampledCodec.cpp b/src/codec/SkSampledCodec.cpp
index ac0539f..cf97b0b 100644
--- a/src/codec/SkSampledCodec.cpp
+++ b/src/codec/SkSampledCodec.cpp
@@ -75,7 +75,6 @@
// Create an Options struct for the codec.
SkCodec::Options codecOptions;
codecOptions.fZeroInitialized = options.fZeroInitialized;
- codecOptions.fPremulBehavior = SkTransferFunctionBehavior::kIgnore;
SkIRect* subset = options.fSubset;
if (!subset || subset->size() == this->codec()->getInfo().dimensions()) {
@@ -170,7 +169,6 @@
// Create options struct for the codec.
SkCodec::Options sampledOptions;
sampledOptions.fZeroInitialized = options.fZeroInitialized;
- sampledOptions.fPremulBehavior = SkTransferFunctionBehavior::kIgnore;
// FIXME: This was already called by onGetAndroidPixels. Can we reduce that?
int sampleSize = options.fSampleSize;
diff --git a/src/codec/SkWebpAdapterCodec.cpp b/src/codec/SkWebpAdapterCodec.cpp
index 81011d0..79a99a7 100644
--- a/src/codec/SkWebpAdapterCodec.cpp
+++ b/src/codec/SkWebpAdapterCodec.cpp
@@ -27,6 +27,5 @@
SkCodec::Options codecOptions;
codecOptions.fZeroInitialized = options.fZeroInitialized;
codecOptions.fSubset = options.fSubset;
- codecOptions.fPremulBehavior = SkTransferFunctionBehavior::kIgnore;
return this->codec()->getPixels(info, pixels, rowBytes, &codecOptions);
}
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index b6485b1..bb6e430 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -514,14 +514,6 @@
const bool blendWithPrevFrame = !independent && frame.blend_method == WEBP_MUX_BLEND
&& frame.has_alpha;
- if (blendWithPrevFrame && options.fPremulBehavior == SkTransferFunctionBehavior::kRespect) {
- // Blending is done with SkRasterPipeline, which requires a color space that is valid for
- // rendering.
- const auto* cs = dstInfo.colorSpace();
- if (!cs || (!cs->gammaCloseToSRGB() && !cs->gammaIsLinear())) {
- return kInvalidConversion;
- }
- }
SkBitmap webpDst;
auto webpInfo = dstInfo;