From e6ab9833745e3e75a2ea92e97178ae7272d229b1 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 10 Mar 2022 01:35:11 +1100 Subject: [PATCH 1/6] Ensure pad color is never overwritten --- .../Transforms/Resize/ResizeProcessor{TPixel}.cs | 3 +++ .../Processors/Transforms/Resize/ResizeWorker.cs | 16 ++++++++++------ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs index c0bf9291e0..e0ccb487f7 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs @@ -61,6 +61,9 @@ public void ApplyTransform(in TResampler sampler) Rectangle destinationRectangle = this.destinationRectangle; bool compand = this.options.Compand; bool premultiplyAlpha = this.options.PremultiplyAlpha; + + this.options.PadColor = Color.GreenYellow; + bool shouldFill = (this.options.Mode == ResizeMode.BoxPad || this.options.Mode == ResizeMode.Pad) && this.options.PadColor != default; TPixel fillColor = this.options.PadColor.ToPixel(); diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs index 90cbf8bda0..5f3c609898 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs @@ -80,19 +80,19 @@ public ResizeWorker( int numberOfWindowBands = ResizeHelper.CalculateResizeWorkerHeightInWindowBands( this.windowBandHeight, - destWidth, + targetWorkingRect.Width, workingBufferLimitHintInBytes); this.workerHeight = Math.Min(this.sourceRectangle.Height, numberOfWindowBands * this.windowBandHeight); this.transposedFirstPassBuffer = configuration.MemoryAllocator.Allocate2D( this.workerHeight, - destWidth, + targetWorkingRect.Width, preferContiguosImageBuffers: true, options: AllocationOptions.Clean); this.tempRowBuffer = configuration.MemoryAllocator.Allocate(this.sourceRectangle.Width); - this.tempColumnBuffer = configuration.MemoryAllocator.Allocate(destWidth); + this.tempColumnBuffer = configuration.MemoryAllocator.Allocate(targetWorkingRect.Width); this.currentWindow = new RowInterval(0, this.workerHeight); } @@ -118,6 +118,9 @@ public void FillDestinationPixels(RowInterval rowInterval, Buffer2D dest // When creating transposedFirstPassBuffer, we made sure it's contiguous: Span transposedFirstPassBufferSpan = this.transposedFirstPassBuffer.DangerousGetSingleSpan(); + int left = this.targetWorkingRect.Left; + int right = this.targetWorkingRect.Right; + int width = this.targetWorkingRect.Width; for (int y = rowInterval.Min; y < rowInterval.Max; y++) { // Ensure offsets are normalized for cropping and padding. @@ -131,17 +134,18 @@ public void FillDestinationPixels(RowInterval rowInterval, Buffer2D dest ref Vector4 tempRowBase = ref MemoryMarshal.GetReference(tempColSpan); int top = kernel.StartIndex - this.currentWindow.Min; + ref Vector4 fpBase = ref transposedFirstPassBufferSpan[top]; - for (int x = 0; x < this.destWidth; x++) + for (int x = left; x < right; x++) { ref Vector4 firstPassColumnBase = ref Unsafe.Add(ref fpBase, x * this.workerHeight); // Destination color components - Unsafe.Add(ref tempRowBase, x) = kernel.ConvolveCore(ref firstPassColumnBase); + Unsafe.Add(ref tempRowBase, x - left) = kernel.ConvolveCore(ref firstPassColumnBase); } - Span targetRowSpan = destination.DangerousGetRowSpan(y); + Span targetRowSpan = destination.DangerousGetRowSpan(y).Slice(left, width); PixelOperations.Instance.FromVector4Destructive(this.configuration, tempColSpan, targetRowSpan, this.conversionModifiers); } From baa8c7b8e76a627352d50a23e1b1a41682c7fb23 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 10 Mar 2022 02:06:47 +1100 Subject: [PATCH 2/6] Fix pad memory access error --- .../Processing/Extensions/Transforms/PadExtensions.cs | 6 +++++- .../Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs | 5 +---- .../Processing/Processors/Transforms/Resize/ResizeWorker.cs | 4 ++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/ImageSharp/Processing/Extensions/Transforms/PadExtensions.cs b/src/ImageSharp/Processing/Extensions/Transforms/PadExtensions.cs index ff374a9ac4..5e9e420321 100644 --- a/src/ImageSharp/Processing/Extensions/Transforms/PadExtensions.cs +++ b/src/ImageSharp/Processing/Extensions/Transforms/PadExtensions.cs @@ -1,6 +1,8 @@ // Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. +using System; + namespace SixLabors.ImageSharp.Processing { /// @@ -29,9 +31,11 @@ public static IImageProcessingContext Pad(this IImageProcessingContext source, i /// The to allow chaining of operations. public static IImageProcessingContext Pad(this IImageProcessingContext source, int width, int height, Color color) { + Size size = source.GetCurrentSize(); var options = new ResizeOptions { - Size = new Size(width, height), + // Prevent downsizing. + Size = new Size(Math.Max(width, size.Width), Math.Max(height, size.Height)), Mode = ResizeMode.BoxPad, Sampler = KnownResamplers.NearestNeighbor, PadColor = color diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs index e0ccb487f7..af8adc2d5e 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs @@ -61,12 +61,9 @@ public void ApplyTransform(in TResampler sampler) Rectangle destinationRectangle = this.destinationRectangle; bool compand = this.options.Compand; bool premultiplyAlpha = this.options.PremultiplyAlpha; - - this.options.PadColor = Color.GreenYellow; - + TPixel fillColor = this.options.PadColor.ToPixel(); bool shouldFill = (this.options.Mode == ResizeMode.BoxPad || this.options.Mode == ResizeMode.Pad) && this.options.PadColor != default; - TPixel fillColor = this.options.PadColor.ToPixel(); // Handle resize dimensions identical to the original if (source.Width == destination.Width diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs index 5f3c609898..a4a504317e 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs @@ -80,14 +80,14 @@ public ResizeWorker( int numberOfWindowBands = ResizeHelper.CalculateResizeWorkerHeightInWindowBands( this.windowBandHeight, - targetWorkingRect.Width, + destWidth, workingBufferLimitHintInBytes); this.workerHeight = Math.Min(this.sourceRectangle.Height, numberOfWindowBands * this.windowBandHeight); this.transposedFirstPassBuffer = configuration.MemoryAllocator.Allocate2D( this.workerHeight, - targetWorkingRect.Width, + destWidth, preferContiguosImageBuffers: true, options: AllocationOptions.Clean); From 06bdf138bd53cb59f9266621eb42be5c20d39e83 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 10 Mar 2022 02:29:22 +1100 Subject: [PATCH 3/6] Optimize memory usage for resize pad operations --- .../Transforms/Resize/ResizeKernelMap.cs | 2 +- .../Resize/ResizeProcessor{TPixel}.cs | 13 +++++-------- .../Transforms/Resize/ResizeWorker.cs | 18 ++++++++---------- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs index c9dda5f6bc..f4bda8bf4f 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs @@ -101,7 +101,7 @@ protected virtual void Dispose(bool disposing) /// Returns a for an index value between 0 and DestinationSize - 1. /// [MethodImpl(InliningOptions.ShortMethod)] - internal ref ResizeKernel GetKernel(int destIdx) => ref this.kernels[destIdx]; + internal ref ResizeKernel GetKernel(nint destIdx) => ref this.kernels[destIdx]; /// /// Computes the weights to apply at each pixel when resizing. diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs index af8adc2d5e..b346758963 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs @@ -209,21 +209,18 @@ private static void ApplyResizeFrameTransform( // To reintroduce parallel processing, we would launch multiple workers // for different row intervals of the image. - using (var worker = new ResizeWorker( + using var worker = new ResizeWorker( configuration, sourceRegion, conversionModifiers, horizontalKernelMap, verticalKernelMap, - destination.Width, interest, - destinationRectangle.Location)) - { - worker.Initialize(); + destinationRectangle.Location); + worker.Initialize(); - var workingInterval = new RowInterval(interest.Top, interest.Bottom); - worker.FillDestinationPixels(workingInterval, destination.PixelBuffer); - } + var workingInterval = new RowInterval(interest.Top, interest.Bottom); + worker.FillDestinationPixels(workingInterval, destination.PixelBuffer); } private readonly struct NNRowOperation : IRowOperation diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs index a4a504317e..2a582ed8ab 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs @@ -39,8 +39,6 @@ internal sealed class ResizeWorker : IDisposable private readonly ResizeKernelMap verticalKernelMap; - private readonly int destWidth; - private readonly Rectangle targetWorkingRect; private readonly Point targetOrigin; @@ -57,7 +55,6 @@ public ResizeWorker( PixelConversionModifiers conversionModifiers, ResizeKernelMap horizontalKernelMap, ResizeKernelMap verticalKernelMap, - int destWidth, Rectangle targetWorkingRect, Point targetOrigin) { @@ -67,7 +64,6 @@ public ResizeWorker( this.conversionModifiers = conversionModifiers; this.horizontalKernelMap = horizontalKernelMap; this.verticalKernelMap = verticalKernelMap; - this.destWidth = destWidth; this.targetWorkingRect = targetWorkingRect; this.targetOrigin = targetOrigin; @@ -80,14 +76,14 @@ public ResizeWorker( int numberOfWindowBands = ResizeHelper.CalculateResizeWorkerHeightInWindowBands( this.windowBandHeight, - destWidth, + targetWorkingRect.Width, workingBufferLimitHintInBytes); this.workerHeight = Math.Min(this.sourceRectangle.Height, numberOfWindowBands * this.windowBandHeight); this.transposedFirstPassBuffer = configuration.MemoryAllocator.Allocate2D( this.workerHeight, - destWidth, + targetWorkingRect.Width, preferContiguosImageBuffers: true, options: AllocationOptions.Clean); @@ -137,9 +133,9 @@ public void FillDestinationPixels(RowInterval rowInterval, Buffer2D dest ref Vector4 fpBase = ref transposedFirstPassBufferSpan[top]; - for (int x = left; x < right; x++) + for (nint x = left; x < right; x++) { - ref Vector4 firstPassColumnBase = ref Unsafe.Add(ref fpBase, x * this.workerHeight); + ref Vector4 firstPassColumnBase = ref Unsafe.Add(ref fpBase, (x - left) * this.workerHeight); // Destination color components Unsafe.Add(ref tempRowBase, x - left) = kernel.ConvolveCore(ref firstPassColumnBase); @@ -174,6 +170,8 @@ private void CalculateFirstPassValues(RowInterval calculationInterval) Span tempRowSpan = this.tempRowBuffer.GetSpan(); Span transposedFirstPassBufferSpan = this.transposedFirstPassBuffer.DangerousGetSingleSpan(); + int left = this.targetWorkingRect.Left; + int right = this.targetWorkingRect.Right; for (int y = calculationInterval.Min; y < calculationInterval.Max; y++) { Span sourceRow = this.source.DangerousGetRowSpan(y); @@ -188,13 +186,13 @@ private void CalculateFirstPassValues(RowInterval calculationInterval) // Span firstPassSpan = transposedFirstPassBufferSpan.Slice(y - this.currentWindow.Min); ref Vector4 firstPassBaseRef = ref transposedFirstPassBufferSpan[y - this.currentWindow.Min]; - for (int x = this.targetWorkingRect.Left; x < this.targetWorkingRect.Right; x++) + for (nint x = left; x < right; x++) { ResizeKernel kernel = this.horizontalKernelMap.GetKernel(x - this.targetOrigin.X); // optimization for: // firstPassSpan[x * this.workerHeight] = kernel.Convolve(tempRowSpan); - Unsafe.Add(ref firstPassBaseRef, x * this.workerHeight) = kernel.Convolve(tempRowSpan); + Unsafe.Add(ref firstPassBaseRef, (x - left) * this.workerHeight) = kernel.Convolve(tempRowSpan); } } } From 0197de6f6c289c068d0b614a5254fe655ac34aee Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 10 Mar 2022 02:38:50 +1100 Subject: [PATCH 4/6] Update tests --- .../Processing/Processors/Transforms/ResizeTests.cs | 6 ++++-- .../ResizeTests/ResizeWithBoxPadMode_CalliphoraPartial.png | 4 ++-- .../ResizeTests/ResizeWithPadMode_CalliphoraPartial.png | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs b/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs index 6e275b824e..b1ff3df08c 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Transforms/ResizeTests.cs @@ -472,7 +472,8 @@ public void ResizeWithBoxPadMode(TestImageProvider provider) var options = new ResizeOptions { Size = new Size(image.Width + 200, image.Height + 200), - Mode = ResizeMode.BoxPad + Mode = ResizeMode.BoxPad, + PadColor = Color.HotPink }; image.Mutate(x => x.Resize(options)); @@ -580,7 +581,8 @@ public void ResizeWithPadMode(TestImageProvider provider) var options = new ResizeOptions { Size = new Size(image.Width + 200, image.Height), - Mode = ResizeMode.Pad + Mode = ResizeMode.Pad, + PadColor = Color.Lavender }; image.Mutate(x => x.Resize(options)); diff --git a/tests/Images/External/ReferenceOutput/ResizeTests/ResizeWithBoxPadMode_CalliphoraPartial.png b/tests/Images/External/ReferenceOutput/ResizeTests/ResizeWithBoxPadMode_CalliphoraPartial.png index 35d633c86f..804c5dcf9a 100644 --- a/tests/Images/External/ReferenceOutput/ResizeTests/ResizeWithBoxPadMode_CalliphoraPartial.png +++ b/tests/Images/External/ReferenceOutput/ResizeTests/ResizeWithBoxPadMode_CalliphoraPartial.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:1642520a9d4491f55af5a83280423929529e3d528637538d148b9dc2ecdd6d2d -size 365839 +oid sha256:77086032bab11b91c68bc1686179063edbadc9d453574e4f087b2bbd677b4c8e +size 402367 diff --git a/tests/Images/External/ReferenceOutput/ResizeTests/ResizeWithPadMode_CalliphoraPartial.png b/tests/Images/External/ReferenceOutput/ResizeTests/ResizeWithPadMode_CalliphoraPartial.png index 690c2ad46c..bfa048f82c 100644 --- a/tests/Images/External/ReferenceOutput/ResizeTests/ResizeWithPadMode_CalliphoraPartial.png +++ b/tests/Images/External/ReferenceOutput/ResizeTests/ResizeWithPadMode_CalliphoraPartial.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:060826324dcd4baa1df1b075707a9bd9527cf87c1d156e3beb3d8abb499bdcd5 -size 361829 +oid sha256:5c0653aa2b726574fbea4cc308c269ff5e534d38bb48c0e77470c11042a395fd +size 400267 From ebea485f96f96debbc883fffa1b4ded20581cc10 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 14 Mar 2022 19:50:39 +1100 Subject: [PATCH 5/6] Update ResizeWorker.cs --- .../Processors/Transforms/Resize/ResizeWorker.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs index 2a582ed8ab..5e7de18037 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs @@ -133,12 +133,12 @@ public void FillDestinationPixels(RowInterval rowInterval, Buffer2D dest ref Vector4 fpBase = ref transposedFirstPassBufferSpan[top]; - for (nint x = left; x < right; x++) + for (nint x = 0; x < (right - left); x++) { - ref Vector4 firstPassColumnBase = ref Unsafe.Add(ref fpBase, (x - left) * this.workerHeight); + ref Vector4 firstPassColumnBase = ref Unsafe.Add(ref fpBase, x * this.workerHeight); // Destination color components - Unsafe.Add(ref tempRowBase, x - left) = kernel.ConvolveCore(ref firstPassColumnBase); + Unsafe.Add(ref tempRowBase, x) = kernel.ConvolveCore(ref firstPassColumnBase); } Span targetRowSpan = destination.DangerousGetRowSpan(y).Slice(left, width); @@ -172,6 +172,7 @@ private void CalculateFirstPassValues(RowInterval calculationInterval) int left = this.targetWorkingRect.Left; int right = this.targetWorkingRect.Right; + int targetOriginX = left > this.targetOrigin.X ? left - this.targetOrigin.X : this.targetOrigin.X; for (int y = calculationInterval.Min; y < calculationInterval.Max; y++) { Span sourceRow = this.source.DangerousGetRowSpan(y); @@ -186,13 +187,13 @@ private void CalculateFirstPassValues(RowInterval calculationInterval) // Span firstPassSpan = transposedFirstPassBufferSpan.Slice(y - this.currentWindow.Min); ref Vector4 firstPassBaseRef = ref transposedFirstPassBufferSpan[y - this.currentWindow.Min]; - for (nint x = left; x < right; x++) + for (nint x = 0; x < (right - left); x++) { - ResizeKernel kernel = this.horizontalKernelMap.GetKernel(x - this.targetOrigin.X); + ResizeKernel kernel = this.horizontalKernelMap.GetKernel(x - targetOriginX); // optimization for: // firstPassSpan[x * this.workerHeight] = kernel.Convolve(tempRowSpan); - Unsafe.Add(ref firstPassBaseRef, (x - left) * this.workerHeight) = kernel.Convolve(tempRowSpan); + Unsafe.Add(ref firstPassBaseRef, x * this.workerHeight) = kernel.Convolve(tempRowSpan); } } } From afe9505b291717e2fdef428c604bd37a4a771dd1 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 14 Mar 2022 20:59:59 +1100 Subject: [PATCH 6/6] Fix offset --- .../Processing/Processors/Transforms/Resize/ResizeWorker.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs index 5e7de18037..45f35b7d90 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs @@ -172,7 +172,7 @@ private void CalculateFirstPassValues(RowInterval calculationInterval) int left = this.targetWorkingRect.Left; int right = this.targetWorkingRect.Right; - int targetOriginX = left > this.targetOrigin.X ? left - this.targetOrigin.X : this.targetOrigin.X; + int targetOriginX = this.targetOrigin.X; for (int y = calculationInterval.Min; y < calculationInterval.Max; y++) { Span sourceRow = this.source.DangerousGetRowSpan(y); @@ -187,13 +187,13 @@ private void CalculateFirstPassValues(RowInterval calculationInterval) // Span firstPassSpan = transposedFirstPassBufferSpan.Slice(y - this.currentWindow.Min); ref Vector4 firstPassBaseRef = ref transposedFirstPassBufferSpan[y - this.currentWindow.Min]; - for (nint x = 0; x < (right - left); x++) + for (nint x = left, z = 0; x < right; x++, z++) { ResizeKernel kernel = this.horizontalKernelMap.GetKernel(x - targetOriginX); // optimization for: // firstPassSpan[x * this.workerHeight] = kernel.Convolve(tempRowSpan); - Unsafe.Add(ref firstPassBaseRef, x * this.workerHeight) = kernel.Convolve(tempRowSpan); + Unsafe.Add(ref firstPassBaseRef, z * this.workerHeight) = kernel.Convolve(tempRowSpan); } } }