From b10a6bd0a7df0ceeea0d53585c049450ec58b4b9 Mon Sep 17 00:00:00 2001 From: "junov@chromium.org" Date: Wed, 25 Jul 2012 17:27:13 +0000 Subject: [PATCH] Refactoring how SkDeferredCanvas manages mutable bitmaps This CL makes the SkGPipe flavor of SkDeferredCanvas properly decide whether to flush or record mutable bitmaps. The flushing is now managed by conditionally switching the canvas to non-deferred mode, which avoids an unnecessary transient copy of the bitmap. BUG=http://code.google.com/p/chromium/issues/detail?id=137884 TEST=DeferredCanvas unit test, sub test TestDeferredCanvasMemoryLimit Review URL: https://codereview.appspot.com/6421060 git-svn-id: http://skia.googlecode.com/svn/trunk@4756 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/utils/SkDeferredCanvas.h | 25 +++++-- src/utils/SkDeferredCanvas.cpp | 119 ++++++++++++++++++++++--------- tests/DeferredCanvasTest.cpp | 10 ++- 3 files changed, 110 insertions(+), 44 deletions(-) diff --git a/include/utils/SkDeferredCanvas.h b/include/utils/SkDeferredCanvas.h index 829a72fccc..9adc3320de 100644 --- a/include/utils/SkDeferredCanvas.h +++ b/include/utils/SkDeferredCanvas.h @@ -85,6 +85,11 @@ public: */ void setDeferredDrawing(bool deferred); + /** + * Returns true if deferred drawing is currenlty enabled. + */ + bool isDeferredDrawing(); + /** * Specify the maximum number of bytes to be allocated for the purpose * of recording draw commands to this canvas. The default limit, is @@ -153,9 +158,6 @@ public: virtual SkBounder* setBounder(SkBounder* bounder) SK_OVERRIDE; virtual SkDrawFilter* setDrawFilter(SkDrawFilter* filter) SK_OVERRIDE; -private: - void flushIfNeeded(const SkBitmap& bitmap); - public: class DeviceContext : public SkRefCnt { public: @@ -241,9 +243,14 @@ public: void flushPending(); void contentsCleared(); - void flushIfNeeded(const SkBitmap& bitmap); 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; @@ -332,6 +339,9 @@ 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 @@ -349,6 +359,13 @@ 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/utils/SkDeferredCanvas.cpp b/src/utils/SkDeferredCanvas.cpp index 728e3d39d3..494b50c39f 100644 --- a/src/utils/SkDeferredCanvas.cpp +++ b/src/utils/SkDeferredCanvas.cpp @@ -20,6 +20,39 @@ enum { kDefaultMaxRecordingStorageBytes = 64*1024*1024, }; +namespace { +bool shouldDrawImmediately(const SkBitmap& bitmap) { + return bitmap.getTexture() && !bitmap.isImmutable(); +} +} + +class AutoImmediateDrawIfNeeded { +public: + AutoImmediateDrawIfNeeded(SkDeferredCanvas& canvas, const SkBitmap& bitmap) { + if (canvas.isDeferredDrawing() && shouldDrawImmediately(bitmap)) { + canvas.setDeferredDrawing(false); + fCanvas = &canvas; + } 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() { + if (fCanvas) { + fCanvas->setDeferredDrawing(true); + } + } +private: + SkDeferredCanvas* fCanvas; +}; + namespace { bool isPaintOpaque(const SkPaint* paint, @@ -110,6 +143,16 @@ 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()); } @@ -120,20 +163,12 @@ SkCanvas* SkDeferredCanvas::drawingCanvas() const { getDeferredDevice()->immediateCanvas(); } -void SkDeferredCanvas::flushIfNeeded(const SkBitmap& bitmap) { - validate(); - if (fDeferredDrawing) { - getDeferredDevice()->flushIfNeeded(bitmap); - } -} - SkDeferredCanvas::DeferredDevice* SkDeferredCanvas::getDeferredDevice() const { return static_cast(getDevice()); } void SkDeferredCanvas::setDeferredDrawing(bool val) { validate(); // Must set device before calling this method - SkASSERT(drawingCanvas()->getSaveCount() == 1); if (val != fDeferredDrawing) { if (fDeferredDrawing) { // Going live. @@ -143,6 +178,10 @@ void SkDeferredCanvas::setDeferredDrawing(bool val) { } } +bool SkDeferredCanvas::isDeferredDrawing() { + return fDeferredDrawing; +} + SkDeferredCanvas::~SkDeferredCanvas() { } @@ -335,8 +374,8 @@ void SkDeferredCanvas::drawBitmap(const SkBitmap& bitmap, SkScalar left, getDeferredDevice()->contentsCleared(); } + AutoImmediateDrawIfNeeded autoDraw(*this, bitmap); drawingCanvas()->drawBitmap(bitmap, left, top, paint); - flushIfNeeded(bitmap); } void SkDeferredCanvas::drawBitmapRect(const SkBitmap& bitmap, @@ -349,9 +388,9 @@ void SkDeferredCanvas::drawBitmapRect(const SkBitmap& bitmap, getDeferredDevice()->contentsCleared(); } + AutoImmediateDrawIfNeeded autoDraw(*this, bitmap); drawingCanvas()->drawBitmapRect(bitmap, src, dst, paint); - flushIfNeeded(bitmap); } @@ -360,8 +399,8 @@ void SkDeferredCanvas::drawBitmapMatrix(const SkBitmap& bitmap, const SkPaint* paint) { // TODO: reset recording canvas if paint+bitmap is opaque and clip rect // covers canvas entirely and transformed bitmap covers canvas entirely + AutoImmediateDrawIfNeeded autoDraw(*this, bitmap); drawingCanvas()->drawBitmapMatrix(bitmap, m, paint); - flushIfNeeded(bitmap); } void SkDeferredCanvas::drawBitmapNine(const SkBitmap& bitmap, @@ -369,9 +408,9 @@ void SkDeferredCanvas::drawBitmapNine(const SkBitmap& bitmap, const SkPaint* paint) { // TODO: reset recording canvas if paint+bitmap is opaque and clip rect // covers canvas entirely and dst covers canvas entirely + AutoImmediateDrawIfNeeded autoDraw(*this, bitmap); drawingCanvas()->drawBitmapNine(bitmap, center, dst, paint); - flushIfNeeded(bitmap); } void SkDeferredCanvas::drawSprite(const SkBitmap& bitmap, int left, int top, @@ -387,9 +426,9 @@ void SkDeferredCanvas::drawSprite(const SkBitmap& bitmap, int left, int top, getDeferredDevice()->contentsCleared(); } + AutoImmediateDrawIfNeeded autoDraw(*this, bitmap); drawingCanvas()->drawSprite(bitmap, left, top, paint); - flushIfNeeded(bitmap); } void SkDeferredCanvas::drawText(const void* text, size_t byteLength, @@ -524,11 +563,13 @@ SkDeferredCanvas::DeferredDevice::DeferredDevice( fImmediateCanvas = SkNEW_ARGS(SkCanvas, (fImmediateDevice)); #if SK_DEFERRED_CANVAS_USES_GPIPE fPipeController.setPlaybackCanvas(fImmediateCanvas); + fTempBitmapStorage = 0; #endif beginRecording(); } SkDeferredCanvas::DeferredDevice::~DeferredDevice() { + flushPending(); SkSafeUnref(fImmediateCanvas); SkSafeUnref(fDeviceContext); } @@ -538,10 +579,26 @@ 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 @@ -550,6 +607,7 @@ 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(), @@ -621,6 +679,7 @@ void SkDeferredCanvas::DeferredDevice::flushPending() { #if SK_DEFERRED_CANVAS_USES_GPIPE fPipeWriter.flushRecording(true); fPipeController.playback(); + fTempBitmapStorage = 0; #else fPicture.draw(fImmediateCanvas); this->beginRecording(); @@ -634,31 +693,14 @@ void SkDeferredCanvas::DeferredDevice::flush() { SkCanvas* SkDeferredCanvas::DeferredDevice::recordingCanvas() { #if SK_DEFERRED_CANVAS_USES_GPIPE - if (fPipeController.storageAllocatedForRecording() > fMaxRecordingStorageBytes) { + if (fPipeController.storageAllocatedForRecording() + fTempBitmapStorage > + fMaxRecordingStorageBytes) { this->flushPending(); } #endif return fRecordingCanvas; } -void SkDeferredCanvas::DeferredDevice::flushIfNeeded(const SkBitmap& bitmap) { -#if SK_DEFERRED_CANVAS_USES_GPIPE - if (bitmap.isImmutable()) { - // FIXME: Make SkGPipe flatten software-backed non-immutable bitmaps - return; - } -#else - if (bitmap.isImmutable() || fPicture.willFlattenPixelsOnRecord(bitmap)) { - return; // safe to defer. - } -#endif - - // For now, drawing a writable bitmap triggers a flush - // TODO: implement read-only semantics and auto buffer duplication on write - // in SkBitmap/SkPixelRef, which will make deferral possible in this case. - this->flushPending(); -} - uint32_t SkDeferredCanvas::DeferredDevice::getDeviceCapabilities() { return fImmediateDevice->getDeviceCapabilities(); } @@ -694,8 +736,17 @@ void SkDeferredCanvas::DeferredDevice::writePixels(const SkBitmap& bitmap, SkPaint paint; paint.setXfermodeMode(SkXfermode::kSrc_Mode); - fRecordingCanvas->drawSprite(bitmap, x, y, &paint); - flushIfNeeded(bitmap); + if (shouldDrawImmediately(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); + } } const SkBitmap& SkDeferredCanvas::DeferredDevice::onAccessBitmap(SkBitmap*) { diff --git a/tests/DeferredCanvasTest.cpp b/tests/DeferredCanvasTest.cpp index 449f5e97bc..0fee1808c4 100644 --- a/tests/DeferredCanvasTest.cpp +++ b/tests/DeferredCanvasTest.cpp @@ -205,16 +205,14 @@ static void TestDeferredCanvasMemoryLimit(skiatest::Reporter* reporter) { sourceImage.setConfig(SkBitmap::kARGB_8888_Config, 100, 100); sourceImage.allocPixels(); - for (int i = 0; i < 6; i++) { + for (int i = 0; i < 5; i++) { sourceImage.notifyPixelsChanged(); // to force re-serialization canvas.drawBitmap(sourceImage, 0, 0, NULL); } - // FIXME: Test temporarily disabled because the SkPicture path is not - // fixed and the SkGPipe path does not yet serialize images, but it - // will soon. -#if 0 - REPORTER_ASSERT(reporter, mockDevice.fDrawBitmapCallCount == 4); + // SkPicture path is not fixed +#if SK_DEFERRED_CANVAS_USES_GPIPE + REPORTER_ASSERT(reporter, mockDevice.fDrawBitmapCallCount == 3); #endif }