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

Simplified skip logic by always running clip commands.  No performance difference on bot or silk SKPs.

BUG=skia:2378
R=bungeman@google.com, fmalita@chromium.org, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/258693006

git-svn-id: http://skia.googlecode.com/svn/trunk@14410 2bbb7eff-a529-9590-31e7-b0007b416f81

6 files changed, 201 insertions, 183 deletions

diff --git a/gyp/record.gyp b/gyp/record.gyp
index 0bfd83e0f3..5fa7f818cd 100644
--- a/gyp/record.gyp
+++ b/gyp/record.gyp
@@ -8,6 +8,7 @@
             '../include/config',
             '../include/core',
             '../include/record',
+            '../src/utils',
         ],
         'direct_dependent_settings': {
             'include_dirs': [
diff --git a/src/record/SkRecordDraw.cpp b/src/record/SkRecordDraw.cpp
index 21b2c7a82b..74549db66f 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>
+    SK_WHEN(SkRecords::IsDraw<T>, bool) skip(const T&) { return fClipEmpty; }
+
+    template <typename T>
+    SK_WHEN(!SkRecords::IsDraw<T>, bool) skip(const 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>
+    SK_WHEN(ChangesClip<T>, void) updateClip() { fClipEmpty = fCanvas->isClipEmpty(); }
+    template <typename T>
+    SK_WHEN(!ChangesClip<T>, void) updateClip() {}
+
     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
 
diff --git a/src/record/SkRecordOpts.cpp b/src/record/SkRecordOpts.cpp
index 4b35b33a3c..5b537de040 100644
--- a/src/record/SkRecordOpts.cpp
+++ b/src/record/SkRecordOpts.cpp
@@ -7,6 +7,7 @@
 
 #include "SkRecordOpts.h"
 
+#include "SkRecordTraits.h"
 #include "SkRecords.h"
 #include "SkTDArray.h"
 
@@ -40,9 +41,28 @@ public:
     explicit SaveRestoreNooper(SkRecord* record)
         : Common(record), fSave(kInactive), fChanged(false) {}
 
-    // Most drawing commands reset to inactive state without nooping anything.
+    // Drawing commands reset state to inactive without nooping.
     template <typename T>
-    void operator()(T*) { fSave = kInactive; }
+    SK_WHEN(SkRecords::IsDraw<T>, void) operator()(T*) { fSave = kInactive; }
+
+    // Most non-drawing commands can be ignored.
+    template <typename T>
+    SK_WHEN(!SkRecords::IsDraw<T>, void) operator()(T*) {}
+
+    void operator()(SkRecords::Save* r) {
+        fSave = SkCanvas::kMatrixClip_SaveFlag == r->flags ? this->index() : kInactive;
+    }
+
+    void operator()(SkRecords::Restore* r) {
+        if (fSave != kInactive) {
+            // Remove everything between the save and restore, inclusive on both sides.
+            fChanged = true;
+            for (unsigned i = fSave; i <= this->index(); i++) {
+                fRecord->replace<SkRecords::NoOp>(i);
+            }
+            fSave = kInactive;
+        }
+    }
 
     bool changed() const { return fChanged; }
 
@@ -52,39 +72,6 @@ private:
     bool fChanged;
 };
 
-// If the command doesn't draw anything, that doesn't reset the state back to inactive.
-// TODO(mtklein): do this with some sort of template-based trait mechanism instead of macros
-#define DOESNT_DRAW(T) template <> void SaveRestoreNooper::operator()(SkRecords::T*) {}
-DOESNT_DRAW(NoOp)
-DOESNT_DRAW(Concat)
-DOESNT_DRAW(SetMatrix)
-DOESNT_DRAW(ClipRect)
-DOESNT_DRAW(ClipRRect)
-DOESNT_DRAW(ClipPath)
-DOESNT_DRAW(ClipRegion)
-DOESNT_DRAW(PairedPushCull)
-DOESNT_DRAW(PushCull)
-DOESNT_DRAW(PopCull)
-#undef DOESNT_DRAW
-
-template <>
-void SaveRestoreNooper::operator()(SkRecords::Save* r) {
-    fSave = SkCanvas::kMatrixClip_SaveFlag == r->flags ? this->index() : kInactive;
-}
-
-template <>
-void SaveRestoreNooper::operator()(SkRecords::Restore* r) {
-    if (fSave != kInactive) {
-        // Remove everything between the save and restore, inclusive on both sides.
-        fChanged = true;
-        for (unsigned i = fSave; i <= this->index(); i++) {
-            fRecord->replace<SkRecords::NoOp>(i);
-        }
-        fSave = kInactive;
-    }
-}
-
-
 // Tries to replace PushCull with PairedPushCull, which lets us skip to the paired PopCull
 // when the canvas can quickReject the cull rect.
 class CullAnnotator : public Common {
@@ -92,8 +79,24 @@ public:
     explicit CullAnnotator(SkRecord* record) : Common(record) {}
 
     // Do nothing to most ops.
-    template <typename T>
-    void operator()(T*) {}
+    template <typename T> void operator()(T*) {}
+
+    void operator()(SkRecords::PushCull* push) {
+        Pair pair = { this->index(), push };
+        fPushStack.push(pair);
+    }
+
+    void operator()(SkRecords::PopCull* pop) {
+        Pair push = fPushStack.top();
+        fPushStack.pop();
+
+        SkASSERT(this->index() > push.index);
+        unsigned skip = this->index() - push.index;
+
+        SkRecords::Adopted<SkRecords::PushCull> adopted(push.command);
+        SkNEW_PLACEMENT_ARGS(fRecord->replace<SkRecords::PairedPushCull>(push.index, adopted),
+                             SkRecords::PairedPushCull, (&adopted, skip));
+    }
 
 private:
     struct Pair {
@@ -104,68 +107,46 @@ private:
     SkTDArray<Pair> fPushStack;
 };
 
-template <>
-void CullAnnotator::operator()(SkRecords::PushCull* push) {
-    Pair pair = { this->index(), push };
-    fPushStack.push(pair);
-}
-
-template <>
-void CullAnnotator::operator()(SkRecords::PopCull* pop) {
-    Pair push = fPushStack.top();
-    fPushStack.pop();
-
-    SkASSERT(this->index() > push.index);
-    unsigned skip = this->index() - push.index;
-
-    SkRecords::Adopted<SkRecords::PushCull> adopted(push.command);
-    SkNEW_PLACEMENT_ARGS(fRecord->replace<SkRecords::PairedPushCull>(push.index, adopted),
-                         SkRecords::PairedPushCull, (&adopted, skip));
-}
-
 // Replaces DrawPosText with DrawPosTextH when all Y coordinates are equal.
 class StrengthReducer : public Common {
 public:
     explicit StrengthReducer(SkRecord* record) : Common(record) {}
 
     // Do nothing to most ops.
-    template <typename T>
-    void operator()(T*) {}
-};
+    template <typename T> void operator()(T*) {}
 
-template <>
-void StrengthReducer::operator()(SkRecords::DrawPosText* r) {
-    const unsigned points = r->paint.countText(r->text, r->byteLength);
-    if (points == 0) {
-        // No point (ha!).
-        return;
-    }
-
-    const SkScalar firstY = r->pos[0].fY;
-    for (unsigned i = 1; i < points; i++) {
-        if (r->pos[i].fY != firstY) {
-            // Needs the full strength of DrawPosText.
+    void operator()(SkRecords::DrawPosText* r) {
+        const unsigned points = r->paint.countText(r->text, r->byteLength);
+        if (points == 0) {
+            // No point (ha!).
             return;
         }
-    }
-    // All ys are the same.  We can replace DrawPosText with DrawPosTextH.
-
-    // r->pos is points SkPoints, [(x,y),(x,y),(x,y),(x,y), ... ].
-    // We're going to squint and look at that as 2*points SkScalars, [x,y,x,y,x,y,x,y, ...].
-    // Then we'll rearrange things so all the xs are in order up front, clobbering the ys.
-    SK_COMPILE_ASSERT(sizeof(SkPoint) == 2 * sizeof(SkScalar), SquintingIsNotSafe);
-    SkScalar* scalars = &r->pos[0].fX;
-    for (unsigned i = 0; i < 2*points; i += 2) {
-        scalars[i/2] = scalars[i];
-    }
 
-    // Extend lifetime of r to the end of the method so we can copy its parts.
-    SkRecords::Adopted<SkRecords::DrawPosText> adopted(r);
-    SkNEW_PLACEMENT_ARGS(fRecord->replace<SkRecords::DrawPosTextH>(this->index(), adopted),
-                         SkRecords::DrawPosTextH,
-                         (r->text, r->byteLength, scalars, firstY, r->paint));
-}
+        const SkScalar firstY = r->pos[0].fY;
+        for (unsigned i = 1; i < points; i++) {
+            if (r->pos[i].fY != firstY) {
+                // Needs the full strength of DrawPosText.
+                return;
+            }
+        }
+        // All ys are the same.  We can replace DrawPosText with DrawPosTextH.
+
+        // r->pos is points SkPoints, [(x,y),(x,y),(x,y),(x,y), ... ].
+        // We're going to squint and look at that as 2*points SkScalars, [x,y,x,y,x,y,x,y, ...].
+        // Then we'll rearrange things so all the xs are in order up front, clobbering the ys.
+        SK_COMPILE_ASSERT(sizeof(SkPoint) == 2 * sizeof(SkScalar), SquintingIsNotSafe);
+        SkScalar* scalars = &r->pos[0].fX;
+        for (unsigned i = 0; i < 2*points; i += 2) {
+            scalars[i/2] = scalars[i];
+        }
 
+        // Extend lifetime of r to the end of the method so we can copy its parts.
+        SkRecords::Adopted<SkRecords::DrawPosText> adopted(r);
+        SkNEW_PLACEMENT_ARGS(fRecord->replace<SkRecords::DrawPosTextH>(this->index(), adopted),
+                             SkRecords::DrawPosTextH,
+                             (r->text, r->byteLength, scalars, firstY, r->paint));
+    }
+};
 
 // Tries to replace DrawPosTextH with BoundedDrawPosTextH, which knows conservative upper and lower
 // bounds to use with SkCanvas::quickRejectY.
@@ -174,37 +155,37 @@ public:
     explicit TextBounder(SkRecord* record) : Common(record) {}
 
     // Do nothing to most ops.
-    template <typename T>
-    void operator()(T*) {}
-};
+    template <typename T> void operator()(T*) {}
 
-template <>
-void TextBounder::operator()(SkRecords::DrawPosTextH* r) {
-    // If we're drawing vertical text, none of the checks we're about to do make any sense.
-    // We'll need to call SkPaint::computeFastBounds() later, so bail if that's not possible.
-    if (r->paint.isVerticalText() || !r->paint.canComputeFastBounds()) {
-        return;
+    void operator()(SkRecords::DrawPosTextH* r) {
+        // If we're drawing vertical text, none of the checks we're about to do make any sense.
+        // We'll need to call SkPaint::computeFastBounds() later, so bail if that's not possible.
+        if (r->paint.isVerticalText() || !r->paint.canComputeFastBounds()) {
+            return;
+        }
+
+        // Rather than checking the top and bottom font metrics, we guess.  Actually looking up the
+        // top and bottom metrics is slow, and this overapproximation should be good enough.
+        const SkScalar buffer = r->paint.getTextSize() * 1.5f;
+        SkDEBUGCODE(SkPaint::FontMetrics metrics;)
+        SkDEBUGCODE(r->paint.getFontMetrics(&metrics);)
+        SkASSERT(-buffer <= metrics.fTop);
+        SkASSERT(+buffer >= metrics.fBottom);
+
+        // Let the paint adjust the text bounds.  We don't care about left and right here, so we use
+        // 0 and 1 respectively just so the bounds rectangle isn't empty.
+        SkRect bounds;
+        bounds.set(0, r->y - buffer, SK_Scalar1, r->y + buffer);
+        SkRect adjusted = r->paint.computeFastBounds(bounds, &bounds);
+
+        SkRecords::Adopted<SkRecords::DrawPosTextH> adopted(r);
+        SkNEW_PLACEMENT_ARGS(
+                fRecord->replace<SkRecords::BoundedDrawPosTextH>(this->index(), adopted),
+                SkRecords::BoundedDrawPosTextH,
+                (&adopted, adjusted.fTop, adjusted.fBottom));
     }
+};
 
-    // Rather than checking the top and bottom font metrics, we guess.  Actually looking up the
-    // top and bottom metrics is slow, and this overapproximation should be good enough.
-    const SkScalar buffer = r->paint.getTextSize() * 1.5f;
-    SkDEBUGCODE(SkPaint::FontMetrics metrics;)
-    SkDEBUGCODE(r->paint.getFontMetrics(&metrics);)
-    SkASSERT(-buffer <= metrics.fTop);
-    SkASSERT(+buffer >= metrics.fBottom);
-
-    // Let the paint adjust the text bounds.  We don't care about left and right here, so we use
-    // 0 and 1 respectively just so the bounds rectangle isn't empty.
-    SkRect bounds;
-    bounds.set(0, r->y - buffer, SK_Scalar1, r->y + buffer);
-    SkRect adjusted = r->paint.computeFastBounds(bounds, &bounds);
-
-    SkRecords::Adopted<SkRecords::DrawPosTextH> adopted(r);
-    SkNEW_PLACEMENT_ARGS(fRecord->replace<SkRecords::BoundedDrawPosTextH>(this->index(), adopted),
-                         SkRecords::BoundedDrawPosTextH,
-                         (&adopted, adjusted.fTop, adjusted.fBottom));
-}
 
 template <typename Pass>
 static void run_pass(Pass& pass, SkRecord* record) {
diff --git a/src/record/SkRecordTraits.h b/src/record/SkRecordTraits.h
new file mode 100644
index 0000000000..570a717e92
--- /dev/null
+++ b/src/record/SkRecordTraits.h
@@ -0,0 +1,31 @@
+#include "SkRecords.h"
+#include "SkTLogic.h"
+
+// Type traits that are useful for working with SkRecords.
+
+namespace SkRecords {
+
+namespace {
+
+// Abstracts away whether the T is optional or not.
+template <typename T> const T* as_ptr(const SkRecords::Optional<T>& x) { return x; }
+template <typename T> const T* as_ptr(const T& x) { return &x; }
+
+}  // namespace
+
+// Gets the paint from any command that may have one.
+template <typename Command> const SkPaint* GetPaint(const Command& x) { return as_ptr(x.paint); }
+
+// Have a paint?  You are a draw command!
+template <typename Command> struct IsDraw {
+    SK_CREATE_MEMBER_DETECTOR(paint);
+    static const bool value = HasMember_paint<Command>::value;
+};
+
+// Have a clip op?  You are a clip command.
+template <typename Command> struct IsClip {
+    SK_CREATE_MEMBER_DETECTOR(op);
+    static const bool value = HasMember_op<Command>::value;
+};
+
+}  // namespace SkRecords
diff --git a/src/utils/SkTLogic.h b/src/utils/SkTLogic.h
index bf9ee1ab25..62952ad13c 100644
--- a/src/utils/SkTLogic.h
+++ b/src/utils/SkTLogic.h
@@ -58,4 +58,35 @@ struct SkTMux {
                               typename SkTIf<b, B, Neither>::type>::type type;
 };
 
+/** SkTEnableIf_c::type = (condition) ? T : [does not exist]; */
+template <bool condition, class T = void> struct SkTEnableIf_c { };
+template <class T> struct SkTEnableIf_c<true, T> {
+    typedef T type;
+};
+
+/** SkTEnableIf::type = (Condition::value) ? T : [does not exist]; */
+template <class Condition, class T = void> struct SkTEnableIf
+    : public SkTEnableIf_c<static_cast<bool>(Condition::value), T> { };
+
+/** Use as a return type to enable a function only when cond_type::value is true,
+ *  like C++14's std::enable_if_t.  E.g.  (N.B. this is a dumb example.)
+ *  SK_WHEN(SkTrue, int) f(void* ptr) { return 1; }
+ *  SK_WHEN(!SkTrue, int) f(void* ptr) { return 2; }
+ */
+#define SK_WHEN(cond_prefix, T) typename SkTEnableIf_c<cond_prefix::value, T>::type
+
+// See http://en.wikibooks.org/wiki/More_C++_Idioms/Member_Detector
+#define SK_CREATE_MEMBER_DETECTOR(member)                                           \
+template <typename T>                                                               \
+class HasMember_##member {                                                          \
+    struct Fallback { int member; };                                                \
+    struct Derived : T, Fallback {};                                                \
+    template <typename U, U> struct Check;                                          \
+    template <typename U> static uint8_t func(Check<int Fallback::*, &U::member>*); \
+    template <typename U> static uint16_t func(...);                                \
+public:                                                                             \
+    typedef HasMember_##member type;                                                \
+    static const bool value = sizeof(func<Derived>(NULL)) == sizeof(uint16_t);      \
+}
+
 #endif
diff --git a/tests/RecordDrawTest.cpp b/tests/RecordDrawTest.cpp
index 4f24c92fe5..8c6635489e 100644
--- a/tests/RecordDrawTest.cpp
+++ b/tests/RecordDrawTest.cpp
@@ -50,14 +50,13 @@ DEF_TEST(RecordDraw_Clipping, r) {
     SkRecord record;
     SkRecorder recorder(SkRecorder::kWriteOnly_Mode, &record, W, H);
 
-    // 9 draw commands.
+    // 8 draw commands.
     recorder.save();
         recorder.clipRect(SkRect::MakeLTRB(0, 0, 100, 100));
         recorder.drawRect(SkRect::MakeLTRB(20, 20, 40, 40), SkPaint());
         recorder.save();
-            // This first clipRect makes the clip empty, so the next two commands do nothing.
+            // This clipRect makes the clip empty, so the next command does nothing.
             recorder.clipRect(SkRect::MakeLTRB(200, 200, 300, 300));
-            recorder.clipRect(SkRect::MakeLTRB(210, 210, 250, 250));              // Skipped
             recorder.drawRect(SkRect::MakeLTRB(220, 220, 240, 240), SkPaint());   // Skipped
         recorder.restore();
     recorder.restore();
@@ -67,6 +66,6 @@ DEF_TEST(RecordDraw_Clipping, r) {
     SkRecorder rerecorder(SkRecorder::kReadWrite_Mode, &rerecord, W, H);
     SkRecordDraw(record, &rerecorder);
 
-    // All commands except the two marked // Skipped above will be preserved.
+    // All commands except the one marked // Skipped will be preserved.
     REPORTER_ASSERT(r, 7 == rerecord.count());
 }