stop calling SkBitmap::flatten

src/core/SkBitmap.cpp

 
 /*
  * Copyright 2008 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 
 #include "SkBitmap.h"
@@ skipping 1240 matching lines @@
         offset->set(dstM.fBounds.fLeft, dstM.fBounds.fTop);
     }
     SkDEBUGCODE(tmpBitmap.validate();)
 
     tmpBitmap.swap(*dst);
     return true;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
+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* srcP = (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* srcBytes = (char*)storage.get();

[scroggo, 2014/05/21 14:48:01]

srcBytes is confusing here. How about dstBytes?

[reed1, 2014/05/21 20:33:04]

Done.

+    for (int y = 0; y < info.height(); ++y) {
+        memcpy(srcBytes, srcP, snugRB);
+        srcBytes += snugRB;
+        srcP += 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);
+    }
+}
+
+SkPixelRef* SkBitmap::ReadRawPixels(SkReadBuffer* buffer) {

[hal.canary, 2014/05/21 15:18:11]

void SkBitmap::WriteRawPixels(SkWriteBuffer* buffer, const SkBitmap& bitmap);
SkPixelRef* SkBitmap::ReadRawPixels(SkReadBuffer* buffer);

Why are these asymmetric?  Why not modify a destination bitmap?

[reed1, 2014/05/21 20:33:04]

Done.

+    const size_t snugRB = buffer->readUInt();
+    if (0 == snugRB) {  // no pixels
+        return NULL;
+    }
+
+    SkImageInfo info;
+    info.unflatten(*buffer);
+
+    const size_t ramRB = info.minRowBytes();

[hal.canary, 2014/05/21 15:20:29]

When is ramRB != snugRB?

[reed1, 2014/05/21 20:33:04]

snugRB is the smallest that they *could* be (used for storage). ramRB could
including padding, which is perfectly allowed in RAM.

[hal.canary, 2014/05/21 20:39:49]

But you set `ramRB = info.minRowBytes()`, which is width * bytesPerPixel, which
is what you selected for snugRB.  I still don't see how they could be unequal.

+    const int height = info.height();
+    const size_t snugSize = snugRB * height;
+    const size_t ramSize = ramRB * height;
+    SkASSERT(snugSize <= ramSize);
+
+    char* srcBytes = (char*)sk_malloc_throw(ramSize);

[scroggo, 2014/05/21 14:48:01]

Again, I find srcBytes confusing here. dstBytes? ramBytes?

[reed1, 2014/05/21 20:33:04]

Done.

+    buffer->readByteArray(srcBytes, snugSize);
+    SkAutoDataUnref data(SkData::NewFromMalloc(srcBytes, ramSize));
+
+    if (snugSize != ramSize) {
+        const char* srcRow = srcBytes + snugRB * (height - 1);
+        char* dstRow = srcBytes + 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)));
+    }
+
+    return SkMallocPixelRef::NewWithData(info, info.minRowBytes(), ctable.get(), data.get());
+}
+
 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);
 
     if (fPixelRef) {
         if (fPixelRef->getFactory()) {
             buffer.writeInt(SERIALIZE_PIXELTYPE_REF_DATA);
             buffer.writeInt(fPixelRefOrigin.fX);
             buffer.writeInt(fPixelRefOrigin.fY);
             buffer.writeFlattenable(fPixelRef);
             return;
         }
         // if we get here, we can't record the pixels
         buffer.writeInt(SERIALIZE_PIXELTYPE_NONE);
     } else {
         buffer.writeInt(SERIALIZE_PIXELTYPE_NONE);
     }
 }
+#endif
 
 void SkBitmap::unflatten(SkReadBuffer& buffer) {
     this->reset();
 
     SkImageInfo info;
     info.unflatten(buffer);
     size_t rowBytes = buffer.readInt();
     if (!buffer.validate((info.width() >= 0) && (info.height() >= 0) &&
                          SkColorTypeIsValid(info.fColorType) &&
                          SkAlphaTypeIsValid(info.fAlphaType) &&
@@ skipping 121 matching lines @@
 ///////////////////////////////////////////////////////////////////////////////
 
 #ifdef SK_DEBUG
 void SkImageInfo::validate() const {
     SkASSERT(fWidth >= 0);
     SkASSERT(fHeight >= 0);
     SkASSERT(SkColorTypeIsValid(fColorType));
     SkASSERT(SkAlphaTypeIsValid(fAlphaType));
 }
 #endif