Make SkDeferredCanvas query SkGPipeWriter for space allocated for bitmaps.
SkGPipe now has a method to report how much memory is used for its shared heap.
BUG=http://code.google.com/p/skia/issues/detail?id=738
TEST=DeferredCanvasTest
Review URL: https://codereview.appspot.com/6445046
git-svn-id: http://skia.googlecode.com/svn/trunk@4791 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/gm/gmmain.cpp b/gm/gmmain.cpp
index 9f109f5..ba8c590 100644
--- a/gm/gmmain.cpp
+++ b/gm/gmmain.cpp
@@ -629,7 +629,7 @@
{ "", 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 878b693..d06148c 100644
--- a/include/pipe/SkGPipe.h
+++ b/include/pipe/SkGPipe.h
@@ -89,7 +89,7 @@
* 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 @@
*/
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 9adc332..68f538f 100644
--- a/include/utils/SkDeferredCanvas.h
+++ b/include/utils/SkDeferredCanvas.h
@@ -245,12 +245,6 @@
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 @@
#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 @@
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 a48d15d..72b7f37 100644
--- a/src/pipe/SkGPipePriv.h
+++ b/src/pipe/SkGPipePriv.h
@@ -144,6 +144,7 @@
BitmapInfo(SkBitmap* bitmap, uint32_t genID, int toBeDrawnCount)
: fBitmap(bitmap)
, fGenID(genID)
+ , fBytesAllocated(0)
, fMoreRecentlyUsed(NULL)
, fLessRecentlyUsed(NULL)
, fToBeDrawnCount(toBeDrawnCount)
@@ -177,7 +178,13 @@
// 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 @@
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 dff3e14..a6ffae1 100644
--- a/src/pipe/SkGPipeWrite.cpp
+++ b/src/pipe/SkGPipeWrite.cpp
@@ -161,19 +161,32 @@
, 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 @@
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 @@
}
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 @@
BitmapInfo* fMostRecentlyUsed;
const bool fCanDoShallowCopies;
const int fNumberOfReaders;
+ size_t fBytesAllocated;
};
// We just "used" info. Update our LRU accordingly
@@ -352,6 +380,10 @@
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 @@
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 a36a604..c5a2183 100644
--- a/src/utils/SkDeferredCanvas.cpp
+++ b/src/utils/SkDeferredCanvas.cpp
@@ -35,13 +35,6 @@
} 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 @@
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 @@
fImmediateCanvas = SkNEW_ARGS(SkCanvas, (fImmediateDevice));
#if SK_DEFERRED_CANVAS_USES_GPIPE
fPipeController.setPlaybackCanvas(fImmediateCanvas);
- fTempBitmapStorage = 0;
#endif
beginRecording();
}
@@ -579,26 +561,10 @@
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::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 @@
#if SK_DEFERRED_CANVAS_USES_GPIPE
fPipeWriter.flushRecording(true);
fPipeController.playback();
- fTempBitmapStorage = 0;
#else
fPicture.draw(fImmediateCanvas);
this->beginRecording();
@@ -693,8 +657,9 @@
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 @@
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 0fee180..7a8ed9d 100644
--- a/tests/DeferredCanvasTest.cpp
+++ b/tests/DeferredCanvasTest.cpp
@@ -212,7 +212,7 @@
// SkPicture path is not fixed
#if SK_DEFERRED_CANVAS_USES_GPIPE
- REPORTER_ASSERT(reporter, mockDevice.fDrawBitmapCallCount == 3);
+ REPORTER_ASSERT(reporter, mockDevice.fDrawBitmapCallCount == 4);
#endif
}