Skip to content

Commit

Permalink
[Impeller] Do not call std::forward on the serialized arguments in th…
Browse files Browse the repository at this point in the history
…e canvas recorder (flutter#52307)

std::forward has move semantics and can not safely be called twice on the same arguments.

Also fix CanvasRecorder's resolution of Canvas::Save, which has a default parameter value.
  • Loading branch information
jason-simmons authored Apr 23, 2024
1 parent 066953b commit 10ee150
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 5 deletions.
5 changes: 3 additions & 2 deletions impeller/aiks/canvas_recorder.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ class CanvasRecorder {
-> decltype((std::declval<Canvas>().*
canvasMethod)(std::forward<Args>(args)...)) {
// Serialize each argument
(serializer_.Write(std::forward<Args>(args)), ...);
(serializer_.Write(args), ...);
serializer_.Write(op);
return (canvas_.*canvasMethod)(std::forward<Args>(args)...);
}
Expand All @@ -110,7 +110,8 @@ class CanvasRecorder {
//////////////////////////////////////////////////////////////////////////////

void Save(uint32_t total_content_depth = Canvas::kMaxDepth) {
return ExecuteAndSerialize(FLT_CANVAS_RECORDER_OP_ARG(Save),
void (Canvas::*save_method)(uint32_t) = &Canvas::Save;
return ExecuteAndSerialize(CanvasRecorderOp::kSave, save_method,
total_content_depth);
}

Expand Down
13 changes: 10 additions & 3 deletions impeller/aiks/canvas_recorder_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class Serializer {
public:
void Write(CanvasRecorderOp op) { last_op_ = op; }

void Write(const Paint& paint) {}
void Write(const Paint& paint) { last_paint_ = paint; }

void Write(const std::optional<Rect> optional_rect) {}

Expand Down Expand Up @@ -59,6 +59,7 @@ class Serializer {
void Write(const ContentBoundsPromise& promise) {}

CanvasRecorderOp last_op_;
Paint last_paint_;
};
} // namespace

Expand Down Expand Up @@ -152,8 +153,11 @@ TEST(CanvasRecorder, DrawPath) {

TEST(CanvasRecorder, DrawPaint) {
CanvasRecorder<Serializer> recorder;
recorder.DrawPaint(Paint());
Paint paint;
paint.color = Color::Red();
recorder.DrawPaint(paint);
ASSERT_EQ(recorder.GetSerializer().last_op_, CanvasRecorderOp::kDrawPaint);
ASSERT_EQ(recorder.GetSerializer().last_paint_.color, paint.color);
}

TEST(CanvasRecorder, DrawLine) {
Expand All @@ -164,8 +168,11 @@ TEST(CanvasRecorder, DrawLine) {

TEST(CanvasRecorder, DrawRect) {
CanvasRecorder<Serializer> recorder;
recorder.DrawRect(Rect(), Paint());
Paint paint;
paint.color = Color::Blue();
recorder.DrawRect(Rect(), paint);
ASSERT_EQ(recorder.GetSerializer().last_op_, CanvasRecorderOp::kDrawRect);
ASSERT_EQ(recorder.GetSerializer().last_paint_.color, paint.color);
}

TEST(CanvasRecorder, DrawOval) {
Expand Down

0 comments on commit 10ee150

Please sign in to comment.