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>
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index 983a0c1..4ac7e80 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -504,9 +504,9 @@
&& priorFramePixels.get()) {
// Copy into pixels
memcpy(pixels.get(), priorFramePixels.get(), safeSize);
- options.fHasPriorFrame = true;
+ options.fPriorFrame = reqFrame;
} else {
- options.fHasPriorFrame = false;
+ options.fPriorFrame = SkCodec::kNone;
}
SkCodec::Result result = codec->getPixels(decodeInfo, pixels.get(),
rowBytes, &options,
diff --git a/gm/animatedGif.cpp b/gm/animatedGif.cpp
index 5f4fe71..80bcdd9 100644
--- a/gm/animatedGif.cpp
+++ b/gm/animatedGif.cpp
@@ -53,7 +53,6 @@
SkCodec::Options opts;
opts.fFrameIndex = frameIndex;
- opts.fHasPriorFrame = false;
const int requiredFrame = fFrameInfos[frameIndex].fRequiredFrame;
if (requiredFrame != SkCodec::kNone) {
SkASSERT(requiredFrame >= 0
@@ -62,7 +61,7 @@
// For simplicity, do not try to cache old frames
if (requiredBitmap.getPixels() &&
sk_tool_utils::copy_to(&bm, requiredBitmap.colorType(), requiredBitmap)) {
- opts.fHasPriorFrame = true;
+ opts.fPriorFrame = requiredFrame;
}
}
diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h
index d498007..640ff46 100644
--- a/include/codec/SkCodec.h
+++ b/include/codec/SkCodec.h
@@ -9,6 +9,7 @@
#define SkCodec_DEFINED
#include "../private/SkTemplates.h"
+#include "SkCodecAnimation.h"
#include "SkColor.h"
#include "SkColorSpaceXform.h"
#include "SkEncodedImageFormat.h"
@@ -249,7 +250,7 @@
: fZeroInitialized(kNo_ZeroInitialized)
, fSubset(nullptr)
, fFrameIndex(0)
- , fHasPriorFrame(false)
+ , fPriorFrame(kNone)
, fPremulBehavior(SkTransferFunctionBehavior::kRespect)
{}
@@ -281,23 +282,19 @@
int fFrameIndex;
/**
- * If true, the dst already contains the prior frame.
+ * If not kNone, the dst already contains the prior frame at this index.
*
* Only meaningful for multi-frame images.
*
* If fFrameIndex needs to be blended with a prior frame (as reported by
* getFrameInfo[fFrameIndex].fRequiredFrame), the client can set this to
- * either true or false:
+ * any non-kRestorePrevious frame in [fRequiredFrame, fFrameIndex) to
+ * indicate that that frame is already in the dst. Options.fZeroInitialized
+ * is ignored in this case.
*
- * true means that the prior frame is already in the dst, and this
- * codec only needs to decode fFrameIndex and blend it with the dst.
- * Options.fZeroInitialized is ignored in this case.
- *
- * false means that the dst does not contain the prior frame, so this
- * codec needs to first decode the prior frame (which in turn may need
- * to decode its prior frame).
+ * If set to kNone, the codec will decode any necessary required frame(s) first.
*/
- bool fHasPriorFrame;
+ int fPriorFrame;
/**
* Indicates whether we should do a linear premultiply or a legacy premultiply.
@@ -613,7 +610,11 @@
struct FrameInfo {
/**
* The frame that this frame needs to be blended with, or
- * kNone.
+ * kNone if this frame is independent.
+ *
+ * Note that this is the *earliest* frame that can be used
+ * for blending. Any frame from [fRequiredFrame, i) can be
+ * used, unless its fDisposalMethod is kRestorePrevious.
*/
int fRequiredFrame;
@@ -635,6 +636,11 @@
* color index-based frame has a color with alpha but does not use it.
*/
SkAlphaType fAlphaType;
+
+ /**
+ * How this frame should be modified before decoding the next one.
+ */
+ SkCodecAnimation::DisposalMethod fDisposalMethod;
};
/**
diff --git a/include/codec/SkCodecAnimation.h b/include/codec/SkCodecAnimation.h
new file mode 100644
index 0000000..9a4daff
--- /dev/null
+++ b/include/codec/SkCodecAnimation.h
@@ -0,0 +1,43 @@
+/*
+ * Copyright 2016 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkCodecAnimation_DEFINED
+#define SkCodecAnimation_DEFINED
+
+namespace SkCodecAnimation {
+ /**
+ * This specifies how the next frame is based on this frame.
+ *
+ * Names are based on the GIF 89a spec.
+ *
+ * The numbers correspond to values in a GIF.
+ */
+ enum class DisposalMethod {
+ /**
+ * The next frame should be drawn on top of this one.
+ *
+ * In a GIF, a value of 0 (not specified) is also treated as Keep.
+ */
+ kKeep = 1,
+
+ /**
+ * Similar to Keep, except the area inside this frame's rectangle
+ * should be cleared to the BackGround color (transparent) before
+ * drawing the next frame.
+ */
+ kRestoreBGColor = 2,
+
+ /**
+ * The next frame should be drawn on top of the previous frame - i.e.
+ * disregarding this one.
+ *
+ * In a GIF, a value of 4 is also treated as RestorePrevious.
+ */
+ kRestorePrevious = 3,
+ };
+};
+#endif // SkCodecAnimation_DEFINED
diff --git a/resources/required.gif b/resources/required.gif
new file mode 100644
index 0000000..91a9fd1
--- /dev/null
+++ b/resources/required.gif
Binary files differ
diff --git a/resources/required.webp b/resources/required.webp
new file mode 100644
index 0000000..9f9a8f8
--- /dev/null
+++ b/resources/required.webp
Binary files differ
diff --git a/src/codec/SkCodecAnimation.h b/src/codec/SkCodecAnimation.h
deleted file mode 100644
index 46a878a..0000000
--- a/src/codec/SkCodecAnimation.h
+++ /dev/null
@@ -1,69 +0,0 @@
-/*
- * Copyright 2016 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#ifndef SkCodecAnimation_DEFINED
-#define SkCodecAnimation_DEFINED
-
-#include "SkCodec.h"
-#include "SkRect.h"
-
-class SkCodecAnimation {
-public:
- /**
- * This specifies how the next frame is based on this frame.
- *
- * Names are based on the GIF 89a spec.
- *
- * The numbers correspond to values in a GIF.
- */
- enum DisposalMethod {
- /**
- * The next frame should be drawn on top of this one.
- *
- * In a GIF, a value of 0 (not specified) is also treated as Keep.
- */
- Keep_DisposalMethod = 1,
-
- /**
- * Similar to Keep, except the area inside this frame's rectangle
- * should be cleared to the BackGround color (transparent) before
- * drawing the next frame.
- */
- RestoreBGColor_DisposalMethod = 2,
-
- /**
- * The next frame should be drawn on top of the previous frame - i.e.
- * disregarding this one.
- *
- * In a GIF, a value of 4 is also treated as RestorePrevious.
- */
- RestorePrevious_DisposalMethod = 3,
- };
-
- /**
- * How to blend the current frame.
- */
- enum class Blend {
- /**
- * Blend with the prior frame. This is the typical case, supported
- * by all animated image types.
- */
- kPriorFrame,
-
- /**
- * Do not blend.
- *
- * This frames pixels overwrite previous pixels "blending" with
- * the background color of transparent.
- */
- kBG,
- };
-
-private:
- SkCodecAnimation();
-};
-#endif // SkCodecAnimation_DEFINED
diff --git a/src/codec/SkCodecAnimationPriv.h b/src/codec/SkCodecAnimationPriv.h
new file mode 100644
index 0000000..233a79b
--- /dev/null
+++ b/src/codec/SkCodecAnimationPriv.h
@@ -0,0 +1,32 @@
+/*
+ * Copyright 2016 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkCodecAnimationPriv_DEFINED
+#define SkCodecAnimationPriv_DEFINED
+
+namespace SkCodecAnimation {
+ /**
+ * How to blend the current frame.
+ */
+ enum class Blend {
+ /**
+ * Blend with the prior frame. This is the typical case, supported
+ * by all animated image types.
+ */
+ kPriorFrame,
+
+ /**
+ * Do not blend.
+ *
+ * This frames pixels overwrite previous pixels "blending" with
+ * the background color of transparent.
+ */
+ kBG,
+ };
+
+}
+#endif // SkCodecAnimationPriv_DEFINED
diff --git a/src/codec/SkFrameHolder.h b/src/codec/SkFrameHolder.h
index e92b40f..0f30b65 100644
--- a/src/codec/SkFrameHolder.h
+++ b/src/codec/SkFrameHolder.h
@@ -10,6 +10,7 @@
#include "SkTypes.h"
#include "SkCodecAnimation.h"
+#include "SkCodecAnimationPriv.h"
/**
* Base class for a single frame of an animated image.
@@ -23,7 +24,7 @@
: fId(id)
, fHasAlpha(false)
, fRequiredFrame(kUninitialized)
- , fDisposalMethod(SkCodecAnimation::Keep_DisposalMethod)
+ , fDisposalMethod(SkCodecAnimation::DisposalMethod::kKeep)
, fDuration(0)
, fBlend(SkCodecAnimation::Blend::kPriorFrame)
{
diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp
index f0495bc..6c42734 100644
--- a/src/codec/SkGifCodec.cpp
+++ b/src/codec/SkGifCodec.cpp
@@ -153,6 +153,7 @@
frameInfo->fFullyReceived = frameContext->isComplete();
frameInfo->fAlphaType = frameContext->hasAlpha() ? kUnpremul_SkAlphaType
: kOpaque_SkAlphaType;
+ frameInfo->fDisposalMethod = frameContext->getDisposalMethod();
}
return true;
}
@@ -398,11 +399,24 @@
}
} else {
// Not independent
- if (!opts.fHasPriorFrame) {
+ // FIXME: Share this code with WEBP
+ const int reqFrame = frameContext->getRequiredFrame();
+ if (opts.fPriorFrame != kNone) {
+ if (opts.fPriorFrame < reqFrame || opts.fPriorFrame >= frameIndex) {
+ // Alternatively, we could correct this to kNone.
+ return kInvalidParameters;
+ }
+ const auto* prevFrame = fReader->frameContext(opts.fPriorFrame);
+ if (prevFrame->getDisposalMethod()
+ == SkCodecAnimation::DisposalMethod::kRestorePrevious) {
+ // Similarly, this could be corrected to kNone.
+ return kInvalidParameters;
+ }
+ } else {
// Decode that frame into pixels.
Options prevFrameOpts(opts);
prevFrameOpts.fFrameIndex = frameContext->getRequiredFrame();
- prevFrameOpts.fHasPriorFrame = false;
+ prevFrameOpts.fPriorFrame = kNone;
// The prior frame may have a different color table, so update it and the
// swizzler.
this->initializeColorTable(dstInfo, prevFrameOpts.fFrameIndex);
@@ -424,24 +438,33 @@
this->initializeColorTable(dstInfo, frameIndex);
this->initializeSwizzler(dstInfo, frameIndex);
}
- const auto* prevFrame = fReader->frameContext(frameContext->getRequiredFrame());
- if (prevFrame->getDisposalMethod() == SkCodecAnimation::RestoreBGColor_DisposalMethod) {
- SkIRect prevRect = prevFrame->frameRect();
- if (prevRect.intersect(this->getInfo().bounds())) {
- // Do the divide ourselves for left and top, since we do not want
- // get_scaled_dimension to upgrade 0 to 1. (This is similar to SkSampledCodec's
- // sampling of the subset.)
- auto left = prevRect.fLeft / fSwizzler->sampleX();
- auto top = prevRect.fTop / fSwizzler->sampleY();
- void* const eraseDst = SkTAddOffset<void>(fDst, top * fDstRowBytes
- + left * SkColorTypeBytesPerPixel(dstInfo.colorType()));
- auto width = get_scaled_dimension(prevRect.width(), fSwizzler->sampleX());
- auto height = get_scaled_dimension(prevRect.height(), fSwizzler->sampleY());
- // fSwizzler->fill() would fill to the scaled width of the frame, but we want to
- // fill to the scaled with of the width of the PRIOR frame, so we do all the
- // scaling ourselves and call the static version.
- SkSampler::Fill(dstInfo.makeWH(width, height), eraseDst,
- fDstRowBytes, this->getFillValue(dstInfo), kNo_ZeroInitialized);
+
+ // If the required frame is RestoreBG, we need to erase it. If a frame after the
+ // required frame is provided, there is no need to erase, since it will be covered
+ // anyway.
+ if (opts.fPriorFrame == reqFrame || opts.fPriorFrame == kNone) {
+ const auto* prevFrame = fReader->frameContext(reqFrame);
+ if (prevFrame->getDisposalMethod()
+ == SkCodecAnimation::DisposalMethod::kRestoreBGColor) {
+ SkIRect prevRect = prevFrame->frameRect();
+ if (prevRect.intersect(this->getInfo().bounds())) {
+ // Do the divide ourselves for left and top, since we do not want
+ // get_scaled_dimension to upgrade 0 to 1. (This is similar to
+ // SkSampledCodec's sampling of the subset.)
+ const auto sampleX = fSwizzler->sampleX();
+ const auto sampleY = fSwizzler->sampleY();
+ auto left = prevRect.fLeft / sampleX;
+ auto top = prevRect.fTop / sampleY;
+ void* const eraseDst = SkTAddOffset<void>(fDst, top * fDstRowBytes
+ + left * SkColorTypeBytesPerPixel(dstInfo.colorType()));
+ auto width = get_scaled_dimension(prevRect.width(), sampleX);
+ auto height = get_scaled_dimension(prevRect.height(), sampleY);
+ // fSwizzler->fill() would fill to the scaled width of the frame, but we
+ // want to fill to the scaled with of the width of the PRIOR frame, so we
+ // do all the scaling ourselves and call the static version.
+ SkSampler::Fill(dstInfo.makeWH(width, height), eraseDst, fDstRowBytes,
+ this->getFillValue(dstInfo), kNo_ZeroInitialized);
+ }
}
}
filledBackground = true;
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index 21d45da..f702a3a 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -7,6 +7,8 @@
#include "SkBitmap.h"
#include "SkCanvas.h"
+#include "SkCodecAnimation.h"
+#include "SkCodecAnimationPriv.h"
#include "SkCodecPriv.h"
#include "SkColorSpaceXform.h"
#include "SkRasterPipeline.h"
@@ -253,8 +255,8 @@
Frame* frame = fFrameHolder.appendNewFrame(iter.has_alpha);
frame->setXYWH(iter.x_offset, iter.y_offset, iter.width, iter.height);
frame->setDisposalMethod(iter.dispose_method == WEBP_MUX_DISPOSE_BACKGROUND ?
- SkCodecAnimation::RestoreBGColor_DisposalMethod :
- SkCodecAnimation::Keep_DisposalMethod);
+ SkCodecAnimation::DisposalMethod::kRestoreBGColor :
+ SkCodecAnimation::DisposalMethod::kKeep);
frame->setDuration(iter.duration);
if (WEBP_MUX_BLEND != iter.blend_method) {
frame->setBlend(SkCodecAnimation::Blend::kBG);
@@ -292,6 +294,7 @@
// animated image.
frameInfo->fFullyReceived = true;
frameInfo->fAlphaType = alpha_type(frame->hasAlpha());
+ frameInfo->fDisposalMethod = frame->getDisposalMethod();
}
return true;
@@ -437,9 +440,15 @@
SkSampler::Fill(dstInfo, dst, rowBytes, 0, options.fZeroInitialized);
}
} else {
- if (!options.fHasPriorFrame) {
+ // FIXME: Share with GIF
+ if (options.fPriorFrame != kNone) {
+ if (options.fPriorFrame < requiredFrame || options.fPriorFrame >= index) {
+ return kInvalidParameters;
+ }
+ } else {
Options prevFrameOpts(options);
prevFrameOpts.fFrameIndex = requiredFrame;
+ prevFrameOpts.fPriorFrame = kNone;
const auto result = this->getPixels(dstInfo, dst, rowBytes, &prevFrameOpts,
nullptr, nullptr);
switch (result) {
@@ -452,16 +461,20 @@
}
}
- // Dispose bg color
- const Frame* priorFrame = fFrameHolder.frame(requiredFrame);
- if (priorFrame->getDisposalMethod() == SkCodecAnimation::RestoreBGColor_DisposalMethod) {
- // FIXME: If we add support for scaling/subsets, this rectangle needs to be adjusted.
- const auto priorRect = priorFrame->frameRect();
- const auto info = dstInfo.makeWH(priorRect.width(), priorRect.height());
- const size_t bpp = SkColorTypeBytesPerPixel(dstInfo.colorType());
- const size_t offset = priorRect.x() * bpp + priorRect.y() * rowBytes;
- auto* eraseDst = SkTAddOffset<void>(dst, offset);
- SkSampler::Fill(info, eraseDst, rowBytes, 0, kNo_ZeroInitialized);
+ if (options.fPriorFrame == requiredFrame || options.fPriorFrame == kNone) {
+ // Dispose bg color
+ const Frame* priorFrame = fFrameHolder.frame(requiredFrame);
+ if (priorFrame->getDisposalMethod()
+ == SkCodecAnimation::DisposalMethod::kRestoreBGColor) {
+ // FIXME: If we add support for scaling/subsets, this rectangle needs to be
+ // adjusted.
+ const auto priorRect = priorFrame->frameRect();
+ const auto info = dstInfo.makeWH(priorRect.width(), priorRect.height());
+ const size_t bpp = SkColorTypeBytesPerPixel(dstInfo.colorType());
+ const size_t offset = priorRect.x() * bpp + priorRect.y() * rowBytes;
+ auto* eraseDst = SkTAddOffset<void>(dst, offset);
+ SkSampler::Fill(info, eraseDst, rowBytes, 0, kNo_ZeroInitialized);
+ }
}
}
diff --git a/tests/CodecAnimTest.cpp b/tests/CodecAnimTest.cpp
index 74ee3c6..27fe003 100644
--- a/tests/CodecAnimTest.cpp
+++ b/tests/CodecAnimTest.cpp
@@ -57,37 +57,53 @@
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 @@
// 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 @@
// 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 },
+ 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 },
+ { "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 @@
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 @@
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 @@
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;
+ }
}
}
}
diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp
index ffaeb2b..f839aaa 100644
--- a/tests/CodecTest.cpp
+++ b/tests/CodecTest.cpp
@@ -1514,7 +1514,8 @@
SkCodec::Options opts;
for (int i = 0; static_cast<size_t>(i) < frameInfos.size(); i++) {
opts.fFrameIndex = i;
- opts.fHasPriorFrame = frameInfos[i].fRequiredFrame == i - 1;
+ const auto reqFrame = frameInfos[i].fRequiredFrame;
+ opts.fPriorFrame = reqFrame == i - 1 ? reqFrame : SkCodec::kNone;
auto result = codec->startIncrementalDecode(info, bm.getPixels(), bm.rowBytes(), &opts);
if (result != SkCodec::kSuccess) {
ERRORF(r, "Failed to start decoding frame %i (out of %i) with error %i\n", i,
diff --git a/third_party/gif/SkGifImageReader.cpp b/third_party/gif/SkGifImageReader.cpp
index 0666fed..76f3edc 100644
--- a/third_party/gif/SkGifImageReader.cpp
+++ b/third_party/gif/SkGifImageReader.cpp
@@ -616,11 +616,11 @@
case 4:
// Some specs say that disposal method 3 is "overwrite previous", others that setting
// the third bit of the field (i.e. method 4) is. We map both to the same value.
- currentFrame->setDisposalMethod(SkCodecAnimation::RestorePrevious_DisposalMethod);
+ currentFrame->setDisposalMethod(SkCodecAnimation::DisposalMethod::kRestorePrevious);
break;
default:
// Other values use the default.
- currentFrame->setDisposalMethod(SkCodecAnimation::Keep_DisposalMethod);
+ currentFrame->setDisposalMethod(SkCodecAnimation::DisposalMethod::kKeep);
break;
}
currentFrame->setDuration(GETINT16(currentComponent + 1) * 10);
@@ -903,7 +903,7 @@
}
static bool restore_bg(const SkFrame& frame) {
- return frame.getDisposalMethod() == SkCodecAnimation::RestoreBGColor_DisposalMethod;
+ return frame.getDisposalMethod() == SkCodecAnimation::DisposalMethod::kRestoreBGColor;
}
bool SkGIFFrameContext::onReportsAlpha() const {
@@ -935,7 +935,7 @@
}
const SkFrame* prevFrame = this->getFrame(i-1);
- while (prevFrame->getDisposalMethod() == SkCodecAnimation::RestorePrevious_DisposalMethod) {
+ while (prevFrame->getDisposalMethod() == SkCodecAnimation::DisposalMethod::kRestorePrevious) {
const int prevId = prevFrame->frameId();
if (0 == prevId) {
frame->setHasAlpha(true);
@@ -992,7 +992,7 @@
return;
}
- SkASSERT(prevFrame->getDisposalMethod() == SkCodecAnimation::Keep_DisposalMethod);
+ SkASSERT(prevFrame->getDisposalMethod() == SkCodecAnimation::DisposalMethod::kKeep);
frame->setRequiredFrame(prevFrame->frameId());
frame->setHasAlpha(prevFrame->hasAlpha() || (reportsAlpha && !blendWithPrevFrame));
}
