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

dm/DMRecordTask.cpp

@@ -19,7 +19,8 @@ RecordTask::RecordTask(const Task& parent, skiagm::GM* gm, SkBitmap reference)
 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);
 

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',

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.
-
-#define CASE(T) template <> void Draw::operator()(const SkRecords::T& r)
-
-CASE(Restore) { canvas->restore(); }
-CASE(Save) { canvas->save(r.flags); }
-CASE(SaveLayer) { canvas->saveLayer(r.bounds, r.paint, r.flags); }
-
-CASE(PopCull) { canvas->popCull(); }
+// 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
 
-CASE(Concat) { canvas->concat(r.matrix); }
-CASE(SetMatrix) { canvas->setMatrix(r.matrix); }
+// 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(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); }
+// 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(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);
     }
 }

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 @@ void SkRecorder::drawRRect(const SkRRect& rrect, const SkPaint& paint) {
     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::drawVertices(VertexMode vmode,
 
 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::onPopCull() {
 
 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);
 }

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 @@ private:
     template <typename T>
     T* copy(const T[], unsigned count);
 
+    const Mode fMode;
     SkRecord* fRecord;
 };
 

tests/RecordCullingTest.cpp

@@ -18,7 +18,7 @@ template <> void PushCullScanner::operator()(const SkRecords::PushCull& record)
 
 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());
 

tests/RecordDrawTest.cpp

+#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());
+}

tests/RecorderTest.cpp

@@ -25,7 +25,7 @@ private:
 
 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());
 

tools/bench_record.cpp

@@ -79,7 +79,7 @@ static void bench_record(SkPicture* src, const char* name, PictureFactory pictur
     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);
             }