Skip to content

Commit 2584e1b

Browse files
committed
Reapply "[Impeller] Fixes stroke path geometry that can draw outside o… (flutter#46334)
This reverts commit 1d15b35.
1 parent 0161916 commit 2584e1b

6 files changed

Lines changed: 151 additions & 13 deletions

File tree

impeller/aiks/aiks_unittests.cc

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

1236+
TEST_P(AiksTest, CanRenderStrokePathThatEndsAtSharpTurn) {
1237+
Canvas canvas;
1238+
1239+
Paint paint;
1240+
paint.color = Color::Red();
1241+
paint.style = Paint::Style::kStroke;
1242+
paint.stroke_width = 200;
1243+
1244+
Rect rect = {100, 100, 200, 200};
1245+
PathBuilder builder;
1246+
builder.AddArc(rect, Degrees(0), Degrees(90), false);
1247+
1248+
canvas.DrawPath(builder.TakePath(), paint);
1249+
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
1250+
}
1251+
1252+
TEST_P(AiksTest, CanRenderStrokePathWithCubicLine) {
1253+
Canvas canvas;
1254+
1255+
Paint paint;
1256+
paint.color = Color::Red();
1257+
paint.style = Paint::Style::kStroke;
1258+
paint.stroke_width = 20;
1259+
1260+
PathBuilder builder;
1261+
builder.AddCubicCurve({0, 200}, {50, 400}, {350, 0}, {400, 200});
1262+
1263+
canvas.DrawPath(builder.TakePath(), paint);
1264+
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
1265+
}
1266+
12361267
TEST_P(AiksTest, CanRenderDifferencePaths) {
12371268
Canvas canvas;
12381269

@@ -2064,6 +2095,22 @@ TEST_P(AiksTest, DrawRectStrokesRenderCorrectly) {
20642095
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
20652096
}
20662097

2098+
TEST_P(AiksTest, DrawRectStrokesWithBevelJoinRenderCorrectly) {
2099+
Canvas canvas;
2100+
Paint paint;
2101+
paint.color = Color::Red();
2102+
paint.style = Paint::Style::kStroke;
2103+
paint.stroke_width = 10;
2104+
paint.stroke_join = Join::kBevel;
2105+
2106+
canvas.Translate({100, 100});
2107+
canvas.DrawPath(
2108+
PathBuilder{}.AddRect(Rect::MakeSize(Size{100, 100})).TakePath(),
2109+
{paint});
2110+
2111+
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
2112+
}
2113+
20672114
TEST_P(AiksTest, SaveLayerDrawsBehindSubsequentEntities) {
20682115
// Compare with https://fiddle.skia.org/c/9e03de8567ffb49e7e83f53b64bcf636
20692116
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)