AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Use correct color table for prior GIF frames

Browse Source 
While investigating skbug.com/5883 I noticed that we use the color
table for the current frame even while recursively decoding frames that
the current frame depends on.

This CL updates fCurrColorTable, fCurrColorTableIsReal and fSwizzler
before decoding prior frames, and then sets them back afterwards.

Move telling the client about the color table into prepareToDecode,
since the other callers do not need to do so. (That is only necessary
for decoding to index 8, which is unsupported for frames with
dependencies.)

Add a test that exposes the bug. colorTables.gif has a local color
table in its second frame that does not match the global table used by
the first frame.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4208

Change-Id: Id2dc9e3283adfd92801d2f38726afa74574b1955
Reviewed-on: https://skia-review.googlesource.com/4208
Reviewed-by: Joost Ouwerling <joostouwerling@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
This commit is contained in:
Leon Scroggins III 2016-10-31 14:08:56 -04:00 committed by Skia Commit-Bot
parent 7f650bdfd8
commit fc49b403eb
4 changed files with 21 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

BIN
resources/colorTables.gif Normal file
View File

Binary file not shown.
Side by Side

After

Width:  |  Height:  |  Size: 2.8 KiB

28
src/codec/SkGifCodec.cpp
View File

@ -139,8 +139,7 @@ std::vector<SkCodec::FrameInfo> SkGifCodec::onGetFrameInfo() {
return result;
}
void SkGifCodec::initializeColorTable(const SkImageInfo& dstInfo, size_t frameIndex,
SkPMColor* inputColorPtr, int* inputColorCount) {
void SkGifCodec::initializeColorTable(const SkImageInfo& dstInfo, size_t frameIndex) {
fCurrColorTable = fReader->getColorTable(dstInfo.colorType(), frameIndex);
fCurrColorTableIsReal = fCurrColorTable;
if (!fCurrColorTable) {
@ -148,12 +147,6 @@ void SkGifCodec::initializeColorTable(const SkImageInfo& dstInfo, size_t frameIn
SkPMColor color = SK_ColorTRANSPARENT;
fCurrColorTable.reset(new SkColorTable(&color, 1));
}
if (inputColorCount) {
*inputColorCount = fCurrColorTable->count();
}
copy_color_table(dstInfo, fCurrColorTable.get(), inputColorPtr, inputColorCount);
}
@ -215,9 +208,15 @@ SkCodec::Result SkGifCodec::prepareToDecode(const SkImageInfo& dstInfo, SkPMColo
fTmpBuffer.reset(new uint8_t[dstInfo.minRowBytes()]);
// Initialize color table and copy to the client if necessary
this->initializeColorTable(dstInfo, frameIndex, inputColorPtr, inputColorCount);
this->initializeColorTable(dstInfo, frameIndex);
this->initializeSwizzler(dstInfo, frameIndex);
SkASSERT(fCurrColorTable);
if (inputColorCount) {
*inputColorCount = fCurrColorTable->count();
}
copy_color_table(dstInfo, fCurrColorTable.get(), inputColorPtr, inputColorCount);
return kSuccess;
}
@ -337,6 +336,11 @@ SkCodec::Result SkGifCodec::decodeFrame(bool firstAttempt, const Options& opts,
Options prevFrameOpts(opts);
prevFrameOpts.fFrameIndex = frameContext->getRequiredFrame();
prevFrameOpts.fHasPriorFrame = false;
// The prior frame may have a different color table, so update it and the
// swizzler.
this->initializeColorTable(dstInfo, prevFrameOpts.fFrameIndex);
this->initializeSwizzler(dstInfo, prevFrameOpts.fFrameIndex);
const Result prevResult = this->decodeFrame(true, prevFrameOpts, nullptr);
switch (prevResult) {
case kSuccess:
@ -348,6 +352,10 @@ SkCodec::Result SkGifCodec::decodeFrame(bool firstAttempt, const Options& opts,
default:
return prevResult;
}
// Go back to using the correct color table for this frame.
this->initializeColorTable(dstInfo, frameIndex);
this->initializeSwizzler(dstInfo, frameIndex);
}
const auto* prevFrame = fReader->frameContext(frameContext->getRequiredFrame());
if (prevFrame->getDisposalMethod() == SkCodecAnimation::RestoreBGColor_DisposalMethod) {

13
src/codec/SkGifCodec.h
View File

@ -62,19 +62,8 @@ private:
*
* @param dstInfo Contains the requested dst color type.
* @param frameIndex Frame whose color table to use.
* @param inputColorPtr Copies the encoded color table to the client's
* input color table if the client requests kIndex8.
* @param inputColorCount If the client requests kIndex8, this will be set
* to the number of colors in the array that
* inputColorPtr now points to. This will typically
* be 256. Since gifs may have up to 8-bit indices,
* using a 256-entry table means a pixel will never
* be out of range. This will be set to 1 if there
* is no color table, since that will be a
* transparent frame.
*/
void initializeColorTable(const SkImageInfo& dstInfo, size_t frameIndex,
SkPMColor* colorPtr, int* inputColorCount);
void initializeColorTable(const SkImageInfo& dstInfo, size_t frameIndex);
/*
* Does necessary setup, including setting up the color table and swizzler,

3
tests/CodecAnimTest.cpp
View File

@ -29,6 +29,7 @@ DEF_TEST(Codec_frames, r) {
{ "box.gif", 1, {}, {} },
{ "color_wheel.gif", 1, {}, {} },
{ "test640x479.gif", 4, { 0, 1, 2 }, { 200, 200, 200, 200 } },
{ "colorTables.gif", 2, { 0 }, { 1000, 1000 } },
{ "arrow.png", 1, {}, {} },
{ "google_chrome.ico", 1, {}, {} },
@ -67,7 +68,7 @@ DEF_TEST(Codec_frames, r) {
if (rec.fRequiredFrames.size() + 1 != expected) {
ERRORF(r, "'%s' has wrong number entries in fRequiredFrames; expected: %i\tactual: %i",
rec.fName, expected, rec.fRequiredFrames.size());
rec.fName, expected, rec.fRequiredFrames.size() + 1);
continue;
}