From 2752bdf86c9b8475b9a5d5966238776e6f29b3c1 Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Thu, 7 Sep 2023 14:11:44 -0700 Subject: [PATCH 1/9] Adds sections to contour to record curve flag --- impeller/aiks/aiks_unittests.cc | 32 +++++++++++++++++++ impeller/core/formats.h | 22 +++++++++++++ .../entity/geometry/stroke_path_geometry.cc | 21 +++++++++--- impeller/geometry/path.cc | 22 +++++++++++-- impeller/geometry/path.h | 16 ++++++++++ 5 files changed, 106 insertions(+), 7 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 3438056a0b6a9..67de952694136 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -1206,6 +1206,22 @@ TEST_P(AiksTest, CanRenderRoundedRectWithNonUniformRadii) { ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } +TEST_P(AiksTest, CanRenderStrokePathThatEndsAtSharpTurn) { + Canvas canvas; + + Paint paint; + paint.color = Color::Red(); + paint.style = Paint::Style::kStroke; + paint.stroke_width = 200; + + Rect rect = {100, 100, 200, 200}; + PathBuilder builder; + builder.AddArc(rect, Degrees(0), Degrees(90), false); + + canvas.DrawPath(builder.TakePath(), paint); + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + TEST_P(AiksTest, CanRenderDifferencePaths) { Canvas canvas; @@ -1952,6 +1968,22 @@ TEST_P(AiksTest, DrawRectStrokesRenderCorrectly) { ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } +TEST_P(AiksTest, DrawRectStrokesWithBevelJoinRenderCorrectly) { + Canvas canvas; + Paint paint; + paint.color = Color::Red(); + paint.style = Paint::Style::kStroke; + paint.stroke_width = 10; + paint.stroke_join = Join::kBevel; + + canvas.Translate({100, 100}); + canvas.DrawPath( + PathBuilder{}.AddRect(Rect::MakeSize(Size{100, 100})).TakePath(), + {paint}); + + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + TEST_P(AiksTest, SaveLayerDrawsBehindSubsequentEntities) { // Compare with https://fiddle.skia.org/c/9e03de8567ffb49e7e83f53b64bcf636 Canvas canvas; diff --git a/impeller/core/formats.h b/impeller/core/formats.h index 6cc151e81939c..e0379528665c2 100644 --- a/impeller/core/formats.h +++ b/impeller/core/formats.h @@ -325,11 +325,33 @@ enum class IndexType { kNone, }; +/// Decides how backend draws pixels based on input vertices. enum class PrimitiveType { + /// Draws a triage for each separate set of three vertices. + /// + /// Vertices [A, B, C, D, E, F] will produce triages + /// [ABC, DEF]. kTriangle, + + /// Draws a triage for every adjacent three vertices. + /// + /// Vertices [A, B, C, D, E, F] will produce triages + /// [ABC, BCD, CDE, DEF]. kTriangleStrip, + + /// Draws a line for each separate set of two vertices. + /// + /// Vertices [A, B, C] will produce discontinued line + /// [AB, BC]. kLine, + + /// Draws a continuous line that connect every input vertices + /// + /// Vertices [A, B, C] will produce one continuous line + /// [ABC]. kLineStrip, + + /// Draws a point at each input vertices. kPoint, // Triangle fans are implementation dependent and need extra extensions // checks. Hence, they are not supported here. diff --git a/impeller/entity/geometry/stroke_path_geometry.cc b/impeller/entity/geometry/stroke_path_geometry.cc index 43648a548e29e..081c21c5ec7e0 100644 --- a/impeller/entity/geometry/stroke_path_geometry.cc +++ b/impeller/entity/geometry/stroke_path_geometry.cc @@ -253,6 +253,7 @@ StrokePathGeometry::CreateSolidStrokeVertices( for (size_t contour_i = 0; contour_i < polyline.contours.size(); contour_i++) { auto contour = polyline.contours[contour_i]; + size_t contour_section = 0; size_t contour_start_point_i, contour_end_point_i; std::tie(contour_start_point_i, contour_end_point_i) = polyline.GetContourPointBounds(contour_i); @@ -308,15 +309,27 @@ StrokePathGeometry::CreateSolidStrokeVertices( // Generate contour geometry. for (size_t point_i = contour_start_point_i + 1; point_i < contour_end_point_i; point_i++) { + if ((contour_section + 1 >= contour.sections.size()) && contour.sections[contour_section + 1].section_start_index <= point_i) { + // The point_i has entered the next section in this contour. + contour_section += 1; + } // Generate line rect. vtx.position = polyline.points[point_i - 1] + offset; vtx_builder.AppendVertex(vtx); vtx.position = polyline.points[point_i - 1] - offset; vtx_builder.AppendVertex(vtx); - vtx.position = polyline.points[point_i] + offset; - vtx_builder.AppendVertex(vtx); - vtx.position = polyline.points[point_i] - offset; - vtx_builder.AppendVertex(vtx); + + // Curve sections don't require the two end points of the rect assuming + // the angles between of a continous two points are close enough. + // + // This also prevents overdraw the line rect if the contour ends at a sharp + // curve with thick stroke width. + if (!contour.sections[contour_section].is_curve) { + vtx.position = polyline.points[point_i] + offset; + vtx_builder.AppendVertex(vtx); + vtx.position = polyline.points[point_i] - offset; + vtx_builder.AppendVertex(vtx); + } if (point_i < contour_end_point_i - 1) { compute_offset(point_i + 1); diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc index ff2c39d6ecaf0..65c33e1cef230 100644 --- a/impeller/geometry/path.cc +++ b/impeller/geometry/path.cc @@ -324,9 +324,10 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const { return Vector2(0, -1); }; + std::vector sections; std::optional previous_path_component_index; auto end_contour = [&polyline, &previous_path_component_index, - &get_path_component]() { + &get_path_component, §ions]() { // Whenever a contour has ended, extract the exact end direction from the // last component. if (polyline.contours.empty()) { @@ -339,6 +340,8 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const { auto& contour = polyline.contours.back(); contour.end_direction = Vector2(0, 1); + contour.sections = sections; + sections.clear(); size_t previous_index = previous_path_component_index.value(); while (!std::holds_alternative( @@ -363,14 +366,26 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const { const auto& component = components_[component_i]; switch (component.type) { case ComponentType::kLinear: + sections.push_back({ + .section_start_index = polyline.points.size(), + .is_curve = false, + }); collect_points(linears_[component.index].CreatePolyline()); previous_path_component_index = component_i; break; case ComponentType::kQuadratic: + sections.push_back({ + .section_start_index = polyline.points.size(), + .is_curve = true, + }); collect_points(quads_[component.index].CreatePolyline(scale)); previous_path_component_index = component_i; break; case ComponentType::kCubic: + sections.push_back({ + .section_start_index = polyline.points.size(), + .is_curve = true, + }); collect_points(cubics_[component.index].CreatePolyline(scale)); previous_path_component_index = component_i; break; @@ -386,13 +401,14 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const { const auto& contour = contours_[component.index]; polyline.contours.push_back({.start_index = polyline.points.size(), .is_closed = contour.is_closed, - .start_direction = start_direction}); + .start_direction = start_direction, + .sections = sections}); previous_contour_point = std::nullopt; collect_points({contour.destination}); break; } - end_contour(); } + end_contour(); return polyline; } diff --git a/impeller/geometry/path.h b/impeller/geometry/path.h index 757c5c03d9548..6cbd2dc539e28 100644 --- a/impeller/geometry/path.h +++ b/impeller/geometry/path.h @@ -61,8 +61,18 @@ class Path { }; struct PolylineContour { + struct Section { + size_t section_start_index; + /// Denotes whether this section is a curve. + /// + /// This is set to true when this section is generated from QuadraticComponent + /// or CubicPathComponent. + bool is_curve; + + }; /// Index that denotes the first point of this contour. size_t start_index; + /// Denotes whether the last point of this contour is connected to the first /// point of this contour or not. bool is_closed; @@ -71,6 +81,12 @@ class Path { Vector2 start_direction; /// The direction of the contour's end cap. Vector2 end_direction; + + /// Distinct sections in thie contour. + /// + /// If this contour is generated from multiple path components, each + /// component forms a section. + std::vector
sections; }; /// One or more contours represented as a series of points and indices in From 311248c5c7ee045e4d47ac54b4861447113b689b Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Thu, 7 Sep 2023 14:12:26 -0700 Subject: [PATCH 2/9] update --- impeller/entity/geometry/stroke_path_geometry.cc | 8 +++++--- impeller/geometry/path.cc | 12 ++++++------ impeller/geometry/path.h | 5 ++--- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/impeller/entity/geometry/stroke_path_geometry.cc b/impeller/entity/geometry/stroke_path_geometry.cc index 081c21c5ec7e0..33bd08ead5593 100644 --- a/impeller/entity/geometry/stroke_path_geometry.cc +++ b/impeller/entity/geometry/stroke_path_geometry.cc @@ -309,7 +309,9 @@ StrokePathGeometry::CreateSolidStrokeVertices( // Generate contour geometry. for (size_t point_i = contour_start_point_i + 1; point_i < contour_end_point_i; point_i++) { - if ((contour_section + 1 >= contour.sections.size()) && contour.sections[contour_section + 1].section_start_index <= point_i) { + if ((contour_section + 1 >= contour.sections.size()) && + contour.sections[contour_section + 1].section_start_index <= + point_i) { // The point_i has entered the next section in this contour. contour_section += 1; } @@ -322,8 +324,8 @@ StrokePathGeometry::CreateSolidStrokeVertices( // Curve sections don't require the two end points of the rect assuming // the angles between of a continous two points are close enough. // - // This also prevents overdraw the line rect if the contour ends at a sharp - // curve with thick stroke width. + // This also prevents overdraw the line rect if the contour ends at a + // sharp curve with thick stroke width. if (!contour.sections[contour_section].is_curve) { vtx.position = polyline.points[point_i] + offset; vtx_builder.AppendVertex(vtx); diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc index 65c33e1cef230..a6bb144fe3cbb 100644 --- a/impeller/geometry/path.cc +++ b/impeller/geometry/path.cc @@ -367,24 +367,24 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const { switch (component.type) { case ComponentType::kLinear: sections.push_back({ - .section_start_index = polyline.points.size(), - .is_curve = false, + .section_start_index = polyline.points.size(), + .is_curve = false, }); collect_points(linears_[component.index].CreatePolyline()); previous_path_component_index = component_i; break; case ComponentType::kQuadratic: sections.push_back({ - .section_start_index = polyline.points.size(), - .is_curve = true, + .section_start_index = polyline.points.size(), + .is_curve = true, }); collect_points(quads_[component.index].CreatePolyline(scale)); previous_path_component_index = component_i; break; case ComponentType::kCubic: sections.push_back({ - .section_start_index = polyline.points.size(), - .is_curve = true, + .section_start_index = polyline.points.size(), + .is_curve = true, }); collect_points(cubics_[component.index].CreatePolyline(scale)); previous_path_component_index = component_i; diff --git a/impeller/geometry/path.h b/impeller/geometry/path.h index 6cbd2dc539e28..4539c29f1c63a 100644 --- a/impeller/geometry/path.h +++ b/impeller/geometry/path.h @@ -65,10 +65,9 @@ class Path { size_t section_start_index; /// Denotes whether this section is a curve. /// - /// This is set to true when this section is generated from QuadraticComponent - /// or CubicPathComponent. + /// This is set to true when this section is generated from + /// QuadraticComponent or CubicPathComponent. bool is_curve; - }; /// Index that denotes the first point of this contour. size_t start_index; From be3e636d589abd9b8243f3e6ea2a98e12c1c6bda Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Thu, 7 Sep 2023 14:20:49 -0700 Subject: [PATCH 3/9] comment --- impeller/entity/geometry/stroke_path_geometry.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/impeller/entity/geometry/stroke_path_geometry.cc b/impeller/entity/geometry/stroke_path_geometry.cc index 33bd08ead5593..6e0047144b8f0 100644 --- a/impeller/entity/geometry/stroke_path_geometry.cc +++ b/impeller/entity/geometry/stroke_path_geometry.cc @@ -321,10 +321,10 @@ StrokePathGeometry::CreateSolidStrokeVertices( vtx.position = polyline.points[point_i - 1] - offset; vtx_builder.AppendVertex(vtx); - // Curve sections don't require the two end points of the rect assuming - // the angles between of a continous two points are close enough. + // Curve sections don't require the two end points of the line rects + // assuming the angles between of a continous two points are close enough. // - // This also prevents overdraw the line rect if the contour ends at a + // This also prevents overdrawing the line rect if the contour ends at a // sharp curve with thick stroke width. if (!contour.sections[contour_section].is_curve) { vtx.position = polyline.points[point_i] + offset; From 201d9de70de6f3395a1ceba0c198dfc6e8a49660 Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Fri, 8 Sep 2023 11:27:52 -0700 Subject: [PATCH 4/9] update --- impeller/entity/geometry/stroke_path_geometry.cc | 16 +++++++++++++--- impeller/geometry/path_component.h | 9 +++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/impeller/entity/geometry/stroke_path_geometry.cc b/impeller/entity/geometry/stroke_path_geometry.cc index 6e0047144b8f0..ca1b0a19a26dc 100644 --- a/impeller/entity/geometry/stroke_path_geometry.cc +++ b/impeller/entity/geometry/stroke_path_geometry.cc @@ -331,6 +331,16 @@ StrokePathGeometry::CreateSolidStrokeVertices( vtx_builder.AppendVertex(vtx); vtx.position = polyline.points[point_i] - offset; vtx_builder.AppendVertex(vtx); + } else if (point_i == contour_end_point_i - 1) { + // If this is a curve and is the end of the contour, two end points + // need to be drawn with the contour end_direction. + auto end_offset = + Vector2(-contour.end_direction.y, contour.end_direction.x) * + stroke_width * 0.5; + vtx.position = polyline.points[contour_end_point_i - 1] + end_offset; + vtx_builder.AppendVertex(vtx); + vtx.position = polyline.points[contour_end_point_i - 1] - end_offset; + vtx_builder.AppendVertex(vtx); } if (point_i < contour_end_point_i - 1) { @@ -343,10 +353,10 @@ StrokePathGeometry::CreateSolidStrokeVertices( } // Generate end cap or join. - if (!polyline.contours[contour_i].is_closed) { + if (!contour.is_closed) { auto cap_offset = - Vector2(-contour.end_direction.y, contour.end_direction.x) * - stroke_width * 0.5; // Clockwise normal + Vector2(-contour.end_direction.y, contour.end_direction.x) * + stroke_width * 0.5; cap_proc(vtx_builder, polyline.points[contour_end_point_i - 1], cap_offset, scale, false); } else { diff --git a/impeller/geometry/path_component.h b/impeller/geometry/path_component.h index c2a808cf18d2a..ef6c4f452e435 100644 --- a/impeller/geometry/path_component.h +++ b/impeller/geometry/path_component.h @@ -47,9 +47,13 @@ struct LinearPathComponent { std::optional GetEndDirection() const; }; +// A component represet a Quadratic Bézier curve. struct QuadraticPathComponent { + // Start point. Point p1; + // Control point. Point cp; + // End point. Point p2; QuadraticPathComponent() {} @@ -87,10 +91,15 @@ struct QuadraticPathComponent { std::optional GetEndDirection() const; }; +// A component represet a Cubic Bézier curve. struct CubicPathComponent { + // Start point. Point p1; + // The first control point. Point cp1; + // The second control point. Point cp2; + // End point. Point p2; CubicPathComponent() {} From 1e388530a614909b99d766d46a698a9ce8e7b967 Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Fri, 8 Sep 2023 11:28:26 -0700 Subject: [PATCH 5/9] format --- impeller/entity/geometry/stroke_path_geometry.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/impeller/entity/geometry/stroke_path_geometry.cc b/impeller/entity/geometry/stroke_path_geometry.cc index ca1b0a19a26dc..c683fc8fbeae4 100644 --- a/impeller/entity/geometry/stroke_path_geometry.cc +++ b/impeller/entity/geometry/stroke_path_geometry.cc @@ -335,8 +335,8 @@ StrokePathGeometry::CreateSolidStrokeVertices( // If this is a curve and is the end of the contour, two end points // need to be drawn with the contour end_direction. auto end_offset = - Vector2(-contour.end_direction.y, contour.end_direction.x) * - stroke_width * 0.5; + Vector2(-contour.end_direction.y, contour.end_direction.x) * + stroke_width * 0.5; vtx.position = polyline.points[contour_end_point_i - 1] + end_offset; vtx_builder.AppendVertex(vtx); vtx.position = polyline.points[contour_end_point_i - 1] - end_offset; @@ -355,8 +355,8 @@ StrokePathGeometry::CreateSolidStrokeVertices( // Generate end cap or join. if (!contour.is_closed) { auto cap_offset = - Vector2(-contour.end_direction.y, contour.end_direction.x) * - stroke_width * 0.5; + Vector2(-contour.end_direction.y, contour.end_direction.x) * + stroke_width * 0.5; cap_proc(vtx_builder, polyline.points[contour_end_point_i - 1], cap_offset, scale, false); } else { From 0ed9510ae0f8db01398493d0a4095206bcfe34b8 Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Fri, 8 Sep 2023 12:09:09 -0700 Subject: [PATCH 6/9] update --- impeller/entity/geometry/stroke_path_geometry.cc | 2 +- impeller/geometry/path.h | 2 +- impeller/geometry/path_component.h | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/impeller/entity/geometry/stroke_path_geometry.cc b/impeller/entity/geometry/stroke_path_geometry.cc index c683fc8fbeae4..b9a95140f8a95 100644 --- a/impeller/entity/geometry/stroke_path_geometry.cc +++ b/impeller/entity/geometry/stroke_path_geometry.cc @@ -356,7 +356,7 @@ StrokePathGeometry::CreateSolidStrokeVertices( if (!contour.is_closed) { auto cap_offset = Vector2(-contour.end_direction.y, contour.end_direction.x) * - stroke_width * 0.5; + stroke_width * 0.5; // Clockwise normal cap_proc(vtx_builder, polyline.points[contour_end_point_i - 1], cap_offset, scale, false); } else { diff --git a/impeller/geometry/path.h b/impeller/geometry/path.h index 4539c29f1c63a..d52458a4118d6 100644 --- a/impeller/geometry/path.h +++ b/impeller/geometry/path.h @@ -81,7 +81,7 @@ class Path { /// The direction of the contour's end cap. Vector2 end_direction; - /// Distinct sections in thie contour. + /// Distinct sections in this contour. /// /// If this contour is generated from multiple path components, each /// component forms a section. diff --git a/impeller/geometry/path_component.h b/impeller/geometry/path_component.h index ef6c4f452e435..da61975d42ed5 100644 --- a/impeller/geometry/path_component.h +++ b/impeller/geometry/path_component.h @@ -47,7 +47,7 @@ struct LinearPathComponent { std::optional GetEndDirection() const; }; -// A component represet a Quadratic Bézier curve. +// A component represets a Quadratic Bézier curve. struct QuadraticPathComponent { // Start point. Point p1; @@ -91,7 +91,7 @@ struct QuadraticPathComponent { std::optional GetEndDirection() const; }; -// A component represet a Cubic Bézier curve. +// A component represets a Cubic Bézier curve. struct CubicPathComponent { // Start point. Point p1; From ca72e9b6607ec465684ccd2af934d1a93eb28b5c Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Fri, 8 Sep 2023 12:09:50 -0700 Subject: [PATCH 7/9] format --- impeller/entity/geometry/stroke_path_geometry.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/entity/geometry/stroke_path_geometry.cc b/impeller/entity/geometry/stroke_path_geometry.cc index b9a95140f8a95..4d6e1704601fe 100644 --- a/impeller/entity/geometry/stroke_path_geometry.cc +++ b/impeller/entity/geometry/stroke_path_geometry.cc @@ -356,7 +356,7 @@ StrokePathGeometry::CreateSolidStrokeVertices( if (!contour.is_closed) { auto cap_offset = Vector2(-contour.end_direction.y, contour.end_direction.x) * - stroke_width * 0.5; // Clockwise normal + stroke_width * 0.5; // Clockwise normal cap_proc(vtx_builder, polyline.points[contour_end_point_i - 1], cap_offset, scale, false); } else { From 87a0e2d1b55897c5674570a3225cd953ded97d94 Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Tue, 12 Sep 2023 14:05:00 -0700 Subject: [PATCH 8/9] Fix more curve --- impeller/aiks/aiks_unittests.cc | 15 ++++++ .../entity/geometry/stroke_path_geometry.cc | 49 ++++++++++--------- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 67de952694136..6af0cbc9283a9 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -1222,6 +1222,21 @@ TEST_P(AiksTest, CanRenderStrokePathThatEndsAtSharpTurn) { ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } +TEST_P(AiksTest, CanRenderStrokePathWithCubicLine) { + Canvas canvas; + + Paint paint; + paint.color = Color::Red(); + paint.style = Paint::Style::kStroke; + paint.stroke_width = 20; + + PathBuilder builder; + builder.AddCubicCurve({0, 200}, {50, 400}, {350, 0}, {400, 200}); + + canvas.DrawPath(builder.TakePath(), paint); + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + TEST_P(AiksTest, CanRenderDifferencePaths) { Canvas canvas; diff --git a/impeller/entity/geometry/stroke_path_geometry.cc b/impeller/entity/geometry/stroke_path_geometry.cc index 4d6e1704601fe..60647cbad16d6 100644 --- a/impeller/entity/geometry/stroke_path_geometry.cc +++ b/impeller/entity/geometry/stroke_path_geometry.cc @@ -321,34 +321,39 @@ StrokePathGeometry::CreateSolidStrokeVertices( vtx.position = polyline.points[point_i - 1] - offset; vtx_builder.AppendVertex(vtx); - // Curve sections don't require the two end points of the line rects - // assuming the angles between of a continous two points are close enough. - // - // This also prevents overdrawing the line rect if the contour ends at a - // sharp curve with thick stroke width. + auto is_end_of_contour = point_i == contour_end_point_i - 1; + if (!contour.sections[contour_section].is_curve) { + /// Add two end points of the line rect to draw a solid + /// straight line. vtx.position = polyline.points[point_i] + offset; vtx_builder.AppendVertex(vtx); vtx.position = polyline.points[point_i] - offset; vtx_builder.AppendVertex(vtx); - } else if (point_i == contour_end_point_i - 1) { - // If this is a curve and is the end of the contour, two end points - // need to be drawn with the contour end_direction. - auto end_offset = - Vector2(-contour.end_direction.y, contour.end_direction.x) * - stroke_width * 0.5; - vtx.position = polyline.points[contour_end_point_i - 1] + end_offset; - vtx_builder.AppendVertex(vtx); - vtx.position = polyline.points[contour_end_point_i - 1] - end_offset; - vtx_builder.AppendVertex(vtx); - } - - if (point_i < contour_end_point_i - 1) { - compute_offset(point_i + 1); - // Generate join from the current line to the next line. - join_proc(vtx_builder, polyline.points[point_i], previous_offset, - offset, scaled_miter_limit, scale); + if (!is_end_of_contour) { + compute_offset(point_i + 1); + // Generate join from the current line to the next line. + join_proc(vtx_builder, polyline.points[point_i], previous_offset, + offset, scaled_miter_limit, scale); + } + } else { + // Curve sections don't require the two end points of the line rects + // assuming the angles between of a continous two points are close + // enough. + if (!is_end_of_contour) { + compute_offset(point_i + 1); + } else { + // If this is a curve and is the end of the contour, two end points + // need to be drawn with the contour end_direction. + auto end_offset = + Vector2(-contour.end_direction.y, contour.end_direction.x) * + stroke_width * 0.5; + vtx.position = polyline.points[contour_end_point_i - 1] + end_offset; + vtx_builder.AppendVertex(vtx); + vtx.position = polyline.points[contour_end_point_i - 1] - end_offset; + vtx_builder.AppendVertex(vtx); + } } } From ef08968873d744cd20fe0691fbd8d96632a01390 Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Tue, 12 Sep 2023 14:27:59 -0700 Subject: [PATCH 9/9] addressing comments --- impeller/core/formats.h | 2 +- .../entity/geometry/stroke_path_geometry.cc | 21 +++++++++--------- impeller/geometry/path.cc | 22 +++++++++---------- impeller/geometry/path.h | 14 ++++++------ impeller/geometry/path_component.h | 4 ++-- 5 files changed, 31 insertions(+), 32 deletions(-) diff --git a/impeller/core/formats.h b/impeller/core/formats.h index e0379528665c2..0e8d0d7d86a43 100644 --- a/impeller/core/formats.h +++ b/impeller/core/formats.h @@ -351,7 +351,7 @@ enum class PrimitiveType { /// [ABC]. kLineStrip, - /// Draws a point at each input vertices. + /// Draws a point at each input vertex. kPoint, // Triangle fans are implementation dependent and need extra extensions // checks. Hence, they are not supported here. diff --git a/impeller/entity/geometry/stroke_path_geometry.cc b/impeller/entity/geometry/stroke_path_geometry.cc index 60647cbad16d6..9b5ab6877cd99 100644 --- a/impeller/entity/geometry/stroke_path_geometry.cc +++ b/impeller/entity/geometry/stroke_path_geometry.cc @@ -253,7 +253,7 @@ StrokePathGeometry::CreateSolidStrokeVertices( for (size_t contour_i = 0; contour_i < polyline.contours.size(); contour_i++) { auto contour = polyline.contours[contour_i]; - size_t contour_section = 0; + size_t contour_component_i = 0; size_t contour_start_point_i, contour_end_point_i; std::tie(contour_start_point_i, contour_end_point_i) = polyline.GetContourPointBounds(contour_i); @@ -309,11 +309,11 @@ StrokePathGeometry::CreateSolidStrokeVertices( // Generate contour geometry. for (size_t point_i = contour_start_point_i + 1; point_i < contour_end_point_i; point_i++) { - if ((contour_section + 1 >= contour.sections.size()) && - contour.sections[contour_section + 1].section_start_index <= + if ((contour_component_i + 1 >= contour.components.size()) && + contour.components[contour_component_i + 1].component_start_index <= point_i) { - // The point_i has entered the next section in this contour. - contour_section += 1; + // The point_i has entered the next component in this contour. + contour_component_i += 1; } // Generate line rect. vtx.position = polyline.points[point_i - 1] + offset; @@ -323,9 +323,9 @@ StrokePathGeometry::CreateSolidStrokeVertices( auto is_end_of_contour = point_i == contour_end_point_i - 1; - if (!contour.sections[contour_section].is_curve) { - /// Add two end points of the line rect to draw a solid - /// straight line. + if (!contour.components[contour_component_i].is_curve) { + // For line components, two additional points need to be appended prior + // to appending a join connecting the next component. vtx.position = polyline.points[point_i] + offset; vtx_builder.AppendVertex(vtx); vtx.position = polyline.points[point_i] - offset; @@ -338,9 +338,8 @@ StrokePathGeometry::CreateSolidStrokeVertices( offset, scaled_miter_limit, scale); } } else { - // Curve sections don't require the two end points of the line rects - // assuming the angles between of a continous two points are close - // enough. + // For curve components, the polyline is detailed enough such that + // it can avoid worrying about joins altogether. if (!is_end_of_contour) { compute_offset(point_i + 1); } else { diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc index a6bb144fe3cbb..863b220455382 100644 --- a/impeller/geometry/path.cc +++ b/impeller/geometry/path.cc @@ -324,10 +324,10 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const { return Vector2(0, -1); }; - std::vector sections; + std::vector components; std::optional previous_path_component_index; auto end_contour = [&polyline, &previous_path_component_index, - &get_path_component, §ions]() { + &get_path_component, &components]() { // Whenever a contour has ended, extract the exact end direction from the // last component. if (polyline.contours.empty()) { @@ -340,8 +340,8 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const { auto& contour = polyline.contours.back(); contour.end_direction = Vector2(0, 1); - contour.sections = sections; - sections.clear(); + contour.components = components; + components.clear(); size_t previous_index = previous_path_component_index.value(); while (!std::holds_alternative( @@ -366,24 +366,24 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const { const auto& component = components_[component_i]; switch (component.type) { case ComponentType::kLinear: - sections.push_back({ - .section_start_index = polyline.points.size(), + components.push_back({ + .component_start_index = polyline.points.size(), .is_curve = false, }); collect_points(linears_[component.index].CreatePolyline()); previous_path_component_index = component_i; break; case ComponentType::kQuadratic: - sections.push_back({ - .section_start_index = polyline.points.size(), + components.push_back({ + .component_start_index = polyline.points.size(), .is_curve = true, }); collect_points(quads_[component.index].CreatePolyline(scale)); previous_path_component_index = component_i; break; case ComponentType::kCubic: - sections.push_back({ - .section_start_index = polyline.points.size(), + components.push_back({ + .component_start_index = polyline.points.size(), .is_curve = true, }); collect_points(cubics_[component.index].CreatePolyline(scale)); @@ -402,7 +402,7 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const { polyline.contours.push_back({.start_index = polyline.points.size(), .is_closed = contour.is_closed, .start_direction = start_direction, - .sections = sections}); + .components = components}); previous_contour_point = std::nullopt; collect_points({contour.destination}); break; diff --git a/impeller/geometry/path.h b/impeller/geometry/path.h index d52458a4118d6..bbbb207793d5f 100644 --- a/impeller/geometry/path.h +++ b/impeller/geometry/path.h @@ -61,11 +61,11 @@ class Path { }; struct PolylineContour { - struct Section { - size_t section_start_index; - /// Denotes whether this section is a curve. + struct Component { + size_t component_start_index; + /// Denotes whether this component is a curve. /// - /// This is set to true when this section is generated from + /// This is set to true when this component is generated from /// QuadraticComponent or CubicPathComponent. bool is_curve; }; @@ -81,11 +81,11 @@ class Path { /// The direction of the contour's end cap. Vector2 end_direction; - /// Distinct sections in this contour. + /// Distinct components in this contour. /// /// If this contour is generated from multiple path components, each - /// component forms a section. - std::vector
sections; + /// path component forms a component in this vector. + std::vector components; }; /// One or more contours represented as a series of points and indices in diff --git a/impeller/geometry/path_component.h b/impeller/geometry/path_component.h index da61975d42ed5..a8b0d4fa9bd68 100644 --- a/impeller/geometry/path_component.h +++ b/impeller/geometry/path_component.h @@ -47,7 +47,7 @@ struct LinearPathComponent { std::optional GetEndDirection() const; }; -// A component represets a Quadratic Bézier curve. +// A component that represets a Quadratic Bézier curve. struct QuadraticPathComponent { // Start point. Point p1; @@ -91,7 +91,7 @@ struct QuadraticPathComponent { std::optional GetEndDirection() const; }; -// A component represets a Cubic Bézier curve. +// A component that represets a Cubic Bézier curve. struct CubicPathComponent { // Start point. Point p1;