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

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<SkDataImageRef> 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"