From 9ffcc51ed6b72f557d804f6393b0a610c673c703 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 15 Feb 2022 02:19:35 +0100 Subject: [PATCH 1/6] Respect PreferContiguousImageBuffers in decoders --- .../Decoder/SpectralConverter{TPixel}.cs | 5 ++- src/ImageSharp/Image.Decode.cs | 6 ++- .../Image/LargeImageIntegrationTests.cs | 37 ++++++++++++++++++- 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs index 40411ef28..430adeb21 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs @@ -111,7 +111,10 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder this.pixelRowsPerStep = majorVerticalSamplingFactor * blockPixelHeight; // pixel buffer for resulting image - this.pixelBuffer = allocator.Allocate2D(frame.PixelWidth, frame.PixelHeight); + this.pixelBuffer = allocator.Allocate2D( + frame.PixelWidth, + frame.PixelHeight, + this.configuration.PreferContiguousImageBuffers); this.paddedProxyPixelRow = allocator.Allocate(frame.PixelWidth + 3); // component processors from spectral to Rgba32 diff --git a/src/ImageSharp/Image.Decode.cs b/src/ImageSharp/Image.Decode.cs index ee340bf86..c892b82ba 100644 --- a/src/ImageSharp/Image.Decode.cs +++ b/src/ImageSharp/Image.Decode.cs @@ -37,8 +37,10 @@ namespace SixLabors.ImageSharp ImageMetadata metadata) where TPixel : unmanaged, IPixel { - Buffer2D uninitializedMemoryBuffer = - configuration.MemoryAllocator.Allocate2D(width, height); + Buffer2D uninitializedMemoryBuffer = configuration.MemoryAllocator.Allocate2D( + width, + height, + configuration.PreferContiguousImageBuffers); return new Image(configuration, uninitializedMemoryBuffer.FastMemoryGroup, width, height, metadata); } diff --git a/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs b/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs index b2ee9d673..00714c816 100644 --- a/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs +++ b/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs @@ -2,7 +2,10 @@ // Licensed under the Apache License, Version 2.0. using System; +using System.IO; using Microsoft.DotNet.RemoteExecutor; +using SixLabors.ImageSharp.Advanced; +using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing; @@ -32,12 +35,44 @@ namespace SixLabors.ImageSharp.Tests Configuration configuration = Configuration.Default.Clone(); configuration.PreferContiguousImageBuffers = true; - using var image = new Image(configuration, 8192, 4096); + using var image = new Image(configuration, 2048, 2048); Assert.True(image.DangerousTryGetSinglePixelMemory(out Memory mem)); Assert.Equal(8192 * 4096, mem.Length); } } + [Theory] + [InlineData("bmp")] + [InlineData("png")] + [InlineData("jpeg")] + [InlineData("gif")] + [InlineData("tiff")] + [InlineData("webp")] + public void PreferContiguousImageBuffers_LoadImage_BufferIsContiguous(string formatOuter) + { + // Run remotely to avoid large allocation in the test process: + // RemoteExecutor.Invoke(RunTest).Dispose(); + RunTest(formatOuter); + + static void RunTest(string formatInner) + { + Configuration configuration = Configuration.Default.Clone(); + configuration.PreferContiguousImageBuffers = true; + IImageEncoder encoder = configuration.ImageFormatsManager.FindEncoder( + configuration.ImageFormatsManager.FindFormatByFileExtension(formatInner)); + string dir = TestEnvironment.CreateOutputDirectory(".Temp"); + string path = Path.Combine(dir, $"{Guid.NewGuid().ToString()}.{formatInner}"); + using (Image temp = new(2048, 2048)) + { + temp.Save(path, encoder); + } + + using var image = Image.Load(configuration, path); + File.Delete(path); + Assert.Equal(1, image.GetPixelMemoryGroup().Count); + } + } + [Theory] [WithBasicTestPatternImages(width: 10, height: 10, PixelTypes.Rgba32)] public void DangerousTryGetSinglePixelMemory_WhenImageTooLarge_ReturnsFalse(TestImageProvider provider) From 1e410719c0b095de8361183184af043529f2384c Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 15 Feb 2022 02:22:25 +0100 Subject: [PATCH 2/6] fix test --- tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs b/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs index 00714c816..f44f124b0 100644 --- a/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs +++ b/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs @@ -51,8 +51,7 @@ namespace SixLabors.ImageSharp.Tests public void PreferContiguousImageBuffers_LoadImage_BufferIsContiguous(string formatOuter) { // Run remotely to avoid large allocation in the test process: - // RemoteExecutor.Invoke(RunTest).Dispose(); - RunTest(formatOuter); + RemoteExecutor.Invoke(RunTest, formatOuter).Dispose(); static void RunTest(string formatInner) { From f387d4a1615d9a4eef11b9b5d49380c4091803e2 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 15 Feb 2022 03:34:35 +0100 Subject: [PATCH 3/6] Fix test --- tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs b/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs index f44f124b0..357d02be4 100644 --- a/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs +++ b/tests/ImageSharp.Tests/Image/LargeImageIntegrationTests.cs @@ -37,7 +37,7 @@ namespace SixLabors.ImageSharp.Tests using var image = new Image(configuration, 2048, 2048); Assert.True(image.DangerousTryGetSinglePixelMemory(out Memory mem)); - Assert.Equal(8192 * 4096, mem.Length); + Assert.Equal(2048 * 2048, mem.Length); } } From 3ea84859896a45d85a633163e9a350ed95006ac6 Mon Sep 17 00:00:00 2001 From: Dmitry Pentin Date: Tue, 15 Feb 2022 06:42:52 +0300 Subject: [PATCH 4/6] Removed redundant CommitConversion() call --- .../Components/Decoder/HuffmanScanDecoder.cs | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs index cf2fd0290..e1faf93f4 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs @@ -31,7 +31,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// /// Number of component in the current scan. /// - private int componentsCount; + private int scanComponentCount; /// /// The reset interval determined by RST markers. @@ -112,11 +112,12 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// /// Decodes the entropy coded data. /// - public void ParseEntropyCodedData(int componentCount) + /// Component count in the current scan. + public void ParseEntropyCodedData(int scanComponentCount) { this.cancellationToken.ThrowIfCancellationRequested(); - this.componentsCount = componentCount; + this.scanComponentCount = scanComponentCount; this.scanBuffer = new HuffmanScanBuffer(this.stream); @@ -148,7 +149,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder private void ParseBaselineData() { - if (this.componentsCount != 1) + if (this.scanComponentCount != 1) { this.ParseBaselineDataInterleaved(); this.spectralConverter.CommitConversion(); @@ -180,7 +181,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder { // Scan an interleaved mcu... process components in order int mcuCol = mcu % mcusPerLine; - for (int k = 0; k < this.componentsCount; k++) + for (int k = 0; k < this.scanComponentCount; k++) { int order = this.frame.ComponentOrder[k]; JpegComponent component = this.components[order]; @@ -228,9 +229,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder // Convert from spectral to actual pixels via given converter this.spectralConverter.ConvertStrideBaseline(); } - - // Stride conversion must be sealed for stride conversion approach - this.spectralConverter.CommitConversion(); } private void ParseBaselineDataNonInterleaved() @@ -336,7 +334,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder } // AC scans may have only one component. - if (this.componentsCount != 1) + if (this.scanComponentCount != 1) { invalid = true; } @@ -368,7 +366,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder { this.CheckProgressiveData(); - if (this.componentsCount == 1) + if (this.scanComponentCount == 1) { this.ParseProgressiveDataNonInterleaved(); } @@ -393,7 +391,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder // Scan an interleaved mcu... process components in order int mcuRow = mcu / mcusPerLine; int mcuCol = mcu % mcusPerLine; - for (int k = 0; k < this.componentsCount; k++) + for (int k = 0; k < this.scanComponentCount; k++) { int order = this.frame.ComponentOrder[k]; JpegComponent component = this.components[order]; From 63792362b45498b215f6d22bdfcccc7a0b9a4b55 Mon Sep 17 00:00:00 2001 From: Dirk Lemstra Date: Fri, 18 Feb 2022 12:12:27 +0100 Subject: [PATCH 5/6] Restored checkboxes. --- .github/ISSUE_TEMPLATE/oss-bug-report.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/ISSUE_TEMPLATE/oss-bug-report.yml b/.github/ISSUE_TEMPLATE/oss-bug-report.yml index 00138d511..a4e5619d4 100644 --- a/.github/ISSUE_TEMPLATE/oss-bug-report.yml +++ b/.github/ISSUE_TEMPLATE/oss-bug-report.yml @@ -2,6 +2,18 @@ name: "OSS : Bug Report" description: Create a report to help us improve the project. OSS Issues are not guaranteed to be triaged. labels: ["needs triage"] body: +- type: checkboxes + attributes: + label: Prerequisites + options: + - label: I have written a descriptive issue title + required: true + - label: I have verified that I am running the latest version of ImageSharp + required: true + - label: I have verified if the problem exist in both `DEBUG` and `RELEASE` mode + required: true + - label: I have searched [open](https://github.com/SixLabors/ImageSharp/issues) and [closed](https://github.com/SixLabors/ImageSharp/issues?q=is%3Aissue+is%3Aclosed) issues to ensure it has not already been reported + required: true - type: input attributes: label: ImageSharp version From 92da254bb8203ea5eb2fc8f58f849f487921466e Mon Sep 17 00:00:00 2001 From: Dirk Lemstra Date: Fri, 18 Feb 2022 12:14:19 +0100 Subject: [PATCH 6/6] Restored checkboxes. --- .github/ISSUE_TEMPLATE/commercial-bug-report.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/.github/ISSUE_TEMPLATE/commercial-bug-report.yml b/.github/ISSUE_TEMPLATE/commercial-bug-report.yml index e96677b24..6b4d914d7 100644 --- a/.github/ISSUE_TEMPLATE/commercial-bug-report.yml +++ b/.github/ISSUE_TEMPLATE/commercial-bug-report.yml @@ -4,6 +4,20 @@ description: | Please contact help@sixlabors.com for issues requiring private support. labels: ["commercial", "needs triage"] body: +- type: checkboxes + attributes: + label: Prerequisites + options: + - label: I have bought a Commercial License + required: true + - label: I have written a descriptive issue title + required: true + - label: I have verified that I am running the latest version of ImageSharp + required: true + - label: I have verified if the problem exist in both `DEBUG` and `RELEASE` mode + required: true + - label: I have searched [open](https://github.com/SixLabors/ImageSharp/issues) and [closed](https://github.com/SixLabors/ImageSharp/issues?q=is%3Aissue+is%3Aclosed) issues to ensure it has not already been reported + required: true - type: input attributes: label: ImageSharp version