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>

1 files changed, 50 insertions, 40 deletions

diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index 2168ac3521..214b157628 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -218,12 +218,18 @@ SkCodec::Result SkCodec::handleFrameIndex(const SkImageInfo& info, void* pixels,
     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 @@ SkCodec::Result SkCodec::handleFrameIndex(const SkImageInfo& info, void* pixels,
     }
 
     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 @@ static inline SkColorSpaceXform::ColorFormat select_xform_format_ct(SkColorType
     }
 }
 
-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(),