Add support for multiple frames in SkCodec

dm/DMSrcSink.cpp

Index: dm/DMSrcSink.cpp
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index 585a51d7fdf28df03783002819f7f3bce6998e5e..73702b56d4b03656422449d69906b914a421e1a4 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -410,7 +410,8 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
 
     const int bpp = SkColorTypeBytesPerPixel(decodeInfo.colorType());
     const size_t rowBytes = size.width() * bpp;
-    SkAutoMalloc pixels(decodeInfo.getSafeSize(rowBytes));
+    const size_t safeSize = decodeInfo.getSafeSize(rowBytes);
+    SkAutoMalloc pixels(safeSize);
     SkPMColor colorPtr[256];
     int colorCount = 256;
 
@@ -427,6 +428,58 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
     }
 
     switch (fMode) {
+        case kAnimated_Mode: {
+            std::vector<SkCodec::FrameInfo> frameInfos = codec->getFrameInfo();
+            if (frameInfos.size() <= 1) {
+                return SkStringPrintf("%s is not an animated image.", fPath.c_str());
+            }
+
+            SkAutoCanvasRestore acr(canvas, true);
+            // Used to cache a frame that future frames will depend on.
+            SkAutoMalloc priorFramePixels;
+            size_t cachedFrame = SkCodec::kIndependentFrame;
+            SkCodec::MultiFrameOptions multiOptions;
+            options.fFrameOptions = &multiOptions;
+            for (size_t i = 0; i < frameInfos.size(); i++) {
+                multiOptions.fIndex = i;
+                // Check for a prior frame
+                const size_t reqFrame = frameInfos[i].fRequiredFrame;
+                if (reqFrame != SkCodec::kIndependentFrame && reqFrame == cachedFrame

[cblume, 2016/10/14 16:52:35]

I wonder if we can find a better name than kIndependentFrame.

I understand it is an invalid frame index which tells us that there is no
required frame. But it is *this* frame which is independent, not the required
frame. So this ends up reading in a bit of a strange way.

This might sound silly but I think it would be easier to read as:
if (reqFrame != SkCodec::kInvalidFrameIndex

We could even provide:
bool isIndependentFrame(size_t index) {
    const size_t reqFrame = frameInfos[i].fRequiredFrame;
    return reqFrame == SkCodec::kInvalidFrameIndex;
}

[scroggo_chromium, 2016/10/14 19:55:59]

I understand your confusion, and I'm not opposed to the name kInvalidFrameIndex
(maybe something simpler like kNone? Blink uses kNotFound, which doesn't quite
sound right to me). I've added a FIXME for now, until we agree on a name. reed@?

[cblume, 2016/10/18 21:57:32]

I agree that I'm not a fan of kNotFound. It wasn't "not found". It doesn't
exist.

I'm cool with kNone. That is short and sweet and clear.
if (reqFrame != SkCodec::kNone
easy

[scroggo_chromium, 2016/10/20 17:57:12]

Done.

+                        && priorFramePixels.get()) {
+                    // Copy into pixels
+                    memcpy(pixels.get(), priorFramePixels.get(), safeSize);
+                    multiOptions.fHasPriorFrame = true;
+                } else {
+                    multiOptions.fHasPriorFrame = false;
+                }
+                const SkCodec::Result result = codec->getPixels(decodeInfo, pixels.get(),
+                                                                rowBytes, &options,
+                                                                colorPtr, &colorCount);
+                switch (result) {
+                    case SkCodec::kSuccess:
+                    case SkCodec::kIncompleteInput:
+                        draw_to_canvas(canvas, bitmapInfo, pixels.get(), rowBytes,
+                                       colorPtr, colorCount, fDstColorType);
+                        if (result == SkCodec::kIncompleteInput) {
+                            return "";
+                        }
+                        break;
+                    default:
+                        return SkStringPrintf("Couldn't getPixels for frame %i in %s.",
+                                              i, fPath.c_str());
+                }
+
+                // If a future frame depends on this one, store it in priorFrame.
+                // (Note that if i+1 does *not* depend on i, then no future frame can.)
+                if (i+1 < frameInfos.size() && frameInfos[i+1].fRequiredFrame == i) {
+                    memcpy(priorFramePixels.reset(safeSize), pixels.get(), safeSize);
+                    cachedFrame = i;
+                }

[cblume, 2016/10/14 16:52:35]

I like this way of dealing with DisposeOverwritePrevious.

+
+                canvas->translate(SkIntToScalar(0), SkIntToScalar(decodeInfo.height()));
+            }
+            break;
+        }
         case kCodecZeroInit_Mode:
         case kCodec_Mode: {
             switch (codec->getPixels(decodeInfo, pixels.get(), rowBytes, &options,
@@ -448,10 +501,12 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
         case kScanline_Mode: {
             void* dst = pixels.get();
             uint32_t height = decodeInfo.height();
-            const bool png = fPath.endsWith("png");
+            const bool useIncremental = fPath.endsWith("png") || fPath.endsWith("gif") || fPath.endsWith("GIF");

[msarett, 2016/10/14 14:58:50]

nit: 100 chars
nit: Should we defensively add PNG here as well?

[cblume, 2016/10/14 16:52:35]

or just toLower

[scroggo_chromium, 2016/10/14 19:55:59]

done
done

+            // ico may use the old scanline method or the new one, depending on whether it
+            // internally holds a bmp or a png.
             const bool ico = fPath.endsWith("ico");
-            bool useOldScanlineMethod = !png && !ico;
-            if (png || ico) {
+            bool useOldScanlineMethod = !useIncremental && !ico;
+            if (useIncremental || ico) {
                 if (SkCodec::kSuccess == codec->startIncrementalDecode(decodeInfo, dst,
                         rowBytes, nullptr, colorPtr, &colorCount)) {
                     int rowsDecoded;
@@ -461,8 +516,8 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
                                                    rowsDecoded);
                     }
                 } else {
-                    if (png) {
-                        // Error: PNG should support incremental decode.
+                    if (useIncremental) {
+                        // Error: These should support incremental decode.
                         return "Could not start incremental decode";
                     }
                     // Otherwise, this is an ICO. Since incremental failed, it must contain a BMP,
@@ -484,17 +539,6 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
                         // image, memory will be filled with a default value.
                         codec->getScanlines(dst, height, rowBytes);
                         break;
-                    case SkCodec::kOutOfOrder_SkScanlineOrder: {
-                        for (int y = 0; y < decodeInfo.height(); y++) {
-                            int dstY = codec->outputScanline(y);
-                            void* dstPtr = SkTAddOffset<void>(dst, rowBytes * dstY);
-                            // We complete the loop, even if this call begins to fail
-                            // due to an incomplete image.  This ensures any uninitialized
-                            // memory will be filled with the proper value.
-                            codec->getScanlines(dstPtr, 1, rowBytes);
-                        }
-                        break;
-                    }
                 }
             }
 
@@ -660,7 +704,14 @@ SkISize CodecSrc::size() const {
     if (nullptr == codec) {
         return SkISize::Make(0, 0);
     }
-    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.
+        const size_t count = codec->getFrameInfo().size();
+        imageSize.fHeight = imageSize.fHeight * count;
+    }
+    return imageSize;
 }
 
 Name CodecSrc::name() const {