Plumbing for using a BBH in SkRecordDraw.

For now this only creates a degenerate bounding box hierarchy where all ops
just have maximal bounds.  I will flesh out FillBounds in future CL(s).

Not quite sure why QuadTree and TileGrid aren't drawing right---haven't even
looked at the diffs yet---so I've disabled those test modes for now.  RTree
seems fine, so that'll at least get us coverage for all this new plumbing.

BUG=skia:
R=robertphillips@google.com, mtklein@google.com, reed@google.com

Author: mtklein@chromium.org

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

dm/DMCpuGMTask.cpp

@@ -23,31 +23,37 @@ CpuGMTask::CpuGMTask(const char* config,
     {}
 
 void CpuGMTask::draw() {
-    SkBitmap bitmap;
+    SkBitmap bm;
-    AllocatePixels(fColorType, fGM->getISize().width(), fGM->getISize().height(), &bitmap);
+    AllocatePixels(fColorType, fGM->getISize().width(), fGM->getISize().height(), &bm);
 
-    SkCanvas canvas(bitmap);
+    SkCanvas canvas(bm);
     canvas.concat(fGM->getInitialTransform());
     fGM->draw(&canvas);
     canvas.flush();
 
 #define SPAWN(ChildTask, ...) this->spawnChild(SkNEW_ARGS(ChildTask, (*this, __VA_ARGS__)))
-    SPAWN(ExpectationsTask, fExpectations, bitmap);
+    SPAWN(ExpectationsTask, fExpectations, bm);
 
-    SPAWN(PipeTask, fGMFactory(NULL), bitmap, PipeTask::kInProcess_Mode);
+    SPAWN(PipeTask, fGMFactory(NULL), bm, PipeTask::kInProcess_Mode);
-    SPAWN(PipeTask, fGMFactory(NULL), bitmap, PipeTask::kCrossProcess_Mode);
+    SPAWN(PipeTask, fGMFactory(NULL), bm, PipeTask::kCrossProcess_Mode);
-    SPAWN(PipeTask, fGMFactory(NULL), bitmap, PipeTask::kSharedAddress_Mode);
+    SPAWN(PipeTask, fGMFactory(NULL), bm, PipeTask::kSharedAddress_Mode);
 
-    SPAWN(QuiltTask, fGMFactory(NULL), bitmap, QuiltTask::kNoBBH_Mode);
+    SPAWN(QuiltTask, fGMFactory(NULL), bm, QuiltTask::kNone_BBH,     QuiltTask::kDefault_Backend);
-    SPAWN(QuiltTask, fGMFactory(NULL), bitmap, QuiltTask::kRTree_Mode);
+    SPAWN(QuiltTask, fGMFactory(NULL), bm, QuiltTask::kRTree_BBH,    QuiltTask::kDefault_Backend);
-    SPAWN(QuiltTask, fGMFactory(NULL), bitmap, QuiltTask::kQuadTree_Mode);
+    SPAWN(QuiltTask, fGMFactory(NULL), bm, QuiltTask::kQuadTree_BBH, QuiltTask::kDefault_Backend);
-    SPAWN(QuiltTask, fGMFactory(NULL), bitmap, QuiltTask::kTileGrid_Mode);
+    SPAWN(QuiltTask, fGMFactory(NULL), bm, QuiltTask::kTileGrid_BBH, QuiltTask::kDefault_Backend);
-    SPAWN(QuiltTask, fGMFactory(NULL), bitmap, QuiltTask::kSkRecord_Mode);
 
-    SPAWN(SerializeTask, fGMFactory(NULL), bitmap, SerializeTask::kNormal_Mode);
+    SPAWN(QuiltTask, fGMFactory(NULL), bm, QuiltTask::kNone_BBH,     QuiltTask::kSkRecord_Backend);
-    SPAWN(SerializeTask, fGMFactory(NULL), bitmap, SerializeTask::kSkRecord_Mode);
+    SPAWN(QuiltTask, fGMFactory(NULL), bm, QuiltTask::kRTree_BBH,    QuiltTask::kSkRecord_Backend);
+    SPAWN(QuiltTask, fGMFactory(NULL), bm, QuiltTask::kQuadTree_BBH, QuiltTask::kSkRecord_Backend);
+    /*  TODO: Failing, not sure why.  Enable these when passing.
+    SPAWN(QuiltTask, fGMFactory(NULL), bm, QuiltTask::kTileGrid_BBH, QuiltTask::kSkRecord_Backend);
+    */
 
-    SPAWN(WriteTask, bitmap);
+    SPAWN(SerializeTask, fGMFactory(NULL), bm, SerializeTask::kNormal_Mode);
+    SPAWN(SerializeTask, fGMFactory(NULL), bm, SerializeTask::kSkRecord_Mode);
+
+    SPAWN(WriteTask, bm);
 #undef SPAWN
 }
 

dm/DMQuiltTask.cpp

@@ -10,14 +10,20 @@
 DEFINE_bool(quilt, true, "If true, draw GM via a picture into a quilt of small tiles and compare.");
 DEFINE_int32(quiltTile, 256, "Dimension of (square) quilt tile.");
 
-static const char* kSuffixes[] = { "nobbh", "rtree", "quadtree", "tilegrid", "skr" };
-
 namespace DM {
 
-QuiltTask::QuiltTask(const Task& parent, skiagm::GM* gm, SkBitmap reference, QuiltTask::Mode mode)
+static SkString suffix(QuiltTask::Backend backend, QuiltTask::BBH bbh) {
+    static const char* kBackends[] = { "default", "skrecord" };
+    static const char* kBBHs[]     = { "nobbh", "rtree", "quadtree", "tilegrid" };
+    return SkStringPrintf("%s-%s", kBackends[backend], kBBHs[bbh]);
+}
+
+QuiltTask::QuiltTask(const Task& parent, skiagm::GM* gm, SkBitmap reference,
+                     QuiltTask::BBH bbh, QuiltTask::Backend backend)
     : CpuTask(parent)
-    , fMode(mode)
+    , fBBH(bbh)
-    , fName(UnderJoin(parent.name().c_str(), kSuffixes[mode]))
+    , fBackend(backend)
+    , fName(UnderJoin(parent.name().c_str(), suffix(backend, bbh).c_str()))
     , fGM(gm)
     , fReference(reference)
     {}
@@ -54,14 +60,15 @@ private:
 
 void QuiltTask::draw() {
     SkAutoTDelete<SkBBHFactory> factory;
-    switch (fMode) {
+    switch (fBBH) {
-        case kRTree_Mode:
+        case kNone_BBH: break;
+        case kRTree_BBH:
             factory.reset(SkNEW(SkRTreeFactory));
             break;
-        case kQuadTree_Mode:
+        case kQuadTree_BBH:
             factory.reset(SkNEW(SkQuadTreeFactory));
             break;
-        case kTileGrid_Mode: {
+        case kTileGrid_BBH: {
             const SkTileGridFactory::TileGridInfo tiles = {
                 { FLAGS_quiltTile, FLAGS_quiltTile },
                 /*overlap: */{0, 0},
@@ -70,10 +77,6 @@ void QuiltTask::draw() {
             factory.reset(SkNEW_ARGS(SkTileGridFactory, (tiles)));
             break;
         }
-
-        case kNoBBH_Mode:
-        case kSkRecord_Mode:
-            break;
     }
 
     // A couple GMs draw wrong when using a bounding box hierarchy.
@@ -84,7 +87,7 @@ void QuiltTask::draw() {
     }
 
     SkAutoTUnref<const SkPicture> recorded(
-            RecordPicture(fGM.get(), factory.get(), kSkRecord_Mode == fMode));
+            RecordPicture(fGM.get(), factory.get(), kSkRecord_Backend == fBackend));
 
     SkBitmap full;
     AllocatePixels(fReference, &full);

dm/DMQuiltTask.h

@@ -12,27 +12,30 @@
 namespace DM {
 
 class QuiltTask : public CpuTask {
-
 public:
-    enum Mode {
+    enum BBH {
-        kNoBBH_Mode,
+        kNone_BBH,
-        kRTree_Mode,
+        kRTree_BBH,
-        kQuadTree_Mode,
+        kQuadTree_BBH,
-        kTileGrid_Mode,
+        kTileGrid_BBH,
-        kSkRecord_Mode,  // Currently uses no BBH.
+    };
+    enum Backend {
+        kDefault_Backend,
+        kSkRecord_Backend,
     };
 
     QuiltTask(const Task& parent,  // QuiltTask must be a child task.  Pass its parent here.
               skiagm::GM*,         // GM to run through a picture.  Takes ownership.
               SkBitmap reference,  // Bitmap to compare picture replay results to.
-              Mode mode);
+              BBH, Backend);
 
     virtual void draw() SK_OVERRIDE;
     virtual bool shouldSkip() const SK_OVERRIDE;
     virtual SkString name() const SK_OVERRIDE { return fName; }
 
 private:
-    const Mode fMode;
+    const BBH fBBH;
+    const Backend fBackend;
     const SkString fName;
     SkAutoTDelete<skiagm::GM> fGM;
     const SkBitmap fReference;

include/core/SkPicture.h

@@ -20,7 +20,6 @@
 class GrContext;
 #endif
 
-class SkBBHFactory;
 class SkBBoxHierarchy;
 class SkCanvas;
 class SkData;
@@ -289,8 +288,11 @@ private:
 
     typedef SkRefCnt INHERITED;
 
-    SkPicture(int width, int height, SkRecord*);  // Takes ownership.
+    // Takes ownership of the SkRecord, refs the (optional) BBH.
-    SkAutoTDelete<SkRecord> fRecord;
+    SkPicture(int width, int height, SkRecord*, SkBBoxHierarchy*);
+
+    SkAutoTDelete<SkRecord>       fRecord;
+    SkAutoTUnref<SkBBoxHierarchy> fBBH;
     bool fRecordWillPlayBackBitmaps; // TODO: const
 };
 

include/core/SkPictureRecorder.h

@@ -80,6 +80,8 @@ private:
     int fWidth;
     int fHeight;
 
+    SkAutoTUnref<SkBBoxHierarchy> fBBH;
+
     // One of these two canvases will be non-NULL.
     SkAutoTUnref<SkPictureRecord> fPictureRecord;  // beginRecording()
     SkAutoTUnref<SkRecorder>      fRecorder;       // EXPERIMENTAL_beginRecording()

src/core/SkPicture.cpp

@@ -14,7 +14,6 @@
 #include "SkPictureRecorder.h"
 #include "SkPictureStateTree.h"
 
-#include "SkBBHFactory.h"
 #include "SkBitmapDevice.h"
 #include "SkCanvas.h"
 #include "SkChunkAlloc.h"
@@ -139,7 +138,7 @@ SkPicture::SkPicture(int width, int height,
 // This for compatibility with serialization code only.  This is not cheap.
 static SkPicture* backport(const SkRecord& src, int width, int height) {
     SkPictureRecorder recorder;
-    SkRecordDraw(src, recorder.beginRecording(width, height));
+    SkRecordDraw(src, recorder.beginRecording(width, height), NULL/*bbh*/, NULL/*callback*/);
     return recorder.endRecording();
 }
 
@@ -267,7 +266,7 @@ void SkPicture::draw(SkCanvas* canvas, SkDrawPictureCallback* callback) const {
         playback.draw(canvas, callback);
     }
     if (NULL != fRecord.get()) {
-        SkRecordDraw(*fRecord, canvas, callback);
+        SkRecordDraw(*fRecord, canvas, fBBH.get(), callback);
     }
 }
 
@@ -490,11 +489,16 @@ uint32_t SkPicture::uniqueID() const {
 }
 
 // fRecord OK
-SkPicture::SkPicture(int width, int height, SkRecord* record)
+SkPicture::SkPicture(int width, int height, SkRecord* record, SkBBoxHierarchy* bbh)
     : fWidth(width)
     , fHeight(height)
     , fRecord(record)
+    , fBBH(SkSafeRef(bbh))
     , fRecordWillPlayBackBitmaps(SkRecordWillPlaybackBitmaps(*record)) {
+    // TODO: delay as much of this work until just before first playback?
+    if (fBBH.get()) {
+        SkRecordFillBounds(*record, fBBH.get());
+    }
     this->needsNewGenID();
 }
 

src/core/SkPictureRecorder.cpp

@@ -26,9 +26,11 @@ SkCanvas* SkPictureRecorder::beginRecording(int width, int height,
     const SkISize size = SkISize::Make(width, height);
 
     if (NULL != bbhFactory) {
-        SkAutoTUnref<SkBBoxHierarchy> tree((*bbhFactory)(width, height));
+        // We don't need to hold a ref on the BBH ourselves, but might as well for
-        SkASSERT(NULL != tree);
+        // consistency with EXPERIMENTAL_beginRecording(), which does need to.
-        fPictureRecord.reset(SkNEW_ARGS(SkBBoxHierarchyRecord, (size, recordFlags, tree.get())));
+        fBBH.reset((*bbhFactory)(width, height));
+        SkASSERT(NULL != fBBH.get());
+        fPictureRecord.reset(SkNEW_ARGS(SkBBoxHierarchyRecord, (size, recordFlags, fBBH.get())));
     } else {
         fPictureRecord.reset(SkNEW_ARGS(SkPictureRecord, (size, recordFlags)));
     }
@@ -42,7 +44,11 @@ SkCanvas* SkPictureRecorder::EXPERIMENTAL_beginRecording(int width, int height,
     fWidth = width;
     fHeight = height;
 
-    // TODO: plumb bbhFactory through
+    if (NULL != bbhFactory) {
+        fBBH.reset((*bbhFactory)(width, height));
+        SkASSERT(NULL != fBBH.get());
+    }
+
     fRecord.reset(SkNEW(SkRecord));
     fRecorder.reset(SkNEW_ARGS(SkRecorder, (fRecord.get(), width, height)));
     return this->getRecordingCanvas();
@@ -59,7 +65,7 @@ SkPicture* SkPictureRecorder::endRecording() {
     SkPicture* picture = NULL;
 
     if (NULL != fRecord.get()) {
-        picture = SkNEW_ARGS(SkPicture, (fWidth, fHeight, fRecord.detach()));
+        picture = SkNEW_ARGS(SkPicture, (fWidth, fHeight, fRecord.detach(), fBBH.get()));
     }
 
     if (NULL != fPictureRecord.get()) {
@@ -83,7 +89,7 @@ void SkPictureRecorder::partialReplay(SkCanvas* canvas) const {
     }
 
     if (NULL != fRecord.get()) {
-        SkRecordDraw(*fRecord, canvas);
+        SkRecordDraw(*fRecord, canvas, NULL/*bbh*/, NULL/*callback*/);
     }
 
     if (NULL != fPictureRecord.get()) {

src/core/SkRecordDraw.cpp

@@ -6,14 +6,46 @@
  */
 
 #include "SkRecordDraw.h"
+#include "SkTSort.h"
 
-void SkRecordDraw(const SkRecord& record, SkCanvas* canvas, SkDrawPictureCallback* callback) {
+void SkRecordDraw(const SkRecord& record,
+                  SkCanvas* canvas,
+                  const SkBBoxHierarchy* bbh,
+                  SkDrawPictureCallback* callback) {
     SkAutoCanvasRestore saveRestore(canvas, true /*save now, restore at exit*/);
-    for (SkRecords::Draw draw(canvas); draw.index() < record.count(); draw.next()) {
+
-        if (NULL != callback && callback->abortDrawing()) {
+    if (NULL != bbh) {
-            return;
+        SkASSERT(bbh->getCount() == SkToInt(record.count()));
+
+        // Draw only ops that affect pixels in the canvas's current clip.
+        SkIRect devBounds;
+        canvas->getClipDeviceBounds(&devBounds);
+        SkTDArray<void*> ops;
+        bbh->search(devBounds, &ops);
+
+        // Until we start filling in real bounds, we should get every op back.
+        SkASSERT(ops.count() == SkToInt(record.count()));
+
+        // FIXME: QuadTree doesn't send these back in the order we inserted them.  :(
+        if (ops.count() > 0) {
+            SkTQSort(ops.begin(), ops.end() - 1, SkTCompareLT<void*>());
+        }
+
+        SkRecords::Draw draw(canvas);
+        for (int i = 0; i < ops.count(); i++) {
+            if (NULL != callback && callback->abortDrawing()) {
+                return;
+            }
+            record.visit<void>((uintptr_t)ops[i], draw);  // See FillBounds below.
+        }
+    } else {
+        // Draw all ops.
+        for (SkRecords::Draw draw(canvas); draw.index() < record.count(); draw.next()) {
+            if (NULL != callback && callback->abortDrawing()) {
+                return;
+            }
+            record.visit<void>(draw.index(), draw);
         }
-        record.visit<void>(draw.index(), draw);
     }
 }
 
@@ -65,4 +97,35 @@ DRAW(DrawVertices, drawVertices(r.vmode, r.vertexCount, r.vertices, r.texs, r.co
                                 r.xmode.get(), r.indices, r.indexCount, r.paint));
 #undef DRAW
 
+
+// This is an SkRecord visitor that fills an SkBBoxHierarchy.
+class FillBounds : SkNoncopyable {
+public:
+    explicit FillBounds(SkBBoxHierarchy* bbh) : fBBH(bbh), fIndex(0) {}
+    ~FillBounds() { fBBH->flushDeferredInserts(); }
+
+    uintptr_t index() const { return fIndex; }
+    void next() { ++fIndex; }
+
+    template <typename T> void operator()(const T& r) {
+        // MakeLargest() is a trivially safe default for ops that haven't been bounded yet.
+        this->insert(this->index(), SkIRect::MakeLargest());
+    }
+
+private:
+    void insert(uintptr_t opIndex, const SkIRect& bounds) {
+        fBBH->insert((void*)opIndex, bounds, true/*ok to defer*/);
+    }
+
+    SkBBoxHierarchy* fBBH;  // Unowned. The BBH is guaranteed to be ref'd for our lifetime.
+    uintptr_t fIndex;
+};
+
 }  // namespace SkRecords
+
+void SkRecordFillBounds(const SkRecord& record, SkBBoxHierarchy* bbh) {
+    SkASSERT(NULL != bbh);
+    for(SkRecords::FillBounds fb(bbh); fb.index() < record.count(); fb.next()) {
+        record.visit<void>(fb.index(), fb);
+    }
+}

src/core/SkRecordDraw.h

@@ -8,12 +8,16 @@
 #ifndef SkRecordDraw_DEFINED
 #define SkRecordDraw_DEFINED
 
-#include "SkRecord.h"
+#include "SkBBoxHierarchy.h"
 #include "SkCanvas.h"
 #include "SkDrawPictureCallback.h"
+#include "SkRecord.h"
+
+// Fill a BBH to be used by SkRecordDraw to accelerate playback.
+void SkRecordFillBounds(const SkRecord&, SkBBoxHierarchy*);
 
 // Draw an SkRecord into an SkCanvas.  A convenience wrapper around SkRecords::Draw.
-void SkRecordDraw(const SkRecord&, SkCanvas*, SkDrawPictureCallback* = NULL);
+void SkRecordDraw(const SkRecord&, SkCanvas*, const SkBBoxHierarchy*, SkDrawPictureCallback*);
 
 namespace SkRecords {
 

src/core/SkRecording.cpp

@@ -20,7 +20,7 @@ SkPlayback::~SkPlayback() {}
 
 void SkPlayback::draw(SkCanvas* canvas) const {
     SkASSERT(fRecord.get() != NULL);
-    SkRecordDraw(*fRecord, canvas);
+    SkRecordDraw(*fRecord, canvas, NULL/*bbh*/, NULL/*callback*/);
 }
 
 SkRecording::SkRecording(int width, int height)

tests/RecordDrawTest.cpp

@@ -38,7 +38,7 @@ DEF_TEST(RecordDraw_Abort, r) {
     SkRecorder canvas(&rerecord, W, H);
 
     JustOneDraw callback;
-    SkRecordDraw(record, &canvas, &callback);
+    SkRecordDraw(record, &canvas, NULL/*bbh*/, &callback);
 
     REPORTER_ASSERT(r, 3 == rerecord.count());
     assert_type<SkRecords::Save>    (r, rerecord, 0);
@@ -53,7 +53,7 @@ DEF_TEST(RecordDraw_Unbalanced, r) {
 
     SkRecord rerecord;
     SkRecorder canvas(&rerecord, W, H);
-    SkRecordDraw(record, &canvas);
+    SkRecordDraw(record, &canvas, NULL/*bbh*/, NULL/*callback*/);
 
     REPORTER_ASSERT(r, 4 == rerecord.count());
     assert_type<SkRecords::Save>    (r, rerecord, 0);
@@ -77,7 +77,7 @@ DEF_TEST(RecordDraw_SetMatrixClobber, r) {
     translate.setTranslate(20, 20);
     translateCanvas.setMatrix(translate);
 
-    SkRecordDraw(scaleRecord, &translateCanvas);
+    SkRecordDraw(scaleRecord, &translateCanvas, NULL/*bbh*/, NULL/*callback*/);
     REPORTER_ASSERT(r, 4 == translateRecord.count());
     assert_type<SkRecords::SetMatrix>(r, translateRecord, 0);
     assert_type<SkRecords::Save>     (r, translateRecord, 1);