From eed22c56caced6bc9cee383c072b316ea9492a02 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Mon, 26 Nov 2018 16:48:29 +0100 Subject: [PATCH] fixed a bug in KernelMap caused by overlapping memory areas --- .../Processors/Transforms/KernelMap.cs | 11 +++++++++-- .../Processors/Transforms/KernelMapTests.cs | 16 +++++++--------- .../Processing/Processors/Transforms/SkewTest.cs | 14 +------------- .../ImageSharp.Tests/TestUtilities/TestUtils.cs | 13 +++++++++++++ 4 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Transforms/KernelMap.cs b/src/ImageSharp/Processing/Processors/Transforms/KernelMap.cs index 3f984fef0..3f2bf8dda 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/KernelMap.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/KernelMap.cs @@ -35,7 +35,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms this.periodicRegionMin = period + radius; this.periodicRegionMax = destinationSize - radius; - int width = radius * 2; + int width = (radius * 2) + 1; this.data = memoryAllocator.Allocate2D(width, destinationSize, AllocationOptions.Clean); this.kernels = new ResizeKernel[destinationSize]; } @@ -141,8 +141,15 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms /// private ResizeKernel CreateKernel(int destIdx, int left, int rightIdx) { - int flatStartIndex = destIdx * this.data.Width; int length = rightIdx - left + 1; + + if (length > this.data.Width) + { + throw new InvalidOperationException($"Error in KernelMap.CreateKernel({destIdx},{left},{rightIdx}): left > this.data.Width"); + } + + int flatStartIndex = destIdx * this.data.Width; + Memory bufferSlice = this.data.Memory.Slice(flatStartIndex, length); return new ResizeKernel(left, bufferSlice); } diff --git a/tests/ImageSharp.Tests/Processing/Processors/Transforms/KernelMapTests.cs b/tests/ImageSharp.Tests/Processing/Processors/Transforms/KernelMapTests.cs index 555ed1567..c5916ce0b 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Transforms/KernelMapTests.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Transforms/KernelMapTests.cs @@ -1,5 +1,6 @@ using System; using System.IO; +using System.Runtime.InteropServices; using System.Text; using SixLabors.ImageSharp.PixelFormats; @@ -40,25 +41,22 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Transforms { nameof(KnownResamplers.Lanczos3), 9, 12 }, { nameof(KnownResamplers.Lanczos3), 6, 8 }, { nameof(KnownResamplers.Lanczos3), 8, 6 }, - - // TODO: What's wrong here: { nameof(KnownResamplers.Lanczos3), 20, 12 }, - {nameof(KnownResamplers.Lanczos8), 500, 200 }, - {nameof(KnownResamplers.Lanczos8), 100, 10 }, - {nameof(KnownResamplers.Lanczos8), 100, 80 }, - {nameof(KnownResamplers.Lanczos8), 10, 100 }, + { nameof(KnownResamplers.Lanczos8), 500, 200 }, + { nameof(KnownResamplers.Lanczos8), 100, 10 }, + { nameof(KnownResamplers.Lanczos8), 100, 80 }, + { nameof(KnownResamplers.Lanczos8), 10, 100 }, }; [Theory] [MemberData(nameof(KernelMapData))] public void KernelMapContentIsCorrect(string resamplerName, int srcSize, int destSize) { - var resampler = (IResampler)typeof(KnownResamplers).GetProperty(resamplerName).GetValue(null); - - var kernelMap = KernelMap.Calculate(resampler, destSize, srcSize, Configuration.Default.MemoryAllocator); + IResampler resampler = TestUtils.GetResampler(resamplerName); var referenceMap = ReferenceKernelMap.Calculate(resampler, destSize, srcSize); + var kernelMap = KernelMap.Calculate(resampler, destSize, srcSize, Configuration.Default.MemoryAllocator); #if DEBUG this.Output.WriteLine($"Actual KernelMap:\n{PrintKernelMap(kernelMap)}\n"); diff --git a/tests/ImageSharp.Tests/Processing/Processors/Transforms/SkewTest.cs b/tests/ImageSharp.Tests/Processing/Processors/Transforms/SkewTest.cs index d1d2ea077..29c51543f 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Transforms/SkewTest.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Transforms/SkewTest.cs @@ -60,7 +60,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Transforms { foreach (string resamplerName in ResamplerNames) { - IResampler sampler = GetResampler(resamplerName); + IResampler sampler = TestUtils.GetResampler(resamplerName); using (Image image = provider.GetImage()) { image.Mutate(i => i.Skew(x, y, sampler)); @@ -68,17 +68,5 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Transforms } } } - - private static IResampler GetResampler(string name) - { - PropertyInfo property = typeof(KnownResamplers).GetTypeInfo().GetProperty(name); - - if (property is null) - { - throw new Exception($"No resampler named '{name}"); - } - - return (IResampler)property.GetValue(null); - } } } \ No newline at end of file diff --git a/tests/ImageSharp.Tests/TestUtilities/TestUtils.cs b/tests/ImageSharp.Tests/TestUtilities/TestUtils.cs index d7755ff7a..5ea0bccf0 100644 --- a/tests/ImageSharp.Tests/TestUtilities/TestUtils.cs +++ b/tests/ImageSharp.Tests/TestUtilities/TestUtils.cs @@ -10,6 +10,7 @@ using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing; +using SixLabors.ImageSharp.Processing.Processors.Transforms; using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; using SixLabors.Primitives; @@ -284,5 +285,17 @@ namespace SixLabors.ImageSharp.Tests } public static string AsInvariantString(this FormattableString formattable) => System.FormattableString.Invariant(formattable); + + public static IResampler GetResampler(string name) + { + PropertyInfo property = typeof(KnownResamplers).GetTypeInfo().GetProperty(name); + + if (property is null) + { + throw new Exception($"No resampler named '{name}"); + } + + return (IResampler)property.GetValue(null); + } } } \ No newline at end of file