stop calling SkBitmap::flatten

BUG=skia:
R=scroggo@google.com, halcanary@google.com

Author: reed@google.com

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

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

10 files changed, 128 insertions, 52 deletions

diff --git a/include/core/SkBitmap.h b/include/core/SkBitmap.h
index 6ffe1bb692..29ff2c5a1a 100644
--- a/include/core/SkBitmap.h
+++ b/include/core/SkBitmap.h
@@ -24,6 +24,8 @@ class SkRegion;
 class SkString;
 class GrTexture;
 
+//#define SK_SUPPORT_LEGACY_BITMAPFLATTEN
+
 /** \class SkBitmap
 
     The SkBitmap class specifies a raster bitmap. A bitmap has an integer width
@@ -691,6 +693,7 @@ public:
     bool extractAlpha(SkBitmap* dst, const SkPaint* paint, Allocator* allocator,
                       SkIPoint* offset) const;
 
+#ifdef SK_SUPPORT_LEGACY_BITMAPFLATTEN
     /** The following two functions provide the means to both flatten and
         unflatten the bitmap AND its pixels into the provided buffer.
         It is recommended that you do not call these functions directly,
@@ -699,7 +702,11 @@ public:
         duplicate bitmaps and pixelRefs.
      */
     void flatten(SkWriteBuffer&) const;
+#else
+private:
+#endif
     void unflatten(SkReadBuffer&);
+public:
 
     SkDEBUGCODE(void validate() const;)
 
@@ -796,7 +803,13 @@ private:
     */
     void freePixels();
     void updatePixelsFromRef() const;
+    
+    static void WriteRawPixels(SkWriteBuffer*, const SkBitmap&);
+    static bool ReadRawPixels(SkReadBuffer*, SkBitmap*);
 
+    friend class SkBitmapSource;    // unflatten
+    friend class SkReadBuffer;      // unflatten, rawpixels
+    friend class SkWriteBuffer;     // rawpixels
     friend struct SkBitmapProcState;
 };
 
diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h
index 58335ac728..44681ea7e6 100644
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -328,13 +328,14 @@ protected:
     // V25: SkDashPathEffect now only writes phase and interval array when flattening
     // V26: Removed boolean from SkColorShader for inheriting color from SkPaint.
     // V27: Remove SkUnitMapper from gradients (and skia).
+    // V28: No longer call bitmap::flatten inside SkWriteBuffer::writeBitmap.
 
     // Note: If the picture version needs to be increased then please follow the
     // steps to generate new SKPs in (only accessible to Googlers): http://goo.gl/qATVcw
 
     // Only SKPs within the min/current picture version range (inclusive) can be read.
     static const uint32_t MIN_PICTURE_VERSION = 19;
-    static const uint32_t CURRENT_PICTURE_VERSION = 27;
+    static const uint32_t CURRENT_PICTURE_VERSION = 28;
 
     mutable uint32_t      fUniqueID;
 
diff --git a/include/core/SkReadBuffer.h b/include/core/SkReadBuffer.h
index e962234ddb..5364bee285 100644
--- a/include/core/SkReadBuffer.h
+++ b/include/core/SkReadBuffer.h
@@ -45,6 +45,7 @@ public:
         kDashWritesPhaseIntervals_Version  = 25,
         kColorShaderNoBool_Version         = 26,
         kNoUnitMappers_Version             = 27,
+        kNoMoreBitmapFlatten_Version       = 28,
     };
 
     /**
@@ -149,7 +150,12 @@ public:
     // helpers to get info about arrays and binary data
     virtual uint32_t getArrayCount();
 
-    virtual void readBitmap(SkBitmap* bitmap);
+    /**
+     *  Returns false if the bitmap could not be completely read. In that case, it will be set
+     *  to have width/height, but no pixels.
+     */
+    bool readBitmap(SkBitmap* bitmap);
+
     virtual SkTypeface* readTypeface();
 
     void setBitmapStorage(SkBitmapHeapReader* bitmapStorage) {
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 84f363e564..70d99074d1 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -1266,11 +1266,88 @@ bool SkBitmap::extractAlpha(SkBitmap* dst, const SkPaint* paint,
 
 ///////////////////////////////////////////////////////////////////////////////
 
+void SkBitmap::WriteRawPixels(SkWriteBuffer* buffer, const SkBitmap& bitmap) {
+    const SkImageInfo info = bitmap.info();
+    SkAutoLockPixels alp(bitmap);
+    if (0 == info.width() || 0 == info.height() || NULL == bitmap.getPixels()) {
+        buffer->writeUInt(0); // instead of snugRB, signaling no pixels
+        return;
+    }
+
+    const size_t snugRB = info.width() * info.bytesPerPixel();
+    const char* src = (const char*)bitmap.getPixels();
+    const size_t ramRB = bitmap.rowBytes();
+    
+    buffer->write32(SkToU32(snugRB));
+    info.flatten(*buffer);
+
+    const size_t size = snugRB * info.height();
+    SkAutoMalloc storage(size);
+    char* dst = (char*)storage.get();
+    for (int y = 0; y < info.height(); ++y) {
+        memcpy(dst, src, snugRB);
+        dst += snugRB;
+        src += ramRB;
+    }
+    buffer->writeByteArray(storage.get(), size);
+
+    SkColorTable* ct = bitmap.getColorTable();
+    if (kIndex_8_SkColorType == info.colorType() && ct) {
+        buffer->writeBool(true);
+        ct->writeToBuffer(*buffer);
+    } else {
+        buffer->writeBool(false);
+    }
+}
+
+bool SkBitmap::ReadRawPixels(SkReadBuffer* buffer, SkBitmap* bitmap) {
+    const size_t snugRB = buffer->readUInt();
+    if (0 == snugRB) {  // no pixels
+        return false;
+    }
+
+    SkImageInfo info;
+    info.unflatten(*buffer);
+
+    const size_t ramRB = info.minRowBytes();
+    const int height = info.height();
+    const size_t snugSize = snugRB * height;
+    const size_t ramSize = ramRB * height;
+    SkASSERT(snugSize <= ramSize);
+
+    char* dst = (char*)sk_malloc_throw(ramSize);
+    buffer->readByteArray(dst, snugSize);
+    SkAutoDataUnref data(SkData::NewFromMalloc(dst, ramSize));
+
+    if (snugSize != ramSize) {
+        const char* srcRow = dst + snugRB * (height - 1);
+        char* dstRow = dst + ramRB * (height - 1);
+        for (int y = height - 1; y >= 1; --y) {
+            memmove(dstRow, srcRow, snugRB);
+            srcRow -= snugRB;
+            dstRow -= ramRB;
+        }
+        SkASSERT(srcRow == dstRow); // first row does not need to be moved
+    }
+    
+    SkAutoTUnref<SkColorTable> ctable;
+    if (buffer->readBool()) {
+        ctable.reset(SkNEW_ARGS(SkColorTable, (*buffer)));
+    }
+    
+    SkAutoTUnref<SkPixelRef> pr(SkMallocPixelRef::NewWithData(info, info.minRowBytes(),
+                                                              ctable.get(), data.get()));
+    bitmap->setConfig(pr->info());
+    bitmap->setPixelRef(pr, 0, 0);
+    return true;
+}
+
 enum {
     SERIALIZE_PIXELTYPE_NONE,
     SERIALIZE_PIXELTYPE_REF_DATA
 };
 
+#ifdef SK_SUPPORT_LEGACY_BITMAPFLATTEN
 void SkBitmap::flatten(SkWriteBuffer& buffer) const {
     fInfo.flatten(buffer);
     buffer.writeInt(fRowBytes);
@@ -1289,6 +1366,7 @@ void SkBitmap::flatten(SkWriteBuffer& buffer) const {
         buffer.writeInt(SERIALIZE_PIXELTYPE_NONE);
     }
 }
+#endif
 
 void SkBitmap::unflatten(SkReadBuffer& buffer) {
     this->reset();
diff --git a/src/core/SkReadBuffer.cpp b/src/core/SkReadBuffer.cpp
index c32c7bd658..3f21078b1f 100644
--- a/src/core/SkReadBuffer.cpp
+++ b/src/core/SkReadBuffer.cpp
@@ -191,7 +191,7 @@ uint32_t SkReadBuffer::getArrayCount() {
     return *(uint32_t*)fReader.peek();
 }
 
-void SkReadBuffer::readBitmap(SkBitmap* bitmap) {
+bool SkReadBuffer::readBitmap(SkBitmap* bitmap) {
     const int width = this->readInt();
     const int height = this->readInt();
     // The writer stored a boolean value to determine whether an SkBitmapHeap was used during
@@ -204,7 +204,7 @@ void SkReadBuffer::readBitmap(SkBitmap* bitmap) {
         if (fBitmapStorage) {
             *bitmap = *fBitmapStorage->getBitmap(index);
             fBitmapStorage->releaseRef(index);
-            return;
+            return true;
         } else {
             // The bitmap was stored in a heap, but there is no way to access it. Set an error and
             // fall through to use a place holder bitmap.
@@ -239,7 +239,7 @@ void SkReadBuffer::readBitmap(SkBitmap* bitmap) {
 #endif // DEBUG_NON_DETERMINISTIC_ASSERT
                     // If the width and height match, there should be no offset.
                     SkASSERT(0 == xOffset && 0 == yOffset);
-                    return;
+                    return true;
                 }
 
                 // This case can only be reached if extractSubset was called, so
@@ -255,7 +255,7 @@ void SkReadBuffer::readBitmap(SkBitmap* bitmap) {
                 SkIRect subset = SkIRect::MakeXYWH(xOffset, yOffset, width, height);
                 if (bitmap->extractSubset(&subsetBm, subset)) {
                     bitmap->swap(subsetBm);
-                    return;
+                    return true;
                 }
             }
             // This bitmap was encoded when written, but we are unable to decode, possibly due to
@@ -264,13 +264,20 @@ void SkReadBuffer::readBitmap(SkBitmap* bitmap) {
                                        "Could not decode bitmap. Resulting bitmap will be red.");
         } else {
             // A size of zero means the SkBitmap was simply flattened.
-            bitmap->unflatten(*this);
-            return;
+            if (this->isVersionLT(kNoMoreBitmapFlatten_Version)) {
+                SkBitmap tmp;
+                tmp.unflatten(*this);
+                // just throw this guy away
+            } else {
+                if (SkBitmap::ReadRawPixels(this, bitmap)) {
+                    return true;
+                }
+            }
         }
     }
     // Could not read the SkBitmap. Use a placeholder bitmap.
-    bitmap->allocPixels(SkImageInfo::MakeN32Premul(width, height));
-    bitmap->eraseColor(SK_ColorRED);
+    bitmap->setConfig(SkImageInfo::MakeUnknown(width, height));
+    return false;
 }
 
 SkTypeface* SkReadBuffer::readTypeface() {
diff --git a/src/core/SkValidatingReadBuffer.cpp b/src/core/SkValidatingReadBuffer.cpp
index 2f530e449d..037a99465b 100644
--- a/src/core/SkValidatingReadBuffer.cpp
+++ b/src/core/SkValidatingReadBuffer.cpp
@@ -210,19 +210,6 @@ uint32_t SkValidatingReadBuffer::getArrayCount() {
     return fError ? 0 : *(uint32_t*)fReader.peek();
 }
 
-void SkValidatingReadBuffer::readBitmap(SkBitmap* bitmap) {
-    const int width = this->readInt();
-    const int height = this->readInt();
-    const bool useBitmapHeap = this->readBool();
-    const size_t length = this->readUInt();
-    // A size of zero means the SkBitmap was simply flattened.
-    if (!this->validate(!useBitmapHeap && (0 == length))) {
-        return;
-    }
-    bitmap->unflatten(*this);
-    this->validate((bitmap->width() == width) && (bitmap->height() == height));
-}
-
 SkTypeface* SkValidatingReadBuffer::readTypeface() {
     // TODO: Implement this (securely) when needed
     return NULL;
diff --git a/src/core/SkValidatingReadBuffer.h b/src/core/SkValidatingReadBuffer.h
index 12f5413109..0a9e2536d4 100644
--- a/src/core/SkValidatingReadBuffer.h
+++ b/src/core/SkValidatingReadBuffer.h
@@ -58,7 +58,6 @@ public:
     // helpers to get info about arrays and binary data
     virtual uint32_t getArrayCount() SK_OVERRIDE;
 
-    virtual void readBitmap(SkBitmap* bitmap) SK_OVERRIDE;
     // TODO: Implement this (securely) when needed
     virtual SkTypeface* readTypeface() SK_OVERRIDE;
 
diff --git a/src/core/SkWriteBuffer.cpp b/src/core/SkWriteBuffer.cpp
index cca7d8170a..f2be41b266 100644
--- a/src/core/SkWriteBuffer.cpp
+++ b/src/core/SkWriteBuffer.cpp
@@ -206,9 +206,8 @@ void SkWriteBuffer::writeBitmap(const SkBitmap& bitmap) {
         }
     }
 
-    // Bitmap was not encoded. Record a zero, implying that the reader need not decode.
-    this->writeUInt(0);
-    bitmap.flatten(*this);
+    this->writeUInt(0); // signal raw pixels
+    SkBitmap::WriteRawPixels(this, bitmap);
 }
 
 void SkWriteBuffer::writeTypeface(SkTypeface* obj) {
diff --git a/src/effects/SkBitmapSource.cpp b/src/effects/SkBitmapSource.cpp
index 6834e9e32e..79f6e92c1d 100644
--- a/src/effects/SkBitmapSource.cpp
+++ b/src/effects/SkBitmapSource.cpp
@@ -21,15 +21,17 @@ SkBitmapSource::SkBitmapSource(const SkBitmap& bitmap)
 }
 
 SkBitmapSource::SkBitmapSource(const SkBitmap& bitmap, const SkRect& srcRect, const SkRect& dstRect)
-  : INHERITED(0, 0),
-    fBitmap(bitmap),
-    fSrcRect(srcRect),
-    fDstRect(dstRect) {
-}
+  : INHERITED(0, 0)
+  , fBitmap(bitmap)
+  , fSrcRect(srcRect)
+  , fDstRect(dstRect) {}
 
-SkBitmapSource::SkBitmapSource(SkReadBuffer& buffer)
-  : INHERITED(0, buffer) {
-    fBitmap.unflatten(buffer);
+SkBitmapSource::SkBitmapSource(SkReadBuffer& buffer) : INHERITED(0, buffer) {
+    if (buffer.isVersionLT(SkReadBuffer::kNoMoreBitmapFlatten_Version)) {
+        fBitmap.unflatten(buffer);
+    } else {
+        buffer.readBitmap(&fBitmap);
+    }
     buffer.readRect(&fSrcRect);
     buffer.readRect(&fDstRect);
     buffer.validate(buffer.isValid() && SkIsValidRect(fSrcRect) && SkIsValidRect(fDstRect));
@@ -37,7 +39,7 @@ SkBitmapSource::SkBitmapSource(SkReadBuffer& buffer)
 
 void SkBitmapSource::flatten(SkWriteBuffer& buffer) const {
     this->INHERITED::flatten(buffer);
-    fBitmap.flatten(buffer);
+    buffer.writeBitmap(fBitmap);
     buffer.writeRect(fSrcRect);
     buffer.writeRect(fDstRect);
 }
diff --git a/tests/FlatDataTest.cpp b/tests/FlatDataTest.cpp
index 2de270d50e..2be92b6f41 100644
--- a/tests/FlatDataTest.cpp
+++ b/tests/FlatDataTest.cpp
@@ -21,11 +21,6 @@ struct SkFlattenableTraits {
         buffer.writeFlattenable(&flattenable);
     }
 };
-struct SkBitmapTraits {
-    static void Flatten(SkWriteBuffer& buffer, const SkBitmap& bitmap) {
-        bitmap.flatten(buffer);
-    }
-};
 
 class Controller : public SkChunkFlatController {
 public:
@@ -65,17 +60,6 @@ DEF_TEST(FlatData, reporter) {
                                                                  SkShader::kRepeat_TileMode));
     testCreate<SkFlattenableTraits>(reporter, *shader);
 
-    // Test SkBitmap
-    {
-        SkBitmap bm;
-        bm.allocN32Pixels(50, 50);
-        SkCanvas canvas(bm);
-        SkPaint paint;
-        paint.setShader(shader);
-        canvas.drawPaint(paint);
-        testCreate<SkBitmapTraits>(reporter, bm);
-    }
-
     // Test SkColorFilter
     SkAutoTUnref<SkColorFilter> cf(SkColorFilter::CreateLightingFilter(SK_ColorBLUE, SK_ColorRED));
     testCreate<SkFlattenableTraits>(reporter, *cf);