diff options
author | commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> | 2014-04-08 23:31:35 +0000 |
---|---|---|
committer | commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> | 2014-04-08 23:31:35 +0000 |
commit | 506db0b7d2905c6bedba9fc5d4aeaf231a9a34ea (patch) | |
tree | 77d73de439e9102d45891be433c5556ee23ce74c | |
parent | 9ffa52d98fc216b6766f33ffd0d9f1c3a1acdb2f (diff) |
SkRecord: make culling work if SkRecordAnnotateCullingPairs is called.
- Allow stateful functors; allow visit()/mutate() at a given index; add count().
- Annotate cull push/pop pairs on the PushCull records. (tested)
- Use those annotations to skip ahead in SkRecordDraw. (not yet tested beyond dm --skr)
- Make SkRecordDraw a function, move its implementation to a .cpp.
BUG=skia:2378
R=fmalita@chromium.org, mtklein@google.com
Author: mtklein@chromium.org
Review URL: https://codereview.chromium.org/229523002
git-svn-id: http://skia.googlecode.com/svn/trunk@14101 2bbb7eff-a529-9590-31e7-b0007b416f81
-rw-r--r-- | dm/DMRecordTask.cpp | 3 | ||||
-rw-r--r-- | gyp/record.gyp | 2 | ||||
-rw-r--r-- | gyp/tests.gypi | 1 | ||||
-rw-r--r-- | src/record/SkRecord.h | 42 | ||||
-rw-r--r-- | src/record/SkRecordCulling.cpp | 38 | ||||
-rw-r--r-- | src/record/SkRecordCulling.h | 9 | ||||
-rw-r--r-- | src/record/SkRecordDraw.cpp | 78 | ||||
-rw-r--r-- | src/record/SkRecordDraw.h | 59 | ||||
-rw-r--r-- | src/record/SkRecorder.cpp | 2 | ||||
-rw-r--r-- | src/record/SkRecords.h | 3 | ||||
-rw-r--r-- | tests/RecordCullingTest.cpp | 41 | ||||
-rw-r--r-- | tests/RecordTest.cpp | 30 | ||||
-rw-r--r-- | tests/RecorderTest.cpp | 22 |
13 files changed, 233 insertions, 97 deletions
diff --git a/dm/DMRecordTask.cpp b/dm/DMRecordTask.cpp index 0109a41d8f..d38fc4ca23 100644 --- a/dm/DMRecordTask.cpp +++ b/dm/DMRecordTask.cpp @@ -27,7 +27,8 @@ void RecordTask::draw() { SkBitmap bitmap; SetupBitmap(fReference.colorType(), fGM.get(), &bitmap); SkCanvas target(bitmap); - record.visit(SkRecordDraw(&target)); + + SkRecordDraw(record, &target); if (!BitmapsEqual(bitmap, fReference)) { this->fail(); diff --git a/gyp/record.gyp b/gyp/record.gyp index bcc42229ed..ae80f7c885 100644 --- a/gyp/record.gyp +++ b/gyp/record.gyp @@ -14,6 +14,8 @@ }, 'sources': [ '../src/record/SkRecorder.cpp', + '../src/record/SkRecordCulling.cpp', + '../src/record/SkRecordDraw.cpp', ], }] } diff --git a/gyp/tests.gypi b/gyp/tests.gypi index 02aed738b5..a3d1e509b7 100644 --- a/gyp/tests.gypi +++ b/gyp/tests.gypi @@ -135,6 +135,7 @@ '../tests/ReadPixelsTest.cpp', '../tests/ReadWriteAlphaTest.cpp', '../tests/Reader32Test.cpp', + '../tests/RecordCullingTest.cpp', '../tests/RecordTest.cpp', '../tests/RecorderTest.cpp', '../tests/RefCntTest.cpp', diff --git a/src/record/SkRecord.h b/src/record/SkRecord.h index 4013874677..0fe79189d4 100644 --- a/src/record/SkRecord.h +++ b/src/record/SkRecord.h @@ -21,33 +21,49 @@ class SkRecord : SkNoncopyable { public: SkRecord(size_t chunkBytes = 4096, unsigned firstReserveCount = 64 / sizeof(void*)) : fAlloc(chunkBytes), fCount(0), fReserved(0), kFirstReserveCount(firstReserveCount) {} - ~SkRecord() { this->mutate(Destroyer()); } + + ~SkRecord() { + Destroyer destroyer; + this->mutate(destroyer); + } + + unsigned count() const { return fCount; } // Accepts a visitor functor with this interface: // template <typename T> - // void operator()()(const T& record) { ... } + // void operator()(const T& record) { ... } // This operator() must be defined for at least all SkRecords::*; your compiler will help you // get this right. - // - // f will be called on each recorded canvas call in the order they were append()ed. template <typename F> - void visit(F f) const { + void visit(unsigned i, F& f) const { + SkASSERT(i < this->count()); + fRecords[i].visit(fTypes[i], f); + } + + // As above. f will be called on each recorded canvas call in the order they were append()ed. + template <typename F> + void visit(F& f) const { for (unsigned i = 0; i < fCount; i++) { - fRecords[i].visit(fTypes[i], f); + this->visit(i, f); } } // Accepts a visitor functor with this interface: // template <typename T> - // void operator()()(T* record) { ... } + // void operator()(T* record) { ... } // This operator() must be defined for at least all SkRecords::*; again, your compiler will help // you get this right. - // - // f will be called on each recorded canvas call in the order they were append()ed. template <typename F> - void mutate(F f) { + void mutate(unsigned i, F& f) { + SkASSERT(i < this->count()); + fRecords[i].mutate(fTypes[i], f); + } + + // As above. f will be called on each recorded canvas call in the order they were append()ed. + template <typename F> + void mutate(F& f) { for (unsigned i = 0; i < fCount; i++) { - fRecords[i].mutate(fTypes[i], f); + this->mutate(i, f); } } @@ -154,7 +170,7 @@ private: // Visit this record with functor F (see public API above) assuming the record we're // pointing to has this type. template <typename F> - void visit(Type8 type, F f) const { + void visit(Type8 type, F& f) const { #define CASE(T) case SkRecords::T##_Type: return f(*this->ptr<SkRecords::T>()); switch(type) { SK_RECORD_TYPES(CASE) } #undef CASE @@ -163,7 +179,7 @@ private: // Mutate this record with functor F (see public API above) assuming the record we're // pointing to has this type. template <typename F> - void mutate(Type8 type, F f) { + void mutate(Type8 type, F& f) { #define CASE(T) case SkRecords::T##_Type: return f(this->ptr<SkRecords::T>()); switch(type) { SK_RECORD_TYPES(CASE) } #undef CASE diff --git a/src/record/SkRecordCulling.cpp b/src/record/SkRecordCulling.cpp new file mode 100644 index 0000000000..e866bd82cb --- /dev/null +++ b/src/record/SkRecordCulling.cpp @@ -0,0 +1,38 @@ +#include "SkRecordCulling.h" + +#include "SkRecords.h" +#include "SkTDArray.h" + +namespace { + +struct Annotator { + unsigned index; + SkTDArray<SkRecords::PushCull*> pushStack; + + // Do nothing to most record types. + template <typename T> void operator()(T*) {} +}; + +template <> void Annotator::operator()(SkRecords::PushCull* push) { + // Store the push's index for now. We'll calculate the offset using this in the paired pop. + push->popOffset = index; + pushStack.push(push); +} + +template <> void Annotator::operator()(SkRecords::PopCull* pop) { + SkRecords::PushCull* push = pushStack.top(); + pushStack.pop(); + + SkASSERT(index > push->popOffset); // push->popOffset holds the index of the push. + push->popOffset = index - push->popOffset; // Now it's the offset between push and pop. +} + +} // namespace + +void SkRecordAnnotateCullingPairs(SkRecord* record) { + Annotator annotator; + + for (annotator.index = 0; annotator.index < record->count(); annotator.index++) { + record->mutate(annotator.index, annotator); + } +} diff --git a/src/record/SkRecordCulling.h b/src/record/SkRecordCulling.h new file mode 100644 index 0000000000..b8ed88c637 --- /dev/null +++ b/src/record/SkRecordCulling.h @@ -0,0 +1,9 @@ +#ifndef SkRecordCulling_DEFINED +#define SkRecordCulling_DEFINED + +#include "SkRecord.h" + +// Annotates PushCull records in record with the relative offset of their paired PopCull. +void SkRecordAnnotateCullingPairs(SkRecord* record); + +#endif//SkRecordCulling_DEFINED diff --git a/src/record/SkRecordDraw.cpp b/src/record/SkRecordDraw.cpp new file mode 100644 index 0000000000..66b48a3559 --- /dev/null +++ b/src/record/SkRecordDraw.cpp @@ -0,0 +1,78 @@ +#include "SkRecordDraw.h" + +namespace { + +// This is an SkRecord visitor that will draw that SkRecord to an SkCanvas. +struct Draw { + unsigned index; + SkCanvas* canvas; + + // No base case, so we'll be compile-time checked that we implemented all possibilities below. + template <typename T> void operator()(const T&); +}; + +template <> void Draw::operator()(const SkRecords::PushCull& pushCull) { + if (pushCull.popOffset != SkRecords::kUnsetPopOffset && + canvas->quickReject(pushCull.rect)) { + // We skip to the popCull, then the loop moves us just beyond it. + index += pushCull.popOffset; + } else { + canvas->pushCull(pushCull.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(); } + +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); +} +#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); + } +} + + diff --git a/src/record/SkRecordDraw.h b/src/record/SkRecordDraw.h index 042147df99..684cc1b2e5 100644 --- a/src/record/SkRecordDraw.h +++ b/src/record/SkRecordDraw.h @@ -2,64 +2,9 @@ #define SkRecordDraw_DEFINED #include "SkRecord.h" -#include "SkRecords.h" #include "SkCanvas.h" -// This is an SkRecord visitor that will draw that SkRecord to an SkCanvas. - -struct SkRecordDraw { - explicit SkRecordDraw(SkCanvas* canvas) : canvas(canvas) {} - - // No base case, so we'll be compile-time checked that we implemented all possibilities below. - template <typename T> void operator()(const T& record); - - SkCanvas* canvas; -}; - -// Nothing fancy here. -// The structs in SkRecord are completely isomorphic to their corresponding SkCanvas calls. - -#define CASE(T) template <> void SkRecordDraw::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(PushCull) { canvas->pushCull(r.rect); } -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); -} - -#undef CASE +// Draw an SkRecord into an SkCanvas. +void SkRecordDraw(const SkRecord&, SkCanvas*); #endif//SkRecordDraw_DEFINED diff --git a/src/record/SkRecorder.cpp b/src/record/SkRecorder.cpp index fabb4be861..cba5f74f8f 100644 --- a/src/record/SkRecorder.cpp +++ b/src/record/SkRecorder.cpp @@ -196,7 +196,7 @@ void SkRecorder::willRestore() { } void SkRecorder::onPushCull(const SkRect& rect) { - APPEND(PushCull, rect); + APPEND(PushCull, rect, SkRecords::kUnsetPopOffset); } void SkRecorder::onPopCull() { diff --git a/src/record/SkRecords.h b/src/record/SkRecords.h index 17a8e849ad..961728af41 100644 --- a/src/record/SkRecords.h +++ b/src/record/SkRecords.h @@ -127,7 +127,8 @@ RECORD0(Restore); RECORD1(Save, SkCanvas::SaveFlags, flags); RECORD3(SaveLayer, SkRect*, bounds, SkPaint*, paint, SkCanvas::SaveFlags, flags); -RECORD1(PushCull, SkRect, rect); +static const unsigned kUnsetPopOffset = 0; +RECORD2(PushCull, SkRect, rect, unsigned, popOffset); RECORD0(PopCull); RECORD1(Concat, SkMatrix, matrix); diff --git a/tests/RecordCullingTest.cpp b/tests/RecordCullingTest.cpp new file mode 100644 index 0000000000..c6711c6f67 --- /dev/null +++ b/tests/RecordCullingTest.cpp @@ -0,0 +1,41 @@ +#include "Test.h" + +#include "SkRecord.h" +#include "SkRecordCulling.h" +#include "SkRecorder.h" +#include "SkRecords.h" + +struct PushCullScanner { + template <typename T> void operator()(const T&) {} + + SkTDArray<unsigned> fPopOffsets; +}; + +template <> void PushCullScanner::operator()(const SkRecords::PushCull& record) { + *fPopOffsets.append() = record.popOffset; +} + + +DEF_TEST(RecordCulling, r) { + SkRecord record; + SkRecorder recorder(&record, 1920, 1080); + + recorder.drawRect(SkRect::MakeWH(1000, 10000), SkPaint()); + + 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(); + + SkRecordAnnotateCullingPairs(&record); + + PushCullScanner scan; + record.visit(scan); + + REPORTER_ASSERT(r, 2 == scan.fPopOffsets.count()); + REPORTER_ASSERT(r, 6 == scan.fPopOffsets[0]); + REPORTER_ASSERT(r, 2 == scan.fPopOffsets[1]); +} diff --git a/tests/RecordTest.cpp b/tests/RecordTest.cpp index 1214b1a5de..58e5de106c 100644 --- a/tests/RecordTest.cpp +++ b/tests/RecordTest.cpp @@ -3,18 +3,20 @@ #include "SkRecord.h" #include "SkRecords.h" -// Adds the area of any DrawRect command it sees into area. +// Sums the area of any DrawRect command it sees. class AreaSummer { public: - explicit AreaSummer(int* area) : fArea(area) {} + AreaSummer() : fArea(0) {} template <typename T> void operator()(const T&) { } + int area() const { return fArea; } + private: - int* fArea; + int fArea; }; template <> void AreaSummer::operator()(const SkRecords::DrawRect& record) { - *fArea += (int) (record.rect.width() * record.rect.height()); + fArea += (int) (record.rect.width() * record.rect.height()); } // Scales out the bottom-right corner of any DrawRect command it sees by 2x. @@ -36,13 +38,15 @@ DEF_TEST(Record, r) { SkNEW_PLACEMENT_ARGS(record.append<SkRecords::DrawRect>(), SkRecords::DrawRect, (rect, paint)); // Its area should be 100. - int area = 0; - record.visit(AreaSummer(&area)); - REPORTER_ASSERT(r, area == 100); - - // Scale 2x. Now it's area should be 400. - record.mutate(Stretch()); - area = 0; - record.visit(AreaSummer(&area)); - REPORTER_ASSERT(r, area == 400); + AreaSummer summer; + record.visit(summer); + REPORTER_ASSERT(r, summer.area() == 100); + + // Scale 2x. + Stretch stretch; + record.mutate(stretch); + + // Now its area should be 100 + 400. + record.visit(summer); + REPORTER_ASSERT(r, summer.area() == 500); } diff --git a/tests/RecorderTest.cpp b/tests/RecorderTest.cpp index 4f8e357533..3c7b0082b1 100644 --- a/tests/RecorderTest.cpp +++ b/tests/RecorderTest.cpp @@ -8,17 +8,19 @@ static const int kRecordTypes = SK_RECORD_TYPES(COUNT); #undef COUNT -// Tallies the types of commands it sees into histogram. +// Tallies the types of commands it sees into a histogram. class Tally { public: - explicit Tally(int histogram[kRecordTypes]) : fHistogram(histogram) {} + Tally() { sk_bzero(&fHistogram, sizeof(fHistogram)); } - template <typename T> void operator()(const T&) { - ++fHistogram[T::kType]; - } + template <typename T> + void operator()(const T&) { ++fHistogram[T::kType]; } + + template <typename T> + int count() const { return fHistogram[T::kType]; } private: - int* fHistogram; + int fHistogram[kRecordTypes]; }; DEF_TEST(Recorder, r) { @@ -27,10 +29,8 @@ DEF_TEST(Recorder, r) { recorder.drawRect(SkRect::MakeWH(10, 10), SkPaint()); - int histogram[kRecordTypes]; - sk_bzero(&histogram, sizeof(histogram)); - - record.visit(Tally(histogram)); + Tally tally; + record.visit(tally); - REPORTER_ASSERT(r, 1 == histogram[SkRecords::DrawRect::kType]); + REPORTER_ASSERT(r, 1 == tally.count<SkRecords::DrawRect>()); } |