Adding bracketing save/restore calls to SkPicture at record time and

preparing tests for enforcing save/restore balancing constraints on SkPicture

Review URL: http://codereview.appspot.com/6354105/



git-svn-id: http://skia.googlecode.com/svn/trunk@4618 2bbb7eff-a529-9590-31e7-b0007b416f81

4 files changed, 30 insertions, 11 deletions

diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index c40275d5f8..a65f454ef2 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -2021,9 +2021,7 @@ void SkCanvas::drawTextOnPathHV(const void* text, size_t byteLength,
 ///////////////////////////////////////////////////////////////////////////////
 
 void SkCanvas::drawPicture(SkPicture& picture) {
-    int saveCount = save();
     picture.draw(this);
-    restoreToCount(saveCount);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index 98bfb16af5..b9afe53b6e 100644
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -547,7 +547,7 @@ void SkPicturePlayback::draw(SkCanvas& canvas) {
                 bool doAA = ClipParams_unpackDoAA(packed);
                 size_t offsetToRestore = getInt();
                 SkASSERT(!offsetToRestore || \
-                    offsetToRestore > fReader.offset());
+                    offsetToRestore >= fReader.offset());
                 if (!canvas.clipPath(path, op, doAA) && offsetToRestore) {
 #ifdef SPEW_CLIP_SKIPPING
                     skipPath.recordSkip(offsetToRestore - fReader.offset());
@@ -561,7 +561,7 @@ void SkPicturePlayback::draw(SkCanvas& canvas) {
                 SkRegion::Op op = ClipParams_unpackRegionOp(packed);
                 size_t offsetToRestore = getInt();
                 SkASSERT(!offsetToRestore || \
-                    offsetToRestore > fReader.offset());
+                    offsetToRestore >= fReader.offset());
                 if (!canvas.clipRegion(region, op) && offsetToRestore) {
 #ifdef SPEW_CLIP_SKIPPING
                     skipRegion.recordSkip(offsetToRestore - fReader.offset());
@@ -576,7 +576,7 @@ void SkPicturePlayback::draw(SkCanvas& canvas) {
                 bool doAA = ClipParams_unpackDoAA(packed);
                 size_t offsetToRestore = getInt();
                 SkASSERT(!offsetToRestore || \
-                    offsetToRestore > fReader.offset());
+                    offsetToRestore >= fReader.offset());
                 if (!canvas.clipRect(rect, op, doAA) && offsetToRestore) {
 #ifdef SPEW_CLIP_SKIPPING
                     skipRect.recordSkip(offsetToRestore - fReader.offset());
diff --git a/src/core/SkPictureRecord.cpp b/src/core/SkPictureRecord.cpp
index 630a1bb649..ed528f293c 100644
--- a/src/core/SkPictureRecord.cpp
+++ b/src/core/SkPictureRecord.cpp
@@ -12,6 +12,10 @@
 #define MIN_WRITER_SIZE 16384
 #define HEAP_BLOCK_SIZE 4096
 
+enum {
+    kNoInitialSave = -1,
+};
+
 SkPictureRecord::SkPictureRecord(uint32_t flags) :
         fHeap(HEAP_BLOCK_SIZE),
         fBitmaps(&fHeap),
@@ -26,7 +30,7 @@ SkPictureRecord::SkPictureRecord(uint32_t flags) :
 #endif
 
     fRestoreOffsetStack.setReserve(32);
-    fRestoreOffsetStack.push(0);
+    fInitialSaveCount = kNoInitialSave;
 
     fPathHeap = NULL;   // lazy allocate
     fFirstSavedLayerIndex = kNoSavedLayerIndex;
@@ -38,6 +42,16 @@ SkPictureRecord::~SkPictureRecord() {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+SkDevice* SkPictureRecord::setDevice(SkDevice* device) {
+    SkASSERT(kNoInitialSave == fInitialSaveCount);
+    this->INHERITED::setDevice(device);
+
+    // The bracketting save() call needs to be recorded after setting the
+    // device otherwise the clip stack will get messed-up
+    fInitialSaveCount = this->save(SkCanvas::kMatrixClip_SaveFlag);
+    return device;
+}
+
 int SkPictureRecord::save(SaveFlags flags) {
     addDraw(SAVE);
     addInt(flags);
@@ -78,6 +92,13 @@ bool SkPictureRecord::isDrawingToLayer() const {
 }
 
 void SkPictureRecord::restore() {
+    // FIXME: SkDeferredCanvas needs to be refactored to respect
+    // save/restore balancing so that the following test can be
+    // turned on permanently.
+#if 0
+    SkASSERT(fRestoreOffsetStack.count() > 1);
+#endif
+
     // check for underflow
     if (fRestoreOffsetStack.count() == 0) {
         return;
@@ -171,11 +192,8 @@ void SkPictureRecord::fillRestoreOffsetPlaceholdersForCurrentStackLevel(
 }
 
 void SkPictureRecord::endRecording() {
-    // clear any remaining unhandled restore offset placeholders
-    while (fRestoreOffsetStack.count()) {
-        this->fillRestoreOffsetPlaceholdersForCurrentStackLevel(0);
-        fRestoreOffsetStack.pop();
-    }
+    SkASSERT(kNoInitialSave != fInitialSaveCount);
+    this->restoreToCount(fInitialSaveCount);
 }
 
 void SkPictureRecord::recordRestoreOffsetPlaceholder(SkRegion::Op op) {
diff --git a/src/core/SkPictureRecord.h b/src/core/SkPictureRecord.h
index c970696f18..13b9db55c5 100644
--- a/src/core/SkPictureRecord.h
+++ b/src/core/SkPictureRecord.h
@@ -21,6 +21,8 @@ public:
     SkPictureRecord(uint32_t recordFlags);
     virtual ~SkPictureRecord();
 
+    virtual SkDevice* setDevice(SkDevice* device) SK_OVERRIDE;
+
     virtual int save(SaveFlags) SK_OVERRIDE;
     virtual int saveLayer(const SkRect* bounds, const SkPaint*, SaveFlags) SK_OVERRIDE;
     virtual void restore() SK_OVERRIDE;
@@ -203,6 +205,7 @@ private:
     SkRefCntSet fTFSet;
 
     uint32_t fRecordFlags;
+    int fInitialSaveCount;
 
     friend class SkPicturePlayback;
     friend class SkPictureTester; // for unit testing