From 99ed4586adb589449a5487f7136cda7ca93f2cc3 Mon Sep 17 00:00:00 2001 From: Unknown Date: Fri, 20 Apr 2018 00:44:53 +0200 Subject: [PATCH] #542: use struct for ColorStops as proposed by @antonfirsov: improving readability and memory locality. --- .../Drawing/Brushes/LinearGradientBrush.cs | 59 ++++++++++++++----- .../Drawing/FillLinearGradientBrushTests.cs | 4 +- 2 files changed, 47 insertions(+), 16 deletions(-) diff --git a/src/ImageSharp.Drawing/Processing/Drawing/Brushes/LinearGradientBrush.cs b/src/ImageSharp.Drawing/Processing/Drawing/Brushes/LinearGradientBrush.cs index bfbeded69..99edb39f4 100644 --- a/src/ImageSharp.Drawing/Processing/Drawing/Brushes/LinearGradientBrush.cs +++ b/src/ImageSharp.Drawing/Processing/Drawing/Brushes/LinearGradientBrush.cs @@ -19,28 +19,59 @@ namespace SixLabors.ImageSharp.Processing.Drawing.Brushes public class LinearGradientBrush : IBrush where TPixel : struct, IPixel { + /// + /// A struct that defines a single color stop. + /// + public struct ColorStop + { + /// + /// Create a new ColorStop + /// + /// Where should it be? 0 is at the start, 1 at the end of the . + /// What color should be used at that point? + public ColorStop(float ratio, TPixel color) + { + this.Ratio = ratio; + this.Color = color; + } + + /// + /// The point along the defined gradient axis. + /// + public float Ratio { get; } + + /// + /// The color to be used. + /// + public TPixel Color { get; } + } + private readonly Point p1; private readonly Point p2; - private readonly Tuple[] keyColors; + private readonly ColorStop[] colorStops; /// /// Initializes a new instance of the class. /// /// Start point /// End point - /// a set of color keys and where they are. The double must be in range [0..1] and is relative between p1 and p2. - public LinearGradientBrush(Point p1, Point p2, params Tuple[] keyColors) + /// + /// A set of color keys and where they are. + /// The double should be in range [0..1] and is relative between p1 and p2. + /// TODO: what about the [0..1] restriction? is it necessary? If so, it should be checked, if not, it should be explained what happens for greater/smaller values. + /// + public LinearGradientBrush(Point p1, Point p2, params ColorStop[] colorStops) { this.p1 = p1; this.p2 = p2; - this.keyColors = keyColors; + this.colorStops = colorStops; } /// public BrushApplicator CreateApplicator(ImageFrame source, RectangleF region, GraphicsOptions options) - => new LinearGradientBrushApplicator(source, this.p1, this.p2, this.keyColors, region, options); + => new LinearGradientBrushApplicator(source, this.p1, this.p2, this.colorStops, region, options); /// /// The linear gradient brush applicator. @@ -51,7 +82,7 @@ namespace SixLabors.ImageSharp.Processing.Drawing.Brushes private readonly Point end; - private readonly Tuple[] colorStops; + private readonly ColorStop[] colorStops; /// /// the vector along the gradient, x component @@ -101,7 +132,7 @@ namespace SixLabors.ImageSharp.Processing.Drawing.Brushes ImageFrame source, Point start, Point end, - Tuple[] colorStops, + ColorStop[] colorStops, RectangleF region, // TODO: use region, compare with other Brushes for reference. GraphicsOptions options) : base(source, options) @@ -138,13 +169,13 @@ namespace SixLabors.ImageSharp.Processing.Drawing.Brushes float onCompleteGradient = this.RatioOnGradient(x, y); var localGradientFrom = this.colorStops[0]; - Tuple localGradientTo = null; + ColorStop localGradientTo = default; // TODO: ensure colorStops has at least 2 items (technically 1 would be okay, but that's no gradient) foreach (var colorStop in this.colorStops) { localGradientTo = colorStop; - if (colorStop.Item1 >= onCompleteGradient) + if (colorStop.Ratio >= onCompleteGradient) { // we're done here, so break it! break; @@ -154,15 +185,15 @@ namespace SixLabors.ImageSharp.Processing.Drawing.Brushes } TPixel resultColor = default; - if (localGradientFrom.Item2.Equals(localGradientTo.Item2)) + if (localGradientFrom.Color.Equals(localGradientTo.Color)) { - resultColor = localGradientFrom.Item2; + resultColor = localGradientFrom.Color; } else { - var fromAsVector = localGradientFrom.Item2.ToVector4(); - var toAsVector = localGradientTo.Item2.ToVector4(); - float onLocalGradient = (onCompleteGradient - localGradientFrom.Item1) / localGradientTo.Item1; // TODO: + var fromAsVector = localGradientFrom.Color.ToVector4(); + var toAsVector = localGradientTo.Color.ToVector4(); + float onLocalGradient = (onCompleteGradient - localGradientFrom.Ratio) / localGradientTo.Ratio; // TODO: Vector4 result = PorterDuffFunctions.Normal( fromAsVector, diff --git a/tests/ImageSharp.Tests/Drawing/FillLinearGradientBrushTests.cs b/tests/ImageSharp.Tests/Drawing/FillLinearGradientBrushTests.cs index 047ffc9b7..88ebc49c3 100644 --- a/tests/ImageSharp.Tests/Drawing/FillLinearGradientBrushTests.cs +++ b/tests/ImageSharp.Tests/Drawing/FillLinearGradientBrushTests.cs @@ -24,8 +24,8 @@ namespace SixLabors.ImageSharp.Tests.Drawing new LinearGradientBrush( new SixLabors.Primitives.Point(0, 0), new SixLabors.Primitives.Point(500, 0), - new Tuple(0, Rgba32.Red), - new Tuple(1, Rgba32.Red)); + new LinearGradientBrush.ColorStop(0, Rgba32.Red), + new LinearGradientBrush.ColorStop(1, Rgba32.Red)); image.Mutate(x => x.Fill(unicolorLinearGradientBrush)); image.Save($"{path}/UnicolorGradient.png");