From d0419b1fe781ed21d4aa0dc80df6b3e79ed37e46 Mon Sep 17 00:00:00 2001 From: "reed@google.com" Date: Mon, 6 Jan 2014 17:08:27 +0000 Subject: Revert "Revert "Revert "Revert of https://codereview.chromium.org/110593003/""" This reverts commit aaa89649590323fe40f52439d9a9a3376bb3b8ae. BUG= Review URL: https://codereview.chromium.org/123223007 git-svn-id: http://skia.googlecode.com/svn/trunk@12910 2bbb7eff-a529-9590-31e7-b0007b416f81 --- src/core/SkBitmapDevice.cpp | 4 +-- src/core/SkMallocPixelRef.cpp | 8 +++-- src/core/SkPixelRef.cpp | 74 ++++++++++++++++++++++++++------------ src/core/SkScaledImageCache.cpp | 23 ++++++------ src/gpu/SkGrPixelRef.cpp | 16 +++++---- src/images/SkImageRef.cpp | 17 +++++---- src/images/SkImageRef_ashmem.cpp | 10 ++---- src/images/SkImageRef_ashmem.h | 4 +-- src/lazy/SkCachingPixelRef.cpp | 17 ++++----- src/lazy/SkCachingPixelRef.h | 2 +- src/lazy/SkDiscardablePixelRef.cpp | 19 +++++++--- src/lazy/SkDiscardablePixelRef.h | 6 +++- 12 files changed, 125 insertions(+), 75 deletions(-) (limited to 'src') diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp index 953956abfc..0570fd9afd 100644 --- a/src/core/SkBitmapDevice.cpp +++ b/src/core/SkBitmapDevice.cpp @@ -27,7 +27,7 @@ SkBitmapDevice::SkBitmapDevice(const SkBitmap& bitmap, const SkDeviceProperties& void SkBitmapDevice::init(SkBitmap::Config config, int width, int height, bool isOpaque) { fBitmap.setConfig(config, width, height, 0, isOpaque ? kOpaque_SkAlphaType : kPremul_SkAlphaType); - + if (SkBitmap::kNo_Config != config) { if (!fBitmap.allocPixels()) { // indicate failure by zeroing our bitmap @@ -45,7 +45,7 @@ SkBitmapDevice::SkBitmapDevice(SkBitmap::Config config, int width, int height, b SkBitmapDevice::SkBitmapDevice(SkBitmap::Config config, int width, int height, bool isOpaque, const SkDeviceProperties& deviceProperties) -: SkBaseDevice(deviceProperties) + : SkBaseDevice(deviceProperties) { this->init(config, width, height, isOpaque); } diff --git a/src/core/SkMallocPixelRef.cpp b/src/core/SkMallocPixelRef.cpp index 075b5f6f2d..50357e469b 100644 --- a/src/core/SkMallocPixelRef.cpp +++ b/src/core/SkMallocPixelRef.cpp @@ -186,9 +186,11 @@ SkMallocPixelRef::~SkMallocPixelRef() { } } -void* SkMallocPixelRef::onLockPixels(SkColorTable** ctable) { - *ctable = fCTable; - return fStorage; +bool SkMallocPixelRef::onNewLockPixels(LockRec* rec) { + rec->fPixels = fStorage; + rec->fRowBytes = fRB; + rec->fColorTable = fCTable; + return true; } void SkMallocPixelRef::onUnlockPixels() { diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp index 8bf1f4eddf..10e1309c0b 100644 --- a/src/core/SkPixelRef.cpp +++ b/src/core/SkPixelRef.cpp @@ -82,20 +82,19 @@ void SkPixelRef::setMutex(SkBaseMutex* mutex) { // just need a > 0 value, so pick a funny one to aid in debugging #define SKPIXELREF_PRELOCKED_LOCKCOUNT 123456789 -SkPixelRef::SkPixelRef(const SkImageInfo& info, SkBaseMutex* mutex) : fInfo(info) { - this->setMutex(mutex); - fPixels = NULL; - fColorTable = NULL; // we do not track ownership of this +SkPixelRef::SkPixelRef(const SkImageInfo& info) : fInfo(info) { + this->setMutex(NULL); + fRec.zero(); fLockCount = 0; this->needsNewGenID(); fIsImmutable = false; fPreLocked = false; } -SkPixelRef::SkPixelRef(const SkImageInfo& info) : fInfo(info) { - this->setMutex(NULL); - fPixels = NULL; - fColorTable = NULL; // we do not track ownership of this + +SkPixelRef::SkPixelRef(const SkImageInfo& info, SkBaseMutex* mutex) : fInfo(info) { + this->setMutex(mutex); + fRec.zero(); fLockCount = 0; this->needsNewGenID(); fIsImmutable = false; @@ -113,8 +112,7 @@ SkPixelRef::SkPixelRef(SkFlattenableReadBuffer& buffer, SkBaseMutex* mutex) , fInfo(read_info(buffer)) { this->setMutex(mutex); - fPixels = NULL; - fColorTable = NULL; // we do not track ownership of this + fRec.zero(); fLockCount = 0; fIsImmutable = buffer.readBool(); fGenerationID = buffer.readUInt(); @@ -138,13 +136,13 @@ void SkPixelRef::cloneGenID(const SkPixelRef& that) { that.fUniqueGenerationID = false; } -// rowBytes will be respected when we land the onNewLockPixels CL void SkPixelRef::setPreLocked(void* pixels, size_t rowBytes, SkColorTable* ctable) { #ifndef SK_IGNORE_PIXELREF_SETPRELOCKED // only call me in your constructor, otherwise fLockCount tracking can get // out of sync. - fPixels = pixels; - fColorTable = ctable; + fRec.fPixels = pixels; + fRec.fColorTable = ctable; + fRec.fRowBytes = rowBytes; fLockCount = SKPIXELREF_PRELOCKED_LOCKCOUNT; fPreLocked = true; #endif @@ -167,20 +165,30 @@ void SkPixelRef::flatten(SkFlattenableWriteBuffer& buffer) const { } } -void SkPixelRef::lockPixels() { +bool SkPixelRef::lockPixels(LockRec* rec) { SkASSERT(!fPreLocked || SKPIXELREF_PRELOCKED_LOCKCOUNT == fLockCount); if (!fPreLocked) { SkAutoMutexAcquire ac(*fMutex); if (1 == ++fLockCount) { - fPixels = this->onLockPixels(&fColorTable); - // If onLockPixels failed, it will return NULL - if (NULL == fPixels) { - fColorTable = NULL; + SkASSERT(fRec.isZero()); + + LockRec rec; + if (!this->onNewLockPixels(&rec)) { + return false; } + SkASSERT(!rec.isZero()); // else why did onNewLock return true? + fRec = rec; } } + *rec = fRec; + return true; +} + +bool SkPixelRef::lockPixels() { + LockRec rec; + return this->lockPixels(&rec); } void SkPixelRef::unlockPixels() { @@ -192,12 +200,11 @@ void SkPixelRef::unlockPixels() { SkASSERT(fLockCount > 0); if (0 == --fLockCount) { // don't call onUnlockPixels unless onLockPixels succeeded - if (fPixels) { + if (fRec.fPixels) { this->onUnlockPixels(); - fPixels = NULL; - fColorTable = NULL; + fRec.zero(); } else { - SkASSERT(NULL == fColorTable); + SkASSERT(fRec.isZero()); } } } @@ -279,6 +286,29 @@ size_t SkPixelRef::getAllocatedSizeInBytes() const { /////////////////////////////////////////////////////////////////////////////// +#ifdef SK_SUPPORT_LEGACY_ONLOCKPIXELS + +void* SkPixelRef::onLockPixels(SkColorTable** ctable) { + return NULL; +} + +bool SkPixelRef::onNewLockPixels(LockRec* rec) { + SkColorTable* ctable = NULL; // for legacy clients who forget to set this + void* pixels = this->onLockPixels(&ctable); + if (!pixels) { + return false; + } + + rec->fPixels = pixels; + rec->fColorTable = ctable; + rec->fRowBytes = 0; // callers don't currently need this (thank goodness) + return true; +} + +#endif + +/////////////////////////////////////////////////////////////////////////////// + #ifdef SK_BUILD_FOR_ANDROID void SkPixelRef::globalRef(void* data) { this->ref(); diff --git a/src/core/SkScaledImageCache.cpp b/src/core/SkScaledImageCache.cpp index 45a5684638..5a772a7cd5 100644 --- a/src/core/SkScaledImageCache.cpp +++ b/src/core/SkScaledImageCache.cpp @@ -199,13 +199,11 @@ public: SK_DECLARE_UNFLATTENABLE_OBJECT() protected: - virtual void* onLockPixels(SkColorTable**) SK_OVERRIDE; + virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE; virtual void onUnlockPixels() SK_OVERRIDE; virtual size_t getAllocatedSizeInBytes() const SK_OVERRIDE; private: - SkImageInfo fInfo; // remove when SkPixelRef gets this in baseclass - SkDiscardableMemory* fDM; size_t fRB; bool fFirstTime; @@ -220,8 +218,6 @@ SkOneShotDiscardablePixelRef::SkOneShotDiscardablePixelRef(const SkImageInfo& in , fDM(dm) , fRB(rowBytes) { - fInfo = info; // remove this redundant field when SkPixelRef has info - SkASSERT(dm->data()); fFirstTime = true; } @@ -230,26 +226,31 @@ SkOneShotDiscardablePixelRef::~SkOneShotDiscardablePixelRef() { SkDELETE(fDM); } -void* SkOneShotDiscardablePixelRef::onLockPixels(SkColorTable** ctable) { +bool SkOneShotDiscardablePixelRef::onNewLockPixels(LockRec* rec) { if (fFirstTime) { // we're already locked SkASSERT(fDM->data()); fFirstTime = false; - return fDM->data(); + goto SUCCESS; } // A previous call to onUnlock may have deleted our DM, so check for that if (NULL == fDM) { - return NULL; + return false; } if (!fDM->lock()) { // since it failed, we delete it now, to free-up the resource delete fDM; fDM = NULL; - return NULL; + return false; } - return fDM->data(); + +SUCCESS: + rec->fPixels = fDM->data(); + rec->fColorTable = NULL; + rec->fRowBytes = fRB; + return true; } void SkOneShotDiscardablePixelRef::onUnlockPixels() { @@ -258,7 +259,7 @@ void SkOneShotDiscardablePixelRef::onUnlockPixels() { } size_t SkOneShotDiscardablePixelRef::getAllocatedSizeInBytes() const { - return fInfo.fHeight * fRB; + return this->info().getSafeSize(fRB); } class SkScaledImageCacheDiscardableAllocator : public SkBitmap::Allocator { diff --git a/src/gpu/SkGrPixelRef.cpp b/src/gpu/SkGrPixelRef.cpp index a068d8dcc1..8a16437780 100644 --- a/src/gpu/SkGrPixelRef.cpp +++ b/src/gpu/SkGrPixelRef.cpp @@ -23,18 +23,22 @@ SkROLockPixelsPixelRef::SkROLockPixelsPixelRef(const SkImageInfo& info) SkROLockPixelsPixelRef::~SkROLockPixelsPixelRef() {} -void* SkROLockPixelsPixelRef::onLockPixels(SkColorTable** ctable) { - if (ctable) { - *ctable = NULL; - } +bool SkROLockPixelsPixelRef::onNewLockPixels(LockRec* rec) { fBitmap.reset(); // SkDebugf("---------- calling readpixels in support of lockpixels\n"); if (!this->onReadPixels(&fBitmap, NULL)) { SkDebugf("SkROLockPixelsPixelRef::onLockPixels failed!\n"); - return NULL; + return false; } fBitmap.lockPixels(); - return fBitmap.getPixels(); + if (NULL == fBitmap.getPixels()) { + return false; + } + + rec->fPixels = fBitmap.getPixels(); + rec->fColorTable = NULL; + rec->fRowBytes = fBitmap.rowBytes(); + return true; } void SkROLockPixelsPixelRef::onUnlockPixels() { diff --git a/src/images/SkImageRef.cpp b/src/images/SkImageRef.cpp index 843f4c01f9..2c1ec38116 100644 --- a/src/images/SkImageRef.cpp +++ b/src/images/SkImageRef.cpp @@ -15,12 +15,12 @@ //#define DUMP_IMAGEREF_LIFECYCLE - /////////////////////////////////////////////////////////////////////////////// SkImageRef::SkImageRef(const SkImageInfo& info, SkStreamRewindable* stream, int sampleSize, SkBaseMutex* mutex) - : SkPixelRef(info, mutex), fErrorInDecoding(false) { + : INHERITED(info, mutex), fErrorInDecoding(false) +{ SkASSERT(stream); stream->ref(); fStream = stream; @@ -39,7 +39,7 @@ SkImageRef::~SkImageRef() { #ifdef DUMP_IMAGEREF_LIFECYCLE SkDebugf("delete ImageRef %p [%d] data=%d\n", - this, fConfig, (int)fStream->getLength()); + this, this->info().fColorType, (int)fStream->getLength()); #endif fStream->unref(); @@ -134,15 +134,18 @@ bool SkImageRef::prepareBitmap(SkImageDecoder::Mode mode) { return false; } -void* SkImageRef::onLockPixels(SkColorTable** ct) { +bool SkImageRef::onNewLockPixels(LockRec* rec) { if (NULL == fBitmap.getPixels()) { (void)this->prepareBitmap(SkImageDecoder::kDecodePixels_Mode); } - if (ct) { - *ct = fBitmap.getColorTable(); + if (NULL == fBitmap.getPixels()) { + return false; } - return fBitmap.getPixels(); + rec->fPixels = fBitmap.getPixels(); + rec->fColorTable = NULL; + rec->fRowBytes = fBitmap.rowBytes(); + return true; } size_t SkImageRef::ramUsed() const { diff --git a/src/images/SkImageRef_ashmem.cpp b/src/images/SkImageRef_ashmem.cpp index 269199faf8..14dedf8bd3 100644 --- a/src/images/SkImageRef_ashmem.cpp +++ b/src/images/SkImageRef_ashmem.cpp @@ -159,7 +159,7 @@ bool SkImageRef_ashmem::onDecode(SkImageDecoder* codec, SkStreamRewindable* stre } } -void* SkImageRef_ashmem::onLockPixels(SkColorTable** ct) { +bool SkImageRef_ashmem::onNewLockPixels(LockRec* rec) { SkASSERT(fBitmap.getPixels() == NULL); SkASSERT(fBitmap.getColorTable() == NULL); @@ -185,17 +185,13 @@ void* SkImageRef_ashmem::onLockPixels(SkColorTable** ct) { #endif } else { SkDebugf("===== ashmem pin_region(%d) returned %d\n", fRec.fFD, pin); - // return null result for failure - if (ct) { - *ct = NULL; - } - return NULL; + return false; } } else { // no FD, will create an ashmem region in allocator } - return this->INHERITED::onLockPixels(ct); + return this->INHERITED::onNewLockPixels(rec); } void SkImageRef_ashmem::onUnlockPixels() { diff --git a/src/images/SkImageRef_ashmem.h b/src/images/SkImageRef_ashmem.h index a2652fbc30..fa30baf99c 100644 --- a/src/images/SkImageRef_ashmem.h +++ b/src/images/SkImageRef_ashmem.h @@ -32,8 +32,8 @@ protected: SkBitmap* bitmap, SkBitmap::Config config, SkImageDecoder::Mode mode); - virtual void* onLockPixels(SkColorTable**); - virtual void onUnlockPixels(); + virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE; + virtual void onUnlockPixels() SK_OVERRIDE; private: void closeFD(); diff --git a/src/lazy/SkCachingPixelRef.cpp b/src/lazy/SkCachingPixelRef.cpp index 452ea4ed03..f1510fb67c 100644 --- a/src/lazy/SkCachingPixelRef.cpp +++ b/src/lazy/SkCachingPixelRef.cpp @@ -8,7 +8,6 @@ #include "SkCachingPixelRef.h" #include "SkScaledImageCache.h" - bool SkCachingPixelRef::Install(SkImageGenerator* generator, SkBitmap* dst) { SkImageInfo info; @@ -41,13 +40,12 @@ SkCachingPixelRef::~SkCachingPixelRef() { // Assert always unlock before unref. } -void* SkCachingPixelRef::onLockPixels(SkColorTable**) { +bool SkCachingPixelRef::onNewLockPixels(LockRec* rec) { if (fErrorInDecoding) { - return NULL; // don't try again. + return false; // don't try again. } - - const SkImageInfo& info = this->info(); + const SkImageInfo& info = this->info(); SkBitmap bitmap; SkASSERT(NULL == fScaledCacheId); fScaledCacheId = SkScaledImageCache::FindAndLock(this->getGenerationID(), @@ -58,12 +56,12 @@ void* SkCachingPixelRef::onLockPixels(SkColorTable**) { // Cache has been purged, must re-decode. if ((!bitmap.setConfig(info, fRowBytes)) || !bitmap.allocPixels()) { fErrorInDecoding = true; - return NULL; + return false; } SkAutoLockPixels autoLockPixels(bitmap); if (!fImageGenerator->getPixels(info, bitmap.getPixels(), fRowBytes)) { fErrorInDecoding = true; - return NULL; + return false; } fScaledCacheId = SkScaledImageCache::AddAndLock(this->getGenerationID(), info.fWidth, @@ -86,7 +84,10 @@ void* SkCachingPixelRef::onLockPixels(SkColorTable**) { // bitmap (SkScaledImageCache::Rec.fBitmap) that holds a // reference to the concrete PixelRef while this record is // locked. - return pixels; + rec->fPixels = pixels; + rec->fColorTable = NULL; + rec->fRowBytes = bitmap.rowBytes(); + return true; } void SkCachingPixelRef::onUnlockPixels() { diff --git a/src/lazy/SkCachingPixelRef.h b/src/lazy/SkCachingPixelRef.h index b1f2fcd669..905ee9bf0d 100644 --- a/src/lazy/SkCachingPixelRef.h +++ b/src/lazy/SkCachingPixelRef.h @@ -40,7 +40,7 @@ public: protected: virtual ~SkCachingPixelRef(); - virtual void* onLockPixels(SkColorTable** colorTable) SK_OVERRIDE; + virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE; virtual void onUnlockPixels() SK_OVERRIDE; virtual bool onLockPixelsAreWritable() const SK_OVERRIDE { return false; } diff --git a/src/lazy/SkDiscardablePixelRef.cpp b/src/lazy/SkDiscardablePixelRef.cpp index 2886156102..abd80f2e0a 100644 --- a/src/lazy/SkDiscardablePixelRef.cpp +++ b/src/lazy/SkDiscardablePixelRef.cpp @@ -36,10 +36,13 @@ SkDiscardablePixelRef::~SkDiscardablePixelRef() { SkDELETE(fGenerator); } -void* SkDiscardablePixelRef::onLockPixels(SkColorTable**) { +bool SkDiscardablePixelRef::onNewLockPixels(LockRec* rec) { if (fDiscardableMemory != NULL) { if (fDiscardableMemory->lock()) { - return fDiscardableMemory->data(); + rec->fPixels = fDiscardableMemory->data(); + rec->fColorTable = NULL; + rec->fRowBytes = fRowBytes; + return true; } SkDELETE(fDiscardableMemory); fDiscardableMemory = NULL; @@ -53,17 +56,23 @@ void* SkDiscardablePixelRef::onLockPixels(SkColorTable**) { fDiscardableMemory = SkDiscardableMemory::Create(size); } if (NULL == fDiscardableMemory) { - return NULL; // Memory allocation failed. + return false; // Memory allocation failed. } + void* pixels = fDiscardableMemory->data(); if (!fGenerator->getPixels(this->info(), pixels, fRowBytes)) { fDiscardableMemory->unlock(); SkDELETE(fDiscardableMemory); fDiscardableMemory = NULL; - return NULL; + return false; } - return pixels; + + rec->fPixels = pixels; + rec->fColorTable = NULL; + rec->fRowBytes = fRowBytes; + return true; } + void SkDiscardablePixelRef::onUnlockPixels() { fDiscardableMemory->unlock(); } diff --git a/src/lazy/SkDiscardablePixelRef.h b/src/lazy/SkDiscardablePixelRef.h index 3367096c26..4a013fda03 100644 --- a/src/lazy/SkDiscardablePixelRef.h +++ b/src/lazy/SkDiscardablePixelRef.h @@ -28,7 +28,8 @@ public: protected: ~SkDiscardablePixelRef(); - virtual void* onLockPixels(SkColorTable**) SK_OVERRIDE; + + virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE; virtual void onUnlockPixels() SK_OVERRIDE; virtual bool onLockPixelsAreWritable() const SK_OVERRIDE { return false; } @@ -49,9 +50,12 @@ private: SkDiscardablePixelRef(const SkImageInfo&, SkImageGenerator*, size_t rowBytes, SkDiscardableMemory::Factory* factory); + friend bool SkInstallDiscardablePixelRef(SkImageGenerator*, SkBitmap*, SkDiscardableMemory::Factory*); + typedef SkPixelRef INHERITED; }; + #endif // SkDiscardablePixelRef_DEFINED -- cgit v1.2.3