Defer saves() until they're needed

src/core/SkCanvas.cpp

Index: src/core/SkCanvas.cpp
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 083a8ed7e8a8cea5ccb505b1d6a3537a8ed95fd7..69e078517539eb31c85293b1e2821c8ebe2b01ab 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -170,8 +170,6 @@ private:
 */
 class SkCanvas::MCRec {
 public:
-    SkRasterClip    fRasterClip;
-    SkMatrix        fMatrix;
     SkDrawFilter*   fFilter;    // the current filter (or null)
     DeviceCM*       fLayer;
     /*  If there are any layers in the stack, this points to the top-most
@@ -180,22 +178,26 @@ public:
         reference counted, since the real owner is either our fLayer field,
         or a previous one in a lower level.)
     */
-    DeviceCM*   fTopLayer;
+    DeviceCM*       fTopLayer;
+    SkRasterClip    fRasterClip;
+    SkMatrix        fMatrix;
+    int             fDeferredSaveCount;

[f(malita), 2014/12/10 19:49:33]

I always imagined this to be an unsigned, but yeah, I know - Skia counts are
ints :)

 
     MCRec(bool conservativeRasterClip) : fRasterClip(conservativeRasterClip) {
-        fMatrix.reset();
         fFilter     = NULL;
         fLayer      = NULL;
         fTopLayer   = NULL;
+        fMatrix.reset();
+        fDeferredSaveCount = 0;
 
         // don't bother initializing fNext
         inc_rec();
     }
-    MCRec(const MCRec& prev) : fRasterClip(prev.fRasterClip) {
-        fMatrix = prev.fMatrix;
+    MCRec(const MCRec& prev) : fRasterClip(prev.fRasterClip), fMatrix(prev.fMatrix) {
         fFilter = SkSafeRef(prev.fFilter);
         fLayer = NULL;
         fTopLayer = prev.fTopLayer;
+        fDeferredSaveCount = 0;
 
         // don't bother initializing fNext
         inc_rec();
@@ -423,6 +425,8 @@ SkBaseDevice* SkCanvas::init(SkBaseDevice* device, InitFlags flags) {
     fMCRec->fLayer = SkNEW_ARGS(DeviceCM, (NULL, 0, 0, NULL, NULL, fConservativeRasterClip));
     fMCRec->fTopLayer = fMCRec->fLayer;
 
+    fCallingWillSave = false;
+
     fSurfaceBase = NULL;
 
     if (device) {
@@ -785,21 +789,51 @@ void SkCanvas::updateDeviceCMCache() {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+void SkCanvas::checkForDeferredSave() {
+    if (fMCRec->fDeferredSaveCount > 0) {
+        fMCRec->fDeferredSaveCount -= 1;
+        this->doSave();
+    }
+}
+
 int SkCanvas::getSaveCount() const {

[f(malita), 2014/12/10 19:49:33]

To avoid perf surprises, we should probably track save_count using a dedicated
counter. Then we could use something like this to validate the stack.

[reed1, 2014/12/10 20:06:36]

Probably true :(

-    return fMCStack.count();
+    int count = 0;
+    SkDeque::Iter iter(fMCStack, SkDeque::Iter::kFront_IterStart);
+    for (;;) {
+        const MCRec* rec = (const MCRec*)iter.next();

[f(malita), 2014/12/10 19:49:33]

I think this will miss all deferred saves on the initial MCRec, no?

[reed1, 2014/12/10 20:06:36]

next() returns the first one : its a funny pattern -- next is curr/next/done all
in one.

+        if (!rec) {
+            break;
+        }
+        count += 1 + rec->fDeferredSaveCount;
+    }
+    return count;

[f(malita), 2014/12/10 19:49:33]

Would it be simpler to just iterate over all MCRecs and then just return (count
- 1) here?

[reed1, 2014/12/10 20:06:35]

see above

[f(malita), 2014/12/10 20:13:58]

Hmm, then something's not clicking: do we need to subtract 1 here?

(a fresh canvas starts with one MCRec and fDeferredSave == 0 => this method
returns 1?)

 }
 
 int SkCanvas::save() {
+    fMCRec->fDeferredSaveCount += 1;
+#if 0
+    this->checkForDeferredSave();
+#endif
+    return this->getSaveCount() - 1;  // return our prev value
+}
+
+void SkCanvas::doSave() {
+    fCallingWillSave = true;
     this->willSave();
-    return this->internalSave();
+    fCallingWillSave = false;

[robertphillips, 2014/12/10 18:43:53]

Do we need this cast ?

[reed1, 2014/12/10 20:06:35]

Done.

+    (void)this->internalSave();
 }
 
 void SkCanvas::restore() {
-    // check for underflow
-    if (fMCStack.count() > 1) {
-        this->willRestore();
-        this->internalRestore();
-        this->didRestore();
+    if (fMCRec->fDeferredSaveCount > 0) {
+        fMCRec->fDeferredSaveCount -= 1;
+    } else {
+        // check for underflow
+        if (fMCStack.count() > 1) {
+            this->willRestore();
+            this->internalRestore();
+            this->didRestore();
+        }
     }
 }
 
@@ -815,16 +849,12 @@ void SkCanvas::restoreToCount(int count) {
     }
 }
 
-int SkCanvas::internalSave() {
-    int saveCount = this->getSaveCount(); // record this before the actual save
-
+void SkCanvas::internalSave() {
     MCRec* newTop = (MCRec*)fMCStack.push_back();
     new (newTop) MCRec(*fMCRec);    // balanced in restore()
     fMCRec = newTop;
 
     fClipStack.save();
-
-    return saveCount;
 }
 
 static bool bounds_affects_clip(SkCanvas::SaveFlags flags) {
@@ -881,16 +911,17 @@ bool SkCanvas::clipRectBounds(const SkRect* bounds, SaveFlags flags,
 
 int SkCanvas::saveLayer(const SkRect* bounds, const SkPaint* paint) {
     SaveLayerStrategy strategy = this->willSaveLayer(bounds, paint, kARGB_ClipLayer_SaveFlag);
-    return this->internalSaveLayer(bounds, paint, kARGB_ClipLayer_SaveFlag, false, strategy);
+    this->internalSaveLayer(bounds, paint, kARGB_ClipLayer_SaveFlag, false, strategy);
+    return this->getSaveCount() - 1;
 }
 
-int SkCanvas::saveLayer(const SkRect* bounds, const SkPaint* paint,
-                        SaveFlags flags) {
+int SkCanvas::saveLayer(const SkRect* bounds, const SkPaint* paint, SaveFlags flags) {
     SaveLayerStrategy strategy = this->willSaveLayer(bounds, paint, flags);
-    return this->internalSaveLayer(bounds, paint, flags, false, strategy);
+    this->internalSaveLayer(bounds, paint, flags, false, strategy);
+    return this->getSaveCount() - 1;
 }
 
-int SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, SaveFlags flags,
+void SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, SaveFlags flags,
                                 bool justForImageFilter, SaveLayerStrategy strategy) {
 #ifndef SK_SUPPORT_LEGACY_CLIPTOLAYERFLAG
     flags |= kClipToLayer_SaveFlag;
@@ -898,19 +929,19 @@ int SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, Save
 
     // do this before we create the layer. We don't call the public save() since
     // that would invoke a possibly overridden virtual
-    int count = this->internalSave();
+    this->internalSave();
 
     fDeviceCMDirty = true;
 
     SkIRect ir;
     if (!this->clipRectBounds(bounds, flags, &ir, paint ? paint->getImageFilter() : NULL)) {
-        return count;
+        return;
     }
 
     // FIXME: do willSaveLayer() overriders returning kNoLayer_SaveLayerStrategy really care about
     // the clipRectBounds() call above?
     if (kNoLayer_SaveLayerStrategy == strategy) {
-        return count;
+        return;
     }
 
     // Kill the imagefilter if our device doesn't allow it
@@ -919,7 +950,7 @@ int SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, Save
         if (!this->getTopDevice()->allowImageFilter(paint->getImageFilter())) {
             if (justForImageFilter) {
                 // early exit if the layer was just for the imageFilter
-                return count;
+                return;
             }
             SkPaint* p = lazyP.set(*paint);
             p->setImageFilter(NULL);
@@ -934,7 +965,7 @@ int SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, Save
     SkBaseDevice* device = this->getTopDevice();
     if (NULL == device) {
         SkDebugf("Unable to find device for layer.");
-        return count;
+        return;
     }
 
     SkBaseDevice::Usage usage = SkBaseDevice::kSaveLayer_Usage;
@@ -945,7 +976,7 @@ int SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, Save
                                                                        fProps.pixelGeometry()));
     if (NULL == device) {
         SkDebugf("Unable to create device for layer.");
-        return count;
+        return;
     }
 
     device->setOrigin(ir.fLeft, ir.fTop);
@@ -958,7 +989,6 @@ int SkCanvas::internalSaveLayer(const SkRect* bounds, const SkPaint* paint, Save
     fMCRec->fTopLayer = layer;    // this field is NOT an owner of layer
 
     fSaveLayerCount += 1;
-    return count;
 }
 
 int SkCanvas::saveLayerAlpha(const SkRect* bounds, U8CPU alpha) {
@@ -1286,6 +1316,7 @@ void SkCanvas::concat(const SkMatrix& matrix) {
         return;
     }
 
+    this->checkForDeferredSave();
     fDeviceCMDirty = true;
     fCachedLocalClipBoundsDirty = true;
     fMCRec->fMatrix.preConcat(matrix);
@@ -1294,6 +1325,7 @@ void SkCanvas::concat(const SkMatrix& matrix) {
 }
 
 void SkCanvas::setMatrix(const SkMatrix& matrix) {
+    this->checkForDeferredSave();
     fDeviceCMDirty = true;
     fCachedLocalClipBoundsDirty = true;
     fMCRec->fMatrix = matrix;
@@ -1310,6 +1342,7 @@ void SkCanvas::resetMatrix() {
 //////////////////////////////////////////////////////////////////////////////
 
 void SkCanvas::clipRect(const SkRect& rect, SkRegion::Op op, bool doAA) {
+    this->checkForDeferredSave();
     ClipEdgeStyle edgeStyle = doAA ? kSoft_ClipEdgeStyle : kHard_ClipEdgeStyle;
     this->onClipRect(rect, op, edgeStyle);
 }
@@ -1367,6 +1400,7 @@ static void rasterclip_path(SkRasterClip* rc, const SkCanvas* canvas, const SkPa
 }
 
 void SkCanvas::clipRRect(const SkRRect& rrect, SkRegion::Op op, bool doAA) {
+    this->checkForDeferredSave();
     ClipEdgeStyle edgeStyle = doAA ? kSoft_ClipEdgeStyle : kHard_ClipEdgeStyle;
     if (rrect.isRect()) {
         this->onClipRect(rrect.getBounds(), op, edgeStyle);
@@ -1402,6 +1436,7 @@ void SkCanvas::onClipRRect(const SkRRect& rrect, SkRegion::Op op, ClipEdgeStyle
 }
 
 void SkCanvas::clipPath(const SkPath& path, SkRegion::Op op, bool doAA) {
+    this->checkForDeferredSave();
     ClipEdgeStyle edgeStyle = doAA ? kSoft_ClipEdgeStyle : kHard_ClipEdgeStyle;
     SkRect r;
     if (!path.isInverseFillType() && path.isRect(&r)) {
@@ -1484,6 +1519,7 @@ void SkCanvas::onClipPath(const SkPath& path, SkRegion::Op op, ClipEdgeStyle edg
 }
 
 void SkCanvas::clipRegion(const SkRegion& rgn, SkRegion::Op op) {
+    this->checkForDeferredSave();
     this->onClipRegion(rgn, op);
 }