Index: cc/paint/paint_op_buffer.h |
diff --git a/cc/paint/paint_op_buffer.h b/cc/paint/paint_op_buffer.h |
index e8d31142123089221a75cfde8da6d15b60ccdfef..329b626bdc08d039e758a6a5c57fedbee5b57cc5 100644 |
--- a/cc/paint/paint_op_buffer.h |
+++ b/cc/paint/paint_op_buffer.h |
@@ -72,6 +72,117 @@ enum class PaintOpType : uint8_t { |
LastPaintOpType = Translate, |
}; |
+template <typename T> |
+struct SafePaintOpData; |
+ |
+// 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; |
+}; |
+ |
+// These types can be copied trivially. |
+// TODO(danakj): These could use std::is_trivially_move_constructible once that |
+// is available on all our toolchains. |
+template <> |
+struct SafePaintOpData<SkRect> { |
+ using type = SkRect; |
+ static_assert(base::is_trivially_copyable<type>::value, ""); |
+}; |
+template <> |
+struct SafePaintOpData<SkIRect> { |
+ using type = SkIRect; |
+ static_assert(base::is_trivially_copyable<type>::value, ""); |
+}; |
+template <> |
+struct SafePaintOpData<SkRRect> { |
+ using type = SkRRect; |
+ static_assert(base::is_trivially_copyable<type>::value, ""); |
+}; |
+template <> |
+struct SafePaintOpData<SkClipOp> { |
+ using type = SkClipOp; |
+ static_assert(base::is_trivially_copyable<type>::value, ""); |
+}; |
+template <> |
+struct SafePaintOpData<SkBlendMode> { |
+ using type = SkBlendMode; |
+ static_assert(base::is_trivially_copyable<type>::value, ""); |
+}; |
+template <> |
+struct SafePaintOpData<SkColor> { |
+ using type = SkColor; |
+ static_assert(base::is_trivially_copyable<type>::value, ""); |
+}; |
+template <> |
+struct SafePaintOpData<PaintCanvas::AnnotationType> { |
+ using type = PaintCanvas::AnnotationType; |
+ static_assert(base::is_trivially_copyable<type>::value, ""); |
+}; |
+template <> |
+struct SafePaintOpData<PaintCanvas::SrcRectConstraint> { |
+ using type = PaintCanvas::SrcRectConstraint; |
+ static_assert(base::is_trivially_copyable<type>::value, ""); |
+}; |
+template <> |
+struct SafePaintOpData<ThreadsafeMatrix> { |
+ using type = ThreadsafeMatrix; |
+ static_assert(base::is_trivially_copyable<type>::value, ""); |
+}; |
+ |
+// 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). 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. |
+// For non-primitive fields of type T in a PaintOp, declare them as a |
+// SafePaintOpData<T>::type, which provides us with an explanation of safety |
+// for each type. |
+// NOTABLY 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 { |
uint32_t type : 8; |
uint32_t skip : 24; |
@@ -124,9 +235,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 |
@@ -168,8 +283,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. |
@@ -239,9 +362,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 { |
@@ -253,8 +376,8 @@ struct CC_PAINT_EXPORT ClipPathOp final : PaintOp { |
const SkMatrix& original_ctm); |
int CountSlowPaths() const; |
- ThreadsafePath path; |
- SkClipOp op; |
+ SafePaintOpData<ThreadsafePath>::type path; |
+ SafePaintOpData<SkClipOp>::type op; |
bool antialias; |
}; |
@@ -266,8 +389,8 @@ struct CC_PAINT_EXPORT ClipRectOp final : PaintOp { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkRect rect; |
- SkClipOp op; |
+ SafePaintOpData<SkRect>::type rect; |
+ SafePaintOpData<SkClipOp>::type op; |
bool antialias; |
}; |
@@ -279,8 +402,8 @@ struct CC_PAINT_EXPORT ClipRRectOp final : PaintOp { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkRRect rrect; |
- SkClipOp op; |
+ SafePaintOpData<SkRRect>::type rrect; |
+ SafePaintOpData<SkClipOp>::type op; |
bool antialias; |
}; |
@@ -291,15 +414,15 @@ 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 { |
static constexpr PaintOpType kType = PaintOpType::DrawArc; |
static constexpr bool kIsDrawOp = true; |
DrawArcOp(const SkRect& oval, |
- SkScalar start_angle, |
- SkScalar sweep_angle, |
+ float start_angle, |
+ float sweep_angle, |
bool use_center, |
const PaintFlags& flags) |
: PaintOpWithFlags(flags), |
@@ -318,19 +441,16 @@ struct CC_PAINT_EXPORT DrawArcOp final : PaintOpWithFlags { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkRect oval; |
- SkScalar start_angle; |
- SkScalar sweep_angle; |
+ SafePaintOpData<SkRect>::type oval; |
+ float start_angle; |
+ float sweep_angle; |
bool use_center; |
}; |
struct CC_PAINT_EXPORT DrawCircleOp final : PaintOpWithFlags { |
static constexpr PaintOpType kType = PaintOpType::DrawCircle; |
static constexpr bool kIsDrawOp = true; |
- DrawCircleOp(SkScalar cx, |
- SkScalar cy, |
- SkScalar radius, |
- const PaintFlags& flags) |
+ DrawCircleOp(float cx, float cy, float radius, const PaintFlags& flags) |
: PaintOpWithFlags(flags), cx(cx), cy(cy), radius(radius) {} |
static void Raster(const PaintOp* op, |
SkCanvas* canvas, |
@@ -343,9 +463,9 @@ struct CC_PAINT_EXPORT DrawCircleOp final : PaintOpWithFlags { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkScalar cx; |
- SkScalar cy; |
- SkScalar radius; |
+ float cx; |
+ float cy; |
+ float radius; |
}; |
struct CC_PAINT_EXPORT DrawColorOp final : PaintOp { |
@@ -356,8 +476,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 { |
@@ -377,7 +497,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 { |
@@ -398,16 +518,16 @@ 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 { |
static constexpr PaintOpType kType = PaintOpType::DrawImage; |
static constexpr bool kIsDrawOp = true; |
DrawImageOp(const PaintImage& image, |
- SkScalar left, |
- SkScalar top, |
+ float left, |
+ float top, |
const PaintFlags* flags); |
~DrawImageOp(); |
static void Raster(const PaintOp* op, |
@@ -422,9 +542,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; |
+ float left; |
+ float top; |
}; |
struct CC_PAINT_EXPORT DrawImageRectOp final : PaintOpWithFlags { |
@@ -448,10 +568,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 { |
@@ -470,17 +590,13 @@ 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 { |
static constexpr PaintOpType kType = PaintOpType::DrawLine; |
static constexpr bool kIsDrawOp = true; |
- DrawLineOp(SkScalar x0, |
- SkScalar y0, |
- SkScalar x1, |
- SkScalar y1, |
- const PaintFlags& flags) |
+ DrawLineOp(float x0, float y0, float x1, float y1, const PaintFlags& flags) |
: PaintOpWithFlags(flags), x0(x0), y0(y0), x1(x1), y1(y1) {} |
static void Raster(const PaintOp* op, |
SkCanvas* canvas, |
@@ -495,10 +611,10 @@ struct CC_PAINT_EXPORT DrawLineOp final : PaintOpWithFlags { |
int CountSlowPaths() const; |
- SkScalar x0; |
- SkScalar y0; |
- SkScalar x1; |
- SkScalar y1; |
+ float x0; |
+ float y0; |
+ float x1; |
+ float y1; |
}; |
struct CC_PAINT_EXPORT DrawOvalOp final : PaintOpWithFlags { |
@@ -517,7 +633,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 { |
@@ -537,7 +653,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> { |
@@ -574,7 +690,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 { |
@@ -593,7 +709,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 { |
@@ -612,13 +728,13 @@ 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 { |
static constexpr PaintOpType kType = PaintOpType::DrawText; |
static constexpr bool kIsDrawOp = true; |
- DrawTextOp(size_t bytes, SkScalar x, SkScalar y, const PaintFlags& flags) |
+ DrawTextOp(size_t bytes, float x, float y, const PaintFlags& flags) |
: PaintOpWithData(flags, bytes), x(x), y(y) {} |
static void Raster(const PaintOp* op, |
SkCanvas* canvas, |
@@ -634,16 +750,16 @@ struct CC_PAINT_EXPORT DrawTextOp final : PaintOpWithData { |
void* GetData() { return GetDataForThis(this); } |
const void* GetData() const { return GetDataForThis(this); } |
- SkScalar x; |
- SkScalar y; |
+ float x; |
+ float y; |
}; |
struct CC_PAINT_EXPORT DrawTextBlobOp final : PaintOpWithFlags { |
static constexpr PaintOpType kType = PaintOpType::DrawTextBlob; |
static constexpr bool kIsDrawOp = true; |
DrawTextBlobOp(sk_sp<SkTextBlob> blob, |
- SkScalar x, |
- SkScalar y, |
+ float x, |
+ float y, |
const PaintFlags& flags); |
~DrawTextBlobOp(); |
static void Raster(const PaintOp* op, |
@@ -657,9 +773,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; |
+ float x; |
+ float y; |
}; |
struct CC_PAINT_EXPORT NoopOp final : PaintOp { |
@@ -678,12 +794,12 @@ struct CC_PAINT_EXPORT RestoreOp final : PaintOp { |
struct CC_PAINT_EXPORT RotateOp final : PaintOp { |
static constexpr PaintOpType kType = PaintOpType::Rotate; |
- explicit RotateOp(SkScalar degrees) : degrees(degrees) {} |
+ explicit RotateOp(float degrees) : degrees(degrees) {} |
static void Raster(const PaintOp* op, |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkScalar degrees; |
+ float degrees; |
enne (OOO)
2017/06/01 16:04:58
If you think we should get rid of SkScalar types,
danakj
2017/06/01 18:01:17
I thought it made sense to use float here because
enne (OOO)
2017/06/01 18:07:33
I didn't see anything wrong with SafePaintOpData<S
|
}; |
struct CC_PAINT_EXPORT SaveOp final : PaintOp { |
@@ -709,7 +825,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 { |
@@ -720,19 +836,19 @@ struct CC_PAINT_EXPORT SaveLayerAlphaOp final : PaintOp { |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkRect bounds; |
+ SafePaintOpData<SkRect>::type bounds; |
uint8_t alpha; |
}; |
struct CC_PAINT_EXPORT ScaleOp final : PaintOp { |
static constexpr PaintOpType kType = PaintOpType::Scale; |
- ScaleOp(SkScalar sx, SkScalar sy) : sx(sx), sy(sy) {} |
+ ScaleOp(float sx, float sy) : sx(sx), sy(sy) {} |
static void Raster(const PaintOp* op, |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkScalar sx; |
- SkScalar sy; |
+ float sx; |
+ float sy; |
}; |
struct CC_PAINT_EXPORT SetMatrixOp final : PaintOp { |
@@ -748,18 +864,18 @@ 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 { |
static constexpr PaintOpType kType = PaintOpType::Translate; |
- TranslateOp(SkScalar dx, SkScalar dy) : dx(dx), dy(dy) {} |
+ TranslateOp(float dx, float dy) : dx(dx), dy(dy) {} |
static void Raster(const PaintOp* op, |
SkCanvas* canvas, |
const SkMatrix& original_ctm); |
- SkScalar dx; |
- SkScalar dy; |
+ float dx; |
+ float dy; |
}; |
using LargestPaintOp = DrawDRRectOp; |