diff options
-rw-r--r-- | gm/gmmain.cpp | 2 | ||||
-rw-r--r-- | include/pipe/SkGPipe.h | 12 | ||||
-rw-r--r-- | include/utils/SkDeferredCanvas.h | 16 | ||||
-rw-r--r-- | src/pipe/SkGPipePriv.h | 9 | ||||
-rw-r--r-- | src/pipe/SkGPipeWrite.cpp | 38 | ||||
-rw-r--r-- | src/utils/SkDeferredCanvas.cpp | 46 | ||||
-rw-r--r-- | tests/DeferredCanvasTest.cpp | 2 |
7 files changed, 60 insertions, 65 deletions
diff --git a/gm/gmmain.cpp b/gm/gmmain.cpp index 9f109f5dc6..ba8c590439 100644 --- a/gm/gmmain.cpp +++ b/gm/gmmain.cpp @@ -629,7 +629,7 @@ static PipeFlagComboData gPipeWritingFlagCombos[] = { { "", 0 }, { " cross-process", SkGPipeWriter::kCrossProcess_Flag }, { " cross-process, shared address", SkGPipeWriter::kCrossProcess_Flag - | SkGPipeWriter::kSharedAddressSpace_SkGPipeFlag } + | SkGPipeWriter::kSharedAddressSpace_Flag } }; static ErrorBitfield test_pipe_playback(GM* gm, diff --git a/include/pipe/SkGPipe.h b/include/pipe/SkGPipe.h index 878b69330a..d06148c662 100644 --- a/include/pipe/SkGPipe.h +++ b/include/pipe/SkGPipe.h @@ -89,7 +89,7 @@ public: * in spite of being cross process, it will have shared address space * with the reader, so the two can share large objects (like SkBitmaps) */ - kSharedAddressSpace_SkGPipeFlag = 1 << 1 + kSharedAddressSpace_Flag = 1 << 1 }; SkCanvas* startRecording(SkGPipeController*, uint32_t flags = 0); @@ -106,9 +106,17 @@ public: */ void flushRecording(bool detachCurrentBlock); + /** + * Return the amount of bytes being used for recording. Note that this + * does not include the amount of storage written to the stream, which is + * controlled by the SkGPipeController. + * Currently only returns the amount used for SkBitmaps, since they are + * potentially unbounded (if the client is not calling playback). + */ + size_t storageAllocatedForRecording(); + private: class SkGPipeCanvas* fCanvas; - SkGPipeController* fController; SkFactorySet fFactorySet; SkWriter32 fWriter; }; diff --git a/include/utils/SkDeferredCanvas.h b/include/utils/SkDeferredCanvas.h index 9adc3320de..68f538fd45 100644 --- a/include/utils/SkDeferredCanvas.h +++ b/include/utils/SkDeferredCanvas.h @@ -245,12 +245,6 @@ public: void contentsCleared(); void setMaxRecordingStorage(size_t); - // FIXME: Temporary solution for tracking memory usage, pending - // resolution of http://code.google.com/p/skia/issues/detail?id=738 -#if SK_DEFERRED_CANVAS_USES_GPIPE - void accountForTempBitmapStorage(const SkBitmap& bitmap); -#endif - virtual uint32_t getDeviceCapabilities() SK_OVERRIDE; virtual int width() const SK_OVERRIDE; virtual int height() const SK_OVERRIDE; @@ -339,9 +333,6 @@ public: #if SK_DEFERRED_CANVAS_USES_GPIPE DeferredPipeController fPipeController; SkGPipeWriter fPipeWriter; - // FIXME: Temporary solution for tracking memory usage, pending - // resolution of http://code.google.com/p/skia/issues/detail?id=738 - size_t fTempBitmapStorage; #else SkPicture fPicture; #endif @@ -359,13 +350,6 @@ protected: virtual SkCanvas* canvasForDrawIter(); private: - // FIXME: Temporary solution for tracking memory usage, pending - // resolution of http://code.google.com/p/skia/issues/detail?id=738 -#if SK_DEFERRED_CANVAS_USES_GPIPE - friend class AutoImmediateDrawIfNeeded; - void accountForTempBitmapStorage(const SkBitmap& bitmap) const; -#endif - SkCanvas* drawingCanvas() const; bool isFullFrame(const SkRect*, const SkPaint*) const; void validate() const; diff --git a/src/pipe/SkGPipePriv.h b/src/pipe/SkGPipePriv.h index a48d15d239..72b7f377a4 100644 --- a/src/pipe/SkGPipePriv.h +++ b/src/pipe/SkGPipePriv.h @@ -144,6 +144,7 @@ public: BitmapInfo(SkBitmap* bitmap, uint32_t genID, int toBeDrawnCount) : fBitmap(bitmap) , fGenID(genID) + , fBytesAllocated(0) , fMoreRecentlyUsed(NULL) , fLessRecentlyUsed(NULL) , fToBeDrawnCount(toBeDrawnCount) @@ -177,7 +178,13 @@ public: // Store the generation ID of the original bitmap, since copying does // not copy this field, so fBitmap's generation ID will not be useful // for comparing. + // FIXME: Is it reasonable to make copying a bitmap/pixelref copy the + // generation ID? uint32_t fGenID; + // Keep track of the bytes allocated for this bitmap. When replacing the + // bitmap or removing this BitmapInfo we know how much memory has been + // reclaimed. + size_t fBytesAllocated; // TODO: Generalize the LRU caching mechanism BitmapInfo* fMoreRecentlyUsed; BitmapInfo* fLessRecentlyUsed; @@ -187,7 +194,7 @@ private: static inline bool shouldFlattenBitmaps(uint32_t flags) { return flags & SkGPipeWriter::kCrossProcess_Flag - && !(flags & SkGPipeWriter::kSharedAddressSpace_SkGPipeFlag); + && !(flags & SkGPipeWriter::kSharedAddressSpace_Flag); } /////////////////////////////////////////////////////////////////////////////// diff --git a/src/pipe/SkGPipeWrite.cpp b/src/pipe/SkGPipeWrite.cpp index dff3e1423e..a6ffae1a37 100644 --- a/src/pipe/SkGPipeWrite.cpp +++ b/src/pipe/SkGPipeWrite.cpp @@ -161,19 +161,32 @@ public: , fMostRecentlyUsed(NULL) , fLeastRecentlyUsed(NULL) , fCanDoShallowCopies(shallow) - , fNumberOfReaders(numOfReaders) {} + , fNumberOfReaders(numOfReaders) + , fBytesAllocated(0) {} ~SharedHeap() { BitmapInfo* iter = fMostRecentlyUsed; while (iter != NULL) { + SkDEBUGCODE(fBytesAllocated -= (iter->fBytesAllocated + sizeof(BitmapInfo))); BitmapInfo* next = iter->fLessRecentlyUsed; SkDELETE(iter); fBitmapCount--; iter = next; } SkASSERT(0 == fBitmapCount); + SkASSERT(0 == fBytesAllocated); } /* + * Get the approximate number of bytes allocated. + * + * Not exact. Some SkBitmaps may share SkPixelRefs, in which case only one + * SkBitmap will take the size of the SkPixelRef into account (the first + * one). It is possible that the one which accounts for the SkPixelRef has + * been removed, in which case we will no longer be counting those bytes. + */ + size_t bytesAllocated() { return fBytesAllocated; } + + /* * Add a copy of a bitmap to the heap. * @param bm The SkBitmap to be copied and placed in the heap. * @return void* Pointer to the BitmapInfo stored in the heap, which @@ -190,6 +203,10 @@ public: while (iter != NULL) { if (genID == iter->fGenID) { SkBitmap* storedBitmap = iter->fBitmap; + // TODO: Perhaps we can share code with + // SkPictureRecord::PixelRefDictionaryEntry/ + // BitmapIndexCacheEntry so we can do a binary search for a + // matching bitmap if (orig.pixelRefOffset() != storedBitmap->pixelRefOffset() || orig.width() != storedBitmap->width() || orig.height() != storedBitmap->height()) { @@ -246,13 +263,23 @@ public: } BitmapInfo* info; if (NULL == replace) { + fBytesAllocated += sizeof(BitmapInfo); info = SkNEW_ARGS(BitmapInfo, (copy, genID, fNumberOfReaders)); fBitmapCount++; } else { + fBytesAllocated -= replace->fBytesAllocated; replace->fGenID = genID; replace->addDraws(fNumberOfReaders); info = replace; } + // Always include the size of the SkBitmap struct. + info->fBytesAllocated = sizeof(SkBitmap); + // If the SkBitmap does not share an SkPixelRef with an SkBitmap already + // in the SharedHeap, also include the size of its pixels. + if (NULL == sharedPixelRef) { + info->fBytesAllocated += orig.getSize(); + } + fBytesAllocated += info->fBytesAllocated; this->setMostRecentlyUsed(info); return info; } @@ -265,6 +292,7 @@ private: BitmapInfo* fMostRecentlyUsed; const bool fCanDoShallowCopies; const int fNumberOfReaders; + size_t fBytesAllocated; }; // We just "used" info. Update our LRU accordingly @@ -352,6 +380,10 @@ public: void flushRecording(bool detachCurrentBlock); + size_t storageAllocatedForRecording() { + return fSharedHeap.bytesAllocated(); + } + // overrides from SkCanvas virtual int save(SaveFlags) SK_OVERRIDE; virtual int saveLayer(const SkRect* bounds, const SkPaint*, @@ -1251,3 +1283,7 @@ void SkGPipeWriter::flushRecording(bool detachCurrentBlock){ fCanvas->flushRecording(detachCurrentBlock); } +size_t SkGPipeWriter::storageAllocatedForRecording() { + return NULL == fCanvas ? 0 : fCanvas->storageAllocatedForRecording(); +} + diff --git a/src/utils/SkDeferredCanvas.cpp b/src/utils/SkDeferredCanvas.cpp index a36a604f4b..c5a21831d4 100644 --- a/src/utils/SkDeferredCanvas.cpp +++ b/src/utils/SkDeferredCanvas.cpp @@ -35,13 +35,6 @@ public: } else { fCanvas = NULL; } - // FIXME: Temporary solution for tracking memory usage, pending - // resolution of http://code.google.com/p/skia/issues/detail?id=738 -#if SK_DEFERRED_CANVAS_USES_GPIPE - if (canvas.isDeferredDrawing()) { - canvas.accountForTempBitmapStorage(bitmap); - } -#endif } ~AutoImmediateDrawIfNeeded() { @@ -143,16 +136,6 @@ void SkDeferredCanvas::setMaxRecordingStorage(size_t maxStorage) { this->getDeferredDevice()->setMaxRecordingStorage(maxStorage); } -// FIXME: Temporary solution for tracking memory usage, pending -// resolution of http://code.google.com/p/skia/issues/detail?id=738 -#if SK_DEFERRED_CANVAS_USES_GPIPE -void SkDeferredCanvas::accountForTempBitmapStorage(const SkBitmap& bitmap) const { - if (fDeferredDrawing) { - this->getDeferredDevice()->accountForTempBitmapStorage(bitmap); - } -} -#endif - void SkDeferredCanvas::validate() const { SkASSERT(getDevice()); } @@ -563,7 +546,6 @@ SkDeferredCanvas::DeferredDevice::DeferredDevice( fImmediateCanvas = SkNEW_ARGS(SkCanvas, (fImmediateDevice)); #if SK_DEFERRED_CANVAS_USES_GPIPE fPipeController.setPlaybackCanvas(fImmediateCanvas); - fTempBitmapStorage = 0; #endif beginRecording(); } @@ -579,26 +561,10 @@ void SkDeferredCanvas::DeferredDevice::setMaxRecordingStorage(size_t maxStorage) recordingCanvas(); // Accessing the recording canvas applies the new limit. } -#if SK_DEFERRED_CANVAS_USES_GPIPE -void SkDeferredCanvas::DeferredDevice::accountForTempBitmapStorage(const SkBitmap& bitmap) { - // SkGPipe will store copies of mutable bitmaps. The memory allocations - // and deallocations for these bitmaps are not tracked by the writer or - // the controller, so we do as best we can to track consumption here - if (!bitmap.isImmutable()) { - // FIXME: Temporary solution for tracking memory usage, pending - // resolution of http://code.google.com/p/skia/issues/detail?id=738 - // This does not take into account duplicates of previously - // copied bitmaps that will not get copied again. - fTempBitmapStorage += bitmap.getSize(); - } -} -#endif - void SkDeferredCanvas::DeferredDevice::endRecording() { #if SK_DEFERRED_CANVAS_USES_GPIPE fPipeWriter.endRecording(); fPipeController.reset(); - fTempBitmapStorage = 0; #else fPicture.endRecording(); #endif @@ -607,7 +573,6 @@ void SkDeferredCanvas::DeferredDevice::endRecording() { void SkDeferredCanvas::DeferredDevice::beginRecording() { #if SK_DEFERRED_CANVAS_USES_GPIPE - SkASSERT(0 == fTempBitmapStorage); fRecordingCanvas = fPipeWriter.startRecording(&fPipeController, 0); #else fRecordingCanvas = fPicture.beginRecording(fImmediateDevice->width(), @@ -679,7 +644,6 @@ void SkDeferredCanvas::DeferredDevice::flushPending() { #if SK_DEFERRED_CANVAS_USES_GPIPE fPipeWriter.flushRecording(true); fPipeController.playback(); - fTempBitmapStorage = 0; #else fPicture.draw(fImmediateCanvas); this->beginRecording(); @@ -693,8 +657,9 @@ void SkDeferredCanvas::DeferredDevice::flush() { SkCanvas* SkDeferredCanvas::DeferredDevice::recordingCanvas() { #if SK_DEFERRED_CANVAS_USES_GPIPE - if (fPipeController.storageAllocatedForRecording() + fTempBitmapStorage > - fMaxRecordingStorageBytes) { + if (fPipeController.storageAllocatedForRecording() + + fPipeWriter.storageAllocatedForRecording() + > fMaxRecordingStorageBytes) { this->flushPending(); } #endif @@ -741,11 +706,6 @@ void SkDeferredCanvas::DeferredDevice::writePixels(const SkBitmap& bitmap, this->flushPending(); fImmediateCanvas->drawSprite(bitmap, x, y, &paint); } else { -#if SK_DEFERRED_CANVAS_USES_GPIPE - // FIXME: Temporary solution for tracking memory usage, pending - // resolution of http://code.google.com/p/skia/issues/detail?id=738 - this->accountForTempBitmapStorage(bitmap); -#endif recordingCanvas()->drawSprite(bitmap, x, y, &paint); } } diff --git a/tests/DeferredCanvasTest.cpp b/tests/DeferredCanvasTest.cpp index 0fee1808c4..7a8ed9d659 100644 --- a/tests/DeferredCanvasTest.cpp +++ b/tests/DeferredCanvasTest.cpp @@ -212,7 +212,7 @@ static void TestDeferredCanvasMemoryLimit(skiatest::Reporter* reporter) { // SkPicture path is not fixed #if SK_DEFERRED_CANVAS_USES_GPIPE - REPORTER_ASSERT(reporter, mockDevice.fDrawBitmapCallCount == 3); + REPORTER_ASSERT(reporter, mockDevice.fDrawBitmapCallCount == 4); #endif } |