Use SkPicture's InstallPixelRefProc to decode SkImages in an skp

src/core/SkReadBuffer.cpp

Index: src/core/SkReadBuffer.cpp
diff --git a/src/core/SkReadBuffer.cpp b/src/core/SkReadBuffer.cpp
index 6823a6cd8e7c1d51c9aebc531b895b5081fa0ea1..cc6d2b66a1d0a021e2a521e595f296ee0d024e45 100644
--- a/src/core/SkReadBuffer.cpp
+++ b/src/core/SkReadBuffer.cpp
@@ -225,39 +225,11 @@ bool SkReadBuffer::readBitmap(SkBitmap* bitmap) {
             const void* data = this->skip(length);
             const int32_t xOffset = this->readInt();
             const int32_t yOffset = this->readInt();
-            if (fBitmapDecoder != nullptr && fBitmapDecoder(data, length, bitmap)) {
-                if (bitmap->width() == width && bitmap->height() == height) {
-#ifdef DEBUG_NON_DETERMINISTIC_ASSERT
-                    if (0 != xOffset || 0 != yOffset) {
-                        SkDebugf("SkReadBuffer::readBitmap: heights match,"
-                                 " but offset is not zero. \nInfo about the bitmap:"
-                                 "\n\tIndex: %d\n\tDimensions: [%d %d]\n\tEncoded"
-                                 " data size: %d\n\tOffset: (%d, %d)\n",
-                                 fDecodedBitmapIndex, width, height, length, xOffset,
-                                 yOffset);
-                    }
-#endif // DEBUG_NON_DETERMINISTIC_ASSERT
-                    // If the width and height match, there should be no offset.
-                    SkASSERT(0 == xOffset && 0 == yOffset);
-                    return true;
-                }
-
-                // This case can only be reached if extractSubset was called, so
-                // the recorded width and height must be smaller than or equal to
-                // the encoded width and height.
-                // FIXME (scroggo): This assert assumes that our decoder and the
-                // sources encoder agree on the width and height which may not
-                // always be the case. Removing until it can be investigated
-                // further.
-                //SkASSERT(width <= bitmap->width() && height <= bitmap->height());
-
-                SkBitmap subsetBm;
-                SkIRect subset = SkIRect::MakeXYWH(xOffset, yOffset, width, height);
-                if (bitmap->extractSubset(&subsetBm, subset)) {
-                    bitmap->swap(subsetBm);
-                    return true;
-                }
+            SkIRect subset = SkIRect::MakeXYWH(xOffset, yOffset, width, height);
+            if (this->decodeBitmap(data, length, bitmap, subset)) {
+                return true;
             }
+
             // This bitmap was encoded when written, but we are unable to decode, possibly due to
             // not having a decoder.
             SkErrorInternals::SetError(kParseError_SkError,
@@ -313,6 +285,14 @@ SkImage* SkReadBuffer::readImage() {
     }
 
     const SkIRect subset = SkIRect::MakeXYWH(originX, originY, width, height);
+    SkBitmap bitmap;
+    if (this->decodeBitmap(encoded->data(), encoded->size(), &bitmap, subset)) {
+        SkImage* image = SkImage::NewFromBitmap(bitmap);

[msarett, 2016/02/24 23:30:54]

This is the quick and easy fix for tools/get_images_from_skps.cpp.

But I'm not sure it's the right fix if we have clients that use this to hook in
their own decoder (and expect to decode efficiently).  SkImage::NewFromBitmap()
may perform a copy.

An alternative is to change the signature of the InstallPixelRefProc to output
memory/SkData and an SkImageInfo.

[scroggo, 2016/02/25 12:52:36]

My preference is to replace InstallPixelRefProc, as I have done in
crrev.com/1717663002. Please take a look at that one.

+        if (image) {
+            return image;
+        }
+    }
+
     SkImage* image = SkImage::NewFromEncoded(encoded, &subset);
     if (image) {
         return image;
@@ -410,3 +390,40 @@ void SkReadBuffer::skipFlattenable() {
     uint32_t sizeRecorded = fReader.readU32();
     fReader.skip(sizeRecorded);
 }
+
+bool SkReadBuffer::decodeBitmap(const void* data, size_t length, SkBitmap* bitmap,
+        const SkIRect& subset) {
+    if (fBitmapDecoder != nullptr && fBitmapDecoder(data, length, bitmap)) {
+        if (bitmap->width() == subset.width() && bitmap->height() == subset.height()) {
+#ifdef DEBUG_NON_DETERMINISTIC_ASSERT
+            if (0 != subset.x() || 0 != subset.y()) {
+                SkDebugf("SkReadBuffer::readBitmap: heights match,"
+                         " but offset is not zero. \nInfo about the bitmap:"
+                         "\n\tIndex: %d\n\tDimensions: [%d %d]\n\tEncoded"
+                         " data size: %d\n\tOffset: (%d, %d)\n",
+                         fDecodedBitmapIndex, subset.width(), subset.height(), length, subset.x(),
+                         subset.y());
+            }
+#endif // DEBUG_NON_DETERMINISTIC_ASSERT
+            // If the width and height match, there should be no offset.
+            SkASSERT(0 == subset.x() && 0 == subset.y());
+            return true;
+        }
+
+        // This case can only be reached if extractSubset was called, so
+        // the recorded width and height must be smaller than or equal to
+        // the encoded width and height.
+        // FIXME (scroggo): This assert assumes that our decoder and the
+        // sources encoder agree on the width and height which may not
+        // always be the case. Removing until it can be investigated
+        // further.
+        //SkASSERT(subset.width() <= bitmap->width() && subset.height() <= bitmap->height());
+        SkBitmap subsetBm;
+        if (bitmap->extractSubset(&subsetBm, subset)) {
+            bitmap->swap(subsetBm);
+            return true;
+        }
+    }
+
+    return false;
+}