From 1f9767c03bad1ef85e5388d84e23e4b5dff4bc1a Mon Sep 17 00:00:00 2001 From: "junov@chromium.org" Date: Tue, 7 Feb 2012 16:27:57 +0000 Subject: [PATCH] Fixing backing store access in SkDeferredCanvas. Chromium CL required for rolling skia DEPS past this change is posted here: https://chromiumcodereview.appspot.com/9341003/ BUG=http://code.google.com/p/skia/issues/detail?id=475 REVIEW=http://codereview.appspot.com/5626047/ TEST=DeferredCanvas unit test git-svn-id: http://skia.googlecode.com/svn/trunk@3147 2bbb7eff-a529-9590-31e7-b0007b416f81 --- gyp/tests.gyp | 1 + include/core/SkDevice.h | 11 +++++--- include/utils/SkDeferredCanvas.h | 3 +- src/core/SkDevice.cpp | 8 +++--- src/utils/SkDeferredCanvas.cpp | 6 ++-- tests/DeferredCanvasTest.cpp | 59 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 76 insertions(+), 12 deletions(-) create mode 100644 tests/DeferredCanvasTest.cpp diff --git a/gyp/tests.gyp b/gyp/tests.gyp index 011030c..63ff7c9 100644 --- a/gyp/tests.gyp +++ b/gyp/tests.gyp @@ -27,6 +27,7 @@ '../tests/ColorFilterTest.cpp', '../tests/ColorTest.cpp', '../tests/DataRefTest.cpp', + '../tests/DeferredCanvasTest.cpp', '../tests/DequeTest.cpp', '../tests/DrawBitmapRectTest.cpp', '../tests/DrawTextTest.cpp', diff --git a/include/core/SkDevice.h b/include/core/SkDevice.h index 6259fe4..3303981 100644 --- a/include/core/SkDevice.h +++ b/include/core/SkDevice.h @@ -279,11 +279,14 @@ protected: /////////////////////////////////////////////////////////////////////////// - /** Update as needed the pixel value in the bitmap, so that the caller can access - the pixels directly. Note: only the pixels field should be altered. The config/width/height/rowbytes - must remain unchanged. + /** Update as needed the pixel value in the bitmap, so that the caller can + access the pixels directly. Note: only the pixels field should be + altered. The config/width/height/rowbytes must remain unchanged. + @param bitmap The device's bitmap + @return Echo the bitmap parameter, or an alternate (shadow) bitmap + maintained by the subclass. */ - virtual void onAccessBitmap(SkBitmap*); + virtual const SkBitmap& onAccessBitmap(SkBitmap*); SkPixelRef* getPixelRef() const { return fBitmap.pixelRef(); } // just for subclasses, to assign a custom pixelref diff --git a/include/utils/SkDeferredCanvas.h b/include/utils/SkDeferredCanvas.h index b1241bd..19d7141 100644 --- a/include/utils/SkDeferredCanvas.h +++ b/include/utils/SkDeferredCanvas.h @@ -199,7 +199,7 @@ public: SkCanvas::Config8888 config8888) SK_OVERRIDE; protected: - virtual void onAccessBitmap(SkBitmap*) SK_OVERRIDE; + virtual const SkBitmap& onAccessBitmap(SkBitmap*) SK_OVERRIDE; virtual bool onReadPixels(const SkBitmap& bitmap, int x, int y, SkCanvas::Config8888 config8888) SK_OVERRIDE; @@ -275,6 +275,7 @@ public: SkCanvas* fImmediateCanvas; SkCanvas* fRecordingCanvas; DeviceContext* fDeviceContext; + bool fBitmapInitialized; }; DeferredDevice* getDeferredDevice() const; diff --git a/src/core/SkDevice.cpp b/src/core/SkDevice.cpp index dc7feaf..34b5c4f 100644 --- a/src/core/SkDevice.cpp +++ b/src/core/SkDevice.cpp @@ -77,11 +77,11 @@ void SkDevice::unlockPixels() { } const SkBitmap& SkDevice::accessBitmap(bool changePixels) { - this->onAccessBitmap(&fBitmap); + const SkBitmap& bitmap = this->onAccessBitmap(&fBitmap); if (changePixels) { - fBitmap.notifyPixelsChanged(); + bitmap.notifyPixelsChanged(); } - return fBitmap; + return bitmap; } void SkDevice::getGlobalBounds(SkIRect* bounds) const { @@ -95,7 +95,7 @@ void SkDevice::clear(SkColor color) { fBitmap.eraseColor(color); } -void SkDevice::onAccessBitmap(SkBitmap* bitmap) {} +const SkBitmap& SkDevice::onAccessBitmap(SkBitmap* bitmap) {return *bitmap;} void SkDevice::setMatrixClip(const SkMatrix& matrix, const SkRegion& region, const SkClipStack& clipStack) { diff --git a/src/utils/SkDeferredCanvas.cpp b/src/utils/SkDeferredCanvas.cpp index 01ee73f..6988517 100644 --- a/src/utils/SkDeferredCanvas.cpp +++ b/src/utils/SkDeferredCanvas.cpp @@ -471,10 +471,10 @@ SkDeferredCanvas::DeferredDevice::DeferredDevice( SkSafeRef(fDeviceContext); fImmediateDevice = immediateDevice; // ref counted via fImmediateCanvas fImmediateCanvas = SkNEW_ARGS(SkCanvas, (fImmediateDevice)); - SkSafeRef(fImmediateCanvas); fRecordingCanvas = fPicture.beginRecording(fImmediateDevice->width(), fImmediateDevice->height(), SkPicture::kUsePathBoundsForClip_RecordingFlag); + fBitmapInitialized = false; } SkDeferredCanvas::DeferredDevice::~DeferredDevice() @@ -593,10 +593,10 @@ void SkDeferredCanvas::DeferredDevice::writePixels(const SkBitmap& bitmap, flushIfNeeded(bitmap); } -void SkDeferredCanvas::DeferredDevice::onAccessBitmap(SkBitmap* bitmap) +const SkBitmap& SkDeferredCanvas::DeferredDevice::onAccessBitmap(SkBitmap*) { - SkASSERT(bitmap); flushPending(); + return fImmediateDevice->accessBitmap(false); } SkDevice* SkDeferredCanvas::DeferredDevice::onCreateCompatibleDevice( diff --git a/tests/DeferredCanvasTest.cpp b/tests/DeferredCanvasTest.cpp new file mode 100644 index 0000000..14ae1f4 --- /dev/null +++ b/tests/DeferredCanvasTest.cpp @@ -0,0 +1,59 @@ + +/* + * Copyright 2012 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ +#include "Test.h" +#include "SkBitmap.h" +#include "SkDeferredCanvas.h" + + +static const int gWidth = 2; +static const int gHeight = 2; + +static void create(SkBitmap* bm, SkBitmap::Config config, SkColor color) { + bm->setConfig(config, gWidth, gHeight); + bm->allocPixels(); + bm->eraseColor(color); +} + +static void TestDeferredCanvasBitmapAccess(skiatest::Reporter* reporter) { + SkBitmap store; + + create(&store, SkBitmap::kARGB_8888_Config, 0xFFFFFFFF); + SkDevice device(store); + SkDeferredCanvas canvas(&device); + + canvas.clear(0x00000000); + + SkAutoLockPixels alp(store); + REPORTER_ASSERT(reporter, store.getColor(0,0) == 0xFFFFFFFF); //verify that clear was deferred + SkBitmap accessed = canvas.getDevice()->accessBitmap(false); + REPORTER_ASSERT(reporter, store.getColor(0,0) == 0x00000000); //verify that clear was executed + REPORTER_ASSERT(reporter, accessed.pixelRef() == store.pixelRef()); +} + +static void TestDeferredCanvasFlush(skiatest::Reporter* reporter) { + SkBitmap store; + + create(&store, SkBitmap::kARGB_8888_Config, 0xFFFFFFFF); + SkDevice device(store); + SkDeferredCanvas canvas(&device); + + canvas.clear(0x00000000); + + SkAutoLockPixels alp(store); + REPORTER_ASSERT(reporter, store.getColor(0,0) == 0xFFFFFFFF); //verify that clear was deferred + canvas.flush(); + REPORTER_ASSERT(reporter, store.getColor(0,0) == 0x00000000); //verify that clear was executed +} + +static void TestDeferredCanvas(skiatest::Reporter* reporter) { + TestDeferredCanvasBitmapAccess(reporter); + TestDeferredCanvasFlush(reporter); +} + +#include "TestClassDef.h" +DEFINE_TESTCLASS("DeferredCanvas", TestDeferredCanvasClass, TestDeferredCanvas) -- 2.7.4