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

[Impeller] Fix stroke cap drawing not correct. #39481

Merged
merged 2 commits into from
Feb 8, 2023
Merged

[Impeller] Fix stroke cap drawing not correct. #39481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f34d3f2
++
luckysmg Feb 8, 2023
f5bceb1
++
luckysmg Feb 8, 2023
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
23 changes: 22 additions & 1 deletion impeller/display_list/display_list_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,34 @@ TEST_P(DisplayListTest, CanDrawArc) {
static float stroke_width = 10;
static bool use_center = true;

static int selected_cap = 0;
const char* cap_names[] = {"Butt", "Round", "Square"};
flutter::DlStrokeCap cap;

ImGui::Begin("Controls", nullptr, ImGuiWindowFlags_AlwaysAutoResize);
ImGui::SliderFloat("Start angle", &start_angle, -360, 360);
ImGui::SliderFloat("Sweep angle", &sweep_angle, -360, 360);
ImGui::SliderFloat("Stroke width", &stroke_width, 0, 100);
ImGui::Combo("Cap", &selected_cap, cap_names,
sizeof(cap_names) / sizeof(char*));
ImGui::Checkbox("Use center", &use_center);
ImGui::End();

switch (selected_cap) {
case 0:
cap = flutter::DlStrokeCap::kButt;
break;
case 1:
cap = flutter::DlStrokeCap::kRound;
break;
case 2:
cap = flutter::DlStrokeCap::kSquare;
break;
default:
cap = flutter::DlStrokeCap::kButt;
break;
}

auto [p1, p2] = IMPELLER_PLAYGROUND_LINE(
Point(200, 200), Point(400, 400), 20, Color::White(), Color::White());

Expand All @@ -136,7 +157,7 @@ TEST_P(DisplayListTest, CanDrawArc) {
Vector2 scale = GetContentScale();
builder.scale(scale.x, scale.y);
builder.setStyle(flutter::DlDrawStyle::kStroke);
builder.setStrokeCap(flutter::DlStrokeCap::kButt);
builder.setStrokeCap(cap);
builder.setStrokeJoin(flutter::DlStrokeJoin::kMiter);
builder.setStrokeMiter(10);
auto rect = SkRect::MakeLTRB(p1.x, p1.y, p2.x, p2.y);
Expand Down
16 changes: 12 additions & 4 deletions impeller/entity/geometry.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ VertexBuffer StrokePathGeometry::CreateSolidStrokeVertices(
HostBuffer& buffer,
Scalar stroke_width,
Scalar scaled_miter_limit,
Cap cap,
const StrokePathGeometry::JoinProc& join_proc,
const StrokePathGeometry::CapProc& cap_proc,
Scalar tolerance) {
Expand Down Expand Up @@ -370,9 +371,15 @@ VertexBuffer StrokePathGeometry::CreateSolidStrokeVertices(

// Generate start cap.
if (!polyline.contours[contour_i].is_closed) {
auto cap_offset =
Vector2(contour.start_direction.y, -contour.start_direction.x) *
stroke_width * 0.5;
Vector2 direction;
if (cap == Cap::kButt) {
direction =
Vector2(contour.start_direction.y, -contour.start_direction.x);
Copy link
Member

Choose a reason for hiding this comment

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

This special case shouldn't be needed for kButt. Is something broken when it's removed?

Copy link
Contributor Author

@luckysmg luckysmg Feb 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

When I just set direction = Vector2(-contour.start_direction.y, contour.start_direction.x)

Render like this...Seems not correct

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I just realized what's happening -- this is probably why I accidentally flipped it in the wrong direction when landing #39124 in the first place...

This issue may occur for the other caps if the conditions are just right. I need to swing back around and rework the bridging solution, but this should be safe to land and cherry-pick. Thanks!

} else {
direction =
Vector2(-contour.start_direction.y, contour.start_direction.x);
}
auto cap_offset = direction * stroke_width * 0.5;
cap_proc(vtx_builder, polyline.points[contour_start_point_i], cap_offset,
tolerance);
}
Expand Down Expand Up @@ -439,7 +446,8 @@ GeometryResult StrokePathGeometry::GetPositionBuffer(
auto& host_buffer = pass.GetTransientsBuffer();
auto vertex_buffer = CreateSolidStrokeVertices(
path_, host_buffer, stroke_width, miter_limit_ * stroke_width_ * 0.5,
GetJoinProc(stroke_join_), GetCapProc(stroke_cap_), tolerance);
stroke_cap_, GetJoinProc(stroke_join_), GetCapProc(stroke_cap_),
tolerance);

return GeometryResult{
.type = PrimitiveType::kTriangleStrip,
Expand Down
1 change: 1 addition & 0 deletions impeller/entity/geometry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ class StrokePathGeometry : public Geometry {
HostBuffer& buffer,
Scalar stroke_width,
Scalar scaled_miter_limit,
Cap cap,
const JoinProc& join_proc,
const CapProc& cap_proc,
Scalar tolerance);
Expand Down