Start using type traits in src/record instead of macros.

src/record/SkRecordDraw.cpp

Index: src/record/SkRecordDraw.cpp
diff --git a/src/record/SkRecordDraw.cpp b/src/record/SkRecordDraw.cpp
index 21b2c7a82b33a630e4769554dfa444cc05c13890..0df0b05a6df3498ef9d76052e58e5267845f4cc1 100644
--- a/src/record/SkRecordDraw.cpp
+++ b/src/record/SkRecordDraw.cpp
@@ -7,8 +7,16 @@
 
 #include "SkRecordDraw.h"
 
+#include "SkRecordTraits.h"
+
 namespace {
 
+// All clip commands, Restore, and SaveLayer may change the clip.
+template <typename T> struct ChangesClip { static const bool value = SkRecords::IsClip<T>::value; };
+template <> struct ChangesClip<SkRecords::Restore> { static const bool value = true; };
+template <> struct ChangesClip<SkRecords::SaveLayer> { static const bool value = true; };
+
+
 // This is an SkRecord visitor that will draw that SkRecord to an SkCanvas.
 class Draw : SkNoncopyable {
 public:
@@ -25,60 +33,44 @@ public:
     }
 
 private:
-    // Return true if we can skip this command, false if not.
-    // Update fIndex here directly to skip more than just this one command.
-    template <typename T> bool skip(const T&) {
-        // We can skip most commands if the clip is empty.  Exceptions are specialized below.
-        return fClipEmpty;
-    }
-
     // No base case, so we'll be compile-time checked that we implemented all possibilities below.
     template <typename T> void draw(const T&);
 
-    // Update fClipEmpty if necessary.
-    template <typename T> void updateClip() {
-        // Most commands don't change the clip.  Exceptions are specialized below.
+    // skip() returns true if we can skip this command, false if not.
+    // Update fIndex directly to skip more than just this one command.
+
+    // If we're drawing into an empty clip, we can skip it.  Otherwise, run the command.
+    template <typename T>
+    bool skip(const T&, SK_ENABLE_IF(SkRecords::IsDraw<T>)) { return fClipEmpty; }
+    template <typename T>
+    bool skip(const T&, SK_ENABLE_IF(!SkRecords::IsDraw<T>)) { return false; }
+
+    // Special versions for commands added by optimizations.
+    bool skip(const SkRecords::PairedPushCull& r) {
+        if (fCanvas->quickReject(r.base->rect)) {
+            fIndex += r.skip;
+            return true;
+        }
+        return this->skip(*r.base);
+    }
+
+    bool skip(const SkRecords::BoundedDrawPosTextH& r) {
+        return this->skip(*r.base) || fCanvas->quickRejectY(r.minY, r.maxY);
     }
 
+    // If we might have changed the clip, update it, else do nothing.
+    template <typename T>
+    void updateClip(SK_ENABLE_IF(ChangesClip<T>)) { fClipEmpty = fCanvas->isClipEmpty(); }

[f(malita), 2014/04/28 14:20:44]

I was thinking we could defer updating the clip until needed (as opposed to on
every clip-changing cmd), but looking at SkCanvas::isClipEmpty() it seems really
cheap - plus in practice I think we rarely tweak the clip repeatedly with no
intervening draws. So probably not worth pursuing, just making a note.

[mtklein, 2014/04/28 15:47:05]

Figure it's cheap enough that it's pointless to track the extra state myself?  I
could just call fCanvas->isClipEmpty() there in skip()?  I'll measure this as a
follow up.

+    template <typename T>
+    void updateClip(SK_ENABLE_IF(!ChangesClip<T>)) {}
+
+
     SkCanvas* fCanvas;
     unsigned fIndex;
     bool fClipEmpty;
 };
 
-// TODO(mtklein): do this specialization with template traits instead of macros
-
-// These commands may change the clip.
-#define UPDATE_CLIP(T) template <> void Draw::updateClip<SkRecords::T>() \
-    { fClipEmpty = fCanvas->isClipEmpty(); }
-UPDATE_CLIP(Restore);
-UPDATE_CLIP(SaveLayer);
-UPDATE_CLIP(ClipPath);
-UPDATE_CLIP(ClipRRect);
-UPDATE_CLIP(ClipRect);
-UPDATE_CLIP(ClipRegion);
-#undef UPDATE_CLIP
-
-// These commands must always run.
-#define SKIP(T) template <> bool Draw::skip(const SkRecords::T&) { return false; }
-SKIP(Restore);
-SKIP(Save);
-SKIP(SaveLayer);
-SKIP(Clear);
-SKIP(PushCull);
-SKIP(PopCull);
-#undef SKIP
-
-// We can skip these commands if they're intersecting with a clip that's already empty.
-#define SKIP(T) template <> bool Draw::skip(const SkRecords::T& r) \
-    { return fClipEmpty && SkRegion::kIntersect_Op == r.op; }
-SKIP(ClipPath);
-SKIP(ClipRRect);
-SKIP(ClipRect);
-SKIP(ClipRegion);
-#undef SKIP
-
-// NoOps can always be skipped and draw nothing.
-template <> bool Draw::skip(const SkRecords::NoOp&) { return true; }
+// NoOps draw nothing.
 template <> void Draw::draw(const SkRecords::NoOp&) {}
 
 #define DRAW(T, call) template <> void Draw::draw(const SkRecords::T& r) { fCanvas->call; }
@@ -116,25 +108,8 @@ DRAW(DrawVertices, drawVertices(r.vmode, r.vertexCount, r.vertices, r.texs, r.co
                                 r.xmode.get(), r.indices, r.indexCount, r.paint));
 #undef DRAW
 
-// Added by SkRecordAnnotateCullingPairs.
-template <> bool Draw::skip(const SkRecords::PairedPushCull& r) {
-    if (fCanvas->quickReject(r.base->rect)) {
-        fIndex += r.skip;
-        return true;
-    }
-    return false;
-}
-
-// Added by SkRecordBoundDrawPosTextH
-template <> bool Draw::skip(const SkRecords::BoundedDrawPosTextH& r) {
-    return fClipEmpty || fCanvas->quickRejectY(r.minY, r.maxY);
-}
-
-// These draw by proxying to the commands they wrap.  (All the optimization is for skip().)
-#define DRAW(T) template <> void Draw::draw(const SkRecords::T& r) { this->draw(*r.base); }
-DRAW(PairedPushCull);
-DRAW(BoundedDrawPosTextH);
-#undef DRAW
+template <> void Draw::draw(const SkRecords::PairedPushCull& r) { this->draw(*r.base); }
+template <> void Draw::draw(const SkRecords::BoundedDrawPosTextH& r) { this->draw(*r.base); }
 
 }  // namespace