From: commit-bot@chromium.org Date: Wed, 9 Apr 2014 23:30:28 +0000 (+0000) Subject: SkRecordDraw: skip draw ops when the clip is empty X-Git-Tag: submit/tizen/20180928.044319~8337 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d9ce2be6b24b1c89d13c2edb63c3462b0f5c6aa3;p=platform%2Fupstream%2FlibSkiaSharp.git 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 d38fc4ca23..fb0e4e6898 100644 --- a/dm/DMRecordTask.cpp +++ b/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); diff --git a/gyp/tests.gypi b/gyp/tests.gypi index a3d1e509b7..cf8e8efb8a 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 4f5fecb500..4782344bed 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 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); } } diff --git a/src/record/SkRecorder.cpp b/src/record/SkRecorder.cpp index cba5f74f8f..0867d98846 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, (__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); } diff --git a/src/record/SkRecorder.h b/src/record/SkRecorder.h index bf317f3bef..0f2162f917 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 @@ private: template T* copy(const T[], unsigned count); + const Mode fMode; SkRecord* fRecord; }; diff --git a/tests/RecordCullingTest.cpp b/tests/RecordCullingTest.cpp index c6711c6f67..c6fe10e07b 100644 --- a/tests/RecordCullingTest.cpp +++ b/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()); diff --git a/tests/RecordDrawTest.cpp b/tests/RecordDrawTest.cpp new file mode 100644 index 0000000000..6468e72c9d --- /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 3c7b0082b1..1c116beb12 100644 --- a/tests/RecorderTest.cpp +++ b/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()); diff --git a/tools/bench_record.cpp b/tools/bench_record.cpp index db0ea9b71f..e59744b1dc 100644 --- a/tools/bench_record.cpp +++ b/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); }