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

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();

gyp/record.gyp

@@ -14,6 +14,8 @@
         },
         'sources': [
             '../src/record/SkRecorder.cpp',
+            '../src/record/SkRecordCulling.cpp',
+            '../src/record/SkRecordDraw.cpp',
         ],
     }]
 }

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

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

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

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

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

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

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() {

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);

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

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);
+    AreaSummer summer;
+    record.visit(summer);
+    REPORTER_ASSERT(r, summer.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);
+    // Scale 2x.
+    Stretch stretch;
+    record.mutate(stretch);
+
+    // Now its area should be 100 + 400.
+    record.visit(summer);
+    REPORTER_ASSERT(r, summer.area() == 500);
 }

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));
+    Tally tally;
+    record.visit(tally);
 
-    record.visit(Tally(histogram));
-
-    REPORTER_ASSERT(r, 1 == histogram[SkRecords::DrawRect::kType]);
+    REPORTER_ASSERT(r, 1 == tally.count<SkRecords::DrawRect>());
 }