Make SkCodec more flexible about its required frame

SkCodec sets fRequiredFrame to be the earliest possible frame that a
given frame can depend on. e.g.

- Frame A fills the screen, Keep
- Frame B does not cover A, Keep
- Frame C covers B but not A, and is opaque

Frame C can depend on either A or B. SkCodec already reports that C
depends on A. This CL allows a client of SkCodec to use either A or
B to create C.

Also expose the DisposalMethod. Since any frame between A and C can
be used to create C except for DisposePrevious frames, the client
needs to be able to know the disposal method so they do not try to
use such a frame to create C.

Further, the disposal method can be used to give the client a better
idea whether they will continue to need a frame. (e.g. if frame i is
DisposePrevious and depends on i-1, the client may not want to steal
i-1 to create i, since i+1 may also depend on i-1.)

TODO: Share code for decoding prior frames between GIF and WEBP

Change-Id: I91a5ae22ba3d8dfbe0bde833fa67ae3da0d81ed6
Reviewed-on: https://skia-review.googlesource.com/13722
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Chris Blume <cblume@chromium.org>
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>

1 files changed, 120 insertions, 61 deletions

diff --git a/tests/CodecAnimTest.cpp b/tests/CodecAnimTest.cpp
index 74ee3c63ed..27fe0038b5 100644
--- a/tests/CodecAnimTest.cpp
+++ b/tests/CodecAnimTest.cpp
@@ -57,37 +57,53 @@ DEF_TEST(Codec_565, r) {
 
     SkCodec::Options options;
     options.fFrameIndex = 1;
-    options.fHasPriorFrame = false;
+    options.fPriorFrame = SkCodec::kNone;
 
     const auto result = codec->getPixels(info, bm.getPixels(), bm.rowBytes(),
                                          &options, nullptr, nullptr);
     REPORTER_ASSERT(r, result == SkCodec::kSuccess);
 }
 
+static bool restore_previous(const SkCodec::FrameInfo& info) {
+    return info.fDisposalMethod == SkCodecAnimation::DisposalMethod::kRestorePrevious;
+}
 
 DEF_TEST(Codec_frames, r) {
-    #define kOpaque     kOpaque_SkAlphaType
-    #define kUnpremul   kUnpremul_SkAlphaType
+    #define kOpaque         kOpaque_SkAlphaType
+    #define kUnpremul       kUnpremul_SkAlphaType
+    #define kKeep           SkCodecAnimation::DisposalMethod::kKeep
+    #define kRestoreBG      SkCodecAnimation::DisposalMethod::kRestoreBGColor
+    #define kRestorePrev    SkCodecAnimation::DisposalMethod::kRestorePrevious
     static const struct {
-        const char*              fName;
-        int                      fFrameCount;
+        const char*                                   fName;
+        int                                           fFrameCount;
         // One less than fFramecount, since the first frame is always
         // independent.
-        std::vector<int>         fRequiredFrames;
+        std::vector<int>                              fRequiredFrames;
         // Same, since the first frame should match getInfo.
-        std::vector<SkAlphaType> fAlphaTypes;
+        std::vector<SkAlphaType>                      fAlphaTypes;
         // The size of this one should match fFrameCount for animated, empty
         // otherwise.
-        std::vector<int>         fDurations;
-        int                      fRepetitionCount;
+        std::vector<int>                              fDurations;
+        int                                           fRepetitionCount;
+        std::vector<SkCodecAnimation::DisposalMethod> fDisposalMethods;
     } gRecs[] = {
+        { "required.gif", 7,
+            { 0, 1, 1, SkCodec::kNone, 4, 4 },
+            { kOpaque, kUnpremul, kUnpremul, kOpaque, kOpaque, kOpaque },
+            { 100, 100, 100, 100, 100, 100, 100 },
+            0,
+            { kKeep, kRestoreBG, kKeep, kKeep, kKeep, kRestoreBG, kKeep } },
         { "alphabetAnim.gif", 13,
             { SkCodec::kNone, 0, 0, 0, 0, 5, 6, SkCodec::kNone,
               SkCodec::kNone, SkCodec::kNone, 10, 11 },
             { kUnpremul, kUnpremul, kUnpremul, kUnpremul, kUnpremul, kUnpremul,
               kUnpremul, kUnpremul, kUnpremul, kOpaque, kOpaque, kUnpremul },
             { 100, 100, 100, 100, 100, 100, 100, 100, 100, 100, 100, 100, 100 },
-            0 },
+            0,
+            { kKeep, kRestorePrev, kRestorePrev, kRestorePrev, kRestorePrev,
+              kRestoreBG, kKeep, kRestoreBG, kRestoreBG, kKeep, kKeep,
+              kRestoreBG, kKeep } },
         { "randPixelsAnim2.gif", 4,
             // required frames
             { 0, 0, 1 },
@@ -96,7 +112,8 @@ DEF_TEST(Codec_frames, r) {
             // durations
             { 0, 1000, 170, 40 },
             // repetition count
-            0 },
+            0,
+            { kKeep, kKeep, kRestorePrev, kKeep } },
         { "randPixelsAnim.gif", 13,
             // required frames
             { SkCodec::kNone, 1, 2, 3, 4, 3, 6, 7, 7, 7, 9, 9 },
@@ -105,33 +122,49 @@ DEF_TEST(Codec_frames, r) {
             // durations
             { 0, 1000, 170, 40, 220, 7770, 90, 90, 90, 90, 90, 90, 90 },
             // repetition count
-            0 },
-        { "box.gif", 1, {}, {}, {}, 0 },
-        { "color_wheel.gif", 1, {}, {}, {}, 0 },
+            0,
+            { kKeep, kKeep, kKeep, kKeep, kRestoreBG, kRestoreBG, kRestoreBG,
+              kRestoreBG, kRestorePrev, kRestoreBG, kRestorePrev, kRestorePrev,
+              kRestorePrev,  } },
+        { "box.gif", 1, {}, {}, {}, 0, { kKeep } },
+        { "color_wheel.gif", 1, {}, {}, {}, 0, { kKeep } },
         { "test640x479.gif", 4, { 0, 1, 2 },
                 { kOpaque, kOpaque, kOpaque },
                 { 200, 200, 200, 200 },
-                SkCodec::kRepetitionCountInfinite },
-        { "colorTables.gif", 2, { 0 }, { kOpaque }, { 1000, 1000 }, 5 },
-
-        { "arrow.png",  1, {}, {}, {}, 0 },
-        { "google_chrome.ico", 1, {}, {}, {}, 0 },
-        { "brickwork-texture.jpg", 1, {}, {}, {}, 0 },
+                SkCodec::kRepetitionCountInfinite,
+                { kKeep, kKeep, kKeep, kKeep } },
+        { "colorTables.gif", 2, { 0 }, { kOpaque }, { 1000, 1000 }, 5,
+                { kKeep, kKeep } },
+
+        { "arrow.png",  1, {}, {}, {}, 0, {} },
+        { "google_chrome.ico", 1, {}, {}, {}, 0, {} },
+        { "brickwork-texture.jpg", 1, {}, {}, {}, 0, {} },
 #if defined(SK_CODEC_DECODES_RAW) && (!defined(_WIN32))
-        { "dng_with_preview.dng", 1, {}, {}, {}, 0 },
+        { "dng_with_preview.dng", 1, {}, {}, {}, 0, {} },
 #endif
-        { "mandrill.wbmp", 1, {}, {}, {}, 0 },
-        { "randPixels.bmp", 1, {}, {}, {}, 0 },
-        { "yellow_rose.webp", 1, {}, {}, {}, 0 },
+        { "mandrill.wbmp", 1, {}, {}, {}, 0, {} },
+        { "randPixels.bmp", 1, {}, {}, {}, 0, {} },
+        { "yellow_rose.webp", 1, {}, {}, {}, 0, {} },
         { "webp-animated.webp", 3, { 0, 1 }, { kOpaque, kOpaque },
-            { 1000, 500, 1000 }, SkCodec::kRepetitionCountInfinite },
+            { 1000, 500, 1000 }, SkCodec::kRepetitionCountInfinite,
+            { kKeep, kKeep, kKeep } },
         { "blendBG.webp", 7, { 0, SkCodec::kNone, SkCodec::kNone, SkCodec::kNone,
                                4, 4 },
             { kOpaque, kOpaque, kUnpremul, kOpaque, kUnpremul, kUnpremul },
-            { 525, 500, 525, 437, 609, 729, 444 }, 7 },
+            { 525, 500, 525, 437, 609, 729, 444 }, 7,
+            { kKeep, kKeep, kKeep, kKeep, kKeep, kKeep, kKeep } },
+        { "required.webp", 7,
+            { 0, 1, 1, SkCodec::kNone, 4, 4 },
+            { kOpaque, kUnpremul, kUnpremul, kOpaque, kOpaque, kOpaque },
+            { 100, 100, 100, 100, 100, 100, 100 },
+            1,
+            { kKeep, kRestoreBG, kKeep, kKeep, kKeep, kRestoreBG, kKeep } },
     };
     #undef kOpaque
     #undef kUnpremul
+    #undef kKeep
+    #undef kRestorePrev
+    #undef kRestoreBG
 
     for (const auto& rec : gRecs) {
         sk_sp<SkData> data(GetResourceAsData(rec.fName));
@@ -178,6 +211,13 @@ DEF_TEST(Codec_frames, r) {
                        rec.fName, expected - 1, rec.fAlphaTypes.size());
                 continue;
             }
+
+            if (rec.fDisposalMethods.size() != static_cast<size_t>(expected)) {
+                ERRORF(r, "'%s' has wrong number entries in fDisposalMethods; "
+                       "expected %i\tactual: %i",
+                       rec.fName, expected, rec.fDisposalMethods.size());
+                continue;
+            }
         }
 
         enum class TestMode {
@@ -254,6 +294,8 @@ DEF_TEST(Codec_frames, r) {
                     ERRORF(r, "%s's frame %i has wrong dependency! expected: %i\tactual: %i",
                            rec.fName, i, rec.fRequiredFrames[i-1], frameInfo.fRequiredFrame);
                 }
+
+                REPORTER_ASSERT(r, frameInfo.fDisposalMethod == rec.fDisposalMethods[i]);
             }
 
             if (TestMode::kIndividual == mode) {
@@ -261,67 +303,84 @@ DEF_TEST(Codec_frames, r) {
                 continue;
             }
 
-            // Compare decoding in two ways:
-            // 1. Provide the frame that a frame depends on, so the codec just has to blend.
-            //    (in the array cachedFrames)
-            // 2. Do not provide the frame that a frame depends on, so the codec has to decode
-            //    all the way back to a key-frame. (in a local variable uncachedFrame)
-            // The two should look the same.
+            // Compare decoding in multiple ways:
+            // - Start from scratch for each frame. |codec| will have to decode the required frame
+            //   (and any it depends on) to decode. This is stored in |cachedFrames|.
+            // - Provide the frame that a frame depends on, so |codec| just has to blend.
+            // - Provide a frame after the required frame, which will be covered up by the newest
+            //   frame.
+            // All should look the same.
             std::vector<SkBitmap> cachedFrames(frameCount);
             const auto info = codec->getInfo().makeColorType(kN32_SkColorType);
 
-            auto decode = [&](SkBitmap* bm, bool cached, int index) {
+            auto decode = [&](SkBitmap* bm, int index, int cachedIndex) {
                 auto decodeInfo = info;
                 if (index > 0) {
                     decodeInfo = info.makeAlphaType(frameInfos[index].fAlphaType);
                 }
                 bm->allocPixels(decodeInfo);
-                if (cached) {
+                if (cachedIndex != SkCodec::kNone) {
                     // First copy the pixels from the cached frame
-                    const int requiredFrame = frameInfos[index].fRequiredFrame;
-                    if (requiredFrame != SkCodec::kNone) {
-                        const bool success = sk_tool_utils::copy_to(bm, kN32_SkColorType,
-                                cachedFrames[requiredFrame]);
-                        REPORTER_ASSERT(r, success);
-                    }
+                    const bool success = sk_tool_utils::copy_to(bm, kN32_SkColorType,
+                            cachedFrames[cachedIndex]);
+                    REPORTER_ASSERT(r, success);
                 }
                 SkCodec::Options opts;
                 opts.fFrameIndex = index;
-                opts.fHasPriorFrame = cached;
+                opts.fPriorFrame = cachedIndex;
                 const auto result = codec->getPixels(decodeInfo, bm->getPixels(), bm->rowBytes(),
                                                      &opts, nullptr, nullptr);
+                if (cachedIndex != SkCodec::kNone && restore_previous(frameInfos[cachedIndex])) {
+                    if (result == SkCodec::kInvalidParameters) {
+                        return true;
+                    }
+                    ERRORF(r, "Using a kRestorePrevious frame as fPriorFrame should fail");
+                    return false;
+                }
                 if (result != SkCodec::kSuccess) {
-                    ERRORF(r, "Failed to decode frame %i from %s when %scached, error %i",
-                           index, rec.fName, (cached ? "" : "not "), result);
+                    ERRORF(r, "Failed to decode frame %i from %s when providing prior frame %i, "
+                              "error %i", index, rec.fName, cachedIndex, result);
                 }
                 return result == SkCodec::kSuccess;
             };
 
             for (int i = 0; i < frameCount; i++) {
                 SkBitmap& cachedFrame = cachedFrames[i];
-                if (!decode(&cachedFrame, true, i)) {
+                if (!decode(&cachedFrame, i, SkCodec::kNone)) {
                     continue;
                 }
-                SkBitmap uncachedFrame;
-                if (!decode(&uncachedFrame, false, i)) {
+                const auto reqFrame = frameInfos[i].fRequiredFrame;
+                if (reqFrame == SkCodec::kNone) {
+                    // Nothing to compare against.
                     continue;
                 }
+                for (int j = reqFrame; j < i; j++) {
+                    SkBitmap frame;
+                    if (restore_previous(frameInfos[j])) {
+                        (void) decode(&frame, i, j);
+                        continue;
+                    }
+                    if (!decode(&frame, i, j)) {
+                        continue;
+                    }
 
-                // Now verify they're equal.
-                const size_t rowLen = info.bytesPerPixel() * info.width();
-                for (int y = 0; y < info.height(); y++) {
-                    const void* cachedAddr = cachedFrame.getAddr(0, y);
-                    SkASSERT(cachedAddr != nullptr);
-                    const void* uncachedAddr = uncachedFrame.getAddr(0, y);
-                    SkASSERT(uncachedAddr != nullptr);
-                    const bool lineMatches = memcmp(cachedAddr, uncachedAddr, rowLen) == 0;
-                    if (!lineMatches) {
-                        SkString name = SkStringPrintf("cached_%i", i);
-                        write_bm(name.c_str(), cachedFrame);
-                        name = SkStringPrintf("uncached_%i", i);
-                        write_bm(name.c_str(), uncachedFrame);
-                        ERRORF(r, "%s's frame %i is different depending on caching!", rec.fName, i);
-                        break;
+                    // Now verify they're equal.
+                    const size_t rowLen = info.bytesPerPixel() * info.width();
+                    for (int y = 0; y < info.height(); y++) {
+                        const void* cachedAddr = cachedFrame.getAddr(0, y);
+                        SkASSERT(cachedAddr != nullptr);
+                        const void* addr = frame.getAddr(0, y);
+                        SkASSERT(addr != nullptr);
+                        const bool lineMatches = memcmp(cachedAddr, addr, rowLen) == 0;
+                        if (!lineMatches) {
+                            SkString name = SkStringPrintf("cached_%i", i);
+                            write_bm(name.c_str(), cachedFrame);
+                            name = SkStringPrintf("frame_%i", i);
+                            write_bm(name.c_str(), frame);
+                            ERRORF(r, "%s's frame %i is different (starting from line %i) when "
+                                      "providing prior frame %i!", rec.fName, i, y, j);
+                            break;
+                        }
                     }
                 }
             }