Fixes for piping bitmaps with encoded data.

Similar goals as https://codereview.chromium.org/14437012.

Builds on patch set 1 from that issue
(https://codereview.chromium.org/14437012/#ps1).

Instead of the changes in patch set 2 from that issue, this
changes SkOrderedWriteBuffer::writeBitmap to store whether an
SkBitmapHeap was used when to store the index of the SkBitmap.
SkOrderedReadBuffer::readBitmap now uses that information to
distinguish between using the heap and unflattening.

In addition, writeBitmap now records the width/height first in
all cases. If now SkBitmapHeapReader is attached, but an
SkBitmapHeap was used to record the bitmap, reading will fail
and provide the same red SkBitmap as in the case where the
SkBitmap was encoded but could not be decoded.

Updates the PICTURE_VERSION as well.

The key differences in this CL to look at are in:
SkOrderedWriteBuffer,
SkOrderedReadBuffer,
and SkPicture.

BUG=
R=djsollen@google.com

Review URL: https://codereview.chromium.org/14230022

git-svn-id: http://skia.googlecode.com/svn/trunk@8917 2bbb7eff-a529-9590-31e7-b0007b416f81

10 files changed, 108 insertions, 50 deletions

diff --git a/gm/gmmain.cpp b/gm/gmmain.cpp
index 420b684956..f3d6ee86e3 100644
--- a/gm/gmmain.cpp
+++ b/gm/gmmain.cpp
@@ -946,7 +946,10 @@ public:
         //@todo thudson 22 April 2011 when can we safely delete [] dst?
         storage.copyTo(dst);
         SkMemoryStream pictReadback(dst, streamSize);
-        SkPicture* retval = new SkPicture (&pictReadback);
+        bool success;
+        // Pass a decoding bitmap function so that the factory GM (which has an SkBitmap with
+        // encoded data) does not fail.
+        SkPicture* retval = new SkPicture (&pictReadback, &success, &SkImageDecoder::DecodeMemory);
         return retval;
     }
 
@@ -1037,7 +1040,9 @@ public:
                 setup_bitmap(gRec, size, &bitmap);
                 SkCanvas canvas(bitmap);
                 installFilter(&canvas);
-                PipeController pipeController(&canvas);
+                // Pass a decoding function so the factory GM (which has an SkBitmap
+                // with encoded data) will not fail playback.
+                PipeController pipeController(&canvas, &SkImageDecoder::DecodeMemory);
                 SkGPipeWriter writer;
                 SkCanvas* pipeCanvas = writer.startRecording(&pipeController,
                                                              gPipeWritingFlagCombos[i].flags,
@@ -1076,7 +1081,7 @@ public:
                 setup_bitmap(gRec, size, &bitmap);
                 SkCanvas canvas(bitmap);
                 installFilter(&canvas);
-                TiledPipeController pipeController(bitmap);
+                TiledPipeController pipeController(bitmap, &SkImageDecoder::DecodeMemory);
                 SkGPipeWriter writer;
                 SkCanvas* pipeCanvas = writer.startRecording(&pipeController,
                                                              gPipeWritingFlagCombos[i].flags,
diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h
index be682b115d..b2c2b62ede 100644
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -203,7 +203,8 @@ protected:
     // V9 : Allow the reader and writer of an SKP disagree on whether to support
     //      SK_SUPPORT_HINTING_SCALE_FACTOR
     // V10: add drawRRect, drawOval, clipRRect
-    static const uint32_t PICTURE_VERSION = 10;
+    // V11: modify how readBitmap and writeBitmap store their info.
+    static const uint32_t PICTURE_VERSION = 11;
 
     // fPlayback, fRecord, fWidth & fHeight are protected to allow derived classes to
     // install their own SkPicturePlayback-derived players,SkPictureRecord-derived
diff --git a/include/pipe/SkGPipe.h b/include/pipe/SkGPipe.h
index 27af16db8c..879ab043ff 100644
--- a/include/pipe/SkGPipe.h
+++ b/include/pipe/SkGPipe.h
@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *
@@ -11,8 +10,9 @@
 #ifndef SkGPipe_DEFINED
 #define SkGPipe_DEFINED
 
-#include "SkWriter32.h"
 #include "SkFlattenable.h"
+#include "SkPicture.h"
+#include "SkWriter32.h"
 
 class SkCanvas;
 
@@ -40,13 +40,20 @@ public:
     };
 
     void setCanvas(SkCanvas*);
+
+    /**
+     *  Set a function for decoding bitmaps that have encoded data.
+     */
+    void setBitmapDecoder(SkPicture::InstallPixelRefProc proc) { fProc = proc; }
+
     // data must be 4-byte aligned
     // length must be a multiple of 4
     Status playback(const void* data, size_t length, uint32_t playbackFlags = 0,
                     size_t* bytesRead = NULL);
 private:
-    SkCanvas*           fCanvas;
-    class SkGPipeState* fState;
+    SkCanvas*                       fCanvas;
+    class SkGPipeState*             fState;
+    SkPicture::InstallPixelRefProc  fProc;
 };
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/samplecode/SampleApp.cpp b/samplecode/SampleApp.cpp
index badc7c01e8..10106323e2 100644
--- a/samplecode/SampleApp.cpp
+++ b/samplecode/SampleApp.cpp
@@ -10,6 +10,7 @@
 #include "SkCanvas.h"
 #include "SkDevice.h"
 #include "SkGraphics.h"
+#include "SkImageDecoder.h"
 #include "SkImageEncoder.h"
 #include "SkPaint.h"
 #include "SkPicture.h"
@@ -2203,6 +2204,7 @@ SimplePC::SimplePC(SkCanvas* target) : fReader(target) {
     fStatus = SkGPipeReader::kDone_Status;
     fTotalWritten = 0;
     fAtomsWritten = 0;
+    fReader.setBitmapDecoder(&SkImageDecoder::DecodeMemory);
 }
 
 SimplePC::~SimplePC() {
@@ -2254,6 +2256,7 @@ void SampleView::draw(SkCanvas* canvas) {
         SkGPipeWriter writer;
         SimplePC controller(canvas);
         TiledPipeController tc(canvas->getDevice()->accessBitmap(false),
+                               &SkImageDecoder::DecodeMemory,
                                &canvas->getTotalMatrix());
         SkGPipeController* pc;
         if (SkOSMenu::kMixedState == fPipeState) {
diff --git a/src/core/SkOrderedReadBuffer.cpp b/src/core/SkOrderedReadBuffer.cpp
index 5a4cf96a92..85491c5607 100644
--- a/src/core/SkOrderedReadBuffer.cpp
+++ b/src/core/SkOrderedReadBuffer.cpp
@@ -169,33 +169,51 @@ uint32_t SkOrderedReadBuffer::getArrayCount() {
 }
 
 void SkOrderedReadBuffer::readBitmap(SkBitmap* bitmap) {
-    const size_t length = this->readUInt();
-    if (length > 0) {
-        // Bitmap was encoded.
-        const void* data = this->skip(length);
-        const int width = this->readInt();
-        const int height = this->readInt();
-        if (fBitmapDecoder != NULL && fBitmapDecoder(data, length, bitmap)) {
-            SkASSERT(bitmap->width() == width && bitmap->height() == height);
+    const int width = this->readInt();
+    const int height = this->readInt();
+    // The writer stored a boolean value to determine whether an SkBitmapHeap was used during
+    // writing.
+    if (this->readBool()) {
+        // An SkBitmapHeap was used for writing. Read the index from the stream and find the
+        // corresponding SkBitmap in fBitmapStorage.
+        const uint32_t index = fReader.readU32();
+        fReader.readU32(); // bitmap generation ID (see SkOrderedWriteBuffer::writeBitmap)
+        if (fBitmapStorage) {
+            *bitmap = *fBitmapStorage->getBitmap(index);
+            fBitmapStorage->releaseRef(index);
+            return;
         } else {
+            // The bitmap was stored in a heap, but there is no way to access it. Set an error and
+            // fall through to use a place holder bitmap.
+            SkErrorInternals::SetError(kParseError_SkError, "SkOrderedWriteBuffer::writeBitmap "
+                                       "stored the SkBitmap in an SkBitmapHeap, but "
+                                       "SkOrderedReadBuffer has no SkBitmapHeapReader to "
+                                       "retrieve the SkBitmap.");
+        }
+    } else {
+        // The writer stored false, meaning the SkBitmap was not stored in an SkBitmapHeap.
+        const size_t length = this->readUInt();
+        if (length > 0) {
+            // A non-zero size means the SkBitmap was encoded.
+            const void* data = this->skip(length);
+            if (fBitmapDecoder != NULL && fBitmapDecoder(data, length, bitmap)) {
+                SkASSERT(bitmap->width() == width && bitmap->height() == height);
+                return;
+            }
             // This bitmap was encoded when written, but we are unable to decode, possibly due to
-            // not having a decoder. Use a placeholder bitmap.
+            // not having a decoder.
             SkErrorInternals::SetError(kParseError_SkError,
                                        "Could not decode bitmap. Resulting bitmap will be red.");
-            bitmap->setConfig(SkBitmap::kARGB_8888_Config, width, height);
-            bitmap->allocPixels();
-            bitmap->eraseColor(SK_ColorRED);
-        }
-    } else {
-        if (fBitmapStorage) {
-            const uint32_t index = fReader.readU32();
-            fReader.readU32(); // bitmap generation ID (see SkOrderedWriteBuffer::writeBitmap)
-            *bitmap = *fBitmapStorage->getBitmap(index);
-            fBitmapStorage->releaseRef(index);
         } else {
+            // A size of zero means the SkBitmap was simply flattened.
             bitmap->unflatten(*this);
+            return;
         }
     }
+    // Could not read the SkBitmap. Use a placeholder bitmap.
+    bitmap->setConfig(SkBitmap::kARGB_8888_Config, width, height);
+    bitmap->allocPixels();
+    bitmap->eraseColor(SK_ColorRED);
 }
 
 SkTypeface* SkOrderedReadBuffer::readTypeface() {
diff --git a/src/core/SkOrderedWriteBuffer.cpp b/src/core/SkOrderedWriteBuffer.cpp
index e458bfe597..729396c07d 100644
--- a/src/core/SkOrderedWriteBuffer.cpp
+++ b/src/core/SkOrderedWriteBuffer.cpp
@@ -145,20 +145,28 @@ bool SkOrderedWriteBuffer::writeToStream(SkWStream* stream) {
 }
 
 void SkOrderedWriteBuffer::writeBitmap(const SkBitmap& bitmap) {
+    // Record the width and height. This way if readBitmap fails a dummy bitmap can be drawn at the
+    // right size.
+    this->writeInt(bitmap.width());
+    this->writeInt(bitmap.height());
+
     // 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.
+    //    bitmap entirely or serialize it later as desired. A boolean value of true will be written
+    //    to the stream to signify that a heap was used.
+    // 2. Write an encoded version of the bitmap. After writing a boolean value of false, signifying
+    //    that a heap was not used, write the size of the encoded data. A non-zero size signifies
+    //    that encoded data was written.
     //    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) {
+    // 3. Call SkBitmap::flatten. After writing a boolean value of false, signifying that a heap was
+    //    not used, write a zero to signify that the data was not encoded.
+    bool useBitmapHeap = fBitmapHeap != NULL;
+    // Write a bool: true if the SkBitmapHeap is to be used, in which case the reader must use an
+    // SkBitmapHeapReader to read the SkBitmap. False if the bitmap was serialized another way.
+    this->writeBool(useBitmapHeap);
+    if (useBitmapHeap) {
         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
@@ -180,7 +188,7 @@ void SkOrderedWriteBuffer::writeBitmap(const SkBitmap& bitmap) {
             // 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());
+            this->writeUInt(SkToU32(data->size()));
             fWriter.writePad(data->data(), data->size());
             encoded = true;
         }
@@ -194,7 +202,7 @@ void SkOrderedWriteBuffer::writeBitmap(const SkBitmap& bitmap) {
             // 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);
+            this->writeUInt(SkToU32(length));
             if (stream.read(fWriter.reservePad(length), 0, length)) {
                 encoded = true;
             } else {
@@ -203,11 +211,7 @@ void SkOrderedWriteBuffer::writeBitmap(const SkBitmap& bitmap) {
             }
         }
     }
-    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 {
+    if (!encoded) {
         // Bitmap was not encoded. Record a zero, implying that the reader need not decode.
         this->writeUInt(0);
         bitmap.flatten(*this);
diff --git a/src/pipe/SkGPipeRead.cpp b/src/pipe/SkGPipeRead.cpp
index f47f3bfe65..3f9ce12db7 100644
--- a/src/pipe/SkGPipeRead.cpp
+++ b/src/pipe/SkGPipeRead.cpp
@@ -124,7 +124,12 @@ public:
         }
     }
 
+    /**
+     * Add a bitmap to the array of bitmaps, or replace an existing one.
+     * This is only used when in cross process mode without a shared heap.
+     */
     void addBitmap(int index) {
+        SkASSERT(shouldFlattenBitmaps(fFlags));
         SkBitmap* bm;
         if(fBitmaps.count() == index) {
             bm = SkNEW(SkBitmap);
@@ -132,14 +137,16 @@ public:
         } else {
             bm = fBitmaps[index];
         }
-        bm->unflatten(*fReader);
+        fReader->readBitmap(bm);
     }
 
     /**
      * Override of SkBitmapHeapReader, so that SkOrderedReadBuffer can use
-     * these SkBitmaps for bitmap shaders.
+     * these SkBitmaps for bitmap shaders. Used only in cross process mode
+     * without a shared heap.
      */
     virtual SkBitmap* getBitmap(int32_t index) const SK_OVERRIDE {
+        SkASSERT(shouldFlattenBitmaps(fFlags));
         return fBitmaps[index];
     }
 
@@ -154,7 +161,12 @@ public:
         this->updateReader();
     }
 
+    /**
+     * Access the shared heap. Only used in the case when bitmaps are not
+     * flattened.
+     */
     SkBitmapHeap* getSharedHeap() const {
+        SkASSERT(!shouldFlattenBitmaps(fFlags));
         return fSharedHeap;
     }
 
@@ -772,12 +784,14 @@ SkGPipeState::~SkGPipeState() {
 SkGPipeReader::SkGPipeReader() {
     fCanvas = NULL;
     fState = NULL;
+    fProc = NULL;
 }
 
 SkGPipeReader::SkGPipeReader(SkCanvas* target) {
     fCanvas = NULL;
     this->setCanvas(target);
     fState = NULL;
+    fProc = NULL;
 }
 
 void SkGPipeReader::setCanvas(SkCanvas *target) {
@@ -805,6 +819,7 @@ SkGPipeReader::Status SkGPipeReader::playback(const void* data, size_t length,
 
     const ReadProc* table = gReadTable;
     SkOrderedReadBuffer reader(data, length);
+    reader.setBitmapDecoder(fProc);
     SkCanvas* canvas = fCanvas;
     Status status = kEOF_Status;
 
diff --git a/src/pipe/SkGPipeWrite.cpp b/src/pipe/SkGPipeWrite.cpp
index 34020dfe7f..32786fdc80 100644
--- a/src/pipe/SkGPipeWrite.cpp
+++ b/src/pipe/SkGPipeWrite.cpp
@@ -343,7 +343,7 @@ bool SkGPipeCanvas::shuttleBitmap(const SkBitmap& bm, int32_t slot) {
     SkASSERT(shouldFlattenBitmaps(fFlags));
     SkOrderedWriteBuffer buffer(1024);
     buffer.setNamedFactoryRecorder(fFactorySet);
-    bm.flatten(buffer);
+    buffer.writeBitmap(bm);
     this->flattenFactoryNames();
     uint32_t size = buffer.size();
     if (this->needOpBytes(size)) {
diff --git a/src/pipe/utils/SamplePipeControllers.cpp b/src/pipe/utils/SamplePipeControllers.cpp
index 10e4ea0f0a..a23d77537c 100644
--- a/src/pipe/utils/SamplePipeControllers.cpp
+++ b/src/pipe/utils/SamplePipeControllers.cpp
@@ -12,10 +12,11 @@
 #include "SkGPipe.h"
 #include "SkMatrix.h"
 
-PipeController::PipeController(SkCanvas* target)
+PipeController::PipeController(SkCanvas* target, SkPicture::InstallPixelRefProc proc)
 :fReader(target) {
     fBlock = NULL;
     fBlockSize = fBytesWritten = 0;
+    fReader.setBitmapDecoder(proc);
 }
 
 PipeController::~PipeController() {
@@ -40,8 +41,9 @@ void PipeController::notifyWritten(size_t bytes) {
 ////////////////////////////////////////////////////////////////////////////////
 
 TiledPipeController::TiledPipeController(const SkBitmap& bitmap,
+                                         SkPicture::InstallPixelRefProc proc,
                                          const SkMatrix* initial)
-: INHERITED(NULL) {
+: INHERITED(NULL, proc) {
     int32_t top = 0;
     int32_t bottom;
     int32_t height = bitmap.height() / NumberOfTiles;
@@ -65,6 +67,7 @@ TiledPipeController::TiledPipeController(const SkBitmap& bitmap,
             fReader.setCanvas(canvas);
         } else {
             fReaders[i - 1].setCanvas(canvas);
+            fReaders[i - 1].setBitmapDecoder(proc);
         }
         canvas->unref();
     }
diff --git a/src/pipe/utils/SamplePipeControllers.h b/src/pipe/utils/SamplePipeControllers.h
index 5efd6f0b9f..35cfba73b6 100644
--- a/src/pipe/utils/SamplePipeControllers.h
+++ b/src/pipe/utils/SamplePipeControllers.h
@@ -8,6 +8,7 @@
 #include "SkBitmap.h"
 #include "SkChunkAlloc.h"
 #include "SkGPipe.h"
+#include "SkPicture.h"
 #include "SkTDArray.h"
 
 class SkCanvas;
@@ -15,7 +16,7 @@ class SkMatrix;
 
 class PipeController : public SkGPipeController {
 public:
-    PipeController(SkCanvas* target);
+    PipeController(SkCanvas* target, SkPicture::InstallPixelRefProc proc = NULL);
     virtual ~PipeController();
     virtual void* requestBlock(size_t minRequest, size_t* actual) SK_OVERRIDE;
     virtual void notifyWritten(size_t bytes) SK_OVERRIDE;
@@ -33,7 +34,8 @@ private:
 
 class TiledPipeController : public PipeController {
 public:
-    TiledPipeController(const SkBitmap&, const SkMatrix* initialMatrix = NULL);
+    TiledPipeController(const SkBitmap&, SkPicture::InstallPixelRefProc proc = NULL,
+                        const SkMatrix* initialMatrix = NULL);
     virtual ~TiledPipeController() {};
     virtual void notifyWritten(size_t bytes) SK_OVERRIDE;
     virtual int numberOfReaders() const SK_OVERRIDE { return NumberOfTiles; }