Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Add a display list op to clear the transformation stack. #32050

Merged
merged 1 commit into from
Mar 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ deps = {
'src': 'https://github.com/flutter/buildroot.git' + '@' + '8cbf38af7d48cc298ae86e614533b4b2d0dc6758',

'src/flutter/impeller':
Var('github_git') + '/flutter/impeller' + '@' + 'c1572a3335c4a533dacc28b86cbebdf08b5a57ed',
Var('github_git') + '/flutter/impeller' + '@' + 'b662deba147396d68e549e41d5d2d6b1cb0b0119',

# Fuchsia compatibility
#
Expand Down
1 change: 1 addition & 0 deletions display_list/display_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ namespace flutter {
V(Skew) \
V(Transform2DAffine) \
V(TransformFullPerspective) \
V(TransformReset) \
\
V(ClipIntersectRect) \
V(ClipIntersectRRect) \
Expand Down
4 changes: 3 additions & 1 deletion display_list/display_list_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -427,8 +427,10 @@ void DisplayListBuilder::transformFullPerspective(
mwx, mwy, mwz, mwt);
}
}

// clang-format on
void DisplayListBuilder::transformReset() {
Push<TransformResetOp>(0, 0);
}

void DisplayListBuilder::clipRect(const SkRect& rect,
SkClipOp clip_op,
Expand Down
2 changes: 1 addition & 1 deletion display_list/display_list_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,8 @@ class DisplayListBuilder final : public virtual Dispatcher,
SkScalar myx, SkScalar myy, SkScalar myz, SkScalar myt,
SkScalar mzx, SkScalar mzy, SkScalar mzz, SkScalar mzt,
SkScalar mwx, SkScalar mwy, SkScalar mwz, SkScalar mwt) override;

// clang-format on
void transformReset() override;

void clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) override;
void clipRRect(const SkRRect& rrect, SkClipOp clip_op, bool is_aa) override;
Expand Down
3 changes: 3 additions & 0 deletions display_list/display_list_canvas_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ void DisplayListCanvasDispatcher::transformFullPerspective(
mwx, mwy, mwz, mwt));
}
// clang-format on
void DisplayListCanvasDispatcher::transformReset() {
canvas_->resetMatrix();
}

void DisplayListCanvasDispatcher::clipRect(const SkRect& rect,
SkClipOp clip_op,
Expand Down
1 change: 1 addition & 0 deletions display_list/display_list_canvas_dispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class DisplayListCanvasDispatcher : public virtual Dispatcher,
SkScalar mzx, SkScalar mzy, SkScalar mzz, SkScalar mzt,
SkScalar mwx, SkScalar mwy, SkScalar mwz, SkScalar mwt) override;
// clang-format on
void transformReset() override;

void clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) override;
void clipRRect(const SkRRect& rrect, SkClipOp clip_op, bool is_aa) override;
Expand Down
9 changes: 4 additions & 5 deletions display_list/display_list_canvas_recorder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ void DisplayListCanvasRecorder::didConcat44(const SkM44& m44) {
m44.rc(3, 0), m44.rc(3, 1), m44.rc(3, 2), m44.rc(3, 3));
}
// clang-format on
void DisplayListCanvasRecorder::didSetM44(const SkM44& matrix) {
builder_->transformReset();
didConcat44(matrix);
}
void DisplayListCanvasRecorder::didTranslate(SkScalar tx, SkScalar ty) {
builder_->translate(tx, ty);
}
Expand Down Expand Up @@ -230,11 +234,6 @@ void DisplayListCanvasRecorder::onDrawShadowRec(const SkPath& path,
<< __FUNCTION__;
}

void DisplayListCanvasRecorder::didSetM44(const SkM44&) {
FML_DLOG(ERROR) << "Unimplemented DisplayListCanvasRecorder::"
<< __FUNCTION__;
}

void DisplayListCanvasRecorder::onDrawBehind(const SkPaint&) {
FML_DLOG(ERROR) << "Unimplemented DisplayListCanvasRecorder::"
<< __FUNCTION__;
Expand Down
4 changes: 3 additions & 1 deletion display_list/display_list_dispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,11 @@ class Dispatcher {
SkScalar myx, SkScalar myy, SkScalar myz, SkScalar myt,
SkScalar mzx, SkScalar mzy, SkScalar mzz, SkScalar mzt,
SkScalar mwx, SkScalar mwy, SkScalar mwz, SkScalar mwt) = 0;

// clang-format on

// Clears the transformation stack.
virtual void transformReset() = 0;

virtual void clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) = 0;
virtual void clipRRect(const SkRRect& rrect,
SkClipOp clip_op,
Expand Down
9 changes: 9 additions & 0 deletions display_list/display_list_ops.h
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,15 @@ struct TransformFullPerspectiveOp final : DLOp {
}
};

// 4 byte header with no payload.
struct TransformResetOp final : DLOp {
static const auto kType = DisplayListOpType::kTransformReset;

TransformResetOp() = default;

void dispatch(Dispatcher& dispatcher) const { dispatcher.transformReset(); }
};

// 4 byte header + 4 byte common payload packs into minimum 8 bytes
// SkRect is 16 more bytes, which packs efficiently into 24 bytes total
// SkRRect is 52 more bytes, which rounds up to 56 bytes (4 bytes unused)
Expand Down
12 changes: 12 additions & 0 deletions display_list/display_list_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1370,6 +1370,18 @@ TEST(DisplayList, DisplayListFullPerspectiveTransformHandling) {
}
}

TEST(DisplayList, DisplayListTransformResetHandling) {
DisplayListBuilder builder;
builder.scale(20.0, 20.0);
builder.transformReset();
auto list = builder.Build();
ASSERT_NE(list, nullptr);
sk_sp<SkSurface> surface = SkSurface::MakeRasterN32Premul(10, 10);
SkCanvas* canvas = surface->getCanvas();
list->RenderTo(canvas);
ASSERT_TRUE(canvas->getTotalMatrix().isIdentity());
}

TEST(DisplayList, SingleOpsMightSupportGroupOpacityWithOrWithoutBlendMode) {
auto run_tests = [](std::string name,
void build(DisplayListBuilder & builder),
Expand Down
5 changes: 5 additions & 0 deletions display_list/display_list_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ void SkMatrixDispatchHelper::transformFullPerspective(

// clang-format on

void SkMatrixDispatchHelper::transformReset() {
matrix_ = {};
matrix33_ = {};
Copy link
Member Author

@chinmaygarde chinmaygarde Mar 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I think saved_ needs to be cleared as well. And, I think there is a bug here where calling an imbalaned restore will call pop_back on an empty vector which is undefined behavior.

Fixing both issues.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind. I can't confirm this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SkMatrixDispatchHelper already has a reset() method. I'm not sure if it is used any more (I used to collect saveLayer bounds in their native coordinate space, but had to back off on that due to bugs).

You don't want saved_ to be cleared, if they restore() then they should undo the reset. The set to identity should not escape the enclosing save/restore.

There probably should be a protection here for over-restore, but the caller generally already does that. It would be redundant for existing use cases, but we shouldn't rely on that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

void SkMatrixDispatchHelper::save() {
saved_.push_back(matrix_);
}
Expand Down
3 changes: 3 additions & 0 deletions display_list/display_list_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class IgnoreTransformDispatchHelper : public virtual Dispatcher {
SkScalar mzx, SkScalar mzy, SkScalar mzz, SkScalar mzt,
SkScalar mwx, SkScalar mwy, SkScalar mwz, SkScalar mwt) override {}
// clang-format on
void transformReset() override {}
};

class IgnoreDrawDispatchHelper : public virtual Dispatcher {
Expand Down Expand Up @@ -273,6 +274,8 @@ class SkMatrixDispatchHelper : public virtual Dispatcher,

// clang-format on

void transformReset() override;

void save() override;
void restore() override;

Expand Down