restore clipstack to heap-ptr, so clients can ref it

BUG=skia:

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

diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h
index 1dbca3a..853877d 100644 (file)
--- a/include/core/SkCanvas.h
+++ b/include/core/SkCanvas.h
@@ -1089,7 +1089,7 @@ public:
      *  @return the current clip stack ("list" of individual clip elements)
      */
     const SkClipStack* getClipStack() const {
-        return &fClipStack;
+        return fClipStack;
     }
 
     typedef SkCanvasClipVisitor ClipVisitor;
@@ -1251,7 +1251,7 @@ protected:
 private:
     class MCRec;
 
-    SkClipStack fClipStack;
+    SkAutoTUnref<SkClipStack> fClipStack;
     SkDeque     fMCStack;
     // points to top of stack
     MCRec*      fMCRec;

diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 83eee62..ec21955 100644 (file)
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -233,7 +233,7 @@ public:
         fCanvas = canvas;
         canvas->updateDeviceCMCache();
 
-        fClipStack = &canvas->fClipStack;
+        fClipStack = canvas->fClipStack;
         fCurrLayer = canvas->fMCRec->fTopLayer;
         fSkipEmptyClips = skipEmptyClips;
     }
@@ -435,6 +435,8 @@ SkBaseDevice* SkCanvas::init(SkBaseDevice* device, InitFlags flags) {
     fSaveCount = 1;
     fMetaData = NULL;
 
+    fClipStack.reset(SkNEW(SkClipStack));
+
     fMCRec = (MCRec*)fMCStack.push_back();
     new (fMCRec) MCRec(fConservativeRasterClip);
 
@@ -739,11 +741,11 @@ void SkCanvas::updateDeviceCMCache() {
         DeviceCM*       layer = fMCRec->fTopLayer;
 
         if (NULL == layer->fNext) {   // only one layer
-            layer->updateMC(totalMatrix, totalClip, fClipStack, NULL);
+            layer->updateMC(totalMatrix, totalClip, *fClipStack, NULL);
         } else {
             SkRasterClip clip(totalClip);
             do {
-                layer->updateMC(totalMatrix, clip, fClipStack, &clip);
+                layer->updateMC(totalMatrix, clip, *fClipStack, &clip);
             } while ((layer = layer->fNext) != NULL);
         }
         fDeviceCMDirty = false;
@@ -820,7 +822,7 @@ void SkCanvas::internalSave() {
     new (newTop) MCRec(*fMCRec);    // balanced in restore()
     fMCRec = newTop;
 
-    fClipStack.save();
+    fClipStack->save();
 }
 
 static bool bounds_affects_clip(SkCanvas::SaveFlags flags) {
@@ -865,7 +867,7 @@ bool SkCanvas::clipRectBounds(const SkRect* bounds, SaveFlags flags,
     if (bounds_affects_clip(flags)) {
         // Simplify the current clips since they will be applied properly during restore()
         fCachedLocalClipBoundsDirty = true;
-        fClipStack.clipDevRect(ir, SkRegion::kReplace_Op);
+        fClipStack->clipDevRect(ir, SkRegion::kReplace_Op);
         fMCRec->fRasterClip.setRect(ir);
     }
 
@@ -974,7 +976,7 @@ void SkCanvas::internalRestore() {
     fDeviceCMDirty = true;
     fCachedLocalClipBoundsDirty = true;
 
-    fClipStack.restore();
+    fClipStack->restore();
 
     // reserve our layer (if any)
     DeviceCM* layer = fMCRec->fLayer;   // may be null
@@ -1261,7 +1263,7 @@ void SkCanvas::onClipRect(const SkRect& rect, SkRegion::Op op, ClipEdgeStyle edg
             fDeviceCMDirty = true;
             fCachedLocalClipBoundsDirty = true;
 
-            fClipStack.clipEmpty();
+            fClipStack->clipEmpty();
             return fMCRec->fRasterClip.setEmpty();
         }
     }
@@ -1283,7 +1285,7 @@ void SkCanvas::onClipRect(const SkRect& rect, SkRegion::Op op, ClipEdgeStyle edg
         SkRect      r;
 
         fMCRec->fMatrix.mapRect(&r, rect);
-        fClipStack.clipDevRect(r, op, kSoft_ClipEdgeStyle == edgeStyle);
+        fClipStack->clipDevRect(r, op, kSoft_ClipEdgeStyle == edgeStyle);
         fMCRec->fRasterClip.op(r, this->getBaseLayerSize(), op, kSoft_ClipEdgeStyle == edgeStyle);
     } else {
         // since we're rotated or some such thing, we convert the rect to a path
@@ -1323,7 +1325,7 @@ void SkCanvas::onClipRRect(const SkRRect& rrect, SkRegion::Op op, ClipEdgeStyle
             edgeStyle = kHard_ClipEdgeStyle;
         }
 
-        fClipStack.clipDevRRect(transformedRRect, op, kSoft_ClipEdgeStyle == edgeStyle);
+        fClipStack->clipDevRRect(transformedRRect, op, kSoft_ClipEdgeStyle == edgeStyle);
 
         SkPath devPath;
         devPath.addRRect(transformedRRect);
@@ -1360,7 +1362,7 @@ void SkCanvas::onClipPath(const SkPath& path, SkRegion::Op op, ClipEdgeStyle edg
             fDeviceCMDirty = true;
             fCachedLocalClipBoundsDirty = true;
 
-            fClipStack.clipEmpty();
+            fClipStack->clipEmpty();
             return fMCRec->fRasterClip.setEmpty();
         }
     }
@@ -1388,7 +1390,7 @@ void SkCanvas::onClipPath(const SkPath& path, SkRegion::Op op, ClipEdgeStyle edg
     }
 
     // if we called path.swap() we could avoid a deep copy of this path
-    fClipStack.clipDevPath(devPath, op, kSoft_ClipEdgeStyle == edgeStyle);
+    fClipStack->clipDevPath(devPath, op, kSoft_ClipEdgeStyle == edgeStyle);
 
     if (fAllowSimplifyClip) {
         bool clipIsAA = getClipStack()->asPath(&devPath);
@@ -1415,7 +1417,7 @@ void SkCanvas::onClipRegion(const SkRegion& rgn, SkRegion::Op op) {
 
     // todo: signal fClipStack that we have a region, and therefore (I guess)
     // we have to ignore it, and use the region directly?
-    fClipStack.clipDevRect(rgn.getBounds(), op);
+    fClipStack->clipDevRect(rgn.getBounds(), op);
 
     fMCRec->fRasterClip.op(rgn, op);
 }
@@ -1433,7 +1435,7 @@ void SkCanvas::validateClip() const {
     ir.set(0, 0, device->width(), device->height());
     SkRasterClip tmpClip(ir, fConservativeRasterClip);
 
-    SkClipStack::B2TIter                iter(fClipStack);
+    SkClipStack::B2TIter                iter(*fClipStack);
     const SkClipStack::Element* element;
     while ((element = iter.next()) != NULL) {
         switch (element->getType()) {
@@ -1456,7 +1458,7 @@ void SkCanvas::validateClip() const {
 #endif
 
 void SkCanvas::replayClips(ClipVisitor* visitor) const {
-    SkClipStack::B2TIter                iter(fClipStack);
+    SkClipStack::B2TIter                iter(*fClipStack);
     const SkClipStack::Element*         element;
 
     while ((element = iter.next()) != NULL) {

diff --git a/tests/CanvasTest.cpp b/tests/CanvasTest.cpp
index d709a93..646d515 100644 (file)
--- a/tests/CanvasTest.cpp
+++ b/tests/CanvasTest.cpp
@@ -45,6 +45,7 @@
  */
 #include "SkBitmap.h"
 #include "SkCanvas.h"
+#include "SkClipStack.h"
 #include "SkDeferredCanvas.h"
 #include "SkDevice.h"
 #include "SkDocument.h"
@@ -217,6 +218,18 @@ static void test_clipVisitor(skiatest::Reporter* reporter, SkCanvas* canvas) {
     REPORTER_ASSERT(reporter, equal_clips(c, *canvas));
 }
 
+static void test_clipstack(skiatest::Reporter* reporter) {
+    // The clipstack is refcounted, and needs to be able to out-live the canvas if a client has
+    // ref'd it.
+    const SkClipStack* cs = NULL;
+    {
+        SkCanvas canvas(10, 10);
+        cs = SkRef(canvas.getClipStack());
+    }
+    REPORTER_ASSERT(reporter, cs->unique());
+    cs->unref();
+}
+
 // Format strings that describe the test context.  The %s token is where
 // the name of the test step is inserted.  The context is required for
 // disambiguating the error in the case of failures that are reported in
@@ -671,6 +684,7 @@ static void TestOverrideStateConsistency(skiatest::Reporter* reporter, const Tes
     if (false) { // avoid bit rot, suppress warning
         test_clipVisitor(reporter, &referenceCanvas);
     }
+    test_clipstack(reporter);
 }
 
 static void test_newraster(skiatest::Reporter* reporter) {