Skip to content

Commit

Permalink
Merge pull request #263 from SixLabors/js/fix234
Browse files Browse the repository at this point in the history
Fix rendering along open paths.
  • Loading branch information
JimBobSquarePants authored Apr 4, 2023
2 parents cee21d9 + 41174d1 commit 1d451a2
Show file tree
Hide file tree
Showing 29 changed files with 146 additions and 134 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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,
Expand All @@ -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) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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/>
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions src/ImageSharp.Drawing/Shapes/ComplexPolygon.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
27 changes: 20 additions & 7 deletions src/ImageSharp.Drawing/Shapes/InternalPath.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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--;
}

Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
{
Expand Down
6 changes: 3 additions & 3 deletions src/ImageSharp.Drawing/Shapes/PolygonClipper/Clipper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
46 changes: 7 additions & 39 deletions src/ImageSharp.Drawing/Shapes/Text/PathGlyphBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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/>
Expand All @@ -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);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/ImageSharp.Drawing/Shapes/Text/TextBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion tests/ImageSharp.Drawing.Tests/Drawing/DrawLinesTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/ImageSharp.Drawing.Tests/Drawing/DrawPathTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}";
Expand Down Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 1d451a2

Please sign in to comment.