From 45b2dee8bb164b45fd9bfb17acaa9c6fa6656b44 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 12 Sep 2022 16:34:46 +1000 Subject: [PATCH 01/17] Add TryGetLinearlySeparableComponents and tests --- .../ConvolutionProcessorHelpers.cs | 104 +++++++++++++++++- .../ConvolutionProcessorHelpersTest.cs | 73 ++++++++++++ 2 files changed, 175 insertions(+), 2 deletions(-) create mode 100644 tests/ImageSharp.Tests/Processing/Processors/Convolution/ConvolutionProcessorHelpersTest.cs diff --git a/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessorHelpers.cs b/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessorHelpers.cs index ef25de6396..7dea3338ce 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessorHelpers.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessorHelpers.cs @@ -11,6 +11,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution /// Kernel radius is calculated using the minimum viable value. /// See . /// + /// The weight of the blur. internal static int GetDefaultGaussianRadius(float sigma) => (int)MathF.Ceiling(sigma * 3); @@ -18,9 +19,11 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution /// Create a 1 dimensional Gaussian kernel using the Gaussian G(x) function. /// /// The convolution kernel. + /// The kernel size. + /// The weight of the blur. internal static float[] CreateGaussianBlurKernel(int size, float weight) { - var kernel = new float[size]; + float[] kernel = new float[size]; float sum = 0F; float midpoint = (size - 1) / 2F; @@ -45,10 +48,12 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution /// /// Create a 1 dimensional Gaussian kernel using the Gaussian G(x) function /// + /// The kernel size. + /// The weight of the blur. /// The convolution kernel. internal static float[] CreateGaussianSharpenKernel(int size, float weight) { - var kernel = new float[size]; + float[] kernel = new float[size]; float sum = 0; @@ -85,5 +90,100 @@ namespace SixLabors.ImageSharp.Processing.Processors.Convolution return kernel; } + + /// + /// Checks whether or not a given NxM matrix is linearly separable, and if so, it extracts the separable components. + /// These would be two 1D vectors, of size N and of size M. + /// This algorithm runs in O(NM). + /// + /// The input 2D matrix to analyze. + /// The resulting 1D row vector, if possible. + /// The resulting 1D column vector, if possible. + /// Whether or not was linearly separable. + public static bool TryGetLinearlySeparableComponents(this DenseMatrix matrix, out float[] row, out float[] column) + { + int height = matrix.Rows; + int width = matrix.Columns; + + float[] tempX = new float[width]; + float[] tempY = new float[height]; + + // This algorithm checks whether the input matrix is linearly separable and extracts two + // 1D components if possible. Note that for a given NxM matrix that is linearly separable, + // there exists an infinite number of possible solutions to the system of linear equations + // representing the possible 1D components that can produce the input matrix as a product. + // Let's assume we have a 3x3 input matrix to describe the logic. We have the following: + // + // | m11, m12, m13 | | c1 | + // M = | m21, m22, m23 |, and we want to find: R = | r1, r2, r3 | and C = | c2 |. + // | m31, m32, m33 | | c3 | + // + // We essentially get the following system of linear equations to solve: + // + // / a11 = r1c1 + // | a12 = r2c1 + // | a13 = r3c1 + // | a21 = r1c2 a11 a12 a13 a11 a12 a13 + // / a22 = r2c2, which gives us: ----- = ----- = ----- and ----- = ----- = -----. + // \ a23 = r3c2 a21 a22 a23 a31 a32 a33 + // | a31 = r1c3 + // | a32 = r2c3 + // \ a33 = r3c3 + // + // As we said, there are infinite solutions to this problem (provided the input matrix is in + // fact linearly separable), but we can look at the equalities above to find a way to define + // one specific solution that is very easy to calculate (and that is equivalent to all others + // anyway). In particular, we can see that in order for it to be linearly separable, the matrix + // needs to have each row linearly dependent on each other. That is, its rank is just 1. This + // means that we can express the whole matrix as a function of one row vector (any of the rows + // in the matrix), and a column vector that represents the ratio of each element in a given column + // j with the corresponding j-th item in the reference row. This same procedure extends naturally + // to lineary separable 2D matrices of any size, too. So we end up with the following generalized + // solution for a matrix M of size NxN (or MxN, that works too) and the R and C vectors: + // + // | m11, m12, m13, ..., m1N | | m11/m11 | + // | m21, m22, m23, ..., m2N | | m21/m11 | + // M = | m31, m32, m33, ..., m3N |, R = | m11, m12, m13, ..., m1N |, C = | m31/m11 |. + // | ... ... ... ... ... | | ... | + // | mN1, mN2, mN3, ..., mNN | | mN1/m11 | + // + // So what this algorithm does is just the following: + // 1) It calculates the C[i] value for each i-th row. + // 2) It checks that every j-th item in the row respects C[i] = M[i, j] / M[0, j]. If this is + // not true for any j-th item in any i-th row, then the matrix is not linearly separable. + // 3) It sets items in R and C to the values detailed above if the validation passed. + for (int y = 1; y < height; y++) + { + float ratio = matrix[y, 0] / matrix[0, 0]; + + for (int x = 1; x < width; x++) + { + if (Math.Abs(ratio - (matrix[y, x] / matrix[0, x])) > 0.0001f) + { + row = null; + column = null; + + return false; + } + } + + tempY[y] = ratio; + } + + // The first row is used as a reference, to the ratio is just 1 + tempY[0] = 1; + + // The row component is simply the reference row in the input matrix. + // In this case, we're just using the first one for simplicity. + for (int x = 0; x < width; x++) + { + tempX[x] = matrix[0, x]; + } + + row = tempX; + column = tempY; + + return true; + } } } diff --git a/tests/ImageSharp.Tests/Processing/Processors/Convolution/ConvolutionProcessorHelpersTest.cs b/tests/ImageSharp.Tests/Processing/Processors/Convolution/ConvolutionProcessorHelpersTest.cs new file mode 100644 index 0000000000..740bdddafd --- /dev/null +++ b/tests/ImageSharp.Tests/Processing/Processors/Convolution/ConvolutionProcessorHelpersTest.cs @@ -0,0 +1,73 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using System; +using SixLabors.ImageSharp.Processing.Processors.Convolution; +using Xunit; + +namespace SixLabors.ImageSharp.Tests.Processing.Processors.Convolution +{ + [GroupOutput("Convolution")] + public class ConvolutionProcessorHelpersTest + { + [Theory] + [InlineData(3)] + [InlineData(5)] + [InlineData(9)] + [InlineData(22)] + [InlineData(33)] + [InlineData(80)] + public void VerifyGaussianKernelDecomposition(int radius) + { + int kernelSize = (radius * 2) + 1; + float sigma = radius / 3F; + float[] kernel = ConvolutionProcessorHelpers.CreateGaussianBlurKernel(kernelSize, sigma); + DenseMatrix matrix = DotProduct(kernel, kernel); + + bool result = matrix.TryGetLinearlySeparableComponents(out float[] row, out float[] column); + + Assert.True(result); + Assert.NotNull(row); + Assert.NotNull(column); + Assert.Equal(row.Length, matrix.Rows); + Assert.Equal(column.Length, matrix.Columns); + + float[,] dotProduct = DotProduct(row, column); + + for (int y = 0; y < column.Length; y++) + { + for (int x = 0; x < row.Length; x++) + { + Assert.True(Math.Abs(matrix[y, x] - dotProduct[y, x]) < 0.0001F); + } + } + } + + [Fact] + public void VerifyNonSeparableMatrix() + { + bool result = LaplacianKernels.LaplacianOfGaussianXY.TryGetLinearlySeparableComponents( + out float[] row, + out float[] column); + + Assert.False(result); + Assert.Null(row); + Assert.Null(column); + } + + private static DenseMatrix DotProduct(float[] row, float[] column) + { + float[,] matrix = new float[column.Length, row.Length]; + + for (int x = 0; x < row.Length; x++) + { + for (int y = 0; y < column.Length; y++) + { + matrix[y, x] = row[x] * column[y]; + } + } + + return matrix; + } + } +} From 65088b362473c0d1544ba2b707cd38a7e3894140 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 15 Sep 2022 22:12:29 +1000 Subject: [PATCH 02/17] Fix merge and namespace style --- .../ConvolutionProcessorHelpers.cs | 9 +- .../ConvolutionProcessorHelpersTest.cs | 97 +++++++++---------- 2 files changed, 54 insertions(+), 52 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessorHelpers.cs b/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessorHelpers.cs index ebda2eb079..37adaf5402 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessorHelpers.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessorHelpers.cs @@ -9,6 +9,7 @@ internal static class ConvolutionProcessorHelpers /// Kernel radius is calculated using the minimum viable value. /// See . /// + /// The weight of the blur. internal static int GetDefaultGaussianRadius(float sigma) => (int)MathF.Ceiling(sigma * 3); @@ -16,9 +17,11 @@ internal static class ConvolutionProcessorHelpers /// Create a 1 dimensional Gaussian kernel using the Gaussian G(x) function. /// /// The convolution kernel. + /// The kernel size. + /// The weight of the blur. internal static float[] CreateGaussianBlurKernel(int size, float weight) { - var kernel = new float[size]; + float[] kernel = new float[size]; float sum = 0F; float midpoint = (size - 1) / 2F; @@ -44,9 +47,11 @@ internal static class ConvolutionProcessorHelpers /// Create a 1 dimensional Gaussian kernel using the Gaussian G(x) function /// /// The convolution kernel. + /// The kernel size. + /// The weight of the blur. internal static float[] CreateGaussianSharpenKernel(int size, float weight) { - var kernel = new float[size]; + float[] kernel = new float[size]; float sum = 0; diff --git a/tests/ImageSharp.Tests/Processing/Processors/Convolution/ConvolutionProcessorHelpersTest.cs b/tests/ImageSharp.Tests/Processing/Processors/Convolution/ConvolutionProcessorHelpersTest.cs index 740bdddafd..574c983710 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Convolution/ConvolutionProcessorHelpersTest.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Convolution/ConvolutionProcessorHelpersTest.cs @@ -1,73 +1,70 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. -using System; using SixLabors.ImageSharp.Processing.Processors.Convolution; -using Xunit; -namespace SixLabors.ImageSharp.Tests.Processing.Processors.Convolution +namespace SixLabors.ImageSharp.Tests.Processing.Processors.Convolution; + +[GroupOutput("Convolution")] +public class ConvolutionProcessorHelpersTest { - [GroupOutput("Convolution")] - public class ConvolutionProcessorHelpersTest + [Theory] + [InlineData(3)] + [InlineData(5)] + [InlineData(9)] + [InlineData(22)] + [InlineData(33)] + [InlineData(80)] + public void VerifyGaussianKernelDecomposition(int radius) { - [Theory] - [InlineData(3)] - [InlineData(5)] - [InlineData(9)] - [InlineData(22)] - [InlineData(33)] - [InlineData(80)] - public void VerifyGaussianKernelDecomposition(int radius) - { - int kernelSize = (radius * 2) + 1; - float sigma = radius / 3F; - float[] kernel = ConvolutionProcessorHelpers.CreateGaussianBlurKernel(kernelSize, sigma); - DenseMatrix matrix = DotProduct(kernel, kernel); + int kernelSize = (radius * 2) + 1; + float sigma = radius / 3F; + float[] kernel = ConvolutionProcessorHelpers.CreateGaussianBlurKernel(kernelSize, sigma); + DenseMatrix matrix = DotProduct(kernel, kernel); - bool result = matrix.TryGetLinearlySeparableComponents(out float[] row, out float[] column); + bool result = matrix.TryGetLinearlySeparableComponents(out float[] row, out float[] column); - Assert.True(result); - Assert.NotNull(row); - Assert.NotNull(column); - Assert.Equal(row.Length, matrix.Rows); - Assert.Equal(column.Length, matrix.Columns); + Assert.True(result); + Assert.NotNull(row); + Assert.NotNull(column); + Assert.Equal(row.Length, matrix.Rows); + Assert.Equal(column.Length, matrix.Columns); - float[,] dotProduct = DotProduct(row, column); + float[,] dotProduct = DotProduct(row, column); - for (int y = 0; y < column.Length; y++) + for (int y = 0; y < column.Length; y++) + { + for (int x = 0; x < row.Length; x++) { - for (int x = 0; x < row.Length; x++) - { - Assert.True(Math.Abs(matrix[y, x] - dotProduct[y, x]) < 0.0001F); - } + Assert.True(Math.Abs(matrix[y, x] - dotProduct[y, x]) < 0.0001F); } } + } - [Fact] - public void VerifyNonSeparableMatrix() - { - bool result = LaplacianKernels.LaplacianOfGaussianXY.TryGetLinearlySeparableComponents( - out float[] row, - out float[] column); + [Fact] + public void VerifyNonSeparableMatrix() + { + bool result = LaplacianKernels.LaplacianOfGaussianXY.TryGetLinearlySeparableComponents( + out float[] row, + out float[] column); - Assert.False(result); - Assert.Null(row); - Assert.Null(column); - } + Assert.False(result); + Assert.Null(row); + Assert.Null(column); + } - private static DenseMatrix DotProduct(float[] row, float[] column) - { - float[,] matrix = new float[column.Length, row.Length]; + private static DenseMatrix DotProduct(float[] row, float[] column) + { + float[,] matrix = new float[column.Length, row.Length]; - for (int x = 0; x < row.Length; x++) + for (int x = 0; x < row.Length; x++) + { + for (int y = 0; y < column.Length; y++) { - for (int y = 0; y < column.Length; y++) - { - matrix[y, x] = row[x] * column[y]; - } + matrix[y, x] = row[x] * column[y]; } - - return matrix; } + + return matrix; } } From 976747490c43cce3a03ef7fa20084072e10c00be Mon Sep 17 00:00:00 2001 From: Ynse Hoornenborg Date: Sat, 17 Sep 2022 13:03:17 +0200 Subject: [PATCH 03/17] Implement auto level processor --- .../Normalization/AutoLevelProcessor.cs | 37 +++++ .../AutoLevelProcessor{TPixel}.cs | 136 ++++++++++++++++++ ...lHistogramEqualizationProcessor{TPixel}.cs | 43 +----- .../GrayscaleLevelsRowOperation{TPixel}.cs | 53 +++++++ .../HistogramEqualizationMethod.cs | 5 + .../HistogramEqualizationProcessor.cs | 3 + .../HistogramEqualizationTests.cs | 18 +++ ...ToReferenceOutput_Rgba32_forest_bridge.png | 3 + 8 files changed, 256 insertions(+), 42 deletions(-) create mode 100644 src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor.cs create mode 100644 src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs create mode 100644 src/ImageSharp/Processing/Processors/Normalization/GrayscaleLevelsRowOperation{TPixel}.cs create mode 100644 tests/Images/External/ReferenceOutput/HistogramEqualizationTests/AutoLevel_CompareToReferenceOutput_Rgba32_forest_bridge.png diff --git a/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor.cs b/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor.cs new file mode 100644 index 0000000000..b33e46ce37 --- /dev/null +++ b/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor.cs @@ -0,0 +1,37 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +namespace SixLabors.ImageSharp.Processing.Processors.Normalization; + +/// +/// Applies a luminance histogram equilization to the image. +/// +public class AutoLevelProcessor : HistogramEqualizationProcessor +{ + /// + /// Initializes a new instance of the class. + /// It uses the exact minimum and maximum values found in the luminance channel, as the BlackPoint and WhitePoint to linearly stretch the colors + /// (and histogram) of the image. + /// + /// The number of different luminance levels. Typical values are 256 for 8-bit grayscale images + /// or 65536 for 16-bit grayscale images. + /// Indicating whether to clip the histogram bins at a specific value. + /// The histogram clip limit. Histogram bins which exceed this limit, will be capped at this value. + public AutoLevelProcessor( + int luminanceLevels, + bool clipHistogram, + int clipLimit) + : base(luminanceLevels, clipHistogram, clipLimit) + { + } + + /// + public override IImageProcessor CreatePixelSpecificProcessor(Configuration configuration, Image source, Rectangle sourceRectangle) + => new AutoLevelProcessor( + configuration, + this.LuminanceLevels, + this.ClipHistogram, + this.ClipLimit, + source, + sourceRectangle); +} diff --git a/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs new file mode 100644 index 0000000000..bdb2a500b5 --- /dev/null +++ b/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs @@ -0,0 +1,136 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using System.Buffers; +using System.Numerics; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using SixLabors.ImageSharp.Advanced; +using SixLabors.ImageSharp.Memory; +using SixLabors.ImageSharp.PixelFormats; + +namespace SixLabors.ImageSharp.Processing.Processors.Normalization; + +/// +/// Applies a luminance histogram equalization to the image. +/// +/// The pixel format. +internal class AutoLevelProcessor : HistogramEqualizationProcessor + where TPixel : unmanaged, IPixel +{ + /// + /// Initializes a new instance of the class. + /// + /// The configuration which allows altering default behaviour or extending the library. + /// + /// The number of different luminance levels. Typical values are 256 for 8-bit grayscale images + /// or 65536 for 16-bit grayscale images. + /// + /// Indicating whether to clip the histogram bins at a specific value. + /// The histogram clip limit. Histogram bins which exceed this limit, will be capped at this value. + /// The source for the current processor instance. + /// The source area to process for the current processor instance. + public AutoLevelProcessor( + Configuration configuration, + int luminanceLevels, + bool clipHistogram, + int clipLimit, + Image source, + Rectangle sourceRectangle) + : base(configuration, luminanceLevels, clipHistogram, clipLimit, source, sourceRectangle) + { + } + + /// + protected override void OnFrameApply(ImageFrame source) + { + MemoryAllocator memoryAllocator = this.Configuration.MemoryAllocator; + int numberOfPixels = source.Width * source.Height; + var interest = Rectangle.Intersect(this.SourceRectangle, source.Bounds()); + + using IMemoryOwner histogramBuffer = memoryAllocator.Allocate(this.LuminanceLevels, AllocationOptions.Clean); + + // Build the histogram of the grayscale levels. + var grayscaleOperation = new GrayscaleLevelsRowOperation(interest, histogramBuffer, source.PixelBuffer, this.LuminanceLevels); + ParallelRowIterator.IterateRows( + this.Configuration, + interest, + in grayscaleOperation); + + Span histogram = histogramBuffer.GetSpan(); + if (this.ClipHistogramEnabled) + { + this.ClipHistogram(histogram, this.ClipLimit); + } + + using IMemoryOwner cdfBuffer = memoryAllocator.Allocate(this.LuminanceLevels, AllocationOptions.Clean); + + // Calculate the cumulative distribution function, which will map each input pixel to a new value. + int cdfMin = CalculateCdf( + ref MemoryMarshal.GetReference(cdfBuffer.GetSpan()), + ref MemoryMarshal.GetReference(histogram), + histogram.Length - 1); + + float numberOfPixelsMinusCdfMin = numberOfPixels - cdfMin; + + // Apply the cdf to each pixel of the image + var cdfOperation = new CdfApplicationRowOperation(interest, cdfBuffer, source.PixelBuffer, this.LuminanceLevels, numberOfPixelsMinusCdfMin); + ParallelRowIterator.IterateRows( + this.Configuration, + interest, + in cdfOperation); + } + + /// + /// A implementing the cdf application levels logic for . + /// + private readonly struct CdfApplicationRowOperation : IRowOperation + { + private readonly Rectangle bounds; + private readonly IMemoryOwner cdfBuffer; + private readonly Buffer2D source; + private readonly int luminanceLevels; + private readonly float numberOfPixelsMinusCdfMin; + + [MethodImpl(InliningOptions.ShortMethod)] + public CdfApplicationRowOperation( + Rectangle bounds, + IMemoryOwner cdfBuffer, + Buffer2D source, + int luminanceLevels, + float numberOfPixelsMinusCdfMin) + { + this.bounds = bounds; + this.cdfBuffer = cdfBuffer; + this.source = source; + this.luminanceLevels = luminanceLevels; + this.numberOfPixelsMinusCdfMin = numberOfPixelsMinusCdfMin; + } + + /// + [MethodImpl(InliningOptions.ShortMethod)] + public void Invoke(int y) + { + ref int cdfBase = ref MemoryMarshal.GetReference(this.cdfBuffer.GetSpan()); + var sourceAccess = new PixelAccessor(this.source); + Span pixelRow = sourceAccess.GetRowSpan(y); + int levels = this.luminanceLevels; + float noOfPixelsMinusCdfMin = this.numberOfPixelsMinusCdfMin; + + for (int x = 0; x < this.bounds.Width; x++) + { + // TODO: We should bulk convert here. + ref TPixel pixel = ref pixelRow[x]; + var vector = pixel.ToVector4() * levels; + + uint originalX = (uint)MathF.Round(vector.X); + float scaledX = Unsafe.Add(ref cdfBase, originalX) / noOfPixelsMinusCdfMin; + uint originalY = (uint)MathF.Round(vector.Y); + float scaledY = Unsafe.Add(ref cdfBase, originalY) / noOfPixelsMinusCdfMin; + uint originalZ = (uint)MathF.Round(vector.Z); + float scaledZ = Unsafe.Add(ref cdfBase, originalZ) / noOfPixelsMinusCdfMin; + pixel.FromVector4(new Vector4(scaledX, scaledY, scaledZ, vector.W)); + } + } + } +} diff --git a/src/ImageSharp/Processing/Processors/Normalization/GlobalHistogramEqualizationProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Normalization/GlobalHistogramEqualizationProcessor{TPixel}.cs index 59c37373ea..d506777be0 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/GlobalHistogramEqualizationProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/GlobalHistogramEqualizationProcessor{TPixel}.cs @@ -51,7 +51,7 @@ internal class GlobalHistogramEqualizationProcessor : HistogramEqualizat using IMemoryOwner histogramBuffer = memoryAllocator.Allocate(this.LuminanceLevels, AllocationOptions.Clean); // Build the histogram of the grayscale levels. - var grayscaleOperation = new GrayscaleLevelsRowOperation(interest, histogramBuffer, source.PixelBuffer, this.LuminanceLevels); + var grayscaleOperation = new GrayscaleLevelsRowOperation(interest, histogramBuffer, source.PixelBuffer, this.LuminanceLevels); ParallelRowIterator.IterateRows( this.Configuration, interest, @@ -81,47 +81,6 @@ internal class GlobalHistogramEqualizationProcessor : HistogramEqualizat in cdfOperation); } - /// - /// A implementing the grayscale levels logic for . - /// - private readonly struct GrayscaleLevelsRowOperation : IRowOperation - { - private readonly Rectangle bounds; - private readonly IMemoryOwner histogramBuffer; - private readonly Buffer2D source; - private readonly int luminanceLevels; - - [MethodImpl(InliningOptions.ShortMethod)] - public GrayscaleLevelsRowOperation( - Rectangle bounds, - IMemoryOwner histogramBuffer, - Buffer2D source, - int luminanceLevels) - { - this.bounds = bounds; - this.histogramBuffer = histogramBuffer; - this.source = source; - this.luminanceLevels = luminanceLevels; - } - - /// - [MethodImpl(InliningOptions.ShortMethod)] - public void Invoke(int y) - { - ref int histogramBase = ref MemoryMarshal.GetReference(this.histogramBuffer.GetSpan()); - Span pixelRow = this.source.DangerousGetRowSpan(y); - int levels = this.luminanceLevels; - - for (int x = 0; x < this.bounds.Width; x++) - { - // TODO: We should bulk convert here. - var vector = pixelRow[x].ToVector4(); - int luminance = ColorNumerics.GetBT709Luminance(ref vector, levels); - Interlocked.Increment(ref Unsafe.Add(ref histogramBase, luminance)); - } - } - } - /// /// A implementing the cdf application levels logic for . /// diff --git a/src/ImageSharp/Processing/Processors/Normalization/GrayscaleLevelsRowOperation{TPixel}.cs b/src/ImageSharp/Processing/Processors/Normalization/GrayscaleLevelsRowOperation{TPixel}.cs new file mode 100644 index 0000000000..f4fcd15782 --- /dev/null +++ b/src/ImageSharp/Processing/Processors/Normalization/GrayscaleLevelsRowOperation{TPixel}.cs @@ -0,0 +1,53 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using System.Buffers; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using SixLabors.ImageSharp.Advanced; +using SixLabors.ImageSharp.Memory; +using SixLabors.ImageSharp.PixelFormats; + +namespace SixLabors.ImageSharp.Processing.Processors.Normalization; + +/// +/// A implementing the grayscale levels logic as . +/// +internal readonly struct GrayscaleLevelsRowOperation : IRowOperation + where TPixel : unmanaged, IPixel +{ + private readonly Rectangle bounds; + private readonly IMemoryOwner histogramBuffer; + private readonly Buffer2D source; + private readonly int luminanceLevels; + + [MethodImpl(InliningOptions.ShortMethod)] + public GrayscaleLevelsRowOperation( + Rectangle bounds, + IMemoryOwner histogramBuffer, + Buffer2D source, + int luminanceLevels) + { + this.bounds = bounds; + this.histogramBuffer = histogramBuffer; + this.source = source; + this.luminanceLevels = luminanceLevels; + } + + /// + [MethodImpl(InliningOptions.ShortMethod)] + public void Invoke(int y) + { + ref int histogramBase = ref MemoryMarshal.GetReference(this.histogramBuffer.GetSpan()); + Span pixelRow = this.source.DangerousGetRowSpan(y); + int levels = this.luminanceLevels; + + for (int x = 0; x < this.bounds.Width; x++) + { + // TODO: We should bulk convert here. + var vector = pixelRow[x].ToVector4(); + int luminance = ColorNumerics.GetBT709Luminance(ref vector, levels); + Interlocked.Increment(ref Unsafe.Add(ref histogramBase, luminance)); + } + } +} diff --git a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationMethod.cs b/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationMethod.cs index c8fb361398..e5cfd0dc78 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationMethod.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationMethod.cs @@ -22,4 +22,9 @@ public enum HistogramEqualizationMethod : int /// Adaptive histogram equalization using sliding window. Slower then the tile interpolation mode, but can yield to better results. /// AdaptiveSlidingWindow, + + /// + /// Global histogram equalization, but applied to each color channel separately. + /// + AutoLevel } diff --git a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationProcessor.cs b/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationProcessor.cs index f90a810790..d493d1734b 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationProcessor.cs @@ -60,6 +60,9 @@ public abstract class HistogramEqualizationProcessor : IImageProcessor HistogramEqualizationMethod.AdaptiveSlidingWindow => new AdaptiveHistogramEqualizationSlidingWindowProcessor(options.LuminanceLevels, options.ClipHistogram, options.ClipLimit, options.NumberOfTiles), + HistogramEqualizationMethod.AutoLevel + => new AutoLevelProcessor(options.LuminanceLevels, options.ClipHistogram, options.ClipLimit), + _ => new GlobalHistogramEqualizationProcessor(options.LuminanceLevels, options.ClipHistogram, options.ClipLimit), }; } diff --git a/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs b/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs index 09ba486a6f..9ef69f76e4 100644 --- a/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs +++ b/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs @@ -134,6 +134,24 @@ public class HistogramEqualizationTests } } + [Theory] + [WithFile(TestImages.Jpeg.Baseline.ForestBridgeDifferentComponentsQuality, PixelTypes.Rgba32)] + public void AutoLevel_CompareToReferenceOutput(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using (Image image = provider.GetImage()) + { + var options = new HistogramEqualizationOptions + { + Method = HistogramEqualizationMethod.AutoLevel, + LuminanceLevels = 256, + }; + image.Mutate(x => x.HistogramEqualization(options)); + image.DebugSave(provider); + image.CompareToReferenceOutput(ValidatorComparer, provider, extension: "png"); + } + } + /// /// This is regression test for a bug with the calculation of the y-start positions, /// where it could happen that one too much start position was calculated in some cases. diff --git a/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/AutoLevel_CompareToReferenceOutput_Rgba32_forest_bridge.png b/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/AutoLevel_CompareToReferenceOutput_Rgba32_forest_bridge.png new file mode 100644 index 0000000000..123cd582c9 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/AutoLevel_CompareToReferenceOutput_Rgba32_forest_bridge.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:25041d2dafe6c01cfec0ae3c2ec15046accd44c02b737a4cfa464ad5f61d01af +size 14107709 From 34891da46b32fa87bb4183112c6cfa2fda2015c8 Mon Sep 17 00:00:00 2001 From: Ynse Hoornenborg Date: Sat, 17 Sep 2022 14:18:34 +0200 Subject: [PATCH 04/17] Add SyncChannels option --- .../Normalization/AutoLevelProcessor.cs | 11 ++- .../AutoLevelProcessor{TPixel}.cs | 86 +++++++++++++++++-- .../HistogramEqualizationOptions.cs | 7 ++ .../HistogramEqualizationProcessor.cs | 2 +- .../HistogramEqualizationTests.cs | 22 ++++- ...oReferenceOutput_Rgba32_forest_bridge.png} | 0 ...ToReferenceOutput_Rgba32_forest_bridge.png | 3 + 7 files changed, 119 insertions(+), 12 deletions(-) rename tests/Images/External/ReferenceOutput/HistogramEqualizationTests/{AutoLevel_CompareToReferenceOutput_Rgba32_forest_bridge.png => AutoLevel_SeparateChannels_CompareToReferenceOutput_Rgba32_forest_bridge.png} (100%) create mode 100644 tests/Images/External/ReferenceOutput/HistogramEqualizationTests/AutoLevel_SynchronizedChannels_CompareToReferenceOutput_Rgba32_forest_bridge.png diff --git a/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor.cs b/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor.cs index b33e46ce37..2721efac6d 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor.cs @@ -17,14 +17,22 @@ public class AutoLevelProcessor : HistogramEqualizationProcessor /// or 65536 for 16-bit grayscale images. /// Indicating whether to clip the histogram bins at a specific value. /// The histogram clip limit. Histogram bins which exceed this limit, will be capped at this value. + /// Whether to apply a synchronized luminance value to each color channel. public AutoLevelProcessor( int luminanceLevels, bool clipHistogram, - int clipLimit) + int clipLimit, + bool syncChannels) : base(luminanceLevels, clipHistogram, clipLimit) { + this.SyncChannels = syncChannels; } + /// + /// Gets whether to apply a synchronized luminance value to each color channel. + /// + public bool SyncChannels { get; } + /// public override IImageProcessor CreatePixelSpecificProcessor(Configuration configuration, Image source, Rectangle sourceRectangle) => new AutoLevelProcessor( @@ -32,6 +40,7 @@ public class AutoLevelProcessor : HistogramEqualizationProcessor this.LuminanceLevels, this.ClipHistogram, this.ClipLimit, + this.SyncChannels, source, sourceRectangle); } diff --git a/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs index bdb2a500b5..28a799a61e 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs @@ -30,17 +30,25 @@ internal class AutoLevelProcessor : HistogramEqualizationProcessorThe histogram clip limit. Histogram bins which exceed this limit, will be capped at this value. /// The source for the current processor instance. /// The source area to process for the current processor instance. + /// Whether to apply a synchronized luminance value to each color channel. public AutoLevelProcessor( Configuration configuration, int luminanceLevels, bool clipHistogram, int clipLimit, + bool syncChannels, Image source, Rectangle sourceRectangle) : base(configuration, luminanceLevels, clipHistogram, clipLimit, source, sourceRectangle) { + this.SyncChannels = syncChannels; } + /// + /// Gets whether to apply a synchronized luminance value to each color channel. + /// + private bool SyncChannels { get; } + /// protected override void OnFrameApply(ImageFrame source) { @@ -73,18 +81,78 @@ internal class AutoLevelProcessor : HistogramEqualizationProcessor + /// A implementing the cdf logic for synchronized color channels. + /// + private readonly struct SynchronizedChannelsRowOperation : IRowOperation + { + private readonly Rectangle bounds; + private readonly IMemoryOwner cdfBuffer; + private readonly Buffer2D source; + private readonly int luminanceLevels; + private readonly float numberOfPixelsMinusCdfMin; + + [MethodImpl(InliningOptions.ShortMethod)] + public SynchronizedChannelsRowOperation( + Rectangle bounds, + IMemoryOwner cdfBuffer, + Buffer2D source, + int luminanceLevels, + float numberOfPixelsMinusCdfMin) + { + this.bounds = bounds; + this.cdfBuffer = cdfBuffer; + this.source = source; + this.luminanceLevels = luminanceLevels; + this.numberOfPixelsMinusCdfMin = numberOfPixelsMinusCdfMin; + } + + /// + [MethodImpl(InliningOptions.ShortMethod)] + public void Invoke(int y) + { + ref int cdfBase = ref MemoryMarshal.GetReference(this.cdfBuffer.GetSpan()); + var sourceAccess = new PixelAccessor(this.source); + Span pixelRow = sourceAccess.GetRowSpan(y); + int levels = this.luminanceLevels; + float noOfPixelsMinusCdfMin = this.numberOfPixelsMinusCdfMin; + + for (int x = 0; x < this.bounds.Width; x++) + { + // TODO: We should bulk convert here. + ref TPixel pixel = ref pixelRow[x]; + var vector = pixel.ToVector4(); + int luminance = ColorNumerics.GetBT709Luminance(ref vector, levels); + float scaledLuminance = Unsafe.Add(ref cdfBase, luminance) / noOfPixelsMinusCdfMin; + float scalingFactor = scaledLuminance * levels / luminance; + Vector4 scaledVector = new Vector4(scalingFactor * vector.X, scalingFactor * vector.Y, scalingFactor * vector.Z, vector.W); + pixel.FromVector4(scaledVector); + } + } } /// - /// A implementing the cdf application levels logic for . + /// A implementing the cdf logic for separate color channels. /// - private readonly struct CdfApplicationRowOperation : IRowOperation + private readonly struct SeperateChannelsRowOperation : IRowOperation { private readonly Rectangle bounds; private readonly IMemoryOwner cdfBuffer; @@ -93,7 +161,7 @@ internal class AutoLevelProcessor : HistogramEqualizationProcessor cdfBuffer, Buffer2D source, diff --git a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationOptions.cs b/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationOptions.cs index 6343788425..1736a067ae 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationOptions.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationOptions.cs @@ -42,4 +42,11 @@ public class HistogramEqualizationOptions /// Defaults to 8. /// public int NumberOfTiles { get; set; } = 8; + + /// + /// Gets or sets a value indicating whether to synchronize the scaling factor over all color channels. + /// This parameter is only applicable to AutoLevel and is ignored for all others. + /// Defaults to true. + /// + public bool SyncChannels { get; set; } = true; } diff --git a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationProcessor.cs b/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationProcessor.cs index d493d1734b..8a9056b1f3 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationProcessor.cs @@ -61,7 +61,7 @@ public abstract class HistogramEqualizationProcessor : IImageProcessor => new AdaptiveHistogramEqualizationSlidingWindowProcessor(options.LuminanceLevels, options.ClipHistogram, options.ClipLimit, options.NumberOfTiles), HistogramEqualizationMethod.AutoLevel - => new AutoLevelProcessor(options.LuminanceLevels, options.ClipHistogram, options.ClipLimit), + => new AutoLevelProcessor(options.LuminanceLevels, options.ClipHistogram, options.ClipLimit, options.SyncChannels), _ => new GlobalHistogramEqualizationProcessor(options.LuminanceLevels, options.ClipHistogram, options.ClipLimit), }; diff --git a/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs b/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs index 9ef69f76e4..60e33835af 100644 --- a/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs +++ b/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs @@ -136,7 +136,7 @@ public class HistogramEqualizationTests [Theory] [WithFile(TestImages.Jpeg.Baseline.ForestBridgeDifferentComponentsQuality, PixelTypes.Rgba32)] - public void AutoLevel_CompareToReferenceOutput(TestImageProvider provider) + public void AutoLevel_SeparateChannels_CompareToReferenceOutput(TestImageProvider provider) where TPixel : unmanaged, IPixel { using (Image image = provider.GetImage()) @@ -145,6 +145,26 @@ public class HistogramEqualizationTests { Method = HistogramEqualizationMethod.AutoLevel, LuminanceLevels = 256, + SyncChannels = false + }; + image.Mutate(x => x.HistogramEqualization(options)); + image.DebugSave(provider); + image.CompareToReferenceOutput(ValidatorComparer, provider, extension: "png"); + } + } + + [Theory] + [WithFile(TestImages.Jpeg.Baseline.ForestBridgeDifferentComponentsQuality, PixelTypes.Rgba32)] + public void AutoLevel_SynchronizedChannels_CompareToReferenceOutput(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using (Image image = provider.GetImage()) + { + var options = new HistogramEqualizationOptions + { + Method = HistogramEqualizationMethod.AutoLevel, + LuminanceLevels = 256, + SyncChannels = true }; image.Mutate(x => x.HistogramEqualization(options)); image.DebugSave(provider); diff --git a/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/AutoLevel_CompareToReferenceOutput_Rgba32_forest_bridge.png b/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/AutoLevel_SeparateChannels_CompareToReferenceOutput_Rgba32_forest_bridge.png similarity index 100% rename from tests/Images/External/ReferenceOutput/HistogramEqualizationTests/AutoLevel_CompareToReferenceOutput_Rgba32_forest_bridge.png rename to tests/Images/External/ReferenceOutput/HistogramEqualizationTests/AutoLevel_SeparateChannels_CompareToReferenceOutput_Rgba32_forest_bridge.png diff --git a/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/AutoLevel_SynchronizedChannels_CompareToReferenceOutput_Rgba32_forest_bridge.png b/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/AutoLevel_SynchronizedChannels_CompareToReferenceOutput_Rgba32_forest_bridge.png new file mode 100644 index 0000000000..ff5b35a5f7 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/AutoLevel_SynchronizedChannels_CompareToReferenceOutput_Rgba32_forest_bridge.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:dca9b5b890d3a79b0002b7093d254d484ada4207e5010d1f0c6248d4dd6e22db +size 13909894 From c086afd273537ee8018635e875b989c3efd30daa Mon Sep 17 00:00:00 2001 From: Ynse Hoornenborg Date: Sat, 17 Sep 2022 14:43:02 +0200 Subject: [PATCH 05/17] Fix SA1623 warnings --- .../Processing/Processors/Normalization/AutoLevelProcessor.cs | 2 +- .../Processors/Normalization/AutoLevelProcessor{TPixel}.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor.cs b/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor.cs index 2721efac6d..43609367ed 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor.cs @@ -29,7 +29,7 @@ public class AutoLevelProcessor : HistogramEqualizationProcessor } /// - /// Gets whether to apply a synchronized luminance value to each color channel. + /// Gets a value indicating whether to apply a synchronized luminance value to each color channel. /// public bool SyncChannels { get; } diff --git a/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs index 28a799a61e..fb3de130a9 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs @@ -45,7 +45,7 @@ internal class AutoLevelProcessor : HistogramEqualizationProcessor - /// Gets whether to apply a synchronized luminance value to each color channel. + /// Gets a value indicating whether to apply a synchronized luminance value to each color channel. /// private bool SyncChannels { get; } From 44477a9c25c4c26b713488dda54da90f8849f663 Mon Sep 17 00:00:00 2001 From: Ynse Hoornenborg Date: Fri, 23 Sep 2022 12:32:34 +0200 Subject: [PATCH 06/17] Test to compare behavior to ImageMagick --- .../Normalization/MagickCompareTests.cs | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 tests/ImageSharp.Tests/Processing/Normalization/MagickCompareTests.cs diff --git a/tests/ImageSharp.Tests/Processing/Normalization/MagickCompareTests.cs b/tests/ImageSharp.Tests/Processing/Normalization/MagickCompareTests.cs new file mode 100644 index 0000000000..6b01624906 --- /dev/null +++ b/tests/ImageSharp.Tests/Processing/Normalization/MagickCompareTests.cs @@ -0,0 +1,78 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Processing; +using SixLabors.ImageSharp.Processing.Processors.Normalization; +using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; + +using ImageMagick; + +namespace SixLabors.ImageSharp.Tests.Processing.Normalization; + +// ReSharper disable InconsistentNaming +[Trait("Category", "Processors")] +public class MagickCompareTests +{ + [Theory] + [WithFile(TestImages.Jpeg.Baseline.ForestBridgeDifferentComponentsQuality, PixelTypes.Rgba32)] + public void AutoLevel_CompareToMagick(TestImageProvider provider) + where TPixel : unmanaged, ImageSharp.PixelFormats.IPixel + { + using Stream stream = LoadAsStream(provider); + var magickImage = new MagickImage(stream); + + // Apply Auto Level using the Grey (BT.709) channel. + magickImage.AutoLevel(Channels.Gray); + Image imageFromMagick = ConvertImageFromMagick(magickImage); + + using (Image image = provider.GetImage()) + { + var options = new HistogramEqualizationOptions + { + Method = HistogramEqualizationMethod.AutoLevel, + LuminanceLevels = 256, + SyncChannels = true + }; + image.Mutate(x => x.HistogramEqualization(options)); + image.DebugSave(provider); + ExactImageComparer.Instance.CompareImages(imageFromMagick, image); + } + } + + private Stream LoadAsStream(TestImageProvider provider) + where TPixel : unmanaged, ImageSharp.PixelFormats.IPixel + { + string path = TestImageProvider.GetFilePathOrNull(provider); + if (path == null) + { + throw new InvalidOperationException("CompareToMagick() works only with file providers!"); + } + + var testFile = TestFile.Create(path); + return new FileStream(testFile.FullPath, FileMode.Open); + } + + private Image ConvertImageFromMagick(MagickImage magickImage) + where TPixel : unmanaged, ImageSharp.PixelFormats.IPixel + { + Configuration configuration = Configuration.Default.Clone(); + configuration.PreferContiguousImageBuffers = true; + var result = new Image(configuration, magickImage.Width, magickImage.Height); + + Assert.True(result.DangerousTryGetSinglePixelMemory(out Memory resultPixels)); + + using (IUnsafePixelCollection pixels = magickImage.GetPixelsUnsafe()) + { + byte[] data = pixels.ToByteArray(PixelMapping.RGBA); + + PixelOperations.Instance.FromRgba32Bytes( + configuration, + data, + resultPixels.Span, + resultPixels.Length); + } + + return result; + } +} From 41fbfb4e5f2c47cea13de57aced62e7bb78cbf04 Mon Sep 17 00:00:00 2001 From: Ynse Hoornenborg Date: Sat, 24 Sep 2022 11:01:31 +0200 Subject: [PATCH 07/17] Rephrase comment to align with ImageMagick --- .../Processors/Normalization/HistogramEqualizationMethod.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationMethod.cs b/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationMethod.cs index e5cfd0dc78..e31ca62795 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationMethod.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationMethod.cs @@ -24,7 +24,8 @@ public enum HistogramEqualizationMethod : int AdaptiveSlidingWindow, /// - /// Global histogram equalization, but applied to each color channel separately. + /// Adjusts the brightness levels of a particular image by scaling to the + /// minimum and maximum values to the full brightness range. /// AutoLevel } From 86da190455c06c0915cea75a6c3a7d8e8d61e7cd Mon Sep 17 00:00:00 2001 From: Ynse Hoornenborg Date: Sat, 24 Sep 2022 11:33:12 +0200 Subject: [PATCH 08/17] Typo --- .../Processors/Normalization/HistogramEqualizationMethod.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationMethod.cs b/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationMethod.cs index e31ca62795..e104734820 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationMethod.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationMethod.cs @@ -24,7 +24,7 @@ public enum HistogramEqualizationMethod : int AdaptiveSlidingWindow, /// - /// Adjusts the brightness levels of a particular image by scaling to the + /// Adjusts the brightness levels of a particular image by scaling the /// minimum and maximum values to the full brightness range. /// AutoLevel From 80def2642e63a5af4b317279bbe8bcb5d98710af Mon Sep 17 00:00:00 2001 From: Ynse Hoornenborg Date: Sat, 24 Sep 2022 11:59:45 +0200 Subject: [PATCH 09/17] Prevent file reading issues in windows --- .../Processing/Normalization/MagickCompareTests.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/ImageSharp.Tests/Processing/Normalization/MagickCompareTests.cs b/tests/ImageSharp.Tests/Processing/Normalization/MagickCompareTests.cs index 6b01624906..5fb0a4e934 100644 --- a/tests/ImageSharp.Tests/Processing/Normalization/MagickCompareTests.cs +++ b/tests/ImageSharp.Tests/Processing/Normalization/MagickCompareTests.cs @@ -19,12 +19,15 @@ public class MagickCompareTests public void AutoLevel_CompareToMagick(TestImageProvider provider) where TPixel : unmanaged, ImageSharp.PixelFormats.IPixel { - using Stream stream = LoadAsStream(provider); - var magickImage = new MagickImage(stream); + Image imageFromMagick; + using (Stream stream = LoadAsStream(provider)) + { + var magickImage = new MagickImage(stream); - // Apply Auto Level using the Grey (BT.709) channel. - magickImage.AutoLevel(Channels.Gray); - Image imageFromMagick = ConvertImageFromMagick(magickImage); + // Apply Auto Level using the Grey (BT.709) channel. + magickImage.AutoLevel(Channels.Gray); + imageFromMagick = ConvertImageFromMagick(magickImage); + } using (Image image = provider.GetImage()) { From 3912bda111537fded67be805b12fa01b313808b3 Mon Sep 17 00:00:00 2001 From: Ynse Hoornenborg Date: Sun, 25 Sep 2022 11:47:19 +0200 Subject: [PATCH 10/17] Fix overflow artifacts --- .../Processors/Normalization/AutoLevelProcessor{TPixel}.cs | 4 ++-- ...Channels_CompareToReferenceOutput_Rgba32_forest_bridge.png | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs index fb3de130a9..856fba3dcb 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs @@ -182,14 +182,14 @@ internal class AutoLevelProcessor : HistogramEqualizationProcessor(this.source); Span pixelRow = sourceAccess.GetRowSpan(y); - int levels = this.luminanceLevels; + int levelsMinusOne = this.luminanceLevels - 1; float noOfPixelsMinusCdfMin = this.numberOfPixelsMinusCdfMin; for (int x = 0; x < this.bounds.Width; x++) { // TODO: We should bulk convert here. ref TPixel pixel = ref pixelRow[x]; - var vector = pixel.ToVector4() * levels; + var vector = pixel.ToVector4() * levelsMinusOne; uint originalX = (uint)MathF.Round(vector.X); float scaledX = Unsafe.Add(ref cdfBase, originalX) / noOfPixelsMinusCdfMin; diff --git a/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/AutoLevel_SeparateChannels_CompareToReferenceOutput_Rgba32_forest_bridge.png b/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/AutoLevel_SeparateChannels_CompareToReferenceOutput_Rgba32_forest_bridge.png index 123cd582c9..de79ec729c 100644 --- a/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/AutoLevel_SeparateChannels_CompareToReferenceOutput_Rgba32_forest_bridge.png +++ b/tests/Images/External/ReferenceOutput/HistogramEqualizationTests/AutoLevel_SeparateChannels_CompareToReferenceOutput_Rgba32_forest_bridge.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:25041d2dafe6c01cfec0ae3c2ec15046accd44c02b737a4cfa464ad5f61d01af +oid sha256:aada4a2ccf45de24f2a591a18d9bc0260ceb3829e104fee6982061013ed87282 size 14107709 From d42d8df88d72c2b76199ea36ebd0a088c26395bf Mon Sep 17 00:00:00 2001 From: Ynse Hoornenborg Date: Mon, 26 Sep 2022 20:52:10 +0200 Subject: [PATCH 11/17] Introduce GetRequiredBufferLength method --- .../Advanced/IRowIntervalOperation{TBuffer}.cs | 7 +++++++ .../Advanced/IRowOperation{TBuffer}.cs | 7 +++++++ .../Advanced/ParallelRowIterator.Wrappers.cs | 16 ++++++++-------- src/ImageSharp/Advanced/ParallelRowIterator.cs | 10 ++++++---- .../AdaptiveThresholdProcessor{TPixel}.cs | 5 +++++ .../BinaryThresholdProcessor{TPixel}.cs | 5 +++++ .../Convolution/BokehBlurProcessor{TPixel}.cs | 17 +++++++++++++++++ .../Convolution2DRowOperation{TPixel}.cs | 5 +++++ .../Convolution2PassProcessor{TPixel}.cs | 10 ++++++++++ .../Convolution/ConvolutionProcessor{TPixel}.cs | 5 +++++ .../Convolution/MedianRowOperation{TPixel}.cs | 5 +++++ ...xelRowDelegateProcessor{TPixel,TDelegate}.cs | 5 +++++ .../Filters/FilterProcessor{TPixel}.cs | 5 +++++ .../Filters/OpaqueProcessor{TPixel}.cs | 5 +++++ .../Overlays/GlowProcessor{TPixel}.cs | 5 +++++ .../Overlays/VignetteProcessor{TPixel}.cs | 5 +++++ .../Linear/AffineTransformProcessor{TPixel}.cs | 5 +++++ .../ProjectiveTransformProcessor{TPixel}.cs | 5 +++++ .../Helpers/ParallelRowIteratorTests.cs | 6 ++++++ .../TestUtilities/TestImageExtensions.cs | 3 +++ 20 files changed, 124 insertions(+), 12 deletions(-) diff --git a/src/ImageSharp/Advanced/IRowIntervalOperation{TBuffer}.cs b/src/ImageSharp/Advanced/IRowIntervalOperation{TBuffer}.cs index 3d61eb7333..ef8ddb3137 100644 --- a/src/ImageSharp/Advanced/IRowIntervalOperation{TBuffer}.cs +++ b/src/ImageSharp/Advanced/IRowIntervalOperation{TBuffer}.cs @@ -12,6 +12,13 @@ namespace SixLabors.ImageSharp.Advanced; public interface IRowIntervalOperation where TBuffer : unmanaged { + /// + /// Return the minimal required number of items in the buffer passed on . + /// + /// The bounds of the operation. + /// The required buffer length. + int GetRequiredBufferLength(Rectangle bounds); + /// /// Invokes the method passing the row interval and a buffer. /// diff --git a/src/ImageSharp/Advanced/IRowOperation{TBuffer}.cs b/src/ImageSharp/Advanced/IRowOperation{TBuffer}.cs index 3b6a3eb0c5..8b46fc5c31 100644 --- a/src/ImageSharp/Advanced/IRowOperation{TBuffer}.cs +++ b/src/ImageSharp/Advanced/IRowOperation{TBuffer}.cs @@ -10,6 +10,13 @@ namespace SixLabors.ImageSharp.Advanced; public interface IRowOperation where TBuffer : unmanaged { + /// + /// Return the minimal required number of items in the buffer passed on . + /// + /// The bounds of the operation. + /// The required buffer length. + int GetRequiredBufferLength(Rectangle bounds); + /// /// Invokes the method passing the row and a buffer. /// diff --git a/src/ImageSharp/Advanced/ParallelRowIterator.Wrappers.cs b/src/ImageSharp/Advanced/ParallelRowIterator.Wrappers.cs index 9e5099b893..9629b0097e 100644 --- a/src/ImageSharp/Advanced/ParallelRowIterator.Wrappers.cs +++ b/src/ImageSharp/Advanced/ParallelRowIterator.Wrappers.cs @@ -63,7 +63,7 @@ public static partial class ParallelRowIterator private readonly int minY; private readonly int maxY; private readonly int stepY; - private readonly int width; + private readonly int bufferLength; private readonly MemoryAllocator allocator; private readonly T action; @@ -72,14 +72,14 @@ public static partial class ParallelRowIterator int minY, int maxY, int stepY, - int width, + int bufferLength, MemoryAllocator allocator, in T action) { this.minY = minY; this.maxY = maxY; this.stepY = stepY; - this.width = width; + this.bufferLength = bufferLength; this.allocator = allocator; this.action = action; } @@ -96,7 +96,7 @@ public static partial class ParallelRowIterator int yMax = Math.Min(yMin + this.stepY, this.maxY); - using IMemoryOwner buffer = this.allocator.Allocate(this.width); + using IMemoryOwner buffer = this.allocator.Allocate(this.bufferLength); Span span = buffer.Memory.Span; @@ -153,7 +153,7 @@ public static partial class ParallelRowIterator private readonly int minY; private readonly int maxY; private readonly int stepY; - private readonly int width; + private readonly int bufferLength; private readonly MemoryAllocator allocator; private readonly T operation; @@ -162,14 +162,14 @@ public static partial class ParallelRowIterator int minY, int maxY, int stepY, - int width, + int bufferLength, MemoryAllocator allocator, in T operation) { this.minY = minY; this.maxY = maxY; this.stepY = stepY; - this.width = width; + this.bufferLength = bufferLength; this.allocator = allocator; this.operation = operation; } @@ -187,7 +187,7 @@ public static partial class ParallelRowIterator int yMax = Math.Min(yMin + this.stepY, this.maxY); var rows = new RowInterval(yMin, yMax); - using IMemoryOwner buffer = this.allocator.Allocate(this.width); + using IMemoryOwner buffer = this.allocator.Allocate(this.bufferLength); Unsafe.AsRef(in this.operation).Invoke(in rows, buffer.Memory.Span); } diff --git a/src/ImageSharp/Advanced/ParallelRowIterator.cs b/src/ImageSharp/Advanced/ParallelRowIterator.cs index 21736b71f0..0eb5952a63 100644 --- a/src/ImageSharp/Advanced/ParallelRowIterator.cs +++ b/src/ImageSharp/Advanced/ParallelRowIterator.cs @@ -118,11 +118,12 @@ public static partial class ParallelRowIterator int maxSteps = DivideCeil(width * height, parallelSettings.MinimumPixelsProcessedPerTask); int numOfSteps = Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps); MemoryAllocator allocator = parallelSettings.MemoryAllocator; + int bufferLength = Unsafe.AsRef(operation).GetRequiredBufferLength(rectangle); // Avoid TPL overhead in this trivial case: if (numOfSteps == 1) { - using IMemoryOwner buffer = allocator.Allocate(width); + using IMemoryOwner buffer = allocator.Allocate(bufferLength); Span span = buffer.Memory.Span; for (int y = top; y < bottom; y++) @@ -135,7 +136,7 @@ public static partial class ParallelRowIterator int verticalStep = DivideCeil(height, numOfSteps); var parallelOptions = new ParallelOptions { MaxDegreeOfParallelism = numOfSteps }; - var wrappingOperation = new RowOperationWrapper(top, bottom, verticalStep, width, allocator, in operation); + var wrappingOperation = new RowOperationWrapper(top, bottom, verticalStep, bufferLength, allocator, in operation); Parallel.For( 0, @@ -244,12 +245,13 @@ public static partial class ParallelRowIterator int maxSteps = DivideCeil(width * height, parallelSettings.MinimumPixelsProcessedPerTask); int numOfSteps = Math.Min(parallelSettings.MaxDegreeOfParallelism, maxSteps); MemoryAllocator allocator = parallelSettings.MemoryAllocator; + int bufferLength = Unsafe.AsRef(operation).GetRequiredBufferLength(rectangle); // Avoid TPL overhead in this trivial case: if (numOfSteps == 1) { var rows = new RowInterval(top, bottom); - using IMemoryOwner buffer = allocator.Allocate(width); + using IMemoryOwner buffer = allocator.Allocate(bufferLength); Unsafe.AsRef(operation).Invoke(in rows, buffer.Memory.Span); @@ -258,7 +260,7 @@ public static partial class ParallelRowIterator int verticalStep = DivideCeil(height, numOfSteps); var parallelOptions = new ParallelOptions { MaxDegreeOfParallelism = numOfSteps }; - var wrappingOperation = new RowIntervalOperationWrapper(top, bottom, verticalStep, width, allocator, in operation); + var wrappingOperation = new RowIntervalOperationWrapper(top, bottom, verticalStep, bufferLength, allocator, in operation); Parallel.For( 0, diff --git a/src/ImageSharp/Processing/Processors/Binarization/AdaptiveThresholdProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Binarization/AdaptiveThresholdProcessor{TPixel}.cs index e3b2025aa5..73c7c3302d 100644 --- a/src/ImageSharp/Processing/Processors/Binarization/AdaptiveThresholdProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Binarization/AdaptiveThresholdProcessor{TPixel}.cs @@ -85,6 +85,11 @@ internal class AdaptiveThresholdProcessor : ImageProcessor this.clusterSize = clusterSize; } + /// + [MethodImpl(InliningOptions.ShortMethod)] + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + /// [MethodImpl(InliningOptions.ShortMethod)] public void Invoke(int y, Span span) diff --git a/src/ImageSharp/Processing/Processors/Binarization/BinaryThresholdProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Binarization/BinaryThresholdProcessor{TPixel}.cs index 1a35973fbd..b710243a56 100644 --- a/src/ImageSharp/Processing/Processors/Binarization/BinaryThresholdProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Binarization/BinaryThresholdProcessor{TPixel}.cs @@ -86,6 +86,11 @@ internal class BinaryThresholdProcessor : ImageProcessor this.configuration = configuration; } + /// + [MethodImpl(InliningOptions.ShortMethod)] + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + /// [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Invoke(int y, Span span) diff --git a/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor{TPixel}.cs index 7e7cf81385..55c03abe6a 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/BokehBlurProcessor{TPixel}.cs @@ -220,6 +220,11 @@ internal class BokehBlurProcessor : ImageProcessor this.configuration = configuration; } + /// + [MethodImpl(InliningOptions.ShortMethod)] + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + /// [MethodImpl(InliningOptions.ShortMethod)] public void Invoke(int y, Span span) @@ -289,6 +294,11 @@ internal class BokehBlurProcessor : ImageProcessor this.gamma = gamma; } + /// + [MethodImpl(InliningOptions.ShortMethod)] + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + /// [MethodImpl(InliningOptions.ShortMethod)] public void Invoke(int y, Span span) @@ -329,6 +339,13 @@ internal class BokehBlurProcessor : ImageProcessor this.configuration = configuration; } + /// + [MethodImpl(InliningOptions.ShortMethod)] + public int GetRequiredBufferLength(Rectangle bounds) + { + return bounds.Width; + } + /// [MethodImpl(InliningOptions.ShortMethod)] public void Invoke(int y, Span span) diff --git a/src/ImageSharp/Processing/Processors/Convolution/Convolution2DRowOperation{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/Convolution2DRowOperation{TPixel}.cs index caeb225731..c04581444f 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/Convolution2DRowOperation{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/Convolution2DRowOperation{TPixel}.cs @@ -46,6 +46,11 @@ internal readonly struct Convolution2DRowOperation : IRowOperation + [MethodImpl(InliningOptions.ShortMethod)] + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + /// [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Invoke(int y, Span span) diff --git a/src/ImageSharp/Processing/Processors/Convolution/Convolution2PassProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/Convolution2PassProcessor{TPixel}.cs index 7aa83b0dd0..fa05a98fd1 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/Convolution2PassProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/Convolution2PassProcessor{TPixel}.cs @@ -143,6 +143,11 @@ internal class Convolution2PassProcessor : ImageProcessor this.preserveAlpha = preserveAlpha; } + /// + [MethodImpl(InliningOptions.ShortMethod)] + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + /// [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Invoke(int y, Span span) @@ -304,6 +309,11 @@ internal class Convolution2PassProcessor : ImageProcessor this.preserveAlpha = preserveAlpha; } + /// + [MethodImpl(InliningOptions.ShortMethod)] + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + /// [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Invoke(int y, Span span) diff --git a/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessor{TPixel}.cs index d7a8f743c7..f20cca3f71 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessor{TPixel}.cs @@ -106,6 +106,11 @@ internal class ConvolutionProcessor : ImageProcessor this.preserveAlpha = preserveAlpha; } + /// + [MethodImpl(InliningOptions.ShortMethod)] + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + /// [MethodImpl(InliningOptions.ShortMethod)] public void Invoke(int y, Span span) diff --git a/src/ImageSharp/Processing/Processors/Convolution/MedianRowOperation{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/MedianRowOperation{TPixel}.cs index aff26865fb..17df7b9be4 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/MedianRowOperation{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/MedianRowOperation{TPixel}.cs @@ -43,6 +43,11 @@ internal readonly struct MedianRowOperation : IRowOperation this.wChannelStart = this.zChannelStart + kernelCount; } + /// + [MethodImpl(InliningOptions.ShortMethod)] + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + public void Invoke(int y, Span span) { // Span has kernelSize^2 followed by bound width. diff --git a/src/ImageSharp/Processing/Processors/Effects/PixelRowDelegateProcessor{TPixel,TDelegate}.cs b/src/ImageSharp/Processing/Processors/Effects/PixelRowDelegateProcessor{TPixel,TDelegate}.cs index e8c7468911..f59b95050e 100644 --- a/src/ImageSharp/Processing/Processors/Effects/PixelRowDelegateProcessor{TPixel,TDelegate}.cs +++ b/src/ImageSharp/Processing/Processors/Effects/PixelRowDelegateProcessor{TPixel,TDelegate}.cs @@ -83,6 +83,11 @@ internal sealed class PixelRowDelegateProcessor : ImageProces this.rowProcessor = rowProcessor; } + /// + [MethodImpl(InliningOptions.ShortMethod)] + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + /// [MethodImpl(InliningOptions.ShortMethod)] public void Invoke(int y, Span span) diff --git a/src/ImageSharp/Processing/Processors/Filters/FilterProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Filters/FilterProcessor{TPixel}.cs index e61b528efb..5ad245e3ce 100644 --- a/src/ImageSharp/Processing/Processors/Filters/FilterProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Filters/FilterProcessor{TPixel}.cs @@ -66,6 +66,11 @@ internal class FilterProcessor : ImageProcessor this.configuration = configuration; } + /// + [MethodImpl(InliningOptions.ShortMethod)] + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + /// [MethodImpl(InliningOptions.ShortMethod)] public void Invoke(int y, Span span) diff --git a/src/ImageSharp/Processing/Processors/Filters/OpaqueProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Filters/OpaqueProcessor{TPixel}.cs index 30ffa7899d..4b8fd9056e 100644 --- a/src/ImageSharp/Processing/Processors/Filters/OpaqueProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Filters/OpaqueProcessor{TPixel}.cs @@ -46,6 +46,11 @@ internal sealed class OpaqueProcessor : ImageProcessor this.bounds = bounds; } + /// + [MethodImpl(InliningOptions.ShortMethod)] + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + /// [MethodImpl(InliningOptions.ShortMethod)] public void Invoke(int y, Span span) diff --git a/src/ImageSharp/Processing/Processors/Overlays/GlowProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Overlays/GlowProcessor{TPixel}.cs index c431650b33..19ce6c417d 100644 --- a/src/ImageSharp/Processing/Processors/Overlays/GlowProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Overlays/GlowProcessor{TPixel}.cs @@ -93,6 +93,11 @@ internal class GlowProcessor : ImageProcessor this.source = source; } + /// + [MethodImpl(InliningOptions.ShortMethod)] + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + [MethodImpl(InliningOptions.ShortMethod)] public void Invoke(int y, Span span) { diff --git a/src/ImageSharp/Processing/Processors/Overlays/VignetteProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Overlays/VignetteProcessor{TPixel}.cs index c69b6360d5..a327deec1c 100644 --- a/src/ImageSharp/Processing/Processors/Overlays/VignetteProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Overlays/VignetteProcessor{TPixel}.cs @@ -101,6 +101,11 @@ internal class VignetteProcessor : ImageProcessor this.source = source; } + /// + [MethodImpl(InliningOptions.ShortMethod)] + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + [MethodImpl(InliningOptions.ShortMethod)] public void Invoke(int y, Span span) { diff --git a/src/ImageSharp/Processing/Processors/Transforms/Linear/AffineTransformProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/Linear/AffineTransformProcessor{TPixel}.cs index 8add73d33c..bed7dcccb7 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Linear/AffineTransformProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Linear/AffineTransformProcessor{TPixel}.cs @@ -176,6 +176,11 @@ internal class AffineTransformProcessor : TransformProcessor, IR this.xRadius = LinearTransformUtility.GetSamplingRadius(in sampler, source.Width, destination.Width); } + /// + [MethodImpl(InliningOptions.ShortMethod)] + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + [MethodImpl(InliningOptions.ShortMethod)] public void Invoke(in RowInterval rows, Span span) { diff --git a/src/ImageSharp/Processing/Processors/Transforms/Linear/ProjectiveTransformProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/Linear/ProjectiveTransformProcessor{TPixel}.cs index 440becf833..14236e3c2b 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Linear/ProjectiveTransformProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Linear/ProjectiveTransformProcessor{TPixel}.cs @@ -176,6 +176,11 @@ internal class ProjectiveTransformProcessor : TransformProcessor this.xRadius = LinearTransformUtility.GetSamplingRadius(in sampler, bounds.Width, destination.Width); } + /// + [MethodImpl(InliningOptions.ShortMethod)] + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + [MethodImpl(InliningOptions.ShortMethod)] public void Invoke(in RowInterval rows, Span span) { diff --git a/tests/ImageSharp.Tests/Helpers/ParallelRowIteratorTests.cs b/tests/ImageSharp.Tests/Helpers/ParallelRowIteratorTests.cs index e77d8fee42..1700b4a734 100644 --- a/tests/ImageSharp.Tests/Helpers/ParallelRowIteratorTests.cs +++ b/tests/ImageSharp.Tests/Helpers/ParallelRowIteratorTests.cs @@ -413,6 +413,9 @@ public class ParallelRowIteratorTests public TestRowIntervalOperation(Action action) => this.action = action; + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + public void Invoke(in RowInterval rows) => this.action(rows); } @@ -424,6 +427,9 @@ public class ParallelRowIteratorTests public TestRowIntervalOperation(RowIntervalAction action) => this.action = action; + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + public void Invoke(in RowInterval rows, Span span) => this.action(rows, span); } diff --git a/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs b/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs index 5032f8de58..0201848350 100644 --- a/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs +++ b/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs @@ -755,6 +755,9 @@ public static class TestImageExtensions this.source = source; } + public int GetRequiredBufferLength(Rectangle bounds) + => bounds.Width; + public void Invoke(in RowInterval rows, Span span) { for (int y = rows.Min; y < rows.Max; y++) From 05588d5d344292c8963bae1a5f6e55ad49c7df88 Mon Sep 17 00:00:00 2001 From: Ynse Hoornenborg Date: Mon, 26 Sep 2022 22:01:21 +0200 Subject: [PATCH 12/17] Buffer length defined by RowOperation struct --- .../Convolution/Convolution2DProcessor{TPixel}.cs | 6 +----- .../Convolution/Convolution2DRowOperation{TPixel}.cs | 2 +- .../Convolution/Convolution2PassProcessor{TPixel}.cs | 12 ++++-------- .../Convolution/ConvolutionProcessor{TPixel}.cs | 7 ++----- .../Convolution/MedianBlurProcessor{TPixel}.cs | 7 +------ .../Convolution/MedianRowOperation{TPixel}.cs | 4 ++-- 6 files changed, 11 insertions(+), 27 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Convolution/Convolution2DProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/Convolution2DProcessor{TPixel}.cs index b8a61e5a32..8a7c424815 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/Convolution2DProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/Convolution2DProcessor{TPixel}.cs @@ -64,10 +64,6 @@ internal class Convolution2DProcessor : ImageProcessor var interest = Rectangle.Intersect(this.SourceRectangle, source.Bounds()); - // We use a rectangle 3x the interest width to allocate a buffer big enough - // for source and target bulk pixel conversion. - var operationBounds = new Rectangle(interest.X, interest.Y, interest.Width * 3, interest.Height); - using (var map = new KernelSamplingMap(allocator)) { // Since the kernel sizes are identical we can use a single map. @@ -85,7 +81,7 @@ internal class Convolution2DProcessor : ImageProcessor ParallelRowIterator.IterateRows, Vector4>( this.Configuration, - operationBounds, + interest, in operation); } diff --git a/src/ImageSharp/Processing/Processors/Convolution/Convolution2DRowOperation{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/Convolution2DRowOperation{TPixel}.cs index c04581444f..e5963bd390 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/Convolution2DRowOperation{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/Convolution2DRowOperation{TPixel}.cs @@ -49,7 +49,7 @@ internal readonly struct Convolution2DRowOperation : IRowOperation [MethodImpl(InliningOptions.ShortMethod)] public int GetRequiredBufferLength(Rectangle bounds) - => bounds.Width; + => 3 * bounds.Width; /// [MethodImpl(MethodImplOptions.AggressiveInlining)] diff --git a/src/ImageSharp/Processing/Processors/Convolution/Convolution2PassProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/Convolution2PassProcessor{TPixel}.cs index fa05a98fd1..094a96f787 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/Convolution2PassProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/Convolution2PassProcessor{TPixel}.cs @@ -70,10 +70,6 @@ internal class Convolution2PassProcessor : ImageProcessor var interest = Rectangle.Intersect(this.SourceRectangle, source.Bounds()); - // We use a rectangle 2x the interest width to allocate a buffer big enough - // for source and target bulk pixel conversion. - var operationBounds = new Rectangle(interest.X, interest.Y, interest.Width * 2, interest.Height); - // We can create a single sampling map with the size as if we were using the non separated 2D kernel // the two 1D kernels represent, and reuse it across both convolution steps, like in the bokeh blur. using var mapXY = new KernelSamplingMap(this.Configuration.MemoryAllocator); @@ -92,7 +88,7 @@ internal class Convolution2PassProcessor : ImageProcessor ParallelRowIterator.IterateRows( this.Configuration, - operationBounds, + interest, in horizontalOperation); // Vertical convolution @@ -107,7 +103,7 @@ internal class Convolution2PassProcessor : ImageProcessor ParallelRowIterator.IterateRows( this.Configuration, - operationBounds, + interest, in verticalOperation); } @@ -146,7 +142,7 @@ internal class Convolution2PassProcessor : ImageProcessor /// [MethodImpl(InliningOptions.ShortMethod)] public int GetRequiredBufferLength(Rectangle bounds) - => bounds.Width; + => 2 * bounds.Width; /// [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -312,7 +308,7 @@ internal class Convolution2PassProcessor : ImageProcessor /// [MethodImpl(InliningOptions.ShortMethod)] public int GetRequiredBufferLength(Rectangle bounds) - => bounds.Width; + => 2 * bounds.Width; /// [MethodImpl(MethodImplOptions.AggressiveInlining)] diff --git a/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessor{TPixel}.cs index f20cca3f71..54dad64a6b 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/ConvolutionProcessor{TPixel}.cs @@ -57,9 +57,6 @@ internal class ConvolutionProcessor : ImageProcessor var interest = Rectangle.Intersect(this.SourceRectangle, source.Bounds()); - // We use a rectangle 2x the interest width to allocate a buffer big enough - // for source and target bulk pixel conversion. - var operationBounds = new Rectangle(interest.X, interest.Y, interest.Width * 2, interest.Height); using (var map = new KernelSamplingMap(allocator)) { map.BuildSamplingOffsetMap(this.KernelXY, interest); @@ -67,7 +64,7 @@ internal class ConvolutionProcessor : ImageProcessor var operation = new RowOperation(interest, targetPixels, source.PixelBuffer, map, this.KernelXY, this.Configuration, this.PreserveAlpha); ParallelRowIterator.IterateRows( this.Configuration, - operationBounds, + interest, in operation); } @@ -109,7 +106,7 @@ internal class ConvolutionProcessor : ImageProcessor /// [MethodImpl(InliningOptions.ShortMethod)] public int GetRequiredBufferLength(Rectangle bounds) - => bounds.Width; + => 2 * bounds.Width; /// [MethodImpl(InliningOptions.ShortMethod)] diff --git a/src/ImageSharp/Processing/Processors/Convolution/MedianBlurProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/MedianBlurProcessor{TPixel}.cs index 4f0c2a36c9..fe3a29d437 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/MedianBlurProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/MedianBlurProcessor{TPixel}.cs @@ -31,11 +31,6 @@ internal sealed class MedianBlurProcessor : ImageProcessor Rectangle interest = Rectangle.Intersect(this.SourceRectangle, source.Bounds()); - // We use a rectangle with width set wider, to allocate a buffer big enough - // for kernel source, channel buffers, source rows and target bulk pixel conversion. - int operationWidth = (2 * kernelSize * kernelSize) + interest.Width + (kernelSize * interest.Width); - Rectangle operationBounds = new(interest.X, interest.Y, operationWidth, interest.Height); - using KernelSamplingMap map = new(this.Configuration.MemoryAllocator); map.BuildSamplingOffsetMap(kernelSize, kernelSize, interest, this.definition.BorderWrapModeX, this.definition.BorderWrapModeY); @@ -50,7 +45,7 @@ internal sealed class MedianBlurProcessor : ImageProcessor ParallelRowIterator.IterateRows, Vector4>( this.Configuration, - operationBounds, + interest, in operation); Buffer2D.SwapOrCopyContent(source.PixelBuffer, targetPixels); diff --git a/src/ImageSharp/Processing/Processors/Convolution/MedianRowOperation{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/MedianRowOperation{TPixel}.cs index 17df7b9be4..8caf33440d 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/MedianRowOperation{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/MedianRowOperation{TPixel}.cs @@ -46,11 +46,11 @@ internal readonly struct MedianRowOperation : IRowOperation /// [MethodImpl(InliningOptions.ShortMethod)] public int GetRequiredBufferLength(Rectangle bounds) - => bounds.Width; + => (2 * this.kernelSize * this.kernelSize) + bounds.Width + (kernelSize * bounds.Width); public void Invoke(int y, Span span) { - // Span has kernelSize^2 followed by bound width. + // Span has kernelSize^2 twice, then bound width followed by kernelsize * bounds width. int boundsX = this.bounds.X; int boundsWidth = this.bounds.Width; int kernelCount = this.kernelSize * this.kernelSize; From 1ec6da892ab1d28fc2f14e348c90575cb802cf76 Mon Sep 17 00:00:00 2001 From: Ynse Hoornenborg Date: Mon, 26 Sep 2022 22:01:35 +0200 Subject: [PATCH 13/17] Bulk pixel conversion --- .../AutoLevelProcessor{TPixel}.cs | 62 +++++++++++++------ ...lHistogramEqualizationProcessor{TPixel}.cs | 33 ++++++---- .../GrayscaleLevelsRowOperation{TPixel}.cs | 23 +++++-- 3 files changed, 81 insertions(+), 37 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs index 856fba3dcb..c07ac3aa34 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/AutoLevelProcessor{TPixel}.cs @@ -59,8 +59,8 @@ internal class AutoLevelProcessor : HistogramEqualizationProcessor histogramBuffer = memoryAllocator.Allocate(this.LuminanceLevels, AllocationOptions.Clean); // Build the histogram of the grayscale levels. - var grayscaleOperation = new GrayscaleLevelsRowOperation(interest, histogramBuffer, source.PixelBuffer, this.LuminanceLevels); - ParallelRowIterator.IterateRows( + var grayscaleOperation = new GrayscaleLevelsRowOperation(this.Configuration, interest, histogramBuffer, source.PixelBuffer, this.LuminanceLevels); + ParallelRowIterator.IterateRows, Vector4>( this.Configuration, interest, in grayscaleOperation); @@ -83,16 +83,16 @@ internal class AutoLevelProcessor : HistogramEqualizationProcessor( this.Configuration, interest, in cdfOperation); } else { - var cdfOperation = new SeperateChannelsRowOperation(interest, cdfBuffer, source.PixelBuffer, this.LuminanceLevels, numberOfPixelsMinusCdfMin); - ParallelRowIterator.IterateRows( + var cdfOperation = new SeperateChannelsRowOperation(this.Configuration, interest, cdfBuffer, source.PixelBuffer, this.LuminanceLevels, numberOfPixelsMinusCdfMin); + ParallelRowIterator.IterateRows( this.Configuration, interest, in cdfOperation); @@ -102,8 +102,9 @@ internal class AutoLevelProcessor : HistogramEqualizationProcessor /// A implementing the cdf logic for synchronized color channels. /// - private readonly struct SynchronizedChannelsRowOperation : IRowOperation + private readonly struct SynchronizedChannelsRowOperation : IRowOperation { + private readonly Configuration configuration; private readonly Rectangle bounds; private readonly IMemoryOwner cdfBuffer; private readonly Buffer2D source; @@ -112,12 +113,14 @@ internal class AutoLevelProcessor : HistogramEqualizationProcessor cdfBuffer, Buffer2D source, int luminanceLevels, float numberOfPixelsMinusCdfMin) { + this.configuration = configuration; this.bounds = bounds; this.cdfBuffer = cdfBuffer; this.source = source; @@ -127,33 +130,42 @@ internal class AutoLevelProcessor : HistogramEqualizationProcessor [MethodImpl(InliningOptions.ShortMethod)] - public void Invoke(int y) + public int GetRequiredBufferLength(Rectangle bounds) => bounds.Width; + + /// + [MethodImpl(InliningOptions.ShortMethod)] + public void Invoke(int y, Span span) { + Span vectorBuffer = span.Slice(0, this.bounds.Width); + ref Vector4 vectorRef = ref MemoryMarshal.GetReference(vectorBuffer); ref int cdfBase = ref MemoryMarshal.GetReference(this.cdfBuffer.GetSpan()); var sourceAccess = new PixelAccessor(this.source); - Span pixelRow = sourceAccess.GetRowSpan(y); int levels = this.luminanceLevels; float noOfPixelsMinusCdfMin = this.numberOfPixelsMinusCdfMin; + Span pixelRow = sourceAccess.GetRowSpan(y).Slice(this.bounds.X, this.bounds.Width); + PixelOperations.Instance.ToVector4(this.configuration, pixelRow, vectorBuffer); + for (int x = 0; x < this.bounds.Width; x++) { - // TODO: We should bulk convert here. - ref TPixel pixel = ref pixelRow[x]; - var vector = pixel.ToVector4(); + var vector = Unsafe.Add(ref vectorRef, x); int luminance = ColorNumerics.GetBT709Luminance(ref vector, levels); float scaledLuminance = Unsafe.Add(ref cdfBase, luminance) / noOfPixelsMinusCdfMin; float scalingFactor = scaledLuminance * levels / luminance; Vector4 scaledVector = new Vector4(scalingFactor * vector.X, scalingFactor * vector.Y, scalingFactor * vector.Z, vector.W); - pixel.FromVector4(scaledVector); + Unsafe.Add(ref vectorRef, x) = scaledVector; } + + PixelOperations.Instance.FromVector4Destructive(this.configuration, vectorBuffer, pixelRow); } } /// /// A implementing the cdf logic for separate color channels. /// - private readonly struct SeperateChannelsRowOperation : IRowOperation + private readonly struct SeperateChannelsRowOperation : IRowOperation { + private readonly Configuration configuration; private readonly Rectangle bounds; private readonly IMemoryOwner cdfBuffer; private readonly Buffer2D source; @@ -162,12 +174,14 @@ internal class AutoLevelProcessor : HistogramEqualizationProcessor cdfBuffer, Buffer2D source, int luminanceLevels, float numberOfPixelsMinusCdfMin) { + this.configuration = configuration; this.bounds = bounds; this.cdfBuffer = cdfBuffer; this.source = source; @@ -177,19 +191,25 @@ internal class AutoLevelProcessor : HistogramEqualizationProcessor [MethodImpl(InliningOptions.ShortMethod)] - public void Invoke(int y) + public int GetRequiredBufferLength(Rectangle bounds) => bounds.Width; + + /// + [MethodImpl(InliningOptions.ShortMethod)] + public void Invoke(int y, Span span) { + Span vectorBuffer = span.Slice(0, this.bounds.Width); + ref Vector4 vectorRef = ref MemoryMarshal.GetReference(vectorBuffer); ref int cdfBase = ref MemoryMarshal.GetReference(this.cdfBuffer.GetSpan()); var sourceAccess = new PixelAccessor(this.source); - Span pixelRow = sourceAccess.GetRowSpan(y); int levelsMinusOne = this.luminanceLevels - 1; float noOfPixelsMinusCdfMin = this.numberOfPixelsMinusCdfMin; + Span pixelRow = sourceAccess.GetRowSpan(y); + PixelOperations.Instance.ToVector4(this.configuration, pixelRow, vectorBuffer); + for (int x = 0; x < this.bounds.Width; x++) { - // TODO: We should bulk convert here. - ref TPixel pixel = ref pixelRow[x]; - var vector = pixel.ToVector4() * levelsMinusOne; + var vector = Unsafe.Add(ref vectorRef, x) * levelsMinusOne; uint originalX = (uint)MathF.Round(vector.X); float scaledX = Unsafe.Add(ref cdfBase, originalX) / noOfPixelsMinusCdfMin; @@ -197,8 +217,10 @@ internal class AutoLevelProcessor : HistogramEqualizationProcessor.Instance.FromVector4Destructive(this.configuration, vectorBuffer, pixelRow); } } } diff --git a/src/ImageSharp/Processing/Processors/Normalization/GlobalHistogramEqualizationProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Normalization/GlobalHistogramEqualizationProcessor{TPixel}.cs index d506777be0..7e9e064642 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/GlobalHistogramEqualizationProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/GlobalHistogramEqualizationProcessor{TPixel}.cs @@ -51,8 +51,8 @@ internal class GlobalHistogramEqualizationProcessor : HistogramEqualizat using IMemoryOwner histogramBuffer = memoryAllocator.Allocate(this.LuminanceLevels, AllocationOptions.Clean); // Build the histogram of the grayscale levels. - var grayscaleOperation = new GrayscaleLevelsRowOperation(interest, histogramBuffer, source.PixelBuffer, this.LuminanceLevels); - ParallelRowIterator.IterateRows( + var grayscaleOperation = new GrayscaleLevelsRowOperation(this.Configuration, interest, histogramBuffer, source.PixelBuffer, this.LuminanceLevels); + ParallelRowIterator.IterateRows, Vector4>( this.Configuration, interest, in grayscaleOperation); @@ -74,8 +74,8 @@ internal class GlobalHistogramEqualizationProcessor : HistogramEqualizat float numberOfPixelsMinusCdfMin = numberOfPixels - cdfMin; // Apply the cdf to each pixel of the image - var cdfOperation = new CdfApplicationRowOperation(interest, cdfBuffer, source.PixelBuffer, this.LuminanceLevels, numberOfPixelsMinusCdfMin); - ParallelRowIterator.IterateRows( + var cdfOperation = new CdfApplicationRowOperation(this.Configuration, interest, cdfBuffer, source.PixelBuffer, this.LuminanceLevels, numberOfPixelsMinusCdfMin); + ParallelRowIterator.IterateRows( this.Configuration, interest, in cdfOperation); @@ -84,8 +84,9 @@ internal class GlobalHistogramEqualizationProcessor : HistogramEqualizat /// /// A implementing the cdf application levels logic for . /// - private readonly struct CdfApplicationRowOperation : IRowOperation + private readonly struct CdfApplicationRowOperation : IRowOperation { + private readonly Configuration configuration; private readonly Rectangle bounds; private readonly IMemoryOwner cdfBuffer; private readonly Buffer2D source; @@ -94,12 +95,14 @@ internal class GlobalHistogramEqualizationProcessor : HistogramEqualizat [MethodImpl(InliningOptions.ShortMethod)] public CdfApplicationRowOperation( + Configuration configuration, Rectangle bounds, IMemoryOwner cdfBuffer, Buffer2D source, int luminanceLevels, float numberOfPixelsMinusCdfMin) { + this.configuration = configuration; this.bounds = bounds; this.cdfBuffer = cdfBuffer; this.source = source; @@ -109,22 +112,30 @@ internal class GlobalHistogramEqualizationProcessor : HistogramEqualizat /// [MethodImpl(InliningOptions.ShortMethod)] - public void Invoke(int y) + public int GetRequiredBufferLength(Rectangle bounds) => bounds.Width; + + /// + [MethodImpl(InliningOptions.ShortMethod)] + public void Invoke(int y, Span span) { + Span vectorBuffer = span.Slice(0, this.bounds.Width); + ref Vector4 vectorRef = ref MemoryMarshal.GetReference(vectorBuffer); ref int cdfBase = ref MemoryMarshal.GetReference(this.cdfBuffer.GetSpan()); - Span pixelRow = this.source.DangerousGetRowSpan(y); int levels = this.luminanceLevels; float noOfPixelsMinusCdfMin = this.numberOfPixelsMinusCdfMin; + Span pixelRow = this.source.DangerousGetRowSpan(y); + PixelOperations.Instance.ToVector4(this.configuration, pixelRow, vectorBuffer); + for (int x = 0; x < this.bounds.Width; x++) { - // TODO: We should bulk convert here. - ref TPixel pixel = ref pixelRow[x]; - var vector = pixel.ToVector4(); + var vector = Unsafe.Add(ref vectorRef, x); int luminance = ColorNumerics.GetBT709Luminance(ref vector, levels); float luminanceEqualized = Unsafe.Add(ref cdfBase, luminance) / noOfPixelsMinusCdfMin; - pixel.FromVector4(new Vector4(luminanceEqualized, luminanceEqualized, luminanceEqualized, vector.W)); + Unsafe.Add(ref vectorRef, x) = new Vector4(luminanceEqualized, luminanceEqualized, luminanceEqualized, vector.W); } + + PixelOperations.Instance.FromVector4Destructive(this.configuration, vectorBuffer, pixelRow); } } } diff --git a/src/ImageSharp/Processing/Processors/Normalization/GrayscaleLevelsRowOperation{TPixel}.cs b/src/ImageSharp/Processing/Processors/Normalization/GrayscaleLevelsRowOperation{TPixel}.cs index f4fcd15782..8895fdc612 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/GrayscaleLevelsRowOperation{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/GrayscaleLevelsRowOperation{TPixel}.cs @@ -2,6 +2,7 @@ // Licensed under the Six Labors Split License. using System.Buffers; +using System.Numerics; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using SixLabors.ImageSharp.Advanced; @@ -11,11 +12,12 @@ using SixLabors.ImageSharp.PixelFormats; namespace SixLabors.ImageSharp.Processing.Processors.Normalization; /// -/// A implementing the grayscale levels logic as . +/// A implementing the grayscale levels logic as . /// -internal readonly struct GrayscaleLevelsRowOperation : IRowOperation +internal readonly struct GrayscaleLevelsRowOperation : IRowOperation where TPixel : unmanaged, IPixel { + private readonly Configuration configuration; private readonly Rectangle bounds; private readonly IMemoryOwner histogramBuffer; private readonly Buffer2D source; @@ -23,11 +25,13 @@ internal readonly struct GrayscaleLevelsRowOperation : IRowOperation [MethodImpl(InliningOptions.ShortMethod)] public GrayscaleLevelsRowOperation( + Configuration configuration, Rectangle bounds, IMemoryOwner histogramBuffer, Buffer2D source, int luminanceLevels) { + this.configuration = configuration; this.bounds = bounds; this.histogramBuffer = histogramBuffer; this.source = source; @@ -36,16 +40,23 @@ internal readonly struct GrayscaleLevelsRowOperation : IRowOperation /// [MethodImpl(InliningOptions.ShortMethod)] - public void Invoke(int y) + public int GetRequiredBufferLength(Rectangle bounds) => bounds.Width; + + /// + [MethodImpl(InliningOptions.ShortMethod)] + public void Invoke(int y, Span span) { + Span vectorBuffer = span.Slice(0, this.bounds.Width); + ref Vector4 vectorRef = ref MemoryMarshal.GetReference(vectorBuffer); ref int histogramBase = ref MemoryMarshal.GetReference(this.histogramBuffer.GetSpan()); - Span pixelRow = this.source.DangerousGetRowSpan(y); int levels = this.luminanceLevels; + Span pixelRow = this.source.DangerousGetRowSpan(y); + PixelOperations.Instance.ToVector4(this.configuration, pixelRow, vectorBuffer); + for (int x = 0; x < this.bounds.Width; x++) { - // TODO: We should bulk convert here. - var vector = pixelRow[x].ToVector4(); + var vector = Unsafe.Add(ref vectorRef, x); int luminance = ColorNumerics.GetBT709Luminance(ref vector, levels); Interlocked.Increment(ref Unsafe.Add(ref histogramBase, luminance)); } From ec79e87715751816e4040053c7e88a7620e08021 Mon Sep 17 00:00:00 2001 From: Ynse Hoornenborg Date: Mon, 26 Sep 2022 22:18:16 +0200 Subject: [PATCH 14/17] Fix SA1101 --- .../Processors/Convolution/MedianRowOperation{TPixel}.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Processing/Processors/Convolution/MedianRowOperation{TPixel}.cs b/src/ImageSharp/Processing/Processors/Convolution/MedianRowOperation{TPixel}.cs index 8caf33440d..dbb62f0c58 100644 --- a/src/ImageSharp/Processing/Processors/Convolution/MedianRowOperation{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Convolution/MedianRowOperation{TPixel}.cs @@ -46,7 +46,7 @@ internal readonly struct MedianRowOperation : IRowOperation /// [MethodImpl(InliningOptions.ShortMethod)] public int GetRequiredBufferLength(Rectangle bounds) - => (2 * this.kernelSize * this.kernelSize) + bounds.Width + (kernelSize * bounds.Width); + => (2 * this.kernelSize * this.kernelSize) + bounds.Width + (this.kernelSize * bounds.Width); public void Invoke(int y, Span span) { From 770a85070b22318c5cc3a0d3c7a5f5e99215a1df Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Tue, 27 Sep 2022 14:51:07 +0200 Subject: [PATCH 15/17] Ignore unknown chunks, fixes #2243 --- src/ImageSharp/Formats/Webp/WebpDecoderCore.cs | 4 +++- .../ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs | 11 +++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Webp/issues/Issue2243.webp | 3 +++ 4 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 tests/Images/Input/Webp/issues/Issue2243.webp diff --git a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs index 4632200f4b..98a2805cd6 100644 --- a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs +++ b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs @@ -221,7 +221,9 @@ internal sealed class WebpDecoderCore : IImageDecoderInternals, IDisposable } else { - WebpThrowHelper.ThrowImageFormatException("Unexpected chunk followed VP8X header"); + // Ignore unknown chunks. + uint chunkSize = this.ReadChunkSize(); + this.currentStream.Skip((int)chunkSize); } } diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs index f5fd98f458..af709c0d86 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs @@ -385,6 +385,17 @@ public class WebpDecoderTests image.CompareToOriginal(provider, ReferenceDecoder); } + // https://github.com/SixLabors/ImageSharp/issues/2243 + [Theory] + [WithFile(Lossy.Issue2243, PixelTypes.Rgba32)] + public void WebpDecoder_CanDecode_Issue2243(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(WebpDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ReferenceDecoder); + } + [Theory] [WithFile(Lossless.LossLessCorruptImage3, PixelTypes.Rgba32)] public void WebpDecoder_ThrowImageFormatException_OnInvalidImages(TestImageProvider provider) diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 676d460e56..989776934f 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -736,6 +736,7 @@ public static class TestImages // Issues public const string Issue1594 = "Webp/issues/Issue1594.webp"; + public const string Issue2243 = "Webp/issues/Issue2243.webp"; } } diff --git a/tests/Images/Input/Webp/issues/Issue2243.webp b/tests/Images/Input/Webp/issues/Issue2243.webp new file mode 100644 index 0000000000..2d1da9d980 --- /dev/null +++ b/tests/Images/Input/Webp/issues/Issue2243.webp @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:c1aae55fae66f3f9469ad5e28eb8134a04b1c5d746acf0a4a19d0f63ca0581cd +size 55068 From b0ba58c07fc8092eeb25e555163d95d5d4ac12e2 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 1 Oct 2022 01:15:50 +1000 Subject: [PATCH 16/17] Fix position handling --- src/ImageSharp/IO/BufferedReadStream.cs | 8 +- .../IO/BufferedReadStreamTests.cs | 423 +++++++++--------- 2 files changed, 208 insertions(+), 223 deletions(-) diff --git a/src/ImageSharp/IO/BufferedReadStream.cs b/src/ImageSharp/IO/BufferedReadStream.cs index 7e233c9655..1afff21923 100644 --- a/src/ImageSharp/IO/BufferedReadStream.cs +++ b/src/ImageSharp/IO/BufferedReadStream.cs @@ -49,7 +49,7 @@ internal sealed class BufferedReadStream : Stream this.BaseStream = stream; this.Length = stream.Length; - this.Position = (int)stream.Position; + this.readerPosition = stream.Position; this.BufferSize = configuration.StreamProcessingBufferSize; this.maxBufferIndex = this.BufferSize - 1; this.readBuffer = ArrayPool.Shared.Rent(this.BufferSize); @@ -96,9 +96,8 @@ internal sealed class BufferedReadStream : Stream else { // Base stream seek will throw for us if invalid. - this.BaseStream.Seek(value, SeekOrigin.Begin); this.readerPosition = value; - this.readBufferIndex = this.BufferSize; + this.FillReadBuffer(); } } } @@ -147,6 +146,7 @@ internal sealed class BufferedReadStream : Stream } this.readerPosition++; + unsafe { return this.pinnedReadBuffer[this.readBufferIndex++]; @@ -202,7 +202,7 @@ internal sealed class BufferedReadStream : Stream if (this.readerPosition != baseStream.Position) { baseStream.Seek(this.readerPosition, SeekOrigin.Begin); - this.readerPosition = (int)baseStream.Position; + this.readerPosition = baseStream.Position; } // Reset to trigger full read on next attempt. diff --git a/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs b/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs index 6a9c344860..dab5db1c57 100644 --- a/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs +++ b/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs @@ -10,12 +10,10 @@ public class BufferedReadStreamTests private readonly Configuration configuration; public BufferedReadStreamTests() - { - this.configuration = Configuration.CreateDefaultInstance(); - } + => this.configuration = Configuration.CreateDefaultInstance(); public static readonly TheoryData BufferSizes = - new TheoryData() + new() { 1, 2, 4, 8, 16, 97, 503, @@ -28,21 +26,19 @@ public class BufferedReadStreamTests public void BufferedStreamCanReadSingleByteFromOrigin(int bufferSize) { this.configuration.StreamProcessingBufferSize = bufferSize; - using (MemoryStream stream = this.CreateTestStream(bufferSize * 3)) + using MemoryStream stream = CreateTestStream(bufferSize * 3); + byte[] expected = stream.ToArray(); + using (BufferedReadStream reader = new(this.configuration, stream)) { - byte[] expected = stream.ToArray(); - using (var reader = new BufferedReadStream(this.configuration, stream)) - { - Assert.Equal(expected[0], reader.ReadByte()); - - // We've read a whole chunk but increment by 1 in our reader. - Assert.True(stream.Position >= bufferSize); - Assert.Equal(1, reader.Position); - } + Assert.Equal(expected[0], reader.ReadByte()); - // Position of the stream should be reset on disposal. - Assert.Equal(1, stream.Position); + // We've read a whole chunk but increment by 1 in our reader. + Assert.True(stream.Position >= bufferSize); + Assert.Equal(1, reader.Position); } + + // Position of the stream should be reset on disposal. + Assert.Equal(1, stream.Position); } [Theory] @@ -50,23 +46,21 @@ public class BufferedReadStreamTests public void BufferedStreamCanReadSingleByteFromOffset(int bufferSize) { this.configuration.StreamProcessingBufferSize = bufferSize; - using (MemoryStream stream = this.CreateTestStream(bufferSize * 3)) + using MemoryStream stream = CreateTestStream(bufferSize * 3); + byte[] expected = stream.ToArray(); + int offset = expected.Length / 2; + using (BufferedReadStream reader = new(this.configuration, stream)) { - byte[] expected = stream.ToArray(); - int offset = expected.Length / 2; - using (var reader = new BufferedReadStream(this.configuration, stream)) - { - reader.Position = offset; + reader.Position = offset; - Assert.Equal(expected[offset], reader.ReadByte()); - - // We've read a whole chunk but increment by 1 in our reader. - Assert.Equal(bufferSize + offset, stream.Position); - Assert.Equal(offset + 1, reader.Position); - } + Assert.Equal(expected[offset], reader.ReadByte()); - Assert.Equal(offset + 1, stream.Position); + // We've read a whole chunk but increment by 1 in our reader. + Assert.Equal(bufferSize + offset, stream.Position); + Assert.Equal(offset + 1, reader.Position); } + + Assert.Equal(offset + 1, stream.Position); } [Theory] @@ -74,36 +68,34 @@ public class BufferedReadStreamTests public void BufferedStreamCanReadSubsequentSingleByteCorrectly(int bufferSize) { this.configuration.StreamProcessingBufferSize = bufferSize; - using (MemoryStream stream = this.CreateTestStream(bufferSize * 3)) + using MemoryStream stream = CreateTestStream(bufferSize * 3); + byte[] expected = stream.ToArray(); + int i; + using (BufferedReadStream reader = new(this.configuration, stream)) { - byte[] expected = stream.ToArray(); - int i; - using (var reader = new BufferedReadStream(this.configuration, stream)) + for (i = 0; i < expected.Length; i++) { - for (i = 0; i < expected.Length; i++) + Assert.Equal(expected[i], reader.ReadByte()); + Assert.Equal(i + 1, reader.Position); + + if (i < bufferSize) { - Assert.Equal(expected[i], reader.ReadByte()); - Assert.Equal(i + 1, reader.Position); - - if (i < bufferSize) - { - Assert.Equal(stream.Position, bufferSize); - } - else if (i >= bufferSize && i < bufferSize * 2) - { - // We should have advanced to the second chunk now. - Assert.Equal(stream.Position, bufferSize * 2); - } - else - { - // We should have advanced to the third chunk now. - Assert.Equal(stream.Position, bufferSize * 3); - } + Assert.Equal(stream.Position, bufferSize); + } + else if (i >= bufferSize && i < bufferSize * 2) + { + // We should have advanced to the second chunk now. + Assert.Equal(stream.Position, bufferSize * 2); + } + else + { + // We should have advanced to the third chunk now. + Assert.Equal(stream.Position, bufferSize * 3); } } - - Assert.Equal(i, stream.Position); } + + Assert.Equal(i, stream.Position); } [Theory] @@ -111,21 +103,17 @@ public class BufferedReadStreamTests public void BufferedStreamCanReadMultipleBytesFromOrigin(int bufferSize) { this.configuration.StreamProcessingBufferSize = bufferSize; - using (MemoryStream stream = this.CreateTestStream(bufferSize * 3)) - { - var buffer = new byte[2]; - byte[] expected = stream.ToArray(); - using (var reader = new BufferedReadStream(this.configuration, stream)) - { - Assert.Equal(2, reader.Read(buffer, 0, 2)); - Assert.Equal(expected[0], buffer[0]); - Assert.Equal(expected[1], buffer[1]); - - // We've read a whole chunk but increment by the buffer length in our reader. - Assert.True(stream.Position >= bufferSize); - Assert.Equal(buffer.Length, reader.Position); - } - } + using MemoryStream stream = CreateTestStream(bufferSize * 3); + byte[] buffer = new byte[2]; + byte[] expected = stream.ToArray(); + using BufferedReadStream reader = new(this.configuration, stream); + Assert.Equal(2, reader.Read(buffer, 0, 2)); + Assert.Equal(expected[0], buffer[0]); + Assert.Equal(expected[1], buffer[1]); + + // We've read a whole chunk but increment by the buffer length in our reader. + Assert.True(stream.Position >= bufferSize); + Assert.Equal(buffer.Length, reader.Position); } [Theory] @@ -133,49 +121,45 @@ public class BufferedReadStreamTests public void BufferedStreamCanReadSubsequentMultipleByteCorrectly(int bufferSize) { this.configuration.StreamProcessingBufferSize = bufferSize; - using (MemoryStream stream = this.CreateTestStream(bufferSize * 3)) + using MemoryStream stream = CreateTestStream(bufferSize * 3); + const int increment = 2; + byte[] buffer = new byte[2]; + byte[] expected = stream.ToArray(); + using BufferedReadStream reader = new(this.configuration, stream); + for (int i = 0, o = 0; i < expected.Length / increment; i++, o += increment) { - const int increment = 2; - var buffer = new byte[2]; - byte[] expected = stream.ToArray(); - using (var reader = new BufferedReadStream(this.configuration, stream)) + // Check values are correct. + Assert.Equal(increment, reader.Read(buffer, 0, increment)); + Assert.Equal(expected[o], buffer[0]); + Assert.Equal(expected[o + 1], buffer[1]); + Assert.Equal(o + increment, reader.Position); + + // These tests ensure that we are correctly reading + // our buffer in chunks of the given size. + int offset = i * increment; + + // First chunk. + if (offset < bufferSize) { - for (int i = 0, o = 0; i < expected.Length / increment; i++, o += increment) - { - // Check values are correct. - Assert.Equal(increment, reader.Read(buffer, 0, increment)); - Assert.Equal(expected[o], buffer[0]); - Assert.Equal(expected[o + 1], buffer[1]); - Assert.Equal(o + increment, reader.Position); - - // These tests ensure that we are correctly reading - // our buffer in chunks of the given size. - int offset = i * increment; - - // First chunk. - if (offset < bufferSize) - { - // We've read an entire chunk once and are - // now reading from that chunk. - Assert.True(stream.Position >= bufferSize); - continue; - } - - // Second chunk - if (offset < bufferSize * 2) - { - Assert.True(stream.Position > bufferSize); - - // Odd buffer size with even increments can - // jump to the third chunk on final read. - Assert.True(stream.Position <= bufferSize * 3); - continue; - } - - // Third chunk - Assert.True(stream.Position > bufferSize * 2); - } + // We've read an entire chunk once and are + // now reading from that chunk. + Assert.True(stream.Position >= bufferSize); + continue; } + + // Second chunk + if (offset < bufferSize * 2) + { + Assert.True(stream.Position > bufferSize); + + // Odd buffer size with even increments can + // jump to the third chunk on final read. + Assert.True(stream.Position <= bufferSize * 3); + continue; + } + + // Third chunk + Assert.True(stream.Position > bufferSize * 2); } } @@ -184,49 +168,45 @@ public class BufferedReadStreamTests public void BufferedStreamCanReadSubsequentMultipleByteSpanCorrectly(int bufferSize) { this.configuration.StreamProcessingBufferSize = bufferSize; - using (MemoryStream stream = this.CreateTestStream(bufferSize * 3)) + using MemoryStream stream = CreateTestStream(bufferSize * 3); + const int increment = 2; + Span buffer = new byte[2]; + byte[] expected = stream.ToArray(); + using BufferedReadStream reader = new(this.configuration, stream); + for (int i = 0, o = 0; i < expected.Length / increment; i++, o += increment) { - const int increment = 2; - Span buffer = new byte[2]; - byte[] expected = stream.ToArray(); - using (var reader = new BufferedReadStream(this.configuration, stream)) + // Check values are correct. + Assert.Equal(increment, reader.Read(buffer, 0, increment)); + Assert.Equal(expected[o], buffer[0]); + Assert.Equal(expected[o + 1], buffer[1]); + Assert.Equal(o + increment, reader.Position); + + // These tests ensure that we are correctly reading + // our buffer in chunks of the given size. + int offset = i * increment; + + // First chunk. + if (offset < bufferSize) { - for (int i = 0, o = 0; i < expected.Length / increment; i++, o += increment) - { - // Check values are correct. - Assert.Equal(increment, reader.Read(buffer, 0, increment)); - Assert.Equal(expected[o], buffer[0]); - Assert.Equal(expected[o + 1], buffer[1]); - Assert.Equal(o + increment, reader.Position); - - // These tests ensure that we are correctly reading - // our buffer in chunks of the given size. - int offset = i * increment; - - // First chunk. - if (offset < bufferSize) - { - // We've read an entire chunk once and are - // now reading from that chunk. - Assert.True(stream.Position >= bufferSize); - continue; - } - - // Second chunk - if (offset < bufferSize * 2) - { - Assert.True(stream.Position > bufferSize); - - // Odd buffer size with even increments can - // jump to the third chunk on final read. - Assert.True(stream.Position <= bufferSize * 3); - continue; - } - - // Third chunk - Assert.True(stream.Position > bufferSize * 2); - } + // We've read an entire chunk once and are + // now reading from that chunk. + Assert.True(stream.Position >= bufferSize); + continue; + } + + // Second chunk + if (offset < bufferSize * 2) + { + Assert.True(stream.Position > bufferSize); + + // Odd buffer size with even increments can + // jump to the third chunk on final read. + Assert.True(stream.Position <= bufferSize * 3); + continue; } + + // Third chunk + Assert.True(stream.Position > bufferSize * 2); } } @@ -235,34 +215,28 @@ public class BufferedReadStreamTests public void BufferedStreamCanSkip(int bufferSize) { this.configuration.StreamProcessingBufferSize = bufferSize; - using (MemoryStream stream = this.CreateTestStream(bufferSize * 4)) - { - byte[] expected = stream.ToArray(); - using (var reader = new BufferedReadStream(this.configuration, stream)) - { - int skip = 1; - int plusOne = 1; - int skip2 = bufferSize; + using MemoryStream stream = CreateTestStream(bufferSize * 4); + byte[] expected = stream.ToArray(); + using BufferedReadStream reader = new(this.configuration, stream); + const int skip = 1; + const int plusOne = 1; + int skip2 = bufferSize; - // Skip - reader.Skip(skip); - Assert.Equal(skip, reader.Position); - Assert.Equal(stream.Position, reader.Position); + // Skip + reader.Skip(skip); + Assert.Equal(skip, reader.Position); - // Read - Assert.Equal(expected[skip], reader.ReadByte()); + // Read + Assert.Equal(expected[skip], reader.ReadByte()); - // Skip Again - reader.Skip(skip2); + // Skip Again + reader.Skip(skip2); - // First Skip + First Read + Second Skip - int position = skip + plusOne + skip2; + // First Skip + First Read + Second Skip + int position = skip + plusOne + skip2; - Assert.Equal(position, reader.Position); - Assert.Equal(stream.Position, reader.Position); - Assert.Equal(expected[position], reader.ReadByte()); - } - } + Assert.Equal(position, reader.Position); + Assert.Equal(expected[position], reader.ReadByte()); } [Theory] @@ -272,23 +246,21 @@ public class BufferedReadStreamTests this.configuration.StreamProcessingBufferSize = bufferSize; // Create a stream smaller than the default buffer length - using (MemoryStream stream = this.CreateTestStream(Math.Max(1, bufferSize / 4))) + using MemoryStream stream = CreateTestStream(Math.Max(1, bufferSize / 4)); + byte[] expected = stream.ToArray(); + int offset = expected.Length / 2; + using (BufferedReadStream reader = new(this.configuration, stream)) { - byte[] expected = stream.ToArray(); - int offset = expected.Length / 2; - using (var reader = new BufferedReadStream(this.configuration, stream)) - { - reader.Position = offset; + reader.Position = offset; - Assert.Equal(expected[offset], reader.ReadByte()); + Assert.Equal(expected[offset], reader.ReadByte()); - // We've read a whole length of the stream but increment by 1 in our reader. - Assert.Equal(Math.Max(1, bufferSize / 4), stream.Position); - Assert.Equal(offset + 1, reader.Position); - } - - Assert.Equal(offset + 1, stream.Position); + // We've read a whole length of the stream but increment by 1 in our reader. + Assert.Equal(Math.Max(1, bufferSize / 4), stream.Position); + Assert.Equal(offset + 1, reader.Position); } + + Assert.Equal(offset + 1, stream.Position); } [Theory] @@ -296,16 +268,12 @@ public class BufferedReadStreamTests public void BufferedStreamReadsCanReadAllAsSingleByteFromOrigin(int bufferSize) { this.configuration.StreamProcessingBufferSize = bufferSize; - using (MemoryStream stream = this.CreateTestStream(bufferSize * 3)) + using MemoryStream stream = CreateTestStream(bufferSize * 3); + byte[] expected = stream.ToArray(); + using BufferedReadStream reader = new(this.configuration, stream); + for (int i = 0; i < expected.Length; i++) { - byte[] expected = stream.ToArray(); - using (var reader = new BufferedReadStream(this.configuration, stream)) - { - for (int i = 0; i < expected.Length; i++) - { - Assert.Equal(expected[i], reader.ReadByte()); - } - } + Assert.Equal(expected[i], reader.ReadByte()); } } @@ -314,13 +282,9 @@ public class BufferedReadStreamTests public void BufferedStreamThrowsOnNegativePosition(int bufferSize) { this.configuration.StreamProcessingBufferSize = bufferSize; - using (MemoryStream stream = this.CreateTestStream(bufferSize)) - { - using (var reader = new BufferedReadStream(this.configuration, stream)) - { - Assert.Throws(() => reader.Position = -stream.Length); - } - } + using MemoryStream stream = CreateTestStream(bufferSize); + using BufferedReadStream reader = new(this.configuration, stream); + Assert.Throws(() => reader.Position = -stream.Length); } [Theory] @@ -328,13 +292,9 @@ public class BufferedReadStreamTests public void BufferedStreamCanSetPositionToEnd(int bufferSize) { this.configuration.StreamProcessingBufferSize = bufferSize; - using (MemoryStream stream = this.CreateTestStream(bufferSize * 2)) - { - using (var reader = new BufferedReadStream(this.configuration, stream)) - { - reader.Position = reader.Length; - } - } + using MemoryStream stream = CreateTestStream(bufferSize * 2); + using BufferedReadStream reader = new(this.configuration, stream); + reader.Position = reader.Length; } [Theory] @@ -342,20 +302,47 @@ public class BufferedReadStreamTests public void BufferedStreamCanSetPositionPastTheEnd(int bufferSize) { this.configuration.StreamProcessingBufferSize = bufferSize; - using (MemoryStream stream = this.CreateTestStream(bufferSize * 2)) + using MemoryStream stream = CreateTestStream(bufferSize * 2); + using BufferedReadStream reader = new(this.configuration, stream); + reader.Position = reader.Length + 1; + Assert.Equal(stream.Length + 1, stream.Position); + } + + [Fact] + public void BufferedStreamCanSetPositionMultipleTimes() + { + Configuration configuration = new() { - using (var reader = new BufferedReadStream(this.configuration, stream)) - { - reader.Position = reader.Length + 1; - Assert.Equal(stream.Length + 1, stream.Position); - } + StreamProcessingBufferSize = 16 + }; + + byte[] buffer = new byte[255]; + for (int i = 0; i < buffer.Length; i++) + { + buffer[i] = (byte)i; + } + + BufferedReadStream bufferedStream = new(configuration, new MemoryStream(buffer)); + + // Read more then fits into the buffer. + for (int i = 0; i < 20; i++) + { + bufferedStream.ReadByte(); } + + // Set the Position twice. + bufferedStream.Position = 10; + bufferedStream.Position = 3; + + // readValue is 25, but should be 3 + int readValue = bufferedStream.ReadByte(); + Assert.Equal(3, readValue); } - private MemoryStream CreateTestStream(int length) + private static MemoryStream CreateTestStream(int length) { - var buffer = new byte[length]; - var random = new Random(); + byte[] buffer = new byte[length]; + Random random = new(); random.NextBytes(buffer); return new EvilStream(buffer); @@ -371,8 +358,6 @@ public class BufferedReadStreamTests } public override int Read(byte[] buffer, int offset, int count) - { - return base.Read(buffer, offset, 1); - } + => base.Read(buffer, offset, 1); } } From da92a42d344b2650c7279ac71ecab16eef440d9d Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Fri, 30 Sep 2022 18:43:44 +0200 Subject: [PATCH 17/17] Remove invalid comment --- tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs b/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs index dab5db1c57..b00d581400 100644 --- a/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs +++ b/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs @@ -334,9 +334,8 @@ public class BufferedReadStreamTests bufferedStream.Position = 10; bufferedStream.Position = 3; - // readValue is 25, but should be 3 - int readValue = bufferedStream.ReadByte(); - Assert.Equal(3, readValue); + int actual = bufferedStream.ReadByte(); + Assert.Equal(3, actual); } private static MemoryStream CreateTestStream(int length)