From c84b8335ee4fd864c30a4703afc643cf4b5010d9 Mon Sep 17 00:00:00 2001 From: "djsollen@google.com" Date: Fri, 27 Jul 2012 13:41:44 +0000 Subject: [PATCH] Reapply "Remove Bitmaps Raw Pixel Support." This CL reapplies r4722. Now that we have addressed the issues in Chrome tests that were causing asserts to be fired. Review URL: https://codereview.appspot.com/6452050 git-svn-id: http://skia.googlecode.com/svn/trunk@4804 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/core/SkBitmap.h | 16 ++++------ include/core/SkMallocPixelRef.h | 3 +- src/core/SkBitmap.cpp | 71 +++++------------------------------------ src/core/SkMallocPixelRef.cpp | 9 ++++-- src/core/SkPixelRef.cpp | 1 - 5 files changed, 23 insertions(+), 77 deletions(-) diff --git a/include/core/SkBitmap.h b/include/core/SkBitmap.h index 5673fb2..9ddd702 100644 --- a/include/core/SkBitmap.h +++ b/include/core/SkBitmap.h @@ -94,10 +94,10 @@ public: */ bool empty() const { return 0 == fWidth || 0 == fHeight; } - /** Return true iff the bitmap has no pixels nor a pixelref. Note: this can - return true even if the dimensions of the bitmap are > 0 (see empty()). + /** Return true iff the bitmap has no pixelref. Note: this can return true even if the + dimensions of the bitmap are > 0 (see empty()). */ - bool isNull() const { return NULL == fPixels && NULL == fPixelRef; } + bool isNull() const { return NULL == fPixelRef; } /** Return the config for the bitmap. */ @@ -361,9 +361,9 @@ public: SkColorTable* getColorTable() const { return fColorTable; } /** Returns a non-zero, unique value corresponding to the pixels in our - pixelref (or raw pixels set via setPixels). Each time the pixels are - changed (and notifyPixelsChanged is called), a different generation ID - will be returned. + pixelref. Each time the pixels are changed (and notifyPixelsChanged + is called), a different generation ID will be returned. Finally, if + their is no pixelRef then zero is returned. */ uint32_t getGenerationID() const; @@ -612,10 +612,6 @@ private: // or a cache of the returned value from fPixelRef->lockPixels() mutable void* fPixels; mutable SkColorTable* fColorTable; // only meaningful for kIndex8 - // When there is no pixel ref (setPixels was called) we still need a - // gen id for SkDevice implementations that may cache a copy of the - // pixels (e.g. as a gpu texture) - mutable int fRawPixelGenerationID; enum Flags { kImageIsOpaque_Flag = 0x01, diff --git a/include/core/SkMallocPixelRef.h b/include/core/SkMallocPixelRef.h index 802e2b2..1bf1afc 100644 --- a/include/core/SkMallocPixelRef.h +++ b/include/core/SkMallocPixelRef.h @@ -21,7 +21,7 @@ public: last owner of this pixelref is gone. If addr is NULL, sk_malloc_throw() is called to allocate it. */ - SkMallocPixelRef(void* addr, size_t size, SkColorTable* ctable); + SkMallocPixelRef(void* addr, size_t size, SkColorTable* ctable, bool ownPixels = true); virtual ~SkMallocPixelRef(); //! Return the allocation size for the pixels @@ -42,6 +42,7 @@ private: void* fStorage; size_t fSize; SkColorTable* fCTable; + bool fOwnPixels; typedef SkPixelRef INHERITED; }; diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp index b1a4b7e..687cf7a 100644 --- a/src/core/SkBitmap.cpp +++ b/src/core/SkBitmap.cpp @@ -22,8 +22,6 @@ SK_DEFINE_INST_COUNT(SkBitmap::Allocator) -extern int32_t SkNextPixelRefGenerationID(); - static bool isPos32Bits(const Sk64& value) { return !value.isNeg() && value.is32(); } @@ -139,7 +137,6 @@ void SkBitmap::swap(SkBitmap& other) { SkTSwap(fPixelLockCount, other.fPixelLockCount); SkTSwap(fMipMap, other.fMipMap); SkTSwap(fPixels, other.fPixels); - SkTSwap(fRawPixelGenerationID, other.fRawPixelGenerationID); SkTSwap(fRowBytes, other.fRowBytes); SkTSwap(fWidth, other.fWidth); SkTSwap(fHeight, other.fHeight); @@ -359,18 +356,16 @@ void SkBitmap::unlockPixels() const { } bool SkBitmap::lockPixelsAreWritable() const { - if (fPixelRef) { - return fPixelRef->lockPixelsAreWritable(); - } else { - return fPixels != NULL; - } + return (fPixelRef) ? fPixelRef->lockPixelsAreWritable() : false; } void SkBitmap::setPixels(void* p, SkColorTable* ctable) { - this->freePixels(); - fPixels = p; - SkRefCnt_SafeAssign(fColorTable, ctable); + Sk64 size = this->getSize64(); + SkASSERT(!size.isNeg() && size.is32()); + this->setPixelRef(new SkMallocPixelRef(p, size.get32(), ctable, false))->unref(); + // since we're already allocated, we lockPixels right away + this->lockPixels(); SkDEBUGCODE(this->validate();) } @@ -412,23 +407,13 @@ void SkBitmap::freeMipMap() { } uint32_t SkBitmap::getGenerationID() const { - if (fPixelRef) { - return fPixelRef->getGenerationID(); - } else { - SkASSERT(fPixels || !fRawPixelGenerationID); - if (fPixels && !fRawPixelGenerationID) { - fRawPixelGenerationID = SkNextPixelRefGenerationID(); - } - return fRawPixelGenerationID; - } + return (fPixelRef) ? fPixelRef->getGenerationID() : 0; } void SkBitmap::notifyPixelsChanged() const { SkASSERT(!this->isImmutable()); if (fPixelRef) { fPixelRef->notifyPixelsChanged(); - } else { - fRawPixelGenerationID = 0; // will grab next ID in getGenerationID } } @@ -791,7 +776,7 @@ static size_t getSubOffset(const SkBitmap& bm, int x, int y) { bool SkBitmap::extractSubset(SkBitmap* result, const SkIRect& subset) const { SkDEBUGCODE(this->validate();) - if (NULL == result || (NULL == fPixelRef && NULL == fPixels)) { + if (NULL == result || NULL == fPixelRef) { return false; // no src pixels } @@ -841,9 +826,6 @@ bool SkBitmap::extractSubset(SkBitmap* result, const SkIRect& subset) const { if (fPixelRef) { // share the pixelref with a custom offset dst.setPixelRef(fPixelRef, fPixelRefOffset + offset); - } else { - // share the pixels (owned by the caller) - dst.setPixels((char*)fPixels + offset, this->getColorTable()); } SkDEBUGCODE(dst.validate();) @@ -1376,8 +1358,6 @@ bool SkBitmap::extractAlpha(SkBitmap* dst, const SkPaint* paint, enum { SERIALIZE_PIXELTYPE_NONE, - SERIALIZE_PIXELTYPE_RAW_WITH_CTABLE, - SERIALIZE_PIXELTYPE_RAW_NO_CTABLE, SERIALIZE_PIXELTYPE_REF_DATA, SERIALIZE_PIXELTYPE_REF_PTR }; @@ -1429,21 +1409,6 @@ void SkBitmap::flatten(SkFlattenableWriteBuffer& buffer) const { } // if we get here, we can't record the pixels buffer.write8(SERIALIZE_PIXELTYPE_NONE); - } else if (fPixels) { - if (fColorTable) { - buffer.write8(SERIALIZE_PIXELTYPE_RAW_WITH_CTABLE); - buffer.writeFlattenable(fColorTable); - } else { - buffer.write8(SERIALIZE_PIXELTYPE_RAW_NO_CTABLE); - } - buffer.writePad(fPixels, this->getSafeSize()); - // There is no writeZeroPad() fcn, so write individual bytes. - if (this->getSize() > this->getSafeSize()) { - size_t deltaSize = this->getSize() - this->getSafeSize(); - // Need aligned pointer to write into due to internal implementa- - // tion of SkWriter32. - memset(buffer.reserve(SkAlign4(deltaSize)), 0, deltaSize); - } } else { buffer.write8(SERIALIZE_PIXELTYPE_NONE); } @@ -1474,26 +1439,6 @@ void SkBitmap::unflatten(SkFlattenableReadBuffer& buffer) { SkSafeUnref(this->setPixelRef(pr, offset)); break; } - case SERIALIZE_PIXELTYPE_RAW_WITH_CTABLE: - case SERIALIZE_PIXELTYPE_RAW_NO_CTABLE: { - SkColorTable* ctable = NULL; - if (SERIALIZE_PIXELTYPE_RAW_WITH_CTABLE == reftype) { - ctable = static_cast(buffer.readFlattenable()); - } - size_t size = this->getSize(); - if (this->allocPixels(ctable)) { - this->lockPixels(); - // Just read what we need. - buffer.read(this->getPixels(), this->getSafeSize()); - // Keep aligned for subsequent reads. - buffer.skip(size - this->getSafeSize()); - this->unlockPixels(); - } else { - buffer.skip(size); // Still skip the full-sized buffer though. - } - SkSafeUnref(ctable); - break; - } case SERIALIZE_PIXELTYPE_NONE: break; default: diff --git a/src/core/SkMallocPixelRef.cpp b/src/core/SkMallocPixelRef.cpp index 4bcf1bd..f0bc057 100644 --- a/src/core/SkMallocPixelRef.cpp +++ b/src/core/SkMallocPixelRef.cpp @@ -10,21 +10,25 @@ #include "SkFlattenable.h" SkMallocPixelRef::SkMallocPixelRef(void* storage, size_t size, - SkColorTable* ctable) { + SkColorTable* ctable, bool ownPixels) { if (NULL == storage) { + SkASSERT(ownPixels); storage = sk_malloc_throw(size); } fStorage = storage; fSize = size; fCTable = ctable; SkSafeRef(ctable); + fOwnPixels = ownPixels; this->setPreLocked(fStorage, fCTable); } SkMallocPixelRef::~SkMallocPixelRef() { SkSafeUnref(fCTable); - sk_free(fStorage); + if (fOwnPixels) { + sk_free(fStorage); + } } void* SkMallocPixelRef::onLockPixels(SkColorTable** ct) { @@ -59,6 +63,7 @@ SkMallocPixelRef::SkMallocPixelRef(SkFlattenableReadBuffer& buffer) } else { fCTable = NULL; } + fOwnPixels = true; this->setPreLocked(fStorage, fCTable); } diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp index 96c991c..a5ae6df 100644 --- a/src/core/SkPixelRef.cpp +++ b/src/core/SkPixelRef.cpp @@ -34,7 +34,6 @@ static SkBaseMutex* get_default_mutex() { /////////////////////////////////////////////////////////////////////////////// -extern int32_t SkNextPixelRefGenerationID(); int32_t SkNextPixelRefGenerationID() { static int32_t gPixelRefGenerationID; // do a loop in case our global wraps around, as we never want to -- 2.7.4