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/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(),