diff --git a/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs index 6352230de2..1491fe073b 100644 --- a/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs @@ -4,6 +4,7 @@ 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; @@ -34,17 +35,25 @@ public OilPaintingProcessor(Configuration configuration, OilPaintingProcessor de /// protected override void OnFrameApply(ImageFrame source) { + int levels = Math.Clamp(this.definition.Levels, 1, 255); int brushSize = Math.Clamp(this.definition.BrushSize, 1, Math.Min(source.Width, source.Height)); using Buffer2D targetPixels = this.Configuration.MemoryAllocator.Allocate2D(source.Size()); source.CopyTo(targetPixels); - RowIntervalOperation operation = new(this.SourceRectangle, targetPixels, source.PixelBuffer, this.Configuration, brushSize >> 1, this.definition.Levels); - ParallelRowIterator.IterateRowIntervals( + RowIntervalOperation operation = new(this.SourceRectangle, targetPixels, source.PixelBuffer, this.Configuration, brushSize >> 1, levels); + try + { + ParallelRowIterator.IterateRowIntervals( this.Configuration, this.SourceRectangle, in operation); + } + catch (Exception ex) + { + throw new ImageProcessingException("The OilPaintProcessor failed. The most likely reason is that a pixel component was outside of its' allowed range.", ex); + } Buffer2D.SwapOrCopyContent(source.PixelBuffer, targetPixels); } @@ -105,18 +114,18 @@ public void Invoke(in RowInterval rows) Span targetRowVector4Span = targetRowBuffer.Memory.Span; Span targetRowAreaVector4Span = targetRowVector4Span.Slice(this.bounds.X, this.bounds.Width); - ref float binsRef = ref bins.GetReference(); - ref int intensityBinRef = ref Unsafe.As(ref binsRef); - ref float redBinRef = ref Unsafe.Add(ref binsRef, (uint)this.levels); - ref float blueBinRef = ref Unsafe.Add(ref redBinRef, (uint)this.levels); - ref float greenBinRef = ref Unsafe.Add(ref blueBinRef, (uint)this.levels); + Span binsSpan = bins.GetSpan(); + Span intensityBinsSpan = MemoryMarshal.Cast(binsSpan); + Span redBinSpan = binsSpan[this.levels..]; + Span blueBinSpan = redBinSpan[this.levels..]; + Span greenBinSpan = blueBinSpan[this.levels..]; for (int y = rows.Min; y < rows.Max; y++) { Span sourceRowPixelSpan = this.source.DangerousGetRowSpan(y); Span sourceRowAreaPixelSpan = sourceRowPixelSpan.Slice(this.bounds.X, this.bounds.Width); - PixelOperations.Instance.ToVector4(this.configuration, sourceRowAreaPixelSpan, sourceRowAreaVector4Span); + PixelOperations.Instance.ToVector4(this.configuration, sourceRowAreaPixelSpan, sourceRowAreaVector4Span, PixelConversionModifiers.Scale); for (int x = this.bounds.X; x < this.bounds.Right; x++) { @@ -140,7 +149,7 @@ public void Invoke(in RowInterval rows) int offsetX = x + fxr; offsetX = Numerics.Clamp(offsetX, 0, maxX); - Vector4 vector = sourceOffsetRow[offsetX].ToVector4(); + Vector4 vector = sourceOffsetRow[offsetX].ToScaledVector4(); float sourceRed = vector.X; float sourceBlue = vector.Z; @@ -148,21 +157,21 @@ public void Invoke(in RowInterval rows) int currentIntensity = (int)MathF.Round((sourceBlue + sourceGreen + sourceRed) / 3F * (this.levels - 1)); - Unsafe.Add(ref intensityBinRef, (uint)currentIntensity)++; - Unsafe.Add(ref redBinRef, (uint)currentIntensity) += sourceRed; - Unsafe.Add(ref blueBinRef, (uint)currentIntensity) += sourceBlue; - Unsafe.Add(ref greenBinRef, (uint)currentIntensity) += sourceGreen; + intensityBinsSpan[currentIntensity]++; + redBinSpan[currentIntensity] += sourceRed; + blueBinSpan[currentIntensity] += sourceBlue; + greenBinSpan[currentIntensity] += sourceGreen; - if (Unsafe.Add(ref intensityBinRef, (uint)currentIntensity) > maxIntensity) + if (intensityBinsSpan[currentIntensity] > maxIntensity) { - maxIntensity = Unsafe.Add(ref intensityBinRef, (uint)currentIntensity); + maxIntensity = intensityBinsSpan[currentIntensity]; maxIndex = currentIntensity; } } - float red = MathF.Abs(Unsafe.Add(ref redBinRef, (uint)maxIndex) / maxIntensity); - float blue = MathF.Abs(Unsafe.Add(ref blueBinRef, (uint)maxIndex) / maxIntensity); - float green = MathF.Abs(Unsafe.Add(ref greenBinRef, (uint)maxIndex) / maxIntensity); + float red = redBinSpan[maxIndex] / maxIntensity; + float blue = blueBinSpan[maxIndex] / maxIntensity; + float green = greenBinSpan[maxIndex] / maxIntensity; float alpha = sourceRowVector4Span[x].W; targetRowVector4Span[x] = new Vector4(red, green, blue, alpha); @@ -171,7 +180,7 @@ public void Invoke(in RowInterval rows) Span targetRowAreaPixelSpan = this.targetPixels.DangerousGetRowSpan(y).Slice(this.bounds.X, this.bounds.Width); - PixelOperations.Instance.FromVector4Destructive(this.configuration, targetRowAreaVector4Span, targetRowAreaPixelSpan); + PixelOperations.Instance.FromVector4Destructive(this.configuration, targetRowAreaVector4Span, targetRowAreaPixelSpan, PixelConversionModifiers.Scale); } } } diff --git a/tests/ImageSharp.Benchmarks/Processing/OilPaint.cs b/tests/ImageSharp.Benchmarks/Processing/OilPaint.cs new file mode 100644 index 0000000000..239d5a93bc --- /dev/null +++ b/tests/ImageSharp.Benchmarks/Processing/OilPaint.cs @@ -0,0 +1,19 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using BenchmarkDotNet.Attributes; +using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Processing; + +namespace SixLabors.ImageSharp.Benchmarks.Processing; + +[Config(typeof(Config.MultiFramework))] +public class OilPaint +{ + [Benchmark] + public void DoOilPaint() + { + using Image image = new Image(1920, 1200, new(127, 191, 255)); + image.Mutate(ctx => ctx.OilPaint()); + } +} diff --git a/tests/ImageSharp.Tests/Processing/Processors/Effects/OilPaintTest.cs b/tests/ImageSharp.Tests/Processing/Processors/Effects/OilPaintTest.cs index 990a97bed0..10811a559e 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Effects/OilPaintTest.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Effects/OilPaintTest.cs @@ -27,8 +27,7 @@ public class OilPaintTest [WithFileCollection(nameof(InputImages), nameof(OilPaintValues), PixelTypes.Rgba32)] public void FullImage(TestImageProvider provider, int levels, int brushSize) where TPixel : unmanaged, IPixel - { - provider.RunValidatingProcessorTest( + => provider.RunValidatingProcessorTest( x => { x.OilPaint(levels, brushSize); @@ -36,17 +35,21 @@ public void FullImage(TestImageProvider provider, int levels, in }, ImageComparer.TolerantPercentage(0.01F), appendPixelTypeToFileName: false); - } [Theory] [WithFileCollection(nameof(InputImages), nameof(OilPaintValues), PixelTypes.Rgba32)] [WithTestPatternImages(nameof(OilPaintValues), 100, 100, PixelTypes.Rgba32)] public void InBox(TestImageProvider provider, int levels, int brushSize) where TPixel : unmanaged, IPixel - { - provider.RunRectangleConstrainedValidatingProcessorTest( + => provider.RunRectangleConstrainedValidatingProcessorTest( (x, rect) => x.OilPaint(levels, brushSize, rect), $"{levels}-{brushSize}", ImageComparer.TolerantPercentage(0.01F)); + + [Fact] + public void Issue2518_PixelComponentOutsideOfRange_ThrowsImageProcessingException() + { + using Image image = new(10, 10, new RgbaVector(1, 1, 100)); + Assert.Throws(() => image.Mutate(ctx => ctx.OilPaint())); } }