Use SkPicture's InstallPixelRefProc to decode SkImages in an skp

src/core/SkReadBuffer.cpp

 
 /*
  * Copyright 2012 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkBitmap.h"
 #include "SkErrorInternals.h"
@@ skipping 207 matching lines @@
         const size_t length = this->readUInt();
         if (length > 0) {
 #ifdef DEBUG_NON_DETERMINISTIC_ASSERT
             fDecodedBitmapIndex++;
 #endif // DEBUG_NON_DETERMINISTIC_ASSERT
             // A non-zero size means the SkBitmap was encoded. Read the data and pixel
             // offset.
             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;
-                }
+            SkIRect subset = SkIRect::MakeXYWH(xOffset, yOffset, width, height);
+            if (this->decodeBitmap(data, length, bitmap, subset)) {
+                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;
-                }
-            }
             // This bitmap was encoded when written, but we are unable to decode, possibly due to
             // not having a decoder.
             SkErrorInternals::SetError(kParseError_SkError,
                                        "Could not decode bitmap. Resulting bitmap will be empty.");
             // Even though we weren't able to decode the pixels, the readbuffer should still be
             // intact, so we return true with an empty bitmap, so we don't force an abort of the
             // larger deserialize.
             bitmap->setInfo(SkImageInfo::MakeUnknown(width, height));
             return true;
         } else if (SkBitmap::ReadRawPixels(this, bitmap)) {
@@ skipping 35 matching lines @@
     }
 
     int originX = this->read32();
     int originY = this->read32();
     if (originX < 0 || originY < 0) {
         this->validate(false);
         return nullptr;
     }
 
     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;
     }
 
     return SkImage::NewFromGenerator(
             new EmptyImageGenerator(SkImageInfo::MakeN32Premul(width, height)));
 }
 
 SkTypeface* SkReadBuffer::readTypeface() {
@@ skipping 77 matching lines @@
             return;
         }
     } else {
         if (nullptr == this->readFunctionPtr()) {
             return;
         }
     }
     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;
+}