Merge pull request #302 from SixLabors/js/fix-arcs-plus-degenerate-ou…

…tlining

Fix arcs and do not throw when outlining

src/ImageSharp.Drawing/Shapes/ArcLineSegment.cs

@@ -32,9 +32,20 @@ public class ArcLineSegment : ILineSegment
     public ArcLineSegment(PointF from, PointF to, SizeF radius, float rotation, bool largeArc, bool sweep)
     {
         rotation = GeometryUtilities.DegreeToRadian(rotation);
-        bool circle = largeArc && ((Vector2)to - (Vector2)from).LengthSquared() < ZeroTolerance && radius.Width > 0 && radius.Height > 0;
-        this.linePoints = EllipticArcFromEndParams(from, to, radius, rotation, largeArc, sweep, circle);
-        this.EndPoint = this.linePoints[this.linePoints.Length - 1];
+        bool ellipse = largeArc && ((Vector2)to - (Vector2)from).LengthSquared() < ZeroTolerance && radius.Width > 0 && radius.Height > 0;
+        if (ellipse)
+        {
+            // The circle always has a start angle of 0 which is positioned at 3 o'clock.
+            // This means the centre point is to the left of the start position.
+            Vector2 center = (Vector2)from - new Vector2(radius.Width, 0);
+            this.linePoints = EllipticArcToBezierCurve(from, center, radius, rotation, 0, sweep ? 2 * MathF.PI : -2 * MathF.PI);
+        }
+        else
+        {
+            this.linePoints = EllipticArcFromEndParams(from, to, radius, rotation, largeArc, sweep);
+        }
+
+        this.EndPoint = this.linePoints[^1];
     }
 
     /// <summary>
@@ -59,16 +70,24 @@ public ArcLineSegment(PointF center, SizeF radius, float rotation, float startAn
 
         bool largeArc = Math.Abs(sweepAngle) > MathF.PI;
         bool sweep = sweepAngle > 0;
-        bool circle = largeArc && (to - from).LengthSquared() < ZeroTolerance && radius.Width > 0 && radius.Height > 0;
+        bool ellipse = largeArc && (to - from).LengthSquared() < ZeroTolerance && radius.Width > 0 && radius.Height > 0;
 
-        this.linePoints = EllipticArcFromEndParams(from, to, radius, rotation, largeArc, sweep, circle);
-        this.EndPoint = this.linePoints[this.linePoints.Length - 1];
+        if (ellipse)
+        {
+            this.linePoints = EllipticArcToBezierCurve(from, center, radius, rotation, startAngle, sweepAngle);
+        }
+        else
+        {
+            this.linePoints = EllipticArcFromEndParams(from, to, radius, rotation, largeArc, sweep);
+        }
+
+        this.EndPoint = this.linePoints[^1];
     }
 
     private ArcLineSegment(PointF[] linePoints)
     {
         this.linePoints = linePoints;
-        this.EndPoint = this.linePoints[this.linePoints.Length - 1];
+        this.EndPoint = this.linePoints[^1];
     }
 
     /// <inheritdoc/>
@@ -89,7 +108,7 @@ public ILineSegment Transform(Matrix3x2 matrix)
             return this;
         }
 
-        var transformedPoints = new PointF[this.linePoints.Length];
+        PointF[] transformedPoints = new PointF[this.linePoints.Length];
         for (int i = 0; i < this.linePoints.Length; i++)
         {
             transformedPoints[i] = PointF.Transform(this.linePoints[i], matrix);
@@ -101,32 +120,23 @@ public ILineSegment Transform(Matrix3x2 matrix)
     /// <inheritdoc/>
     ILineSegment ILineSegment.Transform(Matrix3x2 matrix) => this.Transform(matrix);
 
-    private static PointF[] EllipticArcFromEndParams(PointF from, PointF to, SizeF radius, float rotation, bool largeArc, bool sweep, bool circle)
+    private static PointF[] EllipticArcFromEndParams(
+        PointF from,
+        PointF to,
+        SizeF radius,
+        float rotation,
+        bool largeArc,
+        bool sweep)
     {
-        {
-            var absRadius = Vector2.Abs(radius);
-
-            if (circle)
-            {
-                // It's a circle. SVG arcs cannot handle this so let's hack together our own angles.
-                // This appears to match the behavior of Web CanvasRenderingContext2D.arc().
-                // https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/arc
-                Vector2 center = (Vector2)from - new Vector2(absRadius.X, 0);
-                return EllipticArcToBezierCurve(from, center, absRadius, rotation, 0, 2 * MathF.PI);
-            }
-            else
-            {
-                if (EllipticArcOutOfRange(from, to, radius))
-                {
-                    return new[] { from, to };
-                }
-
-                float xRotation = rotation;
-                EndpointToCenterArcParams(from, to, ref absRadius, xRotation, largeArc, sweep, out Vector2 center, out Vector2 angles);
+        Vector2 absRadius = Vector2.Abs(radius);
 
-                return EllipticArcToBezierCurve(from, center, absRadius, xRotation, angles.X, angles.Y);
-            }
+        if (EllipticArcOutOfRange(from, to, radius))
+        {
+            return new[] { from, to };
         }
+
+        EndpointToCenterArcParams(from, to, ref absRadius, rotation, largeArc, sweep, out Vector2 center, out Vector2 angles);
+        return EllipticArcToBezierCurve(from, center, absRadius, rotation, angles.X, angles.Y);
     }
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
@@ -296,8 +306,8 @@ private static float Clamp(float val, float min, float max)
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     private static float SvgAngle(double ux, double uy, double vx, double vy)
     {
-        var u = new Vector2((float)ux, (float)uy);
-        var v = new Vector2((float)vx, (float)vy);
+        Vector2 u = new((float)ux, (float)uy);
+        Vector2 v = new((float)vx, (float)vy);
 
         // (F.6.5.4)
         float dot = Vector2.Dot(u, v);

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

@@ -14,9 +14,6 @@ internal struct BoundsF
 
     public BoundsF(float l, float t, float r, float b)
     {
-        Guard.MustBeGreaterThanOrEqualTo(r, l, nameof(r));
-        Guard.MustBeGreaterThanOrEqualTo(b, t, nameof(r));
-
         this.Left = l;
         this.Top = t;
         this.Right = r;

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

@@ -99,10 +99,14 @@ public void Execute(float delta, PathsF solution)
             clipper.Execute(ClippingOperation.Union, FillRule.Positive, solution);
         }
 
-        // PolygonClipper will throw for unhandled exceptions but we need to explicitly capture an empty result.
+        // PolygonClipper will throw for unhandled exceptions but if a result is empty
+        // we should just return the original path.
         if (solution.Count == 0)
         {
-            throw new ClipperException("An error occurred while attempting to clip the polygon. Check input for invalid entries.");
+            foreach (PathF path in this.solution)
+            {
+                solution.Add(path);
+            }
         }
     }
 
@@ -213,9 +217,9 @@ private void DoGroupOffset(Group group)
                 }
                 else
                 {
-                    Vector2 d = new(MathF.Ceiling(this.groupDelta));
-                    Vector2 xy = path[0] - d;
-                    BoundsF r = new(xy.X, xy.Y, xy.X, xy.Y);
+                    float d = this.groupDelta;
+                    Vector2 xy = path[0];
+                    BoundsF r = new(xy.X - d, xy.Y - d, xy.X + d, xy.Y + d);
                     group.OutPath = r.AsPath();
                 }
 

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

@@ -19,11 +19,36 @@ public void DrawLines_Simple<TPixel>(TestImageProvider<TPixel> provider, string
         where TPixel : unmanaged, IPixel<TPixel>
     {
         Color color = TestUtils.GetColorByName(colorName).WithAlpha(alpha);
-        var pen = new SolidPen(color, thickness);
+        SolidPen pen = new(color, thickness);
 
         DrawLinesImpl(provider, colorName, alpha, thickness, antialias, pen);
     }
 
+    [Theory]
+    [WithSolidFilledImages(30, 30, "White", PixelTypes.Rgba32, 1f, true)]
+    [WithSolidFilledImages(30, 30, "White", PixelTypes.Rgba32, 5f, true)]
+    [WithSolidFilledImages(30, 30, "White", PixelTypes.Rgba32, 1f, false)]
+    [WithSolidFilledImages(30, 30, "White", PixelTypes.Rgba32, 5f, false)]
+    public void DrawLinesInvalidPoints<TPixel>(TestImageProvider<TPixel> provider, float thickness, bool antialias)
+        where TPixel : unmanaged, IPixel<TPixel>
+    {
+        SolidPen pen = new(Color.Black, thickness);
+        PointF[] path = { new Vector2(15f, 15f), new Vector2(15f, 15f) };
+
+        GraphicsOptions options = new()
+        {
+            Antialias = antialias
+        };
+
+        string aa = antialias ? string.Empty : "_NoAntialias";
+        FormattableString outputDetails = $"T({thickness}){aa}";
+
+        provider.RunValidatingProcessorTest(
+            c => c.SetGraphicsOptions(options).DrawLine(pen, path),
+            outputDetails,
+            appendSourceFileOrDescription: false);
+    }
+
     [Theory]
     [WithBasicTestPatternImages(250, 350, PixelTypes.Rgba32, "White", 1f, 5, false)]
     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
         where TPixel : unmanaged, IPixel<TPixel>
     {
         Color color = TestUtils.GetColorByName(colorName).WithAlpha(alpha);
-        PatternPen pen = new PatternPen(new PenOptions(color, thickness, new float[] { 3f, 3f }) { EndCapStyle = EndCapStyle.Round });
+        PatternPen pen = new(new PenOptions(color, thickness, new float[] { 3f, 3f }) { EndCapStyle = EndCapStyle.Round });
 
         DrawLinesImpl(provider, colorName, alpha, thickness, antialias, pen);
     }
@@ -85,7 +110,7 @@ public void DrawLines_EndCapButt<TPixel>(TestImageProvider<TPixel> provider, str
 where TPixel : unmanaged, IPixel<TPixel>
     {
         Color color = TestUtils.GetColorByName(colorName).WithAlpha(alpha);
-        PatternPen pen = new PatternPen(new PenOptions(color, thickness, new float[] { 3f, 3f }) { EndCapStyle = EndCapStyle.Butt });
+        PatternPen pen = new(new PenOptions(color, thickness, new float[] { 3f, 3f }) { EndCapStyle = EndCapStyle.Butt });
 
         DrawLinesImpl(provider, colorName, alpha, thickness, antialias, pen);
     }
@@ -96,7 +121,7 @@ public void DrawLines_EndCapSquare<TPixel>(TestImageProvider<TPixel> provider, s
 where TPixel : unmanaged, IPixel<TPixel>
     {
         Color color = TestUtils.GetColorByName(colorName).WithAlpha(alpha);
-        PatternPen pen = new PatternPen(new PenOptions(color, thickness, new float[] { 3f, 3f }) { EndCapStyle = EndCapStyle.Square });
+        PatternPen pen = new(new PenOptions(color, thickness, new float[] { 3f, 3f }) { EndCapStyle = EndCapStyle.Square });
 
         DrawLinesImpl(provider, colorName, alpha, thickness, antialias, pen);
     }
@@ -107,7 +132,7 @@ public void DrawLines_JointStyleRound<TPixel>(TestImageProvider<TPixel> provider
 where TPixel : unmanaged, IPixel<TPixel>
     {
         Color color = TestUtils.GetColorByName(colorName).WithAlpha(alpha);
-        var pen = new SolidPen(new PenOptions(color, thickness) { JointStyle = JointStyle.Round });
+        SolidPen pen = new(new PenOptions(color, thickness) { JointStyle = JointStyle.Round });
 
         DrawLinesImpl(provider, colorName, alpha, thickness, antialias, pen);
     }
@@ -118,7 +143,7 @@ public void DrawLines_JointStyleSquare<TPixel>(TestImageProvider<TPixel> provide
 where TPixel : unmanaged, IPixel<TPixel>
     {
         Color color = TestUtils.GetColorByName(colorName).WithAlpha(alpha);
-        var pen = new SolidPen(new PenOptions(color, thickness) { JointStyle = JointStyle.Square });
+        SolidPen pen = new(new PenOptions(color, thickness) { JointStyle = JointStyle.Square });
 
         DrawLinesImpl(provider, colorName, alpha, thickness, antialias, pen);
     }
@@ -129,7 +154,7 @@ public void DrawLines_JointStyleMiter<TPixel>(TestImageProvider<TPixel> provider
 where TPixel : unmanaged, IPixel<TPixel>
     {
         Color color = TestUtils.GetColorByName(colorName).WithAlpha(alpha);
-        var pen = new SolidPen(new PenOptions(color, thickness) { JointStyle = JointStyle.Miter });
+        SolidPen pen = new(new PenOptions(color, thickness) { JointStyle = JointStyle.Miter });
 
         DrawLinesImpl(provider, colorName, alpha, thickness, antialias, pen);
     }
@@ -145,7 +170,8 @@ private static void DrawLinesImpl<TPixel>(
     {
         PointF[] simplePath = { new Vector2(10, 10), new Vector2(200, 150), new Vector2(50, 300) };
 
-        var options = new GraphicsOptions { Antialias = antialias };
+        GraphicsOptions options = new()
+        { Antialias = antialias };
 
         string aa = antialias ? string.Empty : "_NoAntialias";
         FormattableString outputDetails = $"{colorName}_A({alpha})_T({thickness}){aa}";

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

@@ -26,20 +26,20 @@ public class DrawPathTests
     public void DrawPath<TPixel>(TestImageProvider<TPixel> provider, string colorName, byte alpha, float thickness)
         where TPixel : unmanaged, IPixel<TPixel>
     {
-        var linearSegment = new LinearLineSegment(
+        LinearLineSegment linearSegment = new(
             new Vector2(10, 10),
             new Vector2(200, 150),
             new Vector2(50, 300));
-        var bezierSegment = new CubicBezierLineSegment(
+        CubicBezierLineSegment bezierSegment = new(
             new Vector2(50, 300),
             new Vector2(500, 500),
             new Vector2(60, 10),
             new Vector2(10, 400));
 
-        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);
-        var ellipticArcSegment2 = new ArcLineSegment(new(150, 450), new(149F, 450), new SizeF(140, 70), 0, true, true);
+        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);
+        ArcLineSegment ellipticArcSegment2 = new(new(150, 450), new(149F, 450), new SizeF(140, 70), 0, true, true);
 
-        var path = new Path(linearSegment, bezierSegment, ellipticArcSegment1, ellipticArcSegment2);
+        Path path = new(linearSegment, bezierSegment, ellipticArcSegment1, ellipticArcSegment2);
 
         Rgba32 rgba = TestUtils.GetColorByName(colorName);
         rgba.A = alpha;
@@ -67,7 +67,7 @@ public void PathExtendingOffEdgeOfImageShouldNotBeCropped<TPixel>(TestImageProvi
                 {
                     for (int i = 0; i < 300; i += 20)
                     {
-                        var points = new PointF[] { new Vector2(100, 2), new Vector2(-10, i) };
+                        PointF[] points = new PointF[] { new Vector2(100, 2), new Vector2(-10, i) };
                         x.DrawLine(pen, points);
                     }
                 },
@@ -91,7 +91,38 @@ public void DrawPathClippedOnTop<TPixel>(TestImageProvider<TPixel> provider)
 
         provider.VerifyOperation(
             image => image.Mutate(x => x.Draw(Color.Black, 1, path)),
-            appendSourceFileOrDescription: false,
-            appendPixelTypeToFileName: false);
+            appendPixelTypeToFileName: false,
+            appendSourceFileOrDescription: false);
+    }
+
+    [Theory]
+    [WithSolidFilledImages(300, 300, "White", PixelTypes.Rgba32, 360)]
+    [WithSolidFilledImages(300, 300, "White", PixelTypes.Rgba32, 359)]
+    public void DrawCircleUsingAddArc<TPixel>(TestImageProvider<TPixel> provider, float sweep)
+    where TPixel : unmanaged, IPixel<TPixel>
+    {
+        IPath path = new PathBuilder().AddArc(new Point(150, 150), 50, 50, 0, 40, sweep).Build();
+
+        provider.VerifyOperation(
+            image => image.Mutate(x => x.Draw(Color.Black, 1, path)),
+            testOutputDetails: $"{sweep}",
+            appendPixelTypeToFileName: false,
+            appendSourceFileOrDescription: false);
+    }
+
+    [Theory]
+    [WithSolidFilledImages(300, 300, "White", PixelTypes.Rgba32, true)]
+    [WithSolidFilledImages(300, 300, "White", PixelTypes.Rgba32, false)]
+    public void DrawCircleUsingArcTo<TPixel>(TestImageProvider<TPixel> provider, bool sweep)
+        where TPixel : unmanaged, IPixel<TPixel>
+    {
+        Point origin = new(150, 150);
+        IPath path = new PathBuilder().MoveTo(origin).ArcTo(50, 50, 0, true, sweep, origin).Build();
+
+        provider.VerifyOperation(
+            image => image.Mutate(x => x.Draw(Color.Black, 1, path)),
+            testOutputDetails: $"{sweep}",
+            appendPixelTypeToFileName: false,
+            appendSourceFileOrDescription: false);
     }
 }

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

@@ -130,13 +130,4 @@ public void ClippingRectanglesCreateCorrectNumberOfPoints()
 
         Assert.Equal(8, points.Count);
     }
-
-    [Fact]
-    public void ClipperOffsetThrowsPublicException()
-    {
-        PointF naan = new(float.NaN, float.NaN);
-        Polygon path = new(new LinearLineSegment(new[] { naan, naan, naan, naan }));
-
-        Assert.Throws<ClipperException>(() => path.GenerateOutline(10));
-    }
 }