From 449d9b7e2d1b2e20963f18639c6e541ef953f069 Mon Sep 17 00:00:00 2001 From: mtklein Date: Mon, 28 Sep 2015 10:33:02 -0700 Subject: [PATCH] simplify code in SkRecords.h - use C++11 features ({} init, move constructors) to eliminate the need for explicit constructors - collapse RECORD0...RECORD8 into just one RECORD macro - explicitly tag record types instead of using member detectors. Removing member detectors makes this code significantly less fragile. This exposes a few places where we didn't really think through what to do with SkDrawable. I've marked them TODO for now. BUG=skia: Review URL: https://codereview.chromium.org/1360943003 --- dm/DMSrcSink.cpp | 17 +- include/private/SkRecords.h | 390 +++++++++++++++++------------------- include/private/SkTLogic.h | 32 +-- src/core/SkMiniRecorder.cpp | 2 +- src/core/SkPictureCommon.h | 71 ++++--- src/core/SkRecord.h | 4 +- src/core/SkRecordDraw.cpp | 2 +- src/core/SkRecordPattern.h | 13 +- src/core/SkRecorder.cpp | 2 +- tests/RecordTest.cpp | 2 +- 10 files changed, 245 insertions(+), 290 deletions(-) diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp index e5faed7da1..25b5f7b460 100644 --- a/dm/DMSrcSink.cpp +++ b/dm/DMSrcSink.cpp @@ -24,10 +24,11 @@ #include "SkRecordDraw.h" #include "SkRecorder.h" #include "SkSVGCanvas.h" +#include "SkScaledCodec.h" #include "SkScanlineDecoder.h" #include "SkStream.h" +#include "SkTLogic.h" #include "SkXMLWriter.h" -#include "SkScaledCodec.h" DEFINE_bool(multiPage, false, "For document-type backends, render the source" " into multiple pages"); @@ -1212,8 +1213,6 @@ struct DrawsAsSingletonPictures { SkCanvas* fCanvas; const SkDrawableList& fDrawables; - SK_CREATE_MEMBER_DETECTOR(paint); - template void draw(const T& op, SkCanvas* canvas) { // We must pass SkMatrix::I() as our initial matrix. @@ -1223,20 +1222,20 @@ struct DrawsAsSingletonPictures { d(op); } - // Most things that have paints are Draw-type ops. Create sub-pictures for each. + // Draws get their own picture. template - SK_WHEN(HasMember_paint, void) operator()(const T& op) { + SK_WHEN(T::kTags & SkRecords::kDraw_Tag, void) operator()(const T& op) { SkPictureRecorder rec; this->draw(op, rec.beginRecording(SkRect::MakeLargest())); SkAutoTUnref pic(rec.endRecordingAsPicture()); fCanvas->drawPicture(pic); } - // If you don't have a paint or are a SaveLayer, you're not a Draw-type op. - // We cannot make subpictures out of these because they affect state. Draw them directly. + // We'll just issue non-draws directly. template - SK_WHEN(!HasMember_paint, void) operator()(const T& op) { this->draw(op, fCanvas); } - void operator()(const SkRecords::SaveLayer& op) { this->draw(op, fCanvas); } + skstd::enable_if_t operator()(const T& op) { + this->draw(op, fCanvas); + } }; // Record Src into a picture, then record it into a macro picture with a sub-picture for each draw. diff --git a/include/private/SkRecords.h b/include/private/SkRecords.h index 4268bb469c..610d29fb0c 100644 --- a/include/private/SkRecords.h +++ b/include/private/SkRecords.h @@ -72,74 +72,6 @@ namespace SkRecords { enum Type { SK_RECORD_TYPES(ENUM) }; #undef ENUM -// Macros to make it easier to define a record for a draw call with 0 args, 1 args, 2 args, etc. -// These should be clearer when you look at their use below. -#define RECORD0(T) \ -struct T { \ - static const Type kType = T##_Type; \ -}; - -// 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 \ - 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 \ - 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 \ - 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 \ - 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) \ -struct T { \ - static const Type kType = T##_Type; \ - T() {} \ - template \ - 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 \ - 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; } @@ -149,6 +81,9 @@ class RefBox : SkNoncopyable { public: RefBox() {} RefBox(T* obj) : fObj(SkSafeRef(obj)) {} + RefBox(RefBox&& o) : fObj(o.fObj) { + o.fObj = nullptr; + } ~RefBox() { SkSafeUnref(fObj); } ACT_AS_PTR(fObj); @@ -163,6 +98,9 @@ class Optional : SkNoncopyable { public: Optional() : fPtr(nullptr) {} Optional(T* ptr) : fPtr(ptr) {} + Optional(Optional&& o) : fPtr(o.fPtr) { + o.fPtr = nullptr; + } ~Optional() { if (fPtr) fPtr->~T(); } ACT_AS_PTR(fPtr); @@ -207,7 +145,10 @@ private: class ImmutableBitmap : SkNoncopyable { public: ImmutableBitmap() {} - explicit ImmutableBitmap(const SkBitmap& bitmap); + ImmutableBitmap(const SkBitmap& bitmap); + ImmutableBitmap(ImmutableBitmap&& o) { + fBitmap.swap(o.fBitmap); + } int width() const { return fBitmap.width(); } int height() const { return fBitmap.height(); } @@ -223,23 +164,43 @@ private: // Recording is a convenient time to cache these, or we can delay it to between record and playback. struct PreCachedPath : public SkPath { PreCachedPath() {} - explicit PreCachedPath(const SkPath& path); + PreCachedPath(const SkPath& path); }; // Like SkPath::getBounds(), SkMatrix::getType() isn't thread safe unless we precache it. // This may not cover all SkMatrices used by the picture (e.g. some could be hiding in a shader). struct TypedMatrix : public SkMatrix { TypedMatrix() {} - explicit TypedMatrix(const SkMatrix& matrix); + TypedMatrix(const SkMatrix& matrix); }; -RECORD0(NoOp); +enum Tags { + kDraw_Tag = 1, // May draw something (usually named DrawFoo). + kHasImage_Tag = 2, // Contains an SkImage or SkBitmap. + kHasText_Tag = 4, // Contains text. +}; -RECORD2(Restore, SkIRect, devBounds, TypedMatrix, matrix); -RECORD0(Save); -RECORD3(SaveLayer, Optional, bounds, Optional, paint, SkCanvas::SaveFlags, flags); +// A macro to make it a little easier to define a struct that can be stored in SkRecord. +#define RECORD(T, tags, ...) \ +struct T { \ + static const Type kType = T##_Type; \ + static const int kTags = tags; \ + __VA_ARGS__; \ +}; -RECORD1(SetMatrix, TypedMatrix, matrix); +RECORD(NoOp, 0); +RECORD(Restore, 0, + SkIRect devBounds; + TypedMatrix matrix); +RECORD(Save, 0); + +RECORD(SaveLayer, 0, + Optional bounds; + Optional paint; + SkCanvas::SaveFlags flags); + +RECORD(SetMatrix, 0, + TypedMatrix matrix); struct RegionOpAndAA { RegionOpAndAA() {} @@ -249,138 +210,157 @@ struct RegionOpAndAA { }; static_assert(sizeof(RegionOpAndAA) == 4, "RegionOpAndAASize"); -RECORD3(ClipPath, SkIRect, devBounds, PreCachedPath, path, RegionOpAndAA, opAA); -RECORD3(ClipRRect, SkIRect, devBounds, SkRRect, rrect, RegionOpAndAA, opAA); -RECORD3(ClipRect, SkIRect, devBounds, SkRect, rect, RegionOpAndAA, opAA); -RECORD3(ClipRegion, SkIRect, devBounds, SkRegion, region, SkRegion::Op, op); +RECORD(ClipPath, 0, + SkIRect devBounds; + PreCachedPath path; + RegionOpAndAA opAA); +RECORD(ClipRRect, 0, + SkIRect devBounds; + SkRRect rrect; + RegionOpAndAA opAA); +RECORD(ClipRect, 0, + SkIRect devBounds; + SkRect rect; + RegionOpAndAA opAA); +RECORD(ClipRegion, 0, + SkIRect devBounds; + SkRegion region; + SkRegion::Op op); // While not strictly required, if you have an SkPaint, it's fastest to put it first. -RECORD4(DrawBitmap, Optional, paint, - ImmutableBitmap, bitmap, - SkScalar, left, - SkScalar, top); -RECORD4(DrawBitmapNine, Optional, paint, - ImmutableBitmap, bitmap, - SkIRect, center, - SkRect, dst); -RECORD4(DrawBitmapRect, Optional, paint, - ImmutableBitmap, bitmap, - Optional, src, - SkRect, dst); -RECORD4(DrawBitmapRectFast, Optional, paint, - ImmutableBitmap, bitmap, - Optional, src, - SkRect, dst); -RECORD5(DrawBitmapRectFixedSize, SkPaint, paint, - ImmutableBitmap, bitmap, - SkRect, src, - SkRect, dst, - SkCanvas::SrcRectConstraint, constraint); -RECORD3(DrawDRRect, SkPaint, paint, SkRRect, outer, SkRRect, inner); -RECORD3(DrawDrawable, Optional, matrix, SkRect, worstCaseBounds, int32_t, index); -RECORD4(DrawImage, Optional, paint, - RefBox, image, - SkScalar, left, - SkScalar, top); -RECORD5(DrawImageRect, Optional, paint, - RefBox, image, - Optional, src, - SkRect, dst, - SkCanvas::SrcRectConstraint, constraint); -RECORD4(DrawImageNine, Optional, paint, - RefBox, image, - SkIRect, center, - SkRect, dst); -RECORD2(DrawOval, SkPaint, paint, SkRect, oval); -RECORD1(DrawPaint, SkPaint, paint); -RECORD2(DrawPath, SkPaint, paint, PreCachedPath, path); -RECORD3(DrawPicture, Optional, paint, - RefBox, picture, - TypedMatrix, matrix); -RECORD4(DrawPoints, SkPaint, paint, SkCanvas::PointMode, mode, unsigned, count, SkPoint*, pts); -RECORD4(DrawPosText, SkPaint, paint, - PODArray, text, - size_t, byteLength, - PODArray, pos); -RECORD5(DrawPosTextH, SkPaint, paint, - PODArray, text, - unsigned, byteLength, - SkScalar, y, - PODArray, xpos); -RECORD2(DrawRRect, SkPaint, paint, SkRRect, rrect); -RECORD2(DrawRect, SkPaint, paint, SkRect, rect); -RECORD4(DrawSprite, Optional, paint, ImmutableBitmap, bitmap, int, left, int, top); -RECORD5(DrawText, SkPaint, paint, - PODArray, text, - size_t, byteLength, - SkScalar, x, - SkScalar, y); -RECORD4(DrawTextBlob, SkPaint, paint, - RefBox, blob, - SkScalar, x, - SkScalar, y); -RECORD5(DrawTextOnPath, SkPaint, paint, - PODArray, text, - size_t, byteLength, - PreCachedPath, path, - TypedMatrix, matrix); +RECORD(DrawBitmap, kDraw_Tag|kHasImage_Tag, + Optional paint; + ImmutableBitmap bitmap; + SkScalar left; + SkScalar top); +RECORD(DrawBitmapNine, kDraw_Tag|kHasImage_Tag, + Optional paint; + ImmutableBitmap bitmap; + SkIRect center; + SkRect dst); +RECORD(DrawBitmapRect, kDraw_Tag|kHasImage_Tag, + Optional paint; + ImmutableBitmap bitmap; + Optional src; + SkRect dst); +RECORD(DrawBitmapRectFast, kDraw_Tag|kHasImage_Tag, + Optional paint; + ImmutableBitmap bitmap; + Optional src; + SkRect dst); +RECORD(DrawBitmapRectFixedSize, kDraw_Tag|kHasImage_Tag, + SkPaint paint; + ImmutableBitmap bitmap; + SkRect src; + SkRect dst; + SkCanvas::SrcRectConstraint constraint); +RECORD(DrawDRRect, kDraw_Tag, + SkPaint paint; + SkRRect outer; + SkRRect inner); +RECORD(DrawDrawable, kDraw_Tag, + Optional matrix; + SkRect worstCaseBounds; + int32_t index); +RECORD(DrawImage, kDraw_Tag|kHasImage_Tag, + Optional paint; + RefBox image; + SkScalar left; + SkScalar top); +RECORD(DrawImageRect, kDraw_Tag|kHasImage_Tag, + Optional paint; + RefBox image; + Optional src; + SkRect dst; + SkCanvas::SrcRectConstraint constraint); +RECORD(DrawImageNine, kDraw_Tag|kHasImage_Tag, + Optional paint; + RefBox image; + SkIRect center; + SkRect dst); +RECORD(DrawOval, kDraw_Tag, + SkPaint paint; + SkRect oval); +RECORD(DrawPaint, kDraw_Tag, + SkPaint paint); +RECORD(DrawPath, kDraw_Tag, + SkPaint paint; + PreCachedPath path); +RECORD(DrawPicture, kDraw_Tag, + Optional paint; + RefBox picture; + TypedMatrix matrix); +RECORD(DrawPoints, kDraw_Tag, + SkPaint paint; + SkCanvas::PointMode mode; + unsigned count; + SkPoint* pts); +RECORD(DrawPosText, kDraw_Tag|kHasText_Tag, + SkPaint paint; + PODArray text; + size_t byteLength; + PODArray pos); +RECORD(DrawPosTextH, kDraw_Tag|kHasText_Tag, + SkPaint paint; + PODArray text; + unsigned byteLength; + SkScalar y; + PODArray xpos); +RECORD(DrawRRect, kDraw_Tag, + SkPaint paint; + SkRRect rrect); +RECORD(DrawRect, kDraw_Tag, + SkPaint paint; + SkRect rect); +RECORD(DrawSprite, kDraw_Tag|kHasImage_Tag, + Optional paint; + ImmutableBitmap bitmap; + int left; + int top); +RECORD(DrawText, kDraw_Tag|kHasText_Tag, + SkPaint paint; + PODArray text; + size_t byteLength; + SkScalar x; + SkScalar y); +RECORD(DrawTextBlob, kDraw_Tag|kHasText_Tag, + SkPaint paint; + RefBox blob; + SkScalar x; + SkScalar y); +RECORD(DrawTextOnPath, kDraw_Tag|kHasText_Tag, + SkPaint paint; + PODArray text; + size_t byteLength; + PreCachedPath path; + TypedMatrix matrix); +RECORD(DrawPatch, kDraw_Tag, + SkPaint paint; + PODArray cubics; + PODArray colors; + PODArray texCoords; + RefBox xmode); +RECORD(DrawAtlas, kDraw_Tag|kHasImage_Tag, + Optional paint; + RefBox atlas; + PODArray xforms; + PODArray texs; + PODArray colors; + int count; + SkXfermode::Mode mode; + Optional cull); +RECORD(DrawVertices, kDraw_Tag, + SkPaint paint; + SkCanvas::VertexMode vmode; + int vertexCount; + PODArray vertices; + PODArray texs; + PODArray colors; + RefBox xmode; + PODArray indices; + int indexCount); -RECORD5(DrawPatch, SkPaint, paint, - PODArray, cubics, - PODArray, colors, - PODArray, texCoords, - RefBox, xmode); - -RECORD8(DrawAtlas, Optional, paint, - RefBox, atlas, - PODArray, xforms, - PODArray, texs, - PODArray, colors, - int, count, - SkXfermode::Mode, mode, - Optional, cull); - -// This guy is so ugly we just write it manually. -struct DrawVertices { - static const Type kType = DrawVertices_Type; - - DrawVertices(const SkPaint& paint, - SkCanvas::VertexMode vmode, - int vertexCount, - SkPoint* vertices, - SkPoint* texs, - SkColor* colors, - SkXfermode* xmode, - uint16_t* indices, - int indexCount) - : paint(paint) - , vmode(vmode) - , vertexCount(vertexCount) - , vertices(vertices) - , texs(texs) - , colors(colors) - , xmode(SkSafeRef(xmode)) - , indices(indices) - , indexCount(indexCount) {} - - SkPaint paint; - SkCanvas::VertexMode vmode; - int vertexCount; - PODArray vertices; - PODArray texs; - PODArray colors; - SkAutoTUnref xmode; - PODArray indices; - int indexCount; -}; - -#undef RECORD0 -#undef RECORD1 -#undef RECORD2 -#undef RECORD3 -#undef RECORD4 -#undef RECORD5 -#undef RECORD8 +#undef RECORD } // namespace SkRecords diff --git a/include/private/SkTLogic.h b/include/private/SkTLogic.h index e7fc3dd40d..c252001a00 100644 --- a/include/private/SkTLogic.h +++ b/include/private/SkTLogic.h @@ -220,35 +220,7 @@ template using same_cv_t = typename same_cv::type } // namespace sknonstd -/** 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(true_type, int) f(void* ptr) { return 1; } - * SK_WHEN(!true_type, int) f(void* ptr) { return 2; } - */ -#define SK_WHEN(cond_prefix, T) skstd::enable_if_t - -// See http://en.wikibooks.org/wiki/More_C++_Idioms/Member_Detector -#define SK_CREATE_MEMBER_DETECTOR(member) \ -template \ -class HasMember_##member { \ - struct Fallback { int member; }; \ - struct Derived : T, Fallback {}; \ - template struct Check; \ - template static uint8_t func(Check*); \ - template static uint16_t func(...); \ -public: \ - typedef HasMember_##member type; \ - static const bool value = sizeof(func(NULL)) == sizeof(uint16_t); \ -} - -// Same sort of thing as SK_CREATE_MEMBER_DETECTOR, but checks for the existence of a nested type. -#define SK_CREATE_TYPE_DETECTOR(type) \ -template \ -class HasType_##type { \ - template static uint8_t func(typename U::type*); \ - template static uint16_t func(...); \ -public: \ - static const bool value = sizeof(func(NULL)) == sizeof(uint8_t); \ -} +// Just a pithier wrapper for enable_if_t. +#define SK_WHEN(condition, T) skstd::enable_if_t #endif diff --git a/src/core/SkMiniRecorder.cpp b/src/core/SkMiniRecorder.cpp index 0a4859f379..ea179d78b3 100644 --- a/src/core/SkMiniRecorder.cpp +++ b/src/core/SkMiniRecorder.cpp @@ -70,7 +70,7 @@ SkMiniRecorder::~SkMiniRecorder() { #define TRY_TO_STORE(Type, ...) \ if (fState != State::kEmpty) { return false; } \ fState = State::k##Type; \ - new (fBuffer.get()) Type(__VA_ARGS__); \ + new (fBuffer.get()) Type{__VA_ARGS__}; \ return true bool SkMiniRecorder::drawBitmapRect(const SkBitmap& bm, const SkRect* src, const SkRect& dst, diff --git a/src/core/SkPictureCommon.h b/src/core/SkPictureCommon.h index c9c023d2bb..6e95b296e0 100644 --- a/src/core/SkPictureCommon.h +++ b/src/core/SkPictureCommon.h @@ -16,28 +16,18 @@ struct SkTextHunter { // Most ops never have text. Some always do. Subpictures know themeselves. - template bool operator()(const T&) { return false; } - bool operator()(const SkRecords::DrawPosText&) { return true; } - bool operator()(const SkRecords::DrawPosTextH&) { return true; } - bool operator()(const SkRecords::DrawText&) { return true; } - bool operator()(const SkRecords::DrawTextBlob&) { return true; } - bool operator()(const SkRecords::DrawTextOnPath&) { return true; } bool operator()(const SkRecords::DrawPicture& op) { return op.picture->hasText(); } + bool operator()(const SkRecords::DrawDrawable&) { /*TODO*/ return false; } + + template + SK_WHEN(T::kTags & SkRecords::kHasText_Tag, bool) operator()(const T&) { return true; } + template + SK_WHEN(!(T::kTags & SkRecords::kHasText_Tag), bool) operator()(const T&) { return false; } }; // N.B. This name is slightly historical: hunting season is now open for SkImages too. struct SkBitmapHunter { - // Helpers. These let us detect the presence of struct members with particular names. - SK_CREATE_MEMBER_DETECTOR(bitmap); - SK_CREATE_MEMBER_DETECTOR(image); - SK_CREATE_MEMBER_DETECTOR(paint); - - template - struct HasMember_bitmap_or_image { - static const bool value = HasMember_bitmap::value || HasMember_image::value; - }; - // Some ops have a paint, some have an optional paint. Either way, get back a pointer. static const SkPaint* AsPtr(const SkPaint& p) { return &p; } static const SkPaint* AsPtr(const SkRecords::Optional& p) { return p; } @@ -48,26 +38,42 @@ struct SkBitmapHunter { // If the op has a paint and the paint has a bitmap, return true. // Otherwise, return false. bool operator()(const SkRecords::DrawPicture& op) { return op.picture->willPlayBackBitmaps(); } + bool operator()(const SkRecords::DrawDrawable&) { /*TODO*/ return false; } template - bool operator()(const T& r) { return CheckBitmap(r); } + bool operator()(const T& op) { return CheckBitmap(op); } - // If the op has a bitmap, of course we're going to play back bitmaps. + // If the op is tagged as having an image, return true. template - static SK_WHEN(HasMember_bitmap_or_image, bool) CheckBitmap(const T&) { + static SK_WHEN(T::kTags & SkRecords::kHasImage_Tag, bool) CheckBitmap(const T&) { return true; } // If not, look for one in its paint (if it has a paint). template - static SK_WHEN(!HasMember_bitmap_or_image, bool) CheckBitmap(const T& r) { - return CheckPaint(r); + static SK_WHEN(!(T::kTags & SkRecords::kHasImage_Tag), bool) CheckBitmap(const T& op) { + return CheckPaint(op); } - // If we have a paint, dig down into the effects looking for a bitmap. + // Most draws-type ops have paints. template - static SK_WHEN(HasMember_paint, bool) CheckPaint(const T& r) { - const SkPaint* paint = AsPtr(r.paint); + static SK_WHEN(T::kTags & SkRecords::kDraw_Tag, bool) CheckPaint(const T& op) { + return PaintHasBitmap(AsPtr(op.paint)); + } + + // SaveLayers also have a paint to check. + static bool CheckPaint(const SkRecords::SaveLayer& op) { + return PaintHasBitmap(AsPtr(op.paint)); + } + + // Shouldn't be any non-Draw non-SaveLayer ops with paints. + template + static SK_WHEN(!(T::kTags & SkRecords::kDraw_Tag), bool) CheckPaint(const T&) { + return false; + } + +private: + static bool PaintHasBitmap(const SkPaint* paint) { if (paint) { const SkShader* shader = paint->getShader(); if (shader && shader->isABitmap()) { @@ -76,16 +82,10 @@ struct SkBitmapHunter { } return false; } - - // If we don't have a paint, that non-paint has no bitmap. - template - static SK_WHEN(!HasMember_paint, bool) CheckPaint(const T&) { return false; } }; // TODO: might be nicer to have operator() return an int (the number of slow paths) ? struct SkPathCounter { - SK_CREATE_MEMBER_DETECTOR(paint); - // Some ops have a paint, some have an optional paint. Either way, get back a pointer. static const SkPaint* AsPtr(const SkPaint& p) { return &p; } static const SkPaint* AsPtr(const SkRecords::Optional& p) { return p; } @@ -96,6 +96,7 @@ struct SkPathCounter { void operator()(const SkRecords::DrawPicture& op) { fNumSlowPathsAndDashEffects += op.picture->numSlowPaths(); } + void operator()(const SkRecords::DrawDrawable&) { /* TODO */ } void checkPaint(const SkPaint* paint) { if (paint && paint->getPathEffect()) { @@ -134,13 +135,17 @@ struct SkPathCounter { } } - template - SK_WHEN(HasMember_paint, void) operator()(const T& op) { + void operator()(const SkRecords::SaveLayer& op) { this->checkPaint(AsPtr(op.paint)); } template - SK_WHEN(!HasMember_paint, void) operator()(const T& op) { /* do nothing */ } + SK_WHEN(T::kTags & SkRecords::kDraw_Tag, void) operator()(const T& op) { + this->checkPaint(AsPtr(op.paint)); + } + + template + SK_WHEN(!(T::kTags & SkRecords::kDraw_Tag), void) operator()(const T& op) { /* do nothing */ } int fNumSlowPathsAndDashEffects; }; diff --git a/src/core/SkRecord.h b/src/core/SkRecord.h index 6893cb02ab..ee10b15784 100644 --- a/src/core/SkRecord.h +++ b/src/core/SkRecord.h @@ -134,13 +134,13 @@ private: }; template - SK_WHEN(skstd::is_empty, T*) allocCommand() { + SK_WHEN(skstd::is_empty::value, T*) allocCommand() { static T singleton = {}; return &singleton; } template - SK_WHEN(!skstd::is_empty, T*) allocCommand() { return this->alloc(); } + SK_WHEN(!skstd::is_empty::value, T*) allocCommand() { return this->alloc(); } void grow(); diff --git a/src/core/SkRecordDraw.cpp b/src/core/SkRecordDraw.cpp index 1a033482e5..12920da361 100644 --- a/src/core/SkRecordDraw.cpp +++ b/src/core/SkRecordDraw.cpp @@ -116,7 +116,7 @@ DRAW(DrawTextBlob, drawTextBlob(r.blob, r.x, r.y, r.paint)); DRAW(DrawTextOnPath, drawTextOnPath(r.text, r.byteLength, r.path, &r.matrix, r.paint)); DRAW(DrawAtlas, drawAtlas(r.atlas, r.xforms, r.texs, r.colors, r.count, r.mode, r.cull, r.paint)); DRAW(DrawVertices, drawVertices(r.vmode, r.vertexCount, r.vertices, r.texs, r.colors, - r.xmode.get(), r.indices, r.indexCount, r.paint)); + r.xmode, r.indices, r.indexCount, r.paint)); #undef DRAW template <> void Draw::draw(const DrawDrawable& r) { diff --git a/src/core/SkRecordPattern.h b/src/core/SkRecordPattern.h index ecc61b3aaa..d9568b360a 100644 --- a/src/core/SkRecordPattern.h +++ b/src/core/SkRecordPattern.h @@ -41,7 +41,6 @@ private: // Matches any command that draws, and stores its paint. class IsDraw { - SK_CREATE_MEMBER_DETECTOR(paint); public: IsDraw() : fPaint(nullptr) {} @@ -49,19 +48,19 @@ public: type* get() { return fPaint; } template - SK_WHEN(HasMember_paint, bool) operator()(T* draw) { + SK_WHEN(T::kTags & kDraw_Tag, bool) operator()(T* draw) { fPaint = AsPtr(draw->paint); return true; } - template - SK_WHEN(!HasMember_paint, bool) operator()(T*) { + bool operator()(DrawDrawable*) { + static_assert(DrawDrawable::kTags & kDraw_Tag, ""); fPaint = nullptr; - return false; + return true; } - // SaveLayer has an SkPaint named paint, but it's not a draw. - bool operator()(SaveLayer*) { + template + SK_WHEN(!(T::kTags & kDraw_Tag), bool) operator()(T* draw) { fPaint = nullptr; return false; } diff --git a/src/core/SkRecorder.cpp b/src/core/SkRecorder.cpp index 9eccca164f..0605d17fc3 100644 --- a/src/core/SkRecorder.cpp +++ b/src/core/SkRecorder.cpp @@ -70,7 +70,7 @@ void SkRecorder::forgetRecord() { if (fMiniRecorder) { \ this->flushMiniRecorder(); \ } \ - new (fRecord->append()) SkRecords::T(__VA_ARGS__) + new (fRecord->append()) SkRecords::T{__VA_ARGS__} #define TRY_MINIRECORDER(method, ...) \ if (fMiniRecorder && fMiniRecorder->method(__VA_ARGS__)) { return; } diff --git a/tests/RecordTest.cpp b/tests/RecordTest.cpp index cf8c6028bc..9e02c882db 100644 --- a/tests/RecordTest.cpp +++ b/tests/RecordTest.cpp @@ -51,7 +51,7 @@ struct Stretch { } }; -#define APPEND(record, type, ...) new (record.append()) type(__VA_ARGS__) +#define APPEND(record, type, ...) new (record.append()) type{__VA_ARGS__} // Basic tests for the low-level SkRecord code. DEF_TEST(Record, r) {