Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Impeller] Round out extreme angles between curve polyline segments. #53210

Merged
merged 5 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 19 additions & 0 deletions impeller/aiks/aiks_path_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,25 @@ TEST_P(AiksTest, CanRenderStrokePathWithCubicLine) {
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

TEST_P(AiksTest, CanRenderQuadraticStrokeWithInstantTurn) {
Canvas canvas;

Paint paint;
paint.color = Color::Red();
paint.style = Paint::Style::kStroke;
paint.stroke_cap = Cap::kRound;
paint.stroke_width = 50;

// Should draw a diagonal pill shape. If flat on either end, the stroke is
// rendering wrong.
PathBuilder builder;
builder.MoveTo({250, 250});
builder.QuadraticCurveTo({100, 100}, {250, 250});

canvas.DrawPath(builder.TakePath(), paint);
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

TEST_P(AiksTest, CanRenderDifferencePaths) {
Canvas canvas;

Expand Down
113 changes: 92 additions & 21 deletions impeller/entity/geometry/stroke_path_geometry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "impeller/core/buffer_view.h"
#include "impeller/core/formats.h"
#include "impeller/entity/geometry/geometry.h"
#include "impeller/geometry/constants.h"
#include "impeller/geometry/path_builder.h"
#include "impeller/geometry/path_component.h"

Expand Down Expand Up @@ -90,7 +91,7 @@ class StrokeGenerator {
previous_offset = offset;
offset = ComputeOffset(contour_start_point_i, contour_start_point_i,
contour_end_point_i, contour);
const Point contour_first_offset = offset;
const Point contour_first_offset = offset.GetVector();

if (contour_i > 0) {
// This branch only executes when we've just finished drawing a contour
Expand Down Expand Up @@ -155,19 +156,52 @@ class StrokeGenerator {
cap_proc(vtx_builder, polyline.GetPoint(contour_end_point_i - 1),
cap_offset, scale, /*reverse=*/false);
} else {
join_proc(vtx_builder, polyline.GetPoint(contour_start_point_i), offset,
contour_first_offset, scaled_miter_limit, scale);
join_proc(vtx_builder, polyline.GetPoint(contour_start_point_i),
offset.GetVector(), contour_first_offset, scaled_miter_limit,
scale);
}
}
}

/// @brief Represents a ray in 2D space.
///
/// This is a simple convenience struct for handling polyline offset
/// values when generating stroke geometry. For performance reasons,
/// it's sometimes adventageous to track the direction and magnitude
/// for offsets separately.
struct Ray {
Copy link
Member

Choose a reason for hiding this comment

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

This could perhaps go in //impeller/geometry.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not opposed to throwing this in geometry. I'll do so in a follow-up. Also occurs to me that Ray is an inaccurate name for this... Should be something like SeparatedVector2.

/// The normalized direction of the ray.
Vector2 direction;

/// The magnitude of the ray.
Scalar magnitude = 0.0;

/// Returns the vector representation of the ray.
Vector2 GetVector() const { return direction * magnitude; }

/// Returns the scalar alignment of the two rays.
///
/// Domain: [-1, 1]
/// A value of 1 indicates the rays are parallel and pointing in the same
/// direction. A value of -1 indicates the rays are parallel and pointing in
/// opposite directions. A value of 0 indicates the rays are perpendicular.
Scalar GetAlignment(const Ray& other) const {
return direction.Dot(other.direction);
}

/// Returns the scalar angle between the two rays.
Radians AngleTo(const Ray& other) const {
return direction.AngleTo(other.direction);
}
};

/// Computes offset by calculating the direction from point_i - 1 to point_i
/// if point_i is within `contour_start_point_i` and `contour_end_point_i`;
/// Otherwise, it uses direction from contour.
Point ComputeOffset(const size_t point_i,
const size_t contour_start_point_i,
const size_t contour_end_point_i,
const Path::PolylineContour& contour) const {
Ray ComputeOffset(const size_t point_i,
const size_t contour_start_point_i,
const size_t contour_end_point_i,
const Path::PolylineContour& contour) const {
Point direction;
if (point_i >= contour_end_point_i) {
direction = contour.end_direction;
Expand All @@ -177,7 +211,8 @@ class StrokeGenerator {
direction = (polyline.GetPoint(point_i) - polyline.GetPoint(point_i - 1))
.Normalize();
}
return Vector2{-direction.y, direction.x} * stroke_width * 0.5f;
return {.direction = Vector2{-direction.y, direction.x},
.magnitude = stroke_width * 0.5f};
}

void AddVerticesForLinearComponent(VertexWriter& vtx_builder,
Expand All @@ -192,25 +227,29 @@ class StrokeGenerator {
for (size_t point_i = component_start_index; point_i < component_end_index;
point_i++) {
bool is_end_of_component = point_i == component_end_index - 1;
vtx.position = polyline.GetPoint(point_i) + offset;

Point offset_vector = offset.GetVector();

vtx.position = polyline.GetPoint(point_i) + offset_vector;
vtx_builder.AppendVertex(vtx.position);
vtx.position = polyline.GetPoint(point_i) - offset;
vtx.position = polyline.GetPoint(point_i) - offset_vector;
vtx_builder.AppendVertex(vtx.position);

// For line components, two additional points need to be appended
// prior to appending a join connecting the next component.
vtx.position = polyline.GetPoint(point_i + 1) + offset;
vtx.position = polyline.GetPoint(point_i + 1) + offset_vector;
vtx_builder.AppendVertex(vtx.position);
vtx.position = polyline.GetPoint(point_i + 1) - offset;
vtx.position = polyline.GetPoint(point_i + 1) - offset_vector;
vtx_builder.AppendVertex(vtx.position);

previous_offset = offset;
offset = ComputeOffset(point_i + 2, contour_start_point_i,
contour_end_point_i, contour);
if (!is_last_component && is_end_of_component) {
// Generate join from the current line to the next line.
join_proc(vtx_builder, polyline.GetPoint(point_i + 1), previous_offset,
offset, scaled_miter_limit, scale);
join_proc(vtx_builder, polyline.GetPoint(point_i + 1),
previous_offset.GetVector(), offset.GetVector(),
scaled_miter_limit, scale);
}
}
}
Expand All @@ -228,14 +267,44 @@ class StrokeGenerator {
point_i++) {
bool is_end_of_component = point_i == component_end_index - 1;

vtx.position = polyline.GetPoint(point_i) + offset;
vtx.position = polyline.GetPoint(point_i) + offset.GetVector();
vtx_builder.AppendVertex(vtx.position);
vtx.position = polyline.GetPoint(point_i) - offset;
vtx.position = polyline.GetPoint(point_i) - offset.GetVector();
vtx_builder.AppendVertex(vtx.position);

previous_offset = offset;
offset = ComputeOffset(point_i + 2, contour_start_point_i,
contour_end_point_i, contour);

// If the angle to the next segment is too sharp, round out the join.
if (!is_end_of_component) {
constexpr Scalar kAngleThreshold = 10 * kPi / 180;
// `std::cosf` is not constexpr-able, unfortunately, so we have to bake
// the alignment constant.
constexpr Scalar kAlignmentThreshold =
0.984807753012208; // std::cosf(kThresholdAngle) -- 10 degrees

// Use a cheap dot product to determine whether the angle is too sharp.
if (previous_offset.GetAlignment(offset) < kAlignmentThreshold) {
Scalar angle_total = previous_offset.AngleTo(offset).radians;
Scalar angle = kAngleThreshold;

// Bridge the large angle with additional geometry at
// `kAngleThreshold` interval.
while (angle < std::abs(angle_total)) {
Scalar signed_angle = angle_total < 0 ? -angle : angle;
Point offset =
previous_offset.GetVector().Rotate(Radians(signed_angle));
vtx.position = polyline.GetPoint(point_i) + offset;
vtx_builder.AppendVertex(vtx.position);
vtx.position = polyline.GetPoint(point_i) - offset;
vtx_builder.AppendVertex(vtx.position);

angle += kAngleThreshold;
}
}
}

// For curve components, the polyline is detailed enough such that
// it can avoid worrying about joins altogether.
if (is_end_of_component) {
Expand All @@ -245,16 +314,18 @@ class StrokeGenerator {
// `ComputeOffset` returns the contour's end direction when attempting
// to grab offsets past `contour_end_point_i`, so just use `offset` when
// we're on the last component.
Point last_component_offset =
is_last_component ? offset : previous_offset;
Point last_component_offset = is_last_component
? offset.GetVector()
: previous_offset.GetVector();
vtx.position = polyline.GetPoint(point_i + 1) + last_component_offset;
vtx_builder.AppendVertex(vtx.position);
vtx.position = polyline.GetPoint(point_i + 1) - last_component_offset;
vtx_builder.AppendVertex(vtx.position);
// Generate join from the current line to the next line.
if (!is_last_component) {
join_proc(vtx_builder, polyline.GetPoint(point_i + 1),
previous_offset, offset, scaled_miter_limit, scale);
previous_offset.GetVector(), offset.GetVector(),
scaled_miter_limit, scale);
}
}
}
Expand All @@ -267,8 +338,8 @@ class StrokeGenerator {
const CapProc<VertexWriter>& cap_proc;
const Scalar scale;

Point previous_offset;
Point offset;
Ray previous_offset;
Ray offset;
SolidFillVertexShader::PerVertexData vtx;
};

Expand Down
30 changes: 30 additions & 0 deletions impeller/geometry/geometry_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,36 @@ TEST(GeometryTest, PointAbs) {
ASSERT_POINT_NEAR(a_abs, expected);
}

TEST(GeometryTest, PointRotate) {
{
Point a(1, 0);
auto rotated = a.Rotate(Radians{kPiOver2});
auto expected = Point(0, 1);
ASSERT_POINT_NEAR(rotated, expected);
}

{
Point a(1, 0);
auto rotated = a.Rotate(Radians{-kPiOver2});
auto expected = Point(0, -1);
ASSERT_POINT_NEAR(rotated, expected);
}

{
Point a(1, 0);
auto rotated = a.Rotate(Radians{kPi});
auto expected = Point(-1, 0);
ASSERT_POINT_NEAR(rotated, expected);
}

{
Point a(1, 0);
auto rotated = a.Rotate(Radians{kPi * 1.5});
auto expected = Point(0, -1);
ASSERT_POINT_NEAR(rotated, expected);
}
}

TEST(GeometryTest, PointAngleTo) {
// Negative result in the CCW (with up = -Y) direction.
{
Expand Down
6 changes: 6 additions & 0 deletions impeller/geometry/point.h
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,12 @@ struct TPoint {
return *this - axis * this->Dot(axis) * 2;
}

constexpr TPoint Rotate(const Radians& angle) const {
const auto cos_a = std::cosf(angle.radians);
const auto sin_a = std::sinf(angle.radians);
return {x * cos_a - y * sin_a, x * sin_a + y * cos_a};
}

constexpr Radians AngleTo(const TPoint& p) const {
return Radians{std::atan2(this->Cross(p), this->Dot(p))};
}
Expand Down
3 changes: 3 additions & 0 deletions testing/impeller_golden_tests_output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,9 @@ impeller_Play_AiksTest_CanRenderNestedClips_Vulkan.png
impeller_Play_AiksTest_CanRenderOverlappingMultiContourPath_Metal.png
impeller_Play_AiksTest_CanRenderOverlappingMultiContourPath_OpenGLES.png
impeller_Play_AiksTest_CanRenderOverlappingMultiContourPath_Vulkan.png
impeller_Play_AiksTest_CanRenderQuadraticStrokeWithInstantTurn_Metal.png
impeller_Play_AiksTest_CanRenderQuadraticStrokeWithInstantTurn_OpenGLES.png
impeller_Play_AiksTest_CanRenderQuadraticStrokeWithInstantTurn_Vulkan.png
impeller_Play_AiksTest_CanRenderRadialGradientManyColors_Metal.png
impeller_Play_AiksTest_CanRenderRadialGradientManyColors_OpenGLES.png
impeller_Play_AiksTest_CanRenderRadialGradientManyColors_Vulkan.png
Expand Down