Skip to content

Commit 85721f8

Browse files
JimBobSquarePantsJimBobSquarePants
authored
Merge pull request #302 from SixLabors/js/fix-arcs-plus-degenerate-outlining
Fix arcs and do not throw when outlining
2 parents dfae51c + edb6db2 commit 85721f8

File tree

14 files changed

+149
-66
lines changed

14 files changed

+149
-66
lines changed

‎src/ImageSharp.Drawing/Shapes/ArcLineSegment.cs‎

Lines changed: 43 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,20 @@ public class ArcLineSegment : ILineSegment
3232
public ArcLineSegment(PointF from, PointF to, SizeF radius, float rotation, bool largeArc, bool sweep)
3333
{
3434
rotation = GeometryUtilities.DegreeToRadian(rotation);
35-
bool circle = largeArc && ((Vector2)to - (Vector2)from).LengthSquared() < ZeroTolerance && radius.Width > 0 && radius.Height > 0;
36-
this.linePoints = EllipticArcFromEndParams(from, to, radius, rotation, largeArc, sweep, circle);
37-
this.EndPoint = this.linePoints[this.linePoints.Length - 1];
35+
bool ellipse = largeArc && ((Vector2)to - (Vector2)from).LengthSquared() < ZeroTolerance && radius.Width > 0 && radius.Height > 0;
36+
if (ellipse)
37+
{
38+
// The circle always has a start angle of 0 which is positioned at 3 o'clock.
39+
// This means the centre point is to the left of the start position.
40+
Vector2 center = (Vector2)from - new Vector2(radius.Width, 0);
41+
this.linePoints = EllipticArcToBezierCurve(from, center, radius, rotation, 0, sweep ? 2 * MathF.PI : -2 * MathF.PI);
42+
}
43+
else
44+
{
45+
this.linePoints = EllipticArcFromEndParams(from, to, radius, rotation, largeArc, sweep);
46+
}
47+
48+
this.EndPoint = this.linePoints[^1];
3849
}
3950

4051
/// <summary>
@@ -59,16 +70,24 @@ public ArcLineSegment(PointF center, SizeF radius, float rotation, float startAn
5970

6071
bool largeArc = Math.Abs(sweepAngle) > MathF.PI;
6172
bool sweep = sweepAngle > 0;
62-
bool circle = largeArc && (to - from).LengthSquared() < ZeroTolerance && radius.Width > 0 && radius.Height > 0;
73+
bool ellipse = largeArc && (to - from).LengthSquared() < ZeroTolerance && radius.Width > 0 && radius.Height > 0;
6374

64-
this.linePoints = EllipticArcFromEndParams(from, to, radius, rotation, largeArc, sweep, circle);
65-
this.EndPoint = this.linePoints[this.linePoints.Length - 1];
75+
if (ellipse)
76+
{
77+
this.linePoints = EllipticArcToBezierCurve(from, center, radius, rotation, startAngle, sweepAngle);
78+
}
79+
else
80+
{
81+
this.linePoints = EllipticArcFromEndParams(from, to, radius, rotation, largeArc, sweep);
82+
}
83+
84+
this.EndPoint = this.linePoints[^1];
6685
}
6786

6887
private ArcLineSegment(PointF[] linePoints)
6988
{
7089
this.linePoints = linePoints;
71-
this.EndPoint = this.linePoints[this.linePoints.Length - 1];
90+
this.EndPoint = this.linePoints[^1];
7291
}
7392

7493
/// <inheritdoc/>
@@ -89,7 +108,7 @@ public ILineSegment Transform(Matrix3x2 matrix)
89108
return this;
90109
}
91110

92-
var transformedPoints = new PointF[this.linePoints.Length];
111+
PointF[] transformedPoints = new PointF[this.linePoints.Length];
93112
for (int i = 0; i < this.linePoints.Length; i++)
94113
{
95114
transformedPoints[i] = PointF.Transform(this.linePoints[i], matrix);
@@ -101,32 +120,23 @@ public ILineSegment Transform(Matrix3x2 matrix)
101120
/// <inheritdoc/>
102121
ILineSegment ILineSegment.Transform(Matrix3x2 matrix) => this.Transform(matrix);
103122

104-
private static PointF[] EllipticArcFromEndParams(PointF from, PointF to, SizeF radius, float rotation, bool largeArc, bool sweep, bool circle)
123+
private static PointF[] EllipticArcFromEndParams(
124+
PointF from,
125+
PointF to,
126+
SizeF radius,
127+
float rotation,
128+
bool largeArc,
129+
bool sweep)
105130
{
106-
{
107-
var absRadius = Vector2.Abs(radius);
108-
109-
if (circle)
110-
{
111-
// It's a circle. SVG arcs cannot handle this so let's hack together our own angles.
112-
// This appears to match the behavior of Web CanvasRenderingContext2D.arc().
113-
// https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/arc
114-
Vector2 center = (Vector2)from - new Vector2(absRadius.X, 0);
115-
return EllipticArcToBezierCurve(from, center, absRadius, rotation, 0, 2 * MathF.PI);
116-
}
117-
else
118-
{
119-
if (EllipticArcOutOfRange(from, to, radius))
120-
{
121-
return new[] { from, to };
122-
}
123-
124-
float xRotation = rotation;
125-
EndpointToCenterArcParams(from, to, ref absRadius, xRotation, largeArc, sweep, out Vector2 center, out Vector2 angles);
131+
Vector2 absRadius = Vector2.Abs(radius);
126132

127-
return EllipticArcToBezierCurve(from, center, absRadius, xRotation, angles.X, angles.Y);
128-
}
133+
if (EllipticArcOutOfRange(from, to, radius))
134+
{
135+
return new[] { from, to };
129136
}
137+
138+
EndpointToCenterArcParams(from, to, ref absRadius, rotation, largeArc, sweep, out Vector2 center, out Vector2 angles);
139+
return EllipticArcToBezierCurve(from, center, absRadius, rotation, angles.X, angles.Y);
130140
}
131141

132142
[MethodImpl(MethodImplOptions.AggressiveInlining)]
@@ -296,8 +306,8 @@ private static float Clamp(float val, float min, float max)
296306
[MethodImpl(MethodImplOptions.AggressiveInlining)]
297307
private static float SvgAngle(double ux, double uy, double vx, double vy)
298308
{
299-
var u = new Vector2((float)ux, (float)uy);
300-
var v = new Vector2((float)vx, (float)vy);
309+
Vector2 u = new((float)ux, (float)uy);
310+
Vector2 v = new((float)vx, (float)vy);
301311

302312
// (F.6.5.4)
303313
float dot = Vector2.Dot(u, v);

‎src/ImageSharp.Drawing/Shapes/PolygonClipper/BoundsF.cs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,6 @@ internal struct BoundsF
1414

1515
public BoundsF(float l, float t, float r, float b)
1616
{
17-
Guard.MustBeGreaterThanOrEqualTo(r, l, nameof(r));
18-
Guard.MustBeGreaterThanOrEqualTo(b, t, nameof(r));
19-
2017
this.Left = l;
2118
this.Top = t;
2219
this.Right = r;

‎src/ImageSharp.Drawing/Shapes/PolygonClipper/PolygonOffsetter.cs‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,14 @@ public void Execute(float delta, PathsF solution)
9999
clipper.Execute(ClippingOperation.Union, FillRule.Positive, solution);
100100
}
101101

102-
// PolygonClipper will throw for unhandled exceptions but we need to explicitly capture an empty result.
102+
// PolygonClipper will throw for unhandled exceptions but if a result is empty
103+
// we should just return the original path.
103104
if (solution.Count == 0)
104105
{
105-
throw new ClipperException("An error occurred while attempting to clip the polygon. Check input for invalid entries.");
106+
foreach (PathF path in this.solution)
107+
{
108+
solution.Add(path);
109+
}
106110
}
107111
}
108112

@@ -213,9 +217,9 @@ private void DoGroupOffset(Group group)
213217
}
214218
else
215219
{
216-
Vector2 d = new(MathF.Ceiling(this.groupDelta));
217-
Vector2 xy = path[0] - d;
218-
BoundsF r = new(xy.X, xy.Y, xy.X, xy.Y);
220+
float d = this.groupDelta;
221+
Vector2 xy = path[0];
222+
BoundsF r = new(xy.X - d, xy.Y - d, xy.X + d, xy.Y + d);
219223
group.OutPath = r.AsPath();
220224
}
221225

‎tests/ImageSharp.Drawing.Tests/Drawing/DrawLinesTests.cs‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,36 @@ public void DrawLines_Simple<TPixel>(TestImageProvider<TPixel> provider, string
1919
where TPixel : unmanaged, IPixel<TPixel>
2020
{
2121
Color color = TestUtils.GetColorByName(colorName).WithAlpha(alpha);
22-
var pen = new SolidPen(color, thickness);
22+
SolidPen pen = new(color, thickness);
2323

2424
DrawLinesImpl(provider, colorName, alpha, thickness, antialias, pen);
2525
}
2626

27+
[Theory]
28+
[WithSolidFilledImages(30, 30, "White", PixelTypes.Rgba32, 1f, true)]
29+
[WithSolidFilledImages(30, 30, "White", PixelTypes.Rgba32, 5f, true)]
30+
[WithSolidFilledImages(30, 30, "White", PixelTypes.Rgba32, 1f, false)]
31+
[WithSolidFilledImages(30, 30, "White", PixelTypes.Rgba32, 5f, false)]
32+
public void DrawLinesInvalidPoints<TPixel>(TestImageProvider<TPixel> provider, float thickness, bool antialias)
33+
where TPixel : unmanaged, IPixel<TPixel>
34+
{
35+
SolidPen pen = new(Color.Black, thickness);
36+
PointF[] path = { new Vector2(15f, 15f), new Vector2(15f, 15f) };
37+
38+
GraphicsOptions options = new()
39+
{
40+
Antialias = antialias
41+
};
42+
43+
string aa = antialias ? string.Empty : "_NoAntialias";
44+
FormattableString outputDetails = $"T({thickness}){aa}";
45+
46+
provider.RunValidatingProcessorTest(
47+
c => c.SetGraphicsOptions(options).DrawLine(pen, path),
48+
outputDetails,
49+
appendSourceFileOrDescription: false);
50+
}
51+
2752
[Theory]
2853
[WithBasicTestPatternImages(250, 350, PixelTypes.Rgba32, "White", 1f, 5, false)]
2954
public void DrawLines_Dash<TPixel>(TestImageProvider<TPixel> provider, string colorName, float alpha, float thickness, bool antialias)
@@ -74,7 +99,7 @@ public void DrawLines_EndCapRound<TPixel>(TestImageProvider<TPixel> provider, st
7499
where TPixel : unmanaged, IPixel<TPixel>
75100
{
76101
Color color = TestUtils.GetColorByName(colorName).WithAlpha(alpha);
77-
PatternPen pen = new PatternPen(new PenOptions(color, thickness, new float[] { 3f, 3f }) { EndCapStyle = EndCapStyle.Round });
102+
PatternPen pen = new(new PenOptions(color, thickness, new float[] { 3f, 3f }) { EndCapStyle = EndCapStyle.Round });
78103

79104
DrawLinesImpl(provider, colorName, alpha, thickness, antialias, pen);
80105
}
@@ -85,7 +110,7 @@ public void DrawLines_EndCapButt<TPixel>(TestImageProvider<TPixel> provider, str
85110
where TPixel : unmanaged, IPixel<TPixel>
86111
{
87112
Color color = TestUtils.GetColorByName(colorName).WithAlpha(alpha);
88-
PatternPen pen = new PatternPen(new PenOptions(color, thickness, new float[] { 3f, 3f }) { EndCapStyle = EndCapStyle.Butt });
113+
PatternPen pen = new(new PenOptions(color, thickness, new float[] { 3f, 3f }) { EndCapStyle = EndCapStyle.Butt });
89114

90115
DrawLinesImpl(provider, colorName, alpha, thickness, antialias, pen);
91116
}
@@ -96,7 +121,7 @@ public void DrawLines_EndCapSquare<TPixel>(TestImageProvider<TPixel> provider, s
96121
where TPixel : unmanaged, IPixel<TPixel>
97122
{
98123
Color color = TestUtils.GetColorByName(colorName).WithAlpha(alpha);
99-
PatternPen pen = new PatternPen(new PenOptions(color, thickness, new float[] { 3f, 3f }) { EndCapStyle = EndCapStyle.Square });
124+
PatternPen pen = new(new PenOptions(color, thickness, new float[] { 3f, 3f }) { EndCapStyle = EndCapStyle.Square });
100125

101126
DrawLinesImpl(provider, colorName, alpha, thickness, antialias, pen);
102127
}
@@ -107,7 +132,7 @@ public void DrawLines_JointStyleRound<TPixel>(TestImageProvider<TPixel> provider
107132
where TPixel : unmanaged, IPixel<TPixel>
108133
{
109134
Color color = TestUtils.GetColorByName(colorName).WithAlpha(alpha);
110-
var pen = new SolidPen(new PenOptions(color, thickness) { JointStyle = JointStyle.Round });
135+
SolidPen pen = new(new PenOptions(color, thickness) { JointStyle = JointStyle.Round });
111136

112137
DrawLinesImpl(provider, colorName, alpha, thickness, antialias, pen);
113138
}
@@ -118,7 +143,7 @@ public void DrawLines_JointStyleSquare<TPixel>(TestImageProvider<TPixel> provide
118143
where TPixel : unmanaged, IPixel<TPixel>
119144
{
120145
Color color = TestUtils.GetColorByName(colorName).WithAlpha(alpha);
121-
var pen = new SolidPen(new PenOptions(color, thickness) { JointStyle = JointStyle.Square });
146+
SolidPen pen = new(new PenOptions(color, thickness) { JointStyle = JointStyle.Square });
122147

123148
DrawLinesImpl(provider, colorName, alpha, thickness, antialias, pen);
124149
}
@@ -129,7 +154,7 @@ public void DrawLines_JointStyleMiter<TPixel>(TestImageProvider<TPixel> provider
129154
where TPixel : unmanaged, IPixel<TPixel>
130155
{
131156
Color color = TestUtils.GetColorByName(colorName).WithAlpha(alpha);
132-
var pen = new SolidPen(new PenOptions(color, thickness) { JointStyle = JointStyle.Miter });
157+
SolidPen pen = new(new PenOptions(color, thickness) { JointStyle = JointStyle.Miter });
133158

134159
DrawLinesImpl(provider, colorName, alpha, thickness, antialias, pen);
135160
}
@@ -145,7 +170,8 @@ private static void DrawLinesImpl<TPixel>(
145170
{
146171
PointF[] simplePath = { new Vector2(10, 10), new Vector2(200, 150), new Vector2(50, 300) };
147172

148-
var options = new GraphicsOptions { Antialias = antialias };
173+
GraphicsOptions options = new()
174+
{ Antialias = antialias };
149175

150176
string aa = antialias ? string.Empty : "_NoAntialias";
151177
FormattableString outputDetails = $"{colorName}_A({alpha})_T({thickness}){aa}";

‎tests/ImageSharp.Drawing.Tests/Drawing/DrawPathTests.cs‎

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,20 +26,20 @@ public class DrawPathTests
2626
public void DrawPath<TPixel>(TestImageProvider<TPixel> provider, string colorName, byte alpha, float thickness)
2727
where TPixel : unmanaged, IPixel<TPixel>
2828
{
29-
var linearSegment = new LinearLineSegment(
29+
LinearLineSegment linearSegment = new(
3030
new Vector2(10, 10),
3131
new Vector2(200, 150),
3232
new Vector2(50, 300));
33-
var bezierSegment = new CubicBezierLineSegment(
33+
CubicBezierLineSegment bezierSegment = new(
3434
new Vector2(50, 300),
3535
new Vector2(500, 500),
3636
new Vector2(60, 10),
3737
new Vector2(10, 400));
3838

39-
var ellipticArcSegment1 = new ArcLineSegment(new Vector2(10, 400), new Vector2(150, 450), new SizeF((float)Math.Sqrt(5525), 40), GeometryUtilities.RadianToDegree((float)Math.Atan2(25, 70)), true, true);
40-
var ellipticArcSegment2 = new ArcLineSegment(new(150, 450), new(149F, 450), new SizeF(140, 70), 0, true, true);
39+
ArcLineSegment ellipticArcSegment1 = new(new Vector2(10, 400), new Vector2(150, 450), new SizeF((float)Math.Sqrt(5525), 40), GeometryUtilities.RadianToDegree((float)Math.Atan2(25, 70)), true, true);
40+
ArcLineSegment ellipticArcSegment2 = new(new(150, 450), new(149F, 450), new SizeF(140, 70), 0, true, true);
4141

42-
var path = new Path(linearSegment, bezierSegment, ellipticArcSegment1, ellipticArcSegment2);
42+
Path path = new(linearSegment, bezierSegment, ellipticArcSegment1, ellipticArcSegment2);
4343

4444
Rgba32 rgba = TestUtils.GetColorByName(colorName);
4545
rgba.A = alpha;
@@ -67,7 +67,7 @@ public void PathExtendingOffEdgeOfImageShouldNotBeCropped<TPixel>(TestImageProvi
6767
{
6868
for (int i = 0; i < 300; i += 20)
6969
{
70-
var points = new PointF[] { new Vector2(100, 2), new Vector2(-10, i) };
70+
PointF[] points = new PointF[] { new Vector2(100, 2), new Vector2(-10, i) };
7171
x.DrawLine(pen, points);
7272
}
7373
},
@@ -91,7 +91,38 @@ public void DrawPathClippedOnTop<TPixel>(TestImageProvider<TPixel> provider)
9191

9292
provider.VerifyOperation(
9393
image => image.Mutate(x => x.Draw(Color.Black, 1, path)),
94-
appendSourceFileOrDescription: false,
95-
appendPixelTypeToFileName: false);
94+
appendPixelTypeToFileName: false,
95+
appendSourceFileOrDescription: false);
96+
}
97+
98+
[Theory]
99+
[WithSolidFilledImages(300, 300, "White", PixelTypes.Rgba32, 360)]
100+
[WithSolidFilledImages(300, 300, "White", PixelTypes.Rgba32, 359)]
101+
public void DrawCircleUsingAddArc<TPixel>(TestImageProvider<TPixel> provider, float sweep)
102+
where TPixel : unmanaged, IPixel<TPixel>
103+
{
104+
IPath path = new PathBuilder().AddArc(new Point(150, 150), 50, 50, 0, 40, sweep).Build();
105+
106+
provider.VerifyOperation(
107+
image => image.Mutate(x => x.Draw(Color.Black, 1, path)),
108+
testOutputDetails: $"{sweep}",
109+
appendPixelTypeToFileName: false,
110+
appendSourceFileOrDescription: false);
111+
}
112+
113+
[Theory]
114+
[WithSolidFilledImages(300, 300, "White", PixelTypes.Rgba32, true)]
115+
[WithSolidFilledImages(300, 300, "White", PixelTypes.Rgba32, false)]
116+
public void DrawCircleUsingArcTo<TPixel>(TestImageProvider<TPixel> provider, bool sweep)
117+
where TPixel : unmanaged, IPixel<TPixel>
118+
{
119+
Point origin = new(150, 150);
120+
IPath path = new PathBuilder().MoveTo(origin).ArcTo(50, 50, 0, true, sweep, origin).Build();
121+
122+
provider.VerifyOperation(
123+
image => image.Mutate(x => x.Draw(Color.Black, 1, path)),
124+
testOutputDetails: $"{sweep}",
125+
appendPixelTypeToFileName: false,
126+
appendSourceFileOrDescription: false);
96127
}
97128
}

‎tests/ImageSharp.Drawing.Tests/Shapes/PolygonClipper/ClipperTests.cs‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -130,13 +130,4 @@ public void ClippingRectanglesCreateCorrectNumberOfPoints()
130130

131131
Assert.Equal(8, points.Count);
132132
}
133-
134-
[Fact]
135-
public void ClipperOffsetThrowsPublicException()
136-
{
137-
PointF naan = new(float.NaN, float.NaN);
138-
Polygon path = new(new LinearLineSegment(new[] { naan, naan, naan, naan }));
139-
140-
Assert.Throws<ClipperException>(() => path.GenerateOutline(10));
141-
}
142133
}

‎tests/Images/ReferenceOutput/Drawing/DrawLinesTests/DrawLinesInvalidPoints_Rgba32_T(1).png‎

Lines changed: 3 additions & 0 deletions
Loading

‎tests/Images/ReferenceOutput/Drawing/DrawLinesTests/DrawLinesInvalidPoints_Rgba32_T(1)_NoAntialias.png‎

Lines changed: 3 additions & 0 deletions
Loading

‎tests/Images/ReferenceOutput/Drawing/DrawLinesTests/DrawLinesInvalidPoints_Rgba32_T(5).png‎

Lines changed: 3 additions & 0 deletions
Loading

‎tests/Images/ReferenceOutput/Drawing/DrawLinesTests/DrawLinesInvalidPoints_Rgba32_T(5)_NoAntialias.png‎

Lines changed: 3 additions & 0 deletions
Loading

0 commit comments

Comments
 (0)