Pass arguments to SkRecords structs by const&.

This has the effect of using delay_copy() on every argument,
obviating the need for delay_copy() and, crucially, having
to remember to call delay_copy().

All these constructors are fully inlined, so we'll never pay
a penalty for passing small things by reference... the compiler
can see that and just pass by value.

BUG=skia:

Review URL: https://codereview.chromium.org/1215523004
This commit is contained in:
mtklein 2015-06-30 09:49:49 -07:00 committed by Commit bot
parent 439f23e563
commit c845fa0788
2 changed files with 59 additions and 77 deletions

View File

@ -69,27 +69,6 @@ void SkRecorder::forgetRecord() {
// For methods which must call back into SkCanvas.
#define INHERITED(method, ...) this->SkCanvas::method(__VA_ARGS__)
// The structs we're creating all copy their constructor arguments. Given the way the SkRecords
// framework works, sometimes they happen to technically be copied twice, which is fine and elided
// into a single copy unless the class has a non-trivial copy constructor. For classes with
// non-trivial copy constructors, we skip the first copy (and its destruction) by wrapping the value
// with delay_copy(), forcing the argument to be passed by const&.
//
// This is used below for SkBitmap, SkPaint, SkPath, and SkRegion, which all have non-trivial copy
// constructors and destructors. You'll know you've got a good candidate T if you see ~T() show up
// unexpectedly on a profile of record time. Otherwise don't bother.
template <typename T>
class Reference {
public:
Reference(const T& x) : fX(x) {}
operator const T&() const { return fX; }
private:
const T& fX;
};
template <typename T>
static Reference<T> delay_copy(const T& x) { return Reference<T>(x); }
// Use copy() only for optional arguments, to be copied if present or skipped if not.
// (For most types we just pass by value and let copy constructors do their thing.)
template <typename T>
@ -142,31 +121,31 @@ void SkRecorder::flushMiniRecorder() {
}
void SkRecorder::onDrawPaint(const SkPaint& paint) {
APPEND(DrawPaint, delay_copy(paint));
APPEND(DrawPaint, paint);
}
void SkRecorder::onDrawPoints(PointMode mode,
size_t count,
const SkPoint pts[],
const SkPaint& paint) {
APPEND(DrawPoints, delay_copy(paint), mode, SkToUInt(count), this->copy(pts, count));
APPEND(DrawPoints, paint, mode, SkToUInt(count), this->copy(pts, count));
}
void SkRecorder::onDrawRect(const SkRect& rect, const SkPaint& paint) {
TRY_MINIRECORDER(drawRect, rect, paint);
APPEND(DrawRect, delay_copy(paint), rect);
APPEND(DrawRect, paint, rect);
}
void SkRecorder::onDrawOval(const SkRect& oval, const SkPaint& paint) {
APPEND(DrawOval, delay_copy(paint), oval);
APPEND(DrawOval, paint, oval);
}
void SkRecorder::onDrawRRect(const SkRRect& rrect, const SkPaint& paint) {
APPEND(DrawRRect, delay_copy(paint), rrect);
APPEND(DrawRRect, paint, rrect);
}
void SkRecorder::onDrawDRRect(const SkRRect& outer, const SkRRect& inner, const SkPaint& paint) {
APPEND(DrawDRRect, delay_copy(paint), outer, inner);
APPEND(DrawDRRect, paint, outer, inner);
}
void SkRecorder::onDrawDrawable(SkDrawable* drawable) {
@ -179,14 +158,14 @@ void SkRecorder::onDrawDrawable(SkDrawable* drawable) {
void SkRecorder::onDrawPath(const SkPath& path, const SkPaint& paint) {
TRY_MINIRECORDER(drawPath, path, paint);
APPEND(DrawPath, delay_copy(paint), delay_copy(path));
APPEND(DrawPath, paint, path);
}
void SkRecorder::onDrawBitmap(const SkBitmap& bitmap,
SkScalar left,
SkScalar top,
const SkPaint* paint) {
APPEND(DrawBitmap, this->copy(paint), delay_copy(bitmap), left, top);
APPEND(DrawBitmap, this->copy(paint), bitmap, left, top);
}
void SkRecorder::onDrawBitmapRect(const SkBitmap& bitmap,
@ -196,19 +175,19 @@ void SkRecorder::onDrawBitmapRect(const SkBitmap& bitmap,
DrawBitmapRectFlags flags) {
if (kBleed_DrawBitmapRectFlag == flags) {
APPEND(DrawBitmapRectToRectBleed,
this->copy(paint), delay_copy(bitmap), this->copy(src), dst);
this->copy(paint), bitmap, this->copy(src), dst);
return;
}
SkASSERT(kNone_DrawBitmapRectFlag == flags);
APPEND(DrawBitmapRectToRect,
this->copy(paint), delay_copy(bitmap), this->copy(src), dst);
this->copy(paint), bitmap, this->copy(src), dst);
}
void SkRecorder::onDrawBitmapNine(const SkBitmap& bitmap,
const SkIRect& center,
const SkRect& dst,
const SkPaint* paint) {
APPEND(DrawBitmapNine, this->copy(paint), delay_copy(bitmap), center, dst);
APPEND(DrawBitmapNine, this->copy(paint), bitmap, center, dst);
}
void SkRecorder::onDrawImage(const SkImage* image, SkScalar left, SkScalar top,
@ -228,20 +207,20 @@ void SkRecorder::onDrawImageNine(const SkImage* image, const SkIRect& center,
}
void SkRecorder::onDrawSprite(const SkBitmap& bitmap, int left, int top, const SkPaint* paint) {
APPEND(DrawSprite, this->copy(paint), delay_copy(bitmap), left, top);
APPEND(DrawSprite, this->copy(paint), bitmap, left, top);
}
void SkRecorder::onDrawText(const void* text, size_t byteLength,
SkScalar x, SkScalar y, const SkPaint& paint) {
APPEND(DrawText,
delay_copy(paint), this->copy((const char*)text, byteLength), byteLength, x, y);
paint, this->copy((const char*)text, byteLength), byteLength, x, y);
}
void SkRecorder::onDrawPosText(const void* text, size_t byteLength,
const SkPoint pos[], const SkPaint& paint) {
const unsigned points = paint.countText(text, byteLength);
APPEND(DrawPosText,
delay_copy(paint),
paint,
this->copy((const char*)text, byteLength),
byteLength,
this->copy(pos, points));
@ -251,7 +230,7 @@ void SkRecorder::onDrawPosTextH(const void* text, size_t byteLength,
const SkScalar xpos[], SkScalar constY, const SkPaint& paint) {
const unsigned points = paint.countText(text, byteLength);
APPEND(DrawPosTextH,
delay_copy(paint),
paint,
this->copy((const char*)text, byteLength),
SkToUInt(byteLength),
constY,
@ -261,17 +240,17 @@ void SkRecorder::onDrawPosTextH(const void* text, size_t byteLength,
void SkRecorder::onDrawTextOnPath(const void* text, size_t byteLength, const SkPath& path,
const SkMatrix* matrix, const SkPaint& paint) {
APPEND(DrawTextOnPath,
delay_copy(paint),
paint,
this->copy((const char*)text, byteLength),
byteLength,
delay_copy(path),
path,
matrix ? *matrix : SkMatrix::I());
}
void SkRecorder::onDrawTextBlob(const SkTextBlob* blob, SkScalar x, SkScalar y,
const SkPaint& paint) {
TRY_MINIRECORDER(drawTextBlob, blob, x, y, paint);
APPEND(DrawTextBlob, delay_copy(paint), blob, x, y);
APPEND(DrawTextBlob, paint, blob, x, y);
}
void SkRecorder::onDrawPicture(const SkPicture* pic, const SkMatrix* matrix, const SkPaint* paint) {
@ -284,7 +263,7 @@ void SkRecorder::onDrawVertices(VertexMode vmode,
const SkPoint texs[], const SkColor colors[],
SkXfermode* xmode,
const uint16_t indices[], int indexCount, const SkPaint& paint) {
APPEND(DrawVertices, delay_copy(paint),
APPEND(DrawVertices, paint,
vmode,
vertexCount,
this->copy(vertices, vertexCount),
@ -297,7 +276,7 @@ void SkRecorder::onDrawVertices(VertexMode vmode,
void SkRecorder::onDrawPatch(const SkPoint cubics[12], const SkColor colors[4],
const SkPoint texCoords[4], SkXfermode* xmode, const SkPaint& paint) {
APPEND(DrawPatch, delay_copy(paint),
APPEND(DrawPatch, paint,
cubics ? this->copy(cubics, SkPatchUtils::kNumCtrlPts) : NULL,
colors ? this->copy(colors, SkPatchUtils::kNumCorners) : NULL,
texCoords ? this->copy(texCoords, SkPatchUtils::kNumCorners) : NULL,
@ -360,11 +339,11 @@ void SkRecorder::onClipRRect(const SkRRect& rrect, SkRegion::Op op, ClipEdgeStyl
void SkRecorder::onClipPath(const SkPath& path, SkRegion::Op op, ClipEdgeStyle edgeStyle) {
INHERITED(onClipPath, path, op, edgeStyle);
SkRecords::RegionOpAndAA opAA(op, kSoft_ClipEdgeStyle == edgeStyle);
APPEND(ClipPath, this->devBounds(), delay_copy(path), opAA);
APPEND(ClipPath, this->devBounds(), path, opAA);
}
void SkRecorder::onClipRegion(const SkRegion& deviceRgn, SkRegion::Op op) {
INHERITED(onClipRegion, deviceRgn, op);
APPEND(ClipRegion, this->devBounds(), delay_copy(deviceRgn), op);
APPEND(ClipRegion, this->devBounds(), deviceRgn, op);
}

View File

@ -75,44 +75,43 @@ struct T { \
static const Type kType = T##_Type; \
};
// We try to be flexible about the types the constructors take. Instead of requring the exact type
// A here, we take any type Z which implicitly casts to A. This allows the delay_copy() trick to
// work, allowing the caller to decide whether to pass by value or by const&.
// Instead of requring the exact type A here, we take any type Z which implicitly casts to A.
// This lets our wrappers like ImmutableBitmap work seamlessly.
#define RECORD1(T, A, a) \
struct T { \
static const Type kType = T##_Type; \
T() {} \
template <typename Z> \
T(Z a) : a(a) {} \
T(const Z& a) : a(a) {} \
A a; \
};
#define RECORD2(T, A, a, B, b) \
struct T { \
static const Type kType = T##_Type; \
T() {} \
template <typename Z, typename Y> \
T(Z a, Y b) : a(a), b(b) {} \
A a; B b; \
#define RECORD2(T, A, a, B, b) \
struct T { \
static const Type kType = T##_Type; \
T() {} \
template <typename Z, typename Y> \
T(const Z& a, const Y& b) : a(a), b(b) {} \
A a; B b; \
};
#define RECORD3(T, A, a, B, b, C, c) \
struct T { \
static const Type kType = T##_Type; \
T() {} \
template <typename Z, typename Y, typename X> \
T(Z a, Y b, X c) : a(a), b(b), c(c) {} \
A a; B b; C c; \
#define RECORD3(T, A, a, B, b, C, c) \
struct T { \
static const Type kType = T##_Type; \
T() {} \
template <typename Z, typename Y, typename X> \
T(const Z& a, const Y& b, const X& c) : a(a), b(b), c(c) {} \
A a; B b; C c; \
};
#define RECORD4(T, A, a, B, b, C, c, D, d) \
struct T { \
static const Type kType = T##_Type; \
T() {} \
template <typename Z, typename Y, typename X, typename W> \
T(Z a, Y b, X c, W d) : a(a), b(b), c(c), d(d) {} \
A a; B b; C c; D d; \
#define RECORD4(T, A, a, B, b, C, c, D, d) \
struct T { \
static const Type kType = T##_Type; \
T() {} \
template <typename Z, typename Y, typename X, typename W> \
T(const Z& a, const Y& b, const X& c, const W& d) : a(a), b(b), c(c), d(d) {} \
A a; B b; C c; D d; \
};
#define RECORD5(T, A, a, B, b, C, c, D, d, E, e) \
@ -120,19 +119,23 @@ struct T { \
static const Type kType = T##_Type; \
T() {} \
template <typename Z, typename Y, typename X, typename W, typename V> \
T(Z a, Y b, X c, W d, V e) : a(a), b(b), c(c), d(d), e(e) {} \
T(const Z& a, const Y& b, const X& c, const W& d, const V& e) \
: a(a), b(b), c(c), d(d), e(e) {} \
A a; B b; C c; D d; E e; \
};
#define RECORD8(T, A, a, B, b, C, c, D, d, E, e, F, f, G, g, H, h) \
struct T { \
static const Type kType = T##_Type; \
T() {} \
template <typename Z, typename Y, typename X, typename W, typename V, typename U, typename S, typename R> \
T(Z a, Y b, X c, W d, V e, U f, S g, R h) : a(a), b(b), c(c), d(d), e(e), f(f), g(g), h(h) {} \
A a; B b; C c; D d; E e; F f; G g; H h; \
#define RECORD8(T, A, a, B, b, C, c, D, d, E, e, F, f, G, g, H, h) \
struct T { \
static const Type kType = T##_Type; \
T() {} \
template <typename Z, typename Y, typename X, typename W, \
typename V, typename U, typename S, typename R> \
T(const Z& a, const Y& b, const X& c, const W& d, \
const V& e, const U& f, const S& g, const R& h) \
: a(a), b(b), c(c), d(d), e(e), f(f), g(g), h(h) {} \
A a; B b; C c; D d; E e; F f; G g; H h; \
};
#define ACT_AS_PTR(ptr) \
operator T*() const { return ptr; } \
T* operator->() const { return ptr; }