Revert "SkAndroidCodec: Support decoding all frames"

This reverts commit fc4fdc5b25.

Reason for revert: Google3 and ASAN failures

Change-Id: I890cd76109c0375391637f879550837d01e650f9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334840
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>

BUILD.gn

@@ -1236,7 +1236,7 @@ component("skia") {
     "src/sfnt/SkOTUtils.cpp",
   ]
 
-  defines = [ "SK_HAS_ANDROID_CODEC" ]
+  defines = []
   libs = []
 
   if (is_win) {

dm/DM.cpp

@@ -764,10 +764,6 @@ static void push_codec_srcs(Path path) {
                     push_codec_src(path, CodecSrc::kAnimated_Mode, dstCT, at, 1.0f);
                 }
             }
-            for (float scale : { .5f, .33f }) {
-                push_codec_src(path, CodecSrc::kAnimated_Mode, CodecSrc::kGetFromCanvas_DstColorType,
-                               kPremul_SkAlphaType, scale);
-            }
         }
 
     }

dm/DMSrcSink.cpp

@@ -436,13 +436,6 @@ Result CodecSrc::draw(GrDirectContext*, SkCanvas* canvas) const {
 
     // Try to scale the image if it is desired
     SkISize size = codec->getScaledDimensions(fScale);
-
-    std::unique_ptr<SkAndroidCodec> androidCodec;
-    if (1.0f != fScale && fMode == kAnimated_Mode) {
-        androidCodec = SkAndroidCodec::MakeFromData(encoded);
-        size = androidCodec->getSampledDimensions(1 / fScale);
-    }
-
     if (size == decodeInfo.dimensions() && 1.0f != fScale) {
         return Result::Skip("Test without scaling is uninteresting.");
     }
@@ -474,16 +467,7 @@ Result CodecSrc::draw(GrDirectContext*, SkCanvas* canvas) const {
 
     switch (fMode) {
         case kAnimated_Mode: {
-            SkAndroidCodec::AndroidOptions androidOptions;
-            if (fScale != 1.0f) {
-                SkASSERT(androidCodec);
-                androidOptions.fSampleSize = 1 / fScale;
-                auto dims = androidCodec->getSampledDimensions(androidOptions.fSampleSize);
-                decodeInfo = decodeInfo.makeDimensions(dims);
-            }
-
-            std::vector<SkCodec::FrameInfo> frameInfos = androidCodec
-                    ? androidCodec->codec()->getFrameInfo() : codec->getFrameInfo();
+            std::vector<SkCodec::FrameInfo> frameInfos = codec->getFrameInfo();
             if (frameInfos.size() <= 1) {
                 return Result::Fatal("%s is not an animated image.", fPath.c_str());
             }
@@ -498,21 +482,19 @@ Result CodecSrc::draw(GrDirectContext*, SkCanvas* canvas) const {
             SkAutoMalloc priorFramePixels;
             int cachedFrame = SkCodec::kNoFrame;
             for (int i = 0; static_cast<size_t>(i) < frameInfos.size(); i++) {
-                androidOptions.fFrameIndex = i;
+                options.fFrameIndex = i;
                 // Check for a prior frame
                 const int reqFrame = frameInfos[i].fRequiredFrame;
                 if (reqFrame != SkCodec::kNoFrame && reqFrame == cachedFrame
                         && priorFramePixels.get()) {
                     // Copy into pixels
                     memcpy(pixels.get(), priorFramePixels.get(), safeSize);
-                    androidOptions.fPriorFrame = reqFrame;
+                    options.fPriorFrame = reqFrame;
                 } else {
-                    androidOptions.fPriorFrame = SkCodec::kNoFrame;
+                    options.fPriorFrame = SkCodec::kNoFrame;
                 }
-                SkCodec::Result result = androidCodec
-                        ? androidCodec->getAndroidPixels(decodeInfo, pixels.get(), rowBytes,
-                                                         &androidOptions)
-                        : codec->getPixels(decodeInfo, pixels.get(), rowBytes, &androidOptions);
+                SkCodec::Result result = codec->getPixels(decodeInfo, pixels.get(),
+                                                          rowBytes, &options);
                 if (SkCodec::kInvalidInput == result && i > 0) {
                     // Some of our test images have truncated later frames. Treat that
                     // the same as incomplete.
@@ -777,33 +759,30 @@ SkISize CodecSrc::size() const {
         return {0, 0};
     }
 
-    if (fMode != kAnimated_Mode) {
-        return codec->getScaledDimensions(fScale);
+    auto imageSize = codec->getScaledDimensions(fScale);
+    if (fMode == kAnimated_Mode) {
+        // We'll draw one of each frame, so make it big enough to hold them all
+        // in a grid. The grid will be roughly square, with "factor" frames per
+        // row and up to "factor" rows.
+        const size_t count = codec->getFrameInfo().size();
+        const float root = sqrt((float) count);
+        const int factor = sk_float_ceil2int(root);
+        imageSize.fWidth  = imageSize.fWidth  * factor;
+        imageSize.fHeight = imageSize.fHeight * sk_float_ceil2int((float) count / (float) factor);
     }
-
-    // We'll draw one of each frame, so make it big enough to hold them all
-    // in a grid. The grid will be roughly square, with "factor" frames per
-    // row and up to "factor" rows.
-    const size_t count = codec->getFrameInfo().size();
-    const float root = sqrt((float) count);
-    const int factor = sk_float_ceil2int(root);
-
-    auto androidCodec = SkAndroidCodec::MakeFromCodec(std::move(codec));
-    auto imageSize = androidCodec->getSampledDimensions(1 / fScale);
-    imageSize.fWidth  = imageSize.fWidth  * factor;
-    imageSize.fHeight = imageSize.fHeight * sk_float_ceil2int((float) count / (float) factor);
     return imageSize;
 }
 
 Name CodecSrc::name() const {
-    Name name = SkOSPath::Basename(fPath.c_str());
-    if (fMode == kAnimated_Mode) {
-        name.append("_animated");
-    }
     if (1.0f == fScale) {
+        Name name = SkOSPath::Basename(fPath.c_str());
+        if (fMode == kAnimated_Mode) {
+            name.append("_animated");
+        }
         return name;
     }
-    return get_scaled_name(name.c_str(), fScale);
+    SkASSERT(fMode != kAnimated_Mode);
+    return get_scaled_name(fPath, fScale);
 }
 
 /*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/

include/codec/SkAndroidCodec.h

@@ -195,12 +195,32 @@ public:
     //        called SkAndroidCodec.  On the other hand, it's may be a bit confusing to call
     //        these Options when SkCodec has a slightly different set of Options.  Maybe these
     //        should be DecodeOptions or SamplingOptions?
-    struct AndroidOptions : public SkCodec::Options {
+    struct AndroidOptions {
         AndroidOptions()
-            : SkCodec::Options()
+            : fZeroInitialized(SkCodec::kNo_ZeroInitialized)
+            , fSubset(nullptr)
             , fSampleSize(1)
         {}
 
+        /**
+         *  Indicates is destination pixel memory is zero initialized.
+         *
+         *  The default is SkCodec::kNo_ZeroInitialized.
+         */
+        SkCodec::ZeroInitialized fZeroInitialized;
+
+        /**
+         *  If not NULL, represents a subset of the original image to decode.
+         *
+         *  Must be within the bounds returned by getInfo().
+         *
+         *  If the EncodedFormat is SkEncodedImageFormat::kWEBP, the top and left
+         *  values must be even.
+         *
+         *  The default is NULL, meaning a decode of the entire image.
+         */
+        SkIRect* fSubset;
+
         /**
          *  The client may provide an integer downscale factor for the decode.
          *  The codec may implement this downscaling by sampling or another

include/codec/SkCodec.h

@@ -24,7 +24,6 @@
 
 #include <vector>
 
-class SkAndroidCodec;
 class SkColorSpace;
 class SkData;
 class SkFrameHolder;
@@ -855,10 +854,6 @@ private:
 
     bool                               fStartedIncrementalDecode;
 
-    // Allows SkAndroidCodec to call handleFrameIndex (potentially decoding a prior frame and
-    // clearing to transparent) without SkCodec calling it, too.
-    bool                               fAndroidCodecHandlesFrameIndex;
-
     bool initializeColorXform(const SkImageInfo& dstInfo, SkEncodedInfo::Alpha, bool srcIsOpaque);
 
     /**
@@ -883,15 +878,8 @@ private:
 
     /**
      *  Check for a valid Options.fFrameIndex, and decode prior frames if necessary.
-     *
-     *  If androidCodec is not null, that means this SkCodec is owned by an SkAndroidCodec. In that
-     *  case, the Options will be treated as an AndroidOptions, and SkAndroidCodec will be used to
-     *  decode a prior frame, if a prior frame is needed. When such an owned SkCodec calls
-     *  handleFrameIndex, it will immediately return kSuccess, since SkAndroidCodec already handled
-     *  it.
      */
-    Result handleFrameIndex(const SkImageInfo&, void* pixels, size_t rowBytes, const Options&,
-            SkAndroidCodec* androidCodec = nullptr);
+    Result handleFrameIndex(const SkImageInfo&, void* pixels, size_t rowBytes, const Options&);
 
     // Methods for scanline decoding.
     virtual Result onStartScanlineDecode(const SkImageInfo& /*dstInfo*/,

src/codec/SkAndroidCodec.cpp

@@ -372,33 +372,19 @@ SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& requestInfo,
     AndroidOptions defaultOptions;
     if (!options) {
         options = &defaultOptions;
-    } else {
-        if (options->fSubset) {
-            if (!is_valid_subset(*options->fSubset, adjustedInfo.dimensions())) {
-                return SkCodec::kInvalidParameters;
-            }
-
-            if (SkIRect::MakeSize(adjustedInfo.dimensions()) == *options->fSubset) {
-                // The caller wants the whole thing, rather than a subset. Modify
-                // the AndroidOptions passed to onGetAndroidPixels to not specify
-                // a subset.
-                defaultOptions = *options;
-                defaultOptions.fSubset = nullptr;
-                options = &defaultOptions;
-            }
-        }
-
-        // To simplify frame compositing, force the client to use kIgnore and
-        // handle orientation themselves.
-        if (options->fFrameIndex != 0 && fOrientationBehavior == ExifOrientationBehavior::kRespect
-                && fCodec->getOrigin() != kDefault_SkEncodedOrigin) {
+    } else if (options->fSubset) {
+        if (!is_valid_subset(*options->fSubset, adjustedInfo.dimensions())) {
             return SkCodec::kInvalidParameters;
         }
-    }
 
-    if (auto result = fCodec->handleFrameIndex(requestInfo, requestPixels, requestRowBytes,
-            *options, this); result != SkCodec::kSuccess) {
-        return result;
+        if (SkIRect::MakeSize(adjustedInfo.dimensions()) == *options->fSubset) {
+            // The caller wants the whole thing, rather than a subset. Modify
+            // the AndroidOptions passed to onGetAndroidPixels to not specify
+            // a subset.
+            defaultOptions = *options;
+            defaultOptions.fSubset = nullptr;
+            options = &defaultOptions;
+        }
     }
 
     if (ExifOrientationBehavior::kIgnore == fOrientationBehavior) {

src/codec/SkAndroidCodecAdapter.cpp

@@ -23,5 +23,8 @@ bool SkAndroidCodecAdapter::onGetSupportedSubset(SkIRect* desiredSubset) const {
 
 SkCodec::Result SkAndroidCodecAdapter::onGetAndroidPixels(const SkImageInfo& info, void* pixels,
         size_t rowBytes, const AndroidOptions& options) {
-    return this->codec()->getPixels(info, pixels, rowBytes, &options);
+    SkCodec::Options codecOptions;
+    codecOptions.fZeroInitialized = options.fZeroInitialized;
+    codecOptions.fSubset = options.fSubset;
+    return this->codec()->getPixels(info, pixels, rowBytes, &codecOptions);
 }

src/codec/SkCodec.cpp

@@ -5,7 +5,6 @@
  * found in the LICENSE file.
  */
 
-#include "include/codec/SkAndroidCodec.h"
 #include "include/codec/SkCodec.h"
 #include "include/core/SkColorSpace.h"
 #include "include/core/SkData.h"
@@ -166,7 +165,6 @@ SkCodec::SkCodec(SkEncodedInfo&& info, XformFormat srcFormat, std::unique_ptr<Sk
     , fOptions()
     , fCurrScanline(-1)
     , fStartedIncrementalDecode(false)
-    , fAndroidCodecHandlesFrameIndex(false)
 {}
 
 SkCodec::~SkCodec() {}
@@ -247,21 +245,25 @@ static SkIRect frame_rect_on_screen(SkIRect frameRect,
 
 bool zero_rect(const SkImageInfo& dstInfo, void* pixels, size_t rowBytes,
                SkISize srcDimensions, SkIRect prevRect) {
+    prevRect = frame_rect_on_screen(prevRect, SkIRect::MakeSize(srcDimensions));
+    if (prevRect.isEmpty()) {
+        return true;
+    }
     const auto dimensions = dstInfo.dimensions();
     if (dimensions != srcDimensions) {
         SkRect src = SkRect::Make(srcDimensions);
         SkRect dst = SkRect::Make(dimensions);
-        SkMatrix map = SkMatrix::MakeRectToRect(src, dst, SkMatrix::kFill_ScaleToFit);
+        SkMatrix map = SkMatrix::MakeRectToRect(src, dst, SkMatrix::kCenter_ScaleToFit);
         SkRect asRect = SkRect::Make(prevRect);
         if (!map.mapRect(&asRect)) {
             return false;
         }
-        asRect.roundOut(&prevRect);
-    }
-
-    if (!prevRect.intersect(SkIRect::MakeSize(dimensions))) {
-        // Nothing to zero, due to scaling or bad frame rect.
-        return true;
+        asRect.roundIn(&prevRect);
+        if (prevRect.isEmpty()) {
+            // Down-scaling shrank the empty portion to nothing,
+            // so nothing to zero.
+            return true;
+        }
     }
 
     const SkImageInfo info = dstInfo.makeDimensions(prevRect.size());
@@ -273,19 +275,7 @@ bool zero_rect(const SkImageInfo& dstInfo, void* pixels, size_t rowBytes,
 }
 
 SkCodec::Result SkCodec::handleFrameIndex(const SkImageInfo& info, void* pixels, size_t rowBytes,
-                                          const Options& options, SkAndroidCodec* androidCodec) {
-    if (androidCodec) {
-        // This is never set back to false. If SkAndroidCodec is calling this method, its fCodec
-        // should never call it directly.
-        fAndroidCodecHandlesFrameIndex = true;
-    } else if (fAndroidCodecHandlesFrameIndex) {
-        return kSuccess;
-    }
-
-    if (!this->rewindIfNeeded()) {
-        return kCouldNotRewind;
-    }
-
+                                          const Options& options) {
     const int index = options.fFrameIndex;
     if (0 == index) {
         return this->initializeColorXform(info, fEncodedInfo.alpha(), fEncodedInfo.opaque())
@@ -314,55 +304,46 @@ SkCodec::Result SkCodec::handleFrameIndex(const SkImageInfo& info, void* pixels,
 
     const int requiredFrame = frame->getRequiredFrame();
     if (requiredFrame != kNoFrame) {
-        const SkFrame* preppedFrame = nullptr;
-        if (options.fPriorFrame == kNoFrame) {
-            Result result = kInternalError;
-            if (androidCodec) {
-#ifdef SK_HAS_ANDROID_CODEC
-                SkAndroidCodec::AndroidOptions prevFrameOptions(
-                        reinterpret_cast<const SkAndroidCodec::AndroidOptions&>(options));
-                prevFrameOptions.fFrameIndex = requiredFrame;
-                result = androidCodec->getAndroidPixels(info, pixels, rowBytes, &prevFrameOptions);
-#endif
-            } else {
-                Options prevFrameOptions(options);
-                prevFrameOptions.fFrameIndex = requiredFrame;
-                result = this->getPixels(info, pixels, rowBytes, &prevFrameOptions);
-            }
-            if (result != kSuccess) {
-                return result;
-            }
-            preppedFrame = frameHolder->getFrame(requiredFrame);
-        } else {
+        if (options.fPriorFrame != kNoFrame) {
             // 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;
             }
-            preppedFrame = frameHolder->getFrame(options.fPriorFrame);
-        }
-
-        SkASSERT(preppedFrame);
-        switch (preppedFrame->getDisposalMethod()) {
-            case SkCodecAnimation::DisposalMethod::kRestorePrevious:
-                SkASSERT(options.fPriorFrame != kNoFrame);
-                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.
-                // FIXME: If the required frame is kRestoreBGColor, we don't actually need to decode
-                // it, since we'll just clear it to transparent. Instead, we could decode *its*
-                // required frame and then clear.
-                if (preppedFrame->frameId() == requiredFrame) {
-                    SkIRect preppedRect = preppedFrame->frameRect();
-                    if (!zero_rect(info, pixels, rowBytes, this->dimensions(), preppedRect)) {
-                        return kInternalError;
+            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) {
+                        SkIRect prevRect = prevFrame->frameRect();
+                        if (!zero_rect(info, pixels, rowBytes, this->dimensions(), prevRect)) {
+                            return kInternalError;
+                        }
                     }
+                    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) {
+                auto prevRect = prevFrame->frameRect();
+                if (!zero_rect(info, pixels, rowBytes, this->dimensions(), prevRect)) {
+                    return kInternalError;
                 }
-                break;
-            default:
-                break;
+            }
         }
     }
 
@@ -382,6 +363,10 @@ SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t
         return kInvalidParameters;
     }
 
+    if (!this->rewindIfNeeded()) {
+        return kCouldNotRewind;
+    }
+
     // Default options.
     Options optsStorage;
     if (nullptr == options) {
@@ -447,6 +432,15 @@ SkCodec::Result SkCodec::startIncrementalDecode(const SkImageInfo& info, void* p
         return kInvalidParameters;
     }
 
+    // FIXME: If the rows come after the rows of a previous incremental decode,
+    // we might be able to skip the rewind, but only the implementation knows
+    // that. (e.g. PNG will always need to rewind, since we called longjmp, but
+    // a bottom-up BMP could skip rewinding if the new rows are above the old
+    // rows.)
+    if (!this->rewindIfNeeded()) {
+        return kCouldNotRewind;
+    }
+
     // Set options.
     Options optsStorage;
     if (nullptr == options) {
@@ -501,6 +495,10 @@ SkCodec::Result SkCodec::startScanlineDecode(const SkImageInfo& info,
     // Reset fCurrScanline in case of failure.
     fCurrScanline = -1;
 
+    if (!this->rewindIfNeeded()) {
+        return kCouldNotRewind;
+    }
+
     // Set options.
     Options optsStorage;
     if (nullptr == options) {
@@ -540,15 +538,6 @@ SkCodec::Result SkCodec::startScanlineDecode(const SkImageInfo& info,
         return result;
     }
 
-    // FIXME: See startIncrementalDecode. That method set fNeedsRewind to false
-    // so that when onStartScanlineDecode calls rewindIfNeeded it would not
-    // rewind. But it also relies on that call to rewindIfNeeded to set
-    // fNeedsRewind to true for future decodes. When
-    // fAndroidCodecHandlesFrameIndex is true, that call to rewindIfNeeded is
-    // skipped, so this method sets it back to true.
-    SkASSERT(fAndroidCodecHandlesFrameIndex || fNeedsRewind);
-    fNeedsRewind = true;
-
     fCurrScanline = 0;
     fDstInfo = info;
     fOptions = *options;

src/codec/SkSampledCodec.cpp

@@ -73,10 +73,14 @@ SkISize SkSampledCodec::onGetSampledDimensions(int sampleSize) const {
 
 SkCodec::Result SkSampledCodec::onGetAndroidPixels(const SkImageInfo& info, void* pixels,
         size_t rowBytes, const AndroidOptions& options) {
-    const SkIRect* subset = options.fSubset;
+    // Create an Options struct for the codec.
+    SkCodec::Options codecOptions;
+    codecOptions.fZeroInitialized = options.fZeroInitialized;
+
+    SkIRect* subset = options.fSubset;
     if (!subset || subset->size() == this->codec()->dimensions()) {
         if (this->codec()->dimensionsSupported(info.dimensions())) {
-            return this->codec()->getPixels(info, pixels, rowBytes, &options);
+            return this->codec()->getPixels(info, pixels, rowBytes, &codecOptions);
         }
 
         // If the native codec does not support the requested scale, scale by sampling.
@@ -99,17 +103,15 @@ SkCodec::Result SkSampledCodec::onGetAndroidPixels(const SkImageInfo& info, void
 
     const SkImageInfo scaledInfo = info.makeDimensions(scaledSize);
 
-    // Copy so we can use a different fSubset.
-    AndroidOptions subsetOptions = options;
     {
         // Although startScanlineDecode expects the bottom and top to match the
         // SkImageInfo, startIncrementalDecode uses them to determine which rows to
         // decode.
         SkIRect incrementalSubset = SkIRect::MakeXYWH(scaledSubsetX, scaledSubsetY,
                                                       scaledSubsetWidth, scaledSubsetHeight);
-        subsetOptions.fSubset = &incrementalSubset;
+        codecOptions.fSubset = &incrementalSubset;
         const SkCodec::Result startResult = this->codec()->startIncrementalDecode(
-                scaledInfo, pixels, rowBytes, &subsetOptions);
+                scaledInfo, pixels, rowBytes, &codecOptions);
         if (SkCodec::kSuccess == startResult) {
             int rowsDecoded = 0;
             const SkCodec::Result incResult = this->codec()->incrementalDecode(&rowsDecoded);
@@ -126,17 +128,17 @@ SkCodec::Result SkSampledCodec::onGetAndroidPixels(const SkImageInfo& info, void
             return startResult;
         }
         // Otherwise fall down to use the old scanline decoder.
-        // subsetOptions.fSubset will be reset below, so it will not continue to
+        // codecOptions.fSubset will be reset below, so it will not continue to
         // point to the object that is no longer on the stack.
     }
 
     // Start the scanline decode.
     SkIRect scanlineSubset = SkIRect::MakeXYWH(scaledSubsetX, 0, scaledSubsetWidth,
             scaledSize.height());
-    subsetOptions.fSubset = &scanlineSubset;
+    codecOptions.fSubset = &scanlineSubset;
 
     SkCodec::Result result = this->codec()->startScanlineDecode(scaledInfo,
-            &subsetOptions);
+            &codecOptions);
     if (SkCodec::kSuccess != result) {
         return result;
     }
@@ -165,6 +167,10 @@ SkCodec::Result SkSampledCodec::sampledDecode(const SkImageInfo& info, void* pix
     // We should only call this function when sampling.
     SkASSERT(options.fSampleSize > 1);
 
+    // Create options struct for the codec.
+    SkCodec::Options sampledOptions;
+    sampledOptions.fZeroInitialized = options.fZeroInitialized;
+
     // FIXME: This was already called by onGetAndroidPixels. Can we reduce that?
     int sampleSize = options.fSampleSize;
     int nativeSampleSize;
@@ -192,6 +198,7 @@ SkCodec::Result SkSampledCodec::sampledDecode(const SkImageInfo& info, void* pix
 
         // The scanline decoder only needs to be aware of subsetting in the x-dimension.
         subset.setXYWH(subsetX, 0, subsetWidth, nativeSize.height());
+        sampledOptions.fSubset = ⊂
     }
 
     // Since we guarantee that output dimensions are always at least one (even if the sampleSize
@@ -210,13 +217,13 @@ SkCodec::Result SkSampledCodec::sampledDecode(const SkImageInfo& info, void* pix
         // Although startScanlineDecode expects the bottom and top to match the
         // SkImageInfo, startIncrementalDecode uses them to determine which rows to
         // decode.
-        AndroidOptions incrementalOptions = options;
+        SkCodec::Options incrementalOptions = sampledOptions;
         SkIRect incrementalSubset;
-        if (options.fSubset) {
-            incrementalSubset.fTop     = subsetY;
-            incrementalSubset.fBottom  = subsetY + subsetHeight;
-            incrementalSubset.fLeft    = subset.fLeft;
-            incrementalSubset.fRight   = subset.fRight;
+        if (sampledOptions.fSubset) {
+            incrementalSubset.fTop = subsetY;
+            incrementalSubset.fBottom = subsetY + subsetHeight;
+            incrementalSubset.fLeft = sampledOptions.fSubset->fLeft;
+            incrementalSubset.fRight = sampledOptions.fSubset->fRight;
             incrementalOptions.fSubset = &incrementalSubset;
         }
         const SkCodec::Result startResult = this->codec()->startIncrementalDecode(nativeInfo,
@@ -256,10 +263,6 @@ SkCodec::Result SkSampledCodec::sampledDecode(const SkImageInfo& info, void* pix
     }
 
     // Start the scanline decode.
-    AndroidOptions sampledOptions = options;
-    if (options.fSubset) {
-        sampledOptions.fSubset = ⊂
-    }
     SkCodec::Result result = this->codec()->startScanlineDecode(nativeInfo,
             &sampledOptions);
     if (SkCodec::kIncompleteInput == result || SkCodec::kErrorInInput == result) {

tests/AndroidCodecTest.cpp

@@ -300,61 +300,3 @@ DEF_TEST(AndroidCodec_sampledOrientation, r) {
         ERRORF(r, "got result \"%s\"\n", SkCodec::ResultToString(result));
     }
 }
-
-DEF_TEST(AndroidCodec_animatedOrientation, r) {
-    if (GetResourcePath().isEmpty()) {
-        return;
-    }
-
-    static const struct {
-        const char* path;
-        SkEncodedOrigin origin;
-    } gRec[] = {
-        { "images/stoplight.webp",   kDefault_SkEncodedOrigin },
-        { "images/stoplight_h.webp", kLeftBottom_SkEncodedOrigin } // Rotated 90 CCW
-    };
-    for (auto rec : gRec) {
-        auto data = GetResourceAsData(rec.path);
-        if (!data) {
-            ERRORF(r, "Failed to get resource %s", rec.path);
-            return;
-        }
-
-        // SkAndroidCodec now allows decoding frames beyond the first, but combining this with
-        // kRespect-ing a non-kDefault_SkEncodedOrigin is not supported. If a frame depends on
-        // a prior frame, we allow a client to provide that prior frame. But in order to respect
-        // the origin, we would need to transform the prior frame back to the original orientation
-        // in order to blend (and potentially erase, for a kRestoreBG frame) just to transform it
-        // back. Instead, force the client to handle the orientation after the fact.
-        for (auto behavior : { SkAndroidCodec::ExifOrientationBehavior::kRespect,
-                               SkAndroidCodec::ExifOrientationBehavior::kIgnore   }) {
-            auto androidCodec = SkAndroidCodec::MakeFromCodec(SkCodec::MakeFromData(data),
-                                                              behavior);
-            REPORTER_ASSERT(r, androidCodec->codec()->getOrigin() == rec.origin);
-
-            auto info = androidCodec->getInfo();
-            SkBitmap bm;
-            bm.allocPixels(info);
-            auto result = androidCodec->getAndroidPixels(info, bm.getPixels(), bm.rowBytes());
-            REPORTER_ASSERT(r, result == SkCodec::kSuccess);
-
-            SkAndroidCodec::AndroidOptions options;
-            options.fFrameIndex = 1;
-            options.fPriorFrame = 0;
-            result = androidCodec->getAndroidPixels(info, bm.getPixels(), bm.rowBytes(), &options);
-            switch (behavior) {
-                case SkAndroidCodec::ExifOrientationBehavior::kRespect:
-                    if (rec.origin != kDefault_SkEncodedOrigin) {
-                        REPORTER_ASSERT(r, result == SkCodec::kInvalidParameters,
-                                "Should not be able to decode frame 1 with exif orientation"
-                                " directly! Result: %s", SkCodec::ResultToString(result));
-                        break;
-                    }
-                    [[fallthrough]];
-                case SkAndroidCodec::ExifOrientationBehavior::kIgnore:
-                    REPORTER_ASSERT(r, result == SkCodec::kSuccess);
-                    break;
-            }
-        }
-    }
-}

tests/CodecAnimTest.cpp

@@ -389,15 +389,16 @@ DEF_TEST(Codec_frames, r) {
     }
 }
 
-// Verify that an image can be animated scaled down. These images have a
-// kRestoreBG frame, so they are interesting to test. After decoding that
+// Verify that a webp image can be animated scaled down. This image has a
+// kRestoreBG frame, so it is an interesting image to test. After decoding that
 // frame, we have to erase its rectangle. The rectangle has to be adjusted
 // based on the scaled size.
-static void test_animated_AndroidCodec(skiatest::Reporter* r, const char* file) {
+DEF_TEST(AndroidCodec_animated, r) {
     if (GetResourcePath().isEmpty()) {
         return;
     }
 
+    const char* file = "images/required.webp";
     sk_sp<SkData> data(GetResourceAsData(file));
     if (!data) {
         ERRORF(r, "Missing %s", file);
@@ -455,14 +456,6 @@ static void test_animated_AndroidCodec(skiatest::Reporter* r, const char* file)
     }
 }
 
-DEF_TEST(AndroidCodec_animated, r) {
-    test_animated_AndroidCodec(r, "images/required.webp");
-}
-
-DEF_TEST(AndroidCodec_animated_gif, r) {
-    test_animated_AndroidCodec(r, "images/required.gif");
-}
-
 DEF_TEST(EncodedOriginToMatrixTest, r) {
     // SkAnimCodecPlayer relies on the fact that these matrices are invertible.
     for (auto origin : { kTopLeft_SkEncodedOrigin     ,

tests/CodecTest.cpp

@@ -610,10 +610,7 @@ static void test_dimensions(skiatest::Reporter* r, const char path[]) {
         options.fSampleSize = sampleSize;
         SkCodec::Result result =
                 codec->getAndroidPixels(scaledInfo, pixels.get(), rowBytes, &options);
-        if (result != SkCodec::kSuccess) {
-            ERRORF(r, "Failed to decode %s with sample size %i; error: %s", path, sampleSize,
-                    SkCodec::ResultToString(result));
-        }
+        REPORTER_ASSERT(r, SkCodec::kSuccess == result);
     }
 }