Merge pull request #263 from SixLabors/js/fix234

Fix rendering along open paths.

Author: JimBobSquarePants

src/ImageSharp.Drawing/Processing/Extensions/DrawLineExtensions.cs

@@ -19,7 +19,7 @@ public static class DrawLineExtensions
         /// <param name="thickness">The line thickness.</param>
         /// <param name="points">The points.</param>
         /// <returns>The <see cref="IImageProcessingContext"/> to allow chaining of operations.</returns>
-        public static IImageProcessingContext DrawLines(
+        public static IImageProcessingContext DrawLine(
             this IImageProcessingContext source,
             DrawingOptions options,
             Brush brush,
@@ -35,7 +35,7 @@ public static IImageProcessingContext DrawLines(
         /// <param name="thickness">The line thickness.</param>
         /// <param name="points">The points.</param>
         /// <returns>The <see cref="IImageProcessingContext"/> to allow chaining of operations.</returns>
-        public static IImageProcessingContext DrawLines(
+        public static IImageProcessingContext DrawLine(
             this IImageProcessingContext source,
             Brush brush,
             float thickness,
@@ -50,12 +50,12 @@ public static IImageProcessingContext DrawLines(
         /// <param name="thickness">The line thickness.</param>
         /// <param name="points">The points.</param>
         /// <returns>The <see cref="IImageProcessingContext"/> to allow chaining of operations.</returns>
-        public static IImageProcessingContext DrawLines(
+        public static IImageProcessingContext DrawLine(
             this IImageProcessingContext source,
             Color color,
             float thickness,
             params PointF[] points) =>
-            source.DrawLines(new SolidBrush(color), thickness, points);
+            source.DrawLine(new SolidBrush(color), thickness, points);
 
         /// <summary>
         /// Draws the provided points as an open linear path at the provided thickness with the supplied brush.
@@ -66,13 +66,13 @@ public static IImageProcessingContext DrawLines(
         /// <param name="thickness">The line thickness.</param>
         /// <param name="points">The points.</param>
         /// <returns>The <see cref="IImageProcessingContext"/> to allow chaining of operations.</returns>>
-        public static IImageProcessingContext DrawLines(
+        public static IImageProcessingContext DrawLine(
             this IImageProcessingContext source,
             DrawingOptions options,
             Color color,
             float thickness,
             params PointF[] points) =>
-            source.DrawLines(options, new SolidBrush(color), thickness, points);
+            source.DrawLine(options, new SolidBrush(color), thickness, points);
 
         /// <summary>
         /// Draws the provided points as an open linear path with the supplied pen.
@@ -82,7 +82,7 @@ public static IImageProcessingContext DrawLines(
         /// <param name="pen">The pen.</param>
         /// <param name="points">The points.</param>
         /// <returns>The <see cref="IImageProcessingContext"/> to allow chaining of operations.</returns>
-        public static IImageProcessingContext DrawLines(
+        public static IImageProcessingContext DrawLine(
             this IImageProcessingContext source,
             DrawingOptions options,
             Pen pen,
@@ -96,7 +96,7 @@ public static IImageProcessingContext DrawLines(
         /// <param name="pen">The pen.</param>
         /// <param name="points">The points.</param>
         /// <returns>The <see cref="IImageProcessingContext"/> to allow chaining of operations.</returns>
-        public static IImageProcessingContext DrawLines(
+        public static IImageProcessingContext DrawLine(
             this IImageProcessingContext source,
             Pen pen,
             params PointF[] points) =>

src/ImageSharp.Drawing/Processing/Processors/Text/RichTextGlyphRenderer.cs

@@ -30,7 +30,6 @@ internal sealed class RichTextGlyphRenderer : BaseGlyphBuilder, IColorGlyphRende
         private readonly Pen defaultPen;
         private readonly Brush defaultBrush;
         private readonly IPathInternals path;
-        private Vector2 textPathOffset;
         private bool isDisposed;
 
         private readonly Dictionary<Color, Brush> brushLookup = new();
@@ -96,23 +95,6 @@ protected override void BeginText(in FontRectangle bounds)
             }
 
             this.DrawingOperations.Clear();
-
-            float yOffset = this.textOptions.VerticalAlignment switch
-            {
-                VerticalAlignment.Center => bounds.Bottom - (bounds.Height * .5F),
-                VerticalAlignment.Bottom => bounds.Bottom,
-                VerticalAlignment.Top => bounds.Top,
-                _ => bounds.Top,
-            };
-
-            float xOffset = this.textOptions.HorizontalAlignment switch
-            {
-                HorizontalAlignment.Center => bounds.Right - (bounds.Width * .5F),
-                HorizontalAlignment.Right => bounds.Right,
-                HorizontalAlignment.Left => bounds.Left,
-                _ => bounds.Left,
-            };
-            this.textPathOffset = new(xOffset, yOffset);
         }
 
         /// <inheritdoc/>
@@ -490,23 +472,14 @@ private Matrix3x2 ComputeTransform(in FontRectangle bounds)
                 return Matrix3x2.Identity;
             }
 
-            // Find the intersection point.
-            // This should be offset to ensure we rotate at the bottom-center of the glyph.
-            float halfWidth = bounds.Width * .5F;
-
             // Find the point of this intersection along the given path.
-            SegmentInfo pathPoint = this.path.PointAlongPath(bounds.Left + halfWidth);
-
-            // Now offset our target point since we're aligning the bottom-left location of our glyph against the path.
-            // This is good and accurate when we are vertically aligned to the path however the distance between
-            // characters in multiline text scales with the angle and vertical offset.
-            // This is expected and consistant with other libraries.
-            // Multiple line text should be rendered using multiple paths to avoid this behavior.
-            Vector2 targetPoint = (Vector2)pathPoint.Point + new Vector2(-halfWidth, bounds.Top) - bounds.Location - this.textPathOffset;
-
-            // Due to how matrix combining works you have to combine this in the reverse order of operation.
-            return Matrix3x2.CreateTranslation(targetPoint)
-                * Matrix3x2.CreateRotation(pathPoint.Angle - MathF.PI, pathPoint.Point);
+            // We want to find the point on the path that is closest to the center-bottom side of the glyph.
+            Vector2 half = new(bounds.Width * .5F, 0);
+            SegmentInfo pathPoint = this.path.PointAlongPath(bounds.Left + half.X);
+
+            // Now offset to our target point since we're aligning the top-left location of our glyph against the path.
+            Vector2 translation = (Vector2)pathPoint.Point - bounds.Location - half + new Vector2(0, bounds.Top);
+            return Matrix3x2.CreateTranslation(translation) * Matrix3x2.CreateRotation(pathPoint.Angle - MathF.PI, (Vector2)pathPoint.Point);
         }
 
         private Buffer2D<float> Render(IPath path)

src/ImageSharp.Drawing/Shapes/ComplexPolygon.cs

@@ -174,12 +174,14 @@ SegmentInfo IPathInternals.PointAlongPath(float distance)
                 distance -= p.Length;
             }
 
-            // TODO: Perf. Throwhelper
-            throw new InvalidOperationException("Should not be possible to reach this line");
+            ThrowOutOfRange();
+            return default;
         }
 
         /// <inheritdoc/>
         IReadOnlyList<InternalPath> IInternalPathOwner.GetRingsAsInternalPath()
             => this.internalPaths;
+
+        private static void ThrowOutOfRange() => new InvalidOperationException("Should not be possible to reach this line");
     }
 }

src/ImageSharp.Drawing/Shapes/InternalPath.cs

@@ -141,10 +141,11 @@ private InternalPath(PointData[] points, bool isClosedPath)
         /// <exception cref="InvalidOperationException">Thrown if no points found.</exception>
         internal SegmentInfo PointAlongPath(float distanceAlongPath)
         {
-            distanceAlongPath %= this.Length;
             int pointCount = this.PointCount;
             if (this.closedPath)
             {
+                // Move the distance back to the beginning since this is a closed polygon.
+                distanceAlongPath %= this.Length;
                 pointCount--;
             }
 
@@ -154,8 +155,7 @@ internal SegmentInfo PointAlongPath(float distanceAlongPath)
                 if (distanceAlongPath < this.points[next].Length)
                 {
                     float t = distanceAlongPath / this.points[next].Length;
-                    Vector2 point = (this.points[i].Point * (1 - t)) + (this.points[next].Point * t);
-
+                    var point = Vector2.Lerp(this.points[i].Point, this.points[next].Point, t);
                     Vector2 diff = this.points[i].Point - this.points[next].Point;
 
                     return new SegmentInfo
@@ -168,8 +168,21 @@ internal SegmentInfo PointAlongPath(float distanceAlongPath)
                 distanceAlongPath -= this.points[next].Length;
             }
 
-            // TODO: Perf - Throwhelper.
-            throw new InvalidOperationException("Should always reach a point along the path.");
+            // Closed paths will never reach this point.
+            // For open paths we're going to create a new virtual point that extends past the path.
+            // The position and angle for that point are calculated based upon the last two points.
+            PointF a = this.points[Math.Max(this.points.Length - 2, 0)].Point;
+            PointF b = this.points[this.points.Length - 1].Point;
+            Vector2 delta = a - b;
+            float angle = (float)(Math.Atan2(delta.Y, delta.X) % (Math.PI * 2));
+
+            Matrix3x2 transform = Matrix3x2.CreateRotation(angle - MathF.PI) * Matrix3x2.CreateTranslation(b.X, b.Y);
+
+            return new SegmentInfo
+            {
+                Point = Vector2.Transform(new Vector2(distanceAlongPath, 0), transform),
+                Angle = angle
+            };
         }
 
         internal IMemoryOwner<PointF> ExtractVertices(MemoryAllocator allocator)
@@ -198,7 +211,7 @@ private static PointOrientation CalulateOrientation(Vector2 p, Vector2 q, Vector
             Vector2 rq = r - q;
             float val = (qp.Y * rq.X) - (qp.X * rq.Y);
 
-            if (val > -Epsilon && val < Epsilon)
+            if (val is > -Epsilon and < Epsilon)
             {
                 return PointOrientation.Collinear;  // colinear
             }
@@ -228,7 +241,7 @@ private static PointOrientation CalulateOrientation(Vector2 qp, Vector2 rq)
         /// <param name="isClosed">Weather the path is closed or open.</param>
         /// <param name="removeCloseAndCollinear">Whether to remove close and collinear vertices</param>
         /// <returns>
-        /// The <see cref="T:Vector2[]"/>.
+        /// The <see cref="T:PointData[]"/>.
         /// </returns>
         private static PointData[] Simplify(IReadOnlyList<ILineSegment> segments, bool isClosed, bool removeCloseAndCollinear)
         {

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

@@ -68,11 +68,11 @@ public IPath[] GenerateClippedShapes()
 
             for (int i = 0; i < openPaths.Count; i++)
             {
-                var points = new PointF[closedPaths[i].Count];
+                var points = new PointF[openPaths[i].Count];
 
-                for (int j = 0; j < closedPaths[i].Count; j++)
+                for (int j = 0; j < openPaths[i].Count; j++)
                 {
-                    Point64 p = closedPaths[i][j];
+                    Point64 p = openPaths[i][j];
 
                     // to make the floating point polygons compatable with clipper we had
                     // to scale them up to make them ints but still retain some level of precision

src/ImageSharp.Drawing/Shapes/Text/PathGlyphBuilder.cs

@@ -14,15 +14,12 @@ namespace SixLabors.ImageSharp.Drawing.Text
     internal sealed class PathGlyphBuilder : GlyphBuilder
     {
         private readonly IPathInternals path;
-        private Vector2 textOffset;
-        private readonly TextOptions textOptions;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="PathGlyphBuilder"/> class.
         /// </summary>
         /// <param name="path">The path to render the glyphs along.</param>
-        /// <param name="textOptions">The text rendering options.</param>
-        public PathGlyphBuilder(IPath path, TextOptions textOptions)
+        public PathGlyphBuilder(IPath path)
         {
             if (path is IPathInternals internals)
             {
@@ -32,29 +29,6 @@ public PathGlyphBuilder(IPath path, TextOptions textOptions)
             {
                 this.path = new ComplexPolygon(path);
             }
-
-            this.textOptions = textOptions;
-        }
-
-        /// <inheritdoc/>
-        protected override void BeginText(in FontRectangle bounds)
-        {
-            float yOffset = this.textOptions.VerticalAlignment switch
-            {
-                VerticalAlignment.Center => bounds.Bottom - (bounds.Height * .5F),
-                VerticalAlignment.Bottom => bounds.Bottom,
-                VerticalAlignment.Top => bounds.Top,
-                _ => bounds.Top,
-            };
-
-            float xOffset = this.textOptions.HorizontalAlignment switch
-            {
-                HorizontalAlignment.Center => bounds.Right - (bounds.Width * .5F),
-                HorizontalAlignment.Right => bounds.Right,
-                HorizontalAlignment.Left => bounds.Left,
-                _ => bounds.Left,
-            };
-            this.textOffset = new(xOffset, yOffset);
         }
 
         /// <inheritdoc/>
@@ -64,21 +38,15 @@ protected override void BeginGlyph(in FontRectangle bounds, in GlyphRendererPara
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private void TransformGlyph(in FontRectangle bounds)
         {
-            // Find the intersection point.
-            // This should be offset to ensure we rotate at the bottom-center of the glyph.
-            float halfWidth = bounds.Width * .5F;
-
             // Find the point of this intersection along the given path.
-            SegmentInfo pathPoint = this.path.PointAlongPath(bounds.Left + halfWidth);
+            // We want to find the point on the path that is closest to the center-bottom side of the glyph.
+            Vector2 half = new(bounds.Width * .5F, 0);
+            SegmentInfo pathPoint = this.path.PointAlongPath(bounds.Left + half.X);
 
-            // Now offset our target point since we're aligning the bottom-left location of our glyph against the path.
-            // This is good and accurate when we are vertically aligned to the path however the distance between
-            // characters in multiline text scales with the angle and vertical offset.
-            // This is expected and consistant with other libraries. Multiple line text should be rendered using multiple paths to avoid this behavior.
-            Vector2 targetPoint = (Vector2)pathPoint.Point + new Vector2(-halfWidth, bounds.Top) - bounds.Location - this.textOffset;
+            // Now offset to our target point since we're aligning the top-left location of our glyph against the path.
+            Vector2 translation = (Vector2)pathPoint.Point - bounds.Location - half + new Vector2(0, bounds.Top);
+            Matrix3x2 matrix = Matrix3x2.CreateTranslation(translation) * Matrix3x2.CreateRotation(pathPoint.Angle - MathF.PI, (Vector2)pathPoint.Point);
 
-            // Due to how matrix combining works you have to combine this in the reverse order of operation.
-            Matrix3x2 matrix = Matrix3x2.CreateTranslation(targetPoint) * Matrix3x2.CreateRotation(pathPoint.Angle - MathF.PI, pathPoint.Point);
             this.Builder.SetTransform(matrix);
         }
     }

src/ImageSharp.Drawing/Shapes/Text/TextBuilder.cs

@@ -38,7 +38,7 @@ public static IPathCollection GenerateGlyphs(string text, TextOptions textOption
         public static IPathCollection GenerateGlyphs(string text, IPath path, TextOptions textOptions)
         {
             (IPath Path, TextOptions TextOptions) transformed = ConfigureOptions(textOptions, path);
-            PathGlyphBuilder glyphBuilder = new(transformed.Path, transformed.TextOptions);
+            PathGlyphBuilder glyphBuilder = new(transformed.Path);
             TextRenderer renderer = new(glyphBuilder);
 
             renderer.RenderText(text, transformed.TextOptions);

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

@@ -153,7 +153,7 @@ private static void DrawLinesImpl<TPixel>(
             FormattableString outputDetails = $"{colorName}_A({alpha})_T({thickness}){aa}";
 
             provider.RunValidatingProcessorTest(
-                c => c.SetGraphicsOptions(options).DrawLines(pen, simplePath),
+                c => c.SetGraphicsOptions(options).DrawLine(pen, simplePath),
                 outputDetails,
                 appendSourceFileOrDescription: false);
         }

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

@@ -70,7 +70,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) };
-                            x.DrawLines(pen, points);
+                            x.DrawLine(pen, points);
                         }
                     },
                 appendPixelTypeToFileName: false,

tests/ImageSharp.Drawing.Tests/Drawing/DrawingRobustnessTests.cs

@@ -95,7 +95,7 @@ public void LargeGeoJson_Lines(TestImageProvider<Rgba32> provider, string geoJso
             };
             foreach (PointF[] loop in points)
             {
-                image.Mutate(c => c.DrawLines(options, Color.White, 1.0f, loop));
+                image.Mutate(c => c.DrawLine(options, Color.White, 1.0f, loop));
             }
 
             string details = $"_{System.IO.Path.GetFileName(geoJsonFile)}_{sx}x{sy}_aa{aa}";
@@ -162,7 +162,7 @@ public void LargeGeoJson_Mississippi_Lines(TestImageProvider<Rgba32> provider, i
 
             foreach (PointF[] loop in points)
             {
-                image.Mutate(c => c.DrawLines(Color.White, 1.0f, loop));
+                image.Mutate(c => c.DrawLine(Color.White, 1.0f, loop));
             }
 
             // Very strict tolerance, since the image is sparse (relaxed on .NET Framework)