From 434b6e81a5093bdd72b232de249386af3902248f Mon Sep 17 00:00:00 2001 From: Leon Scroggins Date: Thu, 20 Apr 2017 15:06:53 +0000 Subject: [PATCH] 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 > Reviewed-by: Matt Sarett > 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 Commit-Queue: Leon Scroggins --- gn/tests.gni | 1 - src/codec/SkPngCodec.cpp | 291 ++++++++++++++++++++++++++++++------------- src/codec/SkPngCodec.h | 27 +++- tests/CodecExactReadTest.cpp | 102 --------------- tests/CodecPartialTest.cpp | 5 +- 5 files changed, 228 insertions(+), 198 deletions(-) delete mode 100644 tests/CodecExactReadTest.cpp 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_sources = [ "$_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 // 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 @@ public: SkCodec** codecPtr) : fPng_ptr(png_ptr) , fInfo_ptr(nullptr) + , fDecodedBounds(false) + , fReadHeader(false) , fStream(stream) , fChunkReader(reader) , fOutCodec(codecPtr) @@ -111,12 +117,28 @@ public: 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(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 @@ private: }; #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 @@ bool AutoCleanPng::decodeBounds() { 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(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 @@ void SkPngCodec::processData() { 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; - } - - png_byte* chunk = reinterpret_cast(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) { + const size_t bytesRead = this->stream()->read(buffer, kBufferSize); + png_process_data(fPng_ptr, fInfo_ptr, (png_bytep) buffer, bytesRead); + + 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 @@ public: 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 @@ private: 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 @@ private: } 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 @@ public: decoder->interlacedRowCallback(row, rowNum, pass); } +#ifdef SK_GOOGLE3_PNG_HACK + static void RereadInfoInterlacedCallback(png_structp png_ptr, png_infop) { + static_cast(png_get_progressive_ptr(png_ptr))->rereadInfoInterlaced(); + } +#endif + private: const int fNumberPasses; int fFirstRow; @@ -640,6 +634,14 @@ private: 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 @@ private: 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 @@ private: 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 @@ private: } }; +#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 @@ static bool read_header(SkStream* stream, SkPngChunkReader* chunkReader, SkCodec 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 @@ void AutoCleanPng::infoCallback(size_t idatLength) { 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 @@ void AutoCleanPng::infoCallback(size_t idatLength) { 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 @@ void AutoCleanPng::infoCallback(size_t idatLength) { 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 @@ void AutoCleanPng::infoCallback(size_t idatLength) { } 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 @@ void AutoCleanPng::infoCallback(size_t idatLength) { fChunkReader, fPng_ptr, fInfo_ptr, bitDepth, numberPasses); } (*fOutCodec)->setUnsupportedICC(unsupportedICC); - static_cast(*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 @@ SkPngCodec::SkPngCodec(const SkEncodedInfo& encodedInfo, const SkImageInfo& imag , 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 @@ SkSampler* SkPngCodec::getSampler(bool createIfNecessary) { } 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 @@ bool SkPngCodec::onRewind() { 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 @@ SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst, { 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 @@ SkCodec::Result SkPngCodec::onStartIncrementalDecode(const SkImageInfo& dstInfo, { 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 @@ public: // 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 @@ protected: */ 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 @@ private: 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 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 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 stream(MyStream::Make(path, r)); - if (!stream) { - continue; - } - - std::unique_ptr 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 @@ static void test_partial(skiatest::Reporter* r, const char* name, size_t minByte } 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 @@ DEF_TEST(Codec_partial, r) { 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"); -- 2.7.4