I believe this makes it clearer what is going on; namely:

saveLayers cause their enclosing MC state to become a prefix for their child canvas calls. In such cases we don't want to inadvertently close the nesting MC state but when we do (i.e., when the saveLayer's restore is seen) we want to also restore the nesting MC state to be the current one.

R=bsalomon@google.com

Author: robertphillips@google.com

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

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

2 files changed, 18 insertions, 29 deletions

diff --git a/src/core/SkMatrixClipStateMgr.cpp b/src/core/SkMatrixClipStateMgr.cpp
index be815fd5a5..5f2103054f 100644
--- a/src/core/SkMatrixClipStateMgr.cpp
+++ b/src/core/SkMatrixClipStateMgr.cpp
@@ -28,7 +28,6 @@ bool SkMatrixClipStateMgr::MatrixClipState::ClipInfo::clipRegion(SkPictureRecord
                                                                  int regionID,
                                                                  SkRegion::Op op,
                                                                  int matrixID) {
-    // TODO: add a region dictionary so we don't have to copy the region in here
     ClipOp* newClip = fClips.append();
     newClip->fClipType = kRegion_ClipType;
     newClip->fGeom.fRegionID = regionID;
@@ -166,7 +165,7 @@ int SkMatrixClipStateMgr::saveLayer(const SkRect* bounds, const SkPaint* paint,
     fCurMCState->fExpectedDepth++;   // 1 for saveLayer
 #endif
 
-    fCurMCState->fSaveLayerBaseStateID = fCurOpenStateID;
+    *fStateIDStack.append() = fCurOpenStateID;
     fCurMCState->fSavedSkipOffsets = fSkipOffsets;
 
     // TODO: recycle these rather then new & deleting them on every saveLayer/
@@ -205,7 +204,9 @@ void SkMatrixClipStateMgr::restore() {
         fActualDepth--;
 #endif
 
-        fCurOpenStateID = fCurMCState->fSaveLayerBaseStateID;
+        SkASSERT(fStateIDStack.count() >= 1);
+        fCurOpenStateID = fStateIDStack[fStateIDStack.count()-1];
+        fStateIDStack.pop();
 
         SkASSERT(0 == fSkipOffsets->count());
         SkASSERT(NULL != fCurMCState->fSavedSkipOffsets);
@@ -243,23 +244,8 @@ int32_t SkMatrixClipStateMgr::NewMCStateID() {
     return gMCStateID;
 }
 
-bool SkMatrixClipStateMgr::isCurrentlyOpen(int32_t stateID) {
-    if (fCurMCState->fIsSaveLayer)
-        return false;
-
-    SkDeque::Iter iter(fMatrixClipStack, SkDeque::Iter::kBack_IterStart);
-
-    for (const MatrixClipState* state = (const MatrixClipState*) iter.prev();
-         state != NULL;
-         state = (const MatrixClipState*) iter.prev()) {
-        if (state->fIsSaveLayer) {
-            if (state->fSaveLayerBaseStateID == stateID) {
-                return true;
-            }
-        }
-    }
-
-    return false;
+bool SkMatrixClipStateMgr::isNestingMCState(int stateID) {
+    return fStateIDStack.count() > 0 && fStateIDStack[fStateIDStack.count()-1] == fCurOpenStateID;
 }
 
 bool SkMatrixClipStateMgr::call(CallType callType) {
@@ -280,7 +266,7 @@ bool SkMatrixClipStateMgr::call(CallType callType) {
     }
 
     if (kIdentityWideOpenStateID != fCurOpenStateID &&
-        !this->isCurrentlyOpen(fCurOpenStateID)) {
+        !this->isNestingMCState(fCurOpenStateID)) {
         // Don't write a restore if the open state is one in which a saveLayer
         // is nested. The save after the saveLayer's restore will close it.
         fPicRecord->recordRestore();    // Close the open block
@@ -397,9 +383,7 @@ void SkMatrixClipStateMgr::finish() {
 
 #ifdef SK_DEBUG
 void SkMatrixClipStateMgr::validate() {
-    if (fCurOpenStateID == fCurMCState->fMCStateID &&
-            (!fCurMCState->fIsSaveLayer ||
-             fCurOpenStateID != fCurMCState->fSaveLayerBaseStateID)) {
+    if (fCurOpenStateID == fCurMCState->fMCStateID && !this->isNestingMCState(fCurOpenStateID)) {
         // The current state is the active one so it should have a skip
         // offset for each clip
         SkDeque::Iter iter(fMatrixClipStack, SkDeque::Iter::kBack_IterStart);
diff --git a/src/core/SkMatrixClipStateMgr.h b/src/core/SkMatrixClipStateMgr.h
index bb8b25e877..c4201f86ed 100644
--- a/src/core/SkMatrixClipStateMgr.h
+++ b/src/core/SkMatrixClipStateMgr.h
@@ -53,7 +53,7 @@ public:
     static const int32_t kIdentityWideOpenStateID = 0;
     static const int kIdentityMatID = 0;
 
-    class MatrixClipState {
+    class MatrixClipState : public SkNoncopyable {
     public:
         class MatrixInfo {
         public:
@@ -104,6 +104,8 @@ public:
         private:
             SkMatrix fMatrix;
             int      fMatrixID;
+
+            typedef SkNoncopyable INHERITED;
         };
 
         class ClipInfo : public SkNoncopyable {
@@ -162,7 +164,7 @@ public:
                 ClipType     fClipType;
 
                 union {
-                    SkRRect fRRect;        // also stores clipRect
+                    SkRRect fRRect;        // also stores clip rect
                     int     fPathID;
                     int     fRegionID;
                 } fGeom;
@@ -236,8 +238,7 @@ public:
         // Does this MC state represent a saveLayer call?
         bool         fIsSaveLayer;
 
-        // The next two fields are only valid when fIsSaveLayer is set.
-        int32_t      fSaveLayerBaseStateID;
+        // The next field is only valid when fIsSaveLayer is set.
         SkTDArray<int>* fSavedSkipOffsets;
 
         // Does the MC state have an open block in the skp?
@@ -399,11 +400,15 @@ protected:
         return fMatrixDict[index];
     }
 
-    bool isCurrentlyOpen(int32_t stateID);
+    bool isNestingMCState(int stateID);
 
 #ifdef SK_DEBUG
     int fActualDepth;
 #endif
+
+    // save layers are nested within a specific MC state. This stack tracks
+    // the nesting MC state's ID as save layers are pushed and popped.
+    SkTDArray<int> fStateIDStack;
 };
 
 #endif