Refactor SkPictureStateTree::Iterator to avoid use of kClip_SaveFlag.

src/core/SkPictureStateTree.cpp

 
 /*
  * 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 "SkPictureStateTree.h"
 #include "SkCanvas.h"
@@ skipping 81 matching lines @@
     : fDraws(&draws)
     , fCanvas(canvas)
     , fCurrentNode(root)
     , fPlaybackMatrix(canvas->getTotalMatrix())
     , fCurrentMatrix(NULL)
     , fPlaybackIndex(0)
     , fSave(false)
     , fValid(true) {
 }
 
-uint32_t SkPictureStateTree::Iterator::draw() {
+void SkPictureStateTree::Iterator::setCurrentMatrix(const SkMatrix* matrix) {
+    SkASSERT(NULL != matrix);
+
+    if (matrix == fCurrentMatrix) {
+        return;
+    }
+
+    SkMatrix m = *matrix;
+    m.postConcat(fPlaybackMatrix);

[Justin Novosad, 2014/04/22 15:31:34]

If I understand correctly, this is because 'matrix' tranforms from local to
recording canvas space, and what we want is local to playback canvas space.
Correct? A little comment WBN.

[f(malita), 2014/04/22 15:48:38]

Exactly. Added comment.

+    fCanvas->setMatrix(m);
+    fCurrentMatrix = matrix;
+}
+
+uint32_t SkPictureStateTree::Iterator::nextDraw() {

[Justin Novosad, 2014/04/22 15:31:34]

+1 for the name change.

     SkASSERT(this->isValid());
     if (fPlaybackIndex >= fDraws->count()) {
-        // restore back to where we started
-        fCanvas->setMatrix(fPlaybackMatrix);
         if (fCurrentNode->fFlags & Node::kSaveLayer_Flag) {
             fCanvas->restore();
         }
-        fCurrentNode = fCurrentNode->fParent;
-        while (NULL != fCurrentNode) {

[Justin Novosad, 2014/04/22 15:31:34]

The changes to this loop look mostly like a cleaner re-write. Is there anything
more going on here that I did not catch?

[f(malita), 2014/04/22 15:48:38]

Yes, the loop change itself is just cleanup.

The only significant change in this block is the deferral of
setMatrix(fPlaybackMatrix) until after all restore()s.

-            if (fCurrentNode->fFlags & Node::kSave_Flag) {
+
+        for (fCurrentNode = fCurrentNode->fParent; fCurrentNode;
+             fCurrentNode = fCurrentNode->fParent) {
+            if (fCurrentNode->fFlags & (Node::kSave_Flag | Node::kSaveLayer_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;
     }
 
     Draw* draw = static_cast<Draw*>((*fDraws)[fPlaybackIndex]);
     Node* targetNode = draw->fNode;
 
     if (fSave) {
         fCanvas->save();
         fSave = false;
     }
 
     if (fCurrentNode != targetNode) {
         // If we're not at the target and we don't have a list of nodes to get there, we need to
         // figure out the path from our current node, to the target
         if (fNodes.count() == 0) {
             // Trace back up to a common ancestor, restoring to get our current state to match that
             // of the ancestor, and saving a list of nodes whose state we need to apply to get to
             // the target (we can restore up to the ancestor immediately, but we'll need to return
             // an offset for each node on the way down to the target, to apply the desired clips and
             // saveLayers, so it may take several draw() calls before the next draw actually occurs)
             Node* tmp = fCurrentNode;
             Node* ancestor = targetNode;
             while (tmp != ancestor) {
                 uint16_t currentLevel = tmp->fLevel;
                 uint16_t targetLevel = ancestor->fLevel;
                 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;
                 }
                 if (currentLevel <= targetLevel) {
                     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) {
-                    // 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);
+                    fCanvas->save();
                 }
             }
             fCurrentNode = ancestor;
         }
 
         // 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.
-
-    if (fCurrentMatrix != draw->fMatrix) {
-        SkMatrix tmp = *draw->fMatrix;
-        tmp.postConcat(fPlaybackMatrix);
-        fCanvas->setMatrix(tmp);
-        fCurrentMatrix = draw->fMatrix;
-    }
+    this->setCurrentMatrix(draw->fMatrix);
 
     ++fPlaybackIndex;
     return draw->fOffset;
 }