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

2 files changed, 253 insertions, 14 deletions

diff --git a/src/utils/SkDeferredCanvas.cpp b/src/utils/SkDeferredCanvas.cpp
index 6dce383b12..45fa11f1e0 100644
--- a/src/utils/SkDeferredCanvas.cpp
+++ b/src/utils/SkDeferredCanvas.cpp
@@ -242,6 +242,8 @@ private:
 
     void beginRecording();
     void init();
+    void aboutToDraw();
+    void prepareForImmediatePixelWrite();
 
     DeferredPipeController fPipeController;
     SkGPipeWriter  fPipeWriter;
@@ -343,22 +345,25 @@ bool DeferredDevice::hasPendingCommands() {
     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 @@ GrRenderTarget* DeferredDevice::accessRenderTarget() {
     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 @@ void DeferredDevice::writePixels(const SkBitmap& bitmap,
         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 @@ void DeferredDevice::writePixels(const SkBitmap& bitmap,
     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 1289c036f6..e1af257839 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 @@ static void TestDeferredCanvasBitmapAccess(skiatest::Reporter* reporter) {
     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 @@ static void TestDeferredCanvas(skiatest::Reporter* reporter, GrContextFactory* f
     TestDeferredCanvasBitmapShaderNoLeak(reporter);
     TestDeferredCanvasBitmapSizeThreshold(reporter);
     TestDeferredCanvasCreateCompatibleDevice(reporter);
+    TestDeferredCanvasWritePixelsToSurface(reporter);
     TestDeferredCanvasSurface(reporter, NULL);
     TestDeferredCanvasSetSurface(reporter, NULL);
     if (NULL != factory) {