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
diff --git a/dm/DMCpuGMTask.cpp b/dm/DMCpuGMTask.cpp
index 912ca9e..ceadc26 100644
--- a/dm/DMCpuGMTask.cpp
+++ b/dm/DMCpuGMTask.cpp
@@ -23,31 +23,37 @@
{}
void CpuGMTask::draw() {
- SkBitmap bitmap;
- AllocatePixels(fColorType, fGM->getISize().width(), fGM->getISize().height(), &bitmap);
+ SkBitmap bm;
+ 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), bitmap, PipeTask::kCrossProcess_Mode);
- SPAWN(PipeTask, fGMFactory(NULL), bitmap, PipeTask::kSharedAddress_Mode);
+ SPAWN(PipeTask, fGMFactory(NULL), bm, PipeTask::kInProcess_Mode);
+ SPAWN(PipeTask, fGMFactory(NULL), bm, PipeTask::kCrossProcess_Mode);
+ SPAWN(PipeTask, fGMFactory(NULL), bm, PipeTask::kSharedAddress_Mode);
- SPAWN(QuiltTask, fGMFactory(NULL), bitmap, QuiltTask::kNoBBH_Mode);
- SPAWN(QuiltTask, fGMFactory(NULL), bitmap, QuiltTask::kRTree_Mode);
- SPAWN(QuiltTask, fGMFactory(NULL), bitmap, QuiltTask::kQuadTree_Mode);
- SPAWN(QuiltTask, fGMFactory(NULL), bitmap, QuiltTask::kTileGrid_Mode);
- SPAWN(QuiltTask, fGMFactory(NULL), bitmap, QuiltTask::kSkRecord_Mode);
+ SPAWN(QuiltTask, fGMFactory(NULL), bm, QuiltTask::kNone_BBH, QuiltTask::kDefault_Backend);
+ SPAWN(QuiltTask, fGMFactory(NULL), bm, QuiltTask::kRTree_BBH, QuiltTask::kDefault_Backend);
+ SPAWN(QuiltTask, fGMFactory(NULL), bm, QuiltTask::kQuadTree_BBH, QuiltTask::kDefault_Backend);
+ SPAWN(QuiltTask, fGMFactory(NULL), bm, QuiltTask::kTileGrid_BBH, QuiltTask::kDefault_Backend);
- SPAWN(SerializeTask, fGMFactory(NULL), bitmap, SerializeTask::kNormal_Mode);
- SPAWN(SerializeTask, fGMFactory(NULL), bitmap, SerializeTask::kSkRecord_Mode);
+ SPAWN(QuiltTask, fGMFactory(NULL), bm, QuiltTask::kNone_BBH, QuiltTask::kSkRecord_Backend);
+ 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
}
diff --git a/dm/DMQuiltTask.cpp b/dm/DMQuiltTask.cpp
index 04f5999..469b20f 100644
--- a/dm/DMQuiltTask.cpp
+++ b/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)
- , fName(UnderJoin(parent.name().c_str(), kSuffixes[mode]))
+ , fBBH(bbh)
+ , fBackend(backend)
+ , fName(UnderJoin(parent.name().c_str(), suffix(backend, bbh).c_str()))
, fGM(gm)
, fReference(reference)
{}
@@ -54,14 +60,15 @@
void QuiltTask::draw() {
SkAutoTDelete<SkBBHFactory> factory;
- switch (fMode) {
- case kRTree_Mode:
+ switch (fBBH) {
+ 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 @@
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 @@
}
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);
diff --git a/dm/DMQuiltTask.h b/dm/DMQuiltTask.h
index 1bc6a27..79d8216 100644
--- a/dm/DMQuiltTask.h
+++ b/dm/DMQuiltTask.h
@@ -12,27 +12,30 @@
namespace DM {
class QuiltTask : public CpuTask {
-
public:
- enum Mode {
- kNoBBH_Mode,
- kRTree_Mode,
- kQuadTree_Mode,
- kTileGrid_Mode,
- kSkRecord_Mode, // Currently uses no BBH.
+ enum BBH {
+ kNone_BBH,
+ kRTree_BBH,
+ kQuadTree_BBH,
+ kTileGrid_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;
diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h
index c733e53..e3b33e9 100644
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -20,7 +20,6 @@
class GrContext;
#endif
-class SkBBHFactory;
class SkBBoxHierarchy;
class SkCanvas;
class SkData;
@@ -289,8 +288,11 @@
typedef SkRefCnt INHERITED;
- SkPicture(int width, int height, SkRecord*); // Takes ownership.
- SkAutoTDelete<SkRecord> fRecord;
+ // Takes ownership of the SkRecord, refs the (optional) BBH.
+ SkPicture(int width, int height, SkRecord*, SkBBoxHierarchy*);
+
+ SkAutoTDelete<SkRecord> fRecord;
+ SkAutoTUnref<SkBBoxHierarchy> fBBH;
bool fRecordWillPlayBackBitmaps; // TODO: const
};
diff --git a/include/core/SkPictureRecorder.h b/include/core/SkPictureRecorder.h
index bd22d55..c00d1b3 100644
--- a/include/core/SkPictureRecorder.h
+++ b/include/core/SkPictureRecorder.h
@@ -80,6 +80,8 @@
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()
diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index b402297..8b2bdab 100644
--- a/src/core/SkPicture.cpp
+++ b/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 @@
// 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 @@
playback.draw(canvas, callback);
}
if (NULL != fRecord.get()) {
- SkRecordDraw(*fRecord, canvas, callback);
+ SkRecordDraw(*fRecord, canvas, fBBH.get(), callback);
}
}
@@ -490,11 +489,16 @@
}
// 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();
}
diff --git a/src/core/SkPictureRecorder.cpp b/src/core/SkPictureRecorder.cpp
index 7985145..7d24caf 100644
--- a/src/core/SkPictureRecorder.cpp
+++ b/src/core/SkPictureRecorder.cpp
@@ -26,9 +26,11 @@
const SkISize size = SkISize::Make(width, height);
if (NULL != bbhFactory) {
- SkAutoTUnref<SkBBoxHierarchy> tree((*bbhFactory)(width, height));
- SkASSERT(NULL != tree);
- fPictureRecord.reset(SkNEW_ARGS(SkBBoxHierarchyRecord, (size, recordFlags, tree.get())));
+ // We don't need to hold a ref on the BBH ourselves, but might as well for
+ // consistency with EXPERIMENTAL_beginRecording(), which does need to.
+ 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 @@
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* 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 @@
}
if (NULL != fRecord.get()) {
- SkRecordDraw(*fRecord, canvas);
+ SkRecordDraw(*fRecord, canvas, NULL/*bbh*/, NULL/*callback*/);
}
if (NULL != fPictureRecord.get()) {
diff --git a/src/core/SkRecordDraw.cpp b/src/core/SkRecordDraw.cpp
index d29e0b8..e075074 100644
--- a/src/core/SkRecordDraw.cpp
+++ b/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()) {
- return;
+
+ if (NULL != bbh) {
+ 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*>());
}
- record.visit<void>(draw.index(), draw);
+
+ 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);
+ }
}
}
@@ -65,4 +97,35 @@
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);
+ }
+}
diff --git a/src/core/SkRecordDraw.h b/src/core/SkRecordDraw.h
index a9557f4..8da7fb5 100644
--- a/src/core/SkRecordDraw.h
+++ b/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 {
diff --git a/src/core/SkRecording.cpp b/src/core/SkRecording.cpp
index 94fabce..368ebb2 100644
--- a/src/core/SkRecording.cpp
+++ b/src/core/SkRecording.cpp
@@ -20,7 +20,7 @@
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)
diff --git a/tests/RecordDrawTest.cpp b/tests/RecordDrawTest.cpp
index 38e1222..2690013 100644
--- a/tests/RecordDrawTest.cpp
+++ b/tests/RecordDrawTest.cpp
@@ -38,7 +38,7 @@
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 @@
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 @@
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);