From ba82bd11e2c055f885b2327aa230d2dac8b53f03 Mon Sep 17 00:00:00 2001 From: "reed@google.com" Date: Mon, 6 Jan 2014 13:34:39 +0000 Subject: Revert "Revert "Revert of https://codereview.chromium.org/110593003/"" This reverts commit 0fef787f33aa38109a0c8427e0098d997efdd5ff. failed in chrome: https://codereview.chromium.org/124503002/ Review URL: https://codereview.chromium.org/105523008 git-svn-id: http://skia.googlecode.com/svn/trunk@12906 2bbb7eff-a529-9590-31e7-b0007b416f81 --- src/core/SkBitmapDevice.cpp | 4 +- src/core/SkMallocPixelRef.cpp | 14 +++---- src/core/SkPixelRef.cpp | 75 +++++++++++--------------------------- 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, 78 insertions(+), 129 deletions(-) (limited to 'src') diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp index 0570fd9afd..953956abfc 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 53042d263d..c3e605c358 100644 --- a/src/core/SkMallocPixelRef.cpp +++ b/src/core/SkMallocPixelRef.cpp @@ -152,7 +152,7 @@ SkMallocPixelRef::SkMallocPixelRef(const SkImageInfo& info, void* storage, fRB = rowBytes; SkSafeRef(ctable); - this->setPreLocked(fStorage, fRB, fCTable); + this->setPreLocked(fStorage, fCTable); } SkMallocPixelRef::SkMallocPixelRef(const SkImageInfo& info, void* storage, @@ -175,7 +175,7 @@ SkMallocPixelRef::SkMallocPixelRef(const SkImageInfo& info, void* storage, fRB = rowBytes; SkSafeRef(ctable); - this->setPreLocked(fStorage, fRB, fCTable); + this->setPreLocked(fStorage, fCTable); } @@ -186,11 +186,9 @@ SkMallocPixelRef::~SkMallocPixelRef() { } } -bool SkMallocPixelRef::onNewLockPixels(LockRec* rec) { - rec->fPixels = fStorage; - rec->fRowBytes = fRB; - rec->fColorTable = fCTable; - return true; +void* SkMallocPixelRef::onLockPixels(SkColorTable** ctable) { + *ctable = fCTable; + return fStorage; } void SkMallocPixelRef::onUnlockPixels() { @@ -236,5 +234,5 @@ SkMallocPixelRef::SkMallocPixelRef(SkFlattenableReadBuffer& buffer) fCTable = NULL; } - this->setPreLocked(fStorage, fRB, fCTable); + this->setPreLocked(fStorage, fCTable); } diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp index 422d5039fe..fedbb5ac7d 100644 --- a/src/core/SkPixelRef.cpp +++ b/src/core/SkPixelRef.cpp @@ -82,19 +82,20 @@ 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) : fInfo(info) { - this->setMutex(NULL); - fRec.zero(); +SkPixelRef::SkPixelRef(const SkImageInfo& info, SkBaseMutex* mutex) : fInfo(info) { + this->setMutex(mutex); + fPixels = NULL; + fColorTable = NULL; // we do not track ownership of this fLockCount = 0; this->needsNewGenID(); fIsImmutable = false; fPreLocked = false; } - -SkPixelRef::SkPixelRef(const SkImageInfo& info, SkBaseMutex* mutex) : fInfo(info) { - this->setMutex(mutex); - fRec.zero(); +SkPixelRef::SkPixelRef(const SkImageInfo& info) : fInfo(info) { + this->setMutex(NULL); + fPixels = NULL; + fColorTable = NULL; // we do not track ownership of this fLockCount = 0; this->needsNewGenID(); fIsImmutable = false; @@ -112,7 +113,8 @@ SkPixelRef::SkPixelRef(SkFlattenableReadBuffer& buffer, SkBaseMutex* mutex) , fInfo(read_info(buffer)) { this->setMutex(mutex); - fRec.zero(); + fPixels = NULL; + fColorTable = NULL; // we do not track ownership of this fLockCount = 0; fIsImmutable = buffer.readBool(); fGenerationID = buffer.readUInt(); @@ -136,13 +138,12 @@ void SkPixelRef::cloneGenID(const SkPixelRef& that) { that.fUniqueGenerationID = false; } -void SkPixelRef::setPreLocked(void* pixels, size_t rowBytes, SkColorTable* ctable) { +void SkPixelRef::setPreLocked(void* pixels, SkColorTable* ctable) { #ifndef SK_IGNORE_PIXELREF_SETPRELOCKED // only call me in your constructor, otherwise fLockCount tracking can get // out of sync. - fRec.fPixels = pixels; - fRec.fColorTable = ctable; - fRec.fRowBytes = rowBytes; + fPixels = pixels; + fColorTable = ctable; fLockCount = SKPIXELREF_PRELOCKED_LOCKCOUNT; fPreLocked = true; #endif @@ -165,30 +166,20 @@ void SkPixelRef::flatten(SkFlattenableWriteBuffer& buffer) const { } } -bool SkPixelRef::lockPixels(LockRec* rec) { +void SkPixelRef::lockPixels() { SkASSERT(!fPreLocked || SKPIXELREF_PRELOCKED_LOCKCOUNT == fLockCount); if (!fPreLocked) { SkAutoMutexAcquire ac(*fMutex); if (1 == ++fLockCount) { - SkASSERT(fRec.isZero()); - - LockRec rec; - if (!this->onNewLockPixels(&rec)) { - return false; + fPixels = this->onLockPixels(&fColorTable); + // If onLockPixels failed, it will return NULL + if (NULL == fPixels) { + fColorTable = NULL; } - 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() { @@ -200,11 +191,12 @@ void SkPixelRef::unlockPixels() { SkASSERT(fLockCount > 0); if (0 == --fLockCount) { // don't call onUnlockPixels unless onLockPixels succeeded - if (fRec.fPixels) { + if (fPixels) { this->onUnlockPixels(); - fRec.zero(); + fPixels = NULL; + fColorTable = NULL; } else { - SkASSERT(fRec.isZero()); + SkASSERT(NULL == fColorTable); } } } @@ -286,29 +278,6 @@ size_t SkPixelRef::getAllocatedSizeInBytes() const { /////////////////////////////////////////////////////////////////////////////// -#ifdef SK_SUPPORT_LEGACY_ONLOCKPIXELS - -void* SkPixelRef::onLockPixels(SkColorTable** ctable) { - return NULL; -} - -bool SkPixelRef::onNewLockPixels(LockRec* rec) { - SkColorTable* ctable; - 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 5a772a7cd5..45a5684638 100644 --- a/src/core/SkScaledImageCache.cpp +++ b/src/core/SkScaledImageCache.cpp @@ -199,11 +199,13 @@ public: SK_DECLARE_UNFLATTENABLE_OBJECT() protected: - virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE; + virtual void* onLockPixels(SkColorTable**) 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; @@ -218,6 +220,8 @@ SkOneShotDiscardablePixelRef::SkOneShotDiscardablePixelRef(const SkImageInfo& in , fDM(dm) , fRB(rowBytes) { + fInfo = info; // remove this redundant field when SkPixelRef has info + SkASSERT(dm->data()); fFirstTime = true; } @@ -226,31 +230,26 @@ SkOneShotDiscardablePixelRef::~SkOneShotDiscardablePixelRef() { SkDELETE(fDM); } -bool SkOneShotDiscardablePixelRef::onNewLockPixels(LockRec* rec) { +void* SkOneShotDiscardablePixelRef::onLockPixels(SkColorTable** ctable) { if (fFirstTime) { // we're already locked SkASSERT(fDM->data()); fFirstTime = false; - goto SUCCESS; + return fDM->data(); } // A previous call to onUnlock may have deleted our DM, so check for that if (NULL == fDM) { - return false; + return NULL; } if (!fDM->lock()) { // since it failed, we delete it now, to free-up the resource delete fDM; fDM = NULL; - return false; + return NULL; } - -SUCCESS: - rec->fPixels = fDM->data(); - rec->fColorTable = NULL; - rec->fRowBytes = fRB; - return true; + return fDM->data(); } void SkOneShotDiscardablePixelRef::onUnlockPixels() { @@ -259,7 +258,7 @@ void SkOneShotDiscardablePixelRef::onUnlockPixels() { } size_t SkOneShotDiscardablePixelRef::getAllocatedSizeInBytes() const { - return this->info().getSafeSize(fRB); + return fInfo.fHeight * fRB; } class SkScaledImageCacheDiscardableAllocator : public SkBitmap::Allocator { diff --git a/src/gpu/SkGrPixelRef.cpp b/src/gpu/SkGrPixelRef.cpp index 8a16437780..a068d8dcc1 100644 --- a/src/gpu/SkGrPixelRef.cpp +++ b/src/gpu/SkGrPixelRef.cpp @@ -23,22 +23,18 @@ SkROLockPixelsPixelRef::SkROLockPixelsPixelRef(const SkImageInfo& info) SkROLockPixelsPixelRef::~SkROLockPixelsPixelRef() {} -bool SkROLockPixelsPixelRef::onNewLockPixels(LockRec* rec) { +void* SkROLockPixelsPixelRef::onLockPixels(SkColorTable** ctable) { + if (ctable) { + *ctable = NULL; + } fBitmap.reset(); // SkDebugf("---------- calling readpixels in support of lockpixels\n"); if (!this->onReadPixels(&fBitmap, NULL)) { SkDebugf("SkROLockPixelsPixelRef::onLockPixels failed!\n"); - return false; + return NULL; } fBitmap.lockPixels(); - if (NULL == fBitmap.getPixels()) { - return false; - } - - rec->fPixels = fBitmap.getPixels(); - rec->fColorTable = NULL; - rec->fRowBytes = fBitmap.rowBytes(); - return true; + return fBitmap.getPixels(); } void SkROLockPixelsPixelRef::onUnlockPixels() { diff --git a/src/images/SkImageRef.cpp b/src/images/SkImageRef.cpp index 2c1ec38116..843f4c01f9 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) - : INHERITED(info, mutex), fErrorInDecoding(false) -{ + : SkPixelRef(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, this->info().fColorType, (int)fStream->getLength()); + this, fConfig, (int)fStream->getLength()); #endif fStream->unref(); @@ -134,18 +134,15 @@ bool SkImageRef::prepareBitmap(SkImageDecoder::Mode mode) { return false; } -bool SkImageRef::onNewLockPixels(LockRec* rec) { +void* SkImageRef::onLockPixels(SkColorTable** ct) { if (NULL == fBitmap.getPixels()) { (void)this->prepareBitmap(SkImageDecoder::kDecodePixels_Mode); } - if (NULL == fBitmap.getPixels()) { - return false; + if (ct) { + *ct = fBitmap.getColorTable(); } - rec->fPixels = fBitmap.getPixels(); - rec->fColorTable = NULL; - rec->fRowBytes = fBitmap.rowBytes(); - return true; + return fBitmap.getPixels(); } size_t SkImageRef::ramUsed() const { diff --git a/src/images/SkImageRef_ashmem.cpp b/src/images/SkImageRef_ashmem.cpp index 14dedf8bd3..269199faf8 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 } } -bool SkImageRef_ashmem::onNewLockPixels(LockRec* rec) { +void* SkImageRef_ashmem::onLockPixels(SkColorTable** ct) { SkASSERT(fBitmap.getPixels() == NULL); SkASSERT(fBitmap.getColorTable() == NULL); @@ -185,13 +185,17 @@ bool SkImageRef_ashmem::onNewLockPixels(LockRec* rec) { #endif } else { SkDebugf("===== ashmem pin_region(%d) returned %d\n", fRec.fFD, pin); - return false; + // return null result for failure + if (ct) { + *ct = NULL; + } + return NULL; } } else { // no FD, will create an ashmem region in allocator } - return this->INHERITED::onNewLockPixels(rec); + return this->INHERITED::onLockPixels(ct); } void SkImageRef_ashmem::onUnlockPixels() { diff --git a/src/images/SkImageRef_ashmem.h b/src/images/SkImageRef_ashmem.h index fa30baf99c..a2652fbc30 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 bool onNewLockPixels(LockRec*) SK_OVERRIDE; - virtual void onUnlockPixels() SK_OVERRIDE; + virtual void* onLockPixels(SkColorTable**); + virtual void onUnlockPixels(); private: void closeFD(); diff --git a/src/lazy/SkCachingPixelRef.cpp b/src/lazy/SkCachingPixelRef.cpp index f1510fb67c..452ea4ed03 100644 --- a/src/lazy/SkCachingPixelRef.cpp +++ b/src/lazy/SkCachingPixelRef.cpp @@ -8,6 +8,7 @@ #include "SkCachingPixelRef.h" #include "SkScaledImageCache.h" + bool SkCachingPixelRef::Install(SkImageGenerator* generator, SkBitmap* dst) { SkImageInfo info; @@ -40,12 +41,13 @@ SkCachingPixelRef::~SkCachingPixelRef() { // Assert always unlock before unref. } -bool SkCachingPixelRef::onNewLockPixels(LockRec* rec) { +void* SkCachingPixelRef::onLockPixels(SkColorTable**) { if (fErrorInDecoding) { - return false; // don't try again. + return NULL; // don't try again. } - + const SkImageInfo& info = this->info(); + SkBitmap bitmap; SkASSERT(NULL == fScaledCacheId); fScaledCacheId = SkScaledImageCache::FindAndLock(this->getGenerationID(), @@ -56,12 +58,12 @@ bool SkCachingPixelRef::onNewLockPixels(LockRec* rec) { // Cache has been purged, must re-decode. if ((!bitmap.setConfig(info, fRowBytes)) || !bitmap.allocPixels()) { fErrorInDecoding = true; - return false; + return NULL; } SkAutoLockPixels autoLockPixels(bitmap); if (!fImageGenerator->getPixels(info, bitmap.getPixels(), fRowBytes)) { fErrorInDecoding = true; - return false; + return NULL; } fScaledCacheId = SkScaledImageCache::AddAndLock(this->getGenerationID(), info.fWidth, @@ -84,10 +86,7 @@ bool SkCachingPixelRef::onNewLockPixels(LockRec* rec) { // bitmap (SkScaledImageCache::Rec.fBitmap) that holds a // reference to the concrete PixelRef while this record is // locked. - rec->fPixels = pixels; - rec->fColorTable = NULL; - rec->fRowBytes = bitmap.rowBytes(); - return true; + return pixels; } void SkCachingPixelRef::onUnlockPixels() { diff --git a/src/lazy/SkCachingPixelRef.h b/src/lazy/SkCachingPixelRef.h index 905ee9bf0d..b1f2fcd669 100644 --- a/src/lazy/SkCachingPixelRef.h +++ b/src/lazy/SkCachingPixelRef.h @@ -40,7 +40,7 @@ public: protected: virtual ~SkCachingPixelRef(); - virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE; + virtual void* onLockPixels(SkColorTable** colorTable) 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 abd80f2e0a..2886156102 100644 --- a/src/lazy/SkDiscardablePixelRef.cpp +++ b/src/lazy/SkDiscardablePixelRef.cpp @@ -36,13 +36,10 @@ SkDiscardablePixelRef::~SkDiscardablePixelRef() { SkDELETE(fGenerator); } -bool SkDiscardablePixelRef::onNewLockPixels(LockRec* rec) { +void* SkDiscardablePixelRef::onLockPixels(SkColorTable**) { if (fDiscardableMemory != NULL) { if (fDiscardableMemory->lock()) { - rec->fPixels = fDiscardableMemory->data(); - rec->fColorTable = NULL; - rec->fRowBytes = fRowBytes; - return true; + return fDiscardableMemory->data(); } SkDELETE(fDiscardableMemory); fDiscardableMemory = NULL; @@ -56,23 +53,17 @@ bool SkDiscardablePixelRef::onNewLockPixels(LockRec* rec) { fDiscardableMemory = SkDiscardableMemory::Create(size); } if (NULL == fDiscardableMemory) { - return false; // Memory allocation failed. + return NULL; // Memory allocation failed. } - void* pixels = fDiscardableMemory->data(); if (!fGenerator->getPixels(this->info(), pixels, fRowBytes)) { fDiscardableMemory->unlock(); SkDELETE(fDiscardableMemory); fDiscardableMemory = NULL; - return false; + return NULL; } - - rec->fPixels = pixels; - rec->fColorTable = NULL; - rec->fRowBytes = fRowBytes; - return true; + return pixels; } - void SkDiscardablePixelRef::onUnlockPixels() { fDiscardableMemory->unlock(); } diff --git a/src/lazy/SkDiscardablePixelRef.h b/src/lazy/SkDiscardablePixelRef.h index 4a013fda03..3367096c26 100644 --- a/src/lazy/SkDiscardablePixelRef.h +++ b/src/lazy/SkDiscardablePixelRef.h @@ -28,8 +28,7 @@ public: protected: ~SkDiscardablePixelRef(); - - virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE; + virtual void* onLockPixels(SkColorTable**) SK_OVERRIDE; virtual void onUnlockPixels() SK_OVERRIDE; virtual bool onLockPixelsAreWritable() const SK_OVERRIDE { return false; } @@ -50,12 +49,9 @@ 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