Replace EncodeBitmap with an interface.
authorscroggo <scroggo@google.com>
Tue, 9 Dec 2014 15:44:36 +0000 (07:44 -0800)
committerCommit bot <commit-bot@chromium.org>
Tue, 9 Dec 2014 15:44:36 +0000 (07:44 -0800)
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

Committed: https://skia.googlesource.com/skia/+/0c4aba6edb9900c597359dfa49d3ce4a41bc5dd1

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

12 files changed:
dm/DMSerializeTask.cpp
gm/gmmain.cpp
gyp/skia_for_chromium_defines.gypi
include/core/SkPicture.h
include/core/SkPixelSerializer.h [new file with mode: 0644]
include/core/SkWriteBuffer.h
src/core/SkPicture.cpp
src/core/SkPictureData.cpp
src/core/SkPictureData.h
src/core/SkWriteBuffer.cpp
tests/PictureTest.cpp
tools/PictureRenderer.cpp

index d8b460d..a3e2503 100644 (file)
@@ -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));
 
index 3eb1247..46b039d 100644 (file)
@@ -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);
index aee4112..2c91d29 100644 (file)
@@ -15,6 +15,8 @@
     'skia_for_chromium_defines': [
       'SK_SUPPORT_LEGACY_TEXTRENDERMODE',
       'SK_IGNORE_GPU_LAYER_HOISTING',
+      # Transition for skbug.com/3190
+      'SK_LEGACY_ENCODE_BITMAP',
     ],
   },
 }
index f764d34..88d8b05 100644 (file)
@@ -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 (file)
index 0000000..8fc445c
--- /dev/null
@@ -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
index 4dbe17b..39739f2 100644 (file)
@@ -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
index 0d7773b..3296264 100644 (file)
@@ -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);
     }
index 2b22654..938274a 100644 (file)
@@ -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);
 
index 6678806..ab78f8a 100644 (file)
@@ -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;
index c79d275..bcae1f8 100644 (file)
@@ -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(bitmap.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;
index 7a8c8fa..ad1459e 100644 (file)
@@ -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();
 }
 
index 33766e9..123cc2c 100644 (file)
@@ -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;