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

Commit 0fca429

Browse files
luckysmgluckysmg
authored
[Impeller] Fix incorrect rendering path when duplicated point exists. (#40115)
[Impeller] Fix incorrect rendering path when duplicated point exists.
1 parent cbc8ae8 commit 0fca429

File tree

3 files changed

+240
-56
lines changed

3 files changed

+240
-56
lines changed

‎impeller/display_list/display_list_unittests.cc‎

Lines changed: 159 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -207,49 +207,173 @@ TEST_P(DisplayListTest, CanDrawArc) {
207207
}
208208

209209
TEST_P(DisplayListTest, StrokedPathsDrawCorrectly) {
210-
flutter::DisplayListBuilder builder;
211-
builder.setColor(SK_ColorRED);
212-
builder.setStyle(flutter::DlDrawStyle::kStroke);
213-
builder.setStrokeWidth(10);
210+
auto callback = [&]() {
211+
flutter::DisplayListBuilder builder;
212+
builder.setColor(SK_ColorRED);
213+
builder.setStyle(flutter::DlDrawStyle::kStroke);
214214

215-
// Rectangle
216-
builder.translate(100, 100);
217-
builder.drawRect(SkRect::MakeSize({100, 100}));
215+
static float stroke_width = 10.0f;
216+
static int selected_stroke_type = 0;
217+
static int selected_join_type = 0;
218+
const char* stroke_types[] = {"Butte", "Round", "Square"};
219+
const char* join_type[] = {"kMiter", "Round", "kBevel"};
218220

219-
// Rounded rectangle
220-
builder.translate(150, 0);
221-
builder.drawRRect(SkRRect::MakeRectXY(SkRect::MakeSize({100, 50}), 10, 10));
221+
ImGui::Begin("Controls", nullptr, ImGuiWindowFlags_AlwaysAutoResize);
222+
ImGui::Combo("Cap", &selected_stroke_type, stroke_types,
223+
sizeof(stroke_types) / sizeof(char*));
224+
ImGui::Combo("Join", &selected_join_type, join_type,
225+
sizeof(join_type) / sizeof(char*));
226+
ImGui::SliderFloat("Stroke Width", &stroke_width, 10.0f, 50.0f);
227+
ImGui::End();
222228

223-
// Double rounded rectangle
224-
builder.translate(150, 0);
225-
builder.drawDRRect(
226-
SkRRect::MakeRectXY(SkRect::MakeSize({100, 50}), 10, 10),
227-
SkRRect::MakeRectXY(SkRect::MakeXYWH(10, 10, 80, 30), 10, 10));
229+
flutter::DlStrokeCap cap;
230+
flutter::DlStrokeJoin join;
231+
switch (selected_stroke_type) {
232+
case 0:
233+
cap = flutter::DlStrokeCap::kButt;
234+
break;
235+
case 1:
236+
cap = flutter::DlStrokeCap::kRound;
237+
break;
238+
case 2:
239+
cap = flutter::DlStrokeCap::kSquare;
240+
break;
241+
default:
242+
cap = flutter::DlStrokeCap::kButt;
243+
break;
244+
}
245+
switch (selected_join_type) {
246+
case 0:
247+
join = flutter::DlStrokeJoin::kMiter;
248+
break;
249+
case 1:
250+
join = flutter::DlStrokeJoin::kRound;
251+
break;
252+
case 2:
253+
join = flutter::DlStrokeJoin::kBevel;
254+
break;
255+
default:
256+
join = flutter::DlStrokeJoin::kMiter;
257+
break;
258+
}
259+
builder.setStrokeCap(cap);
260+
builder.setStrokeJoin(join);
261+
builder.setStrokeWidth(stroke_width);
228262

229-
// Contour with duplicate join points
230-
{
263+
// Make rendering better to watch.
264+
builder.scale(1.5f, 1.5f);
265+
266+
// Rectangle
267+
builder.translate(100, 100);
268+
builder.drawRect(SkRect::MakeSize({100, 100}));
269+
270+
// Rounded rectangle
231271
builder.translate(150, 0);
232-
SkPath path;
233-
path.lineTo({100, 0});
234-
path.lineTo({100, 0});
235-
path.lineTo({100, 100});
236-
builder.drawPath(path);
237-
}
272+
builder.drawRRect(SkRRect::MakeRectXY(SkRect::MakeSize({100, 50}), 10, 10));
238273

239-
// Contour with duplicate end points
240-
{
241-
builder.setStrokeCap(flutter::DlStrokeCap::kRound);
274+
// Double rounded rectangle
242275
builder.translate(150, 0);
243-
SkPath path;
244-
path.moveTo(0, 0);
245-
path.lineTo({0, 0});
246-
path.lineTo({50, 50});
247-
path.lineTo({100, 0});
248-
path.lineTo({100, 0});
249-
builder.drawPath(path);
250-
}
276+
builder.drawDRRect(
277+
SkRRect::MakeRectXY(SkRect::MakeSize({100, 50}), 10, 10),
278+
SkRRect::MakeRectXY(SkRect::MakeXYWH(10, 10, 80, 30), 10, 10));
251279

252-
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
280+
// Contour with duplicate join points
281+
{
282+
builder.translate(150, 0);
283+
SkPath path;
284+
path.moveTo(0, 0);
285+
path.lineTo(0, 0);
286+
path.lineTo({100, 0});
287+
path.lineTo({100, 0});
288+
path.lineTo({100, 100});
289+
builder.drawPath(path);
290+
}
291+
292+
// Contour with duplicate start and end points
293+
294+
// Line.
295+
builder.translate(200, 0);
296+
{
297+
builder.save();
298+
299+
SkPath line_path;
300+
line_path.moveTo(0, 0);
301+
line_path.moveTo(0, 0);
302+
line_path.lineTo({0, 0});
303+
line_path.lineTo({0, 0});
304+
line_path.lineTo({50, 50});
305+
line_path.lineTo({50, 50});
306+
line_path.lineTo({100, 0});
307+
line_path.lineTo({100, 0});
308+
builder.drawPath(line_path);
309+
310+
builder.translate(0, 100);
311+
builder.drawPath(line_path);
312+
313+
builder.translate(0, 100);
314+
SkPath line_path2;
315+
line_path2.moveTo(0, 0);
316+
line_path2.lineTo(0, 0);
317+
line_path2.lineTo(0, 0);
318+
builder.drawPath(line_path2);
319+
320+
builder.restore();
321+
}
322+
323+
// Cubic.
324+
builder.translate(150, 0);
325+
{
326+
builder.save();
327+
328+
SkPath cubic_path;
329+
cubic_path.moveTo({0, 0});
330+
cubic_path.cubicTo(0, 0, 140.0, 100.0, 140, 20);
331+
builder.drawPath(cubic_path);
332+
333+
builder.translate(0, 100);
334+
SkPath cubic_path2;
335+
cubic_path2.moveTo({0, 0});
336+
cubic_path2.cubicTo(0, 0, 0, 0, 150, 150);
337+
builder.drawPath(cubic_path2);
338+
339+
builder.translate(0, 100);
340+
SkPath cubic_path3;
341+
cubic_path3.moveTo({0, 0});
342+
cubic_path3.cubicTo(0, 0, 0, 0, 0, 0);
343+
builder.drawPath(cubic_path3);
344+
345+
builder.restore();
346+
}
347+
348+
// Quad.
349+
builder.translate(200, 0);
350+
{
351+
builder.save();
352+
353+
SkPath quad_path;
354+
quad_path.moveTo(0, 0);
355+
quad_path.moveTo(0, 0);
356+
quad_path.quadTo({100, 40}, {50, 80});
357+
builder.drawPath(quad_path);
358+
359+
builder.translate(0, 150);
360+
SkPath quad_path2;
361+
quad_path2.moveTo(0, 0);
362+
quad_path2.moveTo(0, 0);
363+
quad_path2.quadTo({0, 0}, {100, 100});
364+
builder.drawPath(quad_path2);
365+
366+
builder.translate(0, 100);
367+
SkPath quad_path3;
368+
quad_path3.moveTo(0, 0);
369+
quad_path3.quadTo({0, 0}, {0, 0});
370+
builder.drawPath(quad_path3);
371+
372+
builder.restore();
373+
}
374+
return builder.Build();
375+
};
376+
ASSERT_TRUE(OpenPlaygroundHere(callback));
253377
}
254378

255379
TEST_P(DisplayListTest, CanDrawWithOddPathWinding) {

‎impeller/geometry/path.cc‎

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -261,20 +261,51 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const {
261261
}
262262
};
263263

264-
std::optional<const PathComponent*> previous_path_component;
265-
auto end_contour = [&polyline, &previous_path_component]() {
264+
auto compute_contour_start_direction =
265+
[&get_path_component](size_t current_path_component_index) {
266+
size_t next_component_index = current_path_component_index + 1;
267+
while (get_path_component(next_component_index).has_value()) {
268+
auto next_component =
269+
get_path_component(next_component_index).value();
270+
if (next_component->GetStartDirection().has_value()) {
271+
return next_component->GetStartDirection().value();
272+
} else {
273+
next_component_index++;
274+
}
275+
}
276+
return Vector2(0, -1);
277+
};
278+
279+
std::optional<size_t> previous_path_component_index;
280+
auto end_contour = [&polyline, &previous_path_component_index,
281+
&get_path_component]() {
266282
// Whenever a contour has ended, extract the exact end direction from the
267283
// last component.
268284
if (polyline.contours.empty()) {
269285
return;
270286
}
271-
if (!previous_path_component.has_value()) {
287+
288+
if (!previous_path_component_index.has_value()) {
272289
return;
273290
}
291+
274292
auto& contour = polyline.contours.back();
275-
contour.end_direction =
276-
previous_path_component.value()->GetEndDirection().value_or(
277-
Vector2(0, 1));
293+
contour.end_direction = Vector2(0, 1);
294+
295+
size_t previous_index = previous_path_component_index.value();
296+
while (get_path_component(previous_index).has_value()) {
297+
auto previous_path_component = get_path_component(previous_index).value();
298+
if (previous_path_component->GetEndDirection().has_value()) {
299+
contour.end_direction =
300+
previous_path_component->GetEndDirection().value();
301+
break;
302+
} else {
303+
if (previous_index == 0) {
304+
break;
305+
}
306+
previous_index--;
307+
}
308+
}
278309
};
279310

280311
for (size_t component_i = 0; component_i < components_.size();
@@ -283,15 +314,15 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const {
283314
switch (component.type) {
284315
case ComponentType::kLinear:
285316
collect_points(linears_[component.index].CreatePolyline());
286-
previous_path_component = &linears_[component.index];
317+
previous_path_component_index = component_i;
287318
break;
288319
case ComponentType::kQuadratic:
289320
collect_points(quads_[component.index].CreatePolyline(scale));
290-
previous_path_component = &quads_[component.index];
321+
previous_path_component_index = component_i;
291322
break;
292323
case ComponentType::kCubic:
293324
collect_points(cubics_[component.index].CreatePolyline(scale));
294-
previous_path_component = &cubics_[component.index];
325+
previous_path_component_index = component_i;
295326
break;
296327
case ComponentType::kContour:
297328
if (component_i == components_.size() - 1) {
@@ -301,14 +332,7 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const {
301332
}
302333
end_contour();
303334

304-
Vector2 start_direction(0, -1);
305-
auto first_component = get_path_component(component_i + 1);
306-
if (first_component.has_value()) {
307-
start_direction =
308-
first_component.value()->GetStartDirection().value_or(
309-
Vector2(0, -1));
310-
}
311-
335+
Vector2 start_direction = compute_contour_start_direction(component_i);
312336
const auto& contour = contours_[component.index];
313337
polyline.contours.push_back({.start_index = polyline.points.size(),
314338
.is_closed = contour.is_closed,

‎impeller/geometry/path_component.cc‎

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,16 @@ std::vector<Point> LinearPathComponent::Extrema() const {
7070
}
7171

7272
std::optional<Vector2> LinearPathComponent::GetStartDirection() const {
73+
if (p1 == p2) {
74+
return std::nullopt;
75+
}
7376
return (p1 - p2).Normalize();
7477
}
7578

7679
std::optional<Vector2> LinearPathComponent::GetEndDirection() const {
80+
if (p1 == p2) {
81+
return std::nullopt;
82+
}
7783
return (p2 - p1).Normalize();
7884
}
7985

@@ -150,11 +156,23 @@ std::vector<Point> QuadraticPathComponent::Extrema() const {
150156
}
151157

152158
std::optional<Vector2> QuadraticPathComponent::GetStartDirection() const {
153-
return (p1 - cp).Normalize();
159+
if (p1 != cp) {
160+
return (p1 - cp).Normalize();
161+
}
162+
if (p1 != p2) {
163+
return (p1 - p2).Normalize();
164+
}
165+
return std::nullopt;
154166
}
155167

156168
std::optional<Vector2> QuadraticPathComponent::GetEndDirection() const {
157-
return (p2 - cp).Normalize();
169+
if (p2 != cp) {
170+
return (p2 - cp).Normalize();
171+
}
172+
if (p2 != p1) {
173+
return (p2 - p1).Normalize();
174+
}
175+
return std::nullopt;
158176
}
159177

160178
Point CubicPathComponent::Solve(Scalar time) const {
@@ -302,11 +320,29 @@ std::vector<Point> CubicPathComponent::Extrema() const {
302320
}
303321

304322
std::optional<Vector2> CubicPathComponent::GetStartDirection() const {
305-
return (p1 - cp1).Normalize();
323+
if (p1 != cp1) {
324+
return (p1 - cp1).Normalize();
325+
}
326+
if (p1 != cp2) {
327+
return (p1 - cp2).Normalize();
328+
}
329+
if (p1 != p2) {
330+
return (p1 - p2).Normalize();
331+
}
332+
return std::nullopt;
306333
}
307334

308335
std::optional<Vector2> CubicPathComponent::GetEndDirection() const {
309-
return (p2 - cp2).Normalize();
336+
if (p2 != cp2) {
337+
return (p2 - cp2).Normalize();
338+
}
339+
if (p2 != cp1) {
340+
return (p2 - cp1).Normalize();
341+
}
342+
if (p2 != p1) {
343+
return (p2 - p1).Normalize();
344+
}
345+
return std::nullopt;
310346
}
311347

312348
} // namespace impeller

0 commit comments

Comments
 (0)