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
diff --git a/include/core/SkBitmap.h b/include/core/SkBitmap.h
index 6ffe1bb..29ff2c5 100644
--- a/include/core/SkBitmap.h
+++ b/include/core/SkBitmap.h
@@ -24,6 +24,8 @@
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 @@
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 @@
duplicate bitmaps and pixelRefs.
*/
void flatten(SkWriteBuffer&) const;
+#else
+private:
+#endif
void unflatten(SkReadBuffer&);
+public:
SkDEBUGCODE(void validate() const;)
@@ -796,7 +803,13 @@
*/
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 58335ac..44681ea 100644
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -328,13 +328,14 @@
// 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 e962234..5364bee 100644
--- a/include/core/SkReadBuffer.h
+++ b/include/core/SkReadBuffer.h
@@ -45,6 +45,7 @@
kDashWritesPhaseIntervals_Version = 25,
kColorShaderNoBool_Version = 26,
kNoUnitMappers_Version = 27,
+ kNoMoreBitmapFlatten_Version = 28,
};
/**
@@ -149,7 +150,12 @@
// 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 84f363e..70d9907 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -1266,11 +1266,88 @@
///////////////////////////////////////////////////////////////////////////////
+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 @@
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 c32c7bd..3f21078 100644
--- a/src/core/SkReadBuffer.cpp
+++ b/src/core/SkReadBuffer.cpp
@@ -191,7 +191,7 @@
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 @@
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 @@
#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 @@
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 @@
"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 2f530e4..037a994 100644
--- a/src/core/SkValidatingReadBuffer.cpp
+++ b/src/core/SkValidatingReadBuffer.cpp
@@ -210,19 +210,6 @@
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 12f5413..0a9e253 100644
--- a/src/core/SkValidatingReadBuffer.h
+++ b/src/core/SkValidatingReadBuffer.h
@@ -58,7 +58,6 @@
// 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 cca7d81..f2be41b 100644
--- a/src/core/SkWriteBuffer.cpp
+++ b/src/core/SkWriteBuffer.cpp
@@ -206,9 +206,8 @@
}
}
- // 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 6834e9e..79f6e92 100644
--- a/src/effects/SkBitmapSource.cpp
+++ b/src/effects/SkBitmapSource.cpp
@@ -21,15 +21,17 @@
}
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 @@
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 2de270d..2be92b6 100644
--- a/tests/FlatDataTest.cpp
+++ b/tests/FlatDataTest.cpp
@@ -21,11 +21,6 @@
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 @@
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);