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.

Sign up for GitHub

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

Prevent PathGradientBrush from throwing an error with corner cases #1007

Merged
merged 3 commits into from
Sep 12, 2019
Merged

Prevent PathGradientBrush from throwing an error with corner cases #1007

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1314d00
Remove a redundant check for the paint boundary
mysticfall Sep 12, 2019
6842408
Ignore invalid gradient positions rather than throwing an error
mysticfall Sep 12, 2019
1eea497
Accept points instead of line segments as constructor argument
mysticfall Sep 12, 2019
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
53 changes: 27 additions & 26 deletions src/ImageSharp.Drawing/Processing/PathGradientBrush.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,27 @@ namespace SixLabors.ImageSharp.Processing
/// </summary>
public sealed class PathGradientBrush : IBrush
{
private readonly Polygon path;

private readonly IList<Edge> edges;

private readonly Color centerColor;

/// <summary>
/// Initializes a new instance of the <see cref="PathGradientBrush"/> class.
/// </summary>
/// <param name="lines">Line segments of a polygon that represents the gradient area.</param>
/// <param name="points">Points that constitute a polygon that represents the gradient area.</param>
/// <param name="colors">Array of colors that correspond to each point in the polygon.</param>
/// <param name="centerColor">Color at the center of the gradient area to which the other colors converge.</param>
public PathGradientBrush(ILineSegment[] lines, Color[] colors, Color centerColor)
public PathGradientBrush(PointF[] points, Color[] colors, Color centerColor)
{
if (lines == null)
if (points == null)
{
throw new ArgumentNullException(nameof(lines));
throw new ArgumentNullException(nameof(points));
}

if (lines.Length < 3)
if (points.Length < 3)
{
throw new ArgumentOutOfRangeException(
nameof(lines),
nameof(points),
"There must be at least 3 lines to construct a path gradient brush.");
}

Expand All @@ -56,22 +54,30 @@ public PathGradientBrush(ILineSegment[] lines, Color[] colors, Color centerColor
"One or more color is needed to construct a path gradient brush.");
}

this.path = new Polygon(lines);
int size = points.Length;

var lines = new ILineSegment[size];

for (int i = 0; i < size; i++)
{
lines[i] = new LinearLineSegment(points[i % size], points[(i + 1) % size]);
}

this.centerColor = centerColor;

Color ColorAt(int index) => colors[index % colors.Length];

this.edges = this.path.LineSegments.Select(s => new Path(s))
this.edges = lines.Select(s => new Path(s))
.Select((path, i) => new Edge(path, ColorAt(i), ColorAt(i + 1))).ToList();
}

/// <summary>
/// Initializes a new instance of the <see cref="PathGradientBrush"/> class.
/// </summary>
/// <param name="lines">Line segments of a polygon that represents the gradient area.</param>
/// <param name="points">Points that constitute a polygon that represents the gradient area.</param>
/// <param name="colors">Array of colors that correspond to each point in the polygon.</param>
public PathGradientBrush(ILineSegment[] lines, Color[] colors)
: this(lines, colors, CalculateCenterColor(colors))
public PathGradientBrush(PointF[] points, Color[] colors)
: this(points, colors, CalculateCenterColor(colors))
{
}

Expand All @@ -82,7 +88,7 @@ public BrushApplicator<TPixel> CreateApplicator<TPixel>(
GraphicsOptions options)
where TPixel : struct, IPixel<TPixel>
{
return new PathGradientBrushApplicator<TPixel>(source, this.path, this.edges, this.centerColor, options);
return new PathGradientBrushApplicator<TPixel>(source, this.edges, this.centerColor, options);
}

private static Color CalculateCenterColor(Color[] colors)
Expand Down Expand Up @@ -182,8 +188,6 @@ public Vector4 ColorAt(float distance)
private class PathGradientBrushApplicator<TPixel> : BrushApplicator<TPixel>
where TPixel : struct, IPixel<TPixel>
{
private readonly Path path;

private readonly PointF center;

private readonly Vector4 centerColor;
Expand All @@ -196,24 +200,21 @@ private class PathGradientBrushApplicator<TPixel> : BrushApplicator<TPixel>
/// Initializes a new instance of the <see cref="PathGradientBrushApplicator{TPixel}"/> class.
/// </summary>
/// <param name="source">The source image.</param>
/// <param name="path">A polygon that represents the gradient area.</param>
/// <param name="edges">Edges of the polygon.</param>
/// <param name="centerColor">Color at the center of the gradient area to which the other colors converge.</param>
/// <param name="options">The options.</param>
public PathGradientBrushApplicator(
ImageFrame<TPixel> source,
Path path,
IList<Edge> edges,
Color centerColor,
GraphicsOptions options)
: base(source, options)
{
this.path = path;
this.edges = edges;

PointF[] points = path.LineSegments.Select(s => s.EndPoint).ToArray();
PointF[] points = edges.Select(s => s.Start).ToArray();

this.center = points.Aggregate((p1, p2) => p1 + p2) / points.Length;
this.center = points.Aggregate((p1, p2) => p1 + p2) / edges.Count;
this.centerColor = centerColor.ToVector4();

this.maxDistance = points.Select(p => (Vector2)(p - this.center)).Select(d => d.Length()).Max();
Expand All @@ -231,17 +232,17 @@ public PathGradientBrushApplicator(
return new Color(this.centerColor).ToPixel<TPixel>();
}

if (!this.path.Contains(point))
{
return Color.Transparent.ToPixel<TPixel>();
}

Vector2 direction = Vector2.Normalize(point - this.center);

PointF end = point + (PointF)(direction * this.maxDistance);

(Edge edge, Intersection? info) = this.FindIntersection(point, end);

if (!info.HasValue)
{
return Color.Transparent.ToPixel<TPixel>();
}

PointF intersection = info.Value.Point;

Vector4 edgeColor = edge.ColorAt(intersection);
Expand Down
86 changes: 17 additions & 69 deletions tests/ImageSharp.Tests/Drawing/FillPathGradientBrushTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using SixLabors.ImageSharp.Processing;
using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison;
using SixLabors.Primitives;
using SixLabors.Shapes;

using Xunit;

Expand All @@ -27,17 +26,10 @@ public void FillRectangleWithDifferentColors<TPixel>(TestImageProvider<TPixel> p
TolerantComparer,
image =>
{
ILineSegment[] path =
{
new LinearLineSegment(new PointF(0, 0), new PointF(10, 0)),
new LinearLineSegment(new PointF(10, 0), new PointF(10, 10)),
new LinearLineSegment(new PointF(10, 10), new PointF(0, 10)),
new LinearLineSegment(new PointF(0, 10), new PointF(0, 0))
};

PointF[] points = { new PointF(0, 0), new PointF(10, 0), new PointF(10, 10), new PointF(0, 10) };
Color[] colors = { Color.Black, Color.Red, Color.Yellow, Color.Green };

var brush = new PathGradientBrush(path, colors);
var brush = new PathGradientBrush(points, colors);

image.Mutate(x => x.Fill(brush));
image.DebugSave(provider, appendPixelTypeToFileName: false, appendSourceFileOrDescription: false);
Expand All @@ -53,16 +45,10 @@ public void FillTriangleWithDifferentColors<TPixel>(TestImageProvider<TPixel> pr
TolerantComparer,
image =>
{
ILineSegment[] path =
{
new LinearLineSegment(new PointF(5, 0), new PointF(10, 10)),
new LinearLineSegment(new PointF(10, 10), new PointF(0, 10)),
new LinearLineSegment(new PointF(0, 10), new PointF(5, 0))
};

PointF[] points = { new PointF(5, 0), new PointF(10, 10), new PointF(0, 10) };
Color[] colors = { Color.Red, Color.Green, Color.Blue };

var brush = new PathGradientBrush(path, colors);
var brush = new PathGradientBrush(points, colors);

image.Mutate(x => x.Fill(brush));
image.DebugSave(provider, appendPixelTypeToFileName: false, appendSourceFileOrDescription: false);
Expand All @@ -76,17 +62,10 @@ public void FillRectangleWithSingleColor<TPixel>(TestImageProvider<TPixel> provi
{
using (Image<TPixel> image = provider.GetImage())
{
ILineSegment[] path =
{
new LinearLineSegment(new PointF(0, 0), new PointF(10, 0)),
new LinearLineSegment(new PointF(10, 0), new PointF(10, 10)),
new LinearLineSegment(new PointF(10, 10), new PointF(0, 10)),
new LinearLineSegment(new PointF(0, 10), new PointF(0, 0))
};

PointF[] points = { new PointF(0, 0), new PointF(10, 0), new PointF(10, 10), new PointF(0, 10) };
Color[] colors = { Color.Red };

var brush = new PathGradientBrush(path, colors);
var brush = new PathGradientBrush(points, colors);

image.Mutate(x => x.Fill(brush));

Expand All @@ -103,17 +82,10 @@ public void ShouldRotateTheColorsWhenThereAreMorePoints<TPixel>(TestImageProvide
TolerantComparer,
image =>
{
ILineSegment[] path =
{
new LinearLineSegment(new PointF(0, 0), new PointF(10, 0)),
new LinearLineSegment(new PointF(10, 0), new PointF(10, 10)),
new LinearLineSegment(new PointF(10, 10), new PointF(0, 10)),
new LinearLineSegment(new PointF(0, 10), new PointF(0, 0))
};

PointF[] points = { new PointF(0, 0), new PointF(10, 0), new PointF(10, 10), new PointF(0, 10) };
Color[] colors = { Color.Red, Color.Yellow };

var brush = new PathGradientBrush(path, colors);
var brush = new PathGradientBrush(points, colors);

image.Mutate(x => x.Fill(brush));
image.DebugSave(provider, appendPixelTypeToFileName: false, appendSourceFileOrDescription: false);
Expand All @@ -129,17 +101,10 @@ public void FillWithCustomCenterColor<TPixel>(TestImageProvider<TPixel> provider
TolerantComparer,
image =>
{
ILineSegment[] path =
{
new LinearLineSegment(new PointF(0, 0), new PointF(10, 0)),
new LinearLineSegment(new PointF(10, 0), new PointF(10, 10)),
new LinearLineSegment(new PointF(10, 10), new PointF(0, 10)),
new LinearLineSegment(new PointF(0, 10), new PointF(0, 0))
};

PointF[] points = { new PointF(0, 0), new PointF(10, 0), new PointF(10, 10), new PointF(0, 10) };
Color[] colors = { Color.Black, Color.Red, Color.Yellow, Color.Green };

var brush = new PathGradientBrush(path, colors, Color.White);
var brush = new PathGradientBrush(points, colors, Color.White);

image.Mutate(x => x.Fill(brush));
image.DebugSave(provider, appendPixelTypeToFileName: false, appendSourceFileOrDescription: false);
Expand All @@ -157,51 +122,34 @@ public void ShouldThrowArgumentNullExceptionWhenLinesAreNull()
}

[Fact]
public void ShouldThrowArgumentOutOfRangeExceptionWhenLessThan3LinesAreGiven()
public void ShouldThrowArgumentOutOfRangeExceptionWhenLessThan3PointsAreGiven()
{
ILineSegment[] path =
{
new LinearLineSegment(new PointF(0, 0), new PointF(10, 0)),
new LinearLineSegment(new PointF(10, 0), new PointF(10, 10))
};

PointF[] points = { new PointF(0, 0), new PointF(10, 0) };
Color[] colors = { Color.Black, Color.Red, Color.Yellow, Color.Green };

PathGradientBrush Create() => new PathGradientBrush(path, colors, Color.White);
PathGradientBrush Create() => new PathGradientBrush(points, colors, Color.White);

Assert.Throws<ArgumentOutOfRangeException>(Create);
}

[Fact]
public void ShouldThrowArgumentNullExceptionWhenColorsAreNull()
{
ILineSegment[] path =
{
new LinearLineSegment(new PointF(0, 0), new PointF(10, 0)),
new LinearLineSegment(new PointF(10, 0), new PointF(10, 10)),
new LinearLineSegment(new PointF(10, 10), new PointF(0, 10)),
new LinearLineSegment(new PointF(0, 10), new PointF(0, 0))
};
PointF[] points = { new PointF(0, 0), new PointF(10, 0), new PointF(10, 10), new PointF(0, 10) };

PathGradientBrush Create() => new PathGradientBrush(path, null, Color.White);
PathGradientBrush Create() => new PathGradientBrush(points, null, Color.White);

Assert.Throws<ArgumentNullException>(Create);
}

[Fact]
public void ShouldThrowArgumentOutOfRangeExceptionWhenEmptyColorArrayIsGiven()
{
ILineSegment[] path =
{
new LinearLineSegment(new PointF(0, 0), new PointF(10, 0)),
new LinearLineSegment(new PointF(10, 0), new PointF(10, 10)),
new LinearLineSegment(new PointF(10, 10), new PointF(0, 10)),
new LinearLineSegment(new PointF(0, 10), new PointF(0, 0))
};
PointF[] points = { new PointF(0, 0), new PointF(10, 0), new PointF(10, 10), new PointF(0, 10) };

var colors = new Color[0];

PathGradientBrush Create() => new PathGradientBrush(path, colors, Color.White);
PathGradientBrush Create() => new PathGradientBrush(points, colors, Color.White);

Assert.Throws<ArgumentOutOfRangeException>(Create);
}
Expand Down