From da08852291ac5948c4932885c9f6ba11575624cc Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 20 Feb 2020 01:37:15 +0100 Subject: [PATCH] fix JpegDecoder --- .../Jpeg/Components/Block8x8F.CopyTo.cs | 2 +- .../Decoder/JpegComponentPostProcessor.cs | 10 +++++--- src/ImageSharp/Memory/Buffer2DExtensions.cs | 9 ------- src/ImageSharp/Memory/Buffer2D{T}.cs | 3 ++- .../Memory/MemoryAllocatorExtensions.cs | 20 +++++++++++++-- .../Formats/Jpg/JpegDecoderTests.Baseline.cs | 3 ++- .../Formats/Jpg/JpegDecoderTests.Images.cs | 8 ++++-- .../Helpers/RowIntervalTests.cs | 25 ------------------- .../ImageSharp.Tests/Memory/Buffer2DTests.cs | 13 ++++++++++ tests/ImageSharp.Tests/TestImages.cs | 4 +-- .../TestUtilities/TestImageExtensions.cs | 14 +++++------ 11 files changed, 56 insertions(+), 55 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.CopyTo.cs b/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.CopyTo.cs index 6bf9c8483a..64d1d68b7c 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.CopyTo.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Block8x8F.CopyTo.cs @@ -139,4 +139,4 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components } } } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponentPostProcessor.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponentPostProcessor.cs index 39c8be3120..b84d65271c 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponentPostProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegComponentPostProcessor.cs @@ -31,12 +31,14 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder { this.Component = component; this.ImagePostProcessor = imagePostProcessor; - this.ColorBuffer = memoryAllocator.Allocate2D( + this.blockAreaSize = this.Component.SubSamplingDivisors * 8; + this.ColorBuffer = memoryAllocator.Allocate2DOveraligned( imagePostProcessor.PostProcessorBufferSize.Width, - imagePostProcessor.PostProcessorBufferSize.Height); + imagePostProcessor.PostProcessorBufferSize.Height, + this.blockAreaSize.Height); this.BlockRowsPerStep = JpegImagePostProcessor.BlockRowsPerStep / this.Component.SubSamplingDivisors.Height; - this.blockAreaSize = this.Component.SubSamplingDivisors * 8; + } /// @@ -111,4 +113,4 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder this.currentComponentRowInBlocks += this.BlockRowsPerStep; } } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Memory/Buffer2DExtensions.cs b/src/ImageSharp/Memory/Buffer2DExtensions.cs index 361132a827..297d498162 100644 --- a/src/ImageSharp/Memory/Buffer2DExtensions.cs +++ b/src/ImageSharp/Memory/Buffer2DExtensions.cs @@ -156,15 +156,6 @@ namespace SixLabors.ImageSharp.Memory where T : struct => new BufferArea(buffer); - /// - /// Gets a span for all the pixels in defined by - /// - internal static Span GetMultiRowSpan(this Buffer2D buffer, in RowInterval rows) - where T : struct - { - return buffer.GetSingleSpan().Slice(rows.Min * buffer.Width, rows.Height * buffer.Width); - } - /// /// Returns the size of the buffer. /// diff --git a/src/ImageSharp/Memory/Buffer2D{T}.cs b/src/ImageSharp/Memory/Buffer2D{T}.cs index fe9e28e2ee..831d406410 100644 --- a/src/ImageSharp/Memory/Buffer2D{T}.cs +++ b/src/ImageSharp/Memory/Buffer2D{T}.cs @@ -95,6 +95,7 @@ namespace SixLabors.ImageSharp.Memory /// is being propagated from lower levels. /// /// The row index. + /// The of the pixels in the row. /// Thrown when row index is out of range. [MethodImpl(InliningOptions.ShortMethod)] public Span GetRowSpan(int y) @@ -117,7 +118,7 @@ namespace SixLabors.ImageSharp.Memory return ref Unsafe.Add(ref start, (y * this.Width) + x); } - return ref GetElementSlow(x, y); + return ref this.GetElementSlow(x, y); } /// diff --git a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs index 1f1ded9a03..22d1bddd2f 100644 --- a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs +++ b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs @@ -28,8 +28,8 @@ namespace SixLabors.ImageSharp.Memory where T : struct { long groupLength = (long)width * height; - MemoryGroup memorySource = memoryAllocator.AllocateGroup(groupLength, width, options); - return new Buffer2D(memorySource, width, height); + MemoryGroup memoryGroup = memoryAllocator.AllocateGroup(groupLength, width, options); + return new Buffer2D(memoryGroup, width, height); } /// @@ -48,6 +48,22 @@ namespace SixLabors.ImageSharp.Memory where T : struct => Allocate2D(memoryAllocator, size.Width, size.Height, options); + internal static Buffer2D Allocate2DOveraligned( + this MemoryAllocator memoryAllocator, + int width, + int height, + int alignmentMultiplier, + AllocationOptions options = AllocationOptions.None) + where T : struct + { + long groupLength = (long)width * height; + MemoryGroup memoryGroup = memoryAllocator.AllocateGroup( + groupLength, + width * alignmentMultiplier, + options); + return new Buffer2D(memoryGroup, width, height); + } + /// /// Allocates padded buffers for BMP encoder/decoder. (Replacing old PixelRow/PixelArea). /// diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Baseline.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Baseline.cs index 5428039c4a..6ea61892c8 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Baseline.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Baseline.cs @@ -16,6 +16,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg [Theory] [WithFileCollection(nameof(BaselineTestJpegs), PixelTypes.Rgba32, false)] [WithFile(TestImages.Jpeg.Baseline.Calliphora, PixelTypes.Rgba32, true)] + [WithFile(TestImages.Jpeg.Baseline.Turtle420, PixelTypes.Rgba32, true)] public void DecodeBaselineJpeg(TestImageProvider provider, bool enforceDiscontiguousBuffers) where TPixel : struct, IPixel { @@ -26,7 +27,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg if (!string.IsNullOrEmpty(nonContiguousBuffersStr)) { - provider.LimitAllocatorBufferCapacity().InBytesSqrt(200); + provider.LimitAllocatorBufferCapacity().InPixels(1000 * 8); } using Image image = provider.GetImage(JpegDecoder); diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs index b7d7e6b831..a01f4d46cd 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs @@ -13,10 +13,11 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg TestImages.Jpeg.Baseline.Cmyk, TestImages.Jpeg.Baseline.Ycck, TestImages.Jpeg.Baseline.Jpeg400, + TestImages.Jpeg.Baseline.Turtle420, TestImages.Jpeg.Baseline.Testorig420, // BUG: The following image has a high difference compared to the expected output: 1.0096% - // TestImages.Jpeg.Baseline.Jpeg420Small, + TestImages.Jpeg.Baseline.Jpeg420Small, TestImages.Jpeg.Issues.Fuzz.AccessViolationException922, TestImages.Jpeg.Baseline.Jpeg444, TestImages.Jpeg.Baseline.Bad.BadEOF, @@ -95,9 +96,12 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg // Baseline: [TestImages.Jpeg.Baseline.Calliphora] = 0.00002f / 100, [TestImages.Jpeg.Baseline.Bad.BadEOF] = 0.38f / 100, - [TestImages.Jpeg.Baseline.Testorig420] = 0.38f / 100, [TestImages.Jpeg.Baseline.Bad.BadRST] = 0.0589f / 100, + [TestImages.Jpeg.Baseline.Testorig420] = 0.38f / 100, + [TestImages.Jpeg.Baseline.Jpeg420Small] = 1.1f / 100, + [TestImages.Jpeg.Baseline.Turtle420] = 1.0f / 100, + // Progressive: [TestImages.Jpeg.Issues.MissingFF00ProgressiveGirl159] = 0.34f / 100, [TestImages.Jpeg.Issues.BadCoeffsProgressive178] = 0.38f / 100, diff --git a/tests/ImageSharp.Tests/Helpers/RowIntervalTests.cs b/tests/ImageSharp.Tests/Helpers/RowIntervalTests.cs index 2227701951..fd1eb546b5 100644 --- a/tests/ImageSharp.Tests/Helpers/RowIntervalTests.cs +++ b/tests/ImageSharp.Tests/Helpers/RowIntervalTests.cs @@ -10,31 +10,6 @@ namespace SixLabors.ImageSharp.Tests.Helpers { public class RowIntervalTests { - [Theory] - [InlineData(10, 20, 5, 10)] - [InlineData(1, 10, 0, 10)] - [InlineData(1, 10, 5, 8)] - [InlineData(1, 1, 0, 1)] - [InlineData(10, 20, 9, 10)] - [InlineData(10, 20, 0, 1)] - public void GetMultiRowSpan(int width, int height, int min, int max) - { - using (Buffer2D buffer = Configuration.Default.MemoryAllocator.Allocate2D(width, height)) - { - var rows = new RowInterval(min, max); - - Span span = buffer.GetMultiRowSpan(rows); - - ref int expected0 = ref buffer.GetSingleSpan()[min * width]; - int expectedLength = (max - min) * width; - - ref int actual0 = ref span[0]; - - Assert.Equal(span.Length, expectedLength); - Assert.True(Unsafe.AreSame(ref expected0, ref actual0)); - } - } - [Fact] public void Slice1() { diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs index 69de85044d..ea34197415 100644 --- a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs @@ -69,6 +69,19 @@ namespace SixLabors.ImageSharp.Tests.Memory } } + [Theory] + [InlineData(50, 10, 20, 4)] + public void Allocate2DOveraligned(int bufferCapacity, int width, int height, int alignmentMultiplier) + { + this.MemoryAllocator.BufferCapacityInBytes = sizeof(int) * bufferCapacity; + + using Buffer2D buffer = this.MemoryAllocator.Allocate2DOveraligned(width, height, alignmentMultiplier); + MemoryGroup memoryGroup = buffer.MemoryGroup; + int expectedAlignment = width * alignmentMultiplier; + + Assert.Equal(expectedAlignment, memoryGroup.BufferLength); + } + [Fact] public void CreateClean() { diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 16c570d63d..ee919dc2f3 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -142,7 +142,7 @@ namespace SixLabors.ImageSharp.Tests public const string Floorplan = "Jpg/baseline/Floorplan.jpg"; public const string Calliphora = "Jpg/baseline/Calliphora.jpg"; public const string Ycck = "Jpg/baseline/ycck.jpg"; - public const string Turtle = "Jpg/baseline/turtle.jpg"; + public const string Turtle420 = "Jpg/baseline/turtle.jpg"; public const string GammaDalaiLamaGray = "Jpg/baseline/gamma_dalai_lama_gray.jpg"; public const string Hiyamugi = "Jpg/baseline/Hiyamugi.jpg"; public const string Snake = "Jpg/baseline/Snake.jpg"; @@ -161,7 +161,7 @@ namespace SixLabors.ImageSharp.Tests public static readonly string[] All = { Cmyk, Ycck, Exif, Floorplan, - Calliphora, Turtle, GammaDalaiLamaGray, + Calliphora, Turtle420, GammaDalaiLamaGray, Hiyamugi, Jpeg400, Jpeg420Exif, Jpeg444, Ratio1x1, Testorig12bit, YcckSubsample1222 }; diff --git a/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs b/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs index d0836e19f3..4f89af70da 100644 --- a/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs +++ b/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs @@ -762,20 +762,18 @@ namespace SixLabors.ImageSharp.Tests this.pixelSizeInBytes = pixelSizeInBytes; } + public void InBytes(int totalBytes) => this.allocator.BufferCapacityInBytes = totalBytes; + + public void InPixels(int totalPixels) => this.InBytes(totalPixels * this.pixelSizeInBytes); + /// /// Set the maximum buffer capacity to bytesSqrt^2 bytes. /// - public void InBytesSqrt(int bytesSqrt) - { - this.allocator.BufferCapacityInBytes = bytesSqrt * bytesSqrt; - } + public void InBytesSqrt(int bytesSqrt) => this.InBytes(bytesSqrt * bytesSqrt); /// /// Set the maximum buffer capacity to pixelsSqrt^2 x sizeof(TPixel) bytes. /// - public void InPixelsSqrt(int pixelsSqrt) - { - this.allocator.BufferCapacityInBytes = pixelsSqrt * pixelsSqrt * this.pixelSizeInBytes; - } + public void InPixelsSqrt(int pixelsSqrt) => this.InPixels(pixelsSqrt * pixelsSqrt); } }