Retract SkPicture::kOptimizeForClippedPlayback_RecordingFlag from public API

This CL sets the stage for retracting the SkPicture::kOptimizeForClippedPlayback_RecordingFlag flag
from the public API (more work needs to be done in Blink & Chrome). In the new world the only way
to set this flag (and thus instantiate an SkPicture-derived
class) is by passing a factory to the SkPictureRecorder class. This is to get all clients always using
factories so that we can then change the factory call used (i.e., so the factory just creates a BBH) and
do away with the SkPicture-derived classes.

BUG=skia:2315
R=reed@google.com

Author: robertphillips@google.com

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

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

dm/DMReplayTask.cpp

@@ -4,6 +4,7 @@
 
 #include "SkCommandLineFlags.h"
 #include "SkPicture.h"
+#include "SkRTreePicture.h"
 
 DEFINE_bool(replay, true, "If true, run picture replay tests.");
 DEFINE_bool(rtree,  true, "If true, run picture replay tests with an rtree.");
@@ -22,8 +23,11 @@ ReplayTask::ReplayTask(const Task& parent,
     {}
 
 void ReplayTask::draw() {
-    const uint32_t flags = fUseRTree ? SkPicture::kOptimizeForClippedPlayback_RecordingFlag : 0;
-    SkAutoTUnref<SkPicture> recorded(RecordPicture(fGM.get(), flags));
+    SkAutoTUnref<SkPictureFactory> factory;
+    if (fUseRTree) {
+        factory.reset(SkNEW(SkRTreePictureFactory));
+    }
+    SkAutoTUnref<SkPicture> recorded(RecordPicture(fGM.get(), 0, factory));
 
     SkBitmap bitmap;
     SetupBitmap(fReference.colorType(), fGM.get(), &bitmap);

gm/gmmain.cpp

@@ -34,7 +34,9 @@
 #include "SkOSFile.h"
 #include "SkPDFRasterizer.h"
 #include "SkPicture.h"
+#include "SkQuadTreePicture.h"
 #include "SkRefCnt.h"
+#include "SkRTreePicture.h"
 #include "SkScalar.h"
 #include "SkStream.h"
 #include "SkString.h"
@@ -139,6 +141,7 @@ enum BbhType {
     kNone_BbhType,
     kRTree_BbhType,
     kTileGrid_BbhType,
+    kQuadTree_BbhType
 };
 
 enum ConfigFlags {
@@ -1018,9 +1021,10 @@ public:
             info.fOffset.setZero();
             info.fTileInterval.set(16, 16);
             factory.reset(SkNEW_ARGS(SkTileGridPictureFactory, (info)));
-        }
-        if (kNone_BbhType != bbhType) {
-            recordFlags |= SkPicture::kOptimizeForClippedPlayback_RecordingFlag;
+        } else if (kQuadTree_BbhType == bbhType) {
+            factory.reset(SkNEW(SkQuadTreePictureFactory));
+        } else if (kRTree_BbhType == bbhType) {
+            factory.reset(SkNEW(SkRTreePictureFactory));
         }
         SkPictureRecorder recorder(factory);
         SkCanvas* cv = recorder.beginRecording(width, height, recordFlags);
@@ -1449,6 +1453,7 @@ DEFINE_string(mismatchPath, "", "Write images for tests that failed due to "
 DEFINE_string(modulo, "", "[--modulo <remainder> <divisor>]: only run tests for which "
               "testIndex %% divisor == remainder.");
 DEFINE_bool(pipe, false, "Exercise the SkGPipe replay test pass.");
+DEFINE_bool(quadtree, false, "Exercise the QuadTree variant of SkPicture test pass.");
 DEFINE_string2(readPath, r, "", "Read reference images from this dir, and report "
                "any differences between those and the newly generated ones.");
 DEFINE_bool(replay, false, "Exercise the SkPicture replay test pass.");
@@ -1607,14 +1612,29 @@ ErrorCombination run_multiple_modes(GMMain &gmmain, GM *gm, const ConfigData &co
 
     if (FLAGS_rtree) {
         const char renderModeDescriptor[] = "-rtree";
-        if ((gmFlags & GM::kSkipPicture_Flag) ||
-            (gmFlags & GM::kSkipTiled_Flag)) {
+        if ((gmFlags & GM::kSkipPicture_Flag) || (gmFlags & GM::kSkipTiled_Flag)) {
             gmmain.RecordTestResults(kIntentionallySkipped_ErrorType, shortNamePlusConfig,
                                      renderModeDescriptor);
             errorsForAllModes.add(kIntentionallySkipped_ErrorType);
         } else {
-            SkPicture* pict = gmmain.generate_new_picture(
-                gm, kRTree_BbhType, SkPicture::kOptimizeForClippedPlayback_RecordingFlag);
+            SkPicture* pict = gmmain.generate_new_picture(gm, kRTree_BbhType, 0);
+            SkAutoUnref aur(pict);
+            SkBitmap bitmap;
+            gmmain.generate_image_from_picture(gm, compareConfig, pict, &bitmap);
+            errorsForAllModes.add(gmmain.compare_test_results_to_reference_bitmap(
+                gm->getName(), compareConfig.fName, renderModeDescriptor, bitmap,
+                &comparisonBitmap));
+        }
+    }
+
+    if (FLAGS_quadtree) {
+        const char renderModeDescriptor[] = "-quadtree";
+        if ((gmFlags & GM::kSkipPicture_Flag) || (gmFlags & GM::kSkipTiled_Flag)) {
+            gmmain.RecordTestResults(kIntentionallySkipped_ErrorType, shortNamePlusConfig,
+                                     renderModeDescriptor);
+            errorsForAllModes.add(kIntentionallySkipped_ErrorType);
+        } else {
+            SkPicture* pict = gmmain.generate_new_picture(gm, kQuadTree_BbhType, 0);
             SkAutoUnref aur(pict);
             SkBitmap bitmap;
             gmmain.generate_image_from_picture(gm, compareConfig, pict, &bitmap);

gyp/core.gypi

@@ -158,6 +158,7 @@
         '<(skia_src_path)/core/SkRRect.cpp',
         '<(skia_src_path)/core/SkRTree.h',
         '<(skia_src_path)/core/SkRTree.cpp',
+        '<(skia_src_path)/core/SkRTreePicture.cpp',
         '<(skia_src_path)/core/SkScaledImageCache.cpp',
         '<(skia_src_path)/core/SkScalar.cpp',
         '<(skia_src_path)/core/SkScalerContext.cpp',
@@ -279,6 +280,7 @@
         '<(skia_include_path)/core/SkRefCnt.h',
         '<(skia_include_path)/core/SkRegion.h',
         '<(skia_include_path)/core/SkRRect.h',
+        '<(skia_include_path)/core/SkRTreePicture.h',
         '<(skia_include_path)/core/SkScalar.h',
         '<(skia_include_path)/core/SkShader.h',
         '<(skia_include_path)/core/SkStream.h',

gyp/skia_for_chromium_defines.gypi

@@ -16,6 +16,7 @@
       'SK_SUPPORT_LEGACY_PUBLICEFFECTCONSTRUCTORS=1',
       'SK_SUPPORT_LEGACY_GETTOPDEVICE',
       'SK_SUPPORT_LEGACY_PICTURE_CAN_RECORD',
+      'SK_SUPPORT_DEPRECATED_RECORD_FLAGS',
       'SK_SUPPORT_LEGACY_N32_NAME',
     ],
   },

include/core/SkPicture.h

@@ -139,7 +139,16 @@ public:
             clip-query calls will reflect the path's bounds, not the actual
             path.
          */
-        kUsePathBoundsForClip_RecordingFlag = 0x01,
+        kUsePathBoundsForClip_RecordingFlag = 0x01
+    };
+
+#ifndef SK_SUPPORT_DEPRECATED_RECORD_FLAGS
+    // TODO: once kOptimizeForClippedPlayback_RecordingFlag is hidden from
+    // all external consumers, SkPicture::createBBoxHierarchy can also be
+    // cleaned up.
+private:
+#endif
+    enum Deprecated_RecordingFlags {
         /*  This flag causes the picture to compute bounding boxes and build
             up a spatial hierarchy (currently an R-Tree), plus a tree of Canvas'
             usually stack-based clip/etc state. This requires an increase in
@@ -159,6 +168,9 @@ public:
         */
         kOptimizeForClippedPlayback_RecordingFlag = 0x02,
     };
+#ifndef SK_SUPPORT_DEPRECATED_RECORD_FLAGS
+public:
+#endif
 
 #ifndef SK_SUPPORT_LEGACY_PICTURE_CAN_RECORD
 private:
@@ -337,6 +349,7 @@ protected:
     // For testing. Derived classes may instantiate an alternate
     // SkBBoxHierarchy implementation
     virtual SkBBoxHierarchy* createBBoxHierarchy() const;
+
 private:
     // An OperationList encapsulates a set of operation offsets into the picture byte
     // stream along with the CTMs needed for those operation.
@@ -406,6 +419,9 @@ public:
      *  Allocate a new SkPicture. Return NULL on failure.
      */
     virtual SkPicture* create(int width, int height) = 0;
+
+private:
+    typedef SkRefCnt INHERITED;
 };
 
 class SK_API SkPictureRecorder : SkNoncopyable {
@@ -428,6 +444,7 @@ public:
     SkCanvas* beginRecording(int width, int height, uint32_t recordFlags = 0) {
         if (NULL != fFactory) {
             fPicture.reset(fFactory->create(width, height));
+            recordFlags |= SkPicture::kOptimizeForClippedPlayback_RecordingFlag;
         } else {
             fPicture.reset(SkNEW(SkPicture));
         }

include/core/SkRTreePicture.h

@@ -0,0 +1,36 @@
+/*
+ * Copyright 2014 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkRTreePicture_DEFINED
+#define SkRTreePicture_DEFINED
+
+#include "SkPicture.h"
+
+/**
+ * Subclass of SkPicture that creates an RTree acceleration structure. 
+ */
+class SkRTreePicture : public SkPicture {
+public:
+    virtual SkBBoxHierarchy* createBBoxHierarchy() const SK_OVERRIDE;
+
+private:
+    typedef SkPicture INHERITED;
+};
+
+class SkRTreePictureFactory : public SkPictureFactory {
+public:
+    SkRTreePictureFactory() {}
+
+    virtual SkPicture* create(int width, int height) SK_OVERRIDE {
+        return SkNEW(SkRTreePicture);
+    }
+
+private:
+    typedef SkPictureFactory INHERITED;
+};
+
+#endif

include/core/SkTileGridPicture.h

@@ -13,10 +13,8 @@
 #include "SkSize.h"
 
 /**
- * Subclass of SkPicture that override the behavior of the
- * kOptimizeForClippedPlayback_RecordingFlag by creating an SkTileGrid
- * structure rather than an R-Tree. The tile grid has lower recording
- * and playback costs, but is less effective at eliminating extraneous
+ * Subclass of SkPicture that creates an SkTileGrid. The tile grid has lower recording
+ * and playback costs then rTree, but is less effective at eliminating extraneous
  * primitives for arbitrary query rectangles. It is most effective for
  * tiled playback when the tile structure is known at record time.
  */

samplecode/SamplePictFile.cpp

@@ -18,6 +18,7 @@
 #include "SkQuadTreePicture.h"
 #include "SkRandom.h"
 #include "SkRegion.h"
+#include "SkRTreePicture.h"
 #include "SkShader.h"
 #include "SkTileGridPicture.h"
 #include "SkUtils.h"
@@ -166,6 +167,7 @@ private:
             // no bbox playback necessary
             return pic.detach();
         case kRTree_BBoxType:
+            factory.reset(SkNEW(SkRTreePictureFactory));
             break;
         case kQuadTree_BBoxType:
             factory.reset(SkNEW(SkQuadTreePictureFactory));
@@ -184,8 +186,7 @@ private:
         }
 
         SkPictureRecorder recorder(factory);
-        pic->draw(recorder.beginRecording(pic->width(), pic->height(),
-                                          SkPicture::kOptimizeForClippedPlayback_RecordingFlag));
+        pic->draw(recorder.beginRecording(pic->width(), pic->height()));
         return recorder.endRecording();
     }
 

src/core/SkPicture.cpp

@@ -253,6 +253,10 @@ SkCanvas* SkPicture::beginRecording(int width, int height,
 }
 
 SkBBoxHierarchy* SkPicture::createBBoxHierarchy() const {
+    // TODO: this code is now replicated in SkRTreePicture. Once all external
+    // clients have been weaned off of kOptimizeForClippedPlayback_RecordingFlag,
+    // this code can be removed.
+
     // These values were empirically determined to produce reasonable
     // performance in most cases.
     static const int kRTreeMinChildren = 6;

src/core/SkQuadTreePicture.h

@@ -12,9 +12,8 @@
 #include "SkRect.h"
 
 /**
- * Subclass of SkPicture that override the behavior of the
- * kOptimizeForClippedPlayback_RecordingFlag by creating an SkQuadGrid
- * structure rather than an R-Tree. The quad tree has generally faster
+ * Subclass of SkPicture that creates an SkQuadGrid
+ * structure. The quad tree has generally faster
  * tree creation time, but slightly slower query times, as compared to
  * R-Tree, so some cases may be faster and some cases slower.
  */
@@ -29,7 +28,9 @@ private:
 };
 
 class SkQuadTreePictureFactory : public SkPictureFactory {
-private:
+public:
+    SkQuadTreePictureFactory() {}
+
     virtual SkPicture* create(int width, int height) SK_OVERRIDE {
         return SkNEW_ARGS(SkQuadTreePicture, (SkIRect::MakeWH(width, height)));
     }

src/core/SkRTreePicture.cpp

@@ -0,0 +1,25 @@
+/*
+ * Copyright 2014 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkRTreePicture.h"
+
+#include "SkRTree.h"
+
+
+SkBBoxHierarchy* SkRTreePicture::createBBoxHierarchy() const {
+    // These values were empirically determined to produce reasonable
+    // performance in most cases.
+    static const int kRTreeMinChildren = 6;
+    static const int kRTreeMaxChildren = 11;
+
+    SkScalar aspectRatio = SkScalarDiv(SkIntToScalar(fWidth),
+                                       SkIntToScalar(fHeight));
+    bool sortDraws = false;  // Do not sort draw calls when bulk loading.
+
+    return SkRTree::Create(kRTreeMinChildren, kRTreeMaxChildren,
+                           aspectRatio, sortDraws);
+}

tests/ImageFilterTest.cpp

@@ -24,6 +24,7 @@
 #include "SkMorphologyImageFilter.h"
 #include "SkOffsetImageFilter.h"
 #include "SkPicture.h"
+#include "SkRTreePicture.h"
 #include "SkRect.h"
 #include "SkTileImageFilter.h"
 #include "SkXfermodeImageFilter.h"
@@ -276,9 +277,9 @@ DEF_TEST(ImageFilterMatrixTest, reporter) {
 
     SkMatrix expectedMatrix = canvas.getTotalMatrix();
 
-    SkPictureRecorder recorder;
-    SkCanvas* recordingCanvas = recorder.beginRecording(100, 100,
-        SkPicture::kOptimizeForClippedPlayback_RecordingFlag);
+    SkAutoTUnref<SkPictureFactory> factory(SkNEW(SkRTreePictureFactory));
+    SkPictureRecorder recorder(factory);
+    SkCanvas* recordingCanvas = recorder.beginRecording(100, 100);
 
     SkPaint paint;
     SkAutoTUnref<MatrixTestImageFilter> imageFilter(

tests/PictureTest.cpp

@@ -19,6 +19,7 @@
 #include "SkQuadTreePicture.h"
 #include "SkRRect.h"
 #include "SkRandom.h"
+#include "SkRTreePicture.h"
 #include "SkShader.h"
 #include "SkStream.h"
 #include "SkTileGrid.h"
@@ -917,7 +918,7 @@ static void test_draw_empty(skiatest::Reporter* reporter) {
 
         SkAutoTUnref<SkPictureFactory> factory(SkNEW_ARGS(SkTileGridPictureFactory, (gridInfo)));
         SkPictureRecorder recorder(factory);
-        recorder.beginRecording(1, 1, SkPicture::kOptimizeForClippedPlayback_RecordingFlag);
+        recorder.beginRecording(1, 1);
         SkAutoTUnref<SkPicture> picture(recorder.endRecording());
 
         canvas.drawPicture(*picture);
@@ -925,8 +926,9 @@ static void test_draw_empty(skiatest::Reporter* reporter) {
 
     {
         // RTree
-        SkPictureRecorder recorder;
-        recorder.beginRecording(1, 1, SkPicture::kOptimizeForClippedPlayback_RecordingFlag);
+        SkAutoTUnref<SkPictureFactory> factory(SkNEW(SkRTreePictureFactory));
+        SkPictureRecorder recorder(factory);
+        recorder.beginRecording(1, 1);
         SkAutoTUnref<SkPicture> picture(recorder.endRecording());
 
         canvas.drawPicture(*picture);
@@ -936,7 +938,7 @@ static void test_draw_empty(skiatest::Reporter* reporter) {
         // quad tree
         SkAutoTUnref<SkPictureFactory> factory(SkNEW(SkQuadTreePictureFactory));
         SkPictureRecorder recorder(factory);
-        recorder.beginRecording(1, 1, SkPicture::kOptimizeForClippedPlayback_RecordingFlag);
+        recorder.beginRecording(1, 1);
         SkAutoTUnref<SkPicture> picture(recorder.endRecording());
 
         canvas.drawPicture(*picture);

tests/TileGridTest.cpp

@@ -62,7 +62,7 @@ DEF_TEST(TileGrid_UnalignedQuery, reporter) {
                                     SkIntToScalar(1), SkIntToScalar(1));
     SkAutoTUnref<SkPictureFactory> factory(SkNEW_ARGS(SkTileGridPictureFactory, (info)));
     SkPictureRecorder recorder(factory);
-    SkCanvas* canvas = recorder.beginRecording(20, 20, SkPicture::kOptimizeForClippedPlayback_RecordingFlag);
+    SkCanvas* canvas = recorder.beginRecording(20, 20);
     SkPaint paint;
     canvas->drawRect(rect1, paint);
     canvas->drawRect(rect2, paint);
@@ -151,7 +151,7 @@ DEF_TEST(TileGrid_OverlapOffsetQueryAlignment, reporter) {
                                     SkIntToScalar(1), SkIntToScalar(1));
     SkAutoTUnref<SkPictureFactory> factory(SkNEW_ARGS(SkTileGridPictureFactory, (info)));
     SkPictureRecorder recorder(factory);
-    SkCanvas* canvas = recorder.beginRecording(20, 20, SkPicture::kOptimizeForClippedPlayback_RecordingFlag);
+    SkCanvas* canvas = recorder.beginRecording(20, 20);
     SkPaint paint;
     canvas->drawRect(rect1, paint);
     canvas->drawRect(rect2, paint);

tools/PictureRenderer.cpp

@@ -28,7 +28,7 @@
 #include "SkPixelRef.h"
 #include "SkQuadTree.h"
 #include "SkQuadTreePicture.h"
-#include "SkRTree.h"
+#include "SkRTreePicture.h"
 #include "SkScalar.h"
 #include "SkStream.h"
 #include "SkString.h"
@@ -319,9 +319,9 @@ void PictureRenderer::purgeTextures() {
 }
 
 uint32_t PictureRenderer::recordFlags() {
-    return ((kNone_BBoxHierarchyType == fBBoxHierarchyType) ? 0 :
-        SkPicture::kOptimizeForClippedPlayback_RecordingFlag) |
-        SkPicture::kUsePathBoundsForClip_RecordingFlag;
+    return (kNone_BBoxHierarchyType == fBBoxHierarchyType) 
+                            ? 0
+                            : SkPicture::kUsePathBoundsForClip_RecordingFlag;
 }
 
 /**
@@ -975,29 +975,6 @@ SkString PlaybackCreationRenderer::getConfigNameInternal() {
 ///////////////////////////////////////////////////////////////////////////////////////////////
 // SkPicture variants for each BBoxHierarchy type
 
-class RTreePicture : public SkPicture {
-public:
-    virtual SkBBoxHierarchy* createBBoxHierarchy() const SK_OVERRIDE {
-        static const int kRTreeMinChildren = 6;
-        static const int kRTreeMaxChildren = 11;
-        SkScalar aspectRatio = SkScalarDiv(SkIntToScalar(fWidth),
-                                           SkIntToScalar(fHeight));
-        bool sortDraws = false;
-        return SkRTree::Create(kRTreeMinChildren, kRTreeMaxChildren,
-                               aspectRatio, sortDraws);
-    }
-};
-
-class SkRTreePictureFactory : public SkPictureFactory {
-private:
-    virtual SkPicture* create(int width, int height) SK_OVERRIDE {
-        return SkNEW(RTreePicture);
-    }
-
-private:
-    typedef SkPictureFactory INHERITED;
-};
-
 SkPictureFactory* PictureRenderer::getFactory() {
     switch (fBBoxHierarchyType) {
         case kNone_BBoxHierarchyType:

tools/bench_record.cpp

@@ -12,6 +12,7 @@
 #include "SkPicture.h"
 #include "SkQuadTreePicture.h"
 #include "SkRecording.h"
+#include "SkRTreePicture.h"
 #include "SkStream.h"
 #include "SkString.h"
 #include "SkTileGridPicture.h"
@@ -33,19 +34,17 @@ DEFINE_int32(tileGridSize, 512, "Set the tile grid size. Has no effect if bbh is
 DEFINE_string(bbh, "", "Turn on the bbh and select the type, one of rtree, tilegrid, quadtree");
 DEFINE_bool(skr, false, "Record SKR instead of SKP.");
 
-typedef SkPictureFactory* (*PictureFactory)(int* recordingFlags);
+typedef SkPictureFactory* (*PictureFactory)();
 
-static SkPictureFactory* vanilla_factory(int* recordingFlags) {
+static SkPictureFactory* vanilla_factory() {
     return NULL;
 }
 
-static SkPictureFactory* rtree_factory(int* recordingFlags) {
-    *recordingFlags |= SkPicture::kOptimizeForClippedPlayback_RecordingFlag;
-    return NULL;
+static SkPictureFactory* rtree_factory() {
+    return SkNEW(SkRTreePictureFactory);
 }
 
-static SkPictureFactory* tilegrid_factory(int* recordingFlags) {
-    *recordingFlags |= SkPicture::kOptimizeForClippedPlayback_RecordingFlag;
+static SkPictureFactory* tilegrid_factory() {
     SkTileGridPicture::TileGridInfo info;
     info.fTileInterval.set(FLAGS_tileGridSize, FLAGS_tileGridSize);
     info.fMargin.setEmpty();
@@ -53,8 +52,7 @@ static SkPictureFactory* tilegrid_factory(int* recordingFlags) {
     return SkNEW_ARGS(SkTileGridPictureFactory, (info));
 }
 
-static SkPictureFactory* quadtree_factory(int* recordingFlags) {
-    *recordingFlags |= SkPicture::kOptimizeForClippedPlayback_RecordingFlag;
+static SkPictureFactory* quadtree_factory() {
     return SkNEW(SkQuadTreePictureFactory);
 }
 
@@ -94,7 +92,7 @@ static void bench_record(SkPicture* src, const char* name, PictureFactory pictur
             SkDELETE(SkRecording::Delete(recording));  // delete the SkPlayback*.
         } else {
             int recordingFlags = FLAGS_flags;
-            SkAutoTUnref<SkPictureFactory> factory(pictureFactory(&recordingFlags));
+            SkAutoTUnref<SkPictureFactory> factory(pictureFactory());
             SkPictureRecorder recorder(factory);
             SkCanvas* canvas = recorder.beginRecording(width, height, recordingFlags);
             if (NULL != src) {