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

Commit 343dfe8

Browse files
authored
[Impeller] Fixes stroke path geometry that can draw outside of path if the path ends at sharp turn. (#45252)
Fixes flutter/flutter#133032 The issue is that when we draw a line we always try to draw a complete rectangle from point 1 to point2 before drawing the join and the next line ![Untitled drawing (7)](https://github.com/flutter/engine/assets/47866232/c51aa4a8-bd8e-4764-9941-4e2968b4fb8e) This becomes a problem if there is a sharp turn ![Untitled drawing (6)](https://github.com/flutter/engine/assets/47866232/2b743792-52b7-402d-b2e8-f08ed47c0943) ![Untitled drawing (5)](https://github.com/flutter/engine/assets/47866232/794a75c9-5e87-4548-a264-7dd9cf088ae7) Notice there will be a slight over drawing at the bottom. The solution then is to not draw the rect first before drawing the join. It will be the join's responsibility to draw the complete the rect from the line start to the join. This should also save a lot of repeatedly drawing during the join ![rect](https://github.com/flutter/engine/assets/47866232/24802df6-69b5-4543-8d09-fcfbff4cedbb) [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1 parent b71b366 commit 343dfe8

File tree

6 files changed

+151
-13
lines changed

6 files changed

+151
-13
lines changed

impeller/aiks/aiks_unittests.cc

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,6 +1206,37 @@ TEST_P(AiksTest, CanRenderRoundedRectWithNonUniformRadii) {
12061206
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
12071207
}
12081208

1209+
TEST_P(AiksTest, CanRenderStrokePathThatEndsAtSharpTurn) {
1210+
Canvas canvas;
1211+
1212+
Paint paint;
1213+
paint.color = Color::Red();
1214+
paint.style = Paint::Style::kStroke;
1215+
paint.stroke_width = 200;
1216+
1217+
Rect rect = {100, 100, 200, 200};
1218+
PathBuilder builder;
1219+
builder.AddArc(rect, Degrees(0), Degrees(90), false);
1220+
1221+
canvas.DrawPath(builder.TakePath(), paint);
1222+
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
1223+
}
1224+
1225+
TEST_P(AiksTest, CanRenderStrokePathWithCubicLine) {
1226+
Canvas canvas;
1227+
1228+
Paint paint;
1229+
paint.color = Color::Red();
1230+
paint.style = Paint::Style::kStroke;
1231+
paint.stroke_width = 20;
1232+
1233+
PathBuilder builder;
1234+
builder.AddCubicCurve({0, 200}, {50, 400}, {350, 0}, {400, 200});
1235+
1236+
canvas.DrawPath(builder.TakePath(), paint);
1237+
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
1238+
}
1239+
12091240
TEST_P(AiksTest, CanRenderDifferencePaths) {
12101241
Canvas canvas;
12111242

@@ -1952,6 +1983,22 @@ TEST_P(AiksTest, DrawRectStrokesRenderCorrectly) {
19521983
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
19531984
}
19541985

1986+
TEST_P(AiksTest, DrawRectStrokesWithBevelJoinRenderCorrectly) {
1987+
Canvas canvas;
1988+
Paint paint;
1989+
paint.color = Color::Red();
1990+
paint.style = Paint::Style::kStroke;
1991+
paint.stroke_width = 10;
1992+
paint.stroke_join = Join::kBevel;
1993+
1994+
canvas.Translate({100, 100});
1995+
canvas.DrawPath(
1996+
PathBuilder{}.AddRect(Rect::MakeSize(Size{100, 100})).TakePath(),
1997+
{paint});
1998+
1999+
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
2000+
}
2001+
19552002
TEST_P(AiksTest, SaveLayerDrawsBehindSubsequentEntities) {
19562003
// Compare with https://fiddle.skia.org/c/9e03de8567ffb49e7e83f53b64bcf636
19572004
Canvas canvas;

impeller/core/formats.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,11 +325,33 @@ enum class IndexType {
325325
kNone,
326326
};
327327

328+
/// Decides how backend draws pixels based on input vertices.
328329
enum class PrimitiveType {
330+
/// Draws a triage for each separate set of three vertices.
331+
///
332+
/// Vertices [A, B, C, D, E, F] will produce triages
333+
/// [ABC, DEF].
329334
kTriangle,
335+
336+
/// Draws a triage for every adjacent three vertices.
337+
///
338+
/// Vertices [A, B, C, D, E, F] will produce triages
339+
/// [ABC, BCD, CDE, DEF].
330340
kTriangleStrip,
341+
342+
/// Draws a line for each separate set of two vertices.
343+
///
344+
/// Vertices [A, B, C] will produce discontinued line
345+
/// [AB, BC].
331346
kLine,
347+
348+
/// Draws a continuous line that connect every input vertices
349+
///
350+
/// Vertices [A, B, C] will produce one continuous line
351+
/// [ABC].
332352
kLineStrip,
353+
354+
/// Draws a point at each input vertex.
333355
kPoint,
334356
// Triangle fans are implementation dependent and need extra extensions
335357
// checks. Hence, they are not supported here.

impeller/entity/geometry/stroke_path_geometry.cc

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ StrokePathGeometry::CreateSolidStrokeVertices(
253253
for (size_t contour_i = 0; contour_i < polyline.contours.size();
254254
contour_i++) {
255255
auto contour = polyline.contours[contour_i];
256+
size_t contour_component_i = 0;
256257
size_t contour_start_point_i, contour_end_point_i;
257258
std::tie(contour_start_point_i, contour_end_point_i) =
258259
polyline.GetContourPointBounds(contour_i);
@@ -308,27 +309,55 @@ StrokePathGeometry::CreateSolidStrokeVertices(
308309
// Generate contour geometry.
309310
for (size_t point_i = contour_start_point_i + 1;
310311
point_i < contour_end_point_i; point_i++) {
312+
if ((contour_component_i + 1 >= contour.components.size()) &&
313+
contour.components[contour_component_i + 1].component_start_index <=
314+
point_i) {
315+
// The point_i has entered the next component in this contour.
316+
contour_component_i += 1;
317+
}
311318
// Generate line rect.
312319
vtx.position = polyline.points[point_i - 1] + offset;
313320
vtx_builder.AppendVertex(vtx);
314321
vtx.position = polyline.points[point_i - 1] - offset;
315322
vtx_builder.AppendVertex(vtx);
316-
vtx.position = polyline.points[point_i] + offset;
317-
vtx_builder.AppendVertex(vtx);
318-
vtx.position = polyline.points[point_i] - offset;
319-
vtx_builder.AppendVertex(vtx);
320323

321-
if (point_i < contour_end_point_i - 1) {
322-
compute_offset(point_i + 1);
324+
auto is_end_of_contour = point_i == contour_end_point_i - 1;
325+
326+
if (!contour.components[contour_component_i].is_curve) {
327+
// For line components, two additional points need to be appended prior
328+
// to appending a join connecting the next component.
329+
vtx.position = polyline.points[point_i] + offset;
330+
vtx_builder.AppendVertex(vtx);
331+
vtx.position = polyline.points[point_i] - offset;
332+
vtx_builder.AppendVertex(vtx);
323333

324-
// Generate join from the current line to the next line.
325-
join_proc(vtx_builder, polyline.points[point_i], previous_offset,
326-
offset, scaled_miter_limit, scale);
334+
if (!is_end_of_contour) {
335+
compute_offset(point_i + 1);
336+
// Generate join from the current line to the next line.
337+
join_proc(vtx_builder, polyline.points[point_i], previous_offset,
338+
offset, scaled_miter_limit, scale);
339+
}
340+
} else {
341+
// For curve components, the polyline is detailed enough such that
342+
// it can avoid worrying about joins altogether.
343+
if (!is_end_of_contour) {
344+
compute_offset(point_i + 1);
345+
} else {
346+
// If this is a curve and is the end of the contour, two end points
347+
// need to be drawn with the contour end_direction.
348+
auto end_offset =
349+
Vector2(-contour.end_direction.y, contour.end_direction.x) *
350+
stroke_width * 0.5;
351+
vtx.position = polyline.points[contour_end_point_i - 1] + end_offset;
352+
vtx_builder.AppendVertex(vtx);
353+
vtx.position = polyline.points[contour_end_point_i - 1] - end_offset;
354+
vtx_builder.AppendVertex(vtx);
355+
}
327356
}
328357
}
329358

330359
// Generate end cap or join.
331-
if (!polyline.contours[contour_i].is_closed) {
360+
if (!contour.is_closed) {
332361
auto cap_offset =
333362
Vector2(-contour.end_direction.y, contour.end_direction.x) *
334363
stroke_width * 0.5; // Clockwise normal

impeller/geometry/path.cc

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -324,9 +324,10 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const {
324324
return Vector2(0, -1);
325325
};
326326

327+
std::vector<PolylineContour::Component> components;
327328
std::optional<size_t> previous_path_component_index;
328329
auto end_contour = [&polyline, &previous_path_component_index,
329-
&get_path_component]() {
330+
&get_path_component, &components]() {
330331
// Whenever a contour has ended, extract the exact end direction from the
331332
// last component.
332333
if (polyline.contours.empty()) {
@@ -339,6 +340,8 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const {
339340

340341
auto& contour = polyline.contours.back();
341342
contour.end_direction = Vector2(0, 1);
343+
contour.components = components;
344+
components.clear();
342345

343346
size_t previous_index = previous_path_component_index.value();
344347
while (!std::holds_alternative<std::monostate>(
@@ -363,14 +366,26 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const {
363366
const auto& component = components_[component_i];
364367
switch (component.type) {
365368
case ComponentType::kLinear:
369+
components.push_back({
370+
.component_start_index = polyline.points.size(),
371+
.is_curve = false,
372+
});
366373
collect_points(linears_[component.index].CreatePolyline());
367374
previous_path_component_index = component_i;
368375
break;
369376
case ComponentType::kQuadratic:
377+
components.push_back({
378+
.component_start_index = polyline.points.size(),
379+
.is_curve = true,
380+
});
370381
collect_points(quads_[component.index].CreatePolyline(scale));
371382
previous_path_component_index = component_i;
372383
break;
373384
case ComponentType::kCubic:
385+
components.push_back({
386+
.component_start_index = polyline.points.size(),
387+
.is_curve = true,
388+
});
374389
collect_points(cubics_[component.index].CreatePolyline(scale));
375390
previous_path_component_index = component_i;
376391
break;
@@ -386,13 +401,14 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const {
386401
const auto& contour = contours_[component.index];
387402
polyline.contours.push_back({.start_index = polyline.points.size(),
388403
.is_closed = contour.is_closed,
389-
.start_direction = start_direction});
404+
.start_direction = start_direction,
405+
.components = components});
390406
previous_contour_point = std::nullopt;
391407
collect_points({contour.destination});
392408
break;
393409
}
394-
end_contour();
395410
}
411+
end_contour();
396412
return polyline;
397413
}
398414

impeller/geometry/path.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,17 @@ class Path {
6161
};
6262

6363
struct PolylineContour {
64+
struct Component {
65+
size_t component_start_index;
66+
/// Denotes whether this component is a curve.
67+
///
68+
/// This is set to true when this component is generated from
69+
/// QuadraticComponent or CubicPathComponent.
70+
bool is_curve;
71+
};
6472
/// Index that denotes the first point of this contour.
6573
size_t start_index;
74+
6675
/// Denotes whether the last point of this contour is connected to the first
6776
/// point of this contour or not.
6877
bool is_closed;
@@ -71,6 +80,12 @@ class Path {
7180
Vector2 start_direction;
7281
/// The direction of the contour's end cap.
7382
Vector2 end_direction;
83+
84+
/// Distinct components in this contour.
85+
///
86+
/// If this contour is generated from multiple path components, each
87+
/// path component forms a component in this vector.
88+
std::vector<Component> components;
7489
};
7590

7691
/// One or more contours represented as a series of points and indices in

impeller/geometry/path_component.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,13 @@ struct LinearPathComponent {
4747
std::optional<Vector2> GetEndDirection() const;
4848
};
4949

50+
// A component that represets a Quadratic Bézier curve.
5051
struct QuadraticPathComponent {
52+
// Start point.
5153
Point p1;
54+
// Control point.
5255
Point cp;
56+
// End point.
5357
Point p2;
5458

5559
QuadraticPathComponent() {}
@@ -87,10 +91,15 @@ struct QuadraticPathComponent {
8791
std::optional<Vector2> GetEndDirection() const;
8892
};
8993

94+
// A component that represets a Cubic Bézier curve.
9095
struct CubicPathComponent {
96+
// Start point.
9197
Point p1;
98+
// The first control point.
9299
Point cp1;
100+
// The second control point.
93101
Point cp2;
102+
// End point.
94103
Point p2;
95104

96105
CubicPathComponent() {}

0 commit comments

Comments
 (0)