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

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) {
+bool SkMatrixClipStateMgr::isNestingMCState(int stateID) {
-    if (fCurMCState->fIsSaveLayer)
+    return fStateIDStack.count() > 0 && fStateIDStack[fStateIDStack.count()-1] == fCurOpenStateID;
-        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::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 &&
+    if (fCurOpenStateID == fCurMCState->fMCStateID && !this->isNestingMCState(fCurOpenStateID)) {
-            (!fCurMCState->fIsSaveLayer ||
-             fCurOpenStateID != fCurMCState->fSaveLayerBaseStateID)) {
         // 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);

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.
+        // The next field is only valid when fIsSaveLayer is set.
-        int32_t      fSaveLayerBaseStateID;
         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