Revert "Make SkPngCodec only read as much of the stream as necessary"
This reverts commit 2c65d5161260f3d45a63dcd92229bd09c8a12d53.
Reason for revert: Causing failures in Google3 (https://test.corp.google.com/ui#cl=153703311&flags=CAMQAg==&id=OCL:153703311:BASE:153703364:1492695824938:4db2240d&t=//chrome/skia/dm_wrapper:dm_wrapper) and differences in Gold. This change was not intended to change the output.
Original change's description:
> Make SkPngCodec only read as much of the stream as necessary
>
> Previously, SkPngCodec assumed that the stream only contained one
> image, which ended at the end of the stream. It read the stream in
> arbitrarily-sized chunks, and then passed that data to libpng for
> processing.
>
> If a stream contains more than one image, this may result in reading
> beyond the end of the image, making future reads read the wrong data.
>
> Now, SkPngCodec starts by reading 8 bytes at a time. After the
> signature, 8 bytes is enough to know which chunk is next and how many
> bytes are in the chunk.
>
> When decoding the size, we stop when we reach IDAT, and when decoding
> the image, we stop when we reach IEND.
>
> This manual parsing is necessary to support APNG, which is planned in
> the future. It also allows us to remove the SK_GOOGLE3_PNG_HACK, which
> was a workaround for reading more than necessary at the beginning of
> the image.
>
> Add a test that simulates the issue, by decoding a special stream that
> reports an error if the codec attempts to read beyond the end.
>
> Temporarily disable the partial decoding tests for png. A larger change
> will be necessary to get those working again, and no clients are
> currently relying on incrementally decoding PNGs (i.e. decode part of
> an image, then decode further with more data).
>
> Bug: skia:5368
> BUG:34073812
>
> Change-Id: If832f7b20565411226fb5be3c305a4d16bf9269d
> Reviewed-on: https://skia-review.googlesource.com/13900
> Commit-Queue: Leon Scroggins <scroggo@google.com>
> Reviewed-by: Matt Sarett <msarett@google.com>
>
TBR=msarett@google.com,scroggo@google.com,reviews@skia.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Change-Id: I2f82e9960dda7bf5c646774df84320dadb7b930e
Reviewed-on: https://skia-review.googlesource.com/13971
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
diff --git a/gn/tests.gni b/gn/tests.gni
index ffd8b41..5955a32 100644
--- a/gn/tests.gni
+++ b/gn/tests.gni
@@ -34,7 +34,6 @@
"$_tests/ClipperTest.cpp",
"$_tests/ClipStackTest.cpp",
"$_tests/CodecAnimTest.cpp",
- "$_tests/CodecExactReadTest.cpp",
"$_tests/CodecPartialTest.cpp",
"$_tests/CodecTest.cpp",
"$_tests/ColorFilterTest.cpp",
diff --git a/src/codec/SkPngCodec.cpp b/src/codec/SkPngCodec.cpp
index 5e05606..8bab368 100644
--- a/src/codec/SkPngCodec.cpp
+++ b/src/codec/SkPngCodec.cpp
@@ -22,13 +22,17 @@
#include "SkUtils.h"
#include "png.h"
-#include <algorithm>
// This warning triggers false postives way too often in here.
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic ignored "-Wclobbered"
#endif
+#if PNG_LIBPNG_VER_MAJOR > 1 || (PNG_LIBPNG_VER_MAJOR == 1 && PNG_LIBPNG_VER_MINOR >= 5)
+ // This is not needed with version 1.5
+ #undef SK_GOOGLE3_PNG_HACK
+#endif
+
// FIXME (scroggo): We can use png_jumpbuf directly once Google3 is on 1.6
#define PNG_JMPBUF(x) png_jmpbuf((png_structp) x)
@@ -76,6 +80,8 @@
SkCodec** codecPtr)
: fPng_ptr(png_ptr)
, fInfo_ptr(nullptr)
+ , fDecodedBounds(false)
+ , fReadHeader(false)
, fStream(stream)
, fChunkReader(reader)
, fOutCodec(codecPtr)
@@ -111,12 +117,28 @@
private:
png_structp fPng_ptr;
png_infop fInfo_ptr;
+ bool fDecodedBounds;
+ bool fReadHeader;
SkStream* fStream;
SkPngChunkReader* fChunkReader;
SkCodec** fOutCodec;
- void infoCallback(size_t idatLength);
+ /**
+ * Supplied to libpng to call when it has read enough data to determine
+ * bounds.
+ */
+ static void InfoCallback(png_structp png_ptr, png_infop) {
+ // png_get_progressive_ptr returns the pointer we set on the png_ptr with
+ // png_set_progressive_read_fn
+ static_cast<AutoCleanPng*>(png_get_progressive_ptr(png_ptr))->infoCallback();
+ }
+ void infoCallback();
+
+#ifdef SK_GOOGLE3_PNG_HACK
+// public so it can be called by SkPngCodec::rereadHeaderIfNecessary().
+public:
+#endif
void releasePngPtrs() {
fPng_ptr = nullptr;
fInfo_ptr = nullptr;
@@ -124,29 +146,12 @@
};
#define AutoCleanPng(...) SK_REQUIRE_LOCAL_VAR(AutoCleanPng)
-static inline bool is_chunk(const png_byte* chunk, const char* tag) {
- return memcmp(chunk + 4, tag, 4) == 0;
-}
-
-static inline bool process_data(png_structp png_ptr, png_infop info_ptr,
- SkStream* stream, void* buffer, size_t bufferSize, size_t length) {
- while (length > 0) {
- const size_t bytesToProcess = std::min(bufferSize, length);
- if (stream->read(buffer, bytesToProcess) < bytesToProcess) {
- return false;
- }
- png_process_data(png_ptr, info_ptr, (png_bytep) buffer, bytesToProcess);
- length -= bytesToProcess;
- }
- return true;
-}
-
bool AutoCleanPng::decodeBounds() {
if (setjmp(PNG_JMPBUF(fPng_ptr))) {
return false;
}
- png_set_progressive_read_fn(fPng_ptr, nullptr, nullptr, nullptr, nullptr);
+ png_set_progressive_read_fn(fPng_ptr, this, InfoCallback, nullptr, nullptr);
// Arbitrary buffer size, though note that it matches (below)
// SkPngCodec::processData(). FIXME: Can we better suit this to the size of
@@ -154,39 +159,22 @@
constexpr size_t kBufferSize = 4096;
char buffer[kBufferSize];
- {
- // Parse the signature.
- if (fStream->read(buffer, 8) < 8) {
- return false;
- }
-
- png_process_data(fPng_ptr, fInfo_ptr, (png_bytep) buffer, 8);
- }
-
while (true) {
- // Parse chunk length and type.
- if (fStream->read(buffer, 8) < 8) {
+ const size_t bytesRead = fStream->read(buffer, kBufferSize);
+ if (!bytesRead) {
// We have read to the end of the input without decoding bounds.
break;
}
- png_byte* chunk = reinterpret_cast<png_byte*>(buffer);
- png_process_data(fPng_ptr, fInfo_ptr, chunk, 8);
-
- // Include the full chunk + CRC.
- const size_t length = png_get_uint_32(chunk) + 4;
-
- if (is_chunk(chunk, "IDAT")) {
- this->infoCallback(length);
- return true;
- }
-
- if (!process_data(fPng_ptr, fInfo_ptr, fStream, buffer, kBufferSize, length)) {
- return false;
+ png_process_data(fPng_ptr, fInfo_ptr, (png_bytep) buffer, bytesRead);
+ if (fReadHeader) {
+ break;
}
}
- return false;
+ // For safety, clear the pointer to this object.
+ png_set_progressive_read_fn(fPng_ptr, nullptr, nullptr, nullptr, nullptr);
+ return fDecodedBounds;
}
void SkPngCodec::processData() {
@@ -210,30 +198,16 @@
constexpr size_t kBufferSize = 4096;
char buffer[kBufferSize];
- bool iend = false;
while (true) {
- size_t length;
- if (fDecodedIdat) {
- // Parse chunk length and type.
- if (this->stream()->read(buffer, 8) < 8) {
- break;
- }
+ const size_t bytesRead = this->stream()->read(buffer, kBufferSize);
+ png_process_data(fPng_ptr, fInfo_ptr, (png_bytep) buffer, bytesRead);
- png_byte* chunk = reinterpret_cast<png_byte*>(buffer);
- png_process_data(fPng_ptr, fInfo_ptr, chunk, 8);
- if (is_chunk(chunk, "IEND")) {
- iend = true;
- }
-
- // Include the full chunk + CRC.
- length = png_get_uint_32(chunk) + 4;
- } else {
- length = fIdatLength;
- fDecodedIdat = true;
- }
-
- if (!process_data(fPng_ptr, fInfo_ptr, this->stream(), buffer, kBufferSize, length)
- || iend) {
+ if (!bytesRead) {
+ // We have read to the end of the input. Note that we quit *after*
+ // calling png_process_data, because decodeBounds may have told
+ // libpng to save the remainder of the buffer, in which case
+ // png_process_data will process the saved buffer, though the
+ // stream has no more to read.
break;
}
}
@@ -511,6 +485,12 @@
GetDecoder(png_ptr)->rowCallback(row, rowNum);
}
+#ifdef SK_GOOGLE3_PNG_HACK
+ static void RereadInfoCallback(png_structp png_ptr, png_infop) {
+ GetDecoder(png_ptr)->rereadInfoCallback();
+ }
+#endif
+
private:
int fRowsWrittenToOutput;
void* fDst;
@@ -529,7 +509,11 @@
Result decodeAllRows(void* dst, size_t rowBytes, int* rowsDecoded) override {
const int height = this->getInfo().height();
- png_set_progressive_read_fn(this->png_ptr(), this, nullptr, AllRowsCallback, nullptr);
+ png_progressive_info_ptr callback = nullptr;
+#ifdef SK_GOOGLE3_PNG_HACK
+ callback = RereadInfoCallback;
+#endif
+ png_set_progressive_read_fn(this->png_ptr(), this, callback, AllRowsCallback, nullptr);
fDst = dst;
fRowBytes = rowBytes;
@@ -558,7 +542,11 @@
}
void setRange(int firstRow, int lastRow, void* dst, size_t rowBytes) override {
- png_set_progressive_read_fn(this->png_ptr(), this, nullptr, RowCallback, nullptr);
+ png_progressive_info_ptr callback = nullptr;
+#ifdef SK_GOOGLE3_PNG_HACK
+ callback = RereadInfoCallback;
+#endif
+ png_set_progressive_read_fn(this->png_ptr(), this, callback, RowCallback, nullptr);
fFirstRow = firstRow;
fLastRow = lastRow;
fDst = dst;
@@ -627,6 +615,12 @@
decoder->interlacedRowCallback(row, rowNum, pass);
}
+#ifdef SK_GOOGLE3_PNG_HACK
+ static void RereadInfoInterlacedCallback(png_structp png_ptr, png_infop) {
+ static_cast<SkPngInterlacedDecoder*>(png_get_progressive_ptr(png_ptr))->rereadInfoInterlaced();
+ }
+#endif
+
private:
const int fNumberPasses;
int fFirstRow;
@@ -640,6 +634,14 @@
typedef SkPngCodec INHERITED;
+#ifdef SK_GOOGLE3_PNG_HACK
+ void rereadInfoInterlaced() {
+ this->rereadInfoCallback();
+ // Note: This allocates more memory than necessary, if we are sampling/subset.
+ this->setUpInterlaceBuffer(this->getInfo().height());
+ }
+#endif
+
// FIXME: Currently sharing interlaced callback for all rows and subset. It's not
// as expensive as the subset version of non-interlaced, but it still does extra
// work.
@@ -673,7 +675,11 @@
SkCodec::Result decodeAllRows(void* dst, size_t rowBytes, int* rowsDecoded) override {
const int height = this->getInfo().height();
this->setUpInterlaceBuffer(height);
- png_set_progressive_read_fn(this->png_ptr(), this, nullptr, InterlacedRowCallback,
+ png_progressive_info_ptr callback = nullptr;
+#ifdef SK_GOOGLE3_PNG_HACK
+ callback = RereadInfoInterlacedCallback;
+#endif
+ png_set_progressive_read_fn(this->png_ptr(), this, callback, InterlacedRowCallback,
nullptr);
fFirstRow = 0;
@@ -703,7 +709,11 @@
void setRange(int firstRow, int lastRow, void* dst, size_t rowBytes) override {
// FIXME: We could skip rows in the interlace buffer that we won't put in the output.
this->setUpInterlaceBuffer(lastRow - firstRow + 1);
- png_set_progressive_read_fn(this->png_ptr(), this, nullptr, InterlacedRowCallback, nullptr);
+ png_progressive_info_ptr callback = nullptr;
+#ifdef SK_GOOGLE3_PNG_HACK
+ callback = RereadInfoInterlacedCallback;
+#endif
+ png_set_progressive_read_fn(this->png_ptr(), this, callback, InterlacedRowCallback, nullptr);
fFirstRow = firstRow;
fLastRow = lastRow;
fDst = dst;
@@ -757,6 +767,54 @@
}
};
+#ifdef SK_GOOGLE3_PNG_HACK
+bool SkPngCodec::rereadHeaderIfNecessary() {
+ if (!fNeedsToRereadHeader) {
+ return true;
+ }
+
+ // On the first call, we'll need to rewind ourselves. Future calls will
+ // have already rewound in rewindIfNecessary.
+ if (this->stream()->getPosition() > 0) {
+ this->stream()->rewind();
+ }
+
+ this->destroyReadStruct();
+ png_structp png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING, nullptr,
+ sk_error_fn, sk_warning_fn);
+ if (!png_ptr) {
+ return false;
+ }
+
+ // Only use the AutoCleanPng to delete png_ptr as necessary.
+ // (i.e. not for reading bounds etc.)
+ AutoCleanPng autoClean(png_ptr, nullptr, nullptr, nullptr);
+
+ png_infop info_ptr = png_create_info_struct(png_ptr);
+ if (info_ptr == nullptr) {
+ return false;
+ }
+
+ autoClean.setInfoPtr(info_ptr);
+
+#ifdef PNG_READ_UNKNOWN_CHUNKS_SUPPORTED
+ // Hookup our chunkReader so we can see any user-chunks the caller may be interested in.
+ // This needs to be installed before we read the png header. Android may store ninepatch
+ // chunks in the header.
+ if (fPngChunkReader.get()) {
+ png_set_keep_unknown_chunks(png_ptr, PNG_HANDLE_CHUNK_ALWAYS, (png_byte*)"", 0);
+ png_set_read_user_chunk_fn(png_ptr, (png_voidp) fPngChunkReader.get(), sk_read_user_chunk);
+ }
+#endif
+
+ fPng_ptr = png_ptr;
+ fInfo_ptr = info_ptr;
+ autoClean.releasePngPtrs();
+ fNeedsToRereadHeader = false;
+ return true;
+}
+#endif // SK_GOOGLE3_PNG_HACK
+
// Reads the header and initializes the output fields, if not NULL.
//
// @param stream Input data. Will be read to get enough information to properly
@@ -828,17 +886,21 @@
return true;
}
-void AutoCleanPng::infoCallback(size_t idatLength) {
+// FIXME (scroggo): Once SK_GOOGLE3_PNG_HACK is no more, this method can be inline in
+// AutoCleanPng::infoCallback
+static void general_info_callback(png_structp png_ptr, png_infop info_ptr,
+ SkEncodedInfo::Color* outColor, SkEncodedInfo::Alpha* outAlpha,
+ int* outBitDepth) {
png_uint_32 origWidth, origHeight;
int bitDepth, encodedColorType;
- png_get_IHDR(fPng_ptr, fInfo_ptr, &origWidth, &origHeight, &bitDepth,
+ png_get_IHDR(png_ptr, info_ptr, &origWidth, &origHeight, &bitDepth,
&encodedColorType, nullptr, nullptr, nullptr);
// TODO: Should we support 16-bits of precision for gray images?
if (bitDepth == 16 && (PNG_COLOR_TYPE_GRAY == encodedColorType ||
PNG_COLOR_TYPE_GRAY_ALPHA == encodedColorType)) {
bitDepth = 8;
- png_set_strip_16(fPng_ptr);
+ png_set_strip_16(png_ptr);
}
// Now determine the default colorType and alphaType and set the required transforms.
@@ -853,18 +915,18 @@
if (bitDepth < 8) {
// TODO: Should we use SkSwizzler here?
bitDepth = 8;
- png_set_packing(fPng_ptr);
+ png_set_packing(png_ptr);
}
color = SkEncodedInfo::kPalette_Color;
// Set the alpha depending on if a transparency chunk exists.
- alpha = png_get_valid(fPng_ptr, fInfo_ptr, PNG_INFO_tRNS) ?
+ alpha = png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS) ?
SkEncodedInfo::kUnpremul_Alpha : SkEncodedInfo::kOpaque_Alpha;
break;
case PNG_COLOR_TYPE_RGB:
- if (png_get_valid(fPng_ptr, fInfo_ptr, PNG_INFO_tRNS)) {
+ if (png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS)) {
// Convert to RGBA if transparency chunk exists.
- png_set_tRNS_to_alpha(fPng_ptr);
+ png_set_tRNS_to_alpha(png_ptr);
color = SkEncodedInfo::kRGBA_Color;
alpha = SkEncodedInfo::kBinary_Alpha;
} else {
@@ -877,11 +939,11 @@
if (bitDepth < 8) {
// TODO: Should we use SkSwizzler here?
bitDepth = 8;
- png_set_expand_gray_1_2_4_to_8(fPng_ptr);
+ png_set_expand_gray_1_2_4_to_8(png_ptr);
}
- if (png_get_valid(fPng_ptr, fInfo_ptr, PNG_INFO_tRNS)) {
- png_set_tRNS_to_alpha(fPng_ptr);
+ if (png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS)) {
+ png_set_tRNS_to_alpha(png_ptr);
color = SkEncodedInfo::kGrayAlpha_Color;
alpha = SkEncodedInfo::kBinary_Alpha;
} else {
@@ -903,9 +965,43 @@
color = SkEncodedInfo::kRGBA_Color;
alpha = SkEncodedInfo::kUnpremul_Alpha;
}
+ if (outColor) {
+ *outColor = color;
+ }
+ if (outAlpha) {
+ *outAlpha = alpha;
+ }
+ if (outBitDepth) {
+ *outBitDepth = bitDepth;
+ }
+}
+
+#ifdef SK_GOOGLE3_PNG_HACK
+void SkPngCodec::rereadInfoCallback() {
+ general_info_callback(fPng_ptr, fInfo_ptr, nullptr, nullptr, nullptr);
+ png_set_interlace_handling(fPng_ptr);
+ png_read_update_info(fPng_ptr, fInfo_ptr);
+}
+#endif
+
+void AutoCleanPng::infoCallback() {
+ SkEncodedInfo::Color color;
+ SkEncodedInfo::Alpha alpha;
+ int bitDepth;
+ general_info_callback(fPng_ptr, fInfo_ptr, &color, &alpha, &bitDepth);
const int numberPasses = png_set_interlace_handling(fPng_ptr);
+ fReadHeader = true;
+ fDecodedBounds = true;
+#ifndef SK_GOOGLE3_PNG_HACK
+ // 1 tells libpng to save any extra data. We may be able to be more efficient by saving
+ // it ourselves.
+ png_process_data_pause(fPng_ptr, 1);
+#else
+ // Hack to make png_process_data stop.
+ fPng_ptr->buffer_size = 0;
+#endif
if (fOutCodec) {
SkASSERT(nullptr == *fOutCodec);
SkColorSpace_Base::ICCTypeFlag iccType = SkColorSpace_Base::kRGB_ICCTypeFlag;
@@ -920,6 +1016,11 @@
}
SkEncodedInfo encodedInfo = SkEncodedInfo::Make(color, alpha, bitDepth);
+ // FIXME (scroggo): Once we get rid of SK_GOOGLE3_PNG_HACK, general_info_callback can
+ // be inlined, so these values will already be set.
+ png_uint_32 origWidth = png_get_image_width(fPng_ptr, fInfo_ptr);
+ png_uint_32 origHeight = png_get_image_height(fPng_ptr, fInfo_ptr);
+ png_byte bitDepth = png_get_bit_depth(fPng_ptr, fInfo_ptr);
SkImageInfo imageInfo = encodedInfo.makeImageInfo(origWidth, origHeight, colorSpace);
if (SkEncodedInfo::kOpaque_Alpha == alpha) {
@@ -940,9 +1041,9 @@
fChunkReader, fPng_ptr, fInfo_ptr, bitDepth, numberPasses);
}
(*fOutCodec)->setUnsupportedICC(unsupportedICC);
- static_cast<SkPngCodec*>(*fOutCodec)->setIdatLength(idatLength);
}
+
// Release the pointers, which are now owned by the codec or the caller is expected to
// take ownership.
this->releasePngPtrs();
@@ -957,8 +1058,9 @@
, fInfo_ptr(info_ptr)
, fColorXformSrcRow(nullptr)
, fBitDepth(bitDepth)
- , fIdatLength(0)
- , fDecodedIdat(false)
+#ifdef SK_GOOGLE3_PNG_HACK
+ , fNeedsToRereadHeader(true)
+#endif
{}
SkPngCodec::~SkPngCodec() {
@@ -1085,6 +1187,10 @@
}
bool SkPngCodec::onRewind() {
+#ifdef SK_GOOGLE3_PNG_HACK
+ fNeedsToRereadHeader = true;
+ return true;
+#else
// This sets fPng_ptr and fInfo_ptr to nullptr. If read_header
// succeeds, they will be repopulated, and if it fails, they will
// remain nullptr. Any future accesses to fPng_ptr and fInfo_ptr will
@@ -1100,8 +1206,8 @@
fPng_ptr = png_ptr;
fInfo_ptr = info_ptr;
- fDecodedIdat = false;
return true;
+#endif
}
SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
@@ -1113,6 +1219,13 @@
{
return kInvalidConversion;
}
+#ifdef SK_GOOGLE3_PNG_HACK
+ // Note that this is done after initializeXforms. Otherwise that method
+ // would not have png_ptr to use.
+ if (!this->rereadHeaderIfNecessary()) {
+ return kCouldNotRewind;
+ }
+#endif
if (options.fSubset) {
return kUnimplemented;
@@ -1131,6 +1244,12 @@
{
return kInvalidConversion;
}
+#ifdef SK_GOOGLE3_PNG_HACK
+ // See note in onGetPixels.
+ if (!this->rereadHeaderIfNecessary()) {
+ return kCouldNotRewind;
+ }
+#endif
this->allocateStorage(dstInfo);
diff --git a/src/codec/SkPngCodec.h b/src/codec/SkPngCodec.h
index 4809723..09231f1 100644
--- a/src/codec/SkPngCodec.h
+++ b/src/codec/SkPngCodec.h
@@ -16,6 +16,13 @@
#include "SkRefCnt.h"
#include "SkSwizzler.h"
+// FIXME (scroggo): GOOGLE3 is currently using an outdated version of libpng,
+// so we need to work around the lack of the method png_process_data_pause.
+// This code will be unnecessary once we update GOOGLE3. It would make more
+// sense to condition this on the version of libpng being used, but we do not
+// know that here because png.h is only included by the cpp file.
+#define SK_GOOGLE3_PNG_HACK
+
class SkStream;
class SkPngCodec : public SkCodec {
@@ -25,9 +32,6 @@
// Assume IsPng was called and returned true.
static SkCodec* NewFromStream(SkStream*, SkPngChunkReader* = NULL);
- // FIXME (scroggo): Temporarily needed by AutoCleanPng.
- void setIdatLength(size_t len) { fIdatLength = len; }
-
~SkPngCodec() override;
protected:
@@ -72,6 +76,18 @@
*/
void processData();
+#ifdef SK_GOOGLE3_PNG_HACK
+ // In libpng 1.2.56, png_process_data_pause does not exist, so when we wanted to
+ // read the header, we may have read too far. In that case, we need to delete the
+ // png_ptr and info_ptr and recreate them. This method does that (and attaches the
+ // chunk reader.
+ bool rereadHeaderIfNecessary();
+
+ // This method sets up the new png_ptr/info_ptr (created in rereadHeaderIfNecessary)
+ // the way we set up the old one the first time in AutoCleanPng.decodeBounds's callback.
+ void rereadInfoCallback();
+#endif
+
Result onStartIncrementalDecode(const SkImageInfo& dstInfo, void* pixels, size_t rowBytes,
const SkCodec::Options&,
SkPMColor* ctable, int* ctableCount) override;
@@ -118,8 +134,9 @@
SkAlphaType fXformAlphaType;
int fXformWidth;
- size_t fIdatLength;
- bool fDecodedIdat;
+#ifdef SK_GOOGLE3_PNG_HACK
+ bool fNeedsToRereadHeader;
+#endif
typedef SkCodec INHERITED;
};
diff --git a/tests/CodecExactReadTest.cpp b/tests/CodecExactReadTest.cpp
deleted file mode 100644
index 7e0d8ea..0000000
--- a/tests/CodecExactReadTest.cpp
+++ /dev/null
@@ -1,102 +0,0 @@
-/*
- * Copyright 2017 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#include "Resources.h"
-#include "Test.h"
-
-#include "SkBitmap.h"
-#include "SkCodec.h"
-#include "SkData.h"
-#include "SkStream.h"
-
-namespace {
-// This class emits a skiatest failure if a client attempts to read beyond its
-// end. Since it is used with complete, valid images, and contains nothing
-// after the encoded image data, it will emit a failure if the client attempts
-// to read beyond the logical end of the data.
-class MyStream : public SkStream {
-public:
- static MyStream* Make(const char* path, skiatest::Reporter* r) {
- SkASSERT(path);
- sk_sp<SkData> data(GetResourceAsData(path));
- if (!data) {
- return nullptr;
- }
-
- return new MyStream(path, std::move(data), r);
- }
-
- size_t read(void* buf, size_t bytes) override {
- const size_t remaining = fStream.getLength() - fStream.getPosition();
- if (bytes > remaining) {
- ERRORF(fReporter, "Tried to read %lu bytes (only %lu remaining) from %s",
- bytes, remaining, fPath);
- }
- return fStream.read(buf, bytes);
- }
-
- bool rewind() override {
- return fStream.rewind();
- }
-
- bool isAtEnd() const override {
- return fStream.isAtEnd();
- }
-private:
- const char* fPath;
- SkMemoryStream fStream;
- skiatest::Reporter* fReporter; // Unowned
-
- MyStream(const char* path, sk_sp<SkData> data, skiatest::Reporter* r)
- : fPath(path)
- , fStream(std::move(data))
- , fReporter(r)
- {}
-};
-} // namespace
-
-// Test that SkPngCodec does not attempt to read its input beyond the logical
-// end of its data. Some other SkCodecs do, but some Android apps rely on not
-// doing so for PNGs.
-DEF_TEST(Codec_end, r) {
- for (const char* path : { "plane.png",
- "yellow_rose.png",
- "plane_interlaced.png" }) {
- std::unique_ptr<MyStream> stream(MyStream::Make(path, r));
- if (!stream) {
- continue;
- }
-
- std::unique_ptr<SkCodec> codec(SkCodec::NewFromStream(stream.release()));
- if (!codec) {
- ERRORF(r, "Failed to create a codec from %s\n", path);
- continue;
- }
-
- auto info = codec->getInfo().makeColorType(kN32_SkColorType);
- SkBitmap bm;
- bm.allocPixels(info);
-
- auto result = codec->getPixels(bm.info(), bm.getPixels(), bm.rowBytes());
- if (result != SkCodec::kSuccess) {
- ERRORF(r, "Failed to getPixels from %s. error %i", path, result);
- continue;
- }
-
- // Rewind and do an incremental decode.
- result = codec->startIncrementalDecode(bm.info(), bm.getPixels(), bm.rowBytes());
- if (result != SkCodec::kSuccess) {
- ERRORF(r, "Failed to startIncrementalDecode from %s. error %i", path, result);
- continue;
- }
-
- result = codec->incrementalDecode();
- if (result != SkCodec::kSuccess) {
- ERRORF(r, "Failed to incrementalDecode from %s. error %i", path, result);
- }
- }
-}
diff --git a/tests/CodecPartialTest.cpp b/tests/CodecPartialTest.cpp
index c029922..20cd1d1 100644
--- a/tests/CodecPartialTest.cpp
+++ b/tests/CodecPartialTest.cpp
@@ -118,9 +118,6 @@
}
DEF_TEST(Codec_partial, r) {
-#if 0
- // FIXME (scroggo): SkPngCodec needs to use SkStreamBuffer in order to
- // support incremental decoding.
test_partial(r, "plane.png");
test_partial(r, "plane_interlaced.png");
test_partial(r, "yellow_rose.png");
@@ -131,7 +128,7 @@
test_partial(r, "arrow.png");
test_partial(r, "randPixels.png");
test_partial(r, "baby_tux.png");
-#endif
+
test_partial(r, "box.gif");
test_partial(r, "randPixels.gif", 215);
test_partial(r, "color_wheel.gif");
