Fixing SkPicture command pattern optimizations to make them work correctly with bounding box hierar…

src/core/SkPictureRecord.cpp

Index: src/core/SkPictureRecord.cpp
===================================================================
--- src/core/SkPictureRecord.cpp	(revision 8135)
+++ src/core/SkPictureRecord.cpp	(working copy)
@@ -502,20 +502,45 @@
 
 typedef bool (*PictureRecordOptProc)(SkWriter32* writer, int32_t offset,
                                      SkPaintDictionary* paintDict);
+enum PictureRecordOptType {
+    kRewind_OptType,  // Optimization rewinds the command stream
+    kCollapseSaveLayer_OptType,  // Optimization eliminates a save/restore pair
+};
 
+struct PictureRecordOpt {
+    PictureRecordOptProc fProc;
+    PictureRecordOptType fType;
+};
 /*
  * A list of the optimizations that are tried upon seeing a restore
  * TODO: add a real API for such optimizations
  *       Add the ability to fire optimizations on any op (not just RESTORE)
  */
-static const PictureRecordOptProc gPictureRecordOpts[] = {
-    collapse_save_clip_restore,
-#ifndef SK_IGNORE_PICTURE_RECORD_SAVE_LAYER_OPT
-    remove_save_layer1,
-    remove_save_layer2,
-#endif
+static const PictureRecordOpt gPictureRecordOpts[] = {
+    { collapse_save_clip_restore, kRewind_OptType },
+    { remove_save_layer1,         kCollapseSaveLayer_OptType },
+    { remove_save_layer2,         kCollapseSaveLayer_OptType }
 };
 
+void SkPictureRecord::handleOptimization(int opt) {
+    switch (gPictureRecordOpts[opt].fType) {
+    case kCollapseSaveLayer_OptType:

[robertphillips, 2013/03/14 18:10:17]

NULL !=

+        if (fStateTree) {
+            fStateTree->saveCollapsed();
+        }
+        break;
+    case kRewind_OptType:
+        // No need do any rewinding on the state tree, we can just leave the
+        // unreferenced branches dangling.

[robertphillips, 2013/03/14 18:10:17]

NULL !=

+        if (fBoundingHierarchy) {
+            fBoundingHierarchy->rewindInserts();
+        }
+        break;
+    default:
+        SkASSERT(0);
+    }
+}
+
 void SkPictureRecord::restore() {
     // FIXME: SkDeferredCanvas needs to be refactored to respect
     // save/restore balancing so that the following test can be
@@ -536,10 +561,11 @@
     uint32_t initialOffset, size;
     size_t opt;
     for (opt = 0; opt < SK_ARRAY_COUNT(gPictureRecordOpts); ++opt) {
-        if ((*gPictureRecordOpts[opt])(&fWriter, fRestoreOffsetStack.top(), &fPaints)) {
+        if ((*gPictureRecordOpts[opt].fProc)(&fWriter, fRestoreOffsetStack.top(), &fPaints)) {
             // Some optimization fired so don't add the RESTORE
             size = 0;
             initialOffset = fWriter.size();
+            handleOptimization(opt);
             break;
         }
     }