Revert of Refactor SkPictureStateTree::Iterator to avoid use of kClip_SaveFlag. (https://codereview.chromium.org/246893005/)

Reason for revert:
https://code.google.com/p/chromium/issues/detail?id=366889

Original issue's description:
> Refactor SkPictureStateTree::Iterator to avoid use of kClip_SaveFlag.
>
> The current implementation relies on soon-to-be-deprecated
> kClip_SaveFlag behavior. Updated to use default save flags
> (kMatrixClip_SaveFlag) and stop assuming that the matrix survives
> restore() calls.
>
> R=junov@chromium.org,robertphillips@chromium.org,reed@google.com
>
> Committed: http://code.google.com/p/skia/source/detail?r=14319

R=junov@chromium.org, reed@google.com, robertphillips@chromium.org, robertphillips@google.com, fmalita@chromium.org
TBR=fmalita@chromium.org, junov@chromium.org, reed@google.com, robertphillips@chromium.org, robertphillips@google.com
NOTREECHECKS=true
NOTRY=true

Author: bsalomon@google.com

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

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

3 files changed, 32 insertions, 46 deletions

diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index 7d4c843083..bd5244d223 100644
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -851,7 +851,7 @@ void SkPicturePlayback::draw(SkCanvas& canvas, SkDrawPictureCallback* callback)
         fStateTree->getIterator(*activeOps, &canvas);
 
     if (it.isValid()) {
-        uint32_t skipTo = it.nextDraw();
+        uint32_t skipTo = it.draw();
         if (kDrawComplete == skipTo) {
             return;
         }
@@ -907,7 +907,7 @@ void SkPicturePlayback::draw(SkCanvas& canvas, SkDrawPictureCallback* callback)
                 // iterator until at or after skipTo
                 uint32_t adjustedSkipTo;
                 do {
-                    adjustedSkipTo = it.nextDraw();
+                    adjustedSkipTo = it.draw();
                 } while (adjustedSkipTo < skipTo);
                 skipTo = adjustedSkipTo;
             }
@@ -1246,7 +1246,7 @@ void SkPicturePlayback::draw(SkCanvas& canvas, SkDrawPictureCallback* callback)
 #endif
 
         if (it.isValid()) {
-            uint32_t skipTo = it.nextDraw();
+            uint32_t skipTo = it.draw();
             if (kDrawComplete == skipTo) {
                 break;
             }
diff --git a/src/core/SkPictureStateTree.cpp b/src/core/SkPictureStateTree.cpp
index 20a826aa92..891d04ca21 100644
--- a/src/core/SkPictureStateTree.cpp
+++ b/src/core/SkPictureStateTree.cpp
@@ -99,39 +99,24 @@ SkPictureStateTree::Iterator::Iterator(const SkTDArray<void*>& draws, SkCanvas*
     , fValid(true) {
 }
 
-void SkPictureStateTree::Iterator::setCurrentMatrix(const SkMatrix* matrix) {
-    SkASSERT(NULL != matrix);
-
-    if (matrix == fCurrentMatrix) {
-        return;
-    }
-
-    // The matrix is in recording space, but we also inherit
-    // a playback matrix from out target canvas.
-    SkMatrix m = *matrix;
-    m.postConcat(fPlaybackMatrix);
-    fCanvas->setMatrix(m);
-    fCurrentMatrix = matrix;
-}
-
-uint32_t SkPictureStateTree::Iterator::nextDraw() {
+uint32_t SkPictureStateTree::Iterator::draw() {
     SkASSERT(this->isValid());
     if (fPlaybackIndex >= fDraws->count()) {
+        // restore back to where we started
+        fCanvas->setMatrix(fPlaybackMatrix);
         if (fCurrentNode->fFlags & Node::kSaveLayer_Flag) {
             fCanvas->restore();
         }
-
-        for (fCurrentNode = fCurrentNode->fParent; fCurrentNode;
-             fCurrentNode = fCurrentNode->fParent) {
-            if (fCurrentNode->fFlags & (Node::kSave_Flag | Node::kSaveLayer_Flag)) {
+        fCurrentNode = fCurrentNode->fParent;
+        while (NULL != fCurrentNode) {
+            if (fCurrentNode->fFlags & Node::kSave_Flag) {
+                fCanvas->restore();
+            }
+            if (fCurrentNode->fFlags & Node::kSaveLayer_Flag) {
                 fCanvas->restore();
             }
+            fCurrentNode = fCurrentNode->fParent;
         }
-
-        // restore back to where we started
-        fCanvas->setMatrix(fPlaybackMatrix);
-        fCurrentMatrix = NULL;
-
         return kDrawComplete;
     }
 
@@ -160,13 +145,9 @@ uint32_t SkPictureStateTree::Iterator::nextDraw() {
                 if (currentLevel >= targetLevel) {
                     if (tmp != fCurrentNode && tmp->fFlags & Node::kSave_Flag) {
                         fCanvas->restore();
-                        // restore() may change the matrix, so we need to reapply.
-                        fCurrentMatrix = NULL;
                     }
                     if (tmp->fFlags & Node::kSaveLayer_Flag) {
                         fCanvas->restore();
-                        // restore() may change the matrix, so we need to reapply.
-                        fCurrentMatrix = NULL;
                     }
                     tmp = tmp->fParent;
                 }
@@ -174,19 +155,17 @@ uint32_t SkPictureStateTree::Iterator::nextDraw() {
                     fNodes.push(ancestor);
                     ancestor = ancestor->fParent;
                 }
-
-                SkASSERT(NULL != tmp);
-                SkASSERT(NULL != ancestor);
             }
 
             if (ancestor->fFlags & Node::kSave_Flag) {
                 if (fCurrentNode != ancestor) {
                     fCanvas->restore();
-                    // restore() may change the matrix, so we need to reapply.
-                    fCurrentMatrix = NULL;
                 }
                 if (targetNode != ancestor) {
-                    fCanvas->save();
+                    // FIXME: the save below depends on soon-to-be-deprecated
+                    // SaveFlags behavior: it relies on matrix changes persisting
+                    // after restore.
+                    fCanvas->save(SkCanvas::kClip_SaveFlag);
                 }
             }
             fCurrentNode = ancestor;
@@ -195,18 +174,29 @@ uint32_t SkPictureStateTree::Iterator::nextDraw() {
         // If we're not at the target node yet, we'll need to return an offset to make the caller
         // apply the next clip or saveLayer.
         if (fCurrentNode != targetNode) {
+            if (fCurrentMatrix != fNodes.top()->fMatrix) {
+                fCurrentMatrix = fNodes.top()->fMatrix;
+                SkMatrix tmp = *fNodes.top()->fMatrix;
+                tmp.postConcat(fPlaybackMatrix);
+                fCanvas->setMatrix(tmp);
+            }
             uint32_t offset = fNodes.top()->fOffset;
             fCurrentNode = fNodes.top();
             fSave = fCurrentNode != targetNode && fCurrentNode->fFlags & Node::kSave_Flag;
             fNodes.pop();
-            this->setCurrentMatrix(fCurrentNode->fMatrix);
             return offset;
         }
     }
 
     // If we got this far, the clip/saveLayer state is all set, so we can proceed to set the matrix
     // for the draw, and return its offset.
-    this->setCurrentMatrix(draw->fMatrix);
+
+    if (fCurrentMatrix != draw->fMatrix) {
+        SkMatrix tmp = *draw->fMatrix;
+        tmp.postConcat(fPlaybackMatrix);
+        fCanvas->setMatrix(tmp);
+        fCurrentMatrix = draw->fMatrix;
+    }
 
     ++fPlaybackIndex;
     return draw->fOffset;
diff --git a/src/core/SkPictureStateTree.h b/src/core/SkPictureStateTree.h
index d61bf032cb..448c0d31ce 100644
--- a/src/core/SkPictureStateTree.h
+++ b/src/core/SkPictureStateTree.h
@@ -75,16 +75,12 @@ public:
     class Iterator {
     public:
         /** Returns the next offset into the picture stream, or kDrawComplete if complete. */
-        uint32_t nextDraw();
+        uint32_t draw();
         static const uint32_t kDrawComplete = SK_MaxU32;
         Iterator() : fPlaybackMatrix(), fValid(false) { }
         bool isValid() const { return fValid; }
-
     private:
         Iterator(const SkTDArray<void*>& draws, SkCanvas* canvas, Node* root);
-
-        void setCurrentMatrix(const SkMatrix*);
-
         // The draws this iterator is associated with
         const SkTDArray<void*>* fDraws;
 
@@ -101,7 +97,7 @@ public:
         const SkMatrix fPlaybackMatrix;
 
         // Cache of current matrix, so we can avoid redundantly setting it
-        const SkMatrix* fCurrentMatrix;
+        SkMatrix* fCurrentMatrix;
 
         // current position in the array of draws
         int fPlaybackIndex;