From c1ab6b1885fa48aea7d95bcee2145e5a86164ef2 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 15 Aug 2019 19:02:14 +1000 Subject: [PATCH 01/22] remove patternVector in patternBrush --- src/ImageSharp.Drawing/Processing/PatternBrush.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/ImageSharp.Drawing/Processing/PatternBrush.cs b/src/ImageSharp.Drawing/Processing/PatternBrush.cs index a7a6785b92..1999af8a39 100644 --- a/src/ImageSharp.Drawing/Processing/PatternBrush.cs +++ b/src/ImageSharp.Drawing/Processing/PatternBrush.cs @@ -99,7 +99,6 @@ namespace SixLabors.ImageSharp.Processing new PatternBrushApplicator( source, this.pattern.ToPixelMatrix(source.Configuration), - this.patternVector, options); /// @@ -112,20 +111,17 @@ namespace SixLabors.ImageSharp.Processing /// The pattern. /// private readonly DenseMatrix pattern; - private readonly DenseMatrix patternVector; /// /// Initializes a new instance of the class. /// /// The source image. /// The pattern. - /// The patternVector. /// The options - public PatternBrushApplicator(ImageFrame source, in DenseMatrix pattern, DenseMatrix patternVector, GraphicsOptions options) + public PatternBrushApplicator(ImageFrame source, in DenseMatrix pattern, GraphicsOptions options) : base(source, options) { this.pattern = pattern; - this.patternVector = patternVector; } /// From 6af3a38cfab49931aa57ed6c8d53c6aa509904aa Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 15 Aug 2019 19:06:04 +1000 Subject: [PATCH 02/22] ExifTagDescriptionAttribute doesnt need fields since the reflection is done on the constructor arguments --- .../MetaData/Profiles/Exif/ExifTagDescriptionAttribute.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/ImageSharp/MetaData/Profiles/Exif/ExifTagDescriptionAttribute.cs b/src/ImageSharp/MetaData/Profiles/Exif/ExifTagDescriptionAttribute.cs index 0188c662ed..845e4ee734 100644 --- a/src/ImageSharp/MetaData/Profiles/Exif/ExifTagDescriptionAttribute.cs +++ b/src/ImageSharp/MetaData/Profiles/Exif/ExifTagDescriptionAttribute.cs @@ -12,9 +12,6 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Exif [AttributeUsage(AttributeTargets.Field, AllowMultiple = true)] internal sealed class ExifTagDescriptionAttribute : Attribute { - private readonly object value; - private readonly string description; - /// /// Initializes a new instance of the class. /// @@ -22,8 +19,6 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Exif /// The description for the value of the exif tag. public ExifTagDescriptionAttribute(object value, string description) { - this.value = value; - this.description = description; } /// From ccd23cfadd14452a9c5c4e6f63d396dda1e9e0ed Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 15 Aug 2019 19:39:25 +1000 Subject: [PATCH 03/22] use SUPPORTS_EXTENDED_INTRINSICS to filter out some BasicIntrinsics256 methods --- src/ImageSharp/Common/Helpers/SimdUtils.BasicIntrinsics256.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ImageSharp/Common/Helpers/SimdUtils.BasicIntrinsics256.cs b/src/ImageSharp/Common/Helpers/SimdUtils.BasicIntrinsics256.cs index 5aa0b21ec1..bc07fbf317 100644 --- a/src/ImageSharp/Common/Helpers/SimdUtils.BasicIntrinsics256.cs +++ b/src/ImageSharp/Common/Helpers/SimdUtils.BasicIntrinsics256.cs @@ -19,6 +19,7 @@ namespace SixLabors.ImageSharp { public static bool IsAvailable { get; } = IsAvx2CompatibleArchitecture; +#if !SUPPORTS_EXTENDED_INTRINSICS /// /// as many elements as possible, slicing them down (keeping the remainder). /// @@ -74,6 +75,7 @@ namespace SixLabors.ImageSharp dest = dest.Slice(adjustedCount); } } +#endif /// /// SIMD optimized implementation for . From 34c5b5a16c4424c7362ac26a68bdec7267629f0f Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 7 Sep 2019 12:04:51 +1000 Subject: [PATCH 04/22] Fix #997 --- .../Implementation/Converters/HslAndRgbConverter.cs | 4 ++-- .../Colorspaces/Conversion/RgbAndHslConversionTest.cs | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/ColorSpaces/Conversion/Implementation/Converters/HslAndRgbConverter.cs b/src/ImageSharp/ColorSpaces/Conversion/Implementation/Converters/HslAndRgbConverter.cs index 761313b7e0..97465e526a 100644 --- a/src/ImageSharp/ColorSpaces/Conversion/Implementation/Converters/HslAndRgbConverter.cs +++ b/src/ImageSharp/ColorSpaces/Conversion/Implementation/Converters/HslAndRgbConverter.cs @@ -96,7 +96,7 @@ namespace SixLabors.ImageSharp.ColorSpaces.Conversion.Implementation } else { - s = chroma / (2F - chroma); + s = chroma / (2F - max - min); } return new Hsl(h, s, l); @@ -157,4 +157,4 @@ namespace SixLabors.ImageSharp.ColorSpaces.Conversion.Implementation return value; } } -} \ No newline at end of file +} diff --git a/tests/ImageSharp.Tests/Colorspaces/Conversion/RgbAndHslConversionTest.cs b/tests/ImageSharp.Tests/Colorspaces/Conversion/RgbAndHslConversionTest.cs index 502df84133..8b1fed84c2 100644 --- a/tests/ImageSharp.Tests/Colorspaces/Conversion/RgbAndHslConversionTest.cs +++ b/tests/ImageSharp.Tests/Colorspaces/Conversion/RgbAndHslConversionTest.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; @@ -65,6 +65,7 @@ namespace SixLabors.ImageSharp.Tests.Colorspaces.Conversion [InlineData(1, 0, 0, 0, 1, .5F)] [InlineData(0, 1, 0, 120, 1, .5F)] [InlineData(0, 0, 1, 240, 1, .5F)] + [InlineData(0.7, 0.8, 0.6, 90, 0.3333, 0.7F)] public void Convert_Rgb_To_Hsl(float r, float g, float b, float h, float s, float l) { // Arrange From 2a5dd6bc7644d21662549d9a910588b8537e6282 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 8 Sep 2019 13:49:47 +1000 Subject: [PATCH 05/22] Fix #999 and add tests --- .../Transforms/Resize/ResizeHelper.cs | 228 ++++++++---------- .../Transforms/Resize/ResizeProcessor.cs | 43 +--- .../Resize/ResizeProcessor{TPixel}.cs | 14 +- src/ImageSharp/Processing/ResizeOptions.cs | 8 +- .../Transforms/ResizeHelperTests.cs | 143 +++++++++-- .../Processing/Transforms/PadTest.cs | 4 +- .../Processing/Transforms/ResizeTests.cs | 20 +- tests/ImageSharp.Tests/xunit.runner.json | 9 +- 8 files changed, 268 insertions(+), 201 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeHelper.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeHelper.cs index ae7b112fc1..c9df1b2542 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeHelper.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeHelper.cs @@ -2,9 +2,7 @@ // Licensed under the Apache License, Version 2.0. using System; -using System.Linq; using System.Numerics; - using SixLabors.Primitives; namespace SixLabors.ImageSharp.Processing.Processors.Transforms @@ -30,17 +28,32 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// /// The source image size. /// The resize options. - /// The target width - /// The target height /// /// The tuple representing the location and the bounds /// - public static (Size, Rectangle) CalculateTargetLocationAndBounds( - Size sourceSize, - ResizeOptions options, - int width, - int height) + public static (Size, Rectangle) CalculateTargetLocationAndBounds(Size sourceSize, ResizeOptions options) { + int width = options.Size.Width; + int height = options.Size.Height; + + // Ensure target size is populated across both dimensions. + // These dimensions are used to calculate the final dimensions determined by the mode algorithm. + // If only one of the incoming dimensions is 0, it will be modified here to maintain aspect ratio. + // If it is not possible to keep aspect ratio, make sure at least the minimum is is kept. + const int Min = 1; + if (width == 0 && height > 0) + { + width = (int)MathF.Max(Min, MathF.Round(sourceSize.Width * height / (float)sourceSize.Height)); + } + + if (height == 0 && width > 0) + { + height = (int)MathF.Max(Min, MathF.Round(sourceSize.Height * width / (float)sourceSize.Width)); + } + + Guard.MustBeGreaterThan(width, 0, nameof(width)); + Guard.MustBeGreaterThan(height, 0, nameof(height)); + switch (options.Mode) { case ResizeMode.Crop: @@ -50,9 +63,9 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms case ResizeMode.BoxPad: return CalculateBoxPadRectangle(sourceSize, options, width, height); case ResizeMode.Max: - return CalculateMaxRectangle(sourceSize, options, width, height); + return CalculateMaxRectangle(sourceSize, width, height); case ResizeMode.Min: - return CalculateMinRectangle(sourceSize, options, width, height); + return CalculateMinRectangle(sourceSize, width, height); // Last case ResizeMode.Stretch: default: @@ -66,11 +79,6 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms int width, int height) { - if (width <= 0 || height <= 0) - { - return (new Size(source.Width, source.Height), new Rectangle(0, 0, source.Width, source.Height)); - } - int sourceWidth = source.Width; int sourceHeight = source.Height; @@ -84,55 +92,55 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms // Only calculate if upscaling. if (sourceWidth < boxPadWidth && sourceHeight < boxPadHeight) { - int destinationX; - int destinationY; - int destinationWidth = sourceWidth; - int destinationHeight = sourceHeight; + int targetX; + int targetY; + int targetWidth = sourceWidth; + int targetHeight = sourceHeight; width = boxPadWidth; height = boxPadHeight; switch (options.Position) { case AnchorPositionMode.Left: - destinationY = (height - sourceHeight) / 2; - destinationX = 0; + targetY = (height - sourceHeight) / 2; + targetX = 0; break; case AnchorPositionMode.Right: - destinationY = (height - sourceHeight) / 2; - destinationX = width - sourceWidth; + targetY = (height - sourceHeight) / 2; + targetX = width - sourceWidth; break; case AnchorPositionMode.TopRight: - destinationY = 0; - destinationX = width - sourceWidth; + targetY = 0; + targetX = width - sourceWidth; break; case AnchorPositionMode.Top: - destinationY = 0; - destinationX = (width - sourceWidth) / 2; + targetY = 0; + targetX = (width - sourceWidth) / 2; break; case AnchorPositionMode.TopLeft: - destinationY = 0; - destinationX = 0; + targetY = 0; + targetX = 0; break; case AnchorPositionMode.BottomRight: - destinationY = height - sourceHeight; - destinationX = width - sourceWidth; + targetY = height - sourceHeight; + targetX = width - sourceWidth; break; case AnchorPositionMode.Bottom: - destinationY = height - sourceHeight; - destinationX = (width - sourceWidth) / 2; + targetY = height - sourceHeight; + targetX = (width - sourceWidth) / 2; break; case AnchorPositionMode.BottomLeft: - destinationY = height - sourceHeight; - destinationX = 0; + targetY = height - sourceHeight; + targetX = 0; break; default: - destinationY = (height - sourceHeight) / 2; - destinationX = (width - sourceWidth) / 2; + targetY = (height - sourceHeight) / 2; + targetX = (width - sourceWidth) / 2; break; } - return (new Size(width, height), - new Rectangle(destinationX, destinationY, destinationWidth, destinationHeight)); + // Target image width and height can be different to the rectangle width and height. + return (new Size(width, height), new Rectangle(targetX, targetY, targetWidth, targetHeight)); } // Switch to pad mode to downscale and calculate from there. @@ -145,19 +153,14 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms int width, int height) { - if (width <= 0 || height <= 0) - { - return (new Size(source.Width, source.Height), new Rectangle(0, 0, source.Width, source.Height)); - } - float ratio; int sourceWidth = source.Width; int sourceHeight = source.Height; - int destinationX = 0; - int destinationY = 0; - int destinationWidth = width; - int destinationHeight = height; + int targetX = 0; + int targetY = 0; + int targetWidth = width; + int targetHeight = height; // Fractional variants for preserving aspect ratio. float percentHeight = MathF.Abs(height / (float)sourceHeight); @@ -167,19 +170,19 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms { ratio = percentWidth; - if (options.CenterCoordinates.Any()) + if (options.CenterCoordinates.HasValue) { - float center = -(ratio * sourceHeight) * options.CenterCoordinates.ToArray()[1]; - destinationY = (int)MathF.Round(center + (height / 2F)); + float center = -(ratio * sourceHeight) * options.CenterCoordinates.Value.Y; + targetY = (int)MathF.Round(center + (height / 2F)); - if (destinationY > 0) + if (targetY > 0) { - destinationY = 0; + targetY = 0; } - if (destinationY < (int)MathF.Round(height - (sourceHeight * ratio))) + if (targetY < (int)MathF.Round(height - (sourceHeight * ratio))) { - destinationY = (int)MathF.Round(height - (sourceHeight * ratio)); + targetY = (int)MathF.Round(height - (sourceHeight * ratio)); } } else @@ -189,38 +192,38 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms case AnchorPositionMode.Top: case AnchorPositionMode.TopLeft: case AnchorPositionMode.TopRight: - destinationY = 0; + targetY = 0; break; case AnchorPositionMode.Bottom: case AnchorPositionMode.BottomLeft: case AnchorPositionMode.BottomRight: - destinationY = (int)MathF.Round(height - (sourceHeight * ratio)); + targetY = (int)MathF.Round(height - (sourceHeight * ratio)); break; default: - destinationY = (int)MathF.Round((height - (sourceHeight * ratio)) / 2F); + targetY = (int)MathF.Round((height - (sourceHeight * ratio)) / 2F); break; } } - destinationHeight = (int)MathF.Ceiling(sourceHeight * percentWidth); + targetHeight = (int)MathF.Ceiling(sourceHeight * percentWidth); } else { ratio = percentHeight; - if (options.CenterCoordinates.Any()) + if (options.CenterCoordinates.HasValue) { - float center = -(ratio * sourceWidth) * options.CenterCoordinates.First(); - destinationX = (int)MathF.Round(center + (width / 2F)); + float center = -(ratio * sourceWidth) * options.CenterCoordinates.Value.X; + targetX = (int)MathF.Round(center + (width / 2F)); - if (destinationX > 0) + if (targetX > 0) { - destinationX = 0; + targetX = 0; } - if (destinationX < (int)MathF.Round(width - (sourceWidth * ratio))) + if (targetX < (int)MathF.Round(width - (sourceWidth * ratio))) { - destinationX = (int)MathF.Round(width - (sourceWidth * ratio)); + targetX = (int)MathF.Round(width - (sourceWidth * ratio)); } } else @@ -230,68 +233,64 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms case AnchorPositionMode.Left: case AnchorPositionMode.TopLeft: case AnchorPositionMode.BottomLeft: - destinationX = 0; + targetX = 0; break; case AnchorPositionMode.Right: case AnchorPositionMode.TopRight: case AnchorPositionMode.BottomRight: - destinationX = (int)MathF.Round(width - (sourceWidth * ratio)); + targetX = (int)MathF.Round(width - (sourceWidth * ratio)); break; default: - destinationX = (int)MathF.Round((width - (sourceWidth * ratio)) / 2F); + targetX = (int)MathF.Round((width - (sourceWidth * ratio)) / 2F); break; } } - destinationWidth = (int)MathF.Ceiling(sourceWidth * percentHeight); + targetWidth = (int)MathF.Ceiling(sourceWidth * percentHeight); } - return (new Size(width, height), - new Rectangle(destinationX, destinationY, destinationWidth, destinationHeight)); + // Target image width and height can be different to the rectangle width and height. + return (new Size(width, height), new Rectangle(targetX, targetY, targetWidth, targetHeight)); } private static (Size, Rectangle) CalculateMaxRectangle( Size source, - ResizeOptions options, int width, int height) { - int destinationWidth = width; - int destinationHeight = height; + int targetWidth = width; + int targetHeight = height; // Fractional variants for preserving aspect ratio. float percentHeight = MathF.Abs(height / (float)source.Height); float percentWidth = MathF.Abs(width / (float)source.Width); // Integers must be cast to floats to get needed precision - float ratio = options.Size.Height / (float)options.Size.Width; + float ratio = height / (float)width; float sourceRatio = source.Height / (float)source.Width; if (sourceRatio < ratio) { - destinationHeight = (int)MathF.Round(source.Height * percentWidth); - height = destinationHeight; + targetHeight = (int)MathF.Round(source.Height * percentWidth); } else { - destinationWidth = (int)MathF.Round(source.Width * percentHeight); - width = destinationWidth; + targetWidth = (int)MathF.Round(source.Width * percentHeight); } // Replace the size to match the rectangle. - return (new Size(width, height), new Rectangle(0, 0, destinationWidth, destinationHeight)); + return (new Size(targetWidth, targetHeight), new Rectangle(0, 0, targetWidth, targetHeight)); } private static (Size, Rectangle) CalculateMinRectangle( Size source, - ResizeOptions options, int width, int height) { int sourceWidth = source.Width; int sourceHeight = source.Height; - int destinationWidth; - int destinationHeight; + int targetWidth = width; + int targetHeight = height; // Don't upscale if (width > sourceWidth || height > sourceHeight) @@ -306,58 +305,45 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms if (widthDiff < heightDiff) { float sourceRatio = (float)sourceHeight / sourceWidth; - destinationHeight = (int)MathF.Round(width * sourceRatio); - height = destinationHeight; - destinationWidth = width; + targetHeight = (int)MathF.Round(width * sourceRatio); } else if (widthDiff > heightDiff) { float sourceRatioInverse = (float)sourceWidth / sourceHeight; - destinationWidth = (int)MathF.Round(height * sourceRatioInverse); - destinationHeight = height; - width = destinationWidth; + targetWidth = (int)MathF.Round(height * sourceRatioInverse); } else { if (height > width) { - destinationWidth = width; float percentWidth = MathF.Abs(width / (float)sourceWidth); - destinationHeight = (int)MathF.Round(sourceHeight * percentWidth); - height = destinationHeight; + targetHeight = (int)MathF.Round(sourceHeight * percentWidth); } else { - destinationHeight = height; float percentHeight = MathF.Abs(height / (float)sourceHeight); - destinationWidth = (int)MathF.Round(sourceWidth * percentHeight); - width = destinationWidth; + targetWidth = (int)MathF.Round(sourceWidth * percentHeight); } } // Replace the size to match the rectangle. - return (new Size(width, height), new Rectangle(0, 0, destinationWidth, destinationHeight)); + return (new Size(targetWidth, targetHeight), new Rectangle(0, 0, targetWidth, targetHeight)); } private static (Size, Rectangle) CalculatePadRectangle( - Size source, + Size sourceSize, ResizeOptions options, int width, int height) { - if (width <= 0 || height <= 0) - { - return (new Size(source.Width, source.Height), new Rectangle(0, 0, source.Width, source.Height)); - } - float ratio; - int sourceWidth = source.Width; - int sourceHeight = source.Height; + int sourceWidth = sourceSize.Width; + int sourceHeight = sourceSize.Height; - int destinationX = 0; - int destinationY = 0; - int destinationWidth = width; - int destinationHeight = height; + int targetX = 0; + int targetY = 0; + int targetWidth = width; + int targetHeight = height; // Fractional variants for preserving aspect ratio. float percentHeight = MathF.Abs(height / (float)sourceHeight); @@ -366,50 +352,50 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms if (percentHeight < percentWidth) { ratio = percentHeight; - destinationWidth = (int)MathF.Round(sourceWidth * percentHeight); + targetWidth = (int)MathF.Round(sourceWidth * percentHeight); switch (options.Position) { case AnchorPositionMode.Left: case AnchorPositionMode.TopLeft: case AnchorPositionMode.BottomLeft: - destinationX = 0; + targetX = 0; break; case AnchorPositionMode.Right: case AnchorPositionMode.TopRight: case AnchorPositionMode.BottomRight: - destinationX = (int)MathF.Round(width - (sourceWidth * ratio)); + targetX = (int)MathF.Round(width - (sourceWidth * ratio)); break; default: - destinationX = (int)MathF.Round((width - (sourceWidth * ratio)) / 2F); + targetX = (int)MathF.Round((width - (sourceWidth * ratio)) / 2F); break; } } else { ratio = percentWidth; - destinationHeight = (int)MathF.Round(sourceHeight * percentWidth); + targetHeight = (int)MathF.Round(sourceHeight * percentWidth); switch (options.Position) { case AnchorPositionMode.Top: case AnchorPositionMode.TopLeft: case AnchorPositionMode.TopRight: - destinationY = 0; + targetY = 0; break; case AnchorPositionMode.Bottom: case AnchorPositionMode.BottomLeft: case AnchorPositionMode.BottomRight: - destinationY = (int)MathF.Round(height - (sourceHeight * ratio)); + targetY = (int)MathF.Round(height - (sourceHeight * ratio)); break; default: - destinationY = (int)MathF.Round((height - (sourceHeight * ratio)) / 2F); + targetY = (int)MathF.Round((height - (sourceHeight * ratio)) / 2F); break; } } - return (new Size(width, height), - new Rectangle(destinationX, destinationY, destinationWidth, destinationHeight)); + // Target image width and height can be different to the rectangle width and height. + return (new Size(width, height), new Rectangle(targetX, targetY, targetWidth, targetHeight)); } } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs index cf27de5eb1..6f5f09e717 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs @@ -26,19 +26,19 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms { Guard.NotNull(sampler, nameof(sampler)); - // Ensure size is populated across both dimensions. + // Ensure target size is populated across both dimensions. // If only one of the incoming dimensions is 0, it will be modified here to maintain aspect ratio. // If it is not possible to keep aspect ratio, make sure at least the minimum is is kept. - const int min = 1; + const int Min = 1; if (width == 0 && height > 0) { - width = (int)MathF.Max(min, MathF.Round(sourceSize.Width * height / (float)sourceSize.Height)); + width = (int)MathF.Max(Min, MathF.Round(sourceSize.Width * height / (float)sourceSize.Height)); targetRectangle.Width = width; } if (height == 0 && width > 0) { - height = (int)MathF.Max(min, MathF.Round(sourceSize.Height * width / (float)sourceSize.Width)); + height = (int)MathF.Max(Min, MathF.Round(sourceSize.Height * width / (float)sourceSize.Width)); targetRectangle.Height = height; } @@ -46,8 +46,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms Guard.MustBeGreaterThan(height, 0, nameof(height)); this.Sampler = sampler; - this.Width = width; - this.Height = height; + this.TargetWidth = width; + this.TargetHeight = height; this.TargetRectangle = targetRectangle; this.Compand = compand; } @@ -62,32 +62,11 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms Guard.NotNull(options, nameof(options)); Guard.NotNull(options.Sampler, nameof(options.Sampler)); - int targetWidth = options.Size.Width; - int targetHeight = options.Size.Height; - - // Ensure size is populated across both dimensions. - // These dimensions are used to calculate the final dimensions determined by the mode algorithm. - // If only one of the incoming dimensions is 0, it will be modified here to maintain aspect ratio. - // If it is not possible to keep aspect ratio, make sure at least the minimum is is kept. - const int min = 1; - if (targetWidth == 0 && targetHeight > 0) - { - targetWidth = (int)MathF.Max(min, MathF.Round(sourceSize.Width * targetHeight / (float)sourceSize.Height)); - } - - if (targetHeight == 0 && targetWidth > 0) - { - targetHeight = (int)MathF.Max(min, MathF.Round(sourceSize.Height * targetWidth / (float)sourceSize.Width)); - } - - Guard.MustBeGreaterThan(targetWidth, 0, nameof(targetWidth)); - Guard.MustBeGreaterThan(targetHeight, 0, nameof(targetHeight)); - - (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds(sourceSize, options, targetWidth, targetHeight); + (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds(sourceSize, options); this.Sampler = options.Sampler; - this.Width = size.Width; - this.Height = size.Height; + this.TargetWidth = size.Width; + this.TargetHeight = size.Height; this.TargetRectangle = rectangle; this.Compand = options.Compand; } @@ -112,12 +91,12 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// /// Gets the target width. /// - public int Width { get; } + public int TargetWidth { get; } /// /// Gets the target height. /// - public int Height { get; } + public int TargetHeight { get; } /// /// Gets the resize rectangle. diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs index e16d4801ec..b85983a481 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs @@ -45,15 +45,15 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// /// Gets the target width. /// - public int Width => this.parameterSource.Width; + public int TargetWidth => this.parameterSource.TargetWidth; /// /// Gets the target height. /// - public int Height => this.parameterSource.Height; + public int TargetHeight => this.parameterSource.TargetHeight; /// - /// Gets the resize rectangle. + /// Gets the target resize rectangle. /// public Rectangle TargetRectangle => this.parameterSource.TargetRectangle; @@ -80,8 +80,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms IEnumerable> frames = source.Frames.Select, ImageFrame>( x => new ImageFrame( configuration, - this.Width, - this.Height, + this.TargetWidth, + this.TargetHeight, x.Metadata.DeepClone())); // Use the overload to prevent an extra frame being added @@ -128,8 +128,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms return; } - int width = this.Width; - int height = this.Height; + int width = this.TargetWidth; + int height = this.TargetHeight; int sourceX = sourceRectangle.X; int sourceY = sourceRectangle.Y; int startY = this.TargetRectangle.Y; diff --git a/src/ImageSharp/Processing/ResizeOptions.cs b/src/ImageSharp/Processing/ResizeOptions.cs index ee0dde6b27..96de1eee11 100644 --- a/src/ImageSharp/Processing/ResizeOptions.cs +++ b/src/ImageSharp/Processing/ResizeOptions.cs @@ -1,8 +1,6 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. -using System; -using System.Collections.Generic; using SixLabors.ImageSharp.Processing.Processors.Transforms; using SixLabors.Primitives; @@ -26,7 +24,7 @@ namespace SixLabors.ImageSharp.Processing /// /// Gets or sets the center coordinates. /// - public IEnumerable CenterCoordinates { get; set; } = Array.Empty(); + public PointF? CenterCoordinates { get; set; } /// /// Gets or sets the target size. @@ -44,4 +42,4 @@ namespace SixLabors.ImageSharp.Processing /// public bool Compand { get; set; } = false; } -} \ No newline at end of file +} diff --git a/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeHelperTests.cs b/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeHelperTests.cs index b5ed64f7ef..b351ec235f 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeHelperTests.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeHelperTests.cs @@ -11,19 +11,18 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Transforms { public class ResizeHelperTests { - [Theory] [InlineData(20, 100, 1, 2)] - [InlineData(20, 100, 20*100*16, 2)] - [InlineData(20, 100, 40*100*16, 2)] - [InlineData(20, 100, 59*100*16, 2)] - [InlineData(20, 100, 60*100*16, 3)] - [InlineData(17, 63, 5*17*63*16, 5)] - [InlineData(17, 63, 5*17*63*16+1, 5)] - [InlineData(17, 63, 6*17*63*16-1, 5)] - [InlineData(33, 400, 1*1024*1024, 4)] - [InlineData(33, 400, 8*1024*1024, 39)] - [InlineData(50, 300, 1*1024*1024, 4)] + [InlineData(20, 100, 20 * 100 * 16, 2)] + [InlineData(20, 100, 40 * 100 * 16, 2)] + [InlineData(20, 100, 59 * 100 * 16, 2)] + [InlineData(20, 100, 60 * 100 * 16, 3)] + [InlineData(17, 63, 5 * 17 * 63 * 16, 5)] + [InlineData(17, 63, (5 * 17 * 63 * 16) + 1, 5)] + [InlineData(17, 63, (6 * 17 * 63 * 16) - 1, 5)] + [InlineData(33, 400, 1 * 1024 * 1024, 4)] + [InlineData(33, 400, 8 * 1024 * 1024, 39)] + [InlineData(50, 300, 1 * 1024 * 1024, 4)] public void CalculateResizeWorkerHeightInWindowBands( int windowDiameter, int width, @@ -40,17 +39,121 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Transforms var sourceSize = new Size(200, 100); var target = new Size(400, 200); - var actual = ResizeHelper.CalculateTargetLocationAndBounds( + (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds( sourceSize, - new ResizeOptions{ + new ResizeOptions + { Mode = ResizeMode.Min, Size = target - }, - target.Width, - target.Height); - - Assert.Equal(sourceSize, actual.Item1); - Assert.Equal(new Rectangle(0, 0, sourceSize.Width, sourceSize.Height), actual.Item2); + }); + + Assert.Equal(sourceSize, size); + Assert.Equal(new Rectangle(0, 0, sourceSize.Width, sourceSize.Height), rectangle); + } + + [Fact] + public void MaxSizeAndRectangleAreCorrect() + { + var sourceSize = new Size(5072, 6761); + var target = new Size(0, 450); + + var expectedSize = new Size(338, 450); + var expectedRectangle = new Rectangle(Point.Empty, expectedSize); + + (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds( + sourceSize, + new ResizeOptions + { + Mode = ResizeMode.Max, + Size = target + }); + + Assert.Equal(expectedSize, size); + Assert.Equal(expectedRectangle, rectangle); + } + + [Fact] + public void CropSizeAndRectangleAreCorrect() + { + var sourceSize = new Size(100, 100); + var target = new Size(25, 50); + + var expectedSize = new Size(25, 50); + var expectedRectangle = new Rectangle(-12, 0, 50, 50); + + (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds( + sourceSize, + new ResizeOptions + { + Mode = ResizeMode.Crop, + Size = target + }); + + Assert.Equal(expectedSize, size); + Assert.Equal(expectedRectangle, rectangle); + } + + [Fact] + public void BoxPadSizeAndRectangleAreCorrect() + { + var sourceSize = new Size(100, 100); + var target = new Size(120, 110); + + var expectedSize = new Size(120, 110); + var expectedRectangle = new Rectangle(10, 5, 100, 100); + + (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds( + sourceSize, + new ResizeOptions + { + Mode = ResizeMode.BoxPad, + Size = target + }); + + Assert.Equal(expectedSize, size); + Assert.Equal(expectedRectangle, rectangle); + } + + [Fact] + public void PadSizeAndRectangleAreCorrect() + { + var sourceSize = new Size(100, 100); + var target = new Size(120, 110); + + var expectedSize = new Size(120, 110); + var expectedRectangle = new Rectangle(5, 0, 110, 110); + + (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds( + sourceSize, + new ResizeOptions + { + Mode = ResizeMode.Pad, + Size = target + }); + + Assert.Equal(expectedSize, size); + Assert.Equal(expectedRectangle, rectangle); + } + + [Fact] + public void StretchSizeAndRectangleAreCorrect() + { + var sourceSize = new Size(100, 100); + var target = new Size(57, 32); + + var expectedSize = new Size(57, 32); + var expectedRectangle = new Rectangle(Point.Empty, expectedSize); + + (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds( + sourceSize, + new ResizeOptions + { + Mode = ResizeMode.Stretch, + Size = target + }); + + Assert.Equal(expectedSize, size); + Assert.Equal(expectedRectangle, rectangle); } } -} \ No newline at end of file +} diff --git a/tests/ImageSharp.Tests/Processing/Transforms/PadTest.cs b/tests/ImageSharp.Tests/Processing/Transforms/PadTest.cs index b870ddd08a..33da33c717 100644 --- a/tests/ImageSharp.Tests/Processing/Transforms/PadTest.cs +++ b/tests/ImageSharp.Tests/Processing/Transforms/PadTest.cs @@ -20,8 +20,8 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms this.operations.Pad(width, height); ResizeProcessor resizeProcessor = this.Verify(); - Assert.Equal(width, resizeProcessor.Width); - Assert.Equal(height, resizeProcessor.Height); + Assert.Equal(width, resizeProcessor.TargetWidth); + Assert.Equal(height, resizeProcessor.TargetHeight); Assert.Equal(sampler, resizeProcessor.Sampler); } } diff --git a/tests/ImageSharp.Tests/Processing/Transforms/ResizeTests.cs b/tests/ImageSharp.Tests/Processing/Transforms/ResizeTests.cs index c7ebe65e8c..f268eda86c 100644 --- a/tests/ImageSharp.Tests/Processing/Transforms/ResizeTests.cs +++ b/tests/ImageSharp.Tests/Processing/Transforms/ResizeTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using SixLabors.ImageSharp.Processing; @@ -18,8 +18,8 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms this.operations.Resize(width, height); ResizeProcessor resizeProcessor = this.Verify(); - Assert.Equal(width, resizeProcessor.Width); - Assert.Equal(height, resizeProcessor.Height); + Assert.Equal(width, resizeProcessor.TargetWidth); + Assert.Equal(height, resizeProcessor.TargetHeight); } [Fact] @@ -31,8 +31,8 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms this.operations.Resize(width, height, sampler); ResizeProcessor resizeProcessor = this.Verify(); - Assert.Equal(width, resizeProcessor.Width); - Assert.Equal(height, resizeProcessor.Height); + Assert.Equal(width, resizeProcessor.TargetWidth); + Assert.Equal(height, resizeProcessor.TargetHeight); Assert.Equal(sampler, resizeProcessor.Sampler); } @@ -48,8 +48,8 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms this.operations.Resize(width, height, sampler, compand); ResizeProcessor resizeProcessor = this.Verify(); - Assert.Equal(width, resizeProcessor.Width); - Assert.Equal(height, resizeProcessor.Height); + Assert.Equal(width, resizeProcessor.TargetWidth); + Assert.Equal(height, resizeProcessor.TargetHeight); Assert.Equal(sampler, resizeProcessor.Sampler); Assert.Equal(compand, resizeProcessor.Compand); } @@ -74,8 +74,8 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms this.operations.Resize(resizeOptions); ResizeProcessor resizeProcessor = this.Verify(); - Assert.Equal(width, resizeProcessor.Width); - Assert.Equal(height, resizeProcessor.Height); + Assert.Equal(width, resizeProcessor.TargetWidth); + Assert.Equal(height, resizeProcessor.TargetHeight); Assert.Equal(sampler, resizeProcessor.Sampler); Assert.Equal(compand, resizeProcessor.Compand); @@ -87,4 +87,4 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms Assert.Equal(mode, resizeOptions.Mode); } } -} \ No newline at end of file +} diff --git a/tests/ImageSharp.Tests/xunit.runner.json b/tests/ImageSharp.Tests/xunit.runner.json index 5204242f03..d7b466d09d 100644 --- a/tests/ImageSharp.Tests/xunit.runner.json +++ b/tests/ImageSharp.Tests/xunit.runner.json @@ -1,5 +1,6 @@ { - "shadowCopy": false, - "methodDisplay": "method", - "diagnosticMessages": true -} \ No newline at end of file + "shadowCopy": false, + "methodDisplay": "method", + "diagnosticMessages": true, + "preEnumerateTheories": false +} From 54aea38d785cdcda571d3f631337360ff493df37 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 8 Sep 2019 17:20:45 +1000 Subject: [PATCH 06/22] revert preenumeration rule --- tests/ImageSharp.Tests/xunit.runner.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/ImageSharp.Tests/xunit.runner.json b/tests/ImageSharp.Tests/xunit.runner.json index d7b466d09d..749ece4387 100644 --- a/tests/ImageSharp.Tests/xunit.runner.json +++ b/tests/ImageSharp.Tests/xunit.runner.json @@ -1,6 +1,5 @@ { "shadowCopy": false, "methodDisplay": "method", - "diagnosticMessages": true, - "preEnumerateTheories": false + "diagnosticMessages": true } From cf0bb2540f4e8b20b7dfdd50953d5e3eaa036db5 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 8 Sep 2019 22:49:42 +1000 Subject: [PATCH 07/22] Fix #1004 --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 38 +++++++++++++++---- .../Formats/Png/Zlib/ZlibInflateStream.cs | 36 ++++++++++++++---- .../Formats/Png/PngDecoderTests.cs | 3 +- tests/ImageSharp.Tests/TestImages.cs | 1 + .../Images/Input/Png/zlib-ztxt-bad-header.png | 3 ++ 5 files changed, 64 insertions(+), 17 deletions(-) create mode 100644 tests/Images/Input/Png/zlib-ztxt-bad-header.png diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 9bc5a5079d..a9e588f6ee 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -175,11 +175,18 @@ namespace SixLabors.ImageSharp.Formats.Png this.InitializeImage(metadata, out image); } - using (var deframeStream = new ZlibInflateStream(this.currentStream, this.ReadNextDataChunk)) + var deframeStream = new ZlibInflateStream(this.currentStream, this.ReadNextDataChunk); + try { - deframeStream.AllocateNewBytes(chunk.Length); + deframeStream.AllocateNewBytes(chunk.Length, true); this.ReadScanlines(deframeStream.CompressedStream, image.Frames.RootFrame, pngMetadata); } + finally + { + // If an invalid Zlib stream is discovered the decoder will throw an exception + // due to the critical nature of the data chunk. + deframeStream.Dispose(); + } break; case PngChunkType.Palette: @@ -924,7 +931,11 @@ namespace SixLabors.ImageSharp.Formats.Png } ReadOnlySpan compressedData = data.Slice(zeroIndex + 2); - metadata.TextData.Add(new PngTextData(name, this.UncompressTextData(compressedData, PngConstants.Encoding), string.Empty, string.Empty)); + + if (this.TryUncompressTextData(compressedData, PngConstants.Encoding, out string uncompressed)) + { + metadata.TextData.Add(new PngTextData(name, uncompressed, string.Empty, string.Empty)); + } } /// @@ -987,7 +998,11 @@ namespace SixLabors.ImageSharp.Formats.Png if (compressionFlag == 1) { ReadOnlySpan compressedData = data.Slice(dataStartIdx); - metadata.TextData.Add(new PngTextData(keyword, this.UncompressTextData(compressedData, PngConstants.TranslatedEncoding), language, translatedKeyword)); + + if (this.TryUncompressTextData(compressedData, PngConstants.TranslatedEncoding, out string uncompressed)) + { + metadata.TextData.Add(new PngTextData(keyword, uncompressed, language, translatedKeyword)); + } } else { @@ -1001,13 +1016,19 @@ namespace SixLabors.ImageSharp.Formats.Png /// /// Compressed text data bytes. /// The string encoding to use. - /// A string. - private string UncompressTextData(ReadOnlySpan compressedData, Encoding encoding) + /// The uncompressed value. + /// The . + private bool TryUncompressTextData(ReadOnlySpan compressedData, Encoding encoding, out string value) { using (var memoryStream = new MemoryStream(compressedData.ToArray())) using (var inflateStream = new ZlibInflateStream(memoryStream, () => 0)) { - inflateStream.AllocateNewBytes(compressedData.Length); + if (!inflateStream.AllocateNewBytes(compressedData.Length, false)) + { + value = null; + return false; + } + var uncompressedBytes = new List(); // Note: this uses the a buffer which is only 4 bytes long to read the stream, maybe allocating a larger buffer makes sense here. @@ -1018,7 +1039,8 @@ namespace SixLabors.ImageSharp.Formats.Png bytesRead = inflateStream.CompressedStream.Read(this.buffer, 0, this.buffer.Length); } - return encoding.GetString(uncompressedBytes.ToArray()); + value = encoding.GetString(uncompressedBytes.ToArray()); + return true; } } diff --git a/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs b/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs index 405eeafeb9..df0e723322 100644 --- a/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs +++ b/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs @@ -87,13 +87,17 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib /// Adds new bytes from a frame found in the original stream /// /// blabla - public void AllocateNewBytes(int bytes) + /// Whether the chunk to be inflated is a critical chunk. + /// The . + public bool AllocateNewBytes(int bytes, bool isCriticalChunk) { this.currentDataRemaining = bytes; if (this.compressedStream is null) { - this.InitializeInflateStream(); + return this.InitializeInflateStream(isCriticalChunk); } + + return true; } /// @@ -197,7 +201,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib this.isDisposed = true; } - private void InitializeInflateStream() + private bool InitializeInflateStream(bool isCriticalChunk) { // Read the zlib header : http://tools.ietf.org/html/rfc1950 // CMF(Compression Method and flags) @@ -215,7 +219,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib this.currentDataRemaining -= 2; if (cmf == -1 || flag == -1) { - return; + return false; } if ((cmf & 0x0F) == 8) @@ -225,14 +229,28 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib if (cinfo > 7) { - // Values of CINFO above 7 are not allowed in RFC1950. - // CINFO is not defined in this specification for CM not equal to 8. - throw new ImageFormatException($"Invalid window size for ZLIB header: cinfo={cinfo}"); + if (isCriticalChunk) + { + // Values of CINFO above 7 are not allowed in RFC1950. + // CINFO is not defined in this specification for CM not equal to 8. + throw new ImageFormatException($"Invalid window size for ZLIB header: cinfo={cinfo}"); + } + else + { + return false; + } } } else { - throw new ImageFormatException($"Bad method for ZLIB header: cmf={cmf}"); + if (isCriticalChunk) + { + throw new ImageFormatException($"Bad method for ZLIB header: cmf={cmf}"); + } + else + { + return false; + } } // The preset dictionary. @@ -247,6 +265,8 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib // Initialize the deflate Stream. this.compressedStream = new DeflateStream(this, CompressionMode.Decompress, true); + + return true; } } } diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 2e9fd7481e..91b1ef2c17 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -40,7 +40,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png TestImages.Png.GrayAlpha8Bit, TestImages.Png.Gray1BitTrans, TestImages.Png.Bad.ZlibOverflow, - TestImages.Png.Bad.ZlibOverflow2 + TestImages.Png.Bad.ZlibOverflow2, + TestImages.Png.Bad.ZlibZtxtBadHeader, }; public static readonly string[] TestImages48Bpp = diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index e95ce09073..163d09bdde 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -90,6 +90,7 @@ namespace SixLabors.ImageSharp.Tests public const string CorruptedChunk = "Png/big-corrupted-chunk.png"; public const string ZlibOverflow = "Png/zlib-overflow.png"; public const string ZlibOverflow2 = "Png/zlib-overflow2.png"; + public const string ZlibZtxtBadHeader = "Png/zlib-ztxt-bad-header.png"; } public static readonly string[] All = diff --git a/tests/Images/Input/Png/zlib-ztxt-bad-header.png b/tests/Images/Input/Png/zlib-ztxt-bad-header.png new file mode 100644 index 0000000000..0eb37aab87 --- /dev/null +++ b/tests/Images/Input/Png/zlib-ztxt-bad-header.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:ce623255656921d491b5c389cd46931fbd6024575b87522c55d67a496dd761f0 +size 22781 From 855e75f674ddbb50b9d1bcfcd5ca08b65c787fb9 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 9 Sep 2019 23:41:51 +1000 Subject: [PATCH 08/22] Refactor helper to reduce code duplication --- .../Processing/Extensions/ResizeExtensions.cs | 50 +++++++++++++----- .../Transforms/Resize/ResizeHelper.cs | 35 +++++++++++-- .../Transforms/Resize/ResizeProcessor.cs | 51 ------------------- src/ImageSharp/Processing/ResizeMode.cs | 9 +++- src/ImageSharp/Processing/ResizeOptions.cs | 5 ++ 5 files changed, 79 insertions(+), 71 deletions(-) diff --git a/src/ImageSharp/Processing/Extensions/ResizeExtensions.cs b/src/ImageSharp/Processing/Extensions/ResizeExtensions.cs index 81b1c2c663..f494ed9094 100644 --- a/src/ImageSharp/Processing/Extensions/ResizeExtensions.cs +++ b/src/ImageSharp/Processing/Extensions/ResizeExtensions.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using SixLabors.ImageSharp.Processing.Processors.Transforms; @@ -12,16 +12,6 @@ namespace SixLabors.ImageSharp.Processing /// public static class ResizeExtensions { - /// - /// Resizes an image in accordance with the given . - /// - /// The image to resize. - /// The resize options. - /// The to allow chaining of operations. - /// Passing zero for one of height or width within the resize options will automatically preserve the aspect ratio of the original image or the nearest possible ratio. - public static IImageProcessingContext Resize(this IImageProcessingContext source, ResizeOptions options) - => source.ApplyProcessor(new ResizeProcessor(options, source.GetCurrentSize())); - /// /// Resizes an image to the given . /// @@ -128,7 +118,18 @@ namespace SixLabors.ImageSharp.Processing Rectangle sourceRectangle, Rectangle targetRectangle, bool compand) - => source.ApplyProcessor(new ResizeProcessor(sampler, width, height, source.GetCurrentSize(), targetRectangle, compand), sourceRectangle); + { + var options = new ResizeOptions + { + Size = new Size(width, height), + Mode = ResizeMode.Manual, + Sampler = sampler, + TargetRectangle = targetRectangle, + Compand = compand + }; + + return source.ApplyProcessor(new ResizeProcessor(options, source.GetCurrentSize()), sourceRectangle); + } /// /// Resizes an image to the given width and height with the given sampler and source rectangle. @@ -150,6 +151,27 @@ namespace SixLabors.ImageSharp.Processing IResampler sampler, Rectangle targetRectangle, bool compand) - => source.ApplyProcessor(new ResizeProcessor(sampler, width, height, source.GetCurrentSize(), targetRectangle, compand)); + { + var options = new ResizeOptions + { + Size = new Size(width, height), + Mode = ResizeMode.Manual, + Sampler = sampler, + TargetRectangle = targetRectangle, + Compand = compand + }; + + return Resize(source, options); + } + + /// + /// Resizes an image in accordance with the given . + /// + /// The image to resize. + /// The resize options. + /// The to allow chaining of operations. + /// Passing zero for one of height or width within the resize options will automatically preserve the aspect ratio of the original image or the nearest possible ratio. + public static IImageProcessingContext Resize(this IImageProcessingContext source, ResizeOptions options) + => source.ApplyProcessor(new ResizeProcessor(options, source.GetCurrentSize())); } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeHelper.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeHelper.cs index c9df1b2542..eacd3834f1 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeHelper.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeHelper.cs @@ -36,6 +36,11 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms int width = options.Size.Width; int height = options.Size.Height; + if (width <= 0 && height <= 0) + { + ThrowInvalid($"Target width {width} and height {height} must be greater than zero."); + } + // Ensure target size is populated across both dimensions. // These dimensions are used to calculate the final dimensions determined by the mode algorithm. // If only one of the incoming dimensions is 0, it will be modified here to maintain aspect ratio. @@ -51,9 +56,6 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms height = (int)MathF.Max(Min, MathF.Round(sourceSize.Height * width / (float)sourceSize.Width)); } - Guard.MustBeGreaterThan(width, 0, nameof(width)); - Guard.MustBeGreaterThan(height, 0, nameof(height)); - switch (options.Mode) { case ResizeMode.Crop: @@ -66,8 +68,10 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms return CalculateMaxRectangle(sourceSize, width, height); case ResizeMode.Min: return CalculateMinRectangle(sourceSize, width, height); + case ResizeMode.Manual: + return CalculateManualRectangle(options, width, height); - // Last case ResizeMode.Stretch: + // case ResizeMode.Stretch: default: return (new Size(width, height), new Rectangle(0, 0, width, height)); } @@ -397,5 +401,28 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms // Target image width and height can be different to the rectangle width and height. return (new Size(width, height), new Rectangle(targetX, targetY, targetWidth, targetHeight)); } + + private static (Size, Rectangle) CalculateManualRectangle( + ResizeOptions options, + int width, + int height) + { + if (!options.TargetRectangle.HasValue) + { + ThrowInvalid("Manual resizing requires a target location and size."); + } + + Rectangle targetRectangle = options.TargetRectangle.Value; + + int targetX = targetRectangle.X; + int targetY = targetRectangle.Y; + int targetWidth = targetRectangle.Width > 0 ? targetRectangle.Width : width; + int targetHeight = targetRectangle.Height > 0 ? targetRectangle.Height : height; + + // Target image width and height can be different to the rectangle width and height. + return (new Size(width, height), new Rectangle(targetX, targetY, targetWidth, targetHeight)); + } + + private static void ThrowInvalid(string message) => throw new InvalidOperationException(message); } } diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs index 6f5f09e717..35e22757c1 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs @@ -13,45 +13,6 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// public class ResizeProcessor : IImageProcessor { - /// - /// Initializes a new instance of the class. - /// - /// The . - /// The width. - /// The height. - /// The size of the source image. - /// The target rectangle to resize into. - /// A value indicating whether to apply RGBA companding. - public ResizeProcessor(IResampler sampler, int width, int height, Size sourceSize, Rectangle targetRectangle, bool compand) - { - Guard.NotNull(sampler, nameof(sampler)); - - // Ensure target size is populated across both dimensions. - // If only one of the incoming dimensions is 0, it will be modified here to maintain aspect ratio. - // If it is not possible to keep aspect ratio, make sure at least the minimum is is kept. - const int Min = 1; - if (width == 0 && height > 0) - { - width = (int)MathF.Max(Min, MathF.Round(sourceSize.Width * height / (float)sourceSize.Height)); - targetRectangle.Width = width; - } - - if (height == 0 && width > 0) - { - height = (int)MathF.Max(Min, MathF.Round(sourceSize.Height * width / (float)sourceSize.Width)); - targetRectangle.Height = height; - } - - Guard.MustBeGreaterThan(width, 0, nameof(width)); - Guard.MustBeGreaterThan(height, 0, nameof(height)); - - this.Sampler = sampler; - this.TargetWidth = width; - this.TargetHeight = height; - this.TargetRectangle = targetRectangle; - this.Compand = compand; - } - /// /// Initializes a new instance of the class. /// @@ -71,18 +32,6 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms this.Compand = options.Compand; } - /// - /// Initializes a new instance of the class. - /// - /// The sampler to perform the resize operation. - /// The target width. - /// The target height. - /// The source image size - public ResizeProcessor(IResampler sampler, int width, int height, Size sourceSize) - : this(sampler, width, height, sourceSize, new Rectangle(0, 0, width, height), false) - { - } - /// /// Gets the sampler to perform the resize operation. /// diff --git a/src/ImageSharp/Processing/ResizeMode.cs b/src/ImageSharp/Processing/ResizeMode.cs index 6adeac66da..142a926b30 100644 --- a/src/ImageSharp/Processing/ResizeMode.cs +++ b/src/ImageSharp/Processing/ResizeMode.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. namespace SixLabors.ImageSharp.Processing @@ -42,6 +42,11 @@ namespace SixLabors.ImageSharp.Processing /// /// Stretches the resized image to fit the bounds of its container. /// - Stretch + Stretch, + + /// + /// The target location and size of the resized image has been manually set. + /// + Manual } } diff --git a/src/ImageSharp/Processing/ResizeOptions.cs b/src/ImageSharp/Processing/ResizeOptions.cs index 96de1eee11..ef88dc35b3 100644 --- a/src/ImageSharp/Processing/ResizeOptions.cs +++ b/src/ImageSharp/Processing/ResizeOptions.cs @@ -41,5 +41,10 @@ namespace SixLabors.ImageSharp.Processing /// or expand individual pixel colors the value on processing. /// public bool Compand { get; set; } = false; + + /// + /// Gets or sets the target rectangle to resize into. + /// + public Rectangle? TargetRectangle { get; set; } } } From e89c1ef3d49693ffc48c75d1f3d7797fb6a0e9cc Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 10 Sep 2019 17:34:14 +1000 Subject: [PATCH 09/22] Clean up scanline decoding code. --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 38 ++++++++---------- .../Formats/Png/Zlib/ZlibInflateStream.cs | 40 +++++++++++-------- 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index a9e588f6ee..037f648f0a 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -5,6 +5,7 @@ using System; using System.Buffers.Binary; using System.Collections.Generic; using System.IO; +using System.IO.Compression; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; @@ -175,18 +176,7 @@ namespace SixLabors.ImageSharp.Formats.Png this.InitializeImage(metadata, out image); } - var deframeStream = new ZlibInflateStream(this.currentStream, this.ReadNextDataChunk); - try - { - deframeStream.AllocateNewBytes(chunk.Length, true); - this.ReadScanlines(deframeStream.CompressedStream, image.Frames.RootFrame, pngMetadata); - } - finally - { - // If an invalid Zlib stream is discovered the decoder will throw an exception - // due to the critical nature of the data chunk. - deframeStream.Dispose(); - } + this.ReadScanlines(chunk, image.Frames.RootFrame, pngMetadata); break; case PngChunkType.Palette: @@ -472,19 +462,25 @@ namespace SixLabors.ImageSharp.Formats.Png /// Reads the scanlines within the image. /// /// The pixel format. - /// The containing data. + /// The png chunk containing the compressed scanline data. /// The pixel data. /// The png metadata - private void ReadScanlines(Stream dataStream, ImageFrame image, PngMetadata pngMetadata) + private void ReadScanlines(PngChunk chunk, ImageFrame image, PngMetadata pngMetadata) where TPixel : struct, IPixel { - if (this.header.InterlaceMethod == PngInterlaceMode.Adam7) - { - this.DecodeInterlacedPixelData(dataStream, image, pngMetadata); - } - else + using (var deframeStream = new ZlibInflateStream(this.currentStream, this.ReadNextDataChunk)) { - this.DecodePixelData(dataStream, image, pngMetadata); + deframeStream.AllocateNewBytes(chunk.Length, true); + DeflateStream dataStream = deframeStream.CompressedStream; + + if (this.header.InterlaceMethod == PngInterlaceMode.Adam7) + { + this.DecodeInterlacedPixelData(dataStream, image, pngMetadata); + } + else + { + this.DecodePixelData(dataStream, image, pngMetadata); + } } } @@ -1021,7 +1017,7 @@ namespace SixLabors.ImageSharp.Formats.Png private bool TryUncompressTextData(ReadOnlySpan compressedData, Encoding encoding, out string value) { using (var memoryStream = new MemoryStream(compressedData.ToArray())) - using (var inflateStream = new ZlibInflateStream(memoryStream, () => 0)) + using (var inflateStream = new ZlibInflateStream(memoryStream)) { if (!inflateStream.AllocateNewBytes(compressedData.Length, false)) { diff --git a/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs b/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs index df0e723322..e4645c44ac 100644 --- a/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs +++ b/src/ImageSharp/Formats/Png/Zlib/ZlibInflateStream.cs @@ -20,14 +20,14 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib private static readonly byte[] ChecksumBuffer = new byte[4]; /// - /// The inner raw memory stream + /// A default delegate to get more data from the inner stream. /// - private readonly Stream innerStream; + private static readonly Func GetDataNoOp = () => 0; /// - /// The compressed stream sitting over the top of the deframer + /// The inner raw memory stream /// - private DeflateStream compressedStream; + private readonly Stream innerStream; /// /// A value indicating whether this instance of the given entity has been disposed. @@ -55,8 +55,17 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib /// /// Initializes a new instance of the class. /// - /// The inner raw stream - /// A delegate to get more data from the inner stream + /// The inner raw stream. + public ZlibInflateStream(Stream innerStream) + : this(innerStream, GetDataNoOp) + { + } + + /// + /// Initializes a new instance of the class. + /// + /// The inner raw stream. + /// A delegate to get more data from the inner stream. public ZlibInflateStream(Stream innerStream, Func getData) { this.innerStream = innerStream; @@ -76,12 +85,12 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib public override long Length => throw new NotSupportedException(); /// - public override long Position { get => throw new NotImplementedException(); set => throw new NotImplementedException(); } + public override long Position { get => throw new NotSupportedException(); set => throw new NotSupportedException(); } /// /// Gets the compressed stream over the deframed inner stream /// - public DeflateStream CompressedStream => this.compressedStream; + public DeflateStream CompressedStream { get; private set; } /// /// Adds new bytes from a frame found in the original stream @@ -92,7 +101,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib public bool AllocateNewBytes(int bytes, bool isCriticalChunk) { this.currentDataRemaining = bytes; - if (this.compressedStream is null) + if (this.CompressedStream is null) { return this.InitializeInflateStream(isCriticalChunk); } @@ -101,10 +110,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib } /// - public override void Flush() - { - throw new NotSupportedException(); - } + public override void Flush() => throw new NotSupportedException(); /// public override int ReadByte() @@ -186,10 +192,10 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib if (disposing) { // dispose managed resources - if (this.compressedStream != null) + if (this.CompressedStream != null) { - this.compressedStream.Dispose(); - this.compressedStream = null; + this.CompressedStream.Dispose(); + this.CompressedStream = null; } } @@ -264,7 +270,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Zlib } // Initialize the deflate Stream. - this.compressedStream = new DeflateStream(this, CompressionMode.Decompress, true); + this.CompressedStream = new DeflateStream(this, CompressionMode.Decompress, true); return true; } From 75209ca205d37b2d7c0b58aaa905cc6c0bd1711b Mon Sep 17 00:00:00 2001 From: Xavier Cho Date: Thu, 12 Sep 2019 12:55:05 +0900 Subject: [PATCH 10/22] Remove a redundant check for the paint boundary --- src/ImageSharp.Drawing/Processing/PathGradientBrush.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/ImageSharp.Drawing/Processing/PathGradientBrush.cs b/src/ImageSharp.Drawing/Processing/PathGradientBrush.cs index fc84e4a1fa..553ed181c0 100644 --- a/src/ImageSharp.Drawing/Processing/PathGradientBrush.cs +++ b/src/ImageSharp.Drawing/Processing/PathGradientBrush.cs @@ -231,11 +231,6 @@ namespace SixLabors.ImageSharp.Processing return new Color(this.centerColor).ToPixel(); } - if (!this.path.Contains(point)) - { - return Color.Transparent.ToPixel(); - } - Vector2 direction = Vector2.Normalize(point - this.center); PointF end = point + (PointF)(direction * this.maxDistance); From b9d6a14af6a85a257f15976e4b0f6c8271f23d78 Mon Sep 17 00:00:00 2001 From: Xavier Cho Date: Thu, 12 Sep 2019 12:55:28 +0900 Subject: [PATCH 11/22] Ignore invalid gradient positions rather than throwing an error There can be such cases where it fails to find intersection points for a given position, especially when the polygon is small. Such an input would result in an error previously, but now it renders correctly. --- src/ImageSharp.Drawing/Processing/PathGradientBrush.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/ImageSharp.Drawing/Processing/PathGradientBrush.cs b/src/ImageSharp.Drawing/Processing/PathGradientBrush.cs index 553ed181c0..636551b642 100644 --- a/src/ImageSharp.Drawing/Processing/PathGradientBrush.cs +++ b/src/ImageSharp.Drawing/Processing/PathGradientBrush.cs @@ -237,6 +237,11 @@ namespace SixLabors.ImageSharp.Processing (Edge edge, Intersection? info) = this.FindIntersection(point, end); + if (!info.HasValue) + { + return Color.Transparent.ToPixel(); + } + PointF intersection = info.Value.Point; Vector4 edgeColor = edge.ColorAt(intersection); From ad56a756ceba888b37979031f7f72285b7bbca09 Mon Sep 17 00:00:00 2001 From: Turnerj Date: Thu, 12 Sep 2019 16:27:56 +0930 Subject: [PATCH 12/22] Fix horizontally out-of-bounds error when drawing text --- .../Text/DrawTextProcessor{TPixel}.cs | 17 +++++++++++------ .../Drawing/Text/DrawTextOnImageTests.cs | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp.Drawing/Processing/Processors/Text/DrawTextProcessor{TPixel}.cs b/src/ImageSharp.Drawing/Processing/Processors/Text/DrawTextProcessor{TPixel}.cs index 3809200d5f..ea042635dd 100644 --- a/src/ImageSharp.Drawing/Processing/Processors/Text/DrawTextProcessor{TPixel}.cs +++ b/src/ImageSharp.Drawing/Processing/Processors/Text/DrawTextProcessor{TPixel}.cs @@ -90,26 +90,31 @@ namespace SixLabors.ImageSharp.Processing.Processors.Text Buffer2D buffer = operation.Map; int startY = operation.Location.Y; int startX = operation.Location.X; - int offSetSpan = 0; + int offsetSpan = 0; if (startX < 0) { - offSetSpan = -startX; + offsetSpan = -startX; startX = 0; } - int fistRow = 0; + if (startX >= source.Width) + { + continue; + } + + int firstRow = 0; if (startY < 0) { - fistRow = -startY; + firstRow = -startY; } int maxHeight = source.Height - startY; int end = Math.Min(operation.Map.Height, maxHeight); - for (int row = fistRow; row < end; row++) + for (int row = firstRow; row < end; row++) { int y = startY + row; - Span span = buffer.GetRowSpan(row).Slice(offSetSpan); + Span span = buffer.GetRowSpan(row).Slice(offsetSpan); app.Apply(span, startX, y); } } diff --git a/tests/ImageSharp.Tests/Drawing/Text/DrawTextOnImageTests.cs b/tests/ImageSharp.Tests/Drawing/Text/DrawTextOnImageTests.cs index 7bebdabd3a..dc1ccc8134 100644 --- a/tests/ImageSharp.Tests/Drawing/Text/DrawTextOnImageTests.cs +++ b/tests/ImageSharp.Tests/Drawing/Text/DrawTextOnImageTests.cs @@ -65,6 +65,24 @@ namespace SixLabors.ImageSharp.Tests.Drawing.Text } } + [Theory] + [WithSolidFilledImages(1500, 500, "White", PixelTypes.Rgba32)] + public void DoesntThrowExceptionWhenOverlappingRightEdge_Issue688_2(TestImageProvider provider) + where TPixel : struct, IPixel + { + using (Image img = provider.GetImage()) + { + Font font = SystemFonts.CreateFont("Arial", 39, FontStyle.Regular); + string text = new string('a', 10000); // exception + // string text = "Hello"; // no exception + Rgba32 color = Rgba32.Black; + var point = new PointF(100, 100); + + img.Mutate(ctx => ctx.DrawText(text, font, color, point)); + } + } + + [Theory] [WithSolidFilledImages(200, 100, "White", PixelTypes.Rgba32, 50, 0, 0, "SixLaborsSampleAB.woff", AB)] [WithSolidFilledImages(900, 100, "White", PixelTypes.Rgba32, 50, 0, 0, "OpenSans-Regular.ttf", TestText)] From 6a0ce745e8fad13139ebbee05a376316eaaf26b1 Mon Sep 17 00:00:00 2001 From: Turnerj Date: Thu, 12 Sep 2019 16:41:11 +0930 Subject: [PATCH 13/22] Updated font used for test --- tests/ImageSharp.Tests/Drawing/Text/DrawTextOnImageTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/Drawing/Text/DrawTextOnImageTests.cs b/tests/ImageSharp.Tests/Drawing/Text/DrawTextOnImageTests.cs index dc1ccc8134..90740a4bb2 100644 --- a/tests/ImageSharp.Tests/Drawing/Text/DrawTextOnImageTests.cs +++ b/tests/ImageSharp.Tests/Drawing/Text/DrawTextOnImageTests.cs @@ -72,7 +72,7 @@ namespace SixLabors.ImageSharp.Tests.Drawing.Text { using (Image img = provider.GetImage()) { - Font font = SystemFonts.CreateFont("Arial", 39, FontStyle.Regular); + Font font = SystemFonts.CreateFont("OpenSans-Regular.ttf", 39, FontStyle.Regular); string text = new string('a', 10000); // exception // string text = "Hello"; // no exception Rgba32 color = Rgba32.Black; From fa1d9efaa2e43f31e2c6f0ea9a53eb95598b21f9 Mon Sep 17 00:00:00 2001 From: Turnerj Date: Thu, 12 Sep 2019 16:56:30 +0930 Subject: [PATCH 14/22] Use local CreateFont method --- tests/ImageSharp.Tests/Drawing/Text/DrawTextOnImageTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/Drawing/Text/DrawTextOnImageTests.cs b/tests/ImageSharp.Tests/Drawing/Text/DrawTextOnImageTests.cs index 90740a4bb2..a767a686ed 100644 --- a/tests/ImageSharp.Tests/Drawing/Text/DrawTextOnImageTests.cs +++ b/tests/ImageSharp.Tests/Drawing/Text/DrawTextOnImageTests.cs @@ -72,7 +72,7 @@ namespace SixLabors.ImageSharp.Tests.Drawing.Text { using (Image img = provider.GetImage()) { - Font font = SystemFonts.CreateFont("OpenSans-Regular.ttf", 39, FontStyle.Regular); + Font font = CreateFont("OpenSans-Regular.ttf", 39); string text = new string('a', 10000); // exception // string text = "Hello"; // no exception Rgba32 color = Rgba32.Black; From 7e06b63420dea2ca51ecaad108d182ff2bc7919e Mon Sep 17 00:00:00 2001 From: Xavier Cho Date: Thu, 12 Sep 2019 19:37:07 +0900 Subject: [PATCH 15/22] Accept points instead of line segments as constructor argument Note that this is a breaking change. --- .../Processing/PathGradientBrush.cs | 43 +++++----- .../Drawing/FillPathGradientBrushTests.cs | 86 ++++--------------- 2 files changed, 39 insertions(+), 90 deletions(-) diff --git a/src/ImageSharp.Drawing/Processing/PathGradientBrush.cs b/src/ImageSharp.Drawing/Processing/PathGradientBrush.cs index 636551b642..7315dc5a3e 100644 --- a/src/ImageSharp.Drawing/Processing/PathGradientBrush.cs +++ b/src/ImageSharp.Drawing/Processing/PathGradientBrush.cs @@ -18,8 +18,6 @@ namespace SixLabors.ImageSharp.Processing /// public sealed class PathGradientBrush : IBrush { - private readonly Polygon path; - private readonly IList edges; private readonly Color centerColor; @@ -27,20 +25,20 @@ namespace SixLabors.ImageSharp.Processing /// /// Initializes a new instance of the class. /// - /// Line segments of a polygon that represents the gradient area. + /// Points that constitute a polygon that represents the gradient area. /// Array of colors that correspond to each point in the polygon. /// Color at the center of the gradient area to which the other colors converge. - 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."); } @@ -56,22 +54,30 @@ namespace SixLabors.ImageSharp.Processing "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(); } /// /// Initializes a new instance of the class. /// - /// Line segments of a polygon that represents the gradient area. + /// Points that constitute a polygon that represents the gradient area. /// Array of colors that correspond to each point in the polygon. - public PathGradientBrush(ILineSegment[] lines, Color[] colors) - : this(lines, colors, CalculateCenterColor(colors)) + public PathGradientBrush(PointF[] points, Color[] colors) + : this(points, colors, CalculateCenterColor(colors)) { } @@ -82,7 +88,7 @@ namespace SixLabors.ImageSharp.Processing GraphicsOptions options) where TPixel : struct, IPixel { - return new PathGradientBrushApplicator(source, this.path, this.edges, this.centerColor, options); + return new PathGradientBrushApplicator(source, this.edges, this.centerColor, options); } private static Color CalculateCenterColor(Color[] colors) @@ -182,8 +188,6 @@ namespace SixLabors.ImageSharp.Processing private class PathGradientBrushApplicator : BrushApplicator where TPixel : struct, IPixel { - private readonly Path path; - private readonly PointF center; private readonly Vector4 centerColor; @@ -196,24 +200,21 @@ namespace SixLabors.ImageSharp.Processing /// Initializes a new instance of the class. /// /// The source image. - /// A polygon that represents the gradient area. /// Edges of the polygon. /// Color at the center of the gradient area to which the other colors converge. /// The options. public PathGradientBrushApplicator( ImageFrame source, - Path path, IList 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(); diff --git a/tests/ImageSharp.Tests/Drawing/FillPathGradientBrushTests.cs b/tests/ImageSharp.Tests/Drawing/FillPathGradientBrushTests.cs index d76893108b..1ab747bafc 100644 --- a/tests/ImageSharp.Tests/Drawing/FillPathGradientBrushTests.cs +++ b/tests/ImageSharp.Tests/Drawing/FillPathGradientBrushTests.cs @@ -7,7 +7,6 @@ using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; using SixLabors.Primitives; -using SixLabors.Shapes; using Xunit; @@ -27,17 +26,10 @@ namespace SixLabors.ImageSharp.Tests.Drawing 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); @@ -53,16 +45,10 @@ namespace SixLabors.ImageSharp.Tests.Drawing 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); @@ -76,17 +62,10 @@ namespace SixLabors.ImageSharp.Tests.Drawing { using (Image 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)); @@ -103,17 +82,10 @@ namespace SixLabors.ImageSharp.Tests.Drawing 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); @@ -129,17 +101,10 @@ namespace SixLabors.ImageSharp.Tests.Drawing 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); @@ -157,17 +122,12 @@ namespace SixLabors.ImageSharp.Tests.Drawing } [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(Create); } @@ -175,15 +135,9 @@ namespace SixLabors.ImageSharp.Tests.Drawing [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(Create); } @@ -191,17 +145,11 @@ namespace SixLabors.ImageSharp.Tests.Drawing [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(Create); } From 699b6597156a5ceb34a1a6be9e872b61e429406b Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 17 Sep 2019 15:37:50 +1000 Subject: [PATCH 16/22] Refactor Image.Dispose --- src/ImageSharp/IImage.cs | 4 ++-- src/ImageSharp/Image.cs | 27 +++++++++++-------------- src/ImageSharp/ImageExtensions.cs | 13 +----------- src/ImageSharp/ImageFrame.cs | 12 ++++++++++- src/ImageSharp/ImageFrame{TPixel}.cs | 16 +++++++-------- src/ImageSharp/Image{TPixel}.cs | 30 +++++++++++++++++++++++++--- 6 files changed, 61 insertions(+), 41 deletions(-) diff --git a/src/ImageSharp/IImage.cs b/src/ImageSharp/IImage.cs index b9e2cee616..0d4dc3c9d9 100644 --- a/src/ImageSharp/IImage.cs +++ b/src/ImageSharp/IImage.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; @@ -11,4 +11,4 @@ namespace SixLabors.ImageSharp public interface IImage : IImageInfo, IDisposable { } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Image.cs b/src/ImageSharp/Image.cs index 57f60f2e75..696f836628 100644 --- a/src/ImageSharp/Image.cs +++ b/src/ImageSharp/Image.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. +using System; using System.IO; using SixLabors.ImageSharp.Advanced; @@ -80,21 +81,11 @@ namespace SixLabors.ImageSharp /// Configuration IConfigurable.Configuration => this.Configuration; - /// - /// Gets a value indicating whether the image instance is disposed. - /// - public bool IsDisposed { get; private set; } - /// public void Dispose() { - if (this.IsDisposed) - { - return; - } - - this.IsDisposed = true; - this.DisposeImpl(); + this.Dispose(true); + GC.SuppressFinalize(this); } /// @@ -109,7 +100,7 @@ namespace SixLabors.ImageSharp Guard.NotNull(encoder, nameof(encoder)); this.EnsureNotDisposed(); - EncodeVisitor visitor = new EncodeVisitor(encoder, stream); + var visitor = new EncodeVisitor(encoder, stream); this.AcceptVisitor(visitor); } @@ -144,9 +135,15 @@ namespace SixLabors.ImageSharp protected void UpdateSize(Size size) => this.size = size; /// - /// Implements the Dispose logic. + /// Disposes the object and frees resources for the Garbage Collector. + /// + /// Whether to dispose of managed and unmanaged objects. + protected abstract void Dispose(bool disposing); + + /// + /// Throws if the image is disposed. /// - protected abstract void DisposeImpl(); + internal abstract void EnsureNotDisposed(); private class EncodeVisitor : IImageVisitor { diff --git a/src/ImageSharp/ImageExtensions.cs b/src/ImageSharp/ImageExtensions.cs index 6ea2b234c5..6cdc948d40 100644 --- a/src/ImageSharp/ImageExtensions.cs +++ b/src/ImageSharp/ImageExtensions.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; @@ -119,16 +119,5 @@ namespace SixLabors.ImageSharp return $"data:{format.DefaultMimeType};base64,{Convert.ToBase64String(stream.ToArray())}"; } } - - /// - /// Throws if the image is disposed. - /// - internal static void EnsureNotDisposed(this Image image) - { - if (image.IsDisposed) - { - throw new ObjectDisposedException(nameof(image), "Trying to execute an operation on a disposed image."); - } - } } } diff --git a/src/ImageSharp/ImageFrame.cs b/src/ImageSharp/ImageFrame.cs index f3fe1ed8d4..91872b21d6 100644 --- a/src/ImageSharp/ImageFrame.cs +++ b/src/ImageSharp/ImageFrame.cs @@ -74,7 +74,17 @@ namespace SixLabors.ImageSharp public Rectangle Bounds() => new Rectangle(0, 0, this.Width, this.Height); /// - public abstract void Dispose(); + public void Dispose() + { + this.Dispose(true); + GC.SuppressFinalize(this); + } + + /// + /// Disposes the object and frees resources for the Garbage Collector. + /// + /// Whether to dispose of managed and unmanaged objects. + protected abstract void Dispose(bool disposing); internal abstract void CopyPixelsTo(Span destination) where TDestinationPixel : struct, IPixel; diff --git a/src/ImageSharp/ImageFrame{TPixel}.cs b/src/ImageSharp/ImageFrame{TPixel}.cs index 5c9ff489e1..0436eb9d2b 100644 --- a/src/ImageSharp/ImageFrame{TPixel}.cs +++ b/src/ImageSharp/ImageFrame{TPixel}.cs @@ -21,7 +21,7 @@ namespace SixLabors.ImageSharp /// In all other cases it is the only frame of the image. /// /// The pixel format. - public sealed class ImageFrame : ImageFrame, IPixelSource, IDisposable + public sealed class ImageFrame : ImageFrame, IPixelSource where TPixel : struct, IPixel { private bool isDisposed; @@ -196,20 +196,20 @@ namespace SixLabors.ImageSharp this.UpdateSize(this.PixelBuffer.Size()); } - /// - /// Disposes the object and frees resources for the Garbage Collector. - /// - public override void Dispose() + /// + protected override void Dispose(bool disposing) { if (this.isDisposed) { return; } - this.PixelBuffer?.Dispose(); - this.PixelBuffer = null; + if (disposing) + { + this.PixelBuffer?.Dispose(); + this.PixelBuffer = null; + } - // Note disposing is done. this.isDisposed = true; } diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index a7ea58652c..6dbfde20c6 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; @@ -18,9 +18,11 @@ namespace SixLabors.ImageSharp /// For generic -s the pixel type is known at compile time. /// /// The pixel format. - public sealed class Image : Image + public class Image : Image where TPixel : struct, IPixel { + private bool isDisposed; + /// /// Initializes a new instance of the class /// with the height and the width of the image. @@ -185,7 +187,29 @@ namespace SixLabors.ImageSharp } /// - protected override void DisposeImpl() => this.Frames.Dispose(); + protected override void Dispose(bool disposing) + { + if (this.isDisposed) + { + return; + } + + if (disposing) + { + this.Frames.Dispose(); + } + + this.isDisposed = true; + } + + /// + internal override void EnsureNotDisposed() + { + if (this.isDisposed) + { + throw new ObjectDisposedException("Trying to execute an operation on a disposed image."); + } + } /// internal override void AcceptVisitor(IImageVisitor visitor) From 1efaba60c3d05204f7f3c13643b29ed90393630b Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 17 Sep 2019 15:45:58 +1000 Subject: [PATCH 17/22] Refactor IImageProcessor --- .../Processors/CloningImageProcessor{TPixel}.cs | 7 +------ .../Processing/Processors/ImageProcessor{TPixel}.cs | 8 ++------ 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs index 8b3e1eb965..c2c8690529 100644 --- a/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs @@ -15,8 +15,6 @@ namespace SixLabors.ImageSharp.Processing.Processors internal abstract class CloningImageProcessor : ICloningImageProcessor where TPixel : struct, IPixel { - private bool isDisposed; - /// /// Initializes a new instance of the class. /// @@ -104,6 +102,7 @@ namespace SixLabors.ImageSharp.Processing.Processors public void Dispose() { this.Dispose(true); + GC.SuppressFinalize(this); } /// @@ -160,10 +159,6 @@ namespace SixLabors.ImageSharp.Processing.Processors /// Whether to dispose managed and unmanaged objects. protected virtual void Dispose(bool disposing) { - if (!this.isDisposed) - { - this.isDisposed = true; - } } } } diff --git a/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs index 8ac8cd67bc..55b4d3dc73 100644 --- a/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs @@ -15,8 +15,6 @@ namespace SixLabors.ImageSharp.Processing.Processors internal abstract class ImageProcessor : IImageProcessor where TPixel : struct, IPixel { - private bool isDisposed; - /// /// Initializes a new instance of the class. /// @@ -97,6 +95,8 @@ namespace SixLabors.ImageSharp.Processing.Processors /// public virtual void Dispose() { + this.Dispose(true); + GC.SuppressFinalize(this); } /// @@ -142,10 +142,6 @@ namespace SixLabors.ImageSharp.Processing.Processors /// Whether to dispose managed and unmanaged objects. protected virtual void Dispose(bool disposing) { - if (!this.isDisposed) - { - this.isDisposed = true; - } } } } From b49f2ab3e8961097eec432dc6f31862be3635060 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 17 Sep 2019 16:45:05 +1000 Subject: [PATCH 18/22] Expose visitor through advanced namespace. --- .../Processors/Drawing/DrawImageProcessor.cs | 1 + .../Advanced/AdvancedImageExtensions.cs | 9 +++++ .../{ => Advanced}/IImageVisitor.cs | 10 +++--- src/ImageSharp/Image.cs | 8 ++--- src/ImageSharp/Image{TPixel}.cs | 4 +-- .../Extensions/ProcessingExtensions.cs | 33 ++++++++++++------- .../Processors/ImageProcessorExtensions.cs | 1 + 7 files changed, 43 insertions(+), 23 deletions(-) rename src/ImageSharp/{ => Advanced}/IImageVisitor.cs (64%) diff --git a/src/ImageSharp.Drawing/Processing/Processors/Drawing/DrawImageProcessor.cs b/src/ImageSharp.Drawing/Processing/Processors/Drawing/DrawImageProcessor.cs index dc55112c9c..e217fd9a6c 100644 --- a/src/ImageSharp.Drawing/Processing/Processors/Drawing/DrawImageProcessor.cs +++ b/src/ImageSharp.Drawing/Processing/Processors/Drawing/DrawImageProcessor.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. +using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.PixelFormats; using SixLabors.Primitives; diff --git a/src/ImageSharp/Advanced/AdvancedImageExtensions.cs b/src/ImageSharp/Advanced/AdvancedImageExtensions.cs index 4b1d4222cb..22e6d47e9a 100644 --- a/src/ImageSharp/Advanced/AdvancedImageExtensions.cs +++ b/src/ImageSharp/Advanced/AdvancedImageExtensions.cs @@ -15,6 +15,15 @@ namespace SixLabors.ImageSharp.Advanced /// public static class AdvancedImageExtensions { + /// + /// Accepts a to implement a double-dispatch pattern in order to + /// apply pixel-specific operations on non-generic instances + /// + /// The source. + /// The visitor. + public static void AcceptVisitor(this Image source, IImageVisitor visitor) + => source.Accept(visitor); + /// /// Gets the configuration for the image. /// diff --git a/src/ImageSharp/IImageVisitor.cs b/src/ImageSharp/Advanced/IImageVisitor.cs similarity index 64% rename from src/ImageSharp/IImageVisitor.cs rename to src/ImageSharp/Advanced/IImageVisitor.cs index 971c4d37cb..ba8b13e2e8 100644 --- a/src/ImageSharp/IImageVisitor.cs +++ b/src/ImageSharp/Advanced/IImageVisitor.cs @@ -3,13 +3,13 @@ using SixLabors.ImageSharp.PixelFormats; -namespace SixLabors.ImageSharp +namespace SixLabors.ImageSharp.Advanced { /// - /// A visitor to implement double-dispatch pattern in order to apply pixel-specific operations - /// on non-generic instances. The operation is dispatched by . + /// A visitor to implement a double-dispatch pattern in order to apply pixel-specific operations + /// on non-generic instances. /// - internal interface IImageVisitor + public interface IImageVisitor { /// /// Provides a pixel-specific implementation for a given operation. @@ -19,4 +19,4 @@ namespace SixLabors.ImageSharp void Visit(Image image) where TPixel : struct, IPixel; } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Image.cs b/src/ImageSharp/Image.cs index 57f60f2e75..dbdbfbd8f4 100644 --- a/src/ImageSharp/Image.cs +++ b/src/ImageSharp/Image.cs @@ -109,8 +109,7 @@ namespace SixLabors.ImageSharp Guard.NotNull(encoder, nameof(encoder)); this.EnsureNotDisposed(); - EncodeVisitor visitor = new EncodeVisitor(encoder, stream); - this.AcceptVisitor(visitor); + this.AcceptVisitor(new EncodeVisitor(encoder, stream)); } /// @@ -131,11 +130,12 @@ namespace SixLabors.ImageSharp where TPixel2 : struct, IPixel; /// - /// Accept a . + /// Accepts a . /// Implemented by invoking /// with the pixel type of the image. /// - internal abstract void AcceptVisitor(IImageVisitor visitor); + /// The visitor. + protected internal abstract void Accept(IImageVisitor visitor); /// /// Update the size of the image after mutation. diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index a7ea58652c..994a2c586a 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; @@ -188,7 +188,7 @@ namespace SixLabors.ImageSharp protected override void DisposeImpl() => this.Frames.Dispose(); /// - internal override void AcceptVisitor(IImageVisitor visitor) + protected internal override void Accept(IImageVisitor visitor) { this.EnsureNotDisposed(); diff --git a/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs b/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs index 48876251e2..40b1c439e6 100644 --- a/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs +++ b/src/ImageSharp/Processing/Extensions/ProcessingExtensions.cs @@ -25,8 +25,7 @@ namespace SixLabors.ImageSharp.Processing Guard.NotNull(operation, nameof(operation)); source.EnsureNotDisposed(); - var visitor = new ProcessingVisitor(operation, true); - source.AcceptVisitor(visitor); + source.AcceptVisitor(new ProcessingVisitor(operation, true)); } /// @@ -42,8 +41,10 @@ namespace SixLabors.ImageSharp.Processing Guard.NotNull(operation, nameof(operation)); source.EnsureNotDisposed(); - IInternalImageProcessingContext operationsRunner = source.GetConfiguration().ImageOperationsProvider - .CreateImageProcessingContext(source, true); + IInternalImageProcessingContext operationsRunner + = source.GetConfiguration() + .ImageOperationsProvider.CreateImageProcessingContext(source, true); + operation(operationsRunner); } @@ -60,8 +61,10 @@ namespace SixLabors.ImageSharp.Processing Guard.NotNull(operations, nameof(operations)); source.EnsureNotDisposed(); - IInternalImageProcessingContext operationsRunner = source.GetConfiguration().ImageOperationsProvider - .CreateImageProcessingContext(source, true); + IInternalImageProcessingContext operationsRunner + = source.GetConfiguration() + .ImageOperationsProvider.CreateImageProcessingContext(source, true); + operationsRunner.ApplyProcessors(operations); } @@ -96,8 +99,10 @@ namespace SixLabors.ImageSharp.Processing Guard.NotNull(operation, nameof(operation)); source.EnsureNotDisposed(); - IInternalImageProcessingContext operationsRunner = source.GetConfiguration().ImageOperationsProvider - .CreateImageProcessingContext(source, false); + IInternalImageProcessingContext operationsRunner + = source.GetConfiguration() + .ImageOperationsProvider.CreateImageProcessingContext(source, false); + operation(operationsRunner); return operationsRunner.GetResultImage(); } @@ -116,8 +121,10 @@ namespace SixLabors.ImageSharp.Processing Guard.NotNull(operations, nameof(operations)); source.EnsureNotDisposed(); - IInternalImageProcessingContext operationsRunner = source.GetConfiguration().ImageOperationsProvider - .CreateImageProcessingContext(source, false); + IInternalImageProcessingContext operationsRunner + = source.GetConfiguration() + .ImageOperationsProvider.CreateImageProcessingContext(source, false); + operationsRunner.ApplyProcessors(operations); return operationsRunner.GetResultImage(); } @@ -157,8 +164,10 @@ namespace SixLabors.ImageSharp.Processing public void Visit(Image image) where TPixel : struct, IPixel { - IInternalImageProcessingContext operationsRunner = image.GetConfiguration() - .ImageOperationsProvider.CreateImageProcessingContext(image, this.mutate); + IInternalImageProcessingContext operationsRunner = + image.GetConfiguration() + .ImageOperationsProvider.CreateImageProcessingContext(image, this.mutate); + this.operation(operationsRunner); this.ResultImage = operationsRunner.GetResultImage(); } diff --git a/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs b/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs index 53eedfd207..feb4c9f19d 100644 --- a/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs +++ b/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. +using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.PixelFormats; using SixLabors.Primitives; From d62fa6929beafa8a2d4c29cf5664fdf9c5d87b88 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 18 Sep 2019 00:06:40 +1000 Subject: [PATCH 19/22] Make processors public, refactor cloning. --- .../DrawImageProcessor{TPixelBg,TPixelFg}.cs | 16 ++--- .../DefaultImageProcessorContext{TPixel}.cs | 22 +++---- .../Processors/CloningImageProcessor.cs | 31 ++++++++++ .../CloningImageProcessor{TPixel}.cs | 45 +++++++++----- .../ICloningImageProcessor{TPixel}.cs | 15 ++--- .../Processing/Processors/IImageProcessor.cs | 2 +- .../Processors/IImageProcessor{TPixel}.cs | 11 +--- .../Processors/ImageProcessorExtensions.cs | 2 +- .../Processors/ImageProcessor{TPixel}.cs | 9 +-- .../Transforms/AffineTransformProcessor.cs | 13 ++-- .../Processors/Transforms/CropProcessor.cs | 11 ++-- .../ProjectiveTransformProcessor.cs | 11 +--- .../Transforms/Resize/ResizeProcessor.cs | 8 +-- .../Processors/Transforms/RotateProcessor.cs | 7 +-- .../Processors/Transforms/SkewProcessor.cs | 5 +- .../Processing/ImageProcessingContextTests.cs | 59 ++++++++++++------- 16 files changed, 149 insertions(+), 118 deletions(-) create mode 100644 src/ImageSharp/Processing/Processors/CloningImageProcessor.cs diff --git a/src/ImageSharp.Drawing/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs b/src/ImageSharp.Drawing/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs index 6cfa23cce6..76082136c7 100644 --- a/src/ImageSharp.Drawing/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs +++ b/src/ImageSharp.Drawing/Processing/Processors/Drawing/DrawImageProcessor{TPixelBg,TPixelFg}.cs @@ -91,7 +91,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Drawing var workingRect = Rectangle.FromLTRB(minX, minY, maxX, maxY); - // not a valid operation because rectangle does not overlap with this image. + // Not a valid operation because rectangle does not overlap with this image. if (workingRect.Width <= 0 || workingRect.Height <= 0) { throw new ImageProcessingException( @@ -102,14 +102,14 @@ namespace SixLabors.ImageSharp.Processing.Processors.Drawing workingRect, configuration, rows => + { + for (int y = rows.Min; y < rows.Max; y++) { - for (int y = rows.Min; y < rows.Max; y++) - { - Span background = source.GetPixelRowSpan(y).Slice(minX, width); - Span foreground = targetImage.GetPixelRowSpan(y - locationY).Slice(targetX, width); - blender.Blend(configuration, background, background, foreground, this.Opacity); - } - }); + Span background = source.GetPixelRowSpan(y).Slice(minX, width); + Span foreground = targetImage.GetPixelRowSpan(y - locationY).Slice(targetX, width); + blender.Blend(configuration, background, background, foreground, this.Opacity); + } + }); } } } diff --git a/src/ImageSharp/Processing/DefaultImageProcessorContext{TPixel}.cs b/src/ImageSharp/Processing/DefaultImageProcessorContext{TPixel}.cs index a6e3dc03a0..7f73166942 100644 --- a/src/ImageSharp/Processing/DefaultImageProcessorContext{TPixel}.cs +++ b/src/ImageSharp/Processing/DefaultImageProcessorContext{TPixel}.cs @@ -29,6 +29,8 @@ namespace SixLabors.ImageSharp.Processing { this.mutate = mutate; this.source = source; + + // Mutate acts upon the source image only. if (this.mutate) { this.destination = source; @@ -43,7 +45,8 @@ namespace SixLabors.ImageSharp.Processing { if (!this.mutate && this.destination is null) { - // Ensure we have cloned it if we are not mutating as we might have failed to register any processors + // Ensure we have cloned the source if we are not mutating as we might have failed + // to register any processors. this.destination = this.source.Clone(); } @@ -64,26 +67,25 @@ namespace SixLabors.ImageSharp.Processing { if (!this.mutate && this.destination is null) { - // This will only work if the first processor applied is the cloning one thus - // realistically for this optimization to work the resize must the first processor - // applied any only up processors will take the double data path. - using (IImageProcessor specificProcessor = processor.CreatePixelSpecificProcessor(this.source, rectangle)) + // When cloning an image we can optimize the processing pipeline by avoiding an unnecessary + // interim clone if the first processor in the pipeline is a cloning processor. + if (processor is CloningImageProcessor cloningImageProcessor) { - // TODO: if 'specificProcessor' is not an ICloningImageProcessor we are unnecessarily disposing and recreating it. - // This should be solved in a future refactor. - if (specificProcessor is ICloningImageProcessor cloningImageProcessor) + using (ICloningImageProcessor pixelProcessor = cloningImageProcessor.CreatePixelSpecificProcessor(this.source, rectangle)) { - this.destination = cloningImageProcessor.CloneAndApply(); + this.destination = pixelProcessor.CloneAndExecute(); return this; } } + // Not a cloning processor? We need to create a clone to operate on. this.destination = this.source.Clone(); } + // Standard processing pipeline. using (IImageProcessor specificProcessor = processor.CreatePixelSpecificProcessor(this.destination, rectangle)) { - specificProcessor.Apply(); + specificProcessor.Execute(); } return this; diff --git a/src/ImageSharp/Processing/Processors/CloningImageProcessor.cs b/src/ImageSharp/Processing/Processors/CloningImageProcessor.cs new file mode 100644 index 0000000000..6ab0fcb13b --- /dev/null +++ b/src/ImageSharp/Processing/Processors/CloningImageProcessor.cs @@ -0,0 +1,31 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +using SixLabors.ImageSharp.PixelFormats; +using SixLabors.Primitives; + +namespace SixLabors.ImageSharp.Processing.Processors +{ + /// + /// The base class for all cloning image processors. + /// + public abstract class CloningImageProcessor : IImageProcessor + { + /// + /// Creates a pixel specific that is capable of executing + /// the processing algorithm on an . + /// + /// The pixel type. + /// The source image. Cannot be null. + /// + /// The structure that specifies the portion of the image object to draw. + /// + /// The + public abstract ICloningImageProcessor CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) + where TPixel : struct, IPixel; + + /// + IImageProcessor IImageProcessor.CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) + => this.CreatePixelSpecificProcessor(source, sourceRectangle); + } +} diff --git a/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs index 8b3e1eb965..6b0329e772 100644 --- a/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs @@ -9,10 +9,12 @@ using SixLabors.Primitives; namespace SixLabors.ImageSharp.Processing.Processors { /// - /// Allows the application of processing algorithms to a clone of the original image. + /// The base class for all pixel specific cloning image processors. + /// Allows the application of processing algorithms to the image. + /// The image is cloned before operating upon and the buffers swapped upon completion. /// /// The pixel format. - internal abstract class CloningImageProcessor : ICloningImageProcessor + public abstract class CloningImageProcessor : ICloningImageProcessor where TPixel : struct, IPixel { private bool isDisposed; @@ -45,16 +47,12 @@ namespace SixLabors.ImageSharp.Processing.Processors protected Configuration Configuration { get; } /// - public Image CloneAndApply() + public Image CloneAndExecute() { try { Image clone = this.CreateDestination(); - - if (clone.Frames.Count != this.Source.Frames.Count) - { - throw new ImageProcessingException($"An error occurred when processing the image using {this.GetType().Name}. The processor changed the number of frames."); - } + this.CheckFrameCount(this.Source, clone); Configuration configuration = this.Source.GetConfiguration(); this.BeforeImageApply(clone); @@ -86,17 +84,24 @@ namespace SixLabors.ImageSharp.Processing.Processors } /// - public void Apply() + public void Execute() { - using (Image cloned = this.CloneAndApply()) + // Create an interim clone of the source image to operate on. + // Doing this allows for the application of transforms that will alter + // the dimensions of the image. + Image clone = default; + try { - // we now need to move the pixel data/size data from one image base to another - if (cloned.Frames.Count != this.Source.Frames.Count) - { - throw new ImageProcessingException($"An error occurred when processing the image using {this.GetType().Name}. The processor changed the number of frames."); - } + clone = this.CloneAndExecute(); - this.Source.SwapOrCopyPixelsBuffersFrom(cloned); + // We now need to move the pixel data/size data from the clone to the source. + this.CheckFrameCount(this.Source, clone); + this.Source.SwapOrCopyPixelsBuffersFrom(clone); + } + finally + { + // Dispose of the clone now that we have swapped the pixel/size data. + clone?.Dispose(); } } @@ -165,5 +170,13 @@ namespace SixLabors.ImageSharp.Processing.Processors this.isDisposed = true; } } + + private void CheckFrameCount(Image a, Image b) + { + if (a.Frames.Count != b.Frames.Count) + { + throw new ImageProcessingException($"An error occurred when processing the image using {this.GetType().Name}. The processor changed the number of frames."); + } + } } } diff --git a/src/ImageSharp/Processing/Processors/ICloningImageProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/ICloningImageProcessor{TPixel}.cs index 1a21be1f93..c34bf60ae8 100644 --- a/src/ImageSharp/Processing/Processors/ICloningImageProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/ICloningImageProcessor{TPixel}.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. using SixLabors.ImageSharp.PixelFormats; -using SixLabors.Primitives; namespace SixLabors.ImageSharp.Processing.Processors { @@ -10,19 +9,13 @@ namespace SixLabors.ImageSharp.Processing.Processors /// Encapsulates methods to alter the pixels of a new image, cloned from the original image. /// /// The pixel format. - internal interface ICloningImageProcessor : IImageProcessor + public interface ICloningImageProcessor : IImageProcessor where TPixel : struct, IPixel { /// - /// Applies the process to the specified portion of the specified . + /// Clones the specified and executes the process against the clone. /// - /// - /// The target is null. - /// - /// - /// The target doesn't fit the dimension of the image. - /// - /// Returns the cloned image after there processor has been applied to it. - Image CloneAndApply(); + /// The . + Image CloneAndExecute(); } } diff --git a/src/ImageSharp/Processing/Processors/IImageProcessor.cs b/src/ImageSharp/Processing/Processors/IImageProcessor.cs index 4fff5273ac..fb7a6a4d9d 100644 --- a/src/ImageSharp/Processing/Processors/IImageProcessor.cs +++ b/src/ImageSharp/Processing/Processors/IImageProcessor.cs @@ -15,7 +15,7 @@ namespace SixLabors.ImageSharp.Processing.Processors public interface IImageProcessor { /// - /// Creates a pixel specific that is capable for executing + /// Creates a pixel specific that is capable of executing /// the processing algorithm on an . /// /// The pixel type. diff --git a/src/ImageSharp/Processing/Processors/IImageProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/IImageProcessor{TPixel}.cs index 3d6e0d765e..1b874e4b94 100644 --- a/src/ImageSharp/Processing/Processors/IImageProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/IImageProcessor{TPixel}.cs @@ -3,7 +3,6 @@ using System; using SixLabors.ImageSharp.PixelFormats; -using SixLabors.Primitives; namespace SixLabors.ImageSharp.Processing.Processors { @@ -15,14 +14,8 @@ namespace SixLabors.ImageSharp.Processing.Processors where TPixel : struct, IPixel { /// - /// Applies the process to the specified portion of the specified . + /// Executes the process against the specified . /// - /// - /// The target is null. - /// - /// - /// The target doesn't fit the dimension of the image. - /// - void Apply(); + void Execute(); } } diff --git a/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs b/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs index 53eedfd207..9f41e08395 100644 --- a/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs +++ b/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs @@ -30,7 +30,7 @@ namespace SixLabors.ImageSharp.Processing.Processors { using (IImageProcessor processorImpl = this.processor.CreatePixelSpecificProcessor(image, this.sourceRectangle)) { - processorImpl.Apply(); + processorImpl.Execute(); } } } diff --git a/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs index 8ac8cd67bc..b224adc3f4 100644 --- a/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs @@ -9,10 +9,11 @@ using SixLabors.Primitives; namespace SixLabors.ImageSharp.Processing.Processors { /// - /// Allows the application of processors to images. + /// The base class for all pixel specific image processors. + /// Allows the application of processing algorithms to the image. /// /// The pixel format. - internal abstract class ImageProcessor : IImageProcessor + public abstract class ImageProcessor : IImageProcessor where TPixel : struct, IPixel { private bool isDisposed; @@ -45,7 +46,7 @@ namespace SixLabors.ImageSharp.Processing.Processors protected Configuration Configuration { get; } /// - public void Apply() + public void Execute() { try { @@ -71,7 +72,7 @@ namespace SixLabors.ImageSharp.Processing.Processors } /// - /// Applies the processor to just a single ImageBase. + /// Applies the processor to a single image frame. /// /// the source image. public void Apply(ImageFrame source) diff --git a/src/ImageSharp/Processing/Processors/Transforms/AffineTransformProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/AffineTransformProcessor.cs index 6e669e7779..be5675578e 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/AffineTransformProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/AffineTransformProcessor.cs @@ -2,8 +2,6 @@ // Licensed under the Apache License, Version 2.0. using System.Numerics; - -using SixLabors.ImageSharp.PixelFormats; using SixLabors.Primitives; namespace SixLabors.ImageSharp.Processing.Processors.Transforms @@ -11,7 +9,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// /// Defines an affine transformation applicable on an . /// - public class AffineTransformProcessor : IImageProcessor + public class AffineTransformProcessor : CloningImageProcessor { /// /// Initializes a new instance of the class. @@ -42,11 +40,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// public Size TargetDimensions { get; } - /// - public virtual IImageProcessor CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) - where TPixel : struct, IPixel - { - return new AffineTransformProcessor(this, source, sourceRectangle); - } + /// + public override ICloningImageProcessor CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) + => new AffineTransformProcessor(this, source, sourceRectangle); } } diff --git a/src/ImageSharp/Processing/Processors/Transforms/CropProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/CropProcessor.cs index 6105330df3..025592a369 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/CropProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/CropProcessor.cs @@ -1,7 +1,6 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. -using SixLabors.ImageSharp.PixelFormats; using SixLabors.Primitives; namespace SixLabors.ImageSharp.Processing.Processors.Transforms @@ -9,7 +8,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// /// Defines a crop operation on an image. /// - public sealed class CropProcessor : IImageProcessor + public sealed class CropProcessor : CloningImageProcessor { /// /// Initializes a new instance of the class. @@ -23,6 +22,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms new Rectangle(Point.Empty, sourceSize).Contains(cropRectangle), nameof(cropRectangle), "Crop rectangle should be smaller than the source bounds."); + this.CropRectangle = cropRectangle; } @@ -32,10 +32,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms public Rectangle CropRectangle { get; } /// - public IImageProcessor CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) - where TPixel : struct, IPixel - { - return new CropProcessor(this, source, sourceRectangle); - } + public override ICloningImageProcessor CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) + => new CropProcessor(this, source, sourceRectangle); } } diff --git a/src/ImageSharp/Processing/Processors/Transforms/ProjectiveTransformProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/ProjectiveTransformProcessor.cs index 15a6e2d095..babdee593a 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/ProjectiveTransformProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/ProjectiveTransformProcessor.cs @@ -2,8 +2,6 @@ // Licensed under the Apache License, Version 2.0. using System.Numerics; - -using SixLabors.ImageSharp.PixelFormats; using SixLabors.Primitives; namespace SixLabors.ImageSharp.Processing.Processors.Transforms @@ -11,7 +9,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// /// Defines a projective transformation applicable to an . /// - public sealed class ProjectiveTransformProcessor : IImageProcessor + public sealed class ProjectiveTransformProcessor : CloningImageProcessor { /// /// Initializes a new instance of the class. @@ -43,10 +41,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms public Size TargetDimensions { get; } /// - public IImageProcessor CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) - where TPixel : struct, IPixel - { - return new ProjectiveTransformProcessor(this, source, sourceRectangle); - } + public override ICloningImageProcessor CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) + => new ProjectiveTransformProcessor(this, source, sourceRectangle); } } diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs index 35e22757c1..5390fa4a4a 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs @@ -1,9 +1,6 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. -using System; - -using SixLabors.ImageSharp.PixelFormats; using SixLabors.Primitives; namespace SixLabors.ImageSharp.Processing.Processors.Transforms @@ -11,7 +8,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// /// Defines an image resizing operation with the given and dimensional parameters. /// - public class ResizeProcessor : IImageProcessor + public class ResizeProcessor : CloningImageProcessor { /// /// Initializes a new instance of the class. @@ -58,8 +55,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms public bool Compand { get; } /// - public IImageProcessor CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) - where TPixel : struct, IPixel + public override ICloningImageProcessor CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) => new ResizeProcessor(this, source, sourceRectangle); } } diff --git a/src/ImageSharp/Processing/Processors/Transforms/RotateProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/RotateProcessor.cs index 10d6cdc943..277cc05b22 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/RotateProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/RotateProcessor.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. using System.Numerics; - using SixLabors.Primitives; namespace SixLabors.ImageSharp.Processing.Processors.Transforms @@ -47,9 +46,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms public float Degrees { get; } /// - public override IImageProcessor CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) - { - return new RotateProcessor(this, source, sourceRectangle); - } + public override ICloningImageProcessor CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) + => new RotateProcessor(this, source, sourceRectangle); } } diff --git a/src/ImageSharp/Processing/Processors/Transforms/SkewProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/SkewProcessor.cs index 4b87d6d2c1..fb2114e03c 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/SkewProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/SkewProcessor.cs @@ -1,8 +1,7 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System.Numerics; - using SixLabors.Primitives; namespace SixLabors.ImageSharp.Processing.Processors.Transforms @@ -56,4 +55,4 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// public float DegreesY { get; } } -} \ No newline at end of file +} diff --git a/tests/ImageSharp.Tests/Processing/ImageProcessingContextTests.cs b/tests/ImageSharp.Tests/Processing/ImageProcessingContextTests.cs index 589d595275..afb2cbecd2 100644 --- a/tests/ImageSharp.Tests/Processing/ImageProcessingContextTests.cs +++ b/tests/ImageSharp.Tests/Processing/ImageProcessingContextTests.cs @@ -21,6 +21,8 @@ namespace SixLabors.ImageSharp.Tests.Processing private readonly Mock processorDefinition; + private readonly Mock cloningProcessorDefinition; + private readonly Mock> regularProcessorImpl; private readonly Mock> cloningProcessorImpl; @@ -30,18 +32,20 @@ namespace SixLabors.ImageSharp.Tests.Processing public ImageProcessingContextTests() { this.processorDefinition = new Mock(); + this.cloningProcessorDefinition = new Mock(); this.regularProcessorImpl = new Mock>(); this.cloningProcessorImpl = new Mock>(); } // bool throwException, bool useBounds public static readonly TheoryData ProcessorTestData = new TheoryData() - { - { false, false }, - { false, true }, - { true, false }, - { true, true } - }; + { + { false, false }, + { false, true }, + { true, false }, + { true, true } + }; + [Theory] [MemberData(nameof(ProcessorTestData))] public void Mutate_RegularProcessor(bool throwException, bool useBounds) @@ -57,7 +61,7 @@ namespace SixLabors.ImageSharp.Tests.Processing this.MutateApply(useBounds); } - this.regularProcessorImpl.Verify(p => p.Apply(), Times.Once()); + this.regularProcessorImpl.Verify(p => p.Execute(), Times.Once()); this.regularProcessorImpl.Verify(p => p.Dispose(), Times.Once()); } @@ -69,16 +73,15 @@ namespace SixLabors.ImageSharp.Tests.Processing if (throwException) { - Assert.Throws(() => this.CloneApply(useBounds)); + Assert.Throws(() => this.CloneRegularApply(useBounds)); } else { - this.CloneApply(useBounds); + this.CloneRegularApply(useBounds); } - // TODO: This should be Times.Once(). See comments in DefaultImageProcessingContext.ApplyProcessor() - this.regularProcessorImpl.Verify(p => p.Apply(), Times.AtLeast(1)); - this.regularProcessorImpl.Verify(p => p.Dispose(), Times.AtLeast(1)); + this.regularProcessorImpl.Verify(p => p.Execute(), Times.Once); + this.regularProcessorImpl.Verify(p => p.Dispose(), Times.Once); } [Theory] @@ -96,7 +99,7 @@ namespace SixLabors.ImageSharp.Tests.Processing this.MutateApply(useBounds); } - this.cloningProcessorImpl.Verify(p => p.Apply(), Times.Once()); + this.cloningProcessorImpl.Verify(p => p.Execute(), Times.Once()); this.cloningProcessorImpl.Verify(p => p.Dispose(), Times.Once()); } @@ -108,14 +111,14 @@ namespace SixLabors.ImageSharp.Tests.Processing if (throwException) { - Assert.Throws(() => this.CloneApply(useBounds)); + Assert.Throws(() => this.CloneCloneApply(useBounds)); } else { - this.CloneApply(useBounds); + this.CloneCloneApply(useBounds); } - this.cloningProcessorImpl.Verify(p => p.CloneAndApply(), Times.Once()); + this.cloningProcessorImpl.Verify(p => p.CloneAndExecute(), Times.Once()); this.cloningProcessorImpl.Verify(p => p.Dispose(), Times.Once()); } @@ -131,7 +134,7 @@ namespace SixLabors.ImageSharp.Tests.Processing } } - private void CloneApply(bool useBounds) + private void CloneRegularApply(bool useBounds) { if (useBounds) { @@ -143,11 +146,23 @@ namespace SixLabors.ImageSharp.Tests.Processing } } + private void CloneCloneApply(bool useBounds) + { + if (useBounds) + { + this.image.Clone(c => c.ApplyProcessor(this.cloningProcessorDefinition.Object, Bounds)).Dispose(); + } + else + { + this.image.Clone(c => c.ApplyProcessor(this.cloningProcessorDefinition.Object)).Dispose(); + } + } + private void SetupRegularProcessor(bool throwsException) { if (throwsException) { - this.regularProcessorImpl.Setup(p => p.Apply()).Throws(new ImageProcessingException("Test")); + this.regularProcessorImpl.Setup(p => p.Execute()).Throws(new ImageProcessingException("Test")); } this.processorDefinition @@ -159,13 +174,17 @@ namespace SixLabors.ImageSharp.Tests.Processing { if (throwsException) { - this.cloningProcessorImpl.Setup(p => p.Apply()).Throws(new ImageProcessingException("Test")); - this.cloningProcessorImpl.Setup(p => p.CloneAndApply()).Throws(new ImageProcessingException("Test")); + this.cloningProcessorImpl.Setup(p => p.Execute()).Throws(new ImageProcessingException("Test")); + this.cloningProcessorImpl.Setup(p => p.CloneAndExecute()).Throws(new ImageProcessingException("Test")); } this.processorDefinition .Setup(p => p.CreatePixelSpecificProcessor(It.IsAny>(), It.IsAny())) .Returns(this.cloningProcessorImpl.Object); + + this.cloningProcessorDefinition + .Setup(p => p.CreatePixelSpecificProcessor(It.IsAny>(), It.IsAny())) + .Returns(this.cloningProcessorImpl.Object); } } } From 24cd39a62fdfd3674437cf0a0aa968d42c605b41 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 18 Sep 2019 08:36:04 +1000 Subject: [PATCH 20/22] reseal Image{TPixel} --- src/ImageSharp/Image{TPixel}.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index 6dbfde20c6..181e818ee7 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -18,7 +18,7 @@ namespace SixLabors.ImageSharp /// For generic -s the pixel type is known at compile time. /// /// The pixel format. - public class Image : Image + public sealed class Image : Image where TPixel : struct, IPixel { private bool isDisposed; From 985cfbda087c98a5913289d4ecb55c3ba63a71b3 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 18 Sep 2019 08:45:03 +1000 Subject: [PATCH 21/22] Remove protected. --- src/ImageSharp/Image.cs | 16 ++++++++-------- src/ImageSharp/Image{TPixel}.cs | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/ImageSharp/Image.cs b/src/ImageSharp/Image.cs index dbdbfbd8f4..9030ae6afd 100644 --- a/src/ImageSharp/Image.cs +++ b/src/ImageSharp/Image.cs @@ -129,14 +129,6 @@ namespace SixLabors.ImageSharp public abstract Image CloneAs(Configuration configuration) where TPixel2 : struct, IPixel; - /// - /// Accepts a . - /// Implemented by invoking - /// with the pixel type of the image. - /// - /// The visitor. - protected internal abstract void Accept(IImageVisitor visitor); - /// /// Update the size of the image after mutation. /// @@ -148,6 +140,14 @@ namespace SixLabors.ImageSharp /// protected abstract void DisposeImpl(); + /// + /// Accepts a . + /// Implemented by invoking + /// with the pixel type of the image. + /// + /// The visitor. + internal abstract void Accept(IImageVisitor visitor); + private class EncodeVisitor : IImageVisitor { private readonly IImageEncoder encoder; diff --git a/src/ImageSharp/Image{TPixel}.cs b/src/ImageSharp/Image{TPixel}.cs index 994a2c586a..3d92c3be3d 100644 --- a/src/ImageSharp/Image{TPixel}.cs +++ b/src/ImageSharp/Image{TPixel}.cs @@ -187,17 +187,17 @@ namespace SixLabors.ImageSharp /// protected override void DisposeImpl() => this.Frames.Dispose(); + /// + public override string ToString() => $"Image<{typeof(TPixel).Name}>: {this.Width}x{this.Height}"; + /// - protected internal override void Accept(IImageVisitor visitor) + internal override void Accept(IImageVisitor visitor) { this.EnsureNotDisposed(); visitor.Visit(this); } - /// - public override string ToString() => $"Image<{typeof(TPixel).Name}>: {this.Width}x{this.Height}"; - /// /// Switches the buffers used by the image and the pixelSource meaning that the Image will "own" the buffer from the pixelSource and the pixelSource will now own the Images buffer. /// From 40b69d7e9563fc361b874874b0eb3ef76f1871e1 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 20 Sep 2019 00:24:50 +1000 Subject: [PATCH 22/22] Refactor cloning processors and tests --- src/ImageSharp/Advanced/AotCompilerTools.cs | 36 +++---- .../DefaultImageProcessorContext{TPixel}.cs | 4 +- .../Processors/CloningImageProcessor.cs | 17 +-- .../CloningImageProcessor{TPixel}.cs | 35 ++++-- .../EdgeDetector2DProcessor{TPixel}.cs | 2 +- .../EdgeDetectorCompassProcessor{TPixel}.cs | 2 +- .../EdgeDetectorProcessor{TPixel}.cs | 2 +- .../Filters/LomographProcessor{TPixel}.cs | 2 +- .../Filters/PolaroidProcessor{TPixel}.cs | 4 +- .../Processors/ICloningImageProcessor.cs | 27 +++++ .../ICloningImageProcessor{TPixel}.cs | 2 +- .../Processors/ImageProcessorExtensions.cs | 17 +-- .../Processors/ImageProcessor{TPixel}.cs | 2 +- .../Transforms/AffineTransformProcessor.cs | 2 +- .../AffineTransformProcessor{TPixel}.cs | 55 ++++------ .../Transforms/AutoOrientProcessor{TPixel}.cs | 18 ++-- .../Processors/Transforms/CropProcessor.cs | 2 +- .../Transforms/CropProcessor{TPixel}.cs | 46 +++----- .../EntropyCropProcessor{TPixel}.cs | 6 +- .../ProjectiveTransformProcessor.cs | 2 +- .../ProjectiveTransformProcessor{TPixel}.cs | 56 ++++------ .../Transforms/Resize/ResizeProcessor.cs | 2 +- .../Resize/ResizeProcessor{TPixel}.cs | 101 +++++------------- .../Processors/Transforms/RotateProcessor.cs | 2 +- .../Drawing/FillRegionProcessorTests.cs | 4 +- .../Processing/ImageProcessingContextTests.cs | 32 ++++-- 26 files changed, 221 insertions(+), 259 deletions(-) create mode 100644 src/ImageSharp/Processing/Processors/ICloningImageProcessor.cs diff --git a/src/ImageSharp/Advanced/AotCompilerTools.cs b/src/ImageSharp/Advanced/AotCompilerTools.cs index 8d3a074b5f..1ceba5f90e 100644 --- a/src/ImageSharp/Advanced/AotCompilerTools.cs +++ b/src/ImageSharp/Advanced/AotCompilerTools.cs @@ -2,13 +2,12 @@ // Licensed under the Apache License, Version 2.0. using System.Numerics; +using System.Runtime.CompilerServices; using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Jpeg.Components; using SixLabors.ImageSharp.PixelFormats; -using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Processing.Processors.Dithering; using SixLabors.ImageSharp.Processing.Processors.Quantization; -using SixLabors.ImageSharp.Processing.Processors.Transforms; namespace SixLabors.ImageSharp.Advanced { @@ -81,9 +80,8 @@ namespace SixLabors.ImageSharp.Advanced AotCompileWuQuantizer(); AotCompileDithering(); AotCompilePixelOperations(); - AotCompileResizeOperations(); - System.Runtime.CompilerServices.Unsafe.SizeOf(); + Unsafe.SizeOf(); AotCodec(new Formats.Png.PngDecoder(), new Formats.Png.PngEncoder()); AotCodec(new Formats.Bmp.BmpDecoder(), new Formats.Bmp.BmpEncoder()); @@ -107,8 +105,10 @@ namespace SixLabors.ImageSharp.Advanced private static void AotCompileOctreeQuantizer() where TPixel : struct, IPixel { - var test = new OctreeFrameQuantizer(new OctreeQuantizer(false)); - test.AotGetPalette(); + using (var test = new OctreeFrameQuantizer(new OctreeQuantizer(false))) + { + test.AotGetPalette(); + } } /// @@ -118,9 +118,11 @@ namespace SixLabors.ImageSharp.Advanced private static void AotCompileWuQuantizer() where TPixel : struct, IPixel { - var test = new WuFrameQuantizer(Configuration.Default.MemoryAllocator, new WuQuantizer(false)); - test.QuantizeFrame(new ImageFrame(Configuration.Default, 1, 1)); - test.AotGetPalette(); + using (var test = new WuFrameQuantizer(Configuration.Default.MemoryAllocator, new WuQuantizer(false))) + { + test.QuantizeFrame(new ImageFrame(Configuration.Default, 1, 1)); + test.AotGetPalette(); + } } /// @@ -132,7 +134,10 @@ namespace SixLabors.ImageSharp.Advanced { var test = new FloydSteinbergDiffuser(); TPixel pixel = default; - test.Dither(new ImageFrame(Configuration.Default, 1, 1), pixel, pixel, 0, 0, 0, 0, 0, 0); + using (var image = new ImageFrame(Configuration.Default, 1, 1)) + { + test.Dither(image, pixel, pixel, 0, 0, 0, 0, 0, 0); + } } /// @@ -171,16 +176,5 @@ namespace SixLabors.ImageSharp.Advanced var pixelOp = new PixelOperations(); pixelOp.GetPixelBlender(PixelColorBlendingMode.Normal, PixelAlphaCompositionMode.Clear); } - - /// - /// This method pre-seeds the ResizeProcessor for the AoT compiler on iOS. - /// - /// The pixel format. - private static void AotCompileResizeOperations() - where TPixel : struct, IPixel - { - var genericResizeProcessor = (ResizeProcessor)new ResizeProcessor(new ResizeOptions(), default).CreatePixelSpecificProcessor(new Image(0, 0), default); - genericResizeProcessor.AotCreateDestination(); - } } } diff --git a/src/ImageSharp/Processing/DefaultImageProcessorContext{TPixel}.cs b/src/ImageSharp/Processing/DefaultImageProcessorContext{TPixel}.cs index 7f73166942..328ccdf941 100644 --- a/src/ImageSharp/Processing/DefaultImageProcessorContext{TPixel}.cs +++ b/src/ImageSharp/Processing/DefaultImageProcessorContext{TPixel}.cs @@ -69,9 +69,9 @@ namespace SixLabors.ImageSharp.Processing { // When cloning an image we can optimize the processing pipeline by avoiding an unnecessary // interim clone if the first processor in the pipeline is a cloning processor. - if (processor is CloningImageProcessor cloningImageProcessor) + if (processor is ICloningImageProcessor cloningImageProcessor) { - using (ICloningImageProcessor pixelProcessor = cloningImageProcessor.CreatePixelSpecificProcessor(this.source, rectangle)) + using (ICloningImageProcessor pixelProcessor = cloningImageProcessor.CreatePixelSpecificCloningProcessor(this.source, rectangle)) { this.destination = pixelProcessor.CloneAndExecute(); return this; diff --git a/src/ImageSharp/Processing/Processors/CloningImageProcessor.cs b/src/ImageSharp/Processing/Processors/CloningImageProcessor.cs index 6ab0fcb13b..5e9ca2e542 100644 --- a/src/ImageSharp/Processing/Processors/CloningImageProcessor.cs +++ b/src/ImageSharp/Processing/Processors/CloningImageProcessor.cs @@ -9,23 +9,14 @@ namespace SixLabors.ImageSharp.Processing.Processors /// /// The base class for all cloning image processors. /// - public abstract class CloningImageProcessor : IImageProcessor + public abstract class CloningImageProcessor : ICloningImageProcessor { - /// - /// Creates a pixel specific that is capable of executing - /// the processing algorithm on an . - /// - /// The pixel type. - /// The source image. Cannot be null. - /// - /// The structure that specifies the portion of the image object to draw. - /// - /// The - public abstract ICloningImageProcessor CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) + /// + public abstract ICloningImageProcessor CreatePixelSpecificCloningProcessor(Image source, Rectangle sourceRectangle) where TPixel : struct, IPixel; /// IImageProcessor IImageProcessor.CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) - => this.CreatePixelSpecificProcessor(source, sourceRectangle); + => this.CreatePixelSpecificCloningProcessor(source, sourceRectangle); } } diff --git a/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs index 1290a1032b..42d2f0e1df 100644 --- a/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/CloningImageProcessor{TPixel}.cs @@ -2,6 +2,8 @@ // Licensed under the Apache License, Version 2.0. using System; +using System.Collections.Generic; +using System.Linq; using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.PixelFormats; using SixLabors.Primitives; @@ -40,16 +42,16 @@ namespace SixLabors.ImageSharp.Processing.Processors protected Rectangle SourceRectangle { get; } /// - /// Gets the instance to use when performing operations. + /// Gets the instance to use when performing operations. /// protected Configuration Configuration { get; } /// - public Image CloneAndExecute() + Image ICloningImageProcessor.CloneAndExecute() { try { - Image clone = this.CreateDestination(); + Image clone = this.CreateTarget(); this.CheckFrameCount(this.Source, clone); Configuration configuration = this.Source.GetConfiguration(); @@ -82,7 +84,7 @@ namespace SixLabors.ImageSharp.Processing.Processors } /// - public void Execute() + void IImageProcessor.Execute() { // Create an interim clone of the source image to operate on. // Doing this allows for the application of transforms that will alter @@ -90,7 +92,7 @@ namespace SixLabors.ImageSharp.Processing.Processors Image clone = default; try { - clone = this.CloneAndExecute(); + clone = ((ICloningImageProcessor)this).CloneAndExecute(); // We now need to move the pixel data/size data from the clone to the source. this.CheckFrameCount(this.Source, clone); @@ -111,10 +113,10 @@ namespace SixLabors.ImageSharp.Processing.Processors } /// - /// Generates a deep clone of the source image that operations should be applied to. + /// Gets the size of the target image. /// - /// The cloned image. - protected virtual Image CreateDestination() => this.Source.Clone(); + /// The . + protected abstract Size GetTargetSize(); /// /// This method is called before the process is applied to prepare the processor. @@ -166,6 +168,23 @@ namespace SixLabors.ImageSharp.Processing.Processors { } + private Image CreateTarget() + { + Image source = this.Source; + Size targetSize = this.GetTargetSize(); + + // We will always be creating the clone even for mutate because we may need to resize the canvas + IEnumerable> frames = source.Frames.Select, ImageFrame>( + x => new ImageFrame( + source.GetConfiguration(), + targetSize.Width, + targetSize.Height, + x.Metadata.DeepClone())); + + // Use the overload to prevent an extra frame being added + return new Image(this.Configuration, source.Metadata.DeepClone(), frames); + } + private void CheckFrameCount(Image a, Image b) { if (a.Frames.Count != b.Frames.Count) diff --git a/src/ImageSharp/Processing/Processors/Convolution/EdgeDetector2DProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/EdgeDetector2DProcessor{TPixel}.cs index 7b070f99a3..8358abe7df 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/EdgeDetector2DProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/EdgeDetector2DProcessor{TPixel}.cs @@ -54,7 +54,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution { if (this.Grayscale) { - new GrayscaleBt709Processor(1F).Apply(this.Source, this.SourceRectangle); + new GrayscaleBt709Processor(1F).Execute(this.Source, this.SourceRectangle); } base.BeforeImageApply(); diff --git a/src/ImageSharp/Processing/Processors/Convolution/EdgeDetectorCompassProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/EdgeDetectorCompassProcessor{TPixel}.cs index b7119ef44b..dc9974c616 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/EdgeDetectorCompassProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/EdgeDetectorCompassProcessor{TPixel}.cs @@ -45,7 +45,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution { if (this.Grayscale) { - new GrayscaleBt709Processor(1F).Apply(this.Source, this.SourceRectangle); + new GrayscaleBt709Processor(1F).Execute(this.Source, this.SourceRectangle); } base.BeforeImageApply(); diff --git a/src/ImageSharp/Processing/Processors/Convolution/EdgeDetectorProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/EdgeDetectorProcessor{TPixel}.cs index fc762cf1bb..5246dc3b72 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/EdgeDetectorProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/EdgeDetectorProcessor{TPixel}.cs @@ -41,7 +41,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution { if (this.Grayscale) { - new GrayscaleBt709Processor(1F).Apply(this.Source, this.SourceRectangle); + new GrayscaleBt709Processor(1F).Execute(this.Source, this.SourceRectangle); } base.BeforeImageApply(); diff --git a/src/ImageSharp/Processing/Processors/Filters/LomographProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Filters/LomographProcessor{TPixel}.cs index ab832a2759..7d3a5bbc0a 100644 --- a/src/ImageSharp/Processing/Processors/Filters/LomographProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Filters/LomographProcessor{TPixel}.cs @@ -29,7 +29,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Filters /// protected override void AfterImageApply() { - new VignetteProcessor(VeryDarkGreen).Apply(this.Source, this.SourceRectangle); + new VignetteProcessor(VeryDarkGreen).Execute(this.Source, this.SourceRectangle); base.AfterImageApply(); } } diff --git a/src/ImageSharp/Processing/Processors/Filters/PolaroidProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Filters/PolaroidProcessor{TPixel}.cs index 0be5bbb0de..f7ab1a1ec2 100644 --- a/src/ImageSharp/Processing/Processors/Filters/PolaroidProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Filters/PolaroidProcessor{TPixel}.cs @@ -31,8 +31,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Filters /// protected override void AfterImageApply() { - new VignetteProcessor(VeryDarkOrange).Apply(this.Source, this.SourceRectangle); - new GlowProcessor(LightOrange, this.Source.Width / 4F).Apply(this.Source, this.SourceRectangle); + new VignetteProcessor(VeryDarkOrange).Execute(this.Source, this.SourceRectangle); + new GlowProcessor(LightOrange, this.Source.Width / 4F).Execute(this.Source, this.SourceRectangle); base.AfterImageApply(); } } diff --git a/src/ImageSharp/Processing/Processors/ICloningImageProcessor.cs b/src/ImageSharp/Processing/Processors/ICloningImageProcessor.cs new file mode 100644 index 0000000000..554a4b8860 --- /dev/null +++ b/src/ImageSharp/Processing/Processors/ICloningImageProcessor.cs @@ -0,0 +1,27 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +using SixLabors.ImageSharp.PixelFormats; +using SixLabors.Primitives; + +namespace SixLabors.ImageSharp.Processing.Processors +{ + /// + /// Defines an algorithm to alter the pixels of a cloned image. + /// + public interface ICloningImageProcessor : IImageProcessor + { + /// + /// Creates a pixel specific that is capable of executing + /// the processing algorithm on an . + /// + /// The pixel type. + /// The source image. Cannot be null. + /// + /// The structure that specifies the portion of the image object to draw. + /// + /// The + ICloningImageProcessor CreatePixelSpecificCloningProcessor(Image source, Rectangle sourceRectangle) + where TPixel : struct, IPixel; + } +} diff --git a/src/ImageSharp/Processing/Processors/ICloningImageProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/ICloningImageProcessor{TPixel}.cs index c34bf60ae8..84b1262299 100644 --- a/src/ImageSharp/Processing/Processors/ICloningImageProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/ICloningImageProcessor{TPixel}.cs @@ -6,7 +6,7 @@ using SixLabors.ImageSharp.PixelFormats; namespace SixLabors.ImageSharp.Processing.Processors { /// - /// Encapsulates methods to alter the pixels of a new image, cloned from the original image. + /// Implements an algorithm to alter the pixels of a cloned image. /// /// The pixel format. public interface ICloningImageProcessor : IImageProcessor diff --git a/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs b/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs index 19e594a324..ce8ed813b5 100644 --- a/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs +++ b/src/ImageSharp/Processing/Processors/ImageProcessorExtensions.cs @@ -9,18 +9,21 @@ namespace SixLabors.ImageSharp.Processing.Processors { internal static class ImageProcessorExtensions { - public static void Apply(this IImageProcessor processor, Image source, Rectangle sourceRectangle) - { - source.AcceptVisitor(new ApplyVisitor(processor, sourceRectangle)); - } + /// + /// Executes the processor against the given source image and rectangle bounds. + /// + /// The processor. + /// The source image. + /// The source bounds. + public static void Execute(this IImageProcessor processor, Image source, Rectangle sourceRectangle) + => source.AcceptVisitor(new ExecuteVisitor(processor, sourceRectangle)); - private class ApplyVisitor : IImageVisitor + private class ExecuteVisitor : IImageVisitor { private readonly IImageProcessor processor; - private readonly Rectangle sourceRectangle; - public ApplyVisitor(IImageProcessor processor, Rectangle sourceRectangle) + public ExecuteVisitor(IImageProcessor processor, Rectangle sourceRectangle) { this.processor = processor; this.sourceRectangle = sourceRectangle; diff --git a/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs index eb1dc4ba0f..3e46e3c087 100644 --- a/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/ImageProcessor{TPixel}.cs @@ -44,7 +44,7 @@ namespace SixLabors.ImageSharp.Processing.Processors protected Configuration Configuration { get; } /// - public void Execute() + void IImageProcessor.Execute() { try { diff --git a/src/ImageSharp/Processing/Processors/Transforms/AffineTransformProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/AffineTransformProcessor.cs index be5675578e..6ca844fae6 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/AffineTransformProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/AffineTransformProcessor.cs @@ -41,7 +41,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms public Size TargetDimensions { get; } /// - public override ICloningImageProcessor CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) + public override ICloningImageProcessor CreatePixelSpecificCloningProcessor(Image source, Rectangle sourceRectangle) => new AffineTransformProcessor(this, source, sourceRectangle); } } diff --git a/src/ImageSharp/Processing/Processors/Transforms/AffineTransformProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/AffineTransformProcessor{TPixel}.cs index 7c50c04f3a..97b8b009b5 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/AffineTransformProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/AffineTransformProcessor{TPixel}.cs @@ -2,10 +2,7 @@ // Licensed under the Apache License, Version 2.0. using System; -using System.Collections.Generic; -using System.Linq; using System.Numerics; - using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.ParallelUtils; using SixLabors.ImageSharp.PixelFormats; @@ -20,6 +17,10 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms internal class AffineTransformProcessor : TransformProcessor where TPixel : struct, IPixel { + private Size targetSize; + private Matrix3x2 transformMatrix; + private readonly IResampler resampler; + /// /// Initializes a new instance of the class. /// @@ -29,50 +30,37 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms public AffineTransformProcessor(AffineTransformProcessor definition, Image source, Rectangle sourceRectangle) : base(source, sourceRectangle) { - this.Definition = definition; + this.targetSize = definition.TargetDimensions; + this.transformMatrix = definition.TransformMatrix; + this.resampler = definition.Sampler; } - protected AffineTransformProcessor Definition { get; } - - private Size TargetDimensions => this.Definition.TargetDimensions; - - private Matrix3x2 TransformMatrix => this.Definition.TransformMatrix; - - /// - protected override Image CreateDestination() - { - // We will always be creating the clone even for mutate because we may need to resize the canvas - IEnumerable> frames = this.Source.Frames.Select, ImageFrame>( - x => new ImageFrame(this.Configuration, this.TargetDimensions, x.Metadata.DeepClone())); - - // Use the overload to prevent an extra frame being added - return new Image(this.Configuration, this.Source.Metadata.DeepClone(), frames); - } + protected override Size GetTargetSize() => this.targetSize; /// protected override void OnFrameApply(ImageFrame source, ImageFrame destination) { // Handle transforms that result in output identical to the original. - if (this.TransformMatrix.Equals(default) || this.TransformMatrix.Equals(Matrix3x2.Identity)) + if (this.transformMatrix.Equals(default) || this.transformMatrix.Equals(Matrix3x2.Identity)) { // The clone will be blank here copy all the pixel data over source.GetPixelSpan().CopyTo(destination.GetPixelSpan()); return; } - int width = this.TargetDimensions.Width; - var targetBounds = new Rectangle(Point.Empty, this.TargetDimensions); + int width = this.targetSize.Width; + Rectangle sourceBounds = this.SourceRectangle; + var targetBounds = new Rectangle(Point.Empty, this.targetSize); + Configuration configuration = this.Configuration; // Convert from screen to world space. - Matrix3x2.Invert(this.TransformMatrix, out Matrix3x2 matrix); + Matrix3x2.Invert(this.transformMatrix, out Matrix3x2 matrix); - IResampler sampler = this.Definition.Sampler; - - if (sampler is NearestNeighborResampler) + if (this.resampler is NearestNeighborResampler) { ParallelHelper.IterateRows( targetBounds, - this.Configuration, + configuration, rows => { for (int y = rows.Min; y < rows.Max; y++) @@ -82,7 +70,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms for (int x = 0; x < width; x++) { var point = Point.Transform(new Point(x, y), matrix); - if (this.SourceRectangle.Contains(point.X, point.Y)) + if (sourceBounds.Contains(point.X, point.Y)) { destRow[x] = source[point.X, point.Y]; } @@ -93,19 +81,20 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms return; } - var kernel = new TransformKernelMap(this.Configuration, source.Size(), destination.Size(), sampler); + var kernel = new TransformKernelMap(configuration, source.Size(), destination.Size(), this.resampler); + try { ParallelHelper.IterateRowsWithTempBuffer( targetBounds, - this.Configuration, + configuration, (rows, vectorBuffer) => { Span vectorSpan = vectorBuffer.Span; for (int y = rows.Min; y < rows.Max; y++) { Span targetRowSpan = destination.GetPixelRowSpan(y); - PixelOperations.Instance.ToVector4(this.Configuration, targetRowSpan, vectorSpan); + PixelOperations.Instance.ToVector4(configuration, targetRowSpan, vectorSpan); ref float ySpanRef = ref kernel.GetYStartReference(y); ref float xSpanRef = ref kernel.GetXStartReference(y); @@ -124,7 +113,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms } PixelOperations.Instance.FromVector4Destructive( - this.Configuration, + configuration, vectorSpan, targetRowSpan); } diff --git a/src/ImageSharp/Processing/Processors/Transforms/AutoOrientProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/AutoOrientProcessor{TPixel}.cs index a5170c96a1..b9952ac8fe 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/AutoOrientProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/AutoOrientProcessor{TPixel}.cs @@ -34,33 +34,33 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms switch (orientation) { case OrientationMode.TopRight: - new FlipProcessor(FlipMode.Horizontal).Apply(this.Source, this.SourceRectangle); + new FlipProcessor(FlipMode.Horizontal).Execute(this.Source, this.SourceRectangle); break; case OrientationMode.BottomRight: - new RotateProcessor((int)RotateMode.Rotate180, size).Apply(this.Source, this.SourceRectangle); + new RotateProcessor((int)RotateMode.Rotate180, size).Execute(this.Source, this.SourceRectangle); break; case OrientationMode.BottomLeft: - new FlipProcessor(FlipMode.Vertical).Apply(this.Source, this.SourceRectangle); + new FlipProcessor(FlipMode.Vertical).Execute(this.Source, this.SourceRectangle); break; case OrientationMode.LeftTop: - new RotateProcessor((int)RotateMode.Rotate90, size).Apply(this.Source, this.SourceRectangle); - new FlipProcessor(FlipMode.Horizontal).Apply(this.Source, this.SourceRectangle); + new RotateProcessor((int)RotateMode.Rotate90, size).Execute(this.Source, this.SourceRectangle); + new FlipProcessor(FlipMode.Horizontal).Execute(this.Source, this.SourceRectangle); break; case OrientationMode.RightTop: - new RotateProcessor((int)RotateMode.Rotate90, size).Apply(this.Source, this.SourceRectangle); + new RotateProcessor((int)RotateMode.Rotate90, size).Execute(this.Source, this.SourceRectangle); break; case OrientationMode.RightBottom: - new FlipProcessor(FlipMode.Vertical).Apply(this.Source, this.SourceRectangle); - new RotateProcessor((int)RotateMode.Rotate270, size).Apply(this.Source, this.SourceRectangle); + new FlipProcessor(FlipMode.Vertical).Execute(this.Source, this.SourceRectangle); + new RotateProcessor((int)RotateMode.Rotate270, size).Execute(this.Source, this.SourceRectangle); break; case OrientationMode.LeftBottom: - new RotateProcessor((int)RotateMode.Rotate270, size).Apply(this.Source, this.SourceRectangle); + new RotateProcessor((int)RotateMode.Rotate270, size).Execute(this.Source, this.SourceRectangle); break; case OrientationMode.Unknown: diff --git a/src/ImageSharp/Processing/Processors/Transforms/CropProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/CropProcessor.cs index 025592a369..245a542084 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/CropProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/CropProcessor.cs @@ -32,7 +32,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms public Rectangle CropRectangle { get; } /// - public override ICloningImageProcessor CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) + public override ICloningImageProcessor CreatePixelSpecificCloningProcessor(Image source, Rectangle sourceRectangle) => new CropProcessor(this, source, sourceRectangle); } } diff --git a/src/ImageSharp/Processing/Processors/Transforms/CropProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/CropProcessor{TPixel}.cs index 539a11f028..1bbdd0a161 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/CropProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/CropProcessor{TPixel}.cs @@ -2,9 +2,6 @@ // Licensed under the Apache License, Version 2.0. using System; -using System.Collections.Generic; -using System.Linq; - using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.ParallelUtils; using SixLabors.ImageSharp.PixelFormats; @@ -19,7 +16,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms internal class CropProcessor : TransformProcessor where TPixel : struct, IPixel { - private readonly CropProcessor definition; + private Rectangle cropRectangle; /// /// Initializes a new instance of the class. @@ -29,53 +26,42 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// The source area to process for the current processor instance. public CropProcessor(CropProcessor definition, Image source, Rectangle sourceRectangle) : base(source, sourceRectangle) - { - this.definition = definition; - } - - private Rectangle CropRectangle => this.definition.CropRectangle; + => this.cropRectangle = definition.CropRectangle; /// - protected override Image CreateDestination() - { - // We will always be creating the clone even for mutate because we may need to resize the canvas - IEnumerable> frames = this.Source.Frames.Select, ImageFrame>( - x => new ImageFrame( - this.Source.GetConfiguration(), - this.CropRectangle.Width, - this.CropRectangle.Height, - x.Metadata.DeepClone())); - - // Use the overload to prevent an extra frame being added - return new Image(this.Source.GetConfiguration(), this.Source.Metadata.DeepClone(), frames); - } + protected override Size GetTargetSize() => new Size(this.cropRectangle.Width, this.cropRectangle.Height); /// protected override void OnFrameApply(ImageFrame source, ImageFrame destination) { - // Handle resize dimensions identical to the original - if (source.Width == destination.Width && source.Height == destination.Height && this.SourceRectangle == this.CropRectangle) + // Handle crop dimensions identical to the original + if (source.Width == destination.Width + && source.Height == destination.Height + && this.SourceRectangle == this.cropRectangle) { // the cloned will be blank here copy all the pixel data over source.GetPixelSpan().CopyTo(destination.GetPixelSpan()); return; } - Rectangle rect = this.CropRectangle; + Rectangle bounds = this.cropRectangle; // Copying is cheap, we should process more pixels per task: - ParallelExecutionSettings parallelSettings = this.Configuration.GetParallelSettings().MultiplyMinimumPixelsPerTask(4); + ParallelExecutionSettings parallelSettings + = this.Configuration + .GetParallelSettings() + .MultiplyMinimumPixelsPerTask(4); ParallelHelper.IterateRows( - rect, + bounds, parallelSettings, rows => { for (int y = rows.Min; y < rows.Max; y++) { - Span sourceRow = source.GetPixelRowSpan(y).Slice(rect.Left); - Span targetRow = destination.GetPixelRowSpan(y - rect.Top); - sourceRow.Slice(0, rect.Width).CopyTo(targetRow); + Span sourceRow = source.GetPixelRowSpan(y).Slice(bounds.Left); + Span targetRow = destination.GetPixelRowSpan(y - bounds.Top); + sourceRow.Slice(0, bounds.Width).CopyTo(targetRow); } }); } diff --git a/src/ImageSharp/Processing/Processors/Transforms/EntropyCropProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/EntropyCropProcessor{TPixel}.cs index b74fbb0ab7..2b900ee360 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/EntropyCropProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/EntropyCropProcessor{TPixel}.cs @@ -42,16 +42,16 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms Configuration configuration = this.Source.GetConfiguration(); // Detect the edges. - new SobelProcessor(false).Apply(temp, this.SourceRectangle); + new SobelProcessor(false).Execute(temp, this.SourceRectangle); // Apply threshold binarization filter. - new BinaryThresholdProcessor(this.definition.Threshold).Apply(temp, this.SourceRectangle); + new BinaryThresholdProcessor(this.definition.Threshold).Execute(temp, this.SourceRectangle); // Search for the first white pixels rectangle = ImageMaths.GetFilteredBoundingRectangle(temp.Frames.RootFrame, 0); } - new CropProcessor(rectangle, this.Source.Size()).Apply(this.Source, this.SourceRectangle); + new CropProcessor(rectangle, this.Source.Size()).Execute(this.Source, this.SourceRectangle); base.BeforeImageApply(); } diff --git a/src/ImageSharp/Processing/Processors/Transforms/ProjectiveTransformProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/ProjectiveTransformProcessor.cs index babdee593a..d91db9a72b 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/ProjectiveTransformProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/ProjectiveTransformProcessor.cs @@ -41,7 +41,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms public Size TargetDimensions { get; } /// - public override ICloningImageProcessor CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) + public override ICloningImageProcessor CreatePixelSpecificCloningProcessor(Image source, Rectangle sourceRectangle) => new ProjectiveTransformProcessor(this, source, sourceRectangle); } } diff --git a/src/ImageSharp/Processing/Processors/Transforms/ProjectiveTransformProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/ProjectiveTransformProcessor{TPixel}.cs index 29dc8a070d..68bfd817e5 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/ProjectiveTransformProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/ProjectiveTransformProcessor{TPixel}.cs @@ -2,8 +2,6 @@ // Licensed under the Apache License, Version 2.0. using System; -using System.Collections.Generic; -using System.Linq; using System.Numerics; using SixLabors.ImageSharp.Advanced; @@ -20,7 +18,9 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms internal class ProjectiveTransformProcessor : TransformProcessor where TPixel : struct, IPixel { - private readonly ProjectiveTransformProcessor definition; + private Size targetSize; + private readonly IResampler resampler; + private Matrix4x4 transformMatrix; /// /// Initializes a new instance of the class. @@ -31,52 +31,37 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms public ProjectiveTransformProcessor(ProjectiveTransformProcessor definition, Image source, Rectangle sourceRectangle) : base(source, sourceRectangle) { - this.definition = definition; + this.targetSize = definition.TargetDimensions; + this.transformMatrix = definition.TransformMatrix; + this.resampler = definition.Sampler; } - private Size TargetDimensions => this.definition.TargetDimensions; - - /// - protected override Image CreateDestination() - { - // We will always be creating the clone even for mutate because we may need to resize the canvas - IEnumerable> frames = this.Source.Frames.Select, ImageFrame>( - x => new ImageFrame( - this.Source.GetConfiguration(), - this.TargetDimensions.Width, - this.TargetDimensions.Height, - x.Metadata.DeepClone())); - - // Use the overload to prevent an extra frame being added - return new Image(this.Source.GetConfiguration(), this.Source.Metadata.DeepClone(), frames); - } + protected override Size GetTargetSize() => this.targetSize; /// protected override void OnFrameApply(ImageFrame source, ImageFrame destination) { - Matrix4x4 transformMatrix = this.definition.TransformMatrix; - // Handle transforms that result in output identical to the original. - if (transformMatrix.Equals(default) || transformMatrix.Equals(Matrix4x4.Identity)) + if (this.transformMatrix.Equals(default) || this.transformMatrix.Equals(Matrix4x4.Identity)) { // The clone will be blank here copy all the pixel data over source.GetPixelSpan().CopyTo(destination.GetPixelSpan()); return; } - int width = this.TargetDimensions.Width; - var targetBounds = new Rectangle(Point.Empty, this.TargetDimensions); + int width = this.targetSize.Width; + Rectangle sourceBounds = this.SourceRectangle; + var targetBounds = new Rectangle(Point.Empty, this.targetSize); + Configuration configuration = this.Configuration; // Convert from screen to world space. - Matrix4x4.Invert(transformMatrix, out Matrix4x4 matrix); - - IResampler sampler = this.definition.Sampler; + Matrix4x4.Invert(this.transformMatrix, out Matrix4x4 matrix); - if (sampler is NearestNeighborResampler) + if (this.resampler is NearestNeighborResampler) { ParallelHelper.IterateRows( targetBounds, - this.Configuration, + configuration, rows => { for (int y = rows.Min; y < rows.Max; y++) @@ -89,7 +74,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms int px = (int)MathF.Round(point.X); int py = (int)MathF.Round(point.Y); - if (this.SourceRectangle.Contains(px, py)) + if (sourceBounds.Contains(px, py)) { destRow[x] = source[px, py]; } @@ -100,19 +85,20 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms return; } - var kernel = new TransformKernelMap(this.Configuration, source.Size(), destination.Size(), sampler); + var kernel = new TransformKernelMap(configuration, source.Size(), destination.Size(), this.resampler); + try { ParallelHelper.IterateRowsWithTempBuffer( targetBounds, - this.Configuration, + configuration, (rows, vectorBuffer) => { Span vectorSpan = vectorBuffer.Span; for (int y = rows.Min; y < rows.Max; y++) { Span targetRowSpan = destination.GetPixelRowSpan(y); - PixelOperations.Instance.ToVector4(this.Configuration, targetRowSpan, vectorSpan); + PixelOperations.Instance.ToVector4(configuration, targetRowSpan, vectorSpan); ref float ySpanRef = ref kernel.GetYStartReference(y); ref float xSpanRef = ref kernel.GetXStartReference(y); @@ -131,7 +117,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms } PixelOperations.Instance.FromVector4Destructive( - this.Configuration, + configuration, vectorSpan, targetRowSpan); } diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs index 5390fa4a4a..ccaa1ef9e5 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs @@ -55,7 +55,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms public bool Compand { get; } /// - public override ICloningImageProcessor CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) + public override ICloningImageProcessor CreatePixelSpecificCloningProcessor(Image source, Rectangle sourceRectangle) => new ResizeProcessor(this, source, sourceRectangle); } } diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs index b85983a481..78e471ad62 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs @@ -2,8 +2,6 @@ // Licensed under the Apache License, Version 2.0. using System; -using System.Collections.Generic; -using System.Linq; using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.Memory; @@ -24,74 +22,34 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms internal class ResizeProcessor : TransformProcessor where TPixel : struct, IPixel { - private readonly ResizeProcessor parameterSource; private bool isDisposed; + private readonly int targetWidth; + private readonly int targetHeight; + private readonly IResampler resampler; + private Rectangle targetRectangle; + private readonly bool compand; // The following fields are not immutable but are optionally created on demand. private ResizeKernelMap horizontalKernelMap; private ResizeKernelMap verticalKernelMap; - public ResizeProcessor(ResizeProcessor parameterSource, Image source, Rectangle sourceRectangle) + public ResizeProcessor(ResizeProcessor definition, Image source, Rectangle sourceRectangle) : base(source, sourceRectangle) { - this.parameterSource = parameterSource; + this.targetWidth = definition.TargetWidth; + this.targetHeight = definition.TargetHeight; + this.targetRectangle = definition.TargetRectangle; + this.resampler = definition.Sampler; + this.compand = definition.Compand; } - /// - /// Gets the sampler to perform the resize operation. - /// - public IResampler Sampler => this.parameterSource.Sampler; - - /// - /// Gets the target width. - /// - public int TargetWidth => this.parameterSource.TargetWidth; - - /// - /// Gets the target height. - /// - public int TargetHeight => this.parameterSource.TargetHeight; - - /// - /// Gets the target resize rectangle. - /// - public Rectangle TargetRectangle => this.parameterSource.TargetRectangle; - - /// - /// Gets a value indicating whether to compress or expand individual pixel color values on processing. - /// - public bool Compand => this.parameterSource.Compand; - - /// - /// This is a shim for tagging the CreateDestination virtual generic method for the AoT iOS compiler. - /// This method should never be referenced outside of the AotCompiler code. - /// - /// The result returned from . - internal Image AotCreateDestination() - => this.CreateDestination(); - /// - protected override Image CreateDestination() - { - Image source = this.Source; - Configuration configuration = this.Configuration; - - // We will always be creating the clone even for mutate because we may need to resize the canvas - IEnumerable> frames = source.Frames.Select, ImageFrame>( - x => new ImageFrame( - configuration, - this.TargetWidth, - this.TargetHeight, - x.Metadata.DeepClone())); - - // Use the overload to prevent an extra frame being added - return new Image(configuration, source.Metadata.DeepClone(), frames); - } + protected override Size GetTargetSize() => new Size(this.targetWidth, this.targetHeight); /// protected override void BeforeImageApply(Image destination) { - if (!(this.Sampler is NearestNeighborResampler)) + if (!(this.resampler is NearestNeighborResampler)) { Image source = this.Source; Rectangle sourceRectangle = this.SourceRectangle; @@ -99,14 +57,14 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms // Since all image frame dimensions have to be the same we can calculate this for all frames. MemoryAllocator memoryAllocator = source.GetMemoryAllocator(); this.horizontalKernelMap = ResizeKernelMap.Calculate( - this.Sampler, - this.TargetRectangle.Width, + this.resampler, + this.targetRectangle.Width, sourceRectangle.Width, memoryAllocator); this.verticalKernelMap = ResizeKernelMap.Calculate( - this.Sampler, - this.TargetRectangle.Height, + this.resampler, + this.targetRectangle.Height, sourceRectangle.Height, memoryAllocator); } @@ -121,29 +79,29 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms Configuration configuration = this.Configuration; // Handle resize dimensions identical to the original - if (source.Width == destination.Width && source.Height == destination.Height && sourceRectangle == this.TargetRectangle) + if (source.Width == destination.Width && source.Height == destination.Height && sourceRectangle == this.targetRectangle) { // The cloned will be blank here copy all the pixel data over source.GetPixelSpan().CopyTo(destination.GetPixelSpan()); return; } - int width = this.TargetWidth; - int height = this.TargetHeight; + int width = this.targetWidth; + int height = this.targetHeight; int sourceX = sourceRectangle.X; int sourceY = sourceRectangle.Y; - int startY = this.TargetRectangle.Y; - int startX = this.TargetRectangle.X; + int startY = this.targetRectangle.Y; + int startX = this.targetRectangle.X; var targetWorkingRect = Rectangle.Intersect( - this.TargetRectangle, + this.targetRectangle, new Rectangle(0, 0, width, height)); - if (this.Sampler is NearestNeighborResampler) + if (this.resampler is NearestNeighborResampler) { // Scaling factors - float widthFactor = sourceRectangle.Width / (float)this.TargetRectangle.Width; - float heightFactor = sourceRectangle.Height / (float)this.TargetRectangle.Height; + float widthFactor = sourceRectangle.Width / (float)this.targetRectangle.Width; + float heightFactor = sourceRectangle.Height / (float)this.targetRectangle.Height; ParallelHelper.IterateRows( targetWorkingRect, @@ -153,8 +111,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms for (int y = rows.Min; y < rows.Max; y++) { // Y coordinates of source points - Span sourceRow = - source.GetPixelRowSpan((int)(((y - startY) * heightFactor) + sourceY)); + Span sourceRow = source.GetPixelRowSpan((int)(((y - startY) * heightFactor) + sourceY)); Span targetRow = destination.GetPixelRowSpan(y); for (int x = targetWorkingRect.Left; x < targetWorkingRect.Right; x++) @@ -169,7 +126,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms } PixelConversionModifiers conversionModifiers = - PixelConversionModifiers.Premultiply.ApplyCompanding(this.Compand); + PixelConversionModifiers.Premultiply.ApplyCompanding(this.compand); BufferArea sourceArea = source.PixelBuffer.GetArea(sourceRectangle); @@ -183,7 +140,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms this.verticalKernelMap, width, targetWorkingRect, - this.TargetRectangle.Location)) + this.targetRectangle.Location)) { worker.Initialize(); diff --git a/src/ImageSharp/Processing/Processors/Transforms/RotateProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/RotateProcessor.cs index 277cc05b22..7d6ec0e08e 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/RotateProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/RotateProcessor.cs @@ -46,7 +46,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms public float Degrees { get; } /// - public override ICloningImageProcessor CreatePixelSpecificProcessor(Image source, Rectangle sourceRectangle) + public override ICloningImageProcessor CreatePixelSpecificCloningProcessor(Image source, Rectangle sourceRectangle) => new RotateProcessor(this, source, sourceRectangle); } } diff --git a/tests/ImageSharp.Tests/Drawing/FillRegionProcessorTests.cs b/tests/ImageSharp.Tests/Drawing/FillRegionProcessorTests.cs index 972f5cf4a1..c0388ea2d4 100644 --- a/tests/ImageSharp.Tests/Drawing/FillRegionProcessorTests.cs +++ b/tests/ImageSharp.Tests/Drawing/FillRegionProcessorTests.cs @@ -41,7 +41,7 @@ namespace SixLabors.ImageSharp.Tests.Drawing }; var processor = new FillRegionProcessor(brush.Object, region, options); var img = new Image(1, 1); - processor.Apply(img, bounds); + processor.Execute(img, bounds); Assert.Equal(4, region.ScanInvocationCounter); } @@ -54,7 +54,7 @@ namespace SixLabors.ImageSharp.Tests.Drawing var options = new GraphicsOptions(true); var processor = new FillRegionProcessor(brush.Object, new MockRegion1(), options); var img = new Image(10, 10); - processor.Apply(img, bounds); + processor.Execute(img, bounds); } [Fact] diff --git a/tests/ImageSharp.Tests/Processing/ImageProcessingContextTests.cs b/tests/ImageSharp.Tests/Processing/ImageProcessingContextTests.cs index afb2cbecd2..9d16583cd8 100644 --- a/tests/ImageSharp.Tests/Processing/ImageProcessingContextTests.cs +++ b/tests/ImageSharp.Tests/Processing/ImageProcessingContextTests.cs @@ -2,12 +2,10 @@ // Licensed under the Apache License, Version 2.0. using Moq; - using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing; using SixLabors.ImageSharp.Processing.Processors; using SixLabors.Primitives; - using Xunit; namespace SixLabors.ImageSharp.Tests.Processing @@ -21,7 +19,7 @@ namespace SixLabors.ImageSharp.Tests.Processing private readonly Mock processorDefinition; - private readonly Mock cloningProcessorDefinition; + private readonly Mock cloningProcessorDefinition; private readonly Mock> regularProcessorImpl; @@ -32,7 +30,7 @@ namespace SixLabors.ImageSharp.Tests.Processing public ImageProcessingContextTests() { this.processorDefinition = new Mock(); - this.cloningProcessorDefinition = new Mock(); + this.cloningProcessorDefinition = new Mock(); this.regularProcessorImpl = new Mock>(); this.cloningProcessorImpl = new Mock>(); } @@ -54,11 +52,11 @@ namespace SixLabors.ImageSharp.Tests.Processing if (throwException) { - Assert.Throws(() => this.MutateApply(useBounds)); + Assert.Throws(() => this.MutateRegularApply(useBounds)); } else { - this.MutateApply(useBounds); + this.MutateRegularApply(useBounds); } this.regularProcessorImpl.Verify(p => p.Execute(), Times.Once()); @@ -92,11 +90,11 @@ namespace SixLabors.ImageSharp.Tests.Processing if (throwException) { - Assert.Throws(() => this.MutateApply(useBounds)); + Assert.Throws(() => this.MutateCloneApply(useBounds)); } else { - this.MutateApply(useBounds); + this.MutateCloneApply(useBounds); } this.cloningProcessorImpl.Verify(p => p.Execute(), Times.Once()); @@ -122,7 +120,7 @@ namespace SixLabors.ImageSharp.Tests.Processing this.cloningProcessorImpl.Verify(p => p.Dispose(), Times.Once()); } - private void MutateApply(bool useBounds) + private void MutateRegularApply(bool useBounds) { if (useBounds) { @@ -134,6 +132,18 @@ namespace SixLabors.ImageSharp.Tests.Processing } } + private void MutateCloneApply(bool useBounds) + { + if (useBounds) + { + this.image.Mutate(c => c.ApplyProcessor(this.cloningProcessorDefinition.Object, Bounds)); + } + else + { + this.image.Mutate(c => c.ApplyProcessor(this.cloningProcessorDefinition.Object)); + } + } + private void CloneRegularApply(bool useBounds) { if (useBounds) @@ -178,8 +188,8 @@ namespace SixLabors.ImageSharp.Tests.Processing this.cloningProcessorImpl.Setup(p => p.CloneAndExecute()).Throws(new ImageProcessingException("Test")); } - this.processorDefinition - .Setup(p => p.CreatePixelSpecificProcessor(It.IsAny>(), It.IsAny())) + this.cloningProcessorDefinition + .Setup(p => p.CreatePixelSpecificCloningProcessor(It.IsAny>(), It.IsAny())) .Returns(this.cloningProcessorImpl.Object); this.cloningProcessorDefinition