Index: cc/paint/paint_op_buffer.h |
diff --git a/cc/paint/paint_op_buffer.h b/cc/paint/paint_op_buffer.h |
index db9eaadcf22c8c8ffb89e4005e94ebb5f23d9718..51846d136b9ea856a6e432a6f501b47d2d805895 100644 |
--- a/cc/paint/paint_op_buffer.h |
+++ b/cc/paint/paint_op_buffer.h |
@@ -72,6 +72,91 @@ enum class PaintOpType : uint8_t { |
LastPaintOpType = Translate, |
}; |
+// It is a requirement for every field in a PaintOp that we can memcpy it to |
+// another place in memory and free the only one (without calling any |
+// constructors or destructors). Specializations of this template are for |
+// types that we know to be safe for memcpy+free, with static asserts if |
+// possible, and explanations if not. |
+template <typename T> |
+struct SafePaintOpData; |
vmpstr
2017/06/01 22:00:55
Don't need the forward declare anymore
|
+ |
+template <typename T> |
+struct SafePaintOpData { |
+ using type = T; |
+ |
+ // TODO(danakj): This could use std::is_trivially_move_constructible once that |
+ // is available on all our toolchains. |
+ static_assert(base::is_trivially_copyable<type>::value, |
+ "This type is not trivially copyable, which means it may not be" |
+ " okay to change its memory address via memcpy+free. A template" |
+ " specialization should be provided for this type that verifies" |
+ " or explains why it is valid to memcpy+free this type."); |
+}; |
+ |
+// sk_sp<T> pointers behave if their memory address is changed because there |
+// is never a pointer back to the sk_sp<T> itself from inside the T. This is |
+// true because multiple sk_sp<> may point to the same T and they are |
+// transient compared to the lifetime of any T. Additionally, the move |
+// constructor doesn't change any state that needs to be reflected in the new |
+// object. |
+template <typename U> |
+struct SafePaintOpData<sk_sp<U>> { |
+ using type = sk_sp<U>; |
+}; |
+// scoped_refptr<> is able to be moved in memory for the same reasons as |
+// sk_sp<>. |
+template <typename U> |
+struct SafePaintOpData<scoped_refptr<U>> { |
+ using type = scoped_refptr<U>; |
+}; |
+ |
+// PaintFlags is a collection of trivial data types and sk_sp<> ref pointers. |
+// The trivial data types satisfy std::is_trivially_copyable, and sk_sp<> is |
+// valid to have its memory address changed for reasons decribed on |
+// SafePaintOpData<sk_sp>. |
+template <> |
+struct SafePaintOpData<PaintFlags> { |
+ using type = PaintFlags; |
+}; |
+ |
+// PaintImage is a set of trivial types and an sk_sp<> which is able to be |
+// moved in memory as described on SafePaintOpData<sk_sp>. PaintImage does not |
+// hold any pointers back to itself directly or indirectly. |
+template <> |
+struct SafePaintOpData<PaintImage> { |
+ using type = PaintImage; |
+}; |
+ |
+// ThreadsafePath is an SkPath which contains trivial types, sk_sp<>, and |
+// SkAtomic<uint8_t>. sk_sp<> is valid as described on SafePaintOpData<sk_sp>. |
+// SkAtomic<uint8_t> stores a trivial type internally, and takes its address |
+// but does not store it. |
+template <> |
+struct SafePaintOpData<ThreadsafePath> { |
+ using type = ThreadsafePath; |
+}; |
+ |
+// WARNING: It is a requirement for every field in a PaintOp that we can memcpy |
+// it to another place in memory and free the only one (without calling any |
+// constructors or destructors). We think of this as moving the memory address |
+// of the data type. This is required because PaintOpBuffer stores PaintOps in |
+// a memory buffer that is realloced when it grows, changing the address of |
+// everything inside the buffer. |
+// std::is_trivially_copyable<> would guarantee this, but is a superset of the |
+// guarantees that we need. For instance sk_sp<> works correctly when we |
+// memcpy+free it, but it has a non-trivial move constructor and destructor, |
+// which would make it fail a check for std::is_trivially_copyable<>. |
+// std::is_trivially_move_constructible<> also provides enough as we are moving |
+// the objects from one address to another conceptually, but also is not |
+// satisfied by sk_sp<> since it modifies the moved-from type. |
+// |
+// THEREFORE: For each field in a PaintOp, declare it as a |
+// SafePaintOpData<T>::type, which provides us with an explanation of safety for |
+// each type. |
+// |
+// BEWARE: std::vector does *not* work if its memory address is changed, as |
+// implementations can (and do) sometimes have a pointer back to the vector from |
+// its internals, so running its move-assignment operator is required. |
struct CC_PAINT_EXPORT PaintOp { |
vmpstr
2017/06/01 22:00:55
maybe we can add something like
template <typenam
|
uint32_t type : 8; |
uint32_t skip : 24; |
@@ -126,9 +211,13 @@ struct CC_PAINT_EXPORT PaintOpWithFlags : PaintOp { |
// provide a modified PaintFlags. The RasterWithFlags() method is static with |
// a const PaintOpWithFlags* parameter so that it can be used as a function |
// pointer. |
- PaintFlags flags; |
+ SafePaintOpData<PaintFlags>::type flags; |
}; |
+// A PaintOp with an additional block of memory with variable size. The |
+// additional data held in the PaintOpWithData must be able to be moved in |
+// memory without consequences. As such it must not (directly or indirectly) |
+// contain any pointers back to itself or to the PaintOp. |
struct CC_PAINT_EXPORT PaintOpWithData : PaintOpWithFlags { |
// Having data is just a helper for ops that have a varying amount of data and |
// want a way to store that inline. This is for ops that pass in a |
@@ -139,7 +228,7 @@ struct CC_PAINT_EXPORT PaintOpWithData : PaintOpWithFlags { |
// Get data out by calling paint_op_data. This can't be part of the class |
// because it needs to know the size of the derived type. |
- size_t bytes; |
+ SafePaintOpData<size_t>::type bytes; |
protected: |
// For some derived object T, return the internally stored data. |
@@ -170,8 +259,16 @@ struct CC_PAINT_EXPORT PaintOpWithArrayBase : PaintOpWithFlags { |
: PaintOpWithFlags(flags) {} |
}; |
+// A PaintOp with an additional block of memory with variable size that is |
+// an array of a known type. The types held in the array must be able to be |
+// moved in memory without consequences. As such it must not (directly or |
+// indirectly) contain any pointers back to itself or to the PaintOp. We use |
+// std::is_trivially_copyable() to verify this (it is more strict but satisfies |
+// our needs). |
template <typename M> |
struct CC_PAINT_EXPORT PaintOpWithArray : PaintOpWithArrayBase { |
+ static_assert(base::is_trivially_copyable<M>::value, ""); |
+ |
// Paint op that has a M[count] and a char[bytes]. |
// Array data is stored first so that it can be aligned with T's alignment |
// with the arbitrary unaligned char data after it. |
@@ -180,8 +277,8 @@ struct CC_PAINT_EXPORT PaintOpWithArray : PaintOpWithArrayBase { |
PaintOpWithArray(const PaintFlags& flags, size_t bytes, size_t count) |
: PaintOpWithArrayBase(flags), bytes(bytes), count(count) {} |
- size_t bytes; |
- size_t count; |
+ SafePaintOpData<size_t>::type bytes; |
+ SafePaintOpData<size_t>::type count; |
protected: |
template <typename T> |
@@ -241,9 +338,9 @@ struct CC_PAINT_EXPORT AnnotateOp final : PaintOp { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- PaintCanvas::AnnotationType annotation_type; |
- SkRect rect; |
- sk_sp<SkData> data; |
+ SafePaintOpData<PaintCanvas::AnnotationType>::type annotation_type; |
+ SafePaintOpData<SkRect>::type rect; |
+ SafePaintOpData<sk_sp<SkData>>::type data; |
}; |
struct CC_PAINT_EXPORT ClipPathOp final : PaintOp { |
@@ -255,9 +352,9 @@ struct CC_PAINT_EXPORT ClipPathOp final : PaintOp { |
const SkMatrix& original_ctm); |
int CountSlowPaths() const; |
- ThreadsafePath path; |
- SkClipOp op; |
- bool antialias; |
+ SafePaintOpData<ThreadsafePath>::type path; |
+ SafePaintOpData<SkClipOp>::type op; |
+ SafePaintOpData<bool>::type antialias; |
}; |
struct CC_PAINT_EXPORT ClipRectOp final : PaintOp { |
@@ -268,9 +365,9 @@ struct CC_PAINT_EXPORT ClipRectOp final : PaintOp { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkRect rect; |
- SkClipOp op; |
- bool antialias; |
+ SafePaintOpData<SkRect>::type rect; |
+ SafePaintOpData<SkClipOp>::type op; |
+ SafePaintOpData<bool>::type antialias; |
}; |
struct CC_PAINT_EXPORT ClipRRectOp final : PaintOp { |
@@ -281,9 +378,9 @@ struct CC_PAINT_EXPORT ClipRRectOp final : PaintOp { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkRRect rrect; |
- SkClipOp op; |
- bool antialias; |
+ SafePaintOpData<SkRRect>::type rrect; |
+ SafePaintOpData<SkClipOp>::type op; |
+ SafePaintOpData<bool>::type antialias; |
}; |
struct CC_PAINT_EXPORT ConcatOp final : PaintOp { |
@@ -293,7 +390,7 @@ struct CC_PAINT_EXPORT ConcatOp final : PaintOp { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- ThreadsafeMatrix matrix; |
+ SafePaintOpData<ThreadsafeMatrix>::type matrix; |
}; |
struct CC_PAINT_EXPORT DrawArcOp final : PaintOpWithFlags { |
@@ -320,10 +417,10 @@ struct CC_PAINT_EXPORT DrawArcOp final : PaintOpWithFlags { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkRect oval; |
- SkScalar start_angle; |
- SkScalar sweep_angle; |
- bool use_center; |
+ SafePaintOpData<SkRect>::type oval; |
+ SafePaintOpData<SkScalar>::type start_angle; |
+ SafePaintOpData<SkScalar>::type sweep_angle; |
+ SafePaintOpData<bool>::type use_center; |
}; |
struct CC_PAINT_EXPORT DrawCircleOp final : PaintOpWithFlags { |
@@ -345,9 +442,9 @@ struct CC_PAINT_EXPORT DrawCircleOp final : PaintOpWithFlags { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkScalar cx; |
- SkScalar cy; |
- SkScalar radius; |
+ SafePaintOpData<SkScalar>::type cx; |
+ SafePaintOpData<SkScalar>::type cy; |
+ SafePaintOpData<SkScalar>::type radius; |
}; |
struct CC_PAINT_EXPORT DrawColorOp final : PaintOp { |
@@ -358,8 +455,8 @@ struct CC_PAINT_EXPORT DrawColorOp final : PaintOp { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkColor color; |
- SkBlendMode mode; |
+ SafePaintOpData<SkColor>::type color; |
+ SafePaintOpData<SkBlendMode>::type mode; |
}; |
struct CC_PAINT_EXPORT DrawDisplayItemListOp final : PaintOp { |
@@ -379,7 +476,7 @@ struct CC_PAINT_EXPORT DrawDisplayItemListOp final : PaintOp { |
bool HasDiscardableImages() const; |
int CountSlowPaths() const; |
- scoped_refptr<DisplayItemList> list; |
+ SafePaintOpData<scoped_refptr<DisplayItemList>>::type list; |
}; |
struct CC_PAINT_EXPORT DrawDRRectOp final : PaintOpWithFlags { |
@@ -400,8 +497,8 @@ struct CC_PAINT_EXPORT DrawDRRectOp final : PaintOpWithFlags { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkRRect outer; |
- SkRRect inner; |
+ SafePaintOpData<SkRRect>::type outer; |
+ SafePaintOpData<SkRRect>::type inner; |
}; |
struct CC_PAINT_EXPORT DrawImageOp final : PaintOpWithFlags { |
@@ -424,9 +521,9 @@ struct CC_PAINT_EXPORT DrawImageOp final : PaintOpWithFlags { |
const SkMatrix& original_ctm); |
bool HasDiscardableImages() const; |
- PaintImage image; |
- SkScalar left; |
- SkScalar top; |
+ SafePaintOpData<PaintImage>::type image; |
+ SafePaintOpData<SkScalar>::type left; |
+ SafePaintOpData<SkScalar>::type top; |
}; |
struct CC_PAINT_EXPORT DrawImageRectOp final : PaintOpWithFlags { |
@@ -450,10 +547,10 @@ struct CC_PAINT_EXPORT DrawImageRectOp final : PaintOpWithFlags { |
const SkMatrix& original_ctm); |
bool HasDiscardableImages() const; |
- PaintImage image; |
- SkRect src; |
- SkRect dst; |
- PaintCanvas::SrcRectConstraint constraint; |
+ SafePaintOpData<PaintImage>::type image; |
+ SafePaintOpData<SkRect>::type src; |
+ SafePaintOpData<SkRect>::type dst; |
+ SafePaintOpData<PaintCanvas::SrcRectConstraint>::type constraint; |
}; |
struct CC_PAINT_EXPORT DrawIRectOp final : PaintOpWithFlags { |
@@ -472,7 +569,7 @@ struct CC_PAINT_EXPORT DrawIRectOp final : PaintOpWithFlags { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkIRect rect; |
+ SafePaintOpData<SkIRect>::type rect; |
}; |
struct CC_PAINT_EXPORT DrawLineOp final : PaintOpWithFlags { |
@@ -497,10 +594,10 @@ struct CC_PAINT_EXPORT DrawLineOp final : PaintOpWithFlags { |
int CountSlowPaths() const; |
- SkScalar x0; |
- SkScalar y0; |
- SkScalar x1; |
- SkScalar y1; |
+ SafePaintOpData<SkScalar>::type x0; |
+ SafePaintOpData<SkScalar>::type y0; |
+ SafePaintOpData<SkScalar>::type x1; |
+ SafePaintOpData<SkScalar>::type y1; |
}; |
struct CC_PAINT_EXPORT DrawOvalOp final : PaintOpWithFlags { |
@@ -519,7 +616,7 @@ struct CC_PAINT_EXPORT DrawOvalOp final : PaintOpWithFlags { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkRect oval; |
+ SafePaintOpData<SkRect>::type oval; |
}; |
struct CC_PAINT_EXPORT DrawPathOp final : PaintOpWithFlags { |
@@ -539,7 +636,7 @@ struct CC_PAINT_EXPORT DrawPathOp final : PaintOpWithFlags { |
const SkMatrix& original_ctm); |
int CountSlowPaths() const; |
- ThreadsafePath path; |
+ SafePaintOpData<ThreadsafePath>::type path; |
}; |
struct CC_PAINT_EXPORT DrawPosTextOp final : PaintOpWithArray<SkPoint> { |
@@ -576,7 +673,7 @@ struct CC_PAINT_EXPORT DrawRecordOp final : PaintOp { |
bool HasDiscardableImages() const; |
int CountSlowPaths() const; |
- sk_sp<const PaintRecord> record; |
+ SafePaintOpData<sk_sp<const PaintRecord>>::type record; |
}; |
struct CC_PAINT_EXPORT DrawRectOp final : PaintOpWithFlags { |
@@ -595,7 +692,7 @@ struct CC_PAINT_EXPORT DrawRectOp final : PaintOpWithFlags { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkRect rect; |
+ SafePaintOpData<SkRect>::type rect; |
}; |
struct CC_PAINT_EXPORT DrawRRectOp final : PaintOpWithFlags { |
@@ -614,7 +711,7 @@ struct CC_PAINT_EXPORT DrawRRectOp final : PaintOpWithFlags { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkRRect rrect; |
+ SafePaintOpData<SkRRect>::type rrect; |
}; |
struct CC_PAINT_EXPORT DrawTextOp final : PaintOpWithData { |
@@ -636,8 +733,8 @@ struct CC_PAINT_EXPORT DrawTextOp final : PaintOpWithData { |
void* GetData() { return GetDataForThis(this); } |
const void* GetData() const { return GetDataForThis(this); } |
- SkScalar x; |
- SkScalar y; |
+ SafePaintOpData<SkScalar>::type x; |
+ SafePaintOpData<SkScalar>::type y; |
}; |
struct CC_PAINT_EXPORT DrawTextBlobOp final : PaintOpWithFlags { |
@@ -659,9 +756,9 @@ struct CC_PAINT_EXPORT DrawTextBlobOp final : PaintOpWithFlags { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- sk_sp<SkTextBlob> blob; |
- SkScalar x; |
- SkScalar y; |
+ SafePaintOpData<sk_sp<SkTextBlob>>::type blob; |
+ SafePaintOpData<SkScalar>::type x; |
+ SafePaintOpData<SkScalar>::type y; |
}; |
struct CC_PAINT_EXPORT NoopOp final : PaintOp { |
@@ -685,7 +782,7 @@ struct CC_PAINT_EXPORT RotateOp final : PaintOp { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkScalar degrees; |
+ SafePaintOpData<SkScalar>::type degrees; |
}; |
struct CC_PAINT_EXPORT SaveOp final : PaintOp { |
@@ -711,7 +808,7 @@ struct CC_PAINT_EXPORT SaveLayerOp final : PaintOpWithFlags { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkRect bounds; |
+ SafePaintOpData<SkRect>::type bounds; |
}; |
struct CC_PAINT_EXPORT SaveLayerAlphaOp final : PaintOp { |
@@ -722,8 +819,8 @@ struct CC_PAINT_EXPORT SaveLayerAlphaOp final : PaintOp { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkRect bounds; |
- uint8_t alpha; |
+ SafePaintOpData<SkRect>::type bounds; |
+ SafePaintOpData<uint8_t>::type alpha; |
}; |
struct CC_PAINT_EXPORT ScaleOp final : PaintOp { |
@@ -733,8 +830,8 @@ struct CC_PAINT_EXPORT ScaleOp final : PaintOp { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkScalar sx; |
- SkScalar sy; |
+ SafePaintOpData<SkScalar>::type sx; |
+ SafePaintOpData<SkScalar>::type sy; |
}; |
struct CC_PAINT_EXPORT SetMatrixOp final : PaintOp { |
@@ -750,7 +847,7 @@ struct CC_PAINT_EXPORT SetMatrixOp final : PaintOp { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- ThreadsafeMatrix matrix; |
+ SafePaintOpData<ThreadsafeMatrix>::type matrix; |
}; |
struct CC_PAINT_EXPORT TranslateOp final : PaintOp { |
@@ -760,8 +857,8 @@ struct CC_PAINT_EXPORT TranslateOp final : PaintOp { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkScalar dx; |
- SkScalar dy; |
+ SafePaintOpData<SkScalar>::type dx; |
+ SafePaintOpData<SkScalar>::type dy; |
}; |
using LargestPaintOp = DrawDRRectOp; |