From 927138977fa256a6719baf74221882555b24008f Mon Sep 17 00:00:00 2001 From: "reed@google.com" Date: Fri, 3 Jan 2014 17:58:57 +0000 Subject: [PATCH] Revert "Revert of https://codereview.chromium.org/110593003/" This reverts commit c7abb25b25ba8b97948371d2bf0a2e3e78468f73. and fixes the ashmem break BUG= Review URL: https://codereview.chromium.org/119753010 git-svn-id: http://skia.googlecode.com/svn/trunk@12887 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/core/SkBitmapDevice.h | 2 + include/core/SkMallocPixelRef.h | 10 ++--- include/core/SkPixelRef.h | 73 ++++++++++++++++++++++++++--------- include/gpu/SkGrPixelRef.h | 7 ++-- include/images/SkImageRef.h | 4 +- src/core/SkBitmapDevice.cpp | 35 ++++++++--------- src/core/SkMallocPixelRef.cpp | 16 ++++---- src/core/SkPixelRef.cpp | 79 ++++++++++++++++++++++++++------------ 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 | 19 +++++---- src/lazy/SkCachingPixelRef.h | 2 +- src/lazy/SkDiscardablePixelRef.cpp | 21 +++++++--- src/lazy/SkDiscardablePixelRef.h | 6 ++- 17 files changed, 216 insertions(+), 128 deletions(-) diff --git a/include/core/SkBitmapDevice.h b/include/core/SkBitmapDevice.h index 83f480c..f3d40d0 100644 --- a/include/core/SkBitmapDevice.h +++ b/include/core/SkBitmapDevice.h @@ -258,6 +258,8 @@ private: friend class SkSurface_Raster; + void init(SkBitmap::Config config, int width, int height, bool isOpaque); + // used to change the backend's pixels (and possibly config/rowbytes) // but cannot change the width/height, so there should be no change to // any clip information. diff --git a/include/core/SkMallocPixelRef.h b/include/core/SkMallocPixelRef.h index 272dc21..c40afc4 100644 --- a/include/core/SkMallocPixelRef.h +++ b/include/core/SkMallocPixelRef.h @@ -32,11 +32,9 @@ public: /** * Return a new SkMallocPixelRef, automatically allocating storage for the - * pixels. - * - * If rowBytes is 0, an optimal value will be chosen automatically. - * If rowBytes is > 0, then it will be used, unless it is invald for the - * specified info, in which case NULL will be returned (failure). + * pixels. If rowBytes are 0, an optimal value will be chosen automatically. + * If rowBytes is > 0, then it will be respected, or NULL will be returned + * if rowBytes is invalid for the specified info. * * This pixelref will ref() the specified colortable (if not NULL). * @@ -90,7 +88,7 @@ protected: SkMallocPixelRef(SkFlattenableReadBuffer& buffer); virtual ~SkMallocPixelRef(); - virtual void* onLockPixels(SkColorTable**) SK_OVERRIDE; + virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE; virtual void onUnlockPixels() SK_OVERRIDE; virtual void flatten(SkFlattenableWriteBuffer&) const SK_OVERRIDE; virtual size_t getAllocatedSizeInBytes() const SK_OVERRIDE; diff --git a/include/core/SkPixelRef.h b/include/core/SkPixelRef.h index e611dc0..a332b76 100644 --- a/include/core/SkPixelRef.h +++ b/include/core/SkPixelRef.h @@ -14,8 +14,11 @@ #include "SkRefCnt.h" #include "SkString.h" #include "SkFlattenable.h" +#include "SkImageInfo.h" #include "SkTDArray.h" +//#define SK_SUPPORT_LEGACY_ONLOCKPIXELS + #ifdef SK_DEBUG /** * Defining SK_IGNORE_PIXELREF_SETPRELOCKED will force all pixelref @@ -60,23 +63,48 @@ public: /** Return the pixel memory returned from lockPixels, or null if the lockCount is 0. */ - void* pixels() const { return fPixels; } + void* pixels() const { return fRec.fPixels; } /** Return the current colorTable (if any) if pixels are locked, or null. */ - SkColorTable* colorTable() const { return fColorTable; } + SkColorTable* colorTable() const { return fRec.fColorTable; } /** + * To access the actual pixels of a pixelref, it must be "locked". + * Calling lockPixels returns a LockRec struct (on success). + */ + struct LockRec { + void* fPixels; + SkColorTable* fColorTable; + size_t fRowBytes; + + void zero() { sk_bzero(this, sizeof(*this)); } + + bool isZero() const { + return NULL == fPixels && NULL == fColorTable && 0 == fRowBytes; + } + }; + + /** * Returns true if the lockcount > 0 */ bool isLocked() const { return fLockCount > 0; } SkDEBUGCODE(int getLockCount() const { return fLockCount; }) - /** Call to access the pixel memory, which is returned. Balance with a call - to unlockPixels(). - */ - void lockPixels(); + /** + * Call to access the pixel memory. Return true on success. Balance this + * with a call to unlockPixels(). + */ + bool lockPixels(); + + /** + * Call to access the pixel memory. On success, return true and fill out + * the specified rec. On failure, return false and ignore the rec parameter. + * Balance this with a call to unlockPixels(). + */ + bool lockPixels(LockRec* rec); + /** Call to balanace a previous call to lockPixels(). Returns the pixels (or null) after the unlock. NOTE: lock calls can be nested, but the matching number of unlock calls must be made in order to free the @@ -233,18 +261,27 @@ public: void addGenIDChangeListener(GenIDChangeListener* listener); protected: - /** Called when the lockCount goes from 0 to 1. The caller will have already - acquire a mutex for thread safety, so this method need not do that. - */ - virtual void* onLockPixels(SkColorTable**) = 0; +#ifdef SK_SUPPORT_LEGACY_ONLOCKPIXELS + virtual void* onLockPixels(SkColorTable**); + virtual bool onNewLockPixels(LockRec*); +#else + /** + * On success, returns true and fills out the LockRec for the pixels. On + * failure returns false and ignores the LockRec parameter. + * + * The caller will have already acquired a mutex for thread safety, so this + * method need not do that. + */ + virtual bool onNewLockPixels(LockRec*) = 0; +#endif /** - * Called when the lock count goes from 1 to 0. The caller will have - * already acquire a mutex for thread safety, so this method need not do - * that. + * Balancing the previous successful call to onNewLockPixels. The locked + * pixel address will no longer be referenced, so the subclass is free to + * move or discard that memory. * - * If the previous call to onLockPixels failed (i.e. returned NULL), then - * the onUnlockPixels will NOT be called. + * The caller will have already acquired a mutex for thread safety, so this + * method need not do that. */ virtual void onUnlockPixels() = 0; @@ -289,15 +326,15 @@ protected: // only call from constructor. Flags this to always be locked, removing // the need to grab the mutex and call onLockPixels/onUnlockPixels. // Performance tweak to avoid those calls (esp. in multi-thread use case). - void setPreLocked(void* pixels, SkColorTable* ctable); + void setPreLocked(void*, size_t rowBytes, SkColorTable*); private: SkBaseMutex* fMutex; // must remain in scope for the life of this object const SkImageInfo fInfo; - void* fPixels; - SkColorTable* fColorTable; // we do not track ownership, subclass does + // LockRec is only valid if we're in a locked state (isLocked()) + LockRec fRec; int fLockCount; mutable uint32_t fGenerationID; diff --git a/include/gpu/SkGrPixelRef.h b/include/gpu/SkGrPixelRef.h index d893372..63e9756 100644 --- a/include/gpu/SkGrPixelRef.h +++ b/include/gpu/SkGrPixelRef.h @@ -24,10 +24,9 @@ public: virtual ~SkROLockPixelsPixelRef(); protected: - // override from SkPixelRef - virtual void* onLockPixels(SkColorTable** ptr); - virtual void onUnlockPixels(); - virtual bool onLockPixelsAreWritable() const; // return false; + virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE; + virtual void onUnlockPixels() SK_OVERRIDE; + virtual bool onLockPixelsAreWritable() const SK_OVERRIDE; // return false; private: SkBitmap fBitmap; diff --git a/include/images/SkImageRef.h b/include/images/SkImageRef.h index 30b1562..36f95e6 100644 --- a/include/images/SkImageRef.h +++ b/include/images/SkImageRef.h @@ -72,9 +72,9 @@ protected: When these are called, we will have already acquired the mutex! */ - virtual void* onLockPixels(SkColorTable**); + virtual bool onNewLockPixels(LockRec*) SK_OVERRIDE; // override this in your subclass to clean up when we're unlocking pixels - virtual void onUnlockPixels() {} + virtual void onUnlockPixels() SK_OVERRIDE {} SkImageRef(SkFlattenableReadBuffer&, SkBaseMutex* mutex = NULL); virtual void flatten(SkFlattenableWriteBuffer&) const SK_OVERRIDE; diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp index 1668618..368c807 100644 --- a/src/core/SkBitmapDevice.cpp +++ b/src/core/SkBitmapDevice.cpp @@ -24,31 +24,30 @@ SkBitmapDevice::SkBitmapDevice(const SkBitmap& bitmap, const SkDeviceProperties& , fBitmap(bitmap) { } -SkBitmapDevice::SkBitmapDevice(SkBitmap::Config config, int width, int height, bool isOpaque) { +void SkBitmapDevice::init(SkBitmap::Config config, int width, int height, bool isOpaque) { fBitmap.setConfig(config, width, height, 0, isOpaque ? kOpaque_SkAlphaType : kPremul_SkAlphaType); - if (!fBitmap.allocPixels()) { - fBitmap.setConfig(config, 0, 0, 0, isOpaque ? - kOpaque_SkAlphaType : kPremul_SkAlphaType); - } - if (!isOpaque) { - fBitmap.eraseColor(SK_ColorTRANSPARENT); + + if (SkBitmap::kNo_Config != config) { + if (!fBitmap.allocPixels()) { + // indicate failure by zeroing our bitmap + fBitmap.setConfig(config, 0, 0, 0, isOpaque ? + kOpaque_SkAlphaType : kPremul_SkAlphaType); + } else if (!isOpaque) { + fBitmap.eraseColor(SK_ColorTRANSPARENT); + } } } +SkBitmapDevice::SkBitmapDevice(SkBitmap::Config config, int width, int height, bool isOpaque) { + this->init(config, width, height, isOpaque); +} + SkBitmapDevice::SkBitmapDevice(SkBitmap::Config config, int width, int height, bool isOpaque, const SkDeviceProperties& deviceProperties) - : SkBaseDevice(deviceProperties) { - - fBitmap.setConfig(config, width, height, 0, isOpaque ? - kOpaque_SkAlphaType : kPremul_SkAlphaType); - if (!fBitmap.allocPixels()) { - fBitmap.setConfig(config, 0, 0, 0, isOpaque ? - kOpaque_SkAlphaType : kPremul_SkAlphaType); - } - if (!isOpaque) { - fBitmap.eraseColor(SK_ColorTRANSPARENT); - } + : SkBaseDevice(deviceProperties) +{ + this->init(config, width, height, isOpaque); } SkBitmapDevice::~SkBitmapDevice() { diff --git a/src/core/SkMallocPixelRef.cpp b/src/core/SkMallocPixelRef.cpp index c3e605c..c86d33b 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, fCTable); + this->setPreLocked(fStorage, fRB, fCTable); } SkMallocPixelRef::SkMallocPixelRef(const SkImageInfo& info, void* storage, @@ -174,8 +174,8 @@ SkMallocPixelRef::SkMallocPixelRef(const SkImageInfo& info, void* storage, fCTable = ctable; fRB = rowBytes; SkSafeRef(ctable); - - this->setPreLocked(fStorage, fCTable); + + this->setPreLocked(fStorage, fRB, fCTable); } @@ -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() { @@ -234,5 +236,5 @@ SkMallocPixelRef::SkMallocPixelRef(SkFlattenableReadBuffer& buffer) fCTable = NULL; } - this->setPreLocked(fStorage, fCTable); + this->setPreLocked(fStorage, fRB, fCTable); } diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp index fedbb5a..7ec5929 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,12 +136,13 @@ void SkPixelRef::cloneGenID(const SkPixelRef& that) { that.fUniqueGenerationID = false; } -void SkPixelRef::setPreLocked(void* pixels, SkColorTable* ctable) { +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 @@ -166,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() { @@ -191,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()); } } } @@ -278,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; + 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 45a5684..5a772a7 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 a068d8d..8a16437 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 843f4c0..2c1ec38 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 269199f..14dedf8 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 a2652fb..fa30baf 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 fb30d05..033a6b5 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,12 +40,12 @@ SkCachingPixelRef::~SkCachingPixelRef() { // Assert always unlock before unref. } -void* SkCachingPixelRef::onLockPixels(SkColorTable**) { - const SkImageInfo& info = this->info(); - +bool SkCachingPixelRef::onNewLockPixels(LockRec* rec) { if (fErrorInDecoding) { - return NULL; // don't try again. + return false; // don't try again. } + + const SkImageInfo& info = this->info(); SkBitmap bitmap; SkASSERT(NULL == fScaledCacheId); fScaledCacheId = SkScaledImageCache::FindAndLock(this->getGenerationID(), @@ -57,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, @@ -76,6 +75,7 @@ void* SkCachingPixelRef::onLockPixels(SkColorTable**) { SkAutoLockPixels autoLockPixels(bitmap); void* pixels = bitmap.getPixels(); SkASSERT(pixels != NULL); + // At this point, the autoLockPixels will unlockPixels() // to remove bitmap's lock on the pixels. We will then // destroy bitmap. The *only* guarantee that this pointer @@ -84,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 b1f2fcd..905ee9b 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 2886156..3f960d3 100644 --- a/src/lazy/SkDiscardablePixelRef.cpp +++ b/src/lazy/SkDiscardablePixelRef.cpp @@ -36,15 +36,18 @@ 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; } - + const size_t size = this->info().getSafeSize(fRowBytes); if (fDMFactory != 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 3367096..4a013fd 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 -- 2.7.4