Replace EncodeBitmap with an interface.

Gives more flexibility to the caller to decide whether to use the
encoded data returned by refEncodedData().

Provides an implementation that supports the old version of
SkPicture::serialize().

TODO: Update Chrome, so we can remove SK_LEGACY_ENCODE_BITMAP entirely

BUG=skia:3190

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

12 files changed, 201 insertions, 56 deletions

diff --git a/dm/DMSerializeTask.cpp b/dm/DMSerializeTask.cpp
index d8b460dffb..a3e2503c6d 100644
--- a/dm/DMSerializeTask.cpp
+++ b/dm/DMSerializeTask.cpp
@@ -21,7 +21,7 @@ void SerializeTask::draw() {
     SkAutoTUnref<SkPicture> recorded(RecordPicture(fGM.get(), NULL/*no BBH*/));
 
     SkDynamicMemoryWStream wStream;
-    recorded->serialize(&wStream, NULL);
+    recorded->serialize(&wStream);
     SkAutoTUnref<SkStream> rStream(wStream.detachAsStream());
     SkAutoTUnref<SkPicture> reconstructed(SkPicture::CreateFromStream(rStream));
 
diff --git a/gm/gmmain.cpp b/gm/gmmain.cpp
index 3eb1247800..46b039d164 100644
--- a/gm/gmmain.cpp
+++ b/gm/gmmain.cpp
@@ -1039,7 +1039,7 @@ public:
 
     static SkPicture* stream_to_new_picture(const SkPicture& src) {
         SkDynamicMemoryWStream storage;
-        src.serialize(&storage, NULL);
+        src.serialize(&storage);
         SkAutoTUnref<SkStreamAsset> pictReadback(storage.detachAsStream());
         SkPicture* retval = SkPicture::CreateFromStream(pictReadback,
                                                         &SkImageDecoder::DecodeMemory);
diff --git a/gyp/skia_for_chromium_defines.gypi b/gyp/skia_for_chromium_defines.gypi
index e2e85acdff..2bc8584cb5 100644
--- a/gyp/skia_for_chromium_defines.gypi
+++ b/gyp/skia_for_chromium_defines.gypi
@@ -16,6 +16,8 @@
       'SK_SUPPORT_LEGACY_TEXTRENDERMODE',
       'SK_IGNORE_GPU_LAYER_HOISTING',
       'SK_SUPPORT_LEGACY_NewRasterPMColor',
+      # Transition for skbug.com/3190
+      'SK_LEGACY_ENCODE_BITMAP',
     ],
   },
 }
diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h
index eb136e14e4..35458095da 100644
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -24,6 +24,7 @@ class SkBBoxHierarchy;
 class SkCanvas;
 class SkData;
 class SkPictureData;
+class SkPixelSerializer;
 class SkStream;
 class SkWStream;
 
@@ -35,6 +36,8 @@ namespace SkRecords {
     class CollectLayers;
 };
 
+//#define SK_LEGACY_ENCODE_BITMAP
+
 /** \class SkPicture
 
     The SkPicture class records the drawing commands made to a canvas, to
@@ -141,15 +144,30 @@ public:
      *  @param pixelRefOffset DEPRECATED -- caller assumes it will return 0.
      *  @return SkData If non-NULL, holds encoded data representing the passed
      *      in bitmap. The caller is responsible for calling unref().
+     *
+     *  TODO: No longer used by SkPicture (except when SK_LEGACY_ENCODE_BITMAP
+     *  is defined. Still used by PDF though. Move into PDF.
      */
     typedef SkData* (*EncodeBitmap)(size_t* pixelRefOffset, const SkBitmap& bm);
 
+#ifdef SK_LEGACY_ENCODE_BITMAP
     /**
      *  Serialize to a stream. If non NULL, encoder will be used to encode
      *  any bitmaps in the picture.
      *  encoder will never be called with a NULL pixelRefOffset.
+     *  DEPRECATED - use serialize(SkWStream*, SkPixelSerializer* serializer)
+     *  instead.
+     */
+    void serialize(SkWStream* wStream, EncodeBitmap encoder) const;
+#endif
+
+    /**
+     *  Serialize to a stream. If non NULL, serializer will be used to serialize
+     *  any bitmaps in the picture.
+     *
+     *  TODO: Use serializer to serialize SkImages as well.
      */
-    void serialize(SkWStream*, EncodeBitmap encoder = NULL) const;
+    void serialize(SkWStream*, SkPixelSerializer* serializer = NULL) const;
 
     /**
      *  Serialize to a buffer.
diff --git a/include/core/SkPixelSerializer.h b/include/core/SkPixelSerializer.h
new file mode 100644
index 0000000000..8fc445c753
--- /dev/null
+++ b/include/core/SkPixelSerializer.h
@@ -0,0 +1,52 @@
+/*
+ * Copyright 2014 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkPixelSerializer_DEFINED
+#define SkPixelSerializer_DEFINED
+
+#include "SkRefCnt.h"
+
+class SkData;
+struct SkImageInfo;
+
+/**
+ *  Interface for serializing pixels, e.g. SkBitmaps in an SkPicture.
+ */
+class SkPixelSerializer : public SkRefCnt {
+public:
+    virtual ~SkPixelSerializer() {}
+
+    /**
+     *  Call to determine if the client wants to serialize the encoded data. If
+     *  false, serialize another version (e.g. the result of encodePixels).
+     */
+    bool useEncodedData(const void* data, size_t len) {
+        return this->onUseEncodedData(data, len);
+    }
+
+    /**
+     *  Call to get the client's version of encoding these pixels. If it
+     *  returns NULL, serialize the raw pixels.
+     */
+    SkData* encodePixels(const SkImageInfo& info, void* pixels, size_t rowBytes) {
+        return this->onEncodePixels(info, pixels, rowBytes);
+    }
+
+protected:
+    /**
+     *  Return true if you want to serialize the encoded data, false if you want
+     *  another version serialized (e.g. the result of encodePixels).
+     */
+    virtual bool onUseEncodedData(const void* data, size_t len) = 0;
+
+    /**
+     *  If you want to encode these pixels, return the encoded data as an SkData
+     *  Return null if you want to serialize the raw pixels.
+     */
+    virtual SkData* onEncodePixels(const SkImageInfo&, void* pixels, size_t rowBytes) = 0;
+};
+#endif // SkPixelSerializer_DEFINED
diff --git a/include/core/SkWriteBuffer.h b/include/core/SkWriteBuffer.h
index 4dbe17b0d9..39739f2c24 100644
--- a/include/core/SkWriteBuffer.h
+++ b/include/core/SkWriteBuffer.h
@@ -12,6 +12,7 @@
 #include "SkData.h"
 #include "SkPath.h"
 #include "SkPicture.h"
+#include "SkPixelSerializer.h"
 #include "SkRefCnt.h"
 #include "SkWriter32.h"
 
@@ -87,21 +88,24 @@ public:
     /**
      * 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.
+     * Incompatible with an SkPixelSerializer. If an SkPixelSerializer is set,
+     * setting an SkBitmapHeap will set the SkPixelSerializer to NULL in release
+     * and crash in debug.
      */
     void setBitmapHeap(SkBitmapHeap*);
 
     /**
-     * Provide a function to encode an SkBitmap to an SkData. writeBitmap will attempt to use
-     * 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.
-     * bitmapEncoder will never be called with a NULL pixelRefOffset.
+     * Set an SkPixelSerializer to store an encoded representation of pixels,
+     * e.g. SkBitmaps.
      *
-     * Incompatible with the SkBitmapHeap. If an encoder is set fBitmapHeap will be set to NULL in
-     * release and crash in debug.
+     * Calls ref() on the serializer.
+     *
+     * TODO: Encode SkImage pixels as well.
+     *
+     * Incompatible with the SkBitmapHeap. If an encoder is set fBitmapHeap will
+     * be set to NULL in release and crash in debug.
      */
-    void setBitmapEncoder(SkPicture::EncodeBitmap bitmapEncoder);
+    void setPixelSerializer(SkPixelSerializer*);
 
 private:
     bool isValidating() const { return SkToBool(fFlags & kValidation_Flag); }
@@ -114,7 +118,7 @@ private:
     SkBitmapHeap* fBitmapHeap;
     SkRefCntSet* fTFSet;
 
-    SkPicture::EncodeBitmap fBitmapEncoder;
+    SkAutoTUnref<SkPixelSerializer> fPixelSerializer;
 };
 
 #endif // SkWriteBuffer_DEFINED
diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index 0d7773b520..3296264be8 100644
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2007 The Android Open Source Project
  *
@@ -454,7 +453,41 @@ SkPictureData* SkPicture::Backport(const SkRecord& src, const SkPictInfo& info,
     return SkNEW_ARGS(SkPictureData, (rec, info, false/*deep copy ops?*/));
 }
 
-void SkPicture::serialize(SkWStream* stream, EncodeBitmap encoder) const {
+#ifdef SK_LEGACY_ENCODE_BITMAP
+// Helper to support the EncodeBitmap version of serialize.
+// Mimics the old behavior of always accepting the encoded data, and encoding
+// using EncodeBitmap if there was no encoded data.
+class EncodeBitmapSerializer : public SkPixelSerializer {
+public:
+    explicit EncodeBitmapSerializer(SkPicture::EncodeBitmap encoder)
+    : fEncoder(encoder)
+    {
+        SkASSERT(fEncoder);
+    }
+
+    virtual bool onUseEncodedData(const void*, size_t) SK_OVERRIDE { return true; }
+
+    virtual SkData* onEncodePixels(const SkImageInfo& info, void* pixels,
+                                   size_t rowBytes) SK_OVERRIDE {
+        // Required by signature of EncodeBitmap.
+        size_t unused;
+        SkBitmap bm;
+        bm.installPixels(info, pixels, rowBytes);
+        return fEncoder(&unused, bm);
+    }
+
+private:
+    SkPicture::EncodeBitmap fEncoder;
+};
+
+void SkPicture::serialize(SkWStream* wStream, SkPicture::EncodeBitmap encoder) const {
+    EncodeBitmapSerializer serializer(encoder);
+    this->serialize(wStream, &serializer);
+}
+
+#endif
+
+void SkPicture::serialize(SkWStream* stream, SkPixelSerializer* pixelSerializer) const {
     SkPictInfo info;
     this->createHeader(&info);
     SkAutoTDelete<SkPictureData> data(Backport(*fRecord, info, this->drawablePicts(),
@@ -463,7 +496,7 @@ void SkPicture::serialize(SkWStream* stream, EncodeBitmap encoder) const {
     stream->write(&info, sizeof(info));
     if (data) {
         stream->writeBool(true);
-        data->serialize(stream, encoder);
+        data->serialize(stream, pixelSerializer);
     } else {
         stream->writeBool(false);
     }
diff --git a/src/core/SkPictureData.cpp b/src/core/SkPictureData.cpp
index 2b2265403f..938274aceb 100644
--- a/src/core/SkPictureData.cpp
+++ b/src/core/SkPictureData.cpp
@@ -218,14 +218,14 @@ void SkPictureData::flattenToBuffer(SkWriteBuffer& buffer) const {
 }
 
 void SkPictureData::serialize(SkWStream* stream,
-                                  SkPicture::EncodeBitmap encoder) const {
+                              SkPixelSerializer* pixelSerializer) const {
     write_tag_size(stream, SK_PICT_READER_TAG, fOpData->size());
     stream->write(fOpData->bytes(), fOpData->size());
 
     if (fPictureCount > 0) {
         write_tag_size(stream, SK_PICT_PICTURE_TAG, fPictureCount);
         for (int i = 0; i < fPictureCount; i++) {
-            fPictureRefs[i]->serialize(stream, encoder);
+            fPictureRefs[i]->serialize(stream, pixelSerializer);
         }
     }
 
@@ -238,7 +238,7 @@ void SkPictureData::serialize(SkWStream* stream,
         SkWriteBuffer buffer(SkWriteBuffer::kCrossProcess_Flag);
         buffer.setTypefaceRecorder(&typefaceSet);
         buffer.setFactoryRecorder(&factSet);
-        buffer.setBitmapEncoder(encoder);
+        buffer.setPixelSerializer(pixelSerializer);
 
         this->flattenToBuffer(buffer);
 
diff --git a/src/core/SkPictureData.h b/src/core/SkPictureData.h
index 667880675b..ab78f8a313 100644
--- a/src/core/SkPictureData.h
+++ b/src/core/SkPictureData.h
@@ -15,6 +15,7 @@
 
 class SkData;
 class SkPictureRecord;
+class SkPixelSerializer;
 class SkReader32;
 class SkStream;
 class SkWStream;
@@ -64,7 +65,7 @@ public:
 
     virtual ~SkPictureData();
 
-    void serialize(SkWStream*, SkPicture::EncodeBitmap) const;
+    void serialize(SkWStream*, SkPixelSerializer*) const;
     void flatten(SkWriteBuffer&) const;
 
     bool containsBitmaps() const;
diff --git a/src/core/SkWriteBuffer.cpp b/src/core/SkWriteBuffer.cpp
index c79d2758c7..6048c9f5c2 100644
--- a/src/core/SkWriteBuffer.cpp
+++ b/src/core/SkWriteBuffer.cpp
@@ -20,8 +20,7 @@ SkWriteBuffer::SkWriteBuffer(uint32_t flags)
     , fFactorySet(NULL)
     , fNamedFactorySet(NULL)
     , fBitmapHeap(NULL)
-    , fTFSet(NULL)
-    , fBitmapEncoder(NULL) {
+    , fTFSet(NULL) {
 }
 
 SkWriteBuffer::SkWriteBuffer(void* storage, size_t storageSize, uint32_t flags)
@@ -30,8 +29,7 @@ SkWriteBuffer::SkWriteBuffer(void* storage, size_t storageSize, uint32_t flags)
     , fNamedFactorySet(NULL)
     , fWriter(storage, storageSize)
     , fBitmapHeap(NULL)
-    , fTFSet(NULL)
-    , fBitmapEncoder(NULL) {
+    , fTFSet(NULL) {
 }
 
 SkWriteBuffer::~SkWriteBuffer() {
@@ -172,7 +170,7 @@ void SkWriteBuffer::writeBitmap(const SkBitmap& bitmap) {
     // SkBitmapHeapReader to read the SkBitmap. False if the bitmap was serialized another way.
     this->writeBool(useBitmapHeap);
     if (useBitmapHeap) {
-        SkASSERT(NULL == fBitmapEncoder);
+        SkASSERT(NULL == fPixelSerializer);
         int32_t slot = fBitmapHeap->insert(bitmap);
         fWriter.write32(slot);
         // crbug.com/155875
@@ -185,25 +183,33 @@ void SkWriteBuffer::writeBitmap(const SkBitmap& bitmap) {
         return;
     }
 
-    // see if the pixelref already has an encoded version
-    if (bitmap.pixelRef()) {
-        SkAutoDataUnref data(bitmap.pixelRef()->refEncodedData());
-        if (data.get() != NULL) {
-            write_encoded_bitmap(this, data, bitmap.pixelRefOrigin());
-            return;
+    SkPixelRef* pixelRef = bitmap.pixelRef();
+    if (pixelRef) {
+        // see if the pixelref already has an encoded version
+        SkAutoDataUnref existingData(pixelRef->refEncodedData());
+        if (existingData.get() != NULL) {
+            // Assumes that if the client did not set a serializer, they are
+            // happy to get the encoded data.
+            if (!fPixelSerializer || fPixelSerializer->useEncodedData(existingData->data(),
+                                                                      existingData->size())) {
+                write_encoded_bitmap(this, existingData, bitmap.pixelRefOrigin());
+                return;
+            }
         }
-    }
 
-    // see if the caller wants to manually encode
-    if (fBitmapEncoder != NULL) {
-        SkASSERT(NULL == fBitmapHeap);
-        size_t offset = 0;  // this parameter is deprecated/ignored
-        // if we have to "encode" the bitmap, then we assume there is no
-        // offset to share, since we are effectively creating a new pixelref
-        SkAutoDataUnref data(fBitmapEncoder(&offset, bitmap));
-        if (data.get() != NULL) {
-            write_encoded_bitmap(this, data, SkIPoint::Make(0, 0));
-            return;
+        // see if the caller wants to manually encode
+        if (fPixelSerializer) {
+            SkASSERT(NULL == fBitmapHeap);
+            SkAutoLockPixels alp(bitmap);
+            SkAutoDataUnref data(fPixelSerializer->encodePixels(bitmap.info(),
+                                                                bitmap.getPixels(),
+                                                                bitmap.rowBytes()));
+            if (data.get() != NULL) {
+                // if we have to "encode" the bitmap, then we assume there is no
+                // offset to share, since we are effectively creating a new pixelref
+                write_encoded_bitmap(this, data, SkIPoint::Make(0, 0));
+                return;
+            }
         }
     }
 
@@ -245,14 +251,15 @@ SkRefCntSet* SkWriteBuffer::setTypefaceRecorder(SkRefCntSet* rec) {
 void SkWriteBuffer::setBitmapHeap(SkBitmapHeap* bitmapHeap) {
     SkRefCnt_SafeAssign(fBitmapHeap, bitmapHeap);
     if (bitmapHeap != NULL) {
-        SkASSERT(NULL == fBitmapEncoder);
-        fBitmapEncoder = NULL;
+        SkASSERT(NULL == fPixelSerializer);
+        fPixelSerializer.reset(NULL);
     }
 }
 
-void SkWriteBuffer::setBitmapEncoder(SkPicture::EncodeBitmap bitmapEncoder) {
-    fBitmapEncoder = bitmapEncoder;
-    if (bitmapEncoder != NULL) {
+void SkWriteBuffer::setPixelSerializer(SkPixelSerializer* serializer) {
+    fPixelSerializer.reset(serializer);
+    if (serializer) {
+        serializer->ref();
         SkASSERT(NULL == fBitmapHeap);
         SkSafeUnref(fBitmapHeap);
         fBitmapHeap = NULL;
diff --git a/tests/PictureTest.cpp b/tests/PictureTest.cpp
index b4c2a74469..2881e5f5a8 100644
--- a/tests/PictureTest.cpp
+++ b/tests/PictureTest.cpp
@@ -22,6 +22,7 @@
 #include "SkPictureRecorder.h"
 #include "SkPictureUtils.h"
 #include "SkPixelRef.h"
+#include "SkPixelSerializer.h"
 #include "SkRRect.h"
 #include "SkRandom.h"
 #include "SkRecord.h"
@@ -1442,9 +1443,21 @@ static void test_bad_bitmap() {
 }
 #endif
 
-static SkData* encode_bitmap_to_data(size_t*, const SkBitmap& bm) {
-    return SkImageEncoder::EncodeData(bm, SkImageEncoder::kPNG_Type, 100);
-}
+// Encodes to PNG, unless there is already encoded data, in which case that gets
+// used.
+// FIXME: Share with PictureRenderer.cpp?
+class PngPixelSerializer : public SkPixelSerializer {
+public:
+    virtual bool onUseEncodedData(const void*, size_t) SK_OVERRIDE { return true; }
+    virtual SkData* onEncodePixels(const SkImageInfo& info, void* pixels,
+                                   size_t rowBytes) SK_OVERRIDE {
+        SkBitmap bm;
+        if (!bm.installPixels(info, pixels, rowBytes)) {
+            return NULL;
+        }
+        return SkImageEncoder::EncodeData(bm, SkImageEncoder::kPNG_Type, 100);
+    }
+};
 
 static SkData* serialized_picture_from_bitmap(const SkBitmap& bitmap) {
     SkPictureRecorder recorder;
@@ -1454,7 +1467,8 @@ static SkData* serialized_picture_from_bitmap(const SkBitmap& bitmap) {
     SkAutoTUnref<SkPicture> picture(recorder.endRecording());
 
     SkDynamicMemoryWStream wStream;
-    picture->serialize(&wStream, &encode_bitmap_to_data);
+    PngPixelSerializer serializer;
+    picture->serialize(&wStream, &serializer);
     return wStream.copyToData();
 }
 
diff --git a/tools/PictureRenderer.cpp b/tools/PictureRenderer.cpp
index 33766e9619..123cc2c5cc 100644
--- a/tools/PictureRenderer.cpp
+++ b/tools/PictureRenderer.cpp
@@ -28,6 +28,7 @@
 #include "SkPictureRecorder.h"
 #include "SkPictureUtils.h"
 #include "SkPixelRef.h"
+#include "SkPixelSerializer.h"
 #include "SkScalar.h"
 #include "SkStream.h"
 #include "SkString.h"
@@ -359,10 +360,22 @@ SkCanvas* RecordPictureRenderer::setupCanvas(int width, int height) {
     return NULL;
 }
 
-// the size_t* parameter is deprecated, so we ignore it
-static SkData* encode_bitmap_to_data(size_t*, const SkBitmap& bm) {
-    return SkImageEncoder::EncodeData(bm, SkImageEncoder::kPNG_Type, 100);
-}
+// Encodes to PNG, unless there is already encoded data, in which case that gets
+// used.
+// FIXME: Share with PictureTest.cpp?
+
+class PngPixelSerializer : public SkPixelSerializer {
+public:
+    virtual bool onUseEncodedData(const void*, size_t) SK_OVERRIDE { return true; }
+    virtual SkData* onEncodePixels(const SkImageInfo& info, void* pixels,
+                                   size_t rowBytes) SK_OVERRIDE {
+        SkBitmap bm;
+        if (!bm.installPixels(info, pixels, rowBytes)) {
+            return NULL;
+        }
+        return SkImageEncoder::EncodeData(bm, SkImageEncoder::kPNG_Type, 100);
+    }
+};
 
 bool RecordPictureRenderer::render(SkBitmap** out) {
     SkAutoTDelete<SkBBHFactory> factory(this->getFactory());
@@ -378,7 +391,8 @@ bool RecordPictureRenderer::render(SkBitmap** out) {
         // Record the new picture as a new SKP with PNG encoded bitmaps.
         SkString skpPath = SkOSPath::Join(fWritePath.c_str(), fInputFilename.c_str());
         SkFILEWStream stream(skpPath.c_str());
-        picture->serialize(&stream, &encode_bitmap_to_data);
+        PngPixelSerializer serializer;
+        picture->serialize(&stream, &serializer);
         return true;
     }
     return false;