From 7c9d539d8843ad75a1c249633bbc8bb331f5035e Mon Sep 17 00:00:00 2001 From: "scroggo@google.com" Date: Mon, 10 Dec 2012 15:40:55 +0000 Subject: [PATCH] In SKP serialization, use existing encoded data. If an SkBitmap has encoded data, write that during serialization rather than reencoding it. Add a test to ensure that this does not modify the output stream, so the reader need not know the difference. Review URL: https://codereview.appspot.com/6884054 git-svn-id: http://skia.googlecode.com/svn/trunk@6732 2bbb7eff-a529-9590-31e7-b0007b416f81 --- src/core/SkOrderedWriteBuffer.cpp | 110 +++++++++++++++++++++--------- src/core/SkOrderedWriteBuffer.h | 22 +++--- tests/PictureTest.cpp | 68 ++++++++++++++++++ 3 files changed, 159 insertions(+), 41 deletions(-) diff --git a/src/core/SkOrderedWriteBuffer.cpp b/src/core/SkOrderedWriteBuffer.cpp index e43a111ddc..7db0312ef4 100644 --- a/src/core/SkOrderedWriteBuffer.cpp +++ b/src/core/SkOrderedWriteBuffer.cpp @@ -8,6 +8,8 @@ #include "SkOrderedWriteBuffer.h" #include "SkBitmap.h" +#include "SkData.h" +#include "SkPixelRef.h" #include "SkPtrRecorder.h" #include "SkStream.h" #include "SkTypeface.h" @@ -138,45 +140,72 @@ bool SkOrderedWriteBuffer::writeToStream(SkWStream* stream) { } void SkOrderedWriteBuffer::writeBitmap(const SkBitmap& bitmap) { + // Record information about the bitmap in one of three ways, in order of priority: + // 1. If there is an SkBitmapHeap, store it in the heap. The client can avoid serializing the + // bitmap entirely or serialize it later as desired. + // 2. Write an encoded version of the bitmap. Afterwards the width and height are written, so + // a reader without a decoder can draw a dummy bitmap of the right size. + // A. If the bitmap has an encoded representation, write it to the stream. + // B. If there is a function for encoding bitmaps, use it. + // 3. Call SkBitmap::flatten. + // For an encoded bitmap, write the size first. Otherwise store a 0 so the reader knows not to + // decode. + if (fBitmapHeap != NULL) { + SkASSERT(NULL == fBitmapEncoder); + // Bitmap was not encoded. Record a zero, implying that the reader need not decode. + this->writeUInt(0); + int32_t slot = fBitmapHeap->insert(bitmap); + fWriter.write32(slot); + // crbug.com/155875 + // The generation ID is not required information. We write it to prevent collisions + // in SkFlatDictionary. It is possible to get a collision when a previously + // unflattened (i.e. stale) instance of a similar flattenable is in the dictionary + // and the instance currently being written is re-using the same slot from the + // bitmap heap. + fWriter.write32(bitmap.getGenerationID()); + return; + } bool encoded = false; - if (fBitmapEncoder != NULL) { - SkDynamicMemoryWStream pngStream; - if (fBitmapEncoder(&pngStream, bitmap)) { + // Before attempting to encode the SkBitmap, check to see if there is already an encoded + // version. + SkPixelRef* ref = bitmap.pixelRef(); + if (ref != NULL) { + SkAutoDataUnref data(ref->refEncodedData()); + if (data.get() != NULL) { + // Write the length to indicate that the bitmap was encoded successfully, followed + // by the actual data. This must match the case where fBitmapEncoder is used so the + // reader need not know the difference. + this->writeUInt(data->size()); + fWriter.writePad(data->data(), data->size()); encoded = true; - if (encoded) { - uint32_t offset = fWriter.bytesWritten(); - // Write the length to indicate that the bitmap was encoded successfully. - size_t length = pngStream.getOffset(); - this->writeUInt(length); - // Now write the stream. - if (pngStream.read(fWriter.reservePad(length), 0, length)) { - // Write the width and height in case the reader does not have a decoder. - this->writeInt(bitmap.width()); - this->writeInt(bitmap.height()); - } else { - // Writing the stream failed, so go back to original state to store another way. - fWriter.rewindToOffset(offset); - encoded = false; - } + } + } + if (fBitmapEncoder != NULL && !encoded) { + SkASSERT(NULL == fBitmapHeap); + SkDynamicMemoryWStream stream; + if (fBitmapEncoder(&stream, bitmap)) { + uint32_t offset = fWriter.bytesWritten(); + // Write the length to indicate that the bitmap was encoded successfully, followed + // by the actual data. This must match the case where the original data is used so the + // reader need not know the difference. + size_t length = stream.getOffset(); + this->writeUInt(length); + if (stream.read(fWriter.reservePad(length), 0, length)) { + encoded = true; + } else { + // Writing the stream failed, so go back to original state to store another way. + fWriter.rewindToOffset(offset); } } } - if (!encoded) { + if (encoded) { + // Write the width and height in case the reader does not have a decoder. + this->writeInt(bitmap.width()); + this->writeInt(bitmap.height()); + } else { // Bitmap was not encoded. Record a zero, implying that the reader need not decode. this->writeUInt(0); - if (fBitmapHeap) { - int32_t slot = fBitmapHeap->insert(bitmap); - fWriter.write32(slot); - // crbug.com/155875 - // The generation ID is not required information. We write it to prevent collisions - // in SkFlatDictionary. It is possible to get a collision when a previously - // unflattened (i.e. stale) instance of a similar flattenable is in the dictionary - // and the instance currently being written is re-using the same slot from the - // bitmap heap. - fWriter.write32(bitmap.getGenerationID()); - } else { - bitmap.flatten(*this); - } + bitmap.flatten(*this); } } @@ -211,6 +240,23 @@ SkRefCntSet* SkOrderedWriteBuffer::setTypefaceRecorder(SkRefCntSet* rec) { return rec; } +void SkOrderedWriteBuffer::setBitmapHeap(SkBitmapHeap* bitmapHeap) { + SkRefCnt_SafeAssign(fBitmapHeap, bitmapHeap); + if (bitmapHeap != NULL) { + SkASSERT(NULL == fBitmapEncoder); + fBitmapEncoder = NULL; + } +} + +void SkOrderedWriteBuffer::setBitmapEncoder(SkSerializationHelpers::EncodeBitmap bitmapEncoder) { + fBitmapEncoder = bitmapEncoder; + if (bitmapEncoder != NULL) { + SkASSERT(NULL == fBitmapHeap); + SkSafeUnref(fBitmapHeap); + fBitmapHeap = NULL; + } +} + void SkOrderedWriteBuffer::writeFlattenable(SkFlattenable* flattenable) { /* * If we have a factoryset, then the first 32bits tell us... diff --git a/src/core/SkOrderedWriteBuffer.h b/src/core/SkOrderedWriteBuffer.h index dd477f71d9..cd37f4721b 100644 --- a/src/core/SkOrderedWriteBuffer.h +++ b/src/core/SkOrderedWriteBuffer.h @@ -75,19 +75,23 @@ public: SkRefCntSet* getTypefaceRecorder() const { return fTFSet; } SkRefCntSet* setTypefaceRecorder(SkRefCntSet*); - void setBitmapHeap(SkBitmapHeap* bitmapHeap) { - SkRefCnt_SafeAssign(fBitmapHeap, bitmapHeap); - } + /** + * Set an SkBitmapHeap to store bitmaps rather than flattening. + * + * Incompatible with an EncodeBitmap function. If an EncodeBitmap function is set, setting an + * SkBitmapHeap will set the function to NULL in release mode and crash in debug. + */ + void setBitmapHeap(SkBitmapHeap*); /** * Provide a function to encode an SkBitmap to an SkStream. writeBitmap will attempt to use - * bitmapEncoder to store the SkBitmap. Takes priority over the SkBitmapHeap. If the reader does - * not provide a function to decode, it will not be able to restore SkBitmaps, but will still be - * able to read the rest of the stream. + * bitmapEncoder to store the SkBitmap. If the reader does not provide a function to decode, it + * will not be able to restore SkBitmaps, but will still be able to read the rest of the stream. + * + * Incompatible with the SkBitmapHeap. If an encoder is set fBitmapHeap will be set to NULL in + * release and crash in debug. */ - void setBitmapEncoder(SkSerializationHelpers::EncodeBitmap bitmapEncoder) { - fBitmapEncoder = bitmapEncoder; - } + void setBitmapEncoder(SkSerializationHelpers::EncodeBitmap); private: SkFactorySet* fFactorySet; diff --git a/tests/PictureTest.cpp b/tests/PictureTest.cpp index e62d5aa959..d14cf98737 100644 --- a/tests/PictureTest.cpp +++ b/tests/PictureTest.cpp @@ -323,6 +323,73 @@ static void test_bad_bitmap() { } #endif +#include "SkData.h" +#include "SkImageRef_GlobalPool.h" +// Class to test SkPixelRef::onRefEncodedData, since there are currently no implementations in skia. +class SkDataImageRef : public SkImageRef_GlobalPool { + +public: + SkDataImageRef(SkMemoryStream* stream) + : SkImageRef_GlobalPool(stream, SkBitmap::kNo_Config) { + SkASSERT(stream != NULL); + fData = stream->copyToData(); + this->setImmutable(); + } + + ~SkDataImageRef() { + fData->unref(); + } + + virtual SkData* onRefEncodedData() SK_OVERRIDE { + fData->ref(); + return fData; + } + +private: + SkData* fData; +}; + +#include "SkImageEncoder.h" + +static bool PNGEncodeBitmapToStream(SkWStream* wStream, const SkBitmap& bm) { + return SkImageEncoder::EncodeStream(wStream, bm, SkImageEncoder::kPNG_Type, 100); +} + +static SkData* serialized_picture_from_bitmap(const SkBitmap& bitmap) { + SkPicture picture; + SkCanvas* canvas = picture.beginRecording(bitmap.width(), bitmap.height()); + canvas->drawBitmap(bitmap, 0, 0); + SkDynamicMemoryWStream wStream; + picture.serialize(&wStream, &PNGEncodeBitmapToStream); + return wStream.copyToData(); +} + +static void test_bitmap_with_encoded_data(skiatest::Reporter* reporter) { + // Create a bitmap that will be encoded. + SkBitmap original; + make_bm(&original, 100, 100, SK_ColorBLUE, true); + SkDynamicMemoryWStream wStream; + if (!SkImageEncoder::EncodeStream(&wStream, original, SkImageEncoder::kPNG_Type, 100)) { + return; + } + SkAutoDataUnref data(wStream.copyToData()); + SkMemoryStream memStream; + memStream.setData(data); + + // Use the encoded bitmap as the data for an image ref. + SkBitmap bm; + SkAutoTUnref imageRef(SkNEW_ARGS(SkDataImageRef, (&memStream))); + imageRef->getInfo(&bm); + bm.setPixelRef(imageRef); + + // Write both bitmaps to pictures, and ensure that the resulting data streams are the same. + // Flattening original will follow the old path of performing an encode, while flattening bm + // will use the already encoded data. + SkAutoDataUnref picture1(serialized_picture_from_bitmap(original)); + SkAutoDataUnref picture2(serialized_picture_from_bitmap(bm)); + REPORTER_ASSERT(reporter, picture1->equals(picture2)); +} + static void TestPicture(skiatest::Reporter* reporter) { #ifdef SK_DEBUG test_deleting_empty_playback(); @@ -332,6 +399,7 @@ static void TestPicture(skiatest::Reporter* reporter) { #endif test_peephole(reporter); test_gatherpixelrefs(reporter); + test_bitmap_with_encoded_data(reporter); } #include "TestClassDef.h"