From 7ca24437c71af06fc06ab5f6f261b185882fa440 Mon Sep 17 00:00:00 2001 From: "scroggo@google.com" Date: Tue, 14 Aug 2012 15:48:43 +0000 Subject: [PATCH] Use SkBitmapHeap for shaders in SkGPipe. Required adding a new feature to SkBitmapHeap, allowing it to defer adding owners, since sometimes we flatten a shader, but then do not unflatten it, since we already had a copy in the heap, so the owners never get removed. Reviewed at https://codereview.appspot.com/6464053/ Review URL: https://codereview.appspot.com/6465047 git-svn-id: http://skia.googlecode.com/svn/trunk@5082 2bbb7eff-a529-9590-31e7-b0007b416f81 --- src/core/SkBitmapHeap.cpp | 31 +++++++++++++++++++++++++++---- src/core/SkBitmapHeap.h | 19 +++++++++++++++++++ src/pipe/SkGPipeRead.cpp | 20 ++++++++++++++++++-- src/pipe/SkGPipeWrite.cpp | 14 ++++++++++++-- 4 files changed, 76 insertions(+), 8 deletions(-) diff --git a/src/core/SkBitmapHeap.cpp b/src/core/SkBitmapHeap.cpp index 7eb34b0..7caf1a5 100644 --- a/src/core/SkBitmapHeap.cpp +++ b/src/core/SkBitmapHeap.cpp @@ -66,7 +66,8 @@ SkBitmapHeap::SkBitmapHeap(int32_t preferredSize, int32_t ownerCount) , fLeastRecentlyUsed(NULL) , fPreferredCount(preferredSize) , fOwnerCount(ownerCount) - , fBytesAllocated(0) { + , fBytesAllocated(0) + , fDeferAddingOwners(false) { } SkBitmapHeap::SkBitmapHeap(ExternalStorage* storage, int32_t preferredSize) @@ -76,7 +77,8 @@ SkBitmapHeap::SkBitmapHeap(ExternalStorage* storage, int32_t preferredSize) , fLeastRecentlyUsed(NULL) , fPreferredCount(preferredSize) , fOwnerCount(IGNORE_OWNERS) - , fBytesAllocated(0) { + , fBytesAllocated(0) + , fDeferAddingOwners(false) { SkSafeRef(storage); } @@ -165,7 +167,7 @@ SkBitmapHeap::LookupEntry* SkBitmapHeap::findEntryToReplace(const SkBitmap& repl if (heapEntry->fRefCount > 0) { // If the least recently used bitmap has not been unreferenced // by its owner, then according to our LRU specifications a more - // recently used one can not have used all it's references yet either. + // recently used one can not have used all its references yet either. return NULL; } if (replacement.getGenerationID() == iter->fGenerationId) { @@ -281,7 +283,11 @@ int32_t SkBitmapHeap::insert(const SkBitmap& originalBitmap) { if (entry) { // Already had a copy of the bitmap in the heap. if (fOwnerCount != IGNORE_OWNERS) { - entry->addReferences(fOwnerCount); + if (fDeferAddingOwners) { + *fDeferredEntries.append() = entry->fSlot; + } else { + entry->addReferences(fOwnerCount); + } } if (fPreferredCount != UNLIMITED_SIZE) { LookupEntry* lookupEntry = fLookupTable[searchIndex]; @@ -370,3 +376,20 @@ int32_t SkBitmapHeap::insert(const SkBitmap& originalBitmap) { } return entry->fSlot; } + +void SkBitmapHeap::deferAddingOwners() { + fDeferAddingOwners = true; +} + +void SkBitmapHeap::endAddingOwnersDeferral(bool add) { + if (add) { + for (int i = 0; i < fDeferredEntries.count(); i++) { + SkASSERT(fOwnerCount != IGNORE_OWNERS); + SkBitmapHeapEntry* heapEntry = this->getEntry(fDeferredEntries[i]); + SkASSERT(heapEntry != NULL); + heapEntry->addReferences(fOwnerCount); + } + } + fDeferAddingOwners = false; + fDeferredEntries.reset(); +} diff --git a/src/core/SkBitmapHeap.h b/src/core/SkBitmapHeap.h index 71c3405..a16727b 100644 --- a/src/core/SkBitmapHeap.h +++ b/src/core/SkBitmapHeap.h @@ -192,6 +192,22 @@ public: */ size_t freeMemoryIfPossible(size_t bytesToFree); + /** + * Defer any increments of owner counts until endAddingOwnersDeferral is called. So if an + * existing SkBitmap is inserted into the SkBitmapHeap, its corresponding SkBitmapHeapEntry will + * not have addReferences called on it, and the client does not need to make a corresponding + * call to releaseRef. Only meaningful if this SkBitmapHeap was created with an owner count not + * equal to IGNORE_OWNERS. + */ + void deferAddingOwners(); + + /** + * Resume adding references when duplicate SkBitmaps are inserted. + * @param add If true, add references to the SkBitmapHeapEntrys whose SkBitmaps were re-inserted + * while deferring. + */ + void endAddingOwnersDeferral(bool add); + private: struct LookupEntry { LookupEntry(const SkBitmap& bm) @@ -274,6 +290,9 @@ private: const int32_t fOwnerCount; size_t fBytesAllocated; + bool fDeferAddingOwners; + SkTDArray fDeferredEntries; + typedef SkBitmapHeapReader INHERITED; }; diff --git a/src/pipe/SkGPipeRead.cpp b/src/pipe/SkGPipeRead.cpp index 9296e5c..dc7dbfb 100644 --- a/src/pipe/SkGPipeRead.cpp +++ b/src/pipe/SkGPipeRead.cpp @@ -62,7 +62,7 @@ public: ~SkRefCntTDArray() { this->unrefAll(); } }; -class SkGPipeState { +class SkGPipeState : public SkBitmapHeapReader { public: SkGPipeState(); ~SkGPipeState(); @@ -123,13 +123,23 @@ public: bm->unflatten(*fReader); } - SkBitmap* getBitmap(unsigned index) { + /** + * Override of SkBitmapHeapReader, so that SkOrderedReadBuffer can use + * these SkBitmaps for bitmap shaders. + */ + virtual SkBitmap* getBitmap(int32_t index) const SK_OVERRIDE { return fBitmaps[index]; } + /** + * Needed to be a non-abstract subclass of SkBitmapHeapReader. + */ + virtual void releaseRef(int32_t) SK_OVERRIDE {} + void setSharedHeap(SkBitmapHeap* heap) { SkASSERT(!shouldFlattenBitmaps(fFlags) || NULL == heap); SkRefCnt_SafeAssign(fSharedHeap, heap); + this->updateReader(); } SkBitmapHeap* getSharedHeap() const { @@ -160,6 +170,12 @@ private: } else { fReader->setFactoryArray(NULL); } + + if (shouldFlattenBitmaps(fFlags)) { + fReader->setBitmapStorage(this); + } else { + fReader->setBitmapStorage(fSharedHeap); + } } SkOrderedReadBuffer* fReader; SkPaint fPaint; diff --git a/src/pipe/SkGPipeWrite.cpp b/src/pipe/SkGPipeWrite.cpp index fbb67be..119a809 100644 --- a/src/pipe/SkGPipeWrite.cpp +++ b/src/pipe/SkGPipeWrite.cpp @@ -77,6 +77,10 @@ public: virtual void unalloc(void* ptr) SK_OVERRIDE; + void setBitmapStorage(SkBitmapHeap* heap) { + this->setBitmapHeap(heap); + } + const SkFlatData* flatToReplace() const; // Mark an SkFlatData as one that should not be returned by flatToReplace. @@ -170,6 +174,9 @@ public: // there is no circular reference, so the SharedHeap can be // safely unreffed in the destructor. fSharedHeap->unref(); + // This eliminates a similar circular reference (Canvas owns + // the FlattenableHeap which holds a ref to fSharedHeap). + fFlattenableHeap.setBitmapStorage(NULL); fSharedHeap = NULL; } } @@ -345,13 +352,15 @@ int SkGPipeCanvas::flattenToIndex(SkFlattenable* obj, PaintFlats paintflat) { if (isCrossProcess(fFlags)) { writeBufferFlags = SkFlattenableWriteBuffer::kCrossProcess_Flag; } else { - // Needed for bitmap shaders. - writeBufferFlags = SkFlattenableWriteBuffer::kForceFlattenBitmapPixels_Flag; + // TODO: See if we can safely move this into the controller + writeBufferFlags = 0; } + fSharedHeap->deferAddingOwners(); bool added, replaced; const SkFlatData* flat = fFlatDictionary.findAndReplace( *obj, writeBufferFlags, fFlattenableHeap.flatToReplace(), &added, &replaced); + fSharedHeap->endAddingOwnersDeferral(added); int index = flat->index(); if (added) { if (isCrossProcess(fFlags)) { @@ -432,6 +441,7 @@ SkGPipeCanvas::SkGPipeCanvas(SkGPipeController* controller, fWriter.writePtr(static_cast(fSharedHeap)); } } + fFlattenableHeap.setBitmapStorage(fSharedHeap); this->doNotify(); } -- 2.7.4