Call initializeColorXform inside SkCodec
Make initializeColorXform private, and only call in the base class.
Add virtual method to skip initializeColorXform, for classes that do
not need one.
Change SkCodec::FrameInfo's SkAlphaType to an SkEncodedInfo::Alpha.
This allows proper checking internally whether SkCodec needs to do a
color correct premultiply.
Depends on https://chromium-review.googlesource.com/c/620947, for this
API change.
(Separated from review.skia.org/25746)
Bug: skia:5609
Bug: skia:6839
Change-Id: Icb0d46659c546060c34d32eaf792c86708726c7a
Reviewed-on: https://skia-review.googlesource.com/35880
Reviewed-by: Chris Blume <cblume@chromium.org>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp
index d8167ef..6df3c93 100644
--- a/src/codec/SkBmpCodec.cpp
+++ b/src/codec/SkBmpCodec.cpp
@@ -620,10 +620,6 @@
SkCodec::Result SkBmpCodec::prepareToDecode(const SkImageInfo& dstInfo,
const SkCodec::Options& options) {
- if (!this->initializeColorXform(dstInfo, options.fPremulBehavior)) {
- return kInvalidConversion;
- }
-
return this->onPrepareToDecode(dstInfo, options);
}
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index 2168ac3..214b157 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -218,12 +218,18 @@
const int index = options.fFrameIndex;
if (0 == index) {
if (!this->conversionSupported(info, fEncodedInfo.color(), fEncodedInfo.opaque(),
- fSrcInfo.colorSpace())) {
+ fSrcInfo.colorSpace())
+ || !this->initializeColorXform(info, fEncodedInfo.alpha(), options.fPremulBehavior))
+ {
return kInvalidConversion;
}
return kSuccess;
}
+ if (index < 0) {
+ return kInvalidParameters;
+ }
+
if (options.fSubset || info.dimensions() != fSrcInfo.dimensions()) {
// If we add support for these, we need to update the code that zeroes
// a kRestoreBGColor frame.
@@ -246,47 +252,48 @@
}
const int requiredFrame = frame->getRequiredFrame();
- if (requiredFrame == kNone) {
- return kSuccess;
- }
-
- if (options.fPriorFrame != kNone) {
- // Check for a valid frame as a starting point. Alternatively, we could
- // treat an invalid frame as not providing one, but rejecting it will
- // make it easier to catch the mistake.
- if (options.fPriorFrame < requiredFrame || options.fPriorFrame >= index) {
- return kInvalidParameters;
- }
- const auto* prevFrame = frameHolder->getFrame(options.fPriorFrame);
- switch (prevFrame->getDisposalMethod()) {
- case SkCodecAnimation::DisposalMethod::kRestorePrevious:
+ if (requiredFrame != kNone) {
+ if (options.fPriorFrame != kNone) {
+ // Check for a valid frame as a starting point. Alternatively, we could
+ // treat an invalid frame as not providing one, but rejecting it will
+ // make it easier to catch the mistake.
+ if (options.fPriorFrame < requiredFrame || options.fPriorFrame >= index) {
return kInvalidParameters;
- case SkCodecAnimation::DisposalMethod::kRestoreBGColor:
- // If a frame after the required frame is provided, there is no
- // need to clear, since it must be covered by the desired frame.
- if (options.fPriorFrame == requiredFrame) {
- zero_rect(info, pixels, rowBytes, prevFrame->frameRect());
- }
- break;
- default:
- break;
- }
- return kSuccess;
- }
-
- Options prevFrameOptions(options);
- prevFrameOptions.fFrameIndex = requiredFrame;
- prevFrameOptions.fZeroInitialized = kNo_ZeroInitialized;
- const Result result = this->getPixels(info, pixels, rowBytes, &prevFrameOptions);
- if (result == kSuccess) {
- const auto* prevFrame = frameHolder->getFrame(requiredFrame);
- const auto disposalMethod = prevFrame->getDisposalMethod();
- if (disposalMethod == SkCodecAnimation::DisposalMethod::kRestoreBGColor) {
- zero_rect(info, pixels, rowBytes, prevFrame->frameRect());
+ }
+ const auto* prevFrame = frameHolder->getFrame(options.fPriorFrame);
+ switch (prevFrame->getDisposalMethod()) {
+ case SkCodecAnimation::DisposalMethod::kRestorePrevious:
+ return kInvalidParameters;
+ case SkCodecAnimation::DisposalMethod::kRestoreBGColor:
+ // If a frame after the required frame is provided, there is no
+ // need to clear, since it must be covered by the desired frame.
+ if (options.fPriorFrame == requiredFrame) {
+ zero_rect(info, pixels, rowBytes, prevFrame->frameRect());
+ }
+ break;
+ default:
+ break;
+ }
+ } else {
+ Options prevFrameOptions(options);
+ prevFrameOptions.fFrameIndex = requiredFrame;
+ prevFrameOptions.fZeroInitialized = kNo_ZeroInitialized;
+ const Result result = this->getPixels(info, pixels, rowBytes, &prevFrameOptions);
+ if (result != kSuccess) {
+ return result;
+ }
+ const auto* prevFrame = frameHolder->getFrame(requiredFrame);
+ const auto disposalMethod = prevFrame->getDisposalMethod();
+ if (disposalMethod == SkCodecAnimation::DisposalMethod::kRestoreBGColor) {
+ zero_rect(info, pixels, rowBytes, prevFrame->frameRect());
+ }
}
}
- return result;
+ FrameInfo frameInfo;
+ SkAssertResult(this->getFrameInfo(index, &frameInfo));
+ return this->initializeColorXform(info, frameInfo.fAlpha, options.fPremulBehavior)
+ ? kSuccess : kInvalidConversion;
}
SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
@@ -609,11 +616,14 @@
}
}
-bool SkCodec::initializeColorXform(const SkImageInfo& dstInfo,
+bool SkCodec::initializeColorXform(const SkImageInfo& dstInfo, SkEncodedInfo::Alpha encodedAlpha,
SkTransferFunctionBehavior premulBehavior) {
fColorXform = nullptr;
fXformOnDecode = false;
- bool needsColorCorrectPremul = needs_premul(dstInfo, fEncodedInfo) &&
+ if (!this->usesColorXform()) {
+ return true;
+ }
+ bool needsColorCorrectPremul = needs_premul(dstInfo.alphaType(), encodedAlpha) &&
SkTransferFunctionBehavior::kRespect == premulBehavior;
if (needs_color_xform(dstInfo, fSrcInfo.colorSpace(), needsColorCorrectPremul)) {
fColorXform = SkColorSpaceXform_Base::New(fSrcInfo.colorSpace(), dstInfo.colorSpace(),
diff --git a/src/codec/SkCodecPriv.h b/src/codec/SkCodecPriv.h
index a737fb7..93d2925 100644
--- a/src/codec/SkCodecPriv.h
+++ b/src/codec/SkCodecPriv.h
@@ -271,9 +271,8 @@
}
}
-static inline bool needs_premul(const SkImageInfo& dstInfo, const SkEncodedInfo& encodedInfo) {
- return kPremul_SkAlphaType == dstInfo.alphaType() &&
- SkEncodedInfo::kUnpremul_Alpha == encodedInfo.alpha();
+static inline bool needs_premul(SkAlphaType dstAT, SkEncodedInfo::Alpha encodedAlpha) {
+ return kPremul_SkAlphaType == dstAT && SkEncodedInfo::kUnpremul_Alpha == encodedAlpha;
}
static inline bool needs_color_xform(const SkImageInfo& dstInfo, const SkColorSpace* srcCS,
diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp
index 4bd0e17..9f9f8a6 100644
--- a/src/codec/SkGifCodec.cpp
+++ b/src/codec/SkGifCodec.cpp
@@ -146,8 +146,12 @@
frameInfo->fDuration = frameContext->getDuration();
frameInfo->fRequiredFrame = frameContext->getRequiredFrame();
frameInfo->fFullyReceived = frameContext->isComplete();
+#ifdef SK_LEGACY_FRAME_INFO_ALPHA_TYPE
frameInfo->fAlphaType = frameContext->hasAlpha() ? kUnpremul_SkAlphaType
: kOpaque_SkAlphaType;
+#endif
+ frameInfo->fAlpha = frameContext->hasAlpha() ? SkEncodedInfo::kBinary_Alpha
+ : SkEncodedInfo::kOpaque_Alpha;
frameInfo->fDisposalMethod = frameContext->getDisposalMethod();
}
return true;
@@ -226,12 +230,6 @@
SkASSERT(frame->reachedStartOfData());
}
- const auto at = frame->hasAlpha() ? kUnpremul_SkAlphaType : kOpaque_SkAlphaType;
- const auto srcInfo = this->getInfo().makeAlphaType(at);
- if (!this->initializeColorXform(dstInfo, opts.fPremulBehavior)) {
- return gif_error("Cannot convert input type to output type.\n", kInvalidConversion);
- }
-
if (this->xformOnDecode()) {
fXformBuffer.reset(new uint32_t[dstInfo.width()]);
sk_bzero(fXformBuffer.get(), dstInfo.width() * sizeof(uint32_t));
diff --git a/src/codec/SkHeifCodec.cpp b/src/codec/SkHeifCodec.cpp
index 5ccb52a..008c129 100644
--- a/src/codec/SkHeifCodec.cpp
+++ b/src/codec/SkHeifCodec.cpp
@@ -270,10 +270,6 @@
return kUnimplemented;
}
- if (!this->initializeColorXform(dstInfo, options.fPremulBehavior)) {
- return kInvalidConversion;
- }
-
// Check if we can decode to the requested destination and set the output color space
if (!this->setOutputColorFormat(dstInfo)) {
return kInvalidConversion;
@@ -347,10 +343,6 @@
SkCodec::Result SkHeifCodec::onStartScanlineDecode(
const SkImageInfo& dstInfo, const Options& options) {
- if (!this->initializeColorXform(dstInfo, options.fPremulBehavior)) {
- return kInvalidConversion;
- }
-
// Check if we can decode to the requested destination and set the output color space
if (!this->setOutputColorFormat(dstInfo)) {
return kInvalidConversion;
diff --git a/src/codec/SkIcoCodec.h b/src/codec/SkIcoCodec.h
index 62de20f..0b77df5 100644
--- a/src/codec/SkIcoCodec.h
+++ b/src/codec/SkIcoCodec.h
@@ -54,6 +54,8 @@
return true;
}
+ // Handled by the embedded codec.
+ bool usesColorXform() const override { return false; }
private:
Result onStartScanlineDecode(const SkImageInfo& dstInfo,
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index 8f88138..837481c 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -581,10 +581,6 @@
return fDecoderMgr->returnFailure("setjmp", kInvalidInput);
}
- if (!this->initializeColorXform(dstInfo, options.fPremulBehavior)) {
- return kInvalidConversion;
- }
-
// Check if we can decode to the requested destination and set the output color space
if (!this->setOutputColorSpace(dstInfo)) {
return fDecoderMgr->returnFailure("setOutputColorSpace", kInvalidConversion);
@@ -690,10 +686,6 @@
return kInvalidInput;
}
- if (!this->initializeColorXform(dstInfo, options.fPremulBehavior)) {
- return kInvalidConversion;
- }
-
// Check if we can decode to the requested destination and set the output color space
if (!this->setOutputColorSpace(dstInfo)) {
return fDecoderMgr->returnFailure("setOutputColorSpace", kInvalidConversion);
diff --git a/src/codec/SkPngCodec.cpp b/src/codec/SkPngCodec.cpp
index c5dae3e..08f23dd 100644
--- a/src/codec/SkPngCodec.cpp
+++ b/src/codec/SkPngCodec.cpp
@@ -263,7 +263,8 @@
if (png_get_tRNS(fPng_ptr, fInfo_ptr, &alphas, &numColorsWithAlpha, nullptr)) {
// If we are performing a color xform, it will handle the premultiply. Otherwise,
// we'll do it here.
- bool premultiply = !this->colorXform() && needs_premul(dstInfo, this->getEncodedInfo());
+ bool premultiply = !this->colorXform() && needs_premul(dstInfo.alphaType(),
+ this->getEncodedInfo().alpha());
// Choose which function to use to create the color table. If the final destination's
// colortype is unpremultiplied, the color table will store unpremultiplied colors.
@@ -980,10 +981,6 @@
// interlaced scanline decoder may need to rewind.
fSwizzler.reset(nullptr);
- if (!this->initializeColorXform(dstInfo, options.fPremulBehavior)) {
- return kInvalidConversion;
- }
-
// If SkColorSpaceXform directly supports the encoded PNG format, we should skip format
// conversion in the swizzler (or skip swizzling altogether).
bool skipFormatConversion = false;
diff --git a/src/codec/SkRawCodec.cpp b/src/codec/SkRawCodec.cpp
index 93c6db5..f739247 100644
--- a/src/codec/SkRawCodec.cpp
+++ b/src/codec/SkRawCodec.cpp
@@ -696,11 +696,6 @@
SkCodec::Result SkRawCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
size_t dstRowBytes, const Options& options,
int* rowsDecoded) {
- if (!this->initializeColorXform(dstInfo, options.fPremulBehavior)) {
- SkCodecPrintf("Error: cannot convert input type to output type.\n");
- return kInvalidConversion;
- }
-
SkImageInfo swizzlerInfo = dstInfo;
std::unique_ptr<uint32_t[]> xformBuffer = nullptr;
if (this->colorXform()) {
diff --git a/src/codec/SkWbmpCodec.h b/src/codec/SkWbmpCodec.h
index f26967f..57efe21 100644
--- a/src/codec/SkWbmpCodec.h
+++ b/src/codec/SkWbmpCodec.h
@@ -30,6 +30,8 @@
bool onRewind() override;
bool conversionSupported(const SkImageInfo& dst, SkEncodedInfo::Color srcColor,
bool srcIsOpaque, const SkColorSpace* srcCS) const override;
+ // No need to Xform; all pixels are either black or white.
+ bool usesColorXform() const override { return false; }
private:
/*
* Returns a swizzler on success, nullptr on failure
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index 38feb82..dd87c26 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -305,7 +305,11 @@
// libwebp only reports fully received frames for an
// animated image.
frameInfo->fFullyReceived = true;
+#ifdef SK_LEGACY_FRAME_INFO_ALPHA_TYPE
frameInfo->fAlphaType = alpha_type(frame->hasAlpha());
+#endif
+ frameInfo->fAlpha = frame->hasAlpha() ? SkEncodedInfo::kUnpremul_Alpha
+ : SkEncodedInfo::kOpaque_Alpha;
frameInfo->fDisposalMethod = frame->getDisposalMethod();
}
@@ -398,10 +402,6 @@
SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst, size_t rowBytes,
const Options& options, int* rowsDecodedPtr) {
- if (!this->initializeColorXform(dstInfo, options.fPremulBehavior)) {
- return kInvalidConversion;
- }
-
const int index = options.fFrameIndex;
SkASSERT(0 == index || index < fFrameHolder.size());