Fixing SkDeferredCanvas::writePixels to trigger appropriate change notifications to SkSurface
BUG=crbug.com/256269
TEST=DeferredCanvas unit test, TestDeferredCanvasWritePixelsToSurface
R=reed@google.com
Review URL: https://codereview.chromium.org/20628005
git-svn-id: http://skia.googlecode.com/svn/trunk@10513 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/src/utils/SkDeferredCanvas.cpp b/src/utils/SkDeferredCanvas.cpp
index 6dce383..45fa11f 100644
--- a/src/utils/SkDeferredCanvas.cpp
+++ b/src/utils/SkDeferredCanvas.cpp
@@ -242,6 +242,8 @@
void beginRecording();
void init();
+ void aboutToDraw();
+ void prepareForImmediatePixelWrite();
DeferredPipeController fPipeController;
SkGPipeWriter fPipeWriter;
@@ -343,22 +345,25 @@
return fPipeController.hasPendingCommands();
}
+void DeferredDevice::aboutToDraw()
+{
+ if (NULL != fNotificationClient) {
+ fNotificationClient->prepareForDraw();
+ }
+ if (fCanDiscardCanvasContents) {
+ if (NULL != fSurface) {
+ fSurface->notifyContentWillChange(SkSurface::kDiscard_ContentChangeMode);
+ }
+ fCanDiscardCanvasContents = false;
+ }
+}
+
void DeferredDevice::flushPendingCommands(PlaybackMode playbackMode) {
if (!fPipeController.hasPendingCommands()) {
return;
}
if (playbackMode == kNormal_PlaybackMode) {
- if (NULL != fNotificationClient) {
- fNotificationClient->prepareForDraw();
- }
- if (fCanDiscardCanvasContents) {
- if (NULL != fSurface) {
- // Pre-empt notifyContentChanged(false) calls that will happen
- // during flush
- fSurface->notifyContentWillChange(SkSurface::kDiscard_ContentChangeMode);
- }
- fCanDiscardCanvasContents = false;
- }
+ aboutToDraw();
}
fPipeWriter.flushRecording(true);
fPipeController.playback(kSilent_PlaybackMode == playbackMode);
@@ -441,6 +446,23 @@
return immediateDevice()->accessRenderTarget();
}
+void DeferredDevice::prepareForImmediatePixelWrite() {
+ // The purpose of the following code is to make sure commands are flushed, that
+ // aboutToDraw() is called and that notifyContentWillChange is called, without
+ // calling anything redundantly.
+ if (fPipeController.hasPendingCommands()) {
+ this->flushPendingCommands(kNormal_PlaybackMode);
+ } else {
+ bool mustNotifyDirectly = !fCanDiscardCanvasContents;
+ this->aboutToDraw();
+ if (mustNotifyDirectly) {
+ fSurface->notifyContentWillChange(SkSurface::kRetain_ContentChangeMode);
+ }
+ }
+
+ fImmediateCanvas->flush();
+}
+
void DeferredDevice::writePixels(const SkBitmap& bitmap,
int x, int y, SkCanvas::Config8888 config8888) {
@@ -453,7 +475,7 @@
SkCanvas::kNative_Premul_Config8888 != config8888 &&
kPMColorAlias != config8888) {
//Special case config: no deferral
- this->flushPendingCommands(kNormal_PlaybackMode);
+ prepareForImmediatePixelWrite();
immediateDevice()->writePixels(bitmap, x, y, config8888);
return;
}
@@ -461,7 +483,7 @@
SkPaint paint;
paint.setXfermodeMode(SkXfermode::kSrc_Mode);
if (shouldDrawImmediately(&bitmap, NULL, getBitmapSizeThreshold())) {
- this->flushPendingCommands(kNormal_PlaybackMode);
+ prepareForImmediatePixelWrite();
fImmediateCanvas->drawSprite(bitmap, x, y, &paint);
} else {
this->recordingCanvas()->drawSprite(bitmap, x, y, &paint);
diff --git a/tests/DeferredCanvasTest.cpp b/tests/DeferredCanvasTest.cpp
index 1289c03..e1af257 100644
--- a/tests/DeferredCanvasTest.cpp
+++ b/tests/DeferredCanvasTest.cpp
@@ -12,7 +12,8 @@
#include "SkDevice.h"
#include "SkGradientShader.h"
#include "SkShader.h"
-#include "SkSurface.h"
+#include "../src/image/SkSurface_Base.h"
+#include "../src/image/SkImagePriv.h"
#if SK_SUPPORT_GPU
#include "GrContextFactory.h"
#else
@@ -49,6 +50,221 @@
REPORTER_ASSERT(reporter, accessed.pixelRef() == store.pixelRef());
}
+class MockSurface : public SkSurface_Base {
+public:
+ MockSurface(int width, int height) : SkSurface_Base(width, height) {
+ clearCounts();
+ fBitmap.setConfig(SkBitmap::kARGB_8888_Config, width, height);
+ fBitmap.allocPixels();
+ }
+
+ virtual SkCanvas* onNewCanvas() SK_OVERRIDE {
+ return SkNEW_ARGS(SkCanvas, (fBitmap));
+ }
+
+ virtual SkSurface* onNewSurface(const SkImage::Info&) SK_OVERRIDE {
+ return NULL;
+ }
+
+ virtual SkImage* onNewImageSnapshot() SK_OVERRIDE {
+ return SkNewImageFromBitmap(fBitmap, true);
+ }
+
+ virtual void onCopyOnWrite(ContentChangeMode mode) SK_OVERRIDE {
+ if (mode == SkSurface::kDiscard_ContentChangeMode) {
+ fDiscardCount++;
+ } else {
+ fRetainCount++;
+ }
+ }
+
+ void clearCounts() {
+ fDiscardCount = 0;
+ fRetainCount = 0;
+ }
+
+ int fDiscardCount, fRetainCount;
+ SkBitmap fBitmap;
+};
+
+static void TestDeferredCanvasWritePixelsToSurface(skiatest::Reporter* reporter) {
+ SkAutoTUnref<MockSurface> surface(SkNEW_ARGS(MockSurface, (10, 10)));
+ SkAutoTUnref<SkDeferredCanvas> canvas(
+#if SK_DEFERRED_CANVAS_USES_FACTORIES
+ SkDeferredCanvas::Create(surface.get()));
+#else
+ SkNEW_ARGS(SkDeferredCanvas, (surface.get())));
+#endif
+
+ SkBitmap srcBitmap;
+ srcBitmap.setConfig(SkBitmap::kARGB_8888_Config, 10, 10);
+ srcBitmap.allocPixels();
+ srcBitmap.eraseColor(SK_ColorGREEN);
+ // Tests below depend on this bitmap being recognized as opaque
+
+ // Preliminary sanity check: no copy on write if no active snapshot
+ surface->clearCounts();
+ canvas->clear(SK_ColorWHITE);
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->flush();
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ // Case 1: Discard notification happens upon flushing
+ // with an Image attached.
+ surface->clearCounts();
+ SkAutoTUnref<SkImage> image1(canvas->newImageSnapshot());
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->clear(SK_ColorWHITE);
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->flush();
+ REPORTER_ASSERT(reporter, 1 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ // Case 2: Opaque writePixels
+ surface->clearCounts();
+ SkAutoTUnref<SkImage> image2(canvas->newImageSnapshot());
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->writePixels(srcBitmap, 0, 0);
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->flush();
+ REPORTER_ASSERT(reporter, 1 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ // Case 3: writePixels that partially covers the canvas
+ surface->clearCounts();
+ SkAutoTUnref<SkImage> image3(canvas->newImageSnapshot());
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->writePixels(srcBitmap, 5, 0);
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->flush();
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 1 == surface->fRetainCount);
+
+ // Case 4: unpremultiplied opaque writePixels that entirely
+ // covers the canvas
+ surface->clearCounts();
+ SkAutoTUnref<SkImage> image4(canvas->newImageSnapshot());
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->writePixels(srcBitmap, 0, 0, SkCanvas::kRGBA_Unpremul_Config8888);
+ REPORTER_ASSERT(reporter, 1 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->flush();
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ // Case 5: unpremultiplied opaque writePixels that partially
+ // covers the canvas
+ surface->clearCounts();
+ SkAutoTUnref<SkImage> image5(canvas->newImageSnapshot());
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->writePixels(srcBitmap, 5, 0, SkCanvas::kRGBA_Unpremul_Config8888);
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 1 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->flush();
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ // Case 6: unpremultiplied opaque writePixels that entirely
+ // covers the canvas, preceded by clear
+ surface->clearCounts();
+ SkAutoTUnref<SkImage> image6(canvas->newImageSnapshot());
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->clear(SK_ColorWHITE);
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->writePixels(srcBitmap, 0, 0, SkCanvas::kRGBA_Unpremul_Config8888);
+ REPORTER_ASSERT(reporter, 1 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->flush();
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ // Case 7: unpremultiplied opaque writePixels that partially
+ // covers the canvas, preceeded by a clear
+ surface->clearCounts();
+ SkAutoTUnref<SkImage> image7(canvas->newImageSnapshot());
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->clear(SK_ColorWHITE);
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->writePixels(srcBitmap, 5, 0, SkCanvas::kRGBA_Unpremul_Config8888);
+ REPORTER_ASSERT(reporter, 1 == surface->fDiscardCount); // because of the clear
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->flush();
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ // Case 8: unpremultiplied opaque writePixels that partially
+ // covers the canvas, preceeded by a drawREct that partially
+ // covers the canvas
+ surface->clearCounts();
+ SkAutoTUnref<SkImage> image8(canvas->newImageSnapshot());
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ SkPaint paint;
+ canvas->drawRect(SkRect::MakeLTRB(0, 0, 5, 5), paint);
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->writePixels(srcBitmap, 5, 0, SkCanvas::kRGBA_Unpremul_Config8888);
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 1 == surface->fRetainCount);
+
+ surface->clearCounts();
+ canvas->flush();
+ REPORTER_ASSERT(reporter, 0 == surface->fDiscardCount);
+ REPORTER_ASSERT(reporter, 0 == surface->fRetainCount);
+}
+
static void TestDeferredCanvasFlush(skiatest::Reporter* reporter) {
SkBitmap store;
@@ -689,6 +905,7 @@
TestDeferredCanvasBitmapShaderNoLeak(reporter);
TestDeferredCanvasBitmapSizeThreshold(reporter);
TestDeferredCanvasCreateCompatibleDevice(reporter);
+ TestDeferredCanvasWritePixelsToSurface(reporter);
TestDeferredCanvasSurface(reporter, NULL);
TestDeferredCanvasSetSurface(reporter, NULL);
if (NULL != factory) {