SkRecordDraw: skip draw ops when the clip is empty
- Adds tests for SkRecordDraw's two main features: cull- and clip- based skipping.
- Adds full SkCanvas semantic mode to SkRecorder, so it can be used as a target for SkRecordDraw.
BUG=skia:2378
R=fmalita@chromium.org, mtklein@google.com
Author: mtklein@chromium.org
Review URL: https://codereview.chromium.org/231653002
git-svn-id: http://skia.googlecode.com/svn/trunk@14124 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/dm/DMRecordTask.cpp b/dm/DMRecordTask.cpp
index d38fc4c..fb0e4e6 100644
--- a/dm/DMRecordTask.cpp
+++ b/dm/DMRecordTask.cpp
@@ -19,7 +19,8 @@
void RecordTask::draw() {
// Record the GM into an SkRecord.
SkRecord record;
- SkRecorder canvas(&record, fReference.width(), fReference.height());
+ SkRecorder canvas(SkRecorder::kWriteOnly_Mode, &record,
+ fReference.width(), fReference.height());
canvas.concat(fGM->getInitialTransform());
fGM->draw(&canvas);
diff --git a/gyp/tests.gypi b/gyp/tests.gypi
index a3d1e50..cf8e8ef 100644
--- a/gyp/tests.gypi
+++ b/gyp/tests.gypi
@@ -136,6 +136,7 @@
'../tests/ReadWriteAlphaTest.cpp',
'../tests/Reader32Test.cpp',
'../tests/RecordCullingTest.cpp',
+ '../tests/RecordDrawTest.cpp',
'../tests/RecordTest.cpp',
'../tests/RecorderTest.cpp',
'../tests/RefCntTest.cpp',
diff --git a/src/record/SkRecordDraw.cpp b/src/record/SkRecordDraw.cpp
index 4f5fecb..4782344 100644
--- a/src/record/SkRecordDraw.cpp
+++ b/src/record/SkRecordDraw.cpp
@@ -3,74 +3,84 @@
namespace {
// This is an SkRecord visitor that will draw that SkRecord to an SkCanvas.
-struct Draw {
- unsigned index;
- SkCanvas* canvas;
+class Draw : SkNoncopyable {
+public:
+ explicit Draw(SkCanvas* canvas) : fCanvas(canvas), fIndex(0), fClipEmpty(false) {}
+
+ unsigned index() const { return fIndex; }
+ void next() { ++fIndex; }
// No base case, so we'll be compile-time checked that we implemented all possibilities below.
template <typename T> void operator()(const T&);
+
+private:
+ // Must be called after any potential clip change.
+ void updateClip() { fClipEmpty = fCanvas->isClipEmpty(); }
+
+ SkCanvas* fCanvas;
+ unsigned fIndex;
+ bool fClipEmpty;
};
-template <> void Draw::operator()(const SkRecords::PushCull& pushCull) {
- if (pushCull.popOffset != SkRecords::kUnsetPopOffset &&
- canvas->quickReject(pushCull.rect)) {
+template <> void Draw::operator()(const SkRecords::PushCull& r) {
+ if (r.popOffset != SkRecords::kUnsetPopOffset &&
+ fCanvas->quickReject(r.rect)) {
// We skip to the popCull, then the loop moves us just beyond it.
- index += pushCull.popOffset;
+ fIndex += r.popOffset;
} else {
- canvas->pushCull(pushCull.rect);
+ fCanvas->pushCull(r.rect);
}
}
-// Nothing fancy below here.
+// These commands might change the clip.
+#define CASE(T, call) \
+ template <> void Draw::operator()(const SkRecords::T& r) { fCanvas->call; this->updateClip(); }
+CASE(Restore, restore());
+CASE(SaveLayer, saveLayer(r.bounds, r.paint, r.flags));
+CASE(ClipPath, clipPath(r.path, r.op, r.doAA));
+CASE(ClipRRect, clipRRect(r.rrect, r.op, r.doAA));
+CASE(ClipRect, clipRect(r.rect, r.op, r.doAA));
+CASE(ClipRegion, clipRegion(r.region, r.op));
+#undef CASE
-#define CASE(T) template <> void Draw::operator()(const SkRecords::T& r)
+// Commands which must run regardless of the clip.
+#define CASE(T, call) \
+ template <> void Draw::operator()(const SkRecords::T& r) { fCanvas->call; }
+CASE(Save, save(r.flags));
+CASE(Clear, clear(r.color));
+CASE(PopCull, popCull());
+#undef CASE
-CASE(Restore) { canvas->restore(); }
-CASE(Save) { canvas->save(r.flags); }
-CASE(SaveLayer) { canvas->saveLayer(r.bounds, r.paint, r.flags); }
+// Nothing fancy below here. These commands respect and don't change the clip.
+#define CASE(T, call) \
+ template <> void Draw::operator()(const SkRecords::T& r) { if (!fClipEmpty) fCanvas->call; }
+CASE(Concat, concat(r.matrix));
+CASE(SetMatrix, setMatrix(r.matrix));
-CASE(PopCull) { canvas->popCull(); }
-
-CASE(Concat) { canvas->concat(r.matrix); }
-CASE(SetMatrix) { canvas->setMatrix(r.matrix); }
-
-CASE(ClipPath) { canvas->clipPath(r.path, r.op, r.doAA); }
-CASE(ClipRRect) { canvas->clipRRect(r.rrect, r.op, r.doAA); }
-CASE(ClipRect) { canvas->clipRect(r.rect, r.op, r.doAA); }
-CASE(ClipRegion) { canvas->clipRegion(r.region, r.op); }
-
-CASE(Clear) { canvas->clear(r.color); }
-CASE(DrawBitmap) { canvas->drawBitmap(r.bitmap, r.left, r.top, r.paint); }
-CASE(DrawBitmapMatrix) { canvas->drawBitmapMatrix(r.bitmap, r.matrix, r.paint); }
-CASE(DrawBitmapNine) { canvas->drawBitmapNine(r.bitmap, r.center, r.dst, r.paint); }
-CASE(DrawBitmapRectToRect) {
- canvas->drawBitmapRectToRect(r.bitmap, r.src, r.dst, r.paint, r.flags);
-}
-CASE(DrawDRRect) { canvas->drawDRRect(r.outer, r.inner, r.paint); }
-CASE(DrawOval) { canvas->drawOval(r.oval, r.paint); }
-CASE(DrawPaint) { canvas->drawPaint(r.paint); }
-CASE(DrawPath) { canvas->drawPath(r.path, r.paint); }
-CASE(DrawPoints) { canvas->drawPoints(r.mode, r.count, r.pts, r.paint); }
-CASE(DrawPosText) { canvas->drawPosText(r.text, r.byteLength, r.pos, r.paint); }
-CASE(DrawPosTextH) { canvas->drawPosTextH(r.text, r.byteLength, r.xpos, r.y, r.paint); }
-CASE(DrawRRect) { canvas->drawRRect(r.rrect, r.paint); }
-CASE(DrawRect) { canvas->drawRect(r.rect, r.paint); }
-CASE(DrawSprite) { canvas->drawSprite(r.bitmap, r.left, r.top, r.paint); }
-CASE(DrawText) { canvas->drawText(r.text, r.byteLength, r.x, r.y, r.paint); }
-CASE(DrawTextOnPath) { canvas->drawTextOnPath(r.text, r.byteLength, r.path, r.matrix, r.paint); }
-CASE(DrawVertices) {
- canvas->drawVertices(r.vmode, r.vertexCount, r.vertices, r.texs, r.colors,
- r.xmode.get(), r.indices, r.indexCount, r.paint);
-}
+CASE(DrawBitmap, drawBitmap(r.bitmap, r.left, r.top, r.paint));
+CASE(DrawBitmapMatrix, drawBitmapMatrix(r.bitmap, r.matrix, r.paint));
+CASE(DrawBitmapNine, drawBitmapNine(r.bitmap, r.center, r.dst, r.paint));
+CASE(DrawBitmapRectToRect, drawBitmapRectToRect(r.bitmap, r.src, r.dst, r.paint, r.flags));
+CASE(DrawDRRect, drawDRRect(r.outer, r.inner, r.paint));
+CASE(DrawOval, drawOval(r.oval, r.paint));
+CASE(DrawPaint, drawPaint(r.paint));
+CASE(DrawPath, drawPath(r.path, r.paint));
+CASE(DrawPoints, drawPoints(r.mode, r.count, r.pts, r.paint));
+CASE(DrawPosText, drawPosText(r.text, r.byteLength, r.pos, r.paint));
+CASE(DrawPosTextH, drawPosTextH(r.text, r.byteLength, r.xpos, r.y, r.paint));
+CASE(DrawRRect, drawRRect(r.rrect, r.paint));
+CASE(DrawRect, drawRect(r.rect, r.paint));
+CASE(DrawSprite, drawSprite(r.bitmap, r.left, r.top, r.paint));
+CASE(DrawText, drawText(r.text, r.byteLength, r.x, r.y, r.paint));
+CASE(DrawTextOnPath, drawTextOnPath(r.text, r.byteLength, r.path, r.matrix, r.paint));
+CASE(DrawVertices, drawVertices(r.vmode, r.vertexCount, r.vertices, r.texs, r.colors,
+ r.xmode.get(), r.indices, r.indexCount, r.paint));
#undef CASE
} // namespace
void SkRecordDraw(const SkRecord& record, SkCanvas* canvas) {
- Draw draw;
- draw.canvas = canvas;
-
- for (draw.index = 0; draw.index < record.count(); draw.index++) {
- record.visit(draw.index, draw);
+ for (Draw draw(canvas); draw.index() < record.count(); draw.next()) {
+ record.visit(draw.index(), draw);
}
}
diff --git a/src/record/SkRecorder.cpp b/src/record/SkRecorder.cpp
index cba5f74..0867d98 100644
--- a/src/record/SkRecorder.cpp
+++ b/src/record/SkRecorder.cpp
@@ -2,13 +2,16 @@
#include "SkPicture.h"
// SkCanvas will fail in mysterious ways if it doesn't know the real width and height.
-SkRecorder::SkRecorder(SkRecord* record, int width, int height)
- : SkCanvas(width, height), fRecord(record) {}
+SkRecorder::SkRecorder(SkRecorder::Mode mode, SkRecord* record, int width, int height)
+ : SkCanvas(width, height), fMode(mode), fRecord(record) {}
// To make appending to fRecord a little less verbose.
#define APPEND(T, ...) \
SkNEW_PLACEMENT_ARGS(fRecord->append<SkRecords::T>(), SkRecords::T, (__VA_ARGS__))
+// For methods which must call back into SkCanvas in kReadWrite_Mode.
+#define INHERITED(method, ...) if (fMode == kReadWrite_Mode) this->SkCanvas::method(__VA_ARGS__)
+
// The structs we're creating all copy their constructor arguments. Given the way the SkRecords
// framework works, sometimes they happen to technically be copied twice, which is fine and elided
// into a single copy unless the class has a non-trivial copy constructor. For classes with
@@ -94,6 +97,10 @@
APPEND(DrawRRect, rrect, delay_copy(paint));
}
+void SkRecorder::onDrawDRRect(const SkRRect& outer, const SkRRect& inner, const SkPaint& paint) {
+ APPEND(DrawDRRect, outer, inner, delay_copy(paint));
+}
+
void SkRecorder::drawPath(const SkPath& path, const SkPaint& paint) {
APPEND(DrawPath, delay_copy(path), delay_copy(paint));
}
@@ -182,17 +189,20 @@
void SkRecorder::willSave(SkCanvas::SaveFlags flags) {
APPEND(Save, flags);
+ INHERITED(willSave, flags);
}
SkCanvas::SaveLayerStrategy SkRecorder::willSaveLayer(const SkRect* bounds,
const SkPaint* paint,
SkCanvas::SaveFlags flags) {
APPEND(SaveLayer, this->copy(bounds), this->copy(paint), flags);
+ INHERITED(willSaveLayer, bounds, paint, flags);
return SkCanvas::kNoLayer_SaveLayerStrategy;
}
void SkRecorder::willRestore() {
APPEND(Restore);
+ INHERITED(willRestore);
}
void SkRecorder::onPushCull(const SkRect& rect) {
@@ -205,28 +215,30 @@
void SkRecorder::didConcat(const SkMatrix& matrix) {
APPEND(Concat, matrix);
+ INHERITED(didConcat, matrix);
}
void SkRecorder::didSetMatrix(const SkMatrix& matrix) {
APPEND(SetMatrix, matrix);
-}
-
-void SkRecorder::onDrawDRRect(const SkRRect& outer, const SkRRect& inner, const SkPaint& paint) {
- APPEND(DrawDRRect, outer, inner, delay_copy(paint));
+ INHERITED(didSetMatrix, matrix);
}
void SkRecorder::onClipRect(const SkRect& rect, SkRegion::Op op, ClipEdgeStyle edgeStyle) {
APPEND(ClipRect, rect, op, edgeStyle == kSoft_ClipEdgeStyle);
+ INHERITED(onClipRect, rect, op, edgeStyle);
}
void SkRecorder::onClipRRect(const SkRRect& rrect, SkRegion::Op op, ClipEdgeStyle edgeStyle) {
APPEND(ClipRRect, rrect, op, edgeStyle == kSoft_ClipEdgeStyle);
+ INHERITED(updateClipConservativelyUsingBounds, rrect.getBounds(), op, false);
}
void SkRecorder::onClipPath(const SkPath& path, SkRegion::Op op, ClipEdgeStyle edgeStyle) {
APPEND(ClipPath, delay_copy(path), op, edgeStyle == kSoft_ClipEdgeStyle);
+ INHERITED(updateClipConservativelyUsingBounds, path.getBounds(), op, path.isInverseFillType());
}
void SkRecorder::onClipRegion(const SkRegion& deviceRgn, SkRegion::Op op) {
APPEND(ClipRegion, delay_copy(deviceRgn), op);
+ INHERITED(onClipRegion, deviceRgn, op);
}
diff --git a/src/record/SkRecorder.h b/src/record/SkRecorder.h
index bf317f3..0f2162f 100644
--- a/src/record/SkRecorder.h
+++ b/src/record/SkRecorder.h
@@ -9,8 +9,17 @@
class SkRecorder : public SkCanvas {
public:
+ // SkRecorder can work in two modes:
+ // write-only: only a core subset of SkCanvas operations (save/restore, clip, transform, draw)
+ // are supported, and all of the readback methods on SkCanvas will probably fail or lie.
+ //
+ // read-write: all methods should behave with similar semantics to SkCanvas.
+ //
+ // Write-only averages 10-20% faster, but you can't sensibly inspect the canvas while recording.
+ enum Mode { kWriteOnly_Mode, kReadWrite_Mode };
+
// Does not take ownership of the SkRecord.
- SkRecorder(SkRecord*, int width, int height);
+ SkRecorder(Mode mode, SkRecord*, int width, int height);
void clear(SkColor) SK_OVERRIDE;
void drawPaint(const SkPaint& paint) SK_OVERRIDE;
@@ -95,6 +104,7 @@
template <typename T>
T* copy(const T[], unsigned count);
+ const Mode fMode;
SkRecord* fRecord;
};
diff --git a/tests/RecordCullingTest.cpp b/tests/RecordCullingTest.cpp
index c6711c6..c6fe10e 100644
--- a/tests/RecordCullingTest.cpp
+++ b/tests/RecordCullingTest.cpp
@@ -18,7 +18,7 @@
DEF_TEST(RecordCulling, r) {
SkRecord record;
- SkRecorder recorder(&record, 1920, 1080);
+ SkRecorder recorder(SkRecorder::kWriteOnly_Mode, &record, 1920, 1080);
recorder.drawRect(SkRect::MakeWH(1000, 10000), SkPaint());
diff --git a/tests/RecordDrawTest.cpp b/tests/RecordDrawTest.cpp
new file mode 100644
index 0000000..6468e72
--- /dev/null
+++ b/tests/RecordDrawTest.cpp
@@ -0,0 +1,64 @@
+#include "Test.h"
+
+#include "SkDebugCanvas.h"
+#include "SkRecord.h"
+#include "SkRecordCulling.h"
+#include "SkRecordDraw.h"
+#include "SkRecorder.h"
+#include "SkRecords.h"
+
+static const int W = 1920, H = 1080;
+
+DEF_TEST(RecordDraw_Culling, r) {
+ // Record these 7 drawing commands verbatim.
+ SkRecord record;
+ SkRecorder recorder(SkRecorder::kWriteOnly_Mode, &record, W, H);
+
+ recorder.pushCull(SkRect::MakeWH(100, 100));
+ recorder.drawRect(SkRect::MakeWH(10, 10), SkPaint());
+ recorder.drawRect(SkRect::MakeWH(30, 30), SkPaint());
+ recorder.pushCull(SkRect::MakeWH(5, 5));
+ recorder.drawRect(SkRect::MakeWH(1, 1), SkPaint());
+ recorder.popCull();
+ recorder.popCull();
+
+ // Take a pass over to match up pushCulls and popCulls.
+ SkRecordAnnotateCullingPairs(&record);
+
+ // Rerecord into another SkRecord using full SkCanvas semantics,
+ // tracking clips and allowing SkRecordDraw's quickReject() calls to work.
+ SkRecord rerecord;
+ SkRecorder rerecorder(SkRecorder::kReadWrite_Mode, &rerecord, W, H);
+ // This clip intersects the outer cull, but allows us to quick reject the inner one.
+ rerecorder.clipRect(SkRect::MakeLTRB(20, 20, 200, 200));
+
+ SkRecordDraw(record, &rerecorder);
+
+ // We'll keep the clipRect call from above, and the outer two drawRects, and the push/pop pair.
+ // If culling weren't working, we'd see 8 commands recorded here.
+ REPORTER_ASSERT(r, 5 == rerecord.count());
+}
+
+DEF_TEST(RecordDraw_Clipping, r) {
+ SkRecord record;
+ SkRecorder recorder(SkRecorder::kWriteOnly_Mode, &record, W, H);
+
+ // 8 draw commands.
+ // The inner clipRect makes the clip empty, so the inner drawRect does nothing.
+ recorder.save();
+ recorder.clipRect(SkRect::MakeLTRB(0, 0, 100, 100));
+ recorder.drawRect(SkRect::MakeLTRB(20, 20, 40, 40), SkPaint());
+ recorder.save();
+ recorder.clipRect(SkRect::MakeLTRB(200, 200, 300, 300));
+ recorder.drawRect(SkRect::MakeLTRB(220, 220, 240, 240), SkPaint());
+ recorder.restore();
+ recorder.restore();
+
+ // Same deal as above: we need full SkCanvas semantics for clip skipping to work.
+ SkRecord rerecord;
+ SkRecorder rerecorder(SkRecorder::kReadWrite_Mode, &rerecord, W, H);
+ SkRecordDraw(record, &rerecorder);
+
+ // All commands except the drawRect will be preserved.
+ REPORTER_ASSERT(r, 7 == rerecord.count());
+}
diff --git a/tests/RecorderTest.cpp b/tests/RecorderTest.cpp
index 3c7b008..1c116be 100644
--- a/tests/RecorderTest.cpp
+++ b/tests/RecorderTest.cpp
@@ -25,7 +25,7 @@
DEF_TEST(Recorder, r) {
SkRecord record;
- SkRecorder recorder(&record, 1920, 1080);
+ SkRecorder recorder(SkRecorder::kWriteOnly_Mode, &record, 1920, 1080);
recorder.drawRect(SkRect::MakeWH(10, 10), SkPaint());
diff --git a/tools/bench_record.cpp b/tools/bench_record.cpp
index db0ea9b..e59744b 100644
--- a/tools/bench_record.cpp
+++ b/tools/bench_record.cpp
@@ -79,7 +79,7 @@
for (int i = 0; i < FLAGS_loops; i++) {
if (FLAGS_skr) {
SkRecord record;
- SkRecorder canvas(&record, width, height);
+ SkRecorder canvas(SkRecorder::kWriteOnly_Mode, &record, width, height);
if (NULL != src) {
src->draw(&canvas);
}