change old picture serialization to really handle images

BUG=skia:3965

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

gm/cgm.c

@@ -64,7 +64,7 @@ void sk_test_c_api(sk_canvas_t* canvas) {
     sk_data_t* data = sk_image_encode(img0);
     sk_image_unref(img0);
 
-    sk_image_t* img1 = sk_image_new_from_data(data);
+    sk_image_t* img1 = sk_image_new_from_encoded(data, NULL);
     sk_data_unref(data);
 
     if (img1) {

gm/factory.cpp

@@ -37,19 +37,19 @@ protected:
             SkAutoTUnref<SkDiscardableMemoryPool> pool(
                 SkDiscardableMemoryPool::Create(1));
             SkAssertResult(SkInstallDiscardablePixelRef(SkImageGenerator::NewFromData(data),
-                                                        &fBitmap, pool));
+                                                        NULL, &fBitmap, pool));
         }
     }
 
-    virtual SkString onShortName() override {
+    SkString onShortName() override {
         return SkString("factory");
     }
 
-    virtual SkISize onISize() override {
+    SkISize onISize() override {
         return SkISize::Make(640, 480);
     }
 
-    virtual void onDraw(SkCanvas* canvas) override {
+    void onDraw(SkCanvas* canvas) override {
         canvas->drawBitmap(fBitmap, 0, 0);
     }
 

gm/image.cpp

@@ -24,7 +24,7 @@ static void drawJpeg(SkCanvas* canvas, const SkISize& size) {
     if (NULL == data.get()) {
         return;
     }
-    SkImage* image = SkImage::NewFromData(data);
+    SkImage* image = SkImage::NewFromEncoded(data);
     if (image) {
         SkAutoCanvasRestore acr(canvas, true);
         canvas->scale(size.width() * 1.0f / image->width(),

include/c/sk_image.h

@@ -28,7 +28,7 @@ sk_image_t* sk_image_new_raster_copy(const sk_imageinfo_t*, const void* pixels,
  *  On success, the encoded data may be processed immediately, or it may be ref()'d for later
  *  use.
  */
-sk_image_t* sk_image_new_from_data(const sk_data_t* encoded);
+sk_image_t* sk_image_new_from_encoded(const sk_data_t* encoded, const sk_irect_t* subset);
 
 sk_data_t* sk_image_encode(const sk_image_t*);
 

include/c/sk_types.h

@@ -67,6 +67,13 @@ typedef struct {
     float   y;
 } sk_point_t;
 
+typedef struct {
+    int32_t left;
+    int32_t top;
+    int32_t right;
+    int32_t bottom;
+} sk_irect_t;
+
 typedef struct {
     float   left;
     float   top;

include/core/SkImage.h

@@ -65,17 +65,27 @@ public:
      *  Construct a new SkImage based on the given ImageGenerator.
      *  This function will always take ownership of the passed
      *  ImageGenerator.  Returns NULL on error.
+     *
+     *  If a subset is specified, it must be contained within the generator's bounds.
      */
-    static SkImage* NewFromGenerator(SkImageGenerator*);
+    static SkImage* NewFromGenerator(SkImageGenerator*, const SkIRect* subset = NULL);
 
     /**
      *  Construct a new SkImage based on the specified encoded data. Returns NULL on failure,
      *  which can mean that the format of the encoded data was not recognized/supported.
      *
+     *  If a subset is specified, it must be contained within the encoded data's bounds.
+     *
      *  Regardless of success or failure, the caller is responsible for managing their ownership
      *  of the data.
      */
-    static SkImage* NewFromData(SkData* data);
+    static SkImage* NewFromEncoded(SkData* encoded, const SkIRect* subset = NULL);
+
+#ifdef SK_SUPPORT_LEGACY_IMAGE_NEWFROMDATA
+    static SkImage* NewFromData(SkData* data) {
+        return NewFromEncoded(data, NULL);
+    }
+#endif
 
     /**
      *  Create a new image from the specified descriptor. Note - the caller is responsible for
@@ -151,6 +161,15 @@ public:
      */
     const void* peekPixels(SkImageInfo* info, size_t* rowBytes) const;
 
+    /**
+     *  If the image has direct access to its pixels (i.e. they are in local
+     *  RAM) return the (const) address of those pixels, and if not null, return
+     *  true, and if pixmap is not NULL, set it to point into the image.
+     *
+     *  On failure, return false and ignore the pixmap parameter.
+     */
+    bool peekPixels(SkPixmap* pixmap) const;
+
     // DEPRECATED
     GrTexture* getTexture() const;
 
@@ -187,6 +206,8 @@ public:
     bool readPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes,
                     int srcX, int srcY) const;
 
+    bool readPixels(const SkPixmap& dst, int srcX, int srcY) const;
+
     /**
      *  Encode the image's pixels and return the result as a new SkData, which
      *  the caller must manage (i.e. call unref() when they are done).
@@ -194,8 +215,22 @@ public:
      *  If the image type cannot be encoded, or the requested encoder type is
      *  not supported, this will return NULL.
      */
-    SkData* encode(SkImageEncoder::Type t = SkImageEncoder::kPNG_Type,
+    SkData* encode(SkImageEncoder::Type, int quality) const;
-                   int quality = 80) const;
+
+    SkData* encode() const {
+        return this->encode(SkImageEncoder::kPNG_Type, 100);
+    }
+
+    /**
+     *  If the image already has its contents in encoded form (e.g. PNG or JPEG), return a ref
+     *  to that data (which the caller must call unref() on). The caller is responsible for calling
+     *  unref on the data when they are done.
+     *
+     *  If the image does not already has its contents in encoded form, return NULL.
+     *
+     *  Note: to force the image to return its contents as encoded data, try calling encode(...).
+     */
+    SkData* refEncoded() const;
 
     /**
      *  Return a new surface that is compatible with this image's internal representation

include/core/SkPicture.h

@@ -175,10 +175,11 @@ private:
     // V40: Remove UniqueID serialization from SkImageFilter.
     // V41: Added serialization of SkBitmapSource's filterQuality parameter
     // V42: Added a bool to SkPictureShader serialization to indicate did-we-serialize-a-picture?
+    // V43: Added DRAW_IMAGE and DRAW_IMAGE_RECT opt codes to serialized data
 
     // Only SKPs within the min/current picture version range (inclusive) can be read.
     static const uint32_t     MIN_PICTURE_VERSION = 35;     // Produced by Chrome M39.
-    static const uint32_t CURRENT_PICTURE_VERSION = 42;
+    static const uint32_t CURRENT_PICTURE_VERSION = 43;
 
     static_assert(MIN_PICTURE_VERSION <= 41,
                   "Remove kFontFileName and related code from SkFontDescriptor.cpp.");

include/core/SkWriteBuffer.h

@@ -73,6 +73,7 @@ public:
     void writePath(const SkPath& path);
     size_t writeStream(SkStream* stream, size_t length);
     void writeBitmap(const SkBitmap& bitmap);
+    void writeImage(const SkImage*);
     void writeTypeface(SkTypeface* typeface);
     void writePaint(const SkPaint& paint) { paint.flatten(*this); }
 
@@ -106,6 +107,7 @@ public:
      * be set to NULL in release and crash in debug.
      */
     void setPixelSerializer(SkPixelSerializer*);
+    SkPixelSerializer* getPixelSerializer() const { return fPixelSerializer; }
 
 private:
     bool isValidating() const { return SkToBool(fFlags & kValidation_Flag); }

src/c/sk_surface.cpp

@@ -207,8 +207,9 @@ sk_image_t* sk_image_new_raster_copy(const sk_imageinfo_t* cinfo, const void* pi
     return (sk_image_t*)SkImage::NewRasterCopy(info, pixels, rowBytes);
 }
 
-sk_image_t* sk_image_new_from_data(const sk_data_t* cdata) {
+sk_image_t* sk_image_new_from_encoded(const sk_data_t* cdata, const sk_irect_t* subset) {
-    return ToImage(SkImage::NewFromData(AsData(cdata)));
+    return ToImage(SkImage::NewFromEncoded(AsData(cdata),
+                                           reinterpret_cast<const SkIRect*>(subset)));
 }
 
 sk_data_t* sk_image_encode(const sk_image_t* cimage) {

src/core/SkBitmap.cpp

@@ -1270,7 +1270,9 @@ bool SkBitmap::requestLock(SkAutoPixmapUnlock* result) const {
         return false;
     }
 
-    SkPixelRef::LockRequest req = { fInfo.dimensions(), kNone_SkFilterQuality };
+    // We have to lock the whole thing (using the pixelref's dimensions) until the api supports
+    // a partial lock (with offset/origin). Hence we can't use our fInfo.
+    SkPixelRef::LockRequest req = { pr->info().dimensions(), kNone_SkFilterQuality };
     SkPixelRef::LockResult res;
     if (pr->requestLock(req, &res)) {
         SkASSERT(res.fPixels);

src/core/SkImageGeneratorPriv.h

@@ -33,7 +33,7 @@
  *
  *  @return true iff successful.
  */
-bool SkInstallDiscardablePixelRef(SkImageGenerator*, SkBitmap* destination,
+bool SkInstallDiscardablePixelRef(SkImageGenerator*, const SkIRect* subset, SkBitmap* destination,
                                   SkDiscardableMemory::Factory* factory);
 
 #endif

src/core/SkPictureData.cpp

@@ -68,6 +68,15 @@ SkPictureData::SkPictureData(const SkPictureRecord& record,
             fTextBlobRefs[i] = SkRef(blobs[i]);
         }
     }
+        
+    const SkTDArray<const SkImage*>& imgs = record.getImageRefs();
+    fImageCount = imgs.count();
+    if (fImageCount > 0) {
+        fImageRefs = SkNEW_ARRAY(const SkImage*, fImageCount);
+        for (int i = 0; i < fImageCount; ++i) {
+            fImageRefs[i] = SkRef(imgs[i]);
+        }
+    }
 }
 
 void SkPictureData::init() {
@@ -75,6 +84,8 @@ void SkPictureData::init() {
     fPictureCount = 0;
     fTextBlobRefs = NULL;
     fTextBlobCount = 0;
+    fImageRefs = NULL;
+    fImageCount = 0;
     fOpData = NULL;
     fFactoryPlayback = NULL;
 }
@@ -91,12 +102,17 @@ SkPictureData::~SkPictureData() {
         fTextBlobRefs[i]->unref();
     }
     SkDELETE_ARRAY(fTextBlobRefs);
-
+    
+    for (int i = 0; i < fImageCount; i++) {
+        fImageRefs[i]->unref();
+    }
+    SkDELETE_ARRAY(fImageRefs);
+    
     SkDELETE(fFactoryPlayback);
 }
 
 bool SkPictureData::containsBitmaps() const {
-    if (fBitmaps.count() > 0) {
+    if (fBitmaps.count() > 0 || fImageCount > 0) {
         return true;
     }
     for (int i = 0; i < fPictureCount; ++i) {
@@ -217,6 +233,13 @@ void SkPictureData::flattenToBuffer(SkWriteBuffer& buffer) const {
             fTextBlobRefs[i]->flatten(buffer);
         }
     }
+    
+    if (fImageCount > 0) {
+        write_tag_size(buffer, SK_PICT_IMAGE_BUFFER_TAG, fImageCount);
+        for (i = 0; i  < fImageCount; ++i) {
+            buffer.writeImage(fImageRefs[i]);
+        }
+    }
 }
 
 void SkPictureData::serialize(SkWStream* stream,
@@ -403,8 +426,67 @@ bool SkPictureData::parseStreamTag(SkStream* stream,
     return true;    // success
 }
 
-bool SkPictureData::parseBufferTag(SkReadBuffer& buffer,
+static const SkImage* create_image_from_buffer(SkReadBuffer& buffer) {
-                                   uint32_t tag, uint32_t size) {
+    int width = buffer.read32();
+    int height = buffer.read32();
+    if (width <= 0 || height <= 0) {    // SkImage never has a zero dimension
+        buffer.validate(false);
+        return NULL;
+    }
+
+    SkAutoTUnref<SkData> encoded(buffer.readByteArrayAsData());
+    int originX = buffer.read32();
+    int originY = buffer.read32();
+    if (0 == encoded->size() || originX < 0 || originY < 0) {
+        buffer.validate(false);
+        return NULL;
+    }
+
+    const SkIRect subset = SkIRect::MakeXYWH(originX, originY, width, height);
+    return SkImage::NewFromEncoded(encoded, &subset);
+}
+
+// Need a shallow wrapper to return const SkPicture* to match the other factories,
+// as SkPicture::CreateFromBuffer() returns SkPicture*
+static const SkPicture* create_picture_from_buffer(SkReadBuffer& buffer) {
+    return SkPicture::CreateFromBuffer(buffer);
+}
+
+template <typename T>
+bool new_array_from_buffer(SkReadBuffer& buffer, uint32_t inCount,
+                           const T*** array, int* outCount, const T* (*factory)(SkReadBuffer&)) {
+    if (!buffer.validate((0 == *outCount) && (NULL == *array))) {
+        return false;
+    }
+    if (0 == inCount) {
+        return true;
+    }
+    *outCount = inCount;
+    *array = SkNEW_ARRAY(const T*, *outCount);
+    bool success = true;
+    int i = 0;
+    for (; i < *outCount; i++) {
+        (*array)[i] = factory(buffer);
+        if (NULL == (*array)[i]) {
+            success = false;
+            break;
+        }
+    }
+    if (!success) {
+        // Delete all of the blobs that were already created (up to but excluding i):
+        for (int j = 0; j < i; j++) {
+            (*array)[j]->unref();
+        }
+        // Delete the array
+        SkDELETE_ARRAY(*array);
+        *array = NULL;
+        *outCount = 0;
+        return false;
+    }
+    return true;
+}
+
+bool SkPictureData::parseBufferTag(SkReadBuffer& buffer, uint32_t tag, uint32_t size) {
     switch (tag) {
         case SK_PICT_BITMAP_BUFFER_TAG: {
             const int count = SkToInt(size);
@@ -433,33 +515,18 @@ bool SkPictureData::parseBufferTag(SkReadBuffer& buffer,
                     buffer.readPath(&fPaths[i]);
                 }
             } break;
-        case SK_PICT_TEXTBLOB_BUFFER_TAG: {
+        case SK_PICT_TEXTBLOB_BUFFER_TAG:
-            if (!buffer.validate((0 == fTextBlobCount) && (NULL == fTextBlobRefs))) {
+            if (!new_array_from_buffer(buffer, size, &fTextBlobRefs, &fTextBlobCount,
+                                       SkTextBlob::CreateFromBuffer)) {
                 return false;
             }
-            fTextBlobCount = size;
+            break;
-            fTextBlobRefs = SkNEW_ARRAY(const SkTextBlob*, fTextBlobCount);
+        case SK_PICT_IMAGE_BUFFER_TAG:
-            bool success = true;
+            if (!new_array_from_buffer(buffer, size, &fImageRefs, &fImageCount,
-            int i = 0;
+                                       create_image_from_buffer)) {
-            for ( ; i < fTextBlobCount; i++) {
-                fTextBlobRefs[i] = SkTextBlob::CreateFromBuffer(buffer);
-                if (NULL == fTextBlobRefs[i]) {
-                    success = false;
-                    break;
-                }
-            }
-            if (!success) {
-                // Delete all of the blobs that were already created (up to but excluding i):
-                for (int j = 0; j < i; j++) {
-                    fTextBlobRefs[j]->unref();
-                }
-                // Delete the array
-                SkDELETE_ARRAY(fTextBlobRefs);
-                fTextBlobRefs = NULL;
-                fTextBlobCount = 0;
                 return false;
             }
-        } break;
+            break;
         case SK_PICT_READER_TAG: {
             SkAutoDataUnref data(SkData::NewUninitialized(size));
             if (!buffer.readByteArray(data->writable_data(), size) ||
@@ -469,32 +536,11 @@ bool SkPictureData::parseBufferTag(SkReadBuffer& buffer,
             SkASSERT(NULL == fOpData);
             fOpData = data.detach();
         } break;
-        case SK_PICT_PICTURE_TAG: {
+        case SK_PICT_PICTURE_TAG:
-            if (!buffer.validate((0 == fPictureCount) && (NULL == fPictureRefs))) {
+            if (!new_array_from_buffer(buffer, size, &fPictureRefs, &fPictureCount,
+                                       create_picture_from_buffer)) {
                 return false;
             }
-            fPictureCount = size;
-            fPictureRefs = SkNEW_ARRAY(const SkPicture*, fPictureCount);
-            bool success = true;
-            int i = 0;
-            for ( ; i < fPictureCount; i++) {
-                fPictureRefs[i] = SkPicture::CreateFromBuffer(buffer);
-                if (NULL == fPictureRefs[i]) {
-                    success = false;
-                    break;
-                }
-            }
-            if (!success) {
-                // Delete all of the pictures that were already created (up to but excluding i):
-                for (int j = 0; j < i; j++) {
-                    fPictureRefs[j]->unref();
-                }
-                // Delete the array
-                SkDELETE_ARRAY(fPictureRefs);
-                fPictureCount = 0;
-                return false;
-            }
-        } break;
         default:
             // The tag was invalid.
             return false;

src/core/SkPictureData.h

@@ -51,6 +51,7 @@ struct SkPictInfo {
 #define SK_PICT_PAINT_BUFFER_TAG    SkSetFourByteTag('p', 'n', 't', ' ')
 #define SK_PICT_PATH_BUFFER_TAG     SkSetFourByteTag('p', 't', 'h', ' ')
 #define SK_PICT_TEXTBLOB_BUFFER_TAG SkSetFourByteTag('b', 'l', 'o', 'b')
+#define SK_PICT_IMAGE_BUFFER_TAG    SkSetFourByteTag('i', 'm', 'a', 'g')
 
 // Always write this guy last (with no length field afterwards)
 #define SK_PICT_EOF_TAG     SkSetFourByteTag('e', 'o', 'f', ' ')
@@ -90,6 +91,11 @@ public:
         return fBitmaps[index];
     }
 
+    const SkImage* getImage(SkReader32* reader) const {
+        const int index = reader->readInt();
+        return fImageRefs[index];
+    }
+    
     const SkPath& getPath(SkReader32* reader) const {
         int index = reader->readInt() - 1;
         return fPaths[index];
@@ -156,6 +162,8 @@ private:
     int fPictureCount;
     const SkTextBlob** fTextBlobRefs;
     int fTextBlobCount;
+    const SkImage** fImageRefs;
+    int fImageCount;
 
     SkPictureContentInfo fContentInfo;
 

src/core/SkPictureFlat.h

@@ -69,8 +69,10 @@ enum DrawType {
     DRAW_PATCH, // could not add in aphabetical order
     DRAW_PICTURE_MATRIX_PAINT,
     DRAW_TEXT_BLOB,
+    DRAW_IMAGE,
+    DRAW_IMAGE_RECT,
 
-    LAST_DRAWTYPE_ENUM = DRAW_TEXT_BLOB
+    LAST_DRAWTYPE_ENUM = DRAW_IMAGE_RECT
 };
 
 // In the 'match' method, this constant will match any flavor of DRAW_BITMAP*

src/core/SkPicturePlayback.cpp

@@ -216,6 +216,19 @@ void SkPicturePlayback::handleOp(SkReader32* reader,
         case END_COMMENT_GROUP:
             // deprecated (M44)
             break;
+        case DRAW_IMAGE: {
+            const SkPaint* paint = fPictureData->getPaint(reader);
+            const SkImage* image = fPictureData->getImage(reader);
+            const SkPoint& loc = reader->skipT<SkPoint>();
+            canvas->drawImage(image, loc.fX, loc.fY, paint);
+        } break;
+        case DRAW_IMAGE_RECT: {
+            const SkPaint* paint = fPictureData->getPaint(reader);
+            const SkImage* image = fPictureData->getImage(reader);
+            const SkRect* src = get_rect_ptr(reader);   // may be null
+            const SkRect& dst = reader->skipT<SkRect>();     // required
+            canvas->drawImageRect(image, src, dst, paint);
+        } break;
         case DRAW_OVAL: {
             const SkPaint& paint = *fPictureData->getPaint(reader);
             canvas->drawOval(reader->skipT<SkRect>(), paint);

src/core/SkPictureRecord.cpp

@@ -96,6 +96,8 @@ static inline size_t get_paint_offset(DrawType op, size_t opSize) {
         1,  // DRAW_PATCH - right after op code
         1,  // DRAW_PICTURE_MATRIX_PAINT - right after op code
         1,  // DRAW_TEXT_BLOB- right after op code
+        1,  // DRAW_IMAGE - right after op code
+        1,  // DRAW_IMAGE_RECT - right after op code
     };
 
     SK_COMPILE_ASSERT(sizeof(gPaintOffsets) == LAST_DRAWTYPE_ENUM + 1,
@@ -566,18 +568,34 @@ void SkPictureRecord::onDrawBitmapRect(const SkBitmap& bitmap, const SkRect* src
 
 void SkPictureRecord::onDrawImage(const SkImage* image, SkScalar x, SkScalar y,
                                   const SkPaint* paint) {
-    SkBitmap bm;
+    // op + paint_index + image_index + x + y
-    if (as_IB(image)->getROPixels(&bm)) {
+    size_t size = 3 * kUInt32Size + 2 * sizeof(SkScalar);
-        this->SkPictureRecord::onDrawBitmap(bm, x, y, paint);
+    size_t initialOffset = this->addDraw(DRAW_IMAGE, &size);
-    }
+    SkASSERT(initialOffset+get_paint_offset(DRAW_IMAGE, size) == fWriter.bytesWritten());
+    this->addPaintPtr(paint);
+    this->addImage(image);
+    this->addScalar(x);
+    this->addScalar(y);
+    this->validate(initialOffset, size);
 }
 
 void SkPictureRecord::onDrawImageRect(const SkImage* image, const SkRect* src, const SkRect& dst,
                                       const SkPaint* paint) {
-    SkBitmap bm;
+    // id + paint_index + bitmap_index + bool_for_src
-    if (as_IB(image)->getROPixels(&bm)) {
+    size_t size = 4 * kUInt32Size;
-        this->SkPictureRecord::onDrawBitmapRect(bm, src, dst, paint, kNone_DrawBitmapRectFlag);
+    if (src) {
+        size += sizeof(*src);   // + rect
     }
+    size += sizeof(dst);        // + rect
+    
+    size_t initialOffset = this->addDraw(DRAW_IMAGE_RECT, &size);
+    SkASSERT(initialOffset+get_paint_offset(DRAW_IMAGE_RECT, size)
+             == fWriter.bytesWritten());
+    this->addPaintPtr(paint);
+    this->addImage(image);
+    this->addRectPtr(src);  // may be null
+    this->addRect(dst);
+    this->validate(initialOffset, size);
 }
 
 void SkPictureRecord::onDrawBitmapNine(const SkBitmap& bitmap, const SkIRect& center,
@@ -892,6 +910,16 @@ void SkPictureRecord::addBitmap(const SkBitmap& bitmap) {
     this->addInt(fBitmaps.count()-1);  // Remember, 0-based.
 }
 
+void SkPictureRecord::addImage(const SkImage* image) {
+    int index = fImageRefs.find(image);
+    if (index >= 0) {
+        this->addInt(index);
+    } else {
+        *fImageRefs.append() = SkRef(image);
+        this->addInt(fImageRefs.count()-1);
+    }
+}
+
 void SkPictureRecord::addMatrix(const SkMatrix& matrix) {
     fWriter.writeMatrix(matrix);
 }

src/core/SkPictureRecord.h

@@ -37,6 +37,10 @@ public:
         return fTextBlobRefs;
     }
 
+    const SkTDArray<const SkImage* >& getImageRefs() const {
+        return fImageRefs;
+    }
+    
     SkData* opData(bool deepCopy) const {
         this->validate(fWriter.bytesWritten(), 0);
 
@@ -118,6 +122,7 @@ private:
     }
 
     void addBitmap(const SkBitmap& bitmap);
+    void addImage(const SkImage*);
     void addMatrix(const SkMatrix& matrix);
     void addPaint(const SkPaint& paint) { this->addPaintPtr(&paint); }
     void addPaintPtr(const SkPaint* paint);
@@ -223,6 +228,7 @@ private:
     SkWriter32 fWriter;
 
     // we ref each item in these arrays
+    SkTDArray<const SkImage*>    fImageRefs;
     SkTDArray<const SkPicture*>  fPictureRefs;
     SkTDArray<const SkTextBlob*> fTextBlobRefs;
 

src/core/SkPixelRef.cpp

@@ -259,6 +259,10 @@ bool SkPixelRef::requestLock(const LockRequest& request, LockResult* result) {
     if (request.fSize.isEmpty()) {
         return false;
     }
+    // until we support subsets, we have to check this...
+    if (request.fSize.width() != fInfo.width() || request.fSize.height() != fInfo.height()) {
+        return false;
+    }
 
     if (fPreLocked) {
         result->fUnlockProc = NULL;

src/core/SkReadBuffer.h

@@ -58,6 +58,7 @@ public:
         kImageFilterNoUniqueID_Version     = 40,
         kBitmapSourceFilterQuality_Version = 41,
         kPictureShaderHasPictureBool_Version = 42,
+        kHasDrawImageOpCodes_Version       = 43,
     };
 
     /**

src/core/SkWriteBuffer.cpp

@@ -218,6 +218,34 @@ void SkWriteBuffer::writeBitmap(const SkBitmap& bitmap) {
     SkBitmap::WriteRawPixels(this, bitmap);
 }
 
+static bool try_write_encoded(SkWriteBuffer* buffer, SkData* encoded) {
+    SkPixelSerializer* ps = buffer->getPixelSerializer();
+    // Assumes that if the client did not set a serializer, they are
+    // happy to get the encoded data.
+    if (!ps || ps->useEncodedData(encoded->data(), encoded->size())) {
+        write_encoded_bitmap(buffer, encoded, SkIPoint::Make(0, 0));
+        return true;
+    }
+    return false;
+}
+
+void SkWriteBuffer::writeImage(const SkImage* image) {
+    this->writeInt(image->width());
+    this->writeInt(image->height());
+
+    SkAutoTUnref<SkData> encoded(image->refEncoded());
+    if (encoded && try_write_encoded(this, encoded)) {
+        return;
+    }
+
+    encoded.reset(image->encode(SkImageEncoder::kPNG_Type, 100));
+    if (encoded && try_write_encoded(this, encoded)) {
+        return;
+    }
+    
+    this->writeUInt(0); // signal no pixels (in place of the size of the encoded data)
+}
+
 void SkWriteBuffer::writeTypeface(SkTypeface* obj) {
     if (NULL == obj || NULL == fTFSet) {
         fWriter.write32(0);

src/image/SkImage.cpp

@@ -7,6 +7,7 @@
 
 #include "SkBitmap.h"
 #include "SkCanvas.h"
+#include "SkData.h"
 #include "SkImageGenerator.h"
 #include "SkImagePriv.h"
 #include "SkImage_Base.h"
@@ -64,12 +65,16 @@ SkData* SkImage::encode(SkImageEncoder::Type type, int quality) const {
     return NULL;
 }
 
-SkImage* SkImage::NewFromData(SkData* data) {
+SkData* SkImage::refEncoded() const {
-    if (NULL == data) {
+    return as_IB(this)->onRefEncoded();
+}
+
+SkImage* SkImage::NewFromEncoded(SkData* encoded, const SkIRect* subset) {
+    if (NULL == encoded || 0 == encoded->size()) {
         return NULL;
     }
-    SkImageGenerator* generator = SkImageGenerator::NewFromData(data);
+    SkImageGenerator* generator = SkImageGenerator::NewFromData(encoded);
-    return generator ? SkImage::NewFromGenerator(generator) : NULL;
+    return generator ? SkImage::NewFromGenerator(generator, subset) : NULL;
 }
 
 SkSurface* SkImage::newSurface(const SkImageInfo& info, const SkSurfaceProps* props) const {
@@ -201,7 +206,26 @@ SkImage* SkImage_Base::onNewImage(int newWidth, int newHeight, const SkIRect* su
     return surface->newImageSnapshot();
 }
 
-//////////////////////////////////////////////////////////////////////////////////////
+///////////////////////////////////////////////////////////////////////////////////////////////////
+
+bool SkImage::peekPixels(SkPixmap* pmap) const {
+    SkImageInfo info;
+    size_t rowBytes;
+    const void* pixels = this->peekPixels(&info, &rowBytes);
+    if (pixels) {
+        if (pmap) {
+            pmap->reset(info, pixels, rowBytes);
+        }
+        return true;
+    }
+    return false;
+}
+
+bool SkImage::readPixels(const SkPixmap& pmap, int srcX, int srcY) const {
+    return this->readPixels(pmap.info(), pmap.writable_addr(), pmap.rowBytes(), srcX, srcY);
+}
+
+///////////////////////////////////////////////////////////////////////////////////////////////////
 
 #if !SK_SUPPORT_GPU
 

src/image/SkImage_Base.h

@@ -59,6 +59,7 @@ public:
     // newWidth > 0, newHeight > 0, subset either NULL or a proper subset of this bounds
     virtual SkImage* onNewImage(int newWidth, int newHeight, const SkIRect* subset,
                                 SkFilterQuality) const;
+    virtual SkData* onRefEncoded() const { return NULL; }
 
 private:
     const SkSurfaceProps fProps;

src/image/SkImage_Raster.cpp

@@ -9,7 +9,7 @@
 #include "SkBitmap.h"
 #include "SkCanvas.h"
 #include "SkData.h"
-#include "SkImageGenerator.h"
+#include "SkImageGeneratorPriv.h"
 #include "SkImagePriv.h"
 #include "SkPixelRef.h"
 #include "SkSurface.h"
@@ -59,6 +59,7 @@ public:
     SkSurface* onNewSurface(const SkImageInfo&, const SkSurfaceProps&) const override;
     bool onReadPixels(const SkImageInfo&, void*, size_t, int srcX, int srcY) const override;
     const void* onPeekPixels(SkImageInfo*, size_t* /*rowBytes*/) const override;
+    SkData* onRefEncoded() const override;
     bool getROPixels(SkBitmap*) const override;
 
     // exposed for SkSurface_Raster via SkNewImageFromPixelRef
@@ -141,6 +142,18 @@ const void* SkImage_Raster::onPeekPixels(SkImageInfo* infoPtr, size_t* rowBytesP
     return fBitmap.getPixels();
 }
 
+SkData* SkImage_Raster::onRefEncoded() const {
+    SkPixelRef* pr = fBitmap.pixelRef();
+    const SkImageInfo prInfo = pr->info();
+    const SkImageInfo bmInfo = fBitmap.info();
+
+    // we only try if we (the image) cover the entire area of the pixelRef
+    if (prInfo.width() == bmInfo.width() && prInfo.height() == bmInfo.height()) {
+        return pr->refEncodedData();
+    }
+    return NULL;
+}
+
 bool SkImage_Raster::getROPixels(SkBitmap* dst) const {
     *dst = fBitmap;
     return true;
@@ -185,9 +198,9 @@ SkImage* SkImage::NewFromRaster(const SkImageInfo& info, const void* pixels, siz
     return SkNEW_ARGS(SkImage_Raster, (info, data, rowBytes, NULL));
 }
 
-SkImage* SkImage::NewFromGenerator(SkImageGenerator* generator) {
+SkImage* SkImage::NewFromGenerator(SkImageGenerator* generator, const SkIRect* subset) {
     SkBitmap bitmap;
-    if (!SkInstallDiscardablePixelRef(generator, &bitmap)) {
+    if (!SkInstallDiscardablePixelRef(generator, subset, &bitmap, NULL)) {
         return NULL;
     }
     if (0 == bitmap.width() || 0 == bitmap.height()) {

src/lazy/SkDiscardablePixelRef.cpp

@@ -107,37 +107,55 @@ void SkDiscardablePixelRef::onUnlockPixels() {
     fDiscardableMemoryIsLocked = false;
 }
 
-bool SkInstallDiscardablePixelRef(SkImageGenerator* generator, SkBitmap* dst,
+bool SkInstallDiscardablePixelRef(SkImageGenerator* generator, const SkIRect* subset, SkBitmap* dst,
                                   SkDiscardableMemory::Factory* factory) {
     SkAutoTDelete<SkImageGenerator> autoGenerator(generator);
     if (NULL == autoGenerator.get()) {
         return false;
     }
-    SkImageInfo info = autoGenerator->getInfo();
+
-    if (info.isEmpty() || !dst->setInfo(info)) {
+    SkImageInfo prInfo = autoGenerator->getInfo();
+    if (prInfo.isEmpty()) {
         return false;
     }
-    // Since dst->setInfo() may have changed/fixed-up info, we copy it back from that bitmap
-    info = dst->info();
 
-    SkASSERT(info.colorType() != kUnknown_SkColorType);
+    SkIPoint origin = SkIPoint::Make(0, 0);
+    SkImageInfo bmInfo = prInfo;
+    if (subset) {
+        const SkIRect prBounds = SkIRect::MakeWH(prInfo.width(), prInfo.height());
+        if (subset->isEmpty() || !prBounds.contains(*subset)) {
+            return false;
+        }
+        bmInfo = prInfo.makeWH(subset->width(), subset->height());
+        origin.set(subset->x(), subset->y());
+    }
+
+    // must compute our desired rowBytes w.r.t. the pixelRef's dimensions, not ours, which may be
+    // smaller.
+    if (!dst->setInfo(bmInfo, prInfo.minRowBytes())) {
+        return false;
+    }
+
+    // Since dst->setInfo() may have changed/fixed-up info, we check from the bitmap
+    SkASSERT(dst->info().colorType() != kUnknown_SkColorType);
+
     if (dst->empty()) {  // Use a normal pixelref.
         return dst->tryAllocPixels();
     }
     SkAutoTUnref<SkDiscardablePixelRef> ref(
         SkNEW_ARGS(SkDiscardablePixelRef,
-                   (info, autoGenerator.detach(), dst->rowBytes(), factory)));
+                   (prInfo, autoGenerator.detach(), dst->rowBytes(), factory)));
-    dst->setPixelRef(ref);
+    dst->setPixelRef(ref, origin.x(), origin.y());
     return true;
 }
 
 // These are the public API
 
 bool SkInstallDiscardablePixelRef(SkImageGenerator* generator, SkBitmap* dst) {
-    return SkInstallDiscardablePixelRef(generator, dst, NULL);
+    return SkInstallDiscardablePixelRef(generator, NULL, dst, NULL);
 }
 
 bool SkInstallDiscardablePixelRef(SkData* encoded, SkBitmap* dst) {
     SkImageGenerator* generator = SkImageGenerator::NewFromData(encoded);
-    return generator ? SkInstallDiscardablePixelRef(generator, dst, NULL) : false;
+    return generator ? SkInstallDiscardablePixelRef(generator, NULL, dst, NULL) : false;
 }

src/lazy/SkDiscardablePixelRef.h

@@ -9,7 +9,7 @@
 #define SkDiscardablePixelRef_DEFINED
 
 #include "SkDiscardableMemory.h"
-#include "SkImageGenerator.h"
+#include "SkImageGeneratorPriv.h"
 #include "SkImageInfo.h"
 #include "SkPixelRef.h"
 
@@ -61,7 +61,7 @@ private:
         return fGenerator->getYUV8Planes(sizes, planes, rowBytes, colorSpace);
     }
 
-    friend bool SkInstallDiscardablePixelRef(SkImageGenerator*, SkBitmap*,
+    friend bool SkInstallDiscardablePixelRef(SkImageGenerator*, const SkIRect*, SkBitmap*,
                                              SkDiscardableMemory::Factory*);
 
     typedef SkPixelRef INHERITED;

src/ports/SkImageDecoder_CG.cpp

@@ -157,7 +157,18 @@ SkImageDecoder::Result SkImageDecoder_CG::onDecode(SkStream* stream, SkBitmap* b
         }
     }
 
-    bm->setInfo(SkImageInfo::MakeN32Premul(width, height, cpType));
+    SkAlphaType at = kPremul_SkAlphaType;
+    switch (CGImageGetAlphaInfo(image)) {
+        case kCGImageAlphaNone:
+        case kCGImageAlphaNoneSkipLast:
+        case kCGImageAlphaNoneSkipFirst:
+            at = kOpaque_SkAlphaType;
+            break;
+        default:
+            break;
+    }
+
+    bm->setInfo(SkImageInfo::Make(width, height, kN32_SkColorType, at, cpType));
     if (SkImageDecoder::kDecodeBounds_Mode == mode) {
         return kSuccess;
     }

src/utils/SkLua.cpp

@@ -2007,7 +2007,7 @@ static int lsk_loadImage(lua_State* L) {
         const char* name = lua_tolstring(L, 1, NULL);
         SkAutoDataUnref data(SkData::NewFromFileName(name));
         if (data.get()) {
-            SkImage* image = SkImage::NewFromData(data);
+            SkImage* image = SkImage::NewFromEncoded(data);
             if (image) {
                 push_ref(L, image)->unref();
                 return 1;

tests/CachedDecodingPixelRefTest.cpp

@@ -249,7 +249,7 @@ static void check_pixelref(TestImageGenerator::TestType type,
         // Ignore factory; use global cache.
         success = SkCachingPixelRef::Install(gen.detach(), &lazy);
     } else {
-        success = SkInstallDiscardablePixelRef(gen.detach(), &lazy, factory);
+        success = SkInstallDiscardablePixelRef(gen.detach(), NULL, &lazy, factory);
     }
     REPORTER_ASSERT(reporter, success);
     if (TestImageGenerator::kSucceedGetPixels_TestType == type) {

tests/DrawBitmapRectTest.cpp

@@ -33,8 +33,8 @@ static void test_faulty_pixelref(skiatest::Reporter* reporter) {
     SkAutoTUnref<SkDiscardableMemoryPool> pool(
         SkDiscardableMemoryPool::Create(10 * 1000, NULL));
     SkBitmap bm;
-    bool installSuccess = SkInstallDiscardablePixelRef(SkNEW(FailureImageGenerator), &bm, pool);
+    bool success = SkInstallDiscardablePixelRef(SkNEW(FailureImageGenerator), NULL, &bm, pool);
-    REPORTER_ASSERT(reporter, installSuccess);
+    REPORTER_ASSERT(reporter, success);
     // now our bitmap has a pixelref, but we know it will fail to lock
 
     SkAutoTUnref<SkSurface> surface(SkSurface::NewRasterN32Premul(200, 200));

tests/ImageTest.cpp

@@ -0,0 +1,105 @@
+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkCanvas.h"
+#include "SkData.h"
+#include "SkDevice.h"
+#include "SkImageEncoder.h"
+#include "SkImage_Base.h"
+#include "SkRRect.h"
+#include "SkSurface.h"
+#include "SkUtils.h"
+#include "Test.h"
+
+#if SK_SUPPORT_GPU
+#include "GrContextFactory.h"
+#include "GrTest.h"
+#include "gl/GrGLInterface.h"
+#include "gl/GrGLUtil.h"
+#else
+class GrContextFactory;
+class GrContext;
+#endif
+
+static void assert_equal(skiatest::Reporter* reporter, SkImage* a, const SkIRect* subsetA,
+                         SkImage* b) {
+    const int widthA = subsetA ? subsetA->width() : a->width();
+    const int heightA = subsetA ? subsetA->height() : a->height();
+
+    REPORTER_ASSERT(reporter, widthA == b->width());
+    REPORTER_ASSERT(reporter, heightA == b->height());
+#if 0
+    // see skbug.com/3965
+    bool AO = a->isOpaque();
+    bool BO = b->isOpaque();
+    REPORTER_ASSERT(reporter, AO == BO);
+#endif
+
+    SkImageInfo info = SkImageInfo::MakeN32(widthA, heightA,
+                                        a->isOpaque() ? kOpaque_SkAlphaType : kPremul_SkAlphaType);
+    SkAutoPixmapStorage pmapA, pmapB;
+    pmapA.alloc(info);
+    pmapB.alloc(info);
+
+    const int srcX = subsetA ? subsetA->x() : 0;
+    const int srcY = subsetA ? subsetA->y() : 0;
+
+    REPORTER_ASSERT(reporter, a->readPixels(pmapA, srcX, srcY));
+    REPORTER_ASSERT(reporter, b->readPixels(pmapB, 0, 0));
+
+    const size_t widthBytes = widthA * info.bytesPerPixel();
+    for (int y = 0; y < heightA; ++y) {
+        REPORTER_ASSERT(reporter, !memcmp(pmapA.addr32(0, y), pmapB.addr32(0, y), widthBytes));
+    }
+}
+
+static SkImage* make_image(GrContext* ctx, int w, int h, const SkIRect& ir) {
+    const SkImageInfo info = SkImageInfo::MakeN32(w, h, kOpaque_SkAlphaType);
+    SkAutoTUnref<SkSurface> surface(ctx ?
+                                    SkSurface::NewRenderTarget(ctx, SkSurface::kNo_Budgeted, info) :
+                                    SkSurface::NewRaster(info));
+    SkCanvas* canvas = surface->getCanvas();
+    canvas->clear(SK_ColorWHITE);
+
+    SkPaint paint;
+    paint.setColor(SK_ColorBLACK);
+    canvas->drawRect(SkRect::Make(ir), paint);
+    return surface->newImageSnapshot();
+}
+
+static void test_encode(skiatest::Reporter* reporter, GrContext* ctx) {
+    const SkIRect ir = SkIRect::MakeXYWH(5, 5, 10, 10);
+    SkAutoTUnref<SkImage> orig(make_image(ctx, 20, 20, ir));
+    SkAutoTUnref<SkData> origEncoded(orig->encode());
+    REPORTER_ASSERT(reporter, origEncoded);
+    REPORTER_ASSERT(reporter, origEncoded->size() > 0);
+
+    SkAutoTUnref<SkImage> decoded(SkImage::NewFromEncoded(origEncoded));
+    REPORTER_ASSERT(reporter, decoded);
+    assert_equal(reporter, orig, NULL, decoded);
+
+    // Now see if we can instantiate an image from a subset of the surface/origEncoded
+    
+    decoded.reset(SkImage::NewFromEncoded(origEncoded, &ir));
+    REPORTER_ASSERT(reporter, decoded);
+    assert_equal(reporter, orig, &ir, decoded);
+}
+
+DEF_TEST(Image_Encode_Cpu, reporter) {
+    test_encode(reporter, NULL);
+}
+
+#if SK_SUPPORT_GPU
+DEF_GPUTEST(Image_Encode_Gpu, reporter, factory) {
+    GrContext* ctx = factory->get(GrContextFactory::kNative_GLContextType);
+    if (!ctx) {
+        REPORTER_ASSERT(reporter, false);
+        return;
+    }
+    test_encode(reporter, ctx);
+}
+#endif

tests/SurfaceTest.cpp

@@ -251,7 +251,7 @@ static SkImage* createImage(ImageType imageType, GrContext* context, SkColor col
             bitmap.installPixels(info, addr, rowBytes);
             SkAutoTUnref<SkData> src(
                  SkImageEncoder::EncodeData(bitmap, SkImageEncoder::kPNG_Type, 100));
-            return SkImage::NewFromData(src);
+            return SkImage::NewFromEncoded(src);
         }
     }
     SkASSERT(false);

tools/LazyDecodeBitmap.cpp

@@ -40,5 +40,5 @@ bool sk_tools::LazyDecodeBitmap(const void* src, size_t length, SkBitmap* dst) {
         // Only meaningful if platform has a default discardable
         // memory implementation that differs from the global DM pool.
     }
-    return SkInstallDiscardablePixelRef(gen.detach(), dst, pool);
+    return SkInstallDiscardablePixelRef(gen.detach(), NULL, dst, pool);
 }