New API for encoding bitmaps during serialization.

This change gives more flexibility to the implementation of
EncodeBitmap to prefer calling refEncodedData, doing its own
encode, or even doing both and making a decision about which
to use.

The new function signature also allows the implementation to
tell the ordered write buffer whether to store the pixel offset,
in the case where the encoded bitmap represents the larger
bitmap, or to ignore the pixel offset, in the case where the
implementation only encoded the subset that is used.

Requires changes to chromium to use the new function signature.
(https://codereview.chromium.org/15496006/)

SkPicture:
New API for EncodeBitmap.

SkOrderedReadBuffer:
Ifdef'd out addition of reading the offset.

SkOrderedWriteBuffer:
Never call refEncodedData. Allow the user to call that from their
EncodeBitmap function, if desired.
This addresses https://code.google.com/p/skia/issues/detail?id=1239
Add in ifdef'd out code to record the offset.

PictureTest and PictureRenderer:
Implement the new definition of EncodeBitmap. Also update the name
of the function to meet coding style guidelines.

BUG=https://code.google.com/p/skia/issues/detail?id=1239
R=reed@google.com

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

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

include/core/SkPicture.h

@@ -16,6 +16,7 @@
 class SkBBoxHierarchy;
 class SkCanvas;
 class SkDrawPictureCallback;
+class SkData;
 class SkPicturePlayback;
 class SkPictureRecord;
 class SkStream;
@@ -170,17 +171,21 @@ public:
     int height() const { return fHeight; }
 
     /**
-     *  Function to encode an SkBitmap to an SkWStream. A function with this
+     *  Function to encode an SkBitmap to an SkData. A function with this
-     *  signature can be passed to serialize() and SkOrderedWriteBuffer. The
+     *  signature can be passed to serialize() and SkOrderedWriteBuffer.
-     *  function should return true if it succeeds. Otherwise it should return
+     *  Returning NULL will tell the SkOrderedWriteBuffer to use
-     *  false so that SkOrderedWriteBuffer can switch to another method of
+     *  SkBitmap::flatten() to store the bitmap.
-     *  storing SkBitmaps.
+     *  @param pixelRefOffset Output parameter, telling the deserializer what
+     *      offset in the bm's pixelRef corresponds to the encoded data.
+     *  @return SkData If non-NULL, holds encoded data representing the passed
+     *      in bitmap. The caller is responsible for calling unref().
      */
-    typedef bool (*EncodeBitmap)(SkWStream*, const SkBitmap&);
+    typedef SkData* (*EncodeBitmap)(size_t* pixelRefOffset, const SkBitmap& bm);
 
     /**
      *  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.
      */
     void serialize(SkWStream*, EncodeBitmap encoder = NULL) const;
 

src/core/SkBitmap.cpp

@@ -837,7 +837,7 @@ void SkBitmap::eraseARGB(U8CPU a, U8CPU r, U8CPU g, U8CPU b) const {
  *  Also note that (x, y) may be outside the range of (0 - width(), 0 - height()), so long as it is
  *  within the bounds of the SkPixelRef being used.
  */
-static size_t getSubOffset(const SkBitmap& bm, int x, int y) {
+static size_t get_sub_offset(const SkBitmap& bm, int x, int y) {
     switch (bm.getConfig()) {
         case SkBitmap::kA8_Config:
         case SkBitmap:: kIndex8_Config:
@@ -866,18 +866,18 @@ static size_t getSubOffset(const SkBitmap& bm, int x, int y) {
  *  upper left corner of bm relative to its SkPixelRef.
  *  x and y must be non-NULL.
  */
-static bool getUpperLeftFromOffset(const SkBitmap& bm, int32_t* x, int32_t* y) {
+bool get_upper_left_from_offset(SkBitmap::Config config, size_t offset, size_t rowBytes,
+                                   int32_t* x, int32_t* y) {
     SkASSERT(x != NULL && y != NULL);
-    const size_t offset = bm.pixelRefOffset();
     if (0 == offset) {
         *x = *y = 0;
         return true;
     }
     // Use integer division to find the correct y position.
-    *y = SkToS32(offset / bm.rowBytes());
+    *y = SkToS32(offset / rowBytes);
-    // The remainder will be the x position, after we reverse getSubOffset.
+    // The remainder will be the x position, after we reverse get_sub_offset.
-    *x = SkToS32(offset % bm.rowBytes());
+    *x = SkToS32(offset % rowBytes);
-    switch (bm.getConfig()) {
+    switch (config) {
         case SkBitmap::kA8_Config:
             // Fall through.
         case SkBitmap::kIndex8_Config:
@@ -904,6 +904,10 @@ static bool getUpperLeftFromOffset(const SkBitmap& bm, int32_t* x, int32_t* y) {
     return true;
 }
 
+static bool get_upper_left_from_offset(const SkBitmap& bm, int32_t* x, int32_t* y) {
+    return get_upper_left_from_offset(bm.config(), bm.pixelRefOffset(), bm.rowBytes(), x, y);
+}
+
 bool SkBitmap::extractSubset(SkBitmap* result, const SkIRect& subset) const {
     SkDEBUGCODE(this->validate();)
 
@@ -965,7 +969,7 @@ bool SkBitmap::extractSubset(SkBitmap* result, const SkIRect& subset) const {
     SkASSERT(static_cast<unsigned>(r.fLeft) < static_cast<unsigned>(this->width()));
     SkASSERT(static_cast<unsigned>(r.fTop) < static_cast<unsigned>(this->height()));
 
-    size_t offset = getSubOffset(*this, r.fLeft, r.fTop);
+    size_t offset = get_sub_offset(*this, r.fLeft, r.fTop);
     if (SUB_OFFSET_FAILURE == offset) {
         return false;   // config not supported
     }
@@ -1032,7 +1036,7 @@ bool SkBitmap::copyTo(SkBitmap* dst, Config dstConfig, Allocator* alloc) const {
 
     if (fPixelRef) {
         SkIRect subset;
-        if (getUpperLeftFromOffset(*this, &subset.fLeft, &subset.fTop)) {
+        if (get_upper_left_from_offset(*this, &subset.fLeft, &subset.fTop)) {
             subset.fRight = subset.fLeft + fWidth;
             subset.fBottom = subset.fTop + fHeight;
             if (fPixelRef->readPixels(&tmpSrc, &subset)) {
@@ -1144,10 +1148,10 @@ bool SkBitmap::deepCopyTo(SkBitmap* dst, Config dstConfig) const {
                 // Find the correct offset in the new config. This needs to be done after calling
                 // setConfig so dst's fConfig and fRowBytes have been set properly.
                 int32_t x, y;
-                if (!getUpperLeftFromOffset(*this, &x, &y)) {
+                if (!get_upper_left_from_offset(*this, &x, &y)) {
                     return false;
                 }
-                pixelRefOffset = getSubOffset(*dst, x, y);
+                pixelRefOffset = get_sub_offset(*dst, x, y);
                 if (SUB_OFFSET_FAILURE == pixelRefOffset) {
                     return false;
                 }

src/core/SkOrderedReadBuffer.cpp

@@ -207,7 +207,13 @@ void SkOrderedReadBuffer::readBitmap(SkBitmap* bitmap) {
                 // FIXME: Once the writer is changed to record the (x,y) offset,
                 // they will be used to store the correct portion of the picture.
                 SkBitmap subsetBm;
+#ifdef BUMP_PICTURE_VERSION
+                int32_t x = fReader.readS32();
+                int32_t y = fReader.readS32();
+                SkIRect subset = SkIRect::MakeXYWH(x, y, width, height);
+#else
                 SkIRect subset = SkIRect::MakeWH(width, height);
+#endif
                 if (bitmap->extractSubset(&subsetBm, subset)) {
                     bitmap->swap(subsetBm);
                     return;

src/core/SkOrderedWriteBuffer.cpp

@@ -9,7 +9,6 @@
 #include "SkOrderedWriteBuffer.h"
 #include "SkBitmap.h"
 #include "SkData.h"
-#include "SkPixelRef.h"
 #include "SkPtrRecorder.h"
 #include "SkStream.h"
 #include "SkTypeface.h"
@@ -144,6 +143,10 @@ bool SkOrderedWriteBuffer::writeToStream(SkWStream* stream) {
     return fWriter.writeToStream(stream);
 }
 
+// Defined in SkBitmap.cpp
+bool get_upper_left_from_offset(SkBitmap::Config config, size_t offset, size_t rowBytes,
+                            int32_t* x, int32_t* y);
+
 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.
@@ -154,11 +157,9 @@ void SkOrderedWriteBuffer::writeBitmap(const SkBitmap& bitmap) {
     // 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. 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
+    // 2. If there is a function for encoding bitmaps, use it to write an encoded version of the
-    //    that a heap was not used, write the size of the encoded data. A non-zero size signifies
+    //    bitmap. After writing a boolean value of false, signifying that a heap was not used, write
-    //    that encoded data was written.
+    //    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. 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;
@@ -178,44 +179,34 @@ void SkOrderedWriteBuffer::writeBitmap(const SkBitmap& bitmap) {
         fWriter.write32(bitmap.getGenerationID());
         return;
     }
-    bool encoded = false;
+    if (fBitmapEncoder != NULL) {
-    // Before attempting to encode the SkBitmap, check to see if there is already an encoded
+        SkASSERT(NULL == fBitmapHeap);
-    // version.
+        size_t offset;
-    SkPixelRef* ref = bitmap.pixelRef();
+        SkAutoDataUnref data(fBitmapEncoder(&offset, bitmap));
-    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
+            // by the actual data.
-            // reader need not know the difference.
             this->writeUInt(SkToU32(data->size()));
             fWriter.writePad(data->data(), data->size());
-            encoded = true;
+#ifdef BUMP_PICTURE_VERSION
-        }
+            // Recording this fixes https://code.google.com/p/skia/issues/detail?id=1301, but
-    }
+            // also requires bumping PICTURE_VERSION, so leaving out for now.
-    if (fBitmapEncoder != NULL && !encoded) {
+            // Store the coordinate of the offset, rather than fPixelRefOffset, which may be
-        SkASSERT(NULL == fBitmapHeap);
+            // different depending on the decoder.
-        SkDynamicMemoryWStream stream;
+            int32_t x, y;
-        if (fBitmapEncoder(&stream, bitmap)) {
+            if (0 == offset || !get_upper_left_from_offset(bitmap.config(), offset,
-            uint32_t offset = fWriter.bytesWritten();
+                                                           bitmap.rowBytes(), &x, &y)) {
-            // Write the length to indicate that the bitmap was encoded successfully, followed
+                x = y = 0;
-            // 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(SkToU32(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);
             }
+            this->write32(x);
+            this->write32(y);
+#endif
+            return;
         }
     }
-    if (!encoded) {
+    // Bitmap was not encoded. Record a zero, implying that the reader need not decode.
-        // Bitmap was not encoded. Record a zero, implying that the reader need not decode.
+    this->writeUInt(0);
-        this->writeUInt(0);
+    bitmap.flatten(*this);
-        bitmap.flatten(*this);
-    }
 }
 
 void SkOrderedWriteBuffer::writeTypeface(SkTypeface* obj) {

src/core/SkOrderedWriteBuffer.h

@@ -84,14 +84,15 @@ public:
     void setBitmapHeap(SkBitmapHeap*);
 
     /**
-     * Provide a function to encode an SkBitmap to an SkStream. writeBitmap will attempt to use
+     * 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.
      *
      * 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);
+    void setBitmapEncoder(SkPicture::EncodeBitmap bitmapEncoder);
 
 private:
     SkFactorySet* fFactorySet;

tests/PictureTest.cpp

@@ -359,8 +359,9 @@ private:
 
 #include "SkImageEncoder.h"
 
-static bool PNGEncodeBitmapToStream(SkWStream* wStream, const SkBitmap& bm) {
+static SkData* encode_bitmap_to_data(size_t* offset, const SkBitmap& bm) {
-    return SkImageEncoder::EncodeStream(wStream, bm, SkImageEncoder::kPNG_Type, 100);
+    *offset = 0;
+    return SkImageEncoder::EncodeData(bm, SkImageEncoder::kPNG_Type, 100);
 }
 
 static SkData* serialized_picture_from_bitmap(const SkBitmap& bitmap) {
@@ -368,7 +369,7 @@ static SkData* serialized_picture_from_bitmap(const SkBitmap& bitmap) {
     SkCanvas* canvas = picture.beginRecording(bitmap.width(), bitmap.height());
     canvas->drawBitmap(bitmap, 0, 0);
     SkDynamicMemoryWStream wStream;
-    picture.serialize(&wStream, &PNGEncodeBitmapToStream);
+    picture.serialize(&wStream, &encode_bitmap_to_data);
     return wStream.copyToData();
 }
 

tools/PictureRenderer.cpp

@@ -9,6 +9,7 @@
 #include "picture_utils.h"
 #include "SamplePipeControllers.h"
 #include "SkCanvas.h"
+#include "SkData.h"
 #include "SkDevice.h"
 #include "SkGPipe.h"
 #if SK_SUPPORT_GPU
@@ -20,6 +21,8 @@
 #include "SkMaskFilter.h"
 #include "SkMatrix.h"
 #include "SkPicture.h"
+#include "SkPictureUtils.h"
+#include "SkPixelRef.h"
 #include "SkRTree.h"
 #include "SkScalar.h"
 #include "SkStream.h"
@@ -29,8 +32,6 @@
 #include "SkTDArray.h"
 #include "SkThreadUtils.h"
 #include "SkTypes.h"
-#include "SkData.h"
-#include "SkPictureUtils.h"
 
 namespace sk_tools {
 
@@ -257,8 +258,17 @@ SkCanvas* RecordPictureRenderer::setupCanvas(int width, int height) {
     return NULL;
 }
 
-static bool PNGEncodeBitmapToStream(SkWStream* wStream, const SkBitmap& bm) {
+static SkData* encode_bitmap_to_data(size_t* offset, const SkBitmap& bm) {
-    return SkImageEncoder::EncodeStream(wStream, bm, SkImageEncoder::kPNG_Type, 100);
+    SkPixelRef* pr = bm.pixelRef();
+    if (pr != NULL) {
+        SkData* data = pr->refEncodedData();
+        if (data != NULL) {
+            *offset = bm.pixelRefOffset();
+            return data;
+        }
+    }
+    *offset = 0;
+    return SkImageEncoder::EncodeData(bm, SkImageEncoder::kPNG_Type, 100);
 }
 
 bool RecordPictureRenderer::render(const SkString* path, SkBitmap** out) {
@@ -274,7 +284,7 @@ bool RecordPictureRenderer::render(const SkString* path, SkBitmap** out) {
         // ".skp" was removed from 'path' before being passed in here.
         skpPath.append(".skp");
         SkFILEWStream stream(skpPath.c_str());
-        replayer->serialize(&stream, &PNGEncodeBitmapToStream);
+        replayer->serialize(&stream, &encode_bitmap_to_data);
         return true;
     }
     return false;