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
diff --git a/gyp/record.gyp b/gyp/record.gyp
index 0bfd83e..5fa7f81 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 21b2c7a..74549db 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 @@
}
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 @@
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 4b35b33..5b537de 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 @@
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 @@
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 @@
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 @@
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;
}
+
+ 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));
}
- // 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 @@
explicit TextBounder(SkRecord* record) : Common(record) {}
// Do nothing to most ops.
- template <typename T>
- void operator()(T*) {}
+ template <typename T> void operator()(T*) {}
+
+ 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));
+ }
};
-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;
- }
-
- // 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 0000000..570a717
--- /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 bf9ee1a..62952ad 100644
--- a/src/utils/SkTLogic.h
+++ b/src/utils/SkTLogic.h
@@ -58,4 +58,35 @@
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 4f24c92..8c66354 100644
--- a/tests/RecordDrawTest.cpp
+++ b/tests/RecordDrawTest.cpp
@@ -50,14 +50,13 @@
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 @@
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());
}