From 43c542bac85c61ed1f052d2b6b67cd2773428328 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Fri, 25 Aug 2023 03:37:40 +0200 Subject: [PATCH 1/5] OilPaint benchmark --- .../Processing/OilPaint.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 tests/ImageSharp.Benchmarks/Processing/OilPaint.cs 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()); + } +} From 6d6833e01ddf1a84fc70bffd6dff5cc2d75e27cf Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Fri, 25 Aug 2023 03:40:25 +0200 Subject: [PATCH 2/5] fix #2518 --- .../Effects/OilPaintingProcessor{TPixel}.cs | 31 ++++++++++--------- .../Processors/Effects/OilPaintTest.cs | 7 +++++ 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs index 6352230de2..74625cd48b 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; @@ -97,7 +98,7 @@ public void Invoke(in RowInterval rows) * are cleared for each loop iteration, to avoid the repeated allocation for each processed pixel. */ using IMemoryOwner sourceRowBuffer = this.configuration.MemoryAllocator.Allocate(this.source.Width); using IMemoryOwner targetRowBuffer = this.configuration.MemoryAllocator.Allocate(this.source.Width); - using IMemoryOwner bins = this.configuration.MemoryAllocator.Allocate(this.levels * 4); + using IMemoryOwner bins = this.configuration.MemoryAllocator.Allocate(this.levels * 256 * 4); Span sourceRowVector4Span = sourceRowBuffer.Memory.Span; Span sourceRowAreaVector4Span = sourceRowVector4Span.Slice(this.bounds.X, this.bounds.Width); @@ -105,11 +106,11 @@ 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++) { @@ -148,21 +149,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 = MathF.Abs(redBinSpan[maxIndex] / maxIntensity); + float blue = MathF.Abs(blueBinSpan[maxIndex] / maxIntensity); + float green = MathF.Abs(greenBinSpan[maxIndex] / maxIntensity); float alpha = sourceRowVector4Span[x].W; targetRowVector4Span[x] = new Vector4(red, green, blue, alpha); diff --git a/tests/ImageSharp.Tests/Processing/Processors/Effects/OilPaintTest.cs b/tests/ImageSharp.Tests/Processing/Processors/Effects/OilPaintTest.cs index 990a97bed0..7d40cbce5d 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Effects/OilPaintTest.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Effects/OilPaintTest.cs @@ -49,4 +49,11 @@ public void InBox(TestImageProvider provider, int levels, int br $"{levels}-{brushSize}", ImageComparer.TolerantPercentage(0.01F)); } + + [Fact] + public void Issue2518() + { + using Image image = new Image(1920, 1200, new(127, 191, 255)); + image.Mutate(ctx => ctx.OilPaint()); + } } From 075d05f0ba8f65ca59a0119b90b25845deeb4baf Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 28 Aug 2023 21:24:16 +1000 Subject: [PATCH 3/5] Update OilPaintingProcessor{TPixel}.cs --- .../Effects/OilPaintingProcessor{TPixel}.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs index 74625cd48b..ba61821118 100644 --- a/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs @@ -35,13 +35,14 @@ 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); + RowIntervalOperation operation = new(this.SourceRectangle, targetPixels, source.PixelBuffer, this.Configuration, brushSize >> 1, levels); ParallelRowIterator.IterateRowIntervals( this.Configuration, this.SourceRectangle, @@ -117,7 +118,7 @@ public void Invoke(in RowInterval rows) 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++) { @@ -141,7 +142,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; @@ -161,9 +162,9 @@ public void Invoke(in RowInterval rows) } } - float red = MathF.Abs(redBinSpan[maxIndex] / maxIntensity); - float blue = MathF.Abs(blueBinSpan[maxIndex] / maxIntensity); - float green = MathF.Abs(greenBinSpan[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); @@ -172,7 +173,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); } } } From 99771d61a87de9448698aa1368979d5d23e983c6 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Tue, 29 Aug 2023 21:53:35 +0200 Subject: [PATCH 4/5] clamp the vector to 0..1 and undo buffer overallocation --- .../Processors/Effects/OilPaintingProcessor{TPixel}.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs index ba61821118..53ebecbfc7 100644 --- a/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs @@ -99,7 +99,7 @@ public void Invoke(in RowInterval rows) * are cleared for each loop iteration, to avoid the repeated allocation for each processed pixel. */ using IMemoryOwner sourceRowBuffer = this.configuration.MemoryAllocator.Allocate(this.source.Width); using IMemoryOwner targetRowBuffer = this.configuration.MemoryAllocator.Allocate(this.source.Width); - using IMemoryOwner bins = this.configuration.MemoryAllocator.Allocate(this.levels * 256 * 4); + using IMemoryOwner bins = this.configuration.MemoryAllocator.Allocate(this.levels * 4); Span sourceRowVector4Span = sourceRowBuffer.Memory.Span; Span sourceRowAreaVector4Span = sourceRowVector4Span.Slice(this.bounds.X, this.bounds.Width); @@ -142,7 +142,7 @@ public void Invoke(in RowInterval rows) int offsetX = x + fxr; offsetX = Numerics.Clamp(offsetX, 0, maxX); - Vector4 vector = sourceOffsetRow[offsetX].ToScaledVector4(); + Vector4 vector = Vector4.Clamp(sourceOffsetRow[offsetX].ToScaledVector4(), Vector4.Zero, Vector4.One); float sourceRed = vector.X; float sourceBlue = vector.Z; From ac0c1f903d7aa8583bf2eb4e29975f0a88bd5989 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Sat, 2 Sep 2023 00:17:40 +0200 Subject: [PATCH 5/5] throw ImageProcessingException instead of clamping --- .../Effects/OilPaintingProcessor{TPixel}.cs | 11 +++++++++-- .../Processing/Processors/Effects/OilPaintTest.cs | 14 +++++--------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs index 53ebecbfc7..1491fe073b 100644 --- a/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Effects/OilPaintingProcessor{TPixel}.cs @@ -43,10 +43,17 @@ protected override void OnFrameApply(ImageFrame source) source.CopyTo(targetPixels); RowIntervalOperation operation = new(this.SourceRectangle, targetPixels, source.PixelBuffer, this.Configuration, brushSize >> 1, levels); - ParallelRowIterator.IterateRowIntervals( + 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); } @@ -142,7 +149,7 @@ public void Invoke(in RowInterval rows) int offsetX = x + fxr; offsetX = Numerics.Clamp(offsetX, 0, maxX); - Vector4 vector = Vector4.Clamp(sourceOffsetRow[offsetX].ToScaledVector4(), Vector4.Zero, Vector4.One); + Vector4 vector = sourceOffsetRow[offsetX].ToScaledVector4(); float sourceRed = vector.X; float sourceBlue = vector.Z; diff --git a/tests/ImageSharp.Tests/Processing/Processors/Effects/OilPaintTest.cs b/tests/ImageSharp.Tests/Processing/Processors/Effects/OilPaintTest.cs index 7d40cbce5d..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,24 +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() + public void Issue2518_PixelComponentOutsideOfRange_ThrowsImageProcessingException() { - using Image image = new Image(1920, 1200, new(127, 191, 255)); - image.Mutate(ctx => ctx.OilPaint()); + using Image image = new(10, 10, new RgbaVector(1, 1, 100)); + Assert.Throws(() => image.Mutate(ctx => ctx.OilPaint())); } }