From dbf9f884c3996587ec9a9fd0f091357cf998c267 Mon Sep 17 00:00:00 2001 From: "scroggo@google.com" Date: Wed, 21 Aug 2013 15:01:48 +0000 Subject: [PATCH] Avoid getLength in ico decoder. Only call getLength() if hasLength() returned true. Otherwise, copy the stream into an SkDynamicMemoryWStream and copy it into alloc'ed space. Share common code between bmp and ico. BUG=https://b.corp.google.com/issue?id=8432093 R=djsollen@google.com Review URL: https://codereview.chromium.org/23330002 git-svn-id: http://skia.googlecode.com/svn/trunk@10850 2bbb7eff-a529-9590-31e7-b0007b416f81 --- gyp/images.gyp | 2 ++ src/images/SkImageDecoder_libbmp.cpp | 30 +++++---------------- src/images/SkImageDecoder_libico.cpp | 14 +++++----- src/images/SkStreamHelpers.cpp | 40 ++++++++++++++++++++++++++++ src/images/SkStreamHelpers.h | 27 +++++++++++++++++++ 5 files changed, 82 insertions(+), 31 deletions(-) create mode 100644 src/images/SkStreamHelpers.cpp create mode 100644 src/images/SkStreamHelpers.h diff --git a/gyp/images.gyp b/gyp/images.gyp index 0bc8a76982..7abaac23ed 100644 --- a/gyp/images.gyp +++ b/gyp/images.gyp @@ -64,6 +64,8 @@ '../src/images/SkPageFlipper.cpp', '../src/images/SkScaledBitmapSampler.cpp', '../src/images/SkScaledBitmapSampler.h', + '../src/images/SkStreamHelpers.cpp', + '../src/images/SkStreamHelpers.h', '../src/ports/SkImageDecoder_CG.cpp', '../src/ports/SkImageDecoder_WIC.cpp', diff --git a/src/images/SkImageDecoder_libbmp.cpp b/src/images/SkImageDecoder_libbmp.cpp index 6c5ae27c47..73e7a20144 100644 --- a/src/images/SkImageDecoder_libbmp.cpp +++ b/src/images/SkImageDecoder_libbmp.cpp @@ -8,10 +8,11 @@ #include "bmpdecoderhelper.h" +#include "SkColorPriv.h" #include "SkImageDecoder.h" #include "SkScaledBitmapSampler.h" #include "SkStream.h" -#include "SkColorPriv.h" +#include "SkStreamHelpers.h" #include "SkTDArray.h" #include "SkTRegistry.h" @@ -96,31 +97,12 @@ bool SkBMPImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) { // First read the entire stream, so that all of the data can be passed to // the BmpDecoderHelper. - // Byte length of all of the data. - size_t length; // Allocated space used to hold the data. SkAutoMalloc storage; - - if (stream->hasLength()) { - length = stream->getLength(); - void* dst = storage.reset(length); - if (stream->read(dst, length) != length) { - return false; - } - } else { - SkDynamicMemoryWStream tempStream; - // Arbitrary buffer size. - const size_t bufferSize = 256 * 1024; // 256 KB - char buffer[bufferSize]; - length = 0; - do { - size_t bytesRead = stream->read(buffer, bufferSize); - tempStream.write(buffer, bytesRead); - length += bytesRead; - SkASSERT(tempStream.bytesWritten() == length); - } while (!stream->isAtEnd()); - void* dst = storage.reset(length); - tempStream.copyTo(dst); + // Byte length of all of the data. + const size_t length = CopyStreamToStorage(&storage, stream); + if (0 == length) { + return 0; } const bool justBounds = SkImageDecoder::kDecodeBounds_Mode == mode; diff --git a/src/images/SkImageDecoder_libico.cpp b/src/images/SkImageDecoder_libico.cpp index b14e19614c..47fa7293f0 100644 --- a/src/images/SkImageDecoder_libico.cpp +++ b/src/images/SkImageDecoder_libico.cpp @@ -1,4 +1,3 @@ - /* * Copyright 2006 The Android Open Source Project * @@ -6,10 +5,10 @@ * found in the LICENSE file. */ - +#include "SkColorPriv.h" #include "SkImageDecoder.h" #include "SkStream.h" -#include "SkColorPriv.h" +#include "SkStreamHelpers.h" #include "SkTypes.h" class SkICOImageDecoder : public SkImageDecoder { @@ -75,13 +74,14 @@ static int calculateRowBytesFor8888(int w, int bitCount) bool SkICOImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) { - size_t length = stream->getLength(); - SkAutoMalloc autoMal(length); - unsigned char* buf = (unsigned char*)autoMal.get(); - if (stream->read((void*)buf, length) != length) { + SkAutoMalloc autoMal; + const size_t length = CopyStreamToStorage(&autoMal, stream); + if (0 == length) { return false; } + unsigned char* buf = (unsigned char*)autoMal.get(); + //these should always be the same - should i use for error checking? - what about files that have some //incorrect values, but still decode properly? int reserved = read2Bytes(buf, 0); // 0 diff --git a/src/images/SkStreamHelpers.cpp b/src/images/SkStreamHelpers.cpp new file mode 100644 index 0000000000..3e9ee45345 --- /dev/null +++ b/src/images/SkStreamHelpers.cpp @@ -0,0 +1,40 @@ +/* + * Copyright 2013 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "SkStream.h" +#include "SkStreamHelpers.h" +#include "SkTypes.h" + +size_t CopyStreamToStorage(SkAutoMalloc* storage, SkStream* stream) { + SkASSERT(storage != NULL); + SkASSERT(stream != NULL); + + if (stream->hasLength()) { + const size_t length = stream->getLength(); + void* dst = storage->reset(length); + if (stream->read(dst, length) != length) { + return 0; + } + return length; + } + + SkDynamicMemoryWStream tempStream; + // Arbitrary buffer size. + const size_t bufferSize = 256 * 1024; // 256KB + char buffer[bufferSize]; + SkDEBUGCODE(size_t debugLength = 0;) + do { + size_t bytesRead = stream->read(buffer, bufferSize); + tempStream.write(buffer, bytesRead); + SkDEBUGCODE(debugLength += bytesRead); + SkASSERT(tempStream.bytesWritten() == debugLength); + } while (!stream->isAtEnd()); + const size_t length = tempStream.bytesWritten(); + void* dst = storage->reset(length); + tempStream.copyTo(dst); + return length; +} diff --git a/src/images/SkStreamHelpers.h b/src/images/SkStreamHelpers.h new file mode 100644 index 0000000000..7e766b7ecc --- /dev/null +++ b/src/images/SkStreamHelpers.h @@ -0,0 +1,27 @@ +/* + * Copyright 2013 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#ifndef SkStreamHelpers_DEFINED +#define SkStreamHelpers_DEFINED + +class SkAutoMalloc; +class SkStream; + +/** + * Copy the provided stream to memory allocated by storage. + * Used by SkImageDecoder_libbmp and SkImageDecoder_libico. + * @param storage Allocator to hold the memory. Will be reset to be large + * enough to hold the entire stream. Upon successful return, + * storage->get() will point to data holding the SkStream's entire + * contents. + * @param stream SkStream to be copied into storage. + * @return size_t Total number of bytes in the SkStream, which is also the + * number of bytes pointed to by storage->get(). Returns 0 on failure. + */ +size_t CopyStreamToStorage(SkAutoMalloc* storage, SkStream* stream); + +#endif // SkStreamHelpers_DEFINED