SkCanvas::resetForNextPicture()

No diffs against head for DM --config 8888 gpu 2ndpic-8888 2ndpic-gpu.

  picture_overhead_draw	1.62us ->  1.6us	0.99x
picture_overhead_nodraw	 792ns ->  342ns	0.43x

tiles and serialization modes will also test this a bit.

BUG=chromium:470553

Committed: https://skia.googlesource.com/skia/+/f920e468ac66a36c9653d1b11181480295044c7d

Review URL: https://codereview.chromium.org/1067893002

diff --git a/include/core/SkBitmapDevice.h b/include/core/SkBitmapDevice.h
index 9d8c4c5..69adc9a 100644 (file)
--- a/include/core/SkBitmapDevice.h
+++ b/include/core/SkBitmapDevice.h
@@ -150,6 +150,8 @@ private:
 
     SkBitmap    fBitmap;
 
+    void setNewSize(const SkISize&);  // Used by SkCanvas for resetForNextPicture().
+
     typedef SkBaseDevice INHERITED;
 };
 

diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h
index 853877d..828b923 100644 (file)
--- a/include/core/SkCanvas.h
+++ b/include/core/SkCanvas.h
@@ -1301,6 +1301,8 @@ private:
     SkCanvas(const SkIRect& bounds, InitFlags);
     SkCanvas(SkBaseDevice*, const SkSurfaceProps*, InitFlags);
 
+    void resetForNextPicture(const SkIRect& bounds);
+
     // needs gettotalclip()
     friend class SkCanvasStateUtils;
 

diff --git a/include/core/SkPictureRecorder.h b/include/core/SkPictureRecorder.h
index b25e26b..0459737 100644 (file)
--- a/include/core/SkPictureRecorder.h
+++ b/include/core/SkPictureRecorder.h
@@ -96,6 +96,7 @@ private:
     friend class SkPictureRecorderReplayTester; // for unit testing
     void partialReplay(SkCanvas* canvas) const;
 
+    bool                          fActivelyRecording;
     uint32_t                      fFlags;
     SkRect                        fCullRect;
     SkAutoTUnref<SkBBoxHierarchy> fBBH;

diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp
index 5fae2b5..5b2fc38 100644 (file)
--- a/src/core/SkBitmapDevice.cpp
+++ b/src/core/SkBitmapDevice.cpp
@@ -103,6 +103,11 @@ SkImageInfo SkBitmapDevice::imageInfo() const {
     return fBitmap.info();
 }
 
+void SkBitmapDevice::setNewSize(const SkISize& size) {
+    SkASSERT(!fBitmap.pixelRef());
+    fBitmap.setInfo(fBitmap.info().makeWH(size.fWidth, size.fHeight));
+}
+
 void SkBitmapDevice::replaceBitmapBackendForRasterSurface(const SkBitmap& bm) {
     SkASSERT(bm.width() == fBitmap.width());
     SkASSERT(bm.height() == fBitmap.height());

diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index ec21955..63a9241 100644 (file)
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -135,6 +135,13 @@ struct DeviceCM {
         SkDELETE(fPaint);
     }
 
+    void reset(const SkIRect& bounds) {
+        SkASSERT(!fPaint);
+        SkASSERT(!fNext);
+        SkASSERT(fDevice);
+        fClip.setRect(bounds);
+    }
+
     void updateMC(const SkMatrix& totalMatrix, const SkRasterClip& totalClip,
                   const SkClipStack& clipStack, SkRasterClip* updateClip) {
         int x = fDevice->getOrigin().x();
@@ -224,6 +231,15 @@ public:
         SkDELETE(fLayer);
         dec_rec();
     }
+
+    void reset(const SkIRect& bounds) {
+        SkASSERT(fLayer);
+        SkASSERT(fDeferredSaveCount == 0);
+
+        fMatrix.reset();
+        fRasterClip.setRect(bounds);
+        fLayer->reset(bounds);
+    }
 };
 
 class SkDrawIter : public SkDraw {
@@ -425,6 +441,18 @@ bool AutoDrawLooper::doNext(SkDrawFilter::Type drawType) {
 
 ////////////////////////////////////////////////////////////////////////////
 
+void SkCanvas::resetForNextPicture(const SkIRect& bounds) {
+    this->restoreToCount(1);
+    fCachedLocalClipBounds.setEmpty();
+    fCachedLocalClipBoundsDirty = true;
+    fClipStack->reset();
+    fMCRec->reset(bounds);
+
+    // We're peering through a lot of structs here.  Only at this scope do we
+    // know that the device is an SkBitmapDevice (really an SkNoPixelsBitmapDevice).
+    static_cast<SkBitmapDevice*>(fMCRec->fLayer->fDevice)->setNewSize(bounds.size());
+}
+
 SkBaseDevice* SkCanvas::init(SkBaseDevice* device, InitFlags flags) {
     fConservativeRasterClip = SkToBool(flags & kConservativeRasterClip_InitFlag);
     fCachedLocalClipBounds.setEmpty();

diff --git a/src/core/SkPictureRecorder.cpp b/src/core/SkPictureRecorder.cpp
index 850be27..f622e66 100644 (file)
--- a/src/core/SkPictureRecorder.cpp
+++ b/src/core/SkPictureRecorder.cpp
@@ -15,7 +15,10 @@
 #include "SkRecordOpts.h"
 #include "SkTypes.h"
 
-SkPictureRecorder::SkPictureRecorder() {}
+SkPictureRecorder::SkPictureRecorder() {
+    fActivelyRecording = false;
+    fRecorder.reset(SkNEW_ARGS(SkRecorder, (nullptr, SkRect::MakeWH(0,0))));
+}
 
 SkPictureRecorder::~SkPictureRecorder() {}
 
@@ -31,15 +34,18 @@ SkCanvas* SkPictureRecorder::beginRecording(const SkRect& cullRect,
     }
 
     fRecord.reset(SkNEW(SkRecord));
-    fRecorder.reset(SkNEW_ARGS(SkRecorder, (fRecord.get(), cullRect)));
+    fRecorder->reset(fRecord.get(), cullRect);
+    fActivelyRecording = true;
     return this->getRecordingCanvas();
 }
 
 SkCanvas* SkPictureRecorder::getRecordingCanvas() {
-    return fRecorder.get();
+    return fActivelyRecording ? fRecorder.get() : nullptr;
 }
 
 SkPicture* SkPictureRecorder::endRecordingAsPicture() {
+    fActivelyRecording = false;
+    fRecorder->restoreToCount(1);  // If we were missing any restores, add them now.
     // TODO: delay as much of this work until just before first playback?
     SkRecordOptimize(fRecord);
 
@@ -73,7 +79,6 @@ SkPicture* SkPictureRecorder::endRecordingAsPicture() {
     }
 
     // release our refs now, so only the picture will be the owner.
-    fRecorder.reset(NULL);
     fRecord.reset(NULL);
     fBBH.reset(NULL);
 
@@ -158,6 +163,8 @@ protected:
 };
 
 SkDrawable* SkPictureRecorder::endRecordingAsDrawable() {
+    fActivelyRecording = false;
+    fRecorder->restoreToCount(1);  // If we were missing any restores, add them now.
     // TODO: delay as much of this work until just before first playback?
     SkRecordOptimize(fRecord);
 
@@ -171,7 +178,6 @@ SkDrawable* SkPictureRecorder::endRecordingAsDrawable() {
                                          SkToBool(fFlags & kComputeSaveLayerInfo_RecordFlag)));
 
     // release our refs now, so only the drawable will be the owner.
-    fRecorder.reset(NULL);
     fRecord.reset(NULL);
     fBBH.reset(NULL);
 

diff --git a/src/core/SkRecorder.cpp b/src/core/SkRecorder.cpp
index aafb540..8684a8e 100644 (file)
--- a/src/core/SkRecorder.cpp
+++ b/src/core/SkRecorder.cpp
@@ -39,6 +39,12 @@ SkRecorder::SkRecorder(SkRecord* record, const SkRect& bounds)
     : SkCanvas(bounds.roundOut(), SkCanvas::kConservativeRasterClip_InitFlag)
     , fRecord(record) {}
 
+void SkRecorder::reset(SkRecord* record, const SkRect& bounds) {
+    this->forgetRecord();
+    fRecord = record;
+    this->resetForNextPicture(bounds.roundOut());
+}
+
 void SkRecorder::forgetRecord() {
     fDrawableList.reset(NULL);
     fRecord = NULL;

diff --git a/src/core/SkRecorder.h b/src/core/SkRecorder.h
index 834a2b6..6842aec 100644 (file)
--- a/src/core/SkRecorder.h
+++ b/src/core/SkRecorder.h
@@ -39,6 +39,8 @@ public:
     SkRecorder(SkRecord*, int width, int height);   // legacy version
     SkRecorder(SkRecord*, const SkRect& bounds);
 
+    void reset(SkRecord*, const SkRect& bounds);
+
     SkDrawableList* getDrawableList() const { return fDrawableList.get(); }
     SkDrawableList* detachDrawableList() { return fDrawableList.detach(); }
 

diff --git a/tests/PictureTest.cpp b/tests/PictureTest.cpp
index 14e703f..153695b 100644 (file)
--- a/tests/PictureTest.cpp
+++ b/tests/PictureTest.cpp
@@ -1311,3 +1311,15 @@ DEF_TEST(Picture_BitmapLeak, r) {
     REPORTER_ASSERT(r, mut.pixelRef()->unique());
     REPORTER_ASSERT(r, immut.pixelRef()->unique());
 }
+
+// getRecordingCanvas() should return a SkCanvas when recording, null when not recording.
+DEF_TEST(Picture_getRecordingCanvas, r) {
+    SkPictureRecorder rec;
+    REPORTER_ASSERT(r, !rec.getRecordingCanvas());
+    for (int i = 0; i < 3; i++) {
+        rec.beginRecording(100, 100);
+        REPORTER_ASSERT(r, rec.getRecordingCanvas());
+        rec.endRecording()->unref();
+        REPORTER_ASSERT(r, !rec.getRecordingCanvas());
+    }
+}