From 63792362b45498b215f6d22bdfcccc7a0b9a4b55 Mon Sep 17 00:00:00 2001 From: Dirk Lemstra Date: Fri, 18 Feb 2022 12:12:27 +0100 Subject: [PATCH 01/49] 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 02/49] 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 From 204f94e84bc01975c191ce0ed3611e69c1d9425f Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Fri, 18 Feb 2022 14:43:10 +0100 Subject: [PATCH 03/49] Add tiff decoder option to only decode first frame --- src/ImageSharp/Formats/Gif/GifDecoder.cs | 1 - src/ImageSharp/Formats/Tiff/ITiffDecoderOptions.cs | 7 +++++++ src/ImageSharp/Formats/Tiff/TiffDecoder.cs | 6 ++++++ src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs | 14 ++++++++++++-- 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifDecoder.cs b/src/ImageSharp/Formats/Gif/GifDecoder.cs index 196d77ad7..c31a2c1c9 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoder.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoder.cs @@ -5,7 +5,6 @@ using System.IO; using System.Threading; using System.Threading.Tasks; using SixLabors.ImageSharp.IO; -using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.PixelFormats; diff --git a/src/ImageSharp/Formats/Tiff/ITiffDecoderOptions.cs b/src/ImageSharp/Formats/Tiff/ITiffDecoderOptions.cs index 537238439..d6d1bb8a4 100644 --- a/src/ImageSharp/Formats/Tiff/ITiffDecoderOptions.cs +++ b/src/ImageSharp/Formats/Tiff/ITiffDecoderOptions.cs @@ -1,6 +1,8 @@ // Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. +using SixLabors.ImageSharp.Metadata; + namespace SixLabors.ImageSharp.Formats.Tiff { /// @@ -12,5 +14,10 @@ namespace SixLabors.ImageSharp.Formats.Tiff /// Gets a value indicating whether the metadata should be ignored when the image is being decoded. /// bool IgnoreMetadata { get; } + + /// + /// Gets the decoding mode for multi-frame images. + /// + FrameDecodingMode DecodingMode { get; } } } diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoder.cs b/src/ImageSharp/Formats/Tiff/TiffDecoder.cs index 9d52e34df..b4d752019 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoder.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoder.cs @@ -4,6 +4,7 @@ using System.IO; using System.Threading; using System.Threading.Tasks; +using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.PixelFormats; namespace SixLabors.ImageSharp.Formats.Tiff @@ -18,6 +19,11 @@ namespace SixLabors.ImageSharp.Formats.Tiff /// public bool IgnoreMetadata { get; set; } + /// + /// Gets or sets the decoding mode for multi-frame images. + /// + public FrameDecodingMode DecodingMode { get; set; } + /// public Image Decode(Configuration configuration, Stream stream) where TPixel : unmanaged, IPixel diff --git a/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs b/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs index 05c5358f5..cd06282f1 100644 --- a/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs +++ b/src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs @@ -13,7 +13,6 @@ using SixLabors.ImageSharp.IO; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.Metadata.Profiles.Exif; -using SixLabors.ImageSharp.Metadata.Profiles.Xmp; using SixLabors.ImageSharp.PixelFormats; namespace SixLabors.ImageSharp.Formats.Tiff @@ -33,6 +32,11 @@ namespace SixLabors.ImageSharp.Formats.Tiff /// private readonly bool ignoreMetadata; + /// + /// Gets the decoding mode for multi-frame images + /// + private FrameDecodingMode decodingMode; + /// /// The stream to decode from. /// @@ -59,6 +63,7 @@ namespace SixLabors.ImageSharp.Formats.Tiff this.Configuration = configuration ?? Configuration.Default; this.ignoreMetadata = options.IgnoreMetadata; + this.decodingMode = options.DecodingMode; this.memoryAllocator = this.Configuration.MemoryAllocator; } @@ -160,11 +165,16 @@ namespace SixLabors.ImageSharp.Formats.Tiff cancellationToken.ThrowIfCancellationRequested(); ImageFrame frame = this.DecodeFrame(ifd, cancellationToken); frames.Add(frame); + + if (this.decodingMode is FrameDecodingMode.First) + { + break; + } } ImageMetadata metadata = TiffDecoderMetadataCreator.Create(frames, this.ignoreMetadata, reader.ByteOrder, reader.IsBigTiff); - // TODO: Tiff frames can have different sizes + // TODO: Tiff frames can have different sizes. ImageFrame root = frames[0]; this.Dimensions = root.Size(); foreach (ImageFrame frame in frames) From a963fa09adb839d9ddba40f2524cb6ed00b4ed13 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Fri, 18 Feb 2022 14:53:00 +0100 Subject: [PATCH 04/49] Add test for decoding only the first frame --- .../Formats/Tiff/TiffDecoderTests.cs | 12 ++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Tiff/SKC1H3.tiff | 3 +++ 3 files changed, 16 insertions(+) create mode 100644 tests/Images/Input/Tiff/SKC1H3.tiff diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index ea0544acf..51d38dc58 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -4,6 +4,7 @@ // ReSharper disable InconsistentNaming using System; using System.IO; +using SixLabors.ImageSharp.Formats.Tiff; using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; @@ -365,6 +366,17 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff public void TiffDecoder_CanDecode_PackBitsCompressed(TestImageProvider provider) where TPixel : unmanaged, IPixel => TestTiffDecoder(provider); + [Theory] + [WithFile(MultiFrameMipMap, PixelTypes.Rgba32)] + public void CanDecodeJustOneFrame(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using (Image image = provider.GetImage(new TiffDecoder() { DecodingMode = FrameDecodingMode.First })) + { + Assert.Equal(1, image.Frames.Count); + } + } + [Theory] [WithFile(RgbJpegCompressed, PixelTypes.Rgba32)] [WithFile(RgbWithStripsJpegCompressed, PixelTypes.Rgba32)] diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 8b943194a..d7ac9c364 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -867,6 +867,7 @@ namespace SixLabors.ImageSharp.Tests public const string MultiframeDeflateWithPreview = "Tiff/multipage_deflate_withPreview.tiff"; public const string MultiframeDifferentSize = "Tiff/multipage_differentSize.tiff"; public const string MultiframeDifferentVariants = "Tiff/multipage_differentVariants.tiff"; + public const string MultiFrameMipMap = "Tiff/SKC1H3.tiff"; public const string LittleEndianByteOrder = "Tiff/little_endian.tiff"; diff --git a/tests/Images/Input/Tiff/SKC1H3.tiff b/tests/Images/Input/Tiff/SKC1H3.tiff new file mode 100644 index 000000000..9f9a50fdd --- /dev/null +++ b/tests/Images/Input/Tiff/SKC1H3.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:938bbf1c0f8bdbea0c632bb8d51c1150f757f88b3779d7fa18c296a3a3f61e9b +size 13720193 From 8db482834d6b158f944b08f2f64bc20ba205edee Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 19 Feb 2022 02:38:01 +1100 Subject: [PATCH 05/49] Handle empty XMP profiles. Fix #2012 --- .../Common/Extensions/StreamExtensions.cs | 2 +- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 10 ++- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 4 +- src/ImageSharp/Formats/Gif/LzwDecoder.cs | 2 +- .../Sections/GifXmpApplicationExtension.cs | 72 +++++++++++-------- src/ImageSharp/Metadata/ImageMetadata.cs | 3 +- .../Formats/Gif/GifDecoderTests.cs | 12 ++++ .../Formats/WebP/WebpMetaDataTests.cs | 3 +- .../Metadata/Profiles/XMP/XmpProfileTests.cs | 13 ++-- tests/ImageSharp.Tests/TestImages.cs | 1 + ...2012_Stronghold-Crusader-Extreme-Cover.png | 3 + ...2012_Stronghold-Crusader-Extreme-Cover.gif | 3 + 12 files changed, 82 insertions(+), 46 deletions(-) create mode 100644 tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012EmptyXmp_Rgba32_issue2012_Stronghold-Crusader-Extreme-Cover.png create mode 100644 tests/Images/Input/Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif diff --git a/src/ImageSharp/Common/Extensions/StreamExtensions.cs b/src/ImageSharp/Common/Extensions/StreamExtensions.cs index 1193eccee..8746989b3 100644 --- a/src/ImageSharp/Common/Extensions/StreamExtensions.cs +++ b/src/ImageSharp/Common/Extensions/StreamExtensions.cs @@ -13,7 +13,7 @@ namespace SixLabors.ImageSharp internal static class StreamExtensions { /// - /// Writes data from a stream into the provided buffer. + /// Writes data from a stream from the provided buffer. /// /// The stream. /// The buffer. diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 72e3ed144..0a5eba789 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -265,10 +265,14 @@ namespace SixLabors.ImageSharp.Formats.Gif this.stream.Read(this.buffer, 0, GifConstants.ApplicationBlockSize); bool isXmp = this.buffer.AsSpan().StartsWith(GifConstants.XmpApplicationIdentificationBytes); - if (isXmp) + if (isXmp && !this.IgnoreMetadata) { - var extension = GifXmpApplicationExtension.Read(this.stream); - this.metadata.XmpProfile = new XmpProfile(extension.Data); + var extension = GifXmpApplicationExtension.Read(this.stream, this.MemoryAllocator); + if (extension.Data.Length > 0) + { + this.metadata.XmpProfile = new XmpProfile(extension.Data); + } + return; } else diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index a21b050a8..e8bb13612 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -123,7 +123,8 @@ namespace SixLabors.ImageSharp.Formats.Gif this.WriteComments(gifMetadata, stream); // Write application extensions. - this.WriteApplicationExtensions(stream, image.Frames.Count, gifMetadata.RepeatCount, metadata.XmpProfile); + XmpProfile xmpProfile = image.Metadata.XmpProfile ?? image.Frames.RootFrame.Metadata.XmpProfile; + this.WriteApplicationExtensions(stream, image.Frames.Count, gifMetadata.RepeatCount, xmpProfile); if (useGlobalTable) { @@ -137,7 +138,6 @@ namespace SixLabors.ImageSharp.Formats.Gif // Clean up. quantized.Dispose(); - // TODO: Write extension etc stream.WriteByte(GifConstants.EndIntroducer); } diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index 68227db53..1db05bb83 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -68,7 +68,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// The pixel array to decode to. public void DecodePixels(int dataSize, Buffer2D pixels) { - Guard.MustBeLessThan(dataSize, int.MaxValue, nameof(dataSize)); + Guard.MustBeLessThanOrEqualTo(1 << dataSize, MaxStackSize, nameof(dataSize)); // The resulting index table length. int width = pixels.Width; diff --git a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs index 236508fe9..00bf4b7a2 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs @@ -2,9 +2,9 @@ // Licensed under the Apache License, Version 2.0. using System; -using System.Collections.Generic; using System.IO; -using SixLabors.ImageSharp.Metadata.Profiles.Xmp; +using SixLabors.ImageSharp.IO; +using SixLabors.ImageSharp.Memory; namespace SixLabors.ImageSharp.Formats.Gif { @@ -14,7 +14,10 @@ namespace SixLabors.ImageSharp.Formats.Gif public byte Label => GifConstants.ApplicationExtensionLabel; - public int ContentLength => this.Data.Length + 269; // 12 + Data Length + 1 + 256 + // size : 1 + // identifier : 11 + // magic trailer : 257 + public int ContentLength => this.Data.Length + 269; /// /// Gets the raw Data. @@ -25,40 +28,23 @@ namespace SixLabors.ImageSharp.Formats.Gif /// Reads the XMP metadata from the specified stream. /// /// The stream to read from. + /// The memory allocator. /// The XMP metadata /// Thrown if the XMP block is not properly terminated. - public static GifXmpApplicationExtension Read(Stream stream) + public static GifXmpApplicationExtension Read(Stream stream, MemoryAllocator allocator) { - // Read data in blocks, until an \0 character is encountered. - // We overshoot, indicated by the terminatorIndex variable. - const int bufferSize = 256; - var list = new List(); - int terminationIndex = -1; - while (terminationIndex < 0) - { - byte[] temp = new byte[bufferSize]; - int bytesRead = stream.Read(temp); - list.Add(temp); - terminationIndex = Array.IndexOf(temp, (byte)1); - } + byte[] xmpBytes = ReadXmpData(stream, allocator); - // Pack all the blocks (except magic trailer) into one single array again. - int dataSize = ((list.Count - 1) * bufferSize) + terminationIndex; - byte[] buffer = new byte[dataSize]; - Span bufferSpan = buffer; - int pos = 0; - for (int j = 0; j < list.Count - 1; j++) + // Exclude the "magic trailer", see XMP Specification Part 3, 1.1.2 GIF + int xmpLength = xmpBytes.Length - 256; // 257 - unread 0x0 + byte[] buffer = Array.Empty(); + if (xmpLength > 0) { - list[j].CopyTo(bufferSpan.Slice(pos)); - pos += bufferSize; + buffer = new byte[xmpLength]; + xmpBytes.AsSpan(0, xmpLength).CopyTo(buffer); + stream.Skip(1); // Skip the terminator. } - // Last one only needs the portion until terminationIndex copied over. - Span lastBytes = list[list.Count - 1]; - lastBytes.Slice(0, terminationIndex).CopyTo(bufferSpan.Slice(pos)); - - // Skip the remainder of the magic trailer. - stream.Skip(258 - (bufferSize - terminationIndex)); return new GifXmpApplicationExtension(buffer); } @@ -67,7 +53,7 @@ namespace SixLabors.ImageSharp.Formats.Gif int totalSize = this.ContentLength; if (buffer.Length < totalSize) { - throw new InsufficientMemoryException("Unable to write XMP metadata to GIF image"); + ThrowInsufficientMemory(); } int bytesWritten = 0; @@ -93,5 +79,29 @@ namespace SixLabors.ImageSharp.Formats.Gif return totalSize; } + + private static byte[] ReadXmpData(Stream stream, MemoryAllocator allocator) + { + using ChunkedMemoryStream bytes = new(allocator); + + // XMP data doesn't have a fixed length nor is there an indicator of the length. + // So we simply read one byte at a time until we hit the 0x0 value at the end + // of the magic trailer or the end of the stream. + // Using ChunkedMemoryStream reduces the array resize allocation normally associated + // with writing from a non fixed-size buffer. + while (true) + { + int b = stream.ReadByte(); + if (b <= 0) + { + return bytes.ToArray(); + } + + bytes.WriteByte((byte)b); + } + } + + private static void ThrowInsufficientMemory() + => throw new InsufficientMemoryException("Unable to write XMP metadata to GIF image"); } } diff --git a/src/ImageSharp/Metadata/ImageMetadata.cs b/src/ImageSharp/Metadata/ImageMetadata.cs index 89592f776..4760fa141 100644 --- a/src/ImageSharp/Metadata/ImageMetadata.cs +++ b/src/ImageSharp/Metadata/ImageMetadata.cs @@ -68,6 +68,7 @@ namespace SixLabors.ImageSharp.Metadata this.ExifProfile = other.ExifProfile?.DeepClone(); this.IccProfile = other.IccProfile?.DeepClone(); this.IptcProfile = other.IptcProfile?.DeepClone(); + this.XmpProfile = other.XmpProfile?.DeepClone(); } /// @@ -175,7 +176,7 @@ namespace SixLabors.ImageSharp.Metadata } /// - public ImageMetadata DeepClone() => new ImageMetadata(this); + public ImageMetadata DeepClone() => new(this); /// /// Synchronizes the profiles with the current metadata. diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index c8ecdb717..7ca64bea7 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -259,5 +259,17 @@ namespace SixLabors.ImageSharp.Tests.Formats.Gif image.CompareFirstFrameToReferenceOutput(ImageComparer.Exact, provider); } + + // https://github.com/SixLabors/ImageSharp/issues/2012 + [Theory] + [WithFile(TestImages.Gif.Issues.Issue2012EmptyXmp, PixelTypes.Rgba32)] + public void Issue2012EmptyXmp(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(); + + image.DebugSave(provider); + image.CompareFirstFrameToReferenceOutput(ImageComparer.Exact, provider); + } } } diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs index 7fba86b4f..eaa7fb564 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. using System.IO; +using System.Threading.Tasks; using SixLabors.ImageSharp.Formats.Webp; using SixLabors.ImageSharp.Metadata.Profiles.Exif; using SixLabors.ImageSharp.PixelFormats; @@ -66,7 +67,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Webp [Theory] [WithFile(TestImages.Webp.Lossy.WithXmp, PixelTypes.Rgba32, false)] [WithFile(TestImages.Webp.Lossy.WithXmp, PixelTypes.Rgba32, true)] - public async void IgnoreMetadata_ControlsWhetherXmpIsParsed(TestImageProvider provider, bool ignoreMetadata) + public async Task IgnoreMetadata_ControlsWhetherXmpIsParsed(TestImageProvider provider, bool ignoreMetadata) where TPixel : unmanaged, IPixel { var decoder = new WebpDecoder { IgnoreMetadata = ignoreMetadata }; diff --git a/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs b/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs index 81dad699a..a3c15d0b3 100644 --- a/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs +++ b/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs @@ -4,6 +4,7 @@ using System; using System.IO; using System.Text; +using System.Threading.Tasks; using System.Xml.Linq; using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Gif; @@ -31,7 +32,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp [Theory] [WithFile(TestImages.Gif.Receipt, PixelTypes.Rgba32)] - public async void ReadXmpMetadata_FromGif_Works(TestImageProvider provider) + public async Task ReadXmpMetadata_FromGif_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { using (Image image = await provider.GetImageAsync(GifDecoder)) @@ -45,7 +46,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp [WithFile(TestImages.Jpeg.Baseline.Lake, PixelTypes.Rgba32)] [WithFile(TestImages.Jpeg.Baseline.Metadata, PixelTypes.Rgba32)] [WithFile(TestImages.Jpeg.Baseline.ExtendedXmp, PixelTypes.Rgba32)] - public async void ReadXmpMetadata_FromJpg_Works(TestImageProvider provider) + public async Task ReadXmpMetadata_FromJpg_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { using (Image image = await provider.GetImageAsync(JpegDecoder)) @@ -57,7 +58,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp [Theory] [WithFile(TestImages.Png.XmpColorPalette, PixelTypes.Rgba32)] - public async void ReadXmpMetadata_FromPng_Works(TestImageProvider provider) + public async Task ReadXmpMetadata_FromPng_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { using (Image image = await provider.GetImageAsync(PngDecoder)) @@ -69,7 +70,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp [Theory] [WithFile(TestImages.Tiff.SampleMetadata, PixelTypes.Rgba32)] - public async void ReadXmpMetadata_FromTiff_Works(TestImageProvider provider) + public async Task ReadXmpMetadata_FromTiff_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { using (Image image = await provider.GetImageAsync(TiffDecoder)) @@ -81,7 +82,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp [Theory] [WithFile(TestImages.Webp.Lossy.WithXmp, PixelTypes.Rgba32)] - public async void ReadXmpMetadata_FromWebp_Works(TestImageProvider provider) + public async Task ReadXmpMetadata_FromWebp_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { using (Image image = await provider.GetImageAsync(WebpDecoder)) @@ -157,7 +158,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp } [Fact] - public async void WritingJpeg_PreservesExtendedXmpProfile() + public async Task WritingJpeg_PreservesExtendedXmpProfile() { // arrange var provider = TestImageProvider.File(TestImages.Jpeg.Baseline.ExtendedXmp); diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 8b943194a..ffdd6f8f2 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -452,6 +452,7 @@ namespace SixLabors.ImageSharp.Tests public const string Issue1530 = "Gif/issues/issue1530.gif"; public const string InvalidColorIndex = "Gif/issues/issue1668_invalidcolorindex.gif"; public const string Issue1962NoColorTable = "Gif/issues/issue1962_tiniest_gif_1st.gif"; + public const string Issue2012EmptyXmp = "Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif"; } public static readonly string[] All = { Rings, Giphy, Cheers, Trans, Kumin, Leo, Ratio4x1, Ratio1x4 }; diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012EmptyXmp_Rgba32_issue2012_Stronghold-Crusader-Extreme-Cover.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012EmptyXmp_Rgba32_issue2012_Stronghold-Crusader-Extreme-Cover.png new file mode 100644 index 000000000..c646eb860 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012EmptyXmp_Rgba32_issue2012_Stronghold-Crusader-Extreme-Cover.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:3ba8295d8a4b087d6c19fbad7e97cef7b5ce1a69b9c4c4f79cee6bc77e41f236 +size 62778 diff --git a/tests/Images/Input/Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif b/tests/Images/Input/Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif new file mode 100644 index 000000000..90f0e0f1c --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:97f8fdbabfbd9663bf9940dc33f81edf330b62789d1aa573ae85a520903723e5 +size 77498 From c64078cc17369e92ea83df3ec8687e4d9c41daf3 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 19 Feb 2022 16:58:09 +1100 Subject: [PATCH 06/49] Remove unnecessary throw and optimize write. --- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 23 +++++++++++-------- .../Sections/GifGraphicControlExtension.cs | 6 ++--- .../GifNetscapeLoopingApplicationExtension.cs | 2 +- .../Sections/GifXmpApplicationExtension.cs | 13 ++--------- 4 files changed, 19 insertions(+), 25 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index e8bb13612..da5b1cb23 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -428,26 +428,31 @@ namespace SixLabors.ImageSharp.Formats.Gif where TGifExtension : struct, IGifExtension { IMemoryOwner owner = null; - Span buffer; + Span extensionBuffer; int extensionSize = extension.ContentLength; - if (extensionSize > this.buffer.Length - 3) + + if (extensionSize == 0) + { + return; + } + else if (extensionSize > this.buffer.Length - 3) { owner = this.memoryAllocator.Allocate(extensionSize + 3); - buffer = owner.GetSpan(); + extensionBuffer = owner.GetSpan(); } else { - buffer = this.buffer; + extensionBuffer = this.buffer; } - buffer[0] = GifConstants.ExtensionIntroducer; - buffer[1] = extension.Label; + extensionBuffer[0] = GifConstants.ExtensionIntroducer; + extensionBuffer[1] = extension.Label; - extension.WriteTo(buffer.Slice(2)); + extension.WriteTo(extensionBuffer.Slice(2)); - buffer[extensionSize + 2] = GifConstants.Terminator; + extensionBuffer[extensionSize + 2] = GifConstants.Terminator; - stream.Write(buffer, 0, extensionSize + 3); + stream.Write(extensionBuffer, 0, extensionSize + 3); owner?.Dispose(); } diff --git a/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs index 801849c9b..847633694 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs @@ -71,13 +71,11 @@ namespace SixLabors.ImageSharp.Formats.Gif dest = this; - return 5; + return ((IGifExtension)this).ContentLength; } public static GifGraphicControlExtension Parse(ReadOnlySpan buffer) - { - return MemoryMarshal.Cast(buffer)[0]; - } + => MemoryMarshal.Cast(buffer)[0]; public static byte GetPackedValue(GifDisposalMethod disposalMethod, bool userInputFlag = false, bool transparencyFlag = false) { diff --git a/src/ImageSharp/Formats/Gif/Sections/GifNetscapeLoopingApplicationExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifNetscapeLoopingApplicationExtension.cs index 2c7bed611..c9e8033db 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifNetscapeLoopingApplicationExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifNetscapeLoopingApplicationExtension.cs @@ -40,7 +40,7 @@ namespace SixLabors.ImageSharp.Formats.Gif // 0 means loop indefinitely. Count is set as play n + 1 times. BinaryPrimitives.WriteUInt16LittleEndian(buffer.Slice(14, 2), this.RepeatCount); - return 16; // Length - Introducer + Label + Terminator. + return this.ContentLength; // Length - Introducer + Label + Terminator. } } } diff --git a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs index 00bf4b7a2..8c396e7fb 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs @@ -17,7 +17,7 @@ namespace SixLabors.ImageSharp.Formats.Gif // size : 1 // identifier : 11 // magic trailer : 257 - public int ContentLength => this.Data.Length + 269; + public int ContentLength => (this.Data.Length > 0) ? this.Data.Length + 269 : 0; /// /// Gets the raw Data. @@ -50,12 +50,6 @@ namespace SixLabors.ImageSharp.Formats.Gif public int WriteTo(Span buffer) { - int totalSize = this.ContentLength; - if (buffer.Length < totalSize) - { - ThrowInsufficientMemory(); - } - int bytesWritten = 0; buffer[bytesWritten++] = GifConstants.ApplicationBlockSize; @@ -77,7 +71,7 @@ namespace SixLabors.ImageSharp.Formats.Gif buffer[bytesWritten++] = 0x00; - return totalSize; + return this.ContentLength; } private static byte[] ReadXmpData(Stream stream, MemoryAllocator allocator) @@ -100,8 +94,5 @@ namespace SixLabors.ImageSharp.Formats.Gif bytes.WriteByte((byte)b); } } - - private static void ThrowInsufficientMemory() - => throw new InsufficientMemoryException("Unable to write XMP metadata to GIF image"); } } From c129278cae336b997d1de0f90047326e527d9fe2 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 19 Feb 2022 21:10:15 +1100 Subject: [PATCH 07/49] Don't throw for bad min code, just don't decode indices. --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 2 +- src/ImageSharp/Formats/Gif/LzwDecoder.cs | 12 ++++++++---- .../ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs | 12 ++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + .../Issue2012BadMinCode_Rgba32_issue2012_drona1.png | 3 +++ tests/Images/Input/Gif/issues/issue2012_drona1.gif | 3 +++ 6 files changed, 28 insertions(+), 5 deletions(-) create mode 100644 tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png create mode 100644 tests/Images/Input/Gif/issues/issue2012_drona1.gif diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 0a5eba789..98e012658 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -378,8 +378,8 @@ namespace SixLabors.ImageSharp.Formats.Gif } indices = this.Configuration.MemoryAllocator.Allocate2D(this.imageDescriptor.Width, this.imageDescriptor.Height, AllocationOptions.Clean); - this.ReadFrameIndices(indices); + Span rawColorTable = default; if (localColorTable != null) { diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index 1db05bb83..8bb52b637 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -68,16 +68,20 @@ namespace SixLabors.ImageSharp.Formats.Gif /// The pixel array to decode to. public void DecodePixels(int dataSize, Buffer2D pixels) { - Guard.MustBeLessThanOrEqualTo(1 << dataSize, MaxStackSize, nameof(dataSize)); + // Calculate the clear code. The value of the clear code is 2 ^ dataSize + int clearCode = 1 << dataSize; + if (clearCode > MaxStackSize) + { + // Don't attempt to decode the frame indices. + // The image is most likely corrupted. + return; + } // The resulting index table length. int width = pixels.Width; int height = pixels.Height; int length = width * height; - // Calculate the clear code. The value of the clear code is 2 ^ dataSize - int clearCode = 1 << dataSize; - int codeSize = dataSize + 1; // Calculate the end code diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index 7ca64bea7..289bf6a4f 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -271,5 +271,17 @@ namespace SixLabors.ImageSharp.Tests.Formats.Gif image.DebugSave(provider); image.CompareFirstFrameToReferenceOutput(ImageComparer.Exact, provider); } + + // https://github.com/SixLabors/ImageSharp/issues/2012 + [Theory] + [WithFile(TestImages.Gif.Issues.Issue2012BadMinCode, PixelTypes.Rgba32)] + public void Issue2012BadMinCode(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(); + + image.DebugSave(provider); + image.CompareFirstFrameToReferenceOutput(ImageComparer.Exact, provider); + } } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index ffdd6f8f2..6ff116225 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -453,6 +453,7 @@ namespace SixLabors.ImageSharp.Tests public const string InvalidColorIndex = "Gif/issues/issue1668_invalidcolorindex.gif"; public const string Issue1962NoColorTable = "Gif/issues/issue1962_tiniest_gif_1st.gif"; public const string Issue2012EmptyXmp = "Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif"; + public const string Issue2012BadMinCode = "Gif/issues/issue2012_drona1.gif"; } public static readonly string[] All = { Rings, Giphy, Cheers, Trans, Kumin, Leo, Ratio4x1, Ratio1x4 }; diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png new file mode 100644 index 000000000..5e23f5528 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:cf4bc93479140b05b25066eb10c33fa9f82c617828a56526d47f5a8c72035ef0 +size 1871 diff --git a/tests/Images/Input/Gif/issues/issue2012_drona1.gif b/tests/Images/Input/Gif/issues/issue2012_drona1.gif new file mode 100644 index 000000000..803d68487 --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue2012_drona1.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:cbb23b2a19e314969c6da99374ae133d834d76c3f0ab9df4a7edc9334bb065e6 +size 10508 From a01ff4a6427f4ea25d1f132424cdff7525599357 Mon Sep 17 00:00:00 2001 From: Titus Date: Sat, 19 Feb 2022 10:14:46 -0500 Subject: [PATCH 08/49] Fix issue in PNG identify method to skip "uninteresting" chunks, including sBIT. --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index f5fc86ee4..a7f4d5e8d 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -336,6 +336,9 @@ namespace SixLabors.ImageSharp.Formats.Png break; case PngChunkType.End: goto EOF; + default: + this.SkipChunkDataAndCrc(chunk); + break; } } finally From 892dbe318f03b4b21ecbf71d03989b2985e0db54 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sat, 19 Feb 2022 18:12:14 +0100 Subject: [PATCH 09/49] Throw exception, if palette chunk is missing --- src/ImageSharp/Formats/Png/PngScanlineProcessor.cs | 5 +++++ src/ImageSharp/Formats/Png/PngThrowHelper.cs | 3 +++ 2 files changed, 8 insertions(+) diff --git a/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs b/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs index 58fa5aca8..26bc566d6 100644 --- a/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs +++ b/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs @@ -240,6 +240,11 @@ namespace SixLabors.ImageSharp.Formats.Png byte[] paletteAlpha) where TPixel : unmanaged, IPixel { + if (palette.IsEmpty) + { + PngThrowHelper.ThrowMissingPalette(); + } + TPixel pixel = default; ref byte scanlineSpanRef = ref MemoryMarshal.GetReference(scanlineSpan); ref TPixel rowSpanRef = ref MemoryMarshal.GetReference(rowSpan); diff --git a/src/ImageSharp/Formats/Png/PngThrowHelper.cs b/src/ImageSharp/Formats/Png/PngThrowHelper.cs index 8700438bd..ad733f010 100644 --- a/src/ImageSharp/Formats/Png/PngThrowHelper.cs +++ b/src/ImageSharp/Formats/Png/PngThrowHelper.cs @@ -21,6 +21,9 @@ namespace SixLabors.ImageSharp.Formats.Png [MethodImpl(InliningOptions.ColdPath)] public static void ThrowNoData() => throw new InvalidImageContentException("PNG Image does not contain a data chunk"); + [MethodImpl(InliningOptions.ColdPath)] + public static void ThrowMissingPalette() => throw new InvalidImageContentException("PNG Image does not contain palette chunk"); + [MethodImpl(InliningOptions.ColdPath)] public static void ThrowInvalidChunkType() => throw new InvalidImageContentException("Invalid PNG data."); From d98171fa54180dec22db9e0e207e803edc4b6c8a Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sat, 19 Feb 2022 18:23:45 +0100 Subject: [PATCH 10/49] Add tests for missing palette chunk --- .../Formats/Png/PngDecoderTests.cs | 16 ++++++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 2 ++ tests/Images/Input/Png/missing_plte.png | 3 +++ tests/Images/Input/Png/missing_plte_2.png | 3 +++ 4 files changed, 24 insertions(+) create mode 100644 tests/Images/Input/Png/missing_plte.png create mode 100644 tests/Images/Input/Png/missing_plte_2.png diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index c29f8c589..f81a77ca1 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -265,6 +265,22 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png Assert.Contains("PNG Image does not contain a data chunk", ex.Message); } + [Theory] + [WithFile(TestImages.Png.Bad.MissingPaletteChunk1, PixelTypes.Rgba32)] + [WithFile(TestImages.Png.Bad.MissingPaletteChunk2, PixelTypes.Rgba32)] + public void Decode_MissingPaletteChunk_ThrowsException(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + Exception ex = Record.Exception( + () => + { + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + }); + Assert.NotNull(ex); + Assert.Contains("PNG Image does not contain palette chunk", ex.Message); + } + [Theory] [WithFile(TestImages.Png.Bad.BitDepthZero, PixelTypes.Rgba32)] [WithFile(TestImages.Png.Bad.BitDepthThree, PixelTypes.Rgba32)] diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 8b943194a..d03a60183 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -127,6 +127,8 @@ namespace SixLabors.ImageSharp.Tests public const string MissingDataChunk = "Png/xdtn0g01.png"; public const string WrongCrcDataChunk = "Png/xcsn0g01.png"; public const string CorruptedChunk = "Png/big-corrupted-chunk.png"; + public const string MissingPaletteChunk1 = "Png/missing_plte.png"; + public const string MissingPaletteChunk2 = "Png/missing_plte_2.png"; // Zlib errors. public const string ZlibOverflow = "Png/zlib-overflow.png"; diff --git a/tests/Images/Input/Png/missing_plte.png b/tests/Images/Input/Png/missing_plte.png new file mode 100644 index 000000000..0c24883fb --- /dev/null +++ b/tests/Images/Input/Png/missing_plte.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:73fd17a394f8258f4767986bc427c0160277819349c937f18cb29044e7549bc8 +size 506 diff --git a/tests/Images/Input/Png/missing_plte_2.png b/tests/Images/Input/Png/missing_plte_2.png new file mode 100644 index 000000000..8fc6580e5 --- /dev/null +++ b/tests/Images/Input/Png/missing_plte_2.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:797844db61a937c6f31ecb392c8416fbf106017413ba55c6576e0b1fcfc1cf9c +size 597 From b0b7ae641be7123c25a5e75d9ac5937643fc45f4 Mon Sep 17 00:00:00 2001 From: Titus Date: Sat, 19 Feb 2022 13:53:04 -0500 Subject: [PATCH 11/49] Moved SkipChunkDataAndCrc call to "correct" location for this issue --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index a7f4d5e8d..8d0b49261 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -336,9 +336,6 @@ namespace SixLabors.ImageSharp.Formats.Png break; case PngChunkType.End: goto EOF; - default: - this.SkipChunkDataAndCrc(chunk); - break; } } finally @@ -1401,6 +1398,8 @@ namespace SixLabors.ImageSharp.Formats.Png { chunk = new PngChunk(length, type); + this.SkipChunkDataAndCrc(chunk); + return true; } From d4180a92c5b691cf082caba03b30de3e2c2262b4 Mon Sep 17 00:00:00 2001 From: Titus Date: Sat, 19 Feb 2022 13:59:20 -0500 Subject: [PATCH 12/49] cleaned up whitespace --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 8d0b49261..56ad4ad86 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -336,6 +336,14 @@ namespace SixLabors.ImageSharp.Formats.Png break; case PngChunkType.End: goto EOF; + + default: + if (this.colorMetadataOnly) + { + this.SkipChunkDataAndCrc(chunk); + } + + break; } } finally @@ -1398,8 +1406,6 @@ namespace SixLabors.ImageSharp.Formats.Png { chunk = new PngChunk(length, type); - this.SkipChunkDataAndCrc(chunk); - return true; } From 29ddc6053e1a27ab095e87d8baa015f7428c53ef Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sat, 19 Feb 2022 20:31:07 +0100 Subject: [PATCH 13/49] Change error message to "...a palette chunk" --- src/ImageSharp/Formats/Png/PngThrowHelper.cs | 2 +- tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngThrowHelper.cs b/src/ImageSharp/Formats/Png/PngThrowHelper.cs index ad733f010..ae7d16ec7 100644 --- a/src/ImageSharp/Formats/Png/PngThrowHelper.cs +++ b/src/ImageSharp/Formats/Png/PngThrowHelper.cs @@ -22,7 +22,7 @@ namespace SixLabors.ImageSharp.Formats.Png public static void ThrowNoData() => throw new InvalidImageContentException("PNG Image does not contain a data chunk"); [MethodImpl(InliningOptions.ColdPath)] - public static void ThrowMissingPalette() => throw new InvalidImageContentException("PNG Image does not contain palette chunk"); + public static void ThrowMissingPalette() => throw new InvalidImageContentException("PNG Image does not contain a palette chunk"); [MethodImpl(InliningOptions.ColdPath)] public static void ThrowInvalidChunkType() => throw new InvalidImageContentException("Invalid PNG data."); diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index f81a77ca1..a92856c73 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -278,7 +278,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png image.DebugSave(provider); }); Assert.NotNull(ex); - Assert.Contains("PNG Image does not contain palette chunk", ex.Message); + Assert.Contains("PNG Image does not contain a palette chunk", ex.Message); } [Theory] From 78e5b6c19d45b3083b8160cde0b90cd072bead06 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 21 Feb 2022 18:39:29 +1100 Subject: [PATCH 14/49] Throw for corrupt LZW min code. Add test for deferred clear code --- ImageSharp.sln | 7 +++++ src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 4 +-- src/ImageSharp/Formats/Gif/LzwDecoder.cs | 26 ++++++++++++------- .../Formats/Gif/GifDecoderTests.cs | 17 ++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + ...2012BadMinCode_Rgba32_issue2012_drona1.png | 3 --- ...eferredClearCode_Rgba32_bugzilla-55918.png | 3 +++ .../Input/Gif/issues/bugzilla-55918.gif | 3 +++ 8 files changed, 50 insertions(+), 14 deletions(-) delete mode 100644 tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png create mode 100644 tests/Images/External/ReferenceOutput/GifDecoderTests/IssueDeferredClearCode_Rgba32_bugzilla-55918.png create mode 100644 tests/Images/Input/Gif/issues/bugzilla-55918.gif diff --git a/ImageSharp.sln b/ImageSharp.sln index 17d293b43..5428f3394 100644 --- a/ImageSharp.sln +++ b/ImageSharp.sln @@ -142,6 +142,13 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Gif", "Gif", "{EE3FB0B3-1C3 EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "issues", "issues", "{BF8DFDC1-CEE5-4A37-B216-D3085360C776}" ProjectSection(SolutionItems) = preProject + tests\Images\Input\Gif\issues\bugzilla-55918.gif = tests\Images\Input\Gif\issues\bugzilla-55918.gif + tests\Images\Input\Gif\issues\issue1505_argumentoutofrange.png = tests\Images\Input\Gif\issues\issue1505_argumentoutofrange.png + tests\Images\Input\Gif\issues\issue1530.gif = tests\Images\Input\Gif\issues\issue1530.gif + tests\Images\Input\Gif\issues\issue1668_invalidcolorindex.gif = tests\Images\Input\Gif\issues\issue1668_invalidcolorindex.gif + tests\Images\Input\Gif\issues\issue1962_tiniest_gif_1st.gif = tests\Images\Input\Gif\issues\issue1962_tiniest_gif_1st.gif + tests\Images\Input\Gif\issues\issue2012_drona1.gif = tests\Images\Input\Gif\issues\issue2012_drona1.gif + tests\Images\Input\Gif\issues\issue2012_Stronghold-Crusader-Extreme-Cover.gif = tests\Images\Input\Gif\issues\issue2012_Stronghold-Crusader-Extreme-Cover.gif tests\Images\Input\Gif\issues\issue403_baddescriptorwidth.gif = tests\Images\Input\Gif\issues\issue403_baddescriptorwidth.gif tests\Images\Input\Gif\issues\issue405_badappextlength252-2.gif = tests\Images\Input\Gif\issues\issue405_badappextlength252-2.gif tests\Images\Input\Gif\issues\issue405_badappextlength252.gif = tests\Images\Input\Gif\issues\issue405_badappextlength252.gif diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 98e012658..16dca3324 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -410,9 +410,9 @@ namespace SixLabors.ImageSharp.Formats.Gif [MethodImpl(MethodImplOptions.AggressiveInlining)] private void ReadFrameIndices(Buffer2D indices) { - int dataSize = this.stream.ReadByte(); + int minCodeSize = this.stream.ReadByte(); using var lzwDecoder = new LzwDecoder(this.Configuration.MemoryAllocator, this.stream); - lzwDecoder.DecodePixels(dataSize, indices); + lzwDecoder.DecodePixels(minCodeSize, indices); } /// diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index 8bb52b637..470929c4a 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -64,17 +64,22 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// Decodes and decompresses all pixel indices from the stream. /// - /// Size of the data. + /// Minimum code size of the data. /// The pixel array to decode to. - public void DecodePixels(int dataSize, Buffer2D pixels) + public void DecodePixels(int minCodeSize, Buffer2D pixels) { - // Calculate the clear code. The value of the clear code is 2 ^ dataSize - int clearCode = 1 << dataSize; - if (clearCode > MaxStackSize) + // Calculate the clear code. The value of the clear code is 2 ^ minCodeSize + int clearCode = 1 << minCodeSize; + + // It is possible to specify a larger LZW minimum code size than the palette length in bits + // which may leave a gap in the codes where no colors are assigned. + // http://www.matthewflickinger.com/lab/whatsinagif/lzw_image_data.asp#lzw_compression + if (minCodeSize < 2 || clearCode > MaxStackSize) { // Don't attempt to decode the frame indices. - // The image is most likely corrupted. - return; + // Theoretically we could determine a min code size from the length of the provided + // color palette but we won't bother since the image is most likely corrupted. + ThrowBadMinimumCode(); } // The resulting index table length. @@ -82,7 +87,7 @@ namespace SixLabors.ImageSharp.Formats.Gif int height = pixels.Height; int length = width * height; - int codeSize = dataSize + 1; + int codeSize = minCodeSize + 1; // Calculate the end code int endCode = clearCode + 1; @@ -169,7 +174,7 @@ namespace SixLabors.ImageSharp.Formats.Gif if (code == clearCode) { // Reset the decoder - codeSize = dataSize + 1; + codeSize = minCodeSize + 1; codeMask = (1 << codeSize) - 1; availableCode = clearCode + 2; oldCode = NullCode; @@ -258,5 +263,8 @@ namespace SixLabors.ImageSharp.Formats.Gif this.suffix.Dispose(); this.pixelStack.Dispose(); } + + [MethodImpl(InliningOptions.ColdPath)] + public static void ThrowBadMinimumCode() => throw new InvalidImageContentException("Gif Image does not contain a valid LZW minimum code."); } } diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index 289bf6a4f..30bcb7255 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -277,6 +277,23 @@ namespace SixLabors.ImageSharp.Tests.Formats.Gif [WithFile(TestImages.Gif.Issues.Issue2012BadMinCode, PixelTypes.Rgba32)] public void Issue2012BadMinCode(TestImageProvider provider) where TPixel : unmanaged, IPixel + { + Exception ex = Record.Exception( + () => + { + using Image image = provider.GetImage(); + image.DebugSave(provider); + }); + + Assert.NotNull(ex); + Assert.Contains("Gif Image does not contain a valid LZW minimum code.", ex.Message); + } + + // https://bugzilla.mozilla.org/show_bug.cgi?id=55918 + [Theory] + [WithFile(TestImages.Gif.Issues.DeferredClearCode, PixelTypes.Rgba32)] + public void IssueDeferredClearCode(TestImageProvider provider) + where TPixel : unmanaged, IPixel { using Image image = provider.GetImage(); diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 6ff116225..6fbf584d8 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -448,6 +448,7 @@ namespace SixLabors.ImageSharp.Tests public const string BadAppExtLength = "Gif/issues/issue405_badappextlength252.gif"; public const string BadAppExtLength_2 = "Gif/issues/issue405_badappextlength252-2.gif"; public const string BadDescriptorWidth = "Gif/issues/issue403_baddescriptorwidth.gif"; + public const string DeferredClearCode = "Gif/issues/bugzilla-55918.gif"; public const string Issue1505 = "Gif/issues/issue1505_argumentoutofrange.png"; public const string Issue1530 = "Gif/issues/issue1530.gif"; public const string InvalidColorIndex = "Gif/issues/issue1668_invalidcolorindex.gif"; diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png deleted file mode 100644 index 5e23f5528..000000000 --- a/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:cf4bc93479140b05b25066eb10c33fa9f82c617828a56526d47f5a8c72035ef0 -size 1871 diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueDeferredClearCode_Rgba32_bugzilla-55918.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueDeferredClearCode_Rgba32_bugzilla-55918.png new file mode 100644 index 000000000..b5769c2c4 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueDeferredClearCode_Rgba32_bugzilla-55918.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:6b33733518b855b25c5e9a1b2f5c93cacf0699a40a459dde795b0ed91a978909 +size 12776 diff --git a/tests/Images/Input/Gif/issues/bugzilla-55918.gif b/tests/Images/Input/Gif/issues/bugzilla-55918.gif new file mode 100644 index 000000000..929ea67c3 --- /dev/null +++ b/tests/Images/Input/Gif/issues/bugzilla-55918.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d11148669a093c2e39be62bc3482c5863362d28c03c7f26c5a2386d5de28c339 +size 14551 From 85a0ac644d1e51c87b510428c882e51b26aad3e4 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 21 Feb 2022 21:29:13 +1100 Subject: [PATCH 15/49] Use GifThrowHelper --- src/ImageSharp/Formats/Gif/LzwDecoder.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index 470929c4a..2a0720001 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -79,7 +79,7 @@ namespace SixLabors.ImageSharp.Formats.Gif // Don't attempt to decode the frame indices. // Theoretically we could determine a min code size from the length of the provided // color palette but we won't bother since the image is most likely corrupted. - ThrowBadMinimumCode(); + GifThrowHelper.ThrowInvalidImageContentException("Gif Image does not contain a valid LZW minimum code."); } // The resulting index table length. @@ -263,8 +263,5 @@ namespace SixLabors.ImageSharp.Formats.Gif this.suffix.Dispose(); this.pixelStack.Dispose(); } - - [MethodImpl(InliningOptions.ColdPath)] - public static void ThrowBadMinimumCode() => throw new InvalidImageContentException("Gif Image does not contain a valid LZW minimum code."); } } From 0fa6085611e69e5e4eb2827e8293eb747585a034 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Mon, 21 Feb 2022 12:10:23 +0100 Subject: [PATCH 16/49] Throw exception, if gamma chunk does not contain enough data --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 10 ++++++++-- src/ImageSharp/Formats/Png/PngThrowHelper.cs | 3 +++ .../Formats/Png/PngDecoderTests.cs | 15 +++++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Png/length_gama.png | 3 +++ 5 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 tests/Images/Input/Png/length_gama.png diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index f5fc86ee4..090e1e2b0 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -429,10 +429,16 @@ namespace SixLabors.ImageSharp.Formats.Png /// The metadata to read to. /// The data containing physical data. private void ReadGammaChunk(PngMetadata pngMetadata, ReadOnlySpan data) + { + if (data.Length < 4) + { + PngThrowHelper.ThrowInvalidGamma(); + } - // The value is encoded as a 4-byte unsigned integer, representing gamma times 100000. // For example, a gamma of 1/2.2 would be stored as 45455. - => pngMetadata.Gamma = BinaryPrimitives.ReadUInt32BigEndian(data) * 1e-5F; + // The value is encoded as a 4-byte unsigned integer, representing gamma times 100000. + pngMetadata.Gamma = BinaryPrimitives.ReadUInt32BigEndian(data) * 1e-5F; + } /// /// Initializes the image and various buffers needed for processing diff --git a/src/ImageSharp/Formats/Png/PngThrowHelper.cs b/src/ImageSharp/Formats/Png/PngThrowHelper.cs index ae7d16ec7..07372dae2 100644 --- a/src/ImageSharp/Formats/Png/PngThrowHelper.cs +++ b/src/ImageSharp/Formats/Png/PngThrowHelper.cs @@ -24,6 +24,9 @@ namespace SixLabors.ImageSharp.Formats.Png [MethodImpl(InliningOptions.ColdPath)] public static void ThrowMissingPalette() => throw new InvalidImageContentException("PNG Image does not contain a palette chunk"); + [MethodImpl(InliningOptions.ColdPath)] + public static void ThrowInvalidGamma() => throw new InvalidImageContentException("PNG Image does not contain enough data for the gamma chunk"); + [MethodImpl(InliningOptions.ColdPath)] public static void ThrowInvalidChunkType() => throw new InvalidImageContentException("Invalid PNG data."); diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index a92856c73..7fd87258b 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -281,6 +281,21 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png Assert.Contains("PNG Image does not contain a palette chunk", ex.Message); } + [Theory] + [WithFile(TestImages.Png.Bad.InvalidGammaChunk, PixelTypes.Rgba32)] + public void Decode_InvalidGammaChunk_ThrowsException(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + Exception ex = Record.Exception( + () => + { + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + }); + Assert.NotNull(ex); + Assert.Contains("PNG Image does not contain enough data for the gamma chunk", ex.Message); + } + [Theory] [WithFile(TestImages.Png.Bad.BitDepthZero, PixelTypes.Rgba32)] [WithFile(TestImages.Png.Bad.BitDepthThree, PixelTypes.Rgba32)] diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index c6bd20a7d..5bce99ce1 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -129,6 +129,7 @@ namespace SixLabors.ImageSharp.Tests public const string CorruptedChunk = "Png/big-corrupted-chunk.png"; public const string MissingPaletteChunk1 = "Png/missing_plte.png"; public const string MissingPaletteChunk2 = "Png/missing_plte_2.png"; + public const string InvalidGammaChunk = "Png/length_gama.png"; // Zlib errors. public const string ZlibOverflow = "Png/zlib-overflow.png"; diff --git a/tests/Images/Input/Png/length_gama.png b/tests/Images/Input/Png/length_gama.png new file mode 100644 index 000000000..caf0fb01d --- /dev/null +++ b/tests/Images/Input/Png/length_gama.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:824766b34739727c722e88611d7b55401452c2970cd433f56e5f9f1b36d6950d +size 1285 From d76c40a43a85f3ff82bdbc05f577fe7236095939 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Mon, 21 Feb 2022 14:39:10 +0100 Subject: [PATCH 17/49] Ignore invalid gamma chunks --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 3 ++- src/ImageSharp/Formats/Png/PngThrowHelper.cs | 3 --- tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs | 5 ++--- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 090e1e2b0..60437e015 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -432,7 +432,8 @@ namespace SixLabors.ImageSharp.Formats.Png { if (data.Length < 4) { - PngThrowHelper.ThrowInvalidGamma(); + // Ignore invalid gamma chunks. + return; } // For example, a gamma of 1/2.2 would be stored as 45455. diff --git a/src/ImageSharp/Formats/Png/PngThrowHelper.cs b/src/ImageSharp/Formats/Png/PngThrowHelper.cs index 07372dae2..ae7d16ec7 100644 --- a/src/ImageSharp/Formats/Png/PngThrowHelper.cs +++ b/src/ImageSharp/Formats/Png/PngThrowHelper.cs @@ -24,9 +24,6 @@ namespace SixLabors.ImageSharp.Formats.Png [MethodImpl(InliningOptions.ColdPath)] public static void ThrowMissingPalette() => throw new InvalidImageContentException("PNG Image does not contain a palette chunk"); - [MethodImpl(InliningOptions.ColdPath)] - public static void ThrowInvalidGamma() => throw new InvalidImageContentException("PNG Image does not contain enough data for the gamma chunk"); - [MethodImpl(InliningOptions.ColdPath)] public static void ThrowInvalidChunkType() => throw new InvalidImageContentException("Invalid PNG data."); diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 7fd87258b..0af5d9995 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -283,7 +283,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png [Theory] [WithFile(TestImages.Png.Bad.InvalidGammaChunk, PixelTypes.Rgba32)] - public void Decode_InvalidGammaChunk_ThrowsException(TestImageProvider provider) + public void Decode_InvalidGammaChunk_Ignored(TestImageProvider provider) where TPixel : unmanaged, IPixel { Exception ex = Record.Exception( @@ -292,8 +292,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png using Image image = provider.GetImage(PngDecoder); image.DebugSave(provider); }); - Assert.NotNull(ex); - Assert.Contains("PNG Image does not contain enough data for the gamma chunk", ex.Message); + Assert.Null(ex); } [Theory] From 27fc3b01c6678ed5c96e5c137250028b779e449c Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Mon, 21 Feb 2022 22:36:47 +0100 Subject: [PATCH 18/49] Add AVX2 version of adler --- src/ImageSharp/Compression/Zlib/Adler32.cs | 155 ++++++++++++++++----- 1 file changed, 121 insertions(+), 34 deletions(-) diff --git a/src/ImageSharp/Compression/Zlib/Adler32.cs b/src/ImageSharp/Compression/Zlib/Adler32.cs index 7eb3f4516..1f3b7e2a2 100644 --- a/src/ImageSharp/Compression/Zlib/Adler32.cs +++ b/src/ImageSharp/Compression/Zlib/Adler32.cs @@ -31,6 +31,8 @@ namespace SixLabors.ImageSharp.Compression.Zlib #if SUPPORTS_RUNTIME_INTRINSICS private const int MinBufferSize = 64; + private const int BLOCK_SIZE = 1 << 5; + // The C# compiler emits this as a compile-time constant embedded in the PE file. private static ReadOnlySpan Tap1Tap2 => new byte[] { @@ -63,6 +65,11 @@ namespace SixLabors.ImageSharp.Compression.Zlib } #if SUPPORTS_RUNTIME_INTRINSICS + if (Avx2.IsSupported && buffer.Length >= MinBufferSize) + { + return CalculateAvx2(adler, buffer); + } + if (Ssse3.IsSupported && buffer.Length >= MinBufferSize) { return CalculateSse(adler, buffer); @@ -83,8 +90,6 @@ namespace SixLabors.ImageSharp.Compression.Zlib uint s2 = (adler >> 16) & 0xFFFF; // Process the data in blocks. - const int BLOCK_SIZE = 1 << 5; - uint length = (uint)buffer.Length; uint blocks = length / BLOCK_SIZE; length -= blocks * BLOCK_SIZE; @@ -164,45 +169,127 @@ namespace SixLabors.ImageSharp.Compression.Zlib if (length > 0) { - if (length >= 16) - { - s2 += s1 += localBufferPtr[0]; - s2 += s1 += localBufferPtr[1]; - s2 += s1 += localBufferPtr[2]; - s2 += s1 += localBufferPtr[3]; - s2 += s1 += localBufferPtr[4]; - s2 += s1 += localBufferPtr[5]; - s2 += s1 += localBufferPtr[6]; - s2 += s1 += localBufferPtr[7]; - s2 += s1 += localBufferPtr[8]; - s2 += s1 += localBufferPtr[9]; - s2 += s1 += localBufferPtr[10]; - s2 += s1 += localBufferPtr[11]; - s2 += s1 += localBufferPtr[12]; - s2 += s1 += localBufferPtr[13]; - s2 += s1 += localBufferPtr[14]; - s2 += s1 += localBufferPtr[15]; - - localBufferPtr += 16; - length -= 16; - } + HandleLeftOver(localBufferPtr, length, ref s1, ref s2); + } - while (length-- > 0) - { - s2 += s1 += *localBufferPtr++; - } + return s1 | (s2 << 16); + } + } + } - if (s1 >= BASE) - { - s1 -= BASE; - } + // Based on: https://github.com/zlib-ng/zlib-ng/blob/develop/arch/x86/adler32_avx2.c + [MethodImpl(InliningOptions.HotPath | InliningOptions.ShortMethod)] + public static unsafe uint CalculateAvx2(uint adler, ReadOnlySpan buffer) + { + uint s1 = adler & 0xFFFF; + uint s2 = (adler >> 16) & 0xFFFF; + uint length = (uint)buffer.Length; - s2 %= BASE; + fixed (byte* bufferPtr = buffer) + { + byte* localBufferPtr = bufferPtr; + + Vector256 zero = Vector256.Zero; + var dot3v = Vector256.Create((short)1); + var dot2v = Vector256.Create(32, 31, 30, 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1); + + // Process n blocks of data. At most NMAX data bytes can be + // processed before s2 must be reduced modulo BASE. + var vs1 = Vector256.CreateScalar(s1); + var vs2 = Vector256.CreateScalar(s2); + + while (length >= 32) + { + int k = length < NMAX ? (int)length : (int)NMAX; + k -= k % 32; + length -= (uint)k; + + Vector256 vs10 = vs1; + Vector256 vs3 = Vector256.Zero; + + while (k >= 32) + { + // Load 32 input bytes. + Vector256 block = Avx.LoadVector256(localBufferPtr); + + // Sum of abs diff, resulting in 2 x int32's + Vector256 vs1sad = Avx2.SumAbsoluteDifferences(block, zero); + + vs1 = Avx2.Add(vs1, vs1sad.AsUInt32()); + vs3 = Avx2.Add(vs3, vs10); + + // sum 32 uint8s to 16 shorts. + Vector256 vshortsum2 = Avx2.MultiplyAddAdjacent(block, dot2v); + + // sum 16 shorts to 8 uint32s. + Vector256 vsum2 = Avx2.MultiplyAddAdjacent(vshortsum2, dot3v); + + vs2 = Avx2.Add(vsum2.AsUInt32(), vs2); + vs10 = vs1; + + localBufferPtr += BLOCK_SIZE; + k -= 32; } - return s1 | (s2 << 16); + // Defer the multiplication with 32 to outside of the loop. + vs3 = Avx2.ShiftLeftLogical(vs3, 5); + vs2 = Avx2.Add(vs2, vs3); + + s1 = (uint)Numerics.EvenReduceSum(vs1.AsInt32()); + s2 = (uint)Numerics.ReduceSum(vs2.AsInt32()); + + s1 %= BASE; + s2 %= BASE; + + vs1 = Vector256.CreateScalar(s1); + vs2 = Vector256.CreateScalar(s2); } + + if (length > 0) + { + HandleLeftOver(localBufferPtr, length, ref s1, ref s2); + } + + return s1 | (s2 << 16); + } + } + + private static unsafe void HandleLeftOver(byte* localBufferPtr, uint length, ref uint s1, ref uint s2) + { + if (length >= 16) + { + s2 += s1 += localBufferPtr[0]; + s2 += s1 += localBufferPtr[1]; + s2 += s1 += localBufferPtr[2]; + s2 += s1 += localBufferPtr[3]; + s2 += s1 += localBufferPtr[4]; + s2 += s1 += localBufferPtr[5]; + s2 += s1 += localBufferPtr[6]; + s2 += s1 += localBufferPtr[7]; + s2 += s1 += localBufferPtr[8]; + s2 += s1 += localBufferPtr[9]; + s2 += s1 += localBufferPtr[10]; + s2 += s1 += localBufferPtr[11]; + s2 += s1 += localBufferPtr[12]; + s2 += s1 += localBufferPtr[13]; + s2 += s1 += localBufferPtr[14]; + s2 += s1 += localBufferPtr[15]; + + localBufferPtr += 16; + length -= 16; } + + while (length-- > 0) + { + s2 += s1 += *localBufferPtr++; + } + + if (s1 >= BASE) + { + s1 -= BASE; + } + + s2 %= BASE; } #endif From cbf46759194b228660535b8d21122420337e88ed Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 20 Feb 2022 14:23:57 +0100 Subject: [PATCH 19/49] Add adler tests with and without intrinsics --- .../Formats/Png/Adler32Tests.cs | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Png/Adler32Tests.cs b/tests/ImageSharp.Tests/Formats/Png/Adler32Tests.cs index 0886bd84d..97d9e904e 100644 --- a/tests/ImageSharp.Tests/Formats/Png/Adler32Tests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/Adler32Tests.cs @@ -3,6 +3,7 @@ using System; using SixLabors.ImageSharp.Compression.Zlib; +using SixLabors.ImageSharp.Tests.TestUtilities; using Xunit; using SharpAdler32 = ICSharpCode.SharpZipLib.Checksum.Adler32; @@ -15,10 +16,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png [InlineData(0)] [InlineData(1)] [InlineData(2)] - public void ReturnsCorrectWhenEmpty(uint input) - { - Assert.Equal(input, Adler32.Calculate(input, default)); - } + public void CalculateAdler_ReturnsCorrectWhenEmpty(uint input) => Assert.Equal(input, Adler32.Calculate(input, default)); [Theory] [InlineData(0)] @@ -28,24 +26,46 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png [InlineData(1024 + 15)] [InlineData(2034)] [InlineData(4096)] - public void MatchesReference(int length) + public void CalculateAdler_MatchesReference(int length) => CalculateAdlerAndCompareToReference(length); + + private static void CalculateAdlerAndCompareToReference(int length) { - var data = GetBuffer(length); + // arrange + byte[] data = GetBuffer(length); var adler = new SharpAdler32(); adler.Update(data); - long expected = adler.Value; + + // act long actual = Adler32.Calculate(data); + // assert Assert.Equal(expected, actual); } private static byte[] GetBuffer(int length) { - var data = new byte[length]; + byte[] data = new byte[length]; new Random(1).NextBytes(data); return data; } + +#if SUPPORTS_RUNTIME_INTRINSICS + [Fact] + public void RunCalculateAdlerTest_WithHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunCalculateAdlerTest, HwIntrinsics.AllowAll); + + [Fact] + public void RunCalculateAdlerTest_WithoutHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunCalculateAdlerTest, HwIntrinsics.DisableHWIntrinsic); + + private static void RunCalculateAdlerTest() + { + int[] testData = { 0, 8, 215, 1024, 1024 + 15, 2034, 4096 }; + for (int i = 0; i < testData.Length; i++) + { + CalculateAdlerAndCompareToReference(testData[i]); + } + } +#endif } } From 6e6673fcb9d38586394cd56730bd1da879e1b9d4 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Mon, 21 Feb 2022 22:55:21 +0100 Subject: [PATCH 20/49] Add test with AVX disabled --- tests/ImageSharp.Tests/Formats/Png/Adler32Tests.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/ImageSharp.Tests/Formats/Png/Adler32Tests.cs b/tests/ImageSharp.Tests/Formats/Png/Adler32Tests.cs index 97d9e904e..77f2b7663 100644 --- a/tests/ImageSharp.Tests/Formats/Png/Adler32Tests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/Adler32Tests.cs @@ -55,6 +55,9 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png [Fact] public void RunCalculateAdlerTest_WithHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunCalculateAdlerTest, HwIntrinsics.AllowAll); + [Fact] + public void RunCalculateAdlerTest_WithAvxDisabled_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunCalculateAdlerTest, HwIntrinsics.AllowAll | HwIntrinsics.DisableAVX2); + [Fact] public void RunCalculateAdlerTest_WithoutHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunCalculateAdlerTest, HwIntrinsics.DisableHWIntrinsic); From d4fc063bbc1368d4162c02bdf0121f982959b47e Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Mon, 21 Feb 2022 22:57:20 +0100 Subject: [PATCH 21/49] Rename BLOCK_SIZE to BlockSize --- src/ImageSharp/Compression/Zlib/Adler32.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/ImageSharp/Compression/Zlib/Adler32.cs b/src/ImageSharp/Compression/Zlib/Adler32.cs index 1f3b7e2a2..b13e2e421 100644 --- a/src/ImageSharp/Compression/Zlib/Adler32.cs +++ b/src/ImageSharp/Compression/Zlib/Adler32.cs @@ -31,7 +31,7 @@ namespace SixLabors.ImageSharp.Compression.Zlib #if SUPPORTS_RUNTIME_INTRINSICS private const int MinBufferSize = 64; - private const int BLOCK_SIZE = 1 << 5; + private const int BlockSize = 1 << 5; // The C# compiler emits this as a compile-time constant embedded in the PE file. private static ReadOnlySpan Tap1Tap2 => new byte[] @@ -91,15 +91,15 @@ namespace SixLabors.ImageSharp.Compression.Zlib // Process the data in blocks. uint length = (uint)buffer.Length; - uint blocks = length / BLOCK_SIZE; - length -= blocks * BLOCK_SIZE; + uint blocks = length / BlockSize; + length -= blocks * BlockSize; int index = 0; fixed (byte* bufferPtr = buffer) { fixed (byte* tapPtr = Tap1Tap2) { - index += (int)blocks * BLOCK_SIZE; + index += (int)blocks * BlockSize; var localBufferPtr = bufferPtr; // _mm_setr_epi8 on x86 @@ -110,7 +110,7 @@ namespace SixLabors.ImageSharp.Compression.Zlib while (blocks > 0) { - uint n = NMAX / BLOCK_SIZE; /* The NMAX constraint. */ + uint n = NMAX / BlockSize; /* The NMAX constraint. */ if (n > blocks) { n = blocks; @@ -143,7 +143,7 @@ namespace SixLabors.ImageSharp.Compression.Zlib Vector128 mad2 = Ssse3.MultiplyAddAdjacent(bytes2, tap2); v_s2 = Sse2.Add(v_s2, Sse2.MultiplyAddAdjacent(mad2, ones).AsUInt32()); - localBufferPtr += BLOCK_SIZE; + localBufferPtr += BlockSize; } while (--n > 0); @@ -227,7 +227,7 @@ namespace SixLabors.ImageSharp.Compression.Zlib vs2 = Avx2.Add(vsum2.AsUInt32(), vs2); vs10 = vs1; - localBufferPtr += BLOCK_SIZE; + localBufferPtr += BlockSize; k -= 32; } From 854ea5d0962921a8f1831376f77b1bef8a57b75a Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Mon, 21 Feb 2022 23:25:22 +0100 Subject: [PATCH 22/49] workaround for #2001 / https://github.com/dotnet/runtime/issues/65466 --- .../UniformUnmanagedMemoryPoolMemoryAllocator.cs | 13 +++++++++++-- .../UniformUnmanagedPoolMemoryAllocatorTests.cs | 13 +++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index 16a3cb73d..5f591a9bb 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -74,6 +74,10 @@ namespace SixLabors.ImageSharp.Memory this.nonPoolAllocator = new UnmanagedMemoryAllocator(unmanagedBufferSizeInBytes); } + // This delegate allows overriding the method returning the available system memory, + // so we can test our workaround for https://github.com/dotnet/runtime/issues/65466 + internal static Func GetTotalAvailableMemoryBytes { get; set; } = () => GC.GetGCMemoryInfo().TotalAvailableMemoryBytes; + /// protected internal override int GetBufferCapacityInBytes() => this.poolBufferSizeInBytes; @@ -152,8 +156,13 @@ namespace SixLabors.ImageSharp.Memory // https://github.com/dotnet/runtime/issues/55126#issuecomment-876779327 if (Environment.Is64BitProcess || !RuntimeInformation.FrameworkDescription.StartsWith(".NET 5.0")) { - GCMemoryInfo info = GC.GetGCMemoryInfo(); - return info.TotalAvailableMemoryBytes / 8; + long total = GetTotalAvailableMemoryBytes(); + + // Workaround for https://github.com/dotnet/runtime/issues/65466 + if (total > 0) + { + return total / 8; + } } #endif diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index 45a7cc278..0c59e334c 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -379,5 +379,18 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators g1.GetSpan()[0] = 42; } } + + [Fact] + public void Issue2001_NegativeMemoryReportedByGc() + { + RemoteExecutor.Invoke(RunTest).Dispose(); + + static void RunTest() + { + // Emulate GC.GetGCMemoryInfo() issue https://github.com/dotnet/runtime/issues/65466 + UniformUnmanagedMemoryPoolMemoryAllocator.GetTotalAvailableMemoryBytes = () => -402354176; + _ = MemoryAllocator.Create(); + } + } } } From 5b82c57ce773566da96ef176c0fa1426e70c91fe Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 22 Feb 2022 12:42:36 +1100 Subject: [PATCH 23/49] Add compiler directives --- .../UniformUnmanagedMemoryPoolMemoryAllocator.cs | 7 ++++--- .../Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs | 2 ++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index 5f591a9bb..0da4ff9f8 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -1,11 +1,10 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. using System; using System.Buffers; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; -using System.Threading; using SixLabors.ImageSharp.Memory.Internals; namespace SixLabors.ImageSharp.Memory @@ -22,7 +21,7 @@ namespace SixLabors.ImageSharp.Memory private readonly int poolCapacity; private readonly UniformUnmanagedMemoryPool.TrimSettings trimSettings; - private UniformUnmanagedMemoryPool pool; + private readonly UniformUnmanagedMemoryPool pool; private readonly UnmanagedMemoryAllocator nonPoolAllocator; public UniformUnmanagedMemoryPoolMemoryAllocator(int? maxPoolSizeMegabytes) @@ -74,9 +73,11 @@ namespace SixLabors.ImageSharp.Memory this.nonPoolAllocator = new UnmanagedMemoryAllocator(unmanagedBufferSizeInBytes); } +#if NETCOREAPP3_1_OR_GREATER // This delegate allows overriding the method returning the available system memory, // so we can test our workaround for https://github.com/dotnet/runtime/issues/65466 internal static Func GetTotalAvailableMemoryBytes { get; set; } = () => GC.GetGCMemoryInfo().TotalAvailableMemoryBytes; +#endif /// protected internal override int GetBufferCapacityInBytes() => this.poolBufferSizeInBytes; diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index 0c59e334c..414f991f7 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -380,6 +380,7 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators } } +#if NETCOREAPP3_1_OR_GREATER [Fact] public void Issue2001_NegativeMemoryReportedByGc() { @@ -392,5 +393,6 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators _ = MemoryAllocator.Create(); } } +#endif } } From de5f661177f4c902a98fd091a406abea92e2ba36 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 22 Feb 2022 02:53:43 +0100 Subject: [PATCH 24/49] make GetTotalAvailableMemoryBytes conditional --- .../Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs | 2 ++ .../Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index 5f591a9bb..5b72a5804 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -74,9 +74,11 @@ namespace SixLabors.ImageSharp.Memory this.nonPoolAllocator = new UnmanagedMemoryAllocator(unmanagedBufferSizeInBytes); } +#if NETCOREAPP3_1_OR_GREATER // This delegate allows overriding the method returning the available system memory, // so we can test our workaround for https://github.com/dotnet/runtime/issues/65466 internal static Func GetTotalAvailableMemoryBytes { get; set; } = () => GC.GetGCMemoryInfo().TotalAvailableMemoryBytes; +#endif /// protected internal override int GetBufferCapacityInBytes() => this.poolBufferSizeInBytes; diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index 0c59e334c..414f991f7 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -380,6 +380,7 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators } } +#if NETCOREAPP3_1_OR_GREATER [Fact] public void Issue2001_NegativeMemoryReportedByGc() { @@ -392,5 +393,6 @@ namespace SixLabors.ImageSharp.Tests.Memory.Allocators _ = MemoryAllocator.Create(); } } +#endif } } From 747422cf6d0075f03b1d10b0d7b07a2063d7a0c9 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Tue, 22 Feb 2022 13:46:41 +0100 Subject: [PATCH 25/49] Add Sse2 version of Average png filter --- .../Formats/Png/Filters/AverageFilter.cs | 126 +++++++++++++++--- 1 file changed, 110 insertions(+), 16 deletions(-) diff --git a/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs b/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs index 83c638934..94c4fb4d1 100644 --- a/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs @@ -20,9 +20,9 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters internal static class AverageFilter { /// - /// Decodes the scanline + /// Decodes a scanline, which was filtered with the average filter. /// - /// The scanline to decode + /// The scanline to decode. /// The previous scanline. /// The bytes per pixel. [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -33,32 +33,126 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters ref byte scanBaseRef = ref MemoryMarshal.GetReference(scanline); ref byte prevBaseRef = ref MemoryMarshal.GetReference(previousScanline); + // The Avg filter predicts each pixel as the (truncated) average of a and b: // Average(x) + floor((Raw(x-bpp)+Prior(x))/2) - int x = 1; - for (; x <= bytesPerPixel /* Note the <= because x starts at 1 */; ++x) +#if SUPPORTS_RUNTIME_INTRINSICS + if (Sse2.IsSupported && bytesPerPixel is 3 or 4) { - ref byte scan = ref Unsafe.Add(ref scanBaseRef, x); - byte above = Unsafe.Add(ref prevBaseRef, x); - scan = (byte)(scan + (above >> 1)); - } + if (bytesPerPixel is 3) + { + Vector128 a = Vector128.Zero; + Vector128 b = Vector128.Zero; + Vector128 d = Vector128.Zero; + var ones = Vector128.Create((byte)1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1); + + Span scratch = stackalloc byte[4]; + ref byte scratchRef = ref MemoryMarshal.GetReference(scratch); + int rb = scanline.Length; + int offset = 0; + while (rb >= 4) + { + a = d; + b = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref Unsafe.Add(ref prevBaseRef, offset))).AsByte(); + d = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref Unsafe.Add(ref scanBaseRef, offset))).AsByte(); + + d = AverageSubtractAdd(a, b, d, ones); + + // Store the result. + int result = Sse2.ConvertToInt32(d.AsInt32()); + Unsafe.As(ref scratchRef) = result; + scratch.Slice(0, 3).CopyTo(scanline.Slice(offset, 3)); + + rb -= 3; + offset += 3; + } + + if (rb is 3) + { + a = d; + scratch[3] = 0; + previousScanline.Slice(offset, 3).CopyTo(scratch); + b = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref scratchRef)).AsByte(); + scanline.Slice(offset, 3).CopyTo(scratch); + d = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref scratchRef)).AsByte(); + + d = AverageSubtractAdd(a, b, d, ones); + + // Store the result. + int result = Sse2.ConvertToInt32(d.AsInt32()); + Unsafe.As(ref scratchRef) = result; + scratch.Slice(0, 3).CopyTo(scanline.Slice(offset, 3)); + } + } + else + { + Vector128 a = Vector128.Zero; + Vector128 b = Vector128.Zero; + Vector128 d = Vector128.Zero; + var ones = Vector128.Create((byte)1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1); + + Span scratch = stackalloc byte[4]; + ref byte scratchRef = ref MemoryMarshal.GetReference(scratch); + int rb = scanline.Length; + int offset = 0; + while (rb >= 4) + { + a = d; + b = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref Unsafe.Add(ref prevBaseRef, offset))).AsByte(); + d = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref Unsafe.Add(ref scanBaseRef, offset))).AsByte(); - for (; x < scanline.Length; ++x) + d = AverageSubtractAdd(a, b, d, ones); + + // Store the result. + int result = Sse2.ConvertToInt32(d.AsInt32()); + Unsafe.As(ref scratchRef) = result; + scratch.CopyTo(scanline.Slice(offset, 4)); + + rb -= 4; + offset += 4; + } + } + } + else +#endif { - ref byte scan = ref Unsafe.Add(ref scanBaseRef, x); - byte left = Unsafe.Add(ref scanBaseRef, x - bytesPerPixel); - byte above = Unsafe.Add(ref prevBaseRef, x); - scan = (byte)(scan + Average(left, above)); + int x = 1; + for (; x <= bytesPerPixel /* Note the <= because x starts at 1 */; ++x) + { + ref byte scan = ref Unsafe.Add(ref scanBaseRef, x); + byte above = Unsafe.Add(ref prevBaseRef, x); + scan = (byte)(scan + (above >> 1)); + } + + for (; x < scanline.Length; ++x) + { + ref byte scan = ref Unsafe.Add(ref scanBaseRef, x); + byte left = Unsafe.Add(ref scanBaseRef, x - bytesPerPixel); + byte above = Unsafe.Add(ref prevBaseRef, x); + scan = (byte)(scan + Average(left, above)); + } } } +#if SUPPORTS_RUNTIME_INTRINSICS + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static Vector128 AverageSubtractAdd(Vector128 a, Vector128 b, Vector128 d, Vector128 ones) + { + // PNG requires a truncating average, so we can't just use _mm_avg_epu8. + // ...but we can fix it up by subtracting off 1 if it rounded up. + Vector128 avg = Sse2.Average(a, b); + avg = Sse2.Subtract(avg, Sse2.And(Sse2.Xor(a, b), ones)); + return Sse2.Add(d, avg); + } +#endif + /// - /// Encodes the scanline + /// Encodes a scanline with the average filter applied. /// - /// The scanline to encode + /// The scanline to encode. /// The previous scanline. /// The filtered scanline result. /// The bytes per pixel. - /// The sum of the total variance of the filtered row + /// The sum of the total variance of the filtered row. [MethodImpl(MethodImplOptions.AggressiveInlining)] public static void Encode(ReadOnlySpan scanline, ReadOnlySpan previousScanline, Span result, int bytesPerPixel, out int sum) { From 09b2cdb83aab0a889dbde8ec2453d605a1be1725 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Tue, 22 Feb 2022 15:12:00 +0100 Subject: [PATCH 26/49] Review changes from gfoidl --- src/ImageSharp/Compression/Zlib/Adler32.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Compression/Zlib/Adler32.cs b/src/ImageSharp/Compression/Zlib/Adler32.cs index b13e2e421..1f3cbbca6 100644 --- a/src/ImageSharp/Compression/Zlib/Adler32.cs +++ b/src/ImageSharp/Compression/Zlib/Adler32.cs @@ -3,6 +3,7 @@ using System; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; #if SUPPORTS_RUNTIME_INTRINSICS using System.Runtime.Intrinsics; using System.Runtime.Intrinsics.X86; @@ -94,13 +95,11 @@ namespace SixLabors.ImageSharp.Compression.Zlib uint blocks = length / BlockSize; length -= blocks * BlockSize; - int index = 0; - fixed (byte* bufferPtr = buffer) + fixed (byte* bufferPtr = &MemoryMarshal.GetReference(buffer)) { - fixed (byte* tapPtr = Tap1Tap2) + fixed (byte* tapPtr = &MemoryMarshal.GetReference(Tap1Tap2)) { - index += (int)blocks * BlockSize; - var localBufferPtr = bufferPtr; + byte* localBufferPtr = bufferPtr; // _mm_setr_epi8 on x86 Vector128 tap1 = Sse2.LoadVector128((sbyte*)tapPtr); @@ -185,7 +184,7 @@ namespace SixLabors.ImageSharp.Compression.Zlib uint s2 = (adler >> 16) & 0xFFFF; uint length = (uint)buffer.Length; - fixed (byte* bufferPtr = buffer) + fixed (byte* bufferPtr = &MemoryMarshal.GetReference(buffer)) { byte* localBufferPtr = bufferPtr; From 6016c5cc7fb3508ea8b658c9d13c1b9b68bca718 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Tue, 22 Feb 2022 17:38:47 +0100 Subject: [PATCH 27/49] Add note about multiframe images --- src/ImageSharp/Formats/Tiff/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Tiff/README.md b/src/ImageSharp/Formats/Tiff/README.md index 488701e31..51e84ef55 100644 --- a/src/ImageSharp/Formats/Tiff/README.md +++ b/src/ImageSharp/Formats/Tiff/README.md @@ -25,7 +25,7 @@ ## Implementation Status -- The Decoder currently only supports a single frame per image. +- The Decoder currently only supports decoding multiframe images, which have the same dimensions. - Some compression formats are not yet supported. See the list below. ### Deviations from the TIFF spec (to be fixed) From 71520887acf61dd8d09716aee7f273d5135e5f59 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Tue, 22 Feb 2022 17:43:14 +0100 Subject: [PATCH 28/49] No need for the scratch buffer for 4 bytes per pixel --- .../Formats/Png/Filters/AverageFilter.cs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs b/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs index 94c4fb4d1..50058f01d 100644 --- a/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs @@ -51,16 +51,16 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters int offset = 0; while (rb >= 4) { + ref byte scanRef = ref Unsafe.Add(ref scanBaseRef, offset); a = d; b = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref Unsafe.Add(ref prevBaseRef, offset))).AsByte(); - d = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref Unsafe.Add(ref scanBaseRef, offset))).AsByte(); + d = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref scanRef)).AsByte(); d = AverageSubtractAdd(a, b, d, ones); // Store the result. int result = Sse2.ConvertToInt32(d.AsInt32()); - Unsafe.As(ref scratchRef) = result; - scratch.Slice(0, 3).CopyTo(scanline.Slice(offset, 3)); + Unsafe.As(ref scanRef) = result; rb -= 3; offset += 3; @@ -90,22 +90,20 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters Vector128 d = Vector128.Zero; var ones = Vector128.Create((byte)1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1); - Span scratch = stackalloc byte[4]; - ref byte scratchRef = ref MemoryMarshal.GetReference(scratch); int rb = scanline.Length; int offset = 0; while (rb >= 4) { + ref byte scanRef = ref Unsafe.Add(ref scanBaseRef, offset); a = d; - b = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref Unsafe.Add(ref prevBaseRef, offset))).AsByte(); + b = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref scanRef)).AsByte(); d = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref Unsafe.Add(ref scanBaseRef, offset))).AsByte(); d = AverageSubtractAdd(a, b, d, ones); // Store the result. int result = Sse2.ConvertToInt32(d.AsInt32()); - Unsafe.As(ref scratchRef) = result; - scratch.CopyTo(scanline.Slice(offset, 4)); + Unsafe.As(ref scanRef) = result; rb -= 4; offset += 4; From 18548f7eff6f815f9969ee6298810a5761e9499b Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Tue, 22 Feb 2022 22:32:32 +0100 Subject: [PATCH 29/49] Dont use intrinsics for 3 bytes per pixel --- .../Formats/Png/Filters/AverageFilter.cs | 152 +++++++----------- 1 file changed, 56 insertions(+), 96 deletions(-) diff --git a/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs b/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs index 50058f01d..7747d14ea 100644 --- a/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs @@ -30,119 +30,79 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters { DebugGuard.MustBeSameSized(scanline, previousScanline, nameof(scanline)); - ref byte scanBaseRef = ref MemoryMarshal.GetReference(scanline); - ref byte prevBaseRef = ref MemoryMarshal.GetReference(previousScanline); - // The Avg filter predicts each pixel as the (truncated) average of a and b: // Average(x) + floor((Raw(x-bpp)+Prior(x))/2) #if SUPPORTS_RUNTIME_INTRINSICS - if (Sse2.IsSupported && bytesPerPixel is 3 or 4) + if (Sse2.IsSupported && bytesPerPixel is 4) { - if (bytesPerPixel is 3) - { - Vector128 a = Vector128.Zero; - Vector128 b = Vector128.Zero; - Vector128 d = Vector128.Zero; - var ones = Vector128.Create((byte)1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1); - - Span scratch = stackalloc byte[4]; - ref byte scratchRef = ref MemoryMarshal.GetReference(scratch); - int rb = scanline.Length; - int offset = 0; - while (rb >= 4) - { - ref byte scanRef = ref Unsafe.Add(ref scanBaseRef, offset); - a = d; - b = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref Unsafe.Add(ref prevBaseRef, offset))).AsByte(); - d = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref scanRef)).AsByte(); - - d = AverageSubtractAdd(a, b, d, ones); - - // Store the result. - int result = Sse2.ConvertToInt32(d.AsInt32()); - Unsafe.As(ref scanRef) = result; - - rb -= 3; - offset += 3; - } - - if (rb is 3) - { - a = d; - scratch[3] = 0; - previousScanline.Slice(offset, 3).CopyTo(scratch); - b = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref scratchRef)).AsByte(); - scanline.Slice(offset, 3).CopyTo(scratch); - d = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref scratchRef)).AsByte(); - - d = AverageSubtractAdd(a, b, d, ones); - - // Store the result. - int result = Sse2.ConvertToInt32(d.AsInt32()); - Unsafe.As(ref scratchRef) = result; - scratch.Slice(0, 3).CopyTo(scanline.Slice(offset, 3)); - } - } - else - { - Vector128 a = Vector128.Zero; - Vector128 b = Vector128.Zero; - Vector128 d = Vector128.Zero; - var ones = Vector128.Create((byte)1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1); - - int rb = scanline.Length; - int offset = 0; - while (rb >= 4) - { - ref byte scanRef = ref Unsafe.Add(ref scanBaseRef, offset); - a = d; - b = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref scanRef)).AsByte(); - d = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref Unsafe.Add(ref scanBaseRef, offset))).AsByte(); - - d = AverageSubtractAdd(a, b, d, ones); - - // Store the result. - int result = Sse2.ConvertToInt32(d.AsInt32()); - Unsafe.As(ref scanRef) = result; - - rb -= 4; - offset += 4; - } - } + DecodeSse2(scanline, previousScanline); } else #endif { - int x = 1; - for (; x <= bytesPerPixel /* Note the <= because x starts at 1 */; ++x) - { - ref byte scan = ref Unsafe.Add(ref scanBaseRef, x); - byte above = Unsafe.Add(ref prevBaseRef, x); - scan = (byte)(scan + (above >> 1)); - } - - for (; x < scanline.Length; ++x) - { - ref byte scan = ref Unsafe.Add(ref scanBaseRef, x); - byte left = Unsafe.Add(ref scanBaseRef, x - bytesPerPixel); - byte above = Unsafe.Add(ref prevBaseRef, x); - scan = (byte)(scan + Average(left, above)); - } + DecodeScalar(scanline, previousScanline, bytesPerPixel); } } #if SUPPORTS_RUNTIME_INTRINSICS [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static Vector128 AverageSubtractAdd(Vector128 a, Vector128 b, Vector128 d, Vector128 ones) + private static void DecodeSse2(Span scanline, Span previousScanline) { - // PNG requires a truncating average, so we can't just use _mm_avg_epu8. - // ...but we can fix it up by subtracting off 1 if it rounded up. - Vector128 avg = Sse2.Average(a, b); - avg = Sse2.Subtract(avg, Sse2.And(Sse2.Xor(a, b), ones)); - return Sse2.Add(d, avg); + ref byte scanBaseRef = ref MemoryMarshal.GetReference(scanline); + ref byte prevBaseRef = ref MemoryMarshal.GetReference(previousScanline); + + Vector128 d = Vector128.Zero; + var ones = Vector128.Create((byte)1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1); + + int rb = scanline.Length; + int offset = 1; + while (rb >= 4) + { + ref byte scanRef = ref Unsafe.Add(ref scanBaseRef, offset); + Vector128 a = d; + Vector128 b = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref Unsafe.Add(ref prevBaseRef, offset))).AsByte(); + d = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref scanRef)).AsByte(); + + // PNG requires a truncating average, so we can't just use _mm_avg_epu8, + // but we can fix it up by subtracting off 1 if it rounded up. + Vector128 avg = Sse2.Average(a, b); + Vector128 xor = Sse2.Xor(a, b); + Vector128 and = Sse2.And(xor, ones); + avg = Sse2.Subtract(avg, and); + d = Sse2.Add(d, avg); + + // Store the result. + Unsafe.As(ref scanRef) = Sse2.ConvertToInt32(d.AsInt32()); + + rb -= 4; + offset += 4; + } } #endif + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void DecodeScalar(Span scanline, Span previousScanline, int bytesPerPixel) + { + ref byte scanBaseRef = ref MemoryMarshal.GetReference(scanline); + ref byte prevBaseRef = ref MemoryMarshal.GetReference(previousScanline); + + int x = 1; + for (; x <= bytesPerPixel /* Note the <= because x starts at 1 */; ++x) + { + ref byte scan = ref Unsafe.Add(ref scanBaseRef, x); + byte above = Unsafe.Add(ref prevBaseRef, x); + scan = (byte)(scan + (above >> 1)); + } + + for (; x < scanline.Length; ++x) + { + ref byte scan = ref Unsafe.Add(ref scanBaseRef, x); + byte left = Unsafe.Add(ref scanBaseRef, x - bytesPerPixel); + byte above = Unsafe.Add(ref prevBaseRef, x); + scan = (byte)(scan + Average(left, above)); + } + } + /// /// Encodes a scanline with the average filter applied. /// From 9eb71a20af6676afdb9c4cd07005d21cd022817d Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 23 Feb 2022 18:21:21 +0100 Subject: [PATCH 30/49] Add average filter tests --- .../Formats/Png/PngDecoderFilterTests.cs | 62 +++++++++++++++++++ ...ilterTests.cs => PngEncoderFilterTests.cs} | 4 +- 2 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs rename tests/ImageSharp.Tests/Formats/Png/{PngFilterTests.cs => PngEncoderFilterTests.cs} (98%) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs new file mode 100644 index 000000000..a901fc6cd --- /dev/null +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs @@ -0,0 +1,62 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using SixLabors.ImageSharp.Formats.Png.Filters; +using SixLabors.ImageSharp.Tests.TestUtilities; +using Xunit; + +namespace SixLabors.ImageSharp.Tests.Formats.Png +{ + public class PngDecoderFilterTests + { + private static void RunAverageFilterTest() + { + // arrange + byte[] scanline = + { + 3, 39, 39, 39, 0, 4, 4, 4, 0, 1, 1, 1, 0, 1, 1, 1, 0, 2, 2, 2, 0, 2, 2, 2, 0, 2, 2, 2, 0, 4, 4, 4, + 0, 2, 2, 2, 0, 3, 3, 3, 0, 1, 1, 1, 0, 3, 3, 3, 0, 3, 3, 3, 0, 1, 1, 1, 0, 3, 3, 3, 0, 2, 2, 2, 0, + 1, 1, 1, 0, 3, 3, 3, 0, 1, 1, 1, 0, 3, 3, 3, 0, 1, 1, 1, 0, 3, 3, 3, 0, 3, 3, 3, 0, 254, 254, 254, + 0, 6, 6, 6, 14, 71, 71, 71, 157, 254, 254, 254, 28, 251, 251, 251, 0, 4, 4, 4, 0, 2, 2, 2, 0, 11, + 11, 11, 0, 226, 226, 226, 0, 255 + }; + + byte[] previousScanline = + { + 3, 74, 74, 74, 0, 73, 73, 73, 0, 73, 73, 73, 0, 74, 74, 74, 0, 74, 74, 74, 0, 73, 73, 73, 0, 72, 72, + 72, 0, 72, 72, 72, 0, 73, 73, 73, 0, 74, 74, 74, 0, 73, 73, 73, 0, 72, 72, 72, 0, 72, 72, 72, 0, 74, + 74, 74, 0, 72, 72, 72, 0, 73, 73, 73, 0, 75, 75, 75, 0, 73, 73, 73, 0, 74, 74, 74, 0, 72, 72, 72, 0, + 73, 73, 73, 0, 73, 73, 73, 0, 72, 72, 72, 0, 74, 74, 74, 0, 61, 61, 61, 0, 101, 101, 101, 78, 197, + 197, 197, 251, 152, 152, 152, 255, 155, 155, 155, 255, 162, 162, 162, 255, 175, 175, 175, 255, 160, + 160, 160, 255, 139 + }; + + byte[] expected = + { + 3, 76, 76, 76, 0, 78, 78, 78, 0, 76, 76, 76, 0, 76, 76, 76, 0, 77, 77, 77, 0, 77, 77, 77, 0, 76, 76, + 76, 0, 78, 78, 78, 0, 77, 77, 77, 0, 78, 78, 78, 0, 76, 76, 76, 0, 77, 77, 77, 0, 77, 77, 77, 0, 76, + 76, 76, 0, 77, 77, 77, 0, 77, 77, 77, 0, 77, 77, 77, 0, 78, 78, 78, 0, 77, 77, 77, 0, 77, 77, 77, 0, + 76, 76, 76, 0, 77, 77, 77, 0, 77, 77, 77, 0, 73, 73, 73, 0, 73, 73, 73, 14, 158, 158, 158, 203, 175, + 175, 175, 255, 158, 158, 158, 255, 160, 160, 160, 255, 163, 163, 163, 255, 180, 180, 180, 255, 140, + 140, 140, 255, 255 + }; + + // act + AverageFilter.Decode(scanline, previousScanline, 4); + + // assert + Assert.Equal(scanline, expected); + } + + [Fact] + public void AverageInverse_Works() => RunAverageFilterTest(); + +#if SUPPORTS_RUNTIME_INTRINSICS + [Fact] + public void AverageInverse_WithHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunAverageFilterTest, HwIntrinsics.AllowAll); + + [Fact] + public void AverageInverse__WithoutSSE2_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunAverageFilterTest, HwIntrinsics.DisableSSE2); +#endif + } +} diff --git a/tests/ImageSharp.Tests/Formats/Png/PngFilterTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngEncoderFilterTests.cs similarity index 98% rename from tests/ImageSharp.Tests/Formats/Png/PngFilterTests.cs rename to tests/ImageSharp.Tests/Formats/Png/PngEncoderFilterTests.cs index 9b6119380..11e3fbb23 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngFilterTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngEncoderFilterTests.cs @@ -13,7 +13,7 @@ using Xunit.Abstractions; namespace SixLabors.ImageSharp.Tests.Formats.Png { [Trait("Format", "Png")] - public partial class PngFilterTests : MeasureFixture + public class PngEncoderFilterTests : MeasureFixture { #if BENCHMARKING public const int Times = 1000000; @@ -21,7 +21,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public const int Times = 1; #endif - public PngFilterTests(ITestOutputHelper output) + public PngEncoderFilterTests(ITestOutputHelper output) : base(output) { } From a921c57efc84a24b8d81843f61895b850a869077 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 23 Feb 2022 18:33:52 +0100 Subject: [PATCH 31/49] Add benchmark file for average filter with 4bpp --- .../Codecs/Png/DecodeFilteredPng.cs | 20 ++++++++++++------- tests/ImageSharp.Tests/TestImages.cs | 6 ++++-- tests/Images/Input/Png/AverageFilter4Bpp.png | 3 +++ 3 files changed, 20 insertions(+), 9 deletions(-) create mode 100644 tests/Images/Input/Png/AverageFilter4Bpp.png diff --git a/tests/ImageSharp.Benchmarks/Codecs/Png/DecodeFilteredPng.cs b/tests/ImageSharp.Benchmarks/Codecs/Png/DecodeFilteredPng.cs index 6517bf3c4..373b563c8 100644 --- a/tests/ImageSharp.Benchmarks/Codecs/Png/DecodeFilteredPng.cs +++ b/tests/ImageSharp.Benchmarks/Codecs/Png/DecodeFilteredPng.cs @@ -16,7 +16,8 @@ namespace SixLabors.ImageSharp.Benchmarks.Codecs private byte[] filter1; private byte[] filter2; private byte[] filter3; - private byte[] filter4; + private byte[] averageFilter3bpp; + private byte[] averageFilter4bpp; [GlobalSetup] public void ReadImages() @@ -24,8 +25,9 @@ namespace SixLabors.ImageSharp.Benchmarks.Codecs this.filter0 = File.ReadAllBytes(TestImageFullPath(TestImages.Png.Filter0)); this.filter1 = File.ReadAllBytes(TestImageFullPath(TestImages.Png.Filter1)); this.filter2 = File.ReadAllBytes(TestImageFullPath(TestImages.Png.Filter2)); - this.filter3 = File.ReadAllBytes(TestImageFullPath(TestImages.Png.Filter3)); - this.filter4 = File.ReadAllBytes(TestImageFullPath(TestImages.Png.Filter4)); + this.filter3 = File.ReadAllBytes(TestImageFullPath(TestImages.Png.Filter4)); + this.averageFilter3bpp = File.ReadAllBytes(TestImageFullPath(TestImages.Png.AverageFilter3bpp)); + this.averageFilter4bpp = File.ReadAllBytes(TestImageFullPath(TestImages.Png.AverageFilter4bpp)); } [Benchmark(Baseline = true, Description = "None-filtered PNG file")] @@ -40,13 +42,17 @@ namespace SixLabors.ImageSharp.Benchmarks.Codecs public Size PngFilter2() => LoadPng(this.filter2); - [Benchmark(Description = "Average-filtered PNG file")] - public Size PngFilter3() - => LoadPng(this.filter3); + [Benchmark(Description = "Average-filtered PNG file (3bpp)")] + public Size PngAvgFilter1() + => LoadPng(this.averageFilter3bpp); + + [Benchmark(Description = "Average-filtered PNG file (4bpp)")] + public Size PngAvgFilter2() + => LoadPng(this.averageFilter4bpp); [Benchmark(Description = "Paeth-filtered PNG file")] public Size PngFilter4() - => LoadPng(this.filter4); + => LoadPng(this.filter3); [MethodImpl(MethodImplOptions.AggressiveInlining)] private static Size LoadPng(byte[] bytes) diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 5bce99ce1..2f3bb49f6 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -68,9 +68,11 @@ namespace SixLabors.ImageSharp.Tests public const string Filter0 = "Png/filter0.png"; public const string Filter1 = "Png/filter1.png"; public const string Filter2 = "Png/filter2.png"; - public const string Filter3 = "Png/filter3.png"; + public const string AverageFilter3bpp = "Png/filter3.png"; public const string Filter4 = "Png/filter4.png"; + public const string AverageFilter4bpp = "Png/AverageFilter4Bpp.png"; + // Paletted images also from http://www.schaik.com/pngsuite/pngsuite_fil_png.html public const string PalettedTwoColor = "Png/basn3p01.png"; public const string PalettedFourColor = "Png/basn3p02.png"; @@ -159,7 +161,7 @@ namespace SixLabors.ImageSharp.Tests { P1, Pd, Blur, Splash, Cross, Powerpoint, SplashInterlaced, Interlaced, - Filter0, Filter1, Filter2, Filter3, Filter4, + Filter0, Filter1, Filter2, AverageFilter3bpp, Filter4, FilterVar, VimImage1, VimImage2, VersioningImage1, VersioningImage2, Ratio4x1, Ratio1x4 }; diff --git a/tests/Images/Input/Png/AverageFilter4Bpp.png b/tests/Images/Input/Png/AverageFilter4Bpp.png new file mode 100644 index 000000000..728b6cfaf --- /dev/null +++ b/tests/Images/Input/Png/AverageFilter4Bpp.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:7add6fba794bc76ccea2ee3a311b4050cf17f4f78b69a50785f7739b8b35919e +size 181108 From 3ab1ba6bb304f52ef163c341ac8e22a82d62841a Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 23 Feb 2022 19:02:43 +0100 Subject: [PATCH 32/49] Use DisableHWIntrinsic in average filter test --- tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs index a901fc6cd..ce6cd0526 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs @@ -56,7 +56,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png public void AverageInverse_WithHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunAverageFilterTest, HwIntrinsics.AllowAll); [Fact] - public void AverageInverse__WithoutSSE2_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunAverageFilterTest, HwIntrinsics.DisableSSE2); + public void AverageInverse_WithoutHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunAverageFilterTest, HwIntrinsics.DisableHWIntrinsic); #endif } } From 9f322555a96d7b920740608ca736bc5f8bffc911 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 23 Feb 2022 19:17:44 +0100 Subject: [PATCH 33/49] Rename average filter tests --- tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs index ce6cd0526..a096c5eb6 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs @@ -49,14 +49,14 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png } [Fact] - public void AverageInverse_Works() => RunAverageFilterTest(); + public void AverageFilter_Works() => RunAverageFilterTest(); #if SUPPORTS_RUNTIME_INTRINSICS [Fact] - public void AverageInverse_WithHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunAverageFilterTest, HwIntrinsics.AllowAll); + public void AverageFilter_WithHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunAverageFilterTest, HwIntrinsics.AllowAll); [Fact] - public void AverageInverse_WithoutHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunAverageFilterTest, HwIntrinsics.DisableHWIntrinsic); + public void AverageFilter_WithoutHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunAverageFilterTest, HwIntrinsics.DisableHWIntrinsic); #endif } } From 4f6b807e389b7a2cdb20329065fe1d1d51654590 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 23 Feb 2022 19:50:55 +0100 Subject: [PATCH 34/49] Fix average test data --- .../ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs index a096c5eb6..310f53c51 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs @@ -7,6 +7,7 @@ using Xunit; namespace SixLabors.ImageSharp.Tests.Formats.Png { + [Trait("Format", "Png")] public class PngDecoderFilterTests { private static void RunAverageFilterTest() @@ -18,7 +19,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png 0, 2, 2, 2, 0, 3, 3, 3, 0, 1, 1, 1, 0, 3, 3, 3, 0, 3, 3, 3, 0, 1, 1, 1, 0, 3, 3, 3, 0, 2, 2, 2, 0, 1, 1, 1, 0, 3, 3, 3, 0, 1, 1, 1, 0, 3, 3, 3, 0, 1, 1, 1, 0, 3, 3, 3, 0, 3, 3, 3, 0, 254, 254, 254, 0, 6, 6, 6, 14, 71, 71, 71, 157, 254, 254, 254, 28, 251, 251, 251, 0, 4, 4, 4, 0, 2, 2, 2, 0, 11, - 11, 11, 0, 226, 226, 226, 0, 255 + 11, 11, 0, 226, 226, 226, 0, 255, 128, 234 }; byte[] previousScanline = @@ -28,7 +29,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png 74, 74, 0, 72, 72, 72, 0, 73, 73, 73, 0, 75, 75, 75, 0, 73, 73, 73, 0, 74, 74, 74, 0, 72, 72, 72, 0, 73, 73, 73, 0, 73, 73, 73, 0, 72, 72, 72, 0, 74, 74, 74, 0, 61, 61, 61, 0, 101, 101, 101, 78, 197, 197, 197, 251, 152, 152, 152, 255, 155, 155, 155, 255, 162, 162, 162, 255, 175, 175, 175, 255, 160, - 160, 160, 255, 139 + 160, 160, 255, 139, 128, 134 }; byte[] expected = @@ -38,7 +39,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png 76, 76, 0, 77, 77, 77, 0, 77, 77, 77, 0, 77, 77, 77, 0, 78, 78, 78, 0, 77, 77, 77, 0, 77, 77, 77, 0, 76, 76, 76, 0, 77, 77, 77, 0, 77, 77, 77, 0, 73, 73, 73, 0, 73, 73, 73, 14, 158, 158, 158, 203, 175, 175, 175, 255, 158, 158, 158, 255, 160, 160, 160, 255, 163, 163, 163, 255, 180, 180, 180, 255, 140, - 140, 140, 255, 255 + 140, 140, 255, 138, 6, 115 }; // act From c01001fddb1eee22d316ce8648db295a37a7f26c Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 23 Feb 2022 20:07:09 +0100 Subject: [PATCH 35/49] Add comment about pixel layout --- src/ImageSharp/Formats/Png/Filters/AverageFilter.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs b/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs index 7747d14ea..a99b93adb 100644 --- a/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs @@ -32,6 +32,9 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters // The Avg filter predicts each pixel as the (truncated) average of a and b: // Average(x) + floor((Raw(x-bpp)+Prior(x))/2) + // With pixels positioned like this: + // prev: c b + // row: a d #if SUPPORTS_RUNTIME_INTRINSICS if (Sse2.IsSupported && bytesPerPixel is 4) { From e1f96f2647d6a9ccfef3a61c9d339381e7f3c4f9 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Wed, 23 Feb 2022 21:40:51 +0100 Subject: [PATCH 36/49] Additional png decoder tests for average filter --- .../Codecs/Png/DecodeFilteredPng.cs | 4 ++-- tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs | 11 +++++++++++ tests/ImageSharp.Tests/TestImages.cs | 6 +++--- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/tests/ImageSharp.Benchmarks/Codecs/Png/DecodeFilteredPng.cs b/tests/ImageSharp.Benchmarks/Codecs/Png/DecodeFilteredPng.cs index 373b563c8..92eb4af07 100644 --- a/tests/ImageSharp.Benchmarks/Codecs/Png/DecodeFilteredPng.cs +++ b/tests/ImageSharp.Benchmarks/Codecs/Png/DecodeFilteredPng.cs @@ -26,8 +26,8 @@ namespace SixLabors.ImageSharp.Benchmarks.Codecs this.filter1 = File.ReadAllBytes(TestImageFullPath(TestImages.Png.Filter1)); this.filter2 = File.ReadAllBytes(TestImageFullPath(TestImages.Png.Filter2)); this.filter3 = File.ReadAllBytes(TestImageFullPath(TestImages.Png.Filter4)); - this.averageFilter3bpp = File.ReadAllBytes(TestImageFullPath(TestImages.Png.AverageFilter3bpp)); - this.averageFilter4bpp = File.ReadAllBytes(TestImageFullPath(TestImages.Png.AverageFilter4bpp)); + this.averageFilter3bpp = File.ReadAllBytes(TestImageFullPath(TestImages.Png.AverageFilter3BytesPerPixel)); + this.averageFilter4bpp = File.ReadAllBytes(TestImageFullPath(TestImages.Png.AverageFilter4BytesPerPixel)); } [Benchmark(Baseline = true, Description = "None-filtered PNG file")] diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 0af5d9995..7d4708799 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -111,6 +111,17 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png image.CompareToOriginal(provider, ImageComparer.Exact); } + [Theory] + [WithFile(TestImages.Png.AverageFilter3BytesPerPixel, PixelTypes.Rgba64)] + [WithFile(TestImages.Png.AverageFilter4BytesPerPixel, PixelTypes.Rgba64)] + public void Decode_WithAverageFilter(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); + } + [Theory] [WithFile(TestImages.Png.GrayA8Bit, PixelTypes.Rgba32)] [WithFile(TestImages.Png.Gray1BitTrans, PixelTypes.Rgba32)] diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 2f3bb49f6..ad50fc5c7 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -68,10 +68,10 @@ namespace SixLabors.ImageSharp.Tests public const string Filter0 = "Png/filter0.png"; public const string Filter1 = "Png/filter1.png"; public const string Filter2 = "Png/filter2.png"; - public const string AverageFilter3bpp = "Png/filter3.png"; + public const string AverageFilter3BytesPerPixel = "Png/filter3.png"; public const string Filter4 = "Png/filter4.png"; - public const string AverageFilter4bpp = "Png/AverageFilter4Bpp.png"; + public const string AverageFilter4BytesPerPixel = "Png/AverageFilter4Bpp.png"; // Paletted images also from http://www.schaik.com/pngsuite/pngsuite_fil_png.html public const string PalettedTwoColor = "Png/basn3p01.png"; @@ -161,7 +161,7 @@ namespace SixLabors.ImageSharp.Tests { P1, Pd, Blur, Splash, Cross, Powerpoint, SplashInterlaced, Interlaced, - Filter0, Filter1, Filter2, AverageFilter3bpp, Filter4, + Filter0, Filter1, Filter2, AverageFilter3BytesPerPixel, Filter4, FilterVar, VimImage1, VimImage2, VersioningImage1, VersioningImage2, Ratio4x1, Ratio1x4 }; From 8716c51d3044c40eaa46c6406f1f581afd699112 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 24 Feb 2022 11:53:50 +0100 Subject: [PATCH 37/49] Add SSE2 version of up filter --- .../Formats/Png/Filters/UpFilter.cs | 57 +++++++++++++++++-- 1 file changed, 53 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Formats/Png/Filters/UpFilter.cs b/src/ImageSharp/Formats/Png/Filters/UpFilter.cs index 7e0286991..2d2a442a1 100644 --- a/src/ImageSharp/Formats/Png/Filters/UpFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/UpFilter.cs @@ -21,7 +21,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters internal static class UpFilter { /// - /// Decodes the scanline + /// Decodes a scanline, which was filtered with the up filter. /// /// The scanline to decode /// The previous scanline. @@ -30,6 +30,55 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters { DebugGuard.MustBeSameSized(scanline, previousScanline, nameof(scanline)); +#if SUPPORTS_RUNTIME_INTRINSICS + if (Sse2.IsSupported) + { + DecodeSse2(scanline, previousScanline); + } + else +#endif + { + DecodeScalar(scanline, previousScanline); + } + } + +#if SUPPORTS_RUNTIME_INTRINSICS + private static void DecodeSse2(Span scanline, Span previousScanline) + { + ref byte scanBaseRef = ref MemoryMarshal.GetReference(scanline); + ref byte prevBaseRef = ref MemoryMarshal.GetReference(previousScanline); + + // Up(x) + Prior(x) + int rb = scanline.Length; + int offset = 1; + const int bytesPerBatch = 16; + while (rb >= bytesPerBatch) + { + ref byte scanRef = ref Unsafe.Add(ref scanBaseRef, offset); + Vector128 current = Unsafe.As>(ref scanRef); + Vector128 up = Unsafe.As>(ref Unsafe.Add(ref prevBaseRef, offset)); + + Vector128 sum = Sse2.Add(up, current); + Unsafe.As>(ref scanRef) = sum; + + offset += bytesPerBatch; + rb -= bytesPerBatch; + } + + // Handle left over. + for (int i = offset; i < scanline.Length; i++) + { + ref byte scan = ref Unsafe.Add(ref scanBaseRef, offset); + byte above = Unsafe.Add(ref prevBaseRef, offset); + scan = (byte)(scan + above); + offset++; + } + } +#endif + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void DecodeScalar(Span scanline, Span previousScanline) + { ref byte scanBaseRef = ref MemoryMarshal.GetReference(scanline); ref byte prevBaseRef = ref MemoryMarshal.GetReference(previousScanline); @@ -43,12 +92,12 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters } /// - /// Encodes the scanline + /// Encodes a scanline with the up filter applied. /// - /// The scanline to encode + /// The scanline to encode. /// The previous scanline. /// The filtered scanline result. - /// The sum of the total variance of the filtered row + /// The sum of the total variance of the filtered row. [MethodImpl(MethodImplOptions.AggressiveInlining)] public static void Encode(ReadOnlySpan scanline, ReadOnlySpan previousScanline, Span result, out int sum) { From 0c8dbeae8c6854e75ccefa3902df85f01160f2dc Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 24 Feb 2022 13:58:16 +0100 Subject: [PATCH 38/49] Add Avx version of up filter --- .../Formats/Png/Filters/UpFilter.cs | 38 ++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Png/Filters/UpFilter.cs b/src/ImageSharp/Formats/Png/Filters/UpFilter.cs index 2d2a442a1..20a828e52 100644 --- a/src/ImageSharp/Formats/Png/Filters/UpFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/UpFilter.cs @@ -31,7 +31,11 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters DebugGuard.MustBeSameSized(scanline, previousScanline, nameof(scanline)); #if SUPPORTS_RUNTIME_INTRINSICS - if (Sse2.IsSupported) + if (Avx2.IsSupported) + { + DecodeAvx2(scanline, previousScanline); + } + else if (Sse2.IsSupported) { DecodeSse2(scanline, previousScanline); } @@ -43,6 +47,38 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters } #if SUPPORTS_RUNTIME_INTRINSICS + private static void DecodeAvx2(Span scanline, Span previousScanline) + { + ref byte scanBaseRef = ref MemoryMarshal.GetReference(scanline); + ref byte prevBaseRef = ref MemoryMarshal.GetReference(previousScanline); + + // Up(x) + Prior(x) + int rb = scanline.Length; + int offset = 1; + const int bytesPerBatch = 32; + while (rb >= bytesPerBatch) + { + ref byte scanRef = ref Unsafe.Add(ref scanBaseRef, offset); + Vector256 current = Unsafe.As>(ref scanRef); + Vector256 up = Unsafe.As>(ref Unsafe.Add(ref prevBaseRef, offset)); + + Vector256 sum = Avx2.Add(up, current); + Unsafe.As>(ref scanRef) = sum; + + offset += bytesPerBatch; + rb -= bytesPerBatch; + } + + // Handle left over. + for (int i = offset; i < scanline.Length; i++) + { + ref byte scan = ref Unsafe.Add(ref scanBaseRef, offset); + byte above = Unsafe.Add(ref prevBaseRef, offset); + scan = (byte)(scan + above); + offset++; + } + } + private static void DecodeSse2(Span scanline, Span previousScanline) { ref byte scanBaseRef = ref MemoryMarshal.GetReference(scanline); From 849e866221556474e4790e18ae6898e43de7e2f2 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 24 Feb 2022 16:22:29 +0100 Subject: [PATCH 39/49] Add SSE2 version of sub filter --- .../Formats/Png/Filters/SubFilter.cs | 50 +++++++++++++++++-- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/src/ImageSharp/Formats/Png/Filters/SubFilter.cs b/src/ImageSharp/Formats/Png/Filters/SubFilter.cs index c28b877e4..798597754 100644 --- a/src/ImageSharp/Formats/Png/Filters/SubFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/SubFilter.cs @@ -21,12 +21,52 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters internal static class SubFilter { /// - /// Decodes the scanline + /// Decodes a scanline, which was filtered with the sub filter. /// - /// The scanline to decode + /// The scanline to decode. /// The bytes per pixel. [MethodImpl(MethodImplOptions.AggressiveInlining)] public static void Decode(Span scanline, int bytesPerPixel) + { + // The Sub filter predicts each pixel as the previous pixel. +#if SUPPORTS_RUNTIME_INTRINSICS + if (Sse2.IsSupported && bytesPerPixel is 4) + { + DecodeSse2(scanline); + } + else +#endif + { + DecodeScalar(scanline, bytesPerPixel); + } + } + +#if SUPPORTS_RUNTIME_INTRINSICS + private static void DecodeSse2(Span scanline) + { + ref byte scanBaseRef = ref MemoryMarshal.GetReference(scanline); + + Vector128 d = Vector128.Zero; + + int rb = scanline.Length; + int offset = 1; + while (rb >= 4) + { + ref byte scanRef = ref Unsafe.Add(ref scanBaseRef, offset); + Vector128 a = d; + d = Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref scanRef)).AsByte(); + + d = Sse2.Add(d, a); + + Unsafe.As(ref scanRef) = Sse2.ConvertToInt32(d.AsInt32()); + + rb -= 4; + offset += 4; + } + } +#endif + + private static void DecodeScalar(Span scanline, int bytesPerPixel) { ref byte scanBaseRef = ref MemoryMarshal.GetReference(scanline); @@ -42,12 +82,12 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters } /// - /// Encodes the scanline + /// Encodes a scanline with the sup filter applied. /// - /// The scanline to encode + /// The scanline to encode. /// The filtered scanline result. /// The bytes per pixel. - /// The sum of the total variance of the filtered row + /// The sum of the total variance of the filtered row. [MethodImpl(MethodImplOptions.AggressiveInlining)] public static void Encode(ReadOnlySpan scanline, ReadOnlySpan result, int bytesPerPixel, out int sum) { From 670105f17b5102e9d2e7a1370dc4efe0a04c69dd Mon Sep 17 00:00:00 2001 From: Brian Popow <38701097+brianpopow@users.noreply.github.com> Date: Thu, 24 Feb 2022 18:22:34 +0100 Subject: [PATCH 40/49] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Günther Foidl --- src/ImageSharp/Formats/Png/Filters/AverageFilter.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs b/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs index a99b93adb..5d5c4a79d 100644 --- a/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs @@ -55,7 +55,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters ref byte prevBaseRef = ref MemoryMarshal.GetReference(previousScanline); Vector128 d = Vector128.Zero; - var ones = Vector128.Create((byte)1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1); + var ones = Vector128.Create((byte)1); int rb = scanline.Length; int offset = 1; @@ -89,7 +89,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters ref byte scanBaseRef = ref MemoryMarshal.GetReference(scanline); ref byte prevBaseRef = ref MemoryMarshal.GetReference(previousScanline); - int x = 1; + nint x = 1; for (; x <= bytesPerPixel /* Note the <= because x starts at 1 */; ++x) { ref byte scan = ref Unsafe.Add(ref scanBaseRef, x); From 04219b5f74c39bd9134a10b12346f82a5b7affe8 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 24 Feb 2022 18:24:02 +0100 Subject: [PATCH 41/49] Use nint for offset --- src/ImageSharp/Formats/Png/Filters/AverageFilter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs b/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs index 5d5c4a79d..44a16f154 100644 --- a/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/AverageFilter.cs @@ -58,7 +58,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters var ones = Vector128.Create((byte)1); int rb = scanline.Length; - int offset = 1; + nint offset = 1; while (rb >= 4) { ref byte scanRef = ref Unsafe.Add(ref scanBaseRef, offset); From 8432b9fa4d76cfd13fad3221e77ab43f67ecc6cb Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 24 Feb 2022 18:29:07 +0100 Subject: [PATCH 42/49] Use nint for offset --- src/ImageSharp/Formats/Png/Filters/SubFilter.cs | 4 ++-- src/ImageSharp/Formats/Png/Filters/UpFilter.cs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Formats/Png/Filters/SubFilter.cs b/src/ImageSharp/Formats/Png/Filters/SubFilter.cs index 798597754..eaa4dc034 100644 --- a/src/ImageSharp/Formats/Png/Filters/SubFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/SubFilter.cs @@ -49,7 +49,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters Vector128 d = Vector128.Zero; int rb = scanline.Length; - int offset = 1; + nint offset = 1; while (rb >= 4) { ref byte scanRef = ref Unsafe.Add(ref scanBaseRef, offset); @@ -71,7 +71,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters ref byte scanBaseRef = ref MemoryMarshal.GetReference(scanline); // Sub(x) + Raw(x-bpp) - int x = bytesPerPixel + 1; + nint x = bytesPerPixel + 1; Unsafe.Add(ref scanBaseRef, x); for (; x < scanline.Length; ++x) { diff --git a/src/ImageSharp/Formats/Png/Filters/UpFilter.cs b/src/ImageSharp/Formats/Png/Filters/UpFilter.cs index 20a828e52..0d24d9c5d 100644 --- a/src/ImageSharp/Formats/Png/Filters/UpFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/UpFilter.cs @@ -54,7 +54,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters // Up(x) + Prior(x) int rb = scanline.Length; - int offset = 1; + nint offset = 1; const int bytesPerBatch = 32; while (rb >= bytesPerBatch) { @@ -70,7 +70,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters } // Handle left over. - for (int i = offset; i < scanline.Length; i++) + for (nint i = offset; i < scanline.Length; i++) { ref byte scan = ref Unsafe.Add(ref scanBaseRef, offset); byte above = Unsafe.Add(ref prevBaseRef, offset); @@ -86,7 +86,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters // Up(x) + Prior(x) int rb = scanline.Length; - int offset = 1; + nint offset = 1; const int bytesPerBatch = 16; while (rb >= bytesPerBatch) { @@ -102,7 +102,7 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters } // Handle left over. - for (int i = offset; i < scanline.Length; i++) + for (nint i = offset; i < scanline.Length; i++) { ref byte scan = ref Unsafe.Add(ref scanBaseRef, offset); byte above = Unsafe.Add(ref prevBaseRef, offset); From 06843f2a7d81a00da7be04d615ef26dc0f5389e2 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 24 Feb 2022 23:01:53 +0100 Subject: [PATCH 43/49] Add SSE version of paeth filter --- .../Formats/Png/Filters/PaethFilter.cs | 88 ++++++++++++++++++- 1 file changed, 84 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Formats/Png/Filters/PaethFilter.cs b/src/ImageSharp/Formats/Png/Filters/PaethFilter.cs index 6a89a1122..0553eb46a 100644 --- a/src/ImageSharp/Formats/Png/Filters/PaethFilter.cs +++ b/src/ImageSharp/Formats/Png/Filters/PaethFilter.cs @@ -22,9 +22,9 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters internal static class PaethFilter { /// - /// Decodes the scanline + /// Decodes a scanline, which was filtered with the paeth filter. /// - /// The scanline to decode + /// The scanline to decode. /// The previous scanline. /// The bytes per pixel. [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -32,6 +32,86 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters { DebugGuard.MustBeSameSized(scanline, previousScanline, nameof(scanline)); + // Paeth tries to predict pixel d using the pixel to the left of it, a, + // and two pixels from the previous row, b and c: + // prev: c b + // row: a d + // The Paeth function predicts d to be whichever of a, b, or c is nearest to + // p = a + b - c. +#if SUPPORTS_RUNTIME_INTRINSICS + if (Sse41.IsSupported && bytesPerPixel is 4) + { + DecodeSse41(scanline, previousScanline); + } + else +#endif + { + DecodeScalar(scanline, previousScanline, bytesPerPixel); + } + } + +#if SUPPORTS_RUNTIME_INTRINSICS + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void DecodeSse41(Span scanline, Span previousScanline) + { + ref byte scanBaseRef = ref MemoryMarshal.GetReference(scanline); + ref byte prevBaseRef = ref MemoryMarshal.GetReference(previousScanline); + + Vector128 b = Vector128.Zero; + Vector128 d = Vector128.Zero; + + int rb = scanline.Length; + nint offset = 1; + while (rb >= 4) + { + ref byte scanRef = ref Unsafe.Add(ref scanBaseRef, offset); + + // It's easiest to do this math (particularly, deal with pc) with 16-bit intermediates. + Vector128 c = b; + Vector128 a = d; + b = Sse2.UnpackLow( + Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref Unsafe.Add(ref prevBaseRef, offset))).AsByte(), + Vector128.Zero); + d = Sse2.UnpackLow( + Sse2.ConvertScalarToVector128Int32(Unsafe.As(ref scanRef)).AsByte(), + Vector128.Zero); + + // (p-a) == (a+b-c - a) == (b-c) + Vector128 pa = Sse2.Subtract(b.AsInt16(), c.AsInt16()); + + // (p-b) == (a+b-c - b) == (a-c) + Vector128 pb = Sse2.Subtract(a.AsInt16(), c.AsInt16()); + + // (p-c) == (a+b-c - c) == (a+b-c-c) == (b-c)+(a-c) + Vector128 pc = Sse2.Add(pa.AsInt16(), pb.AsInt16()); + + pa = Ssse3.Abs(pa.AsInt16()).AsInt16(); /* |p-a| */ + pb = Ssse3.Abs(pb.AsInt16()).AsInt16(); /* |p-b| */ + pc = Ssse3.Abs(pc.AsInt16()).AsInt16(); /* |p-c| */ + + Vector128 smallest = Sse2.Min(pc, Sse2.Min(pa, pb)); + + // Paeth breaks ties favoring a over b over c. + Vector128 mask = Sse41.BlendVariable(c, b, Sse2.CompareEqual(smallest, pb).AsByte()); + Vector128 nearest = Sse41.BlendVariable(mask, a, Sse2.CompareEqual(smallest, pa).AsByte()); + + // Note `_epi8`: we need addition to wrap modulo 255. + d = Sse2.Add(d, nearest); + + // Store the result. + Unsafe.As(ref scanRef) = Sse2.ConvertToInt32(Sse2.PackUnsignedSaturate(d.AsInt16(), d.AsInt16()).AsInt32()); + + rb -= 4; + offset += 4; + } + } + +#endif + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void DecodeScalar(Span scanline, Span previousScanline, int bytesPerPixel) + { ref byte scanBaseRef = ref MemoryMarshal.GetReference(scanline); ref byte prevBaseRef = ref MemoryMarshal.GetReference(previousScanline); @@ -56,13 +136,13 @@ namespace SixLabors.ImageSharp.Formats.Png.Filters } /// - /// Encodes the scanline + /// Encodes a scanline and applies the paeth filter. /// /// The scanline to encode /// The previous scanline. /// The filtered scanline result. /// The bytes per pixel. - /// The sum of the total variance of the filtered row + /// The sum of the total variance of the filtered row. [MethodImpl(MethodImplOptions.AggressiveInlining)] public static void Encode(ReadOnlySpan scanline, ReadOnlySpan previousScanline, Span result, int bytesPerPixel, out int sum) { From 88b75da62b2c74172f69930b555335bc59fdde9b Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Fri, 25 Feb 2022 13:05:59 +0100 Subject: [PATCH 44/49] Add tests for png filters with and without intrinsics --- .../Formats/Png/PngDecoderFilterTests.cs | 118 +++++++++++++++++- 1 file changed, 117 insertions(+), 1 deletion(-) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs index 310f53c51..edfff19a4 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderFilterTests.cs @@ -46,18 +46,134 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png AverageFilter.Decode(scanline, previousScanline, 4); // assert - Assert.Equal(scanline, expected); + Assert.Equal(expected, scanline); + } + + private static void RunUpFilterTest() + { + // arrange + byte[] scanline = + { + 62, 23, 186, 150, 174, 4, 205, 59, 153, 134, 158, 86, 240, 173, 191, 58, 111, 183, 77, 37, 85, 23, + 93, 204, 110, 139, 9, 20, 87, 154, 176, 54, 207, 214, 40, 11, 179, 199, 7, 219, 174, 242, 112, 220, + 149, 5, 9, 110, 103, 107, 231, 241, 13, 70, 216, 39, 186, 237, 39, 34, 251, 185, 228, 254 + }; + + byte[] previousScanline = + { + 214, 103, 135, 26, 133, 179, 134, 168, 175, 114, 118, 99, 167, 129, 55, 105, 129, 154, 173, 235, + 179, 191, 41, 137, 253, 0, 81, 198, 159, 228, 224, 245, 14, 113, 5, 45, 126, 239, 233, 179, 229, 62, + 66, 155, 207, 117, 128, 56, 181, 190, 160, 96, 11, 248, 74, 23, 62, 253, 29, 132, 98, 192, 9, 202 + }; + + byte[] expected = + { + 62, 126, 65, 176, 51, 183, 83, 227, 72, 248, 20, 185, 151, 46, 246, 163, 240, 81, 250, 16, 8, 214, + 134, 85, 107, 139, 90, 218, 246, 126, 144, 43, 221, 71, 45, 56, 49, 182, 240, 142, 147, 48, 178, + 119, 100, 122, 137, 166, 28, 41, 135, 81, 24, 62, 34, 62, 248, 234, 68, 166, 93, 121, 237, 200 + }; + + // act + UpFilter.Decode(scanline, previousScanline); + + // assert + Assert.Equal(expected, scanline); + } + + private static void RunSubFilterTest() + { + // arrange + byte[] scanline = + { + 62, 23, 186, 150, 174, 4, 205, 59, 153, 134, 158, 86, 240, 173, 191, 58, 111, 183, 77, 37, 85, 23, + 93, 204, 110, 139, 9, 20, 87, 154, 176, 54, 207, 214, 40, 11, 179, 199, 7, 219, 174, 242, 112, 220, + 149, 5, 9, 110, 103, 107, 231, 241, 13, 70, 216, 39, 186, 237, 39, 34, 251, 185, 228, 254 + }; + + byte[] expected = + { + 62, 23, 186, 150, 174, 27, 135, 209, 71, 161, 37, 39, 55, 78, 228, 97, 166, 5, 49, 134, 251, 28, + 142, 82, 105, 167, 151, 102, 192, 65, 71, 156, 143, 23, 111, 167, 66, 222, 118, 130, 240, 208, 230, + 94, 133, 213, 239, 204, 236, 64, 214, 189, 249, 134, 174, 228, 179, 115, 213, 6, 174, 44, 185, 4 + }; + + // act + SubFilter.Decode(scanline, 4); + + // assert + Assert.Equal(expected, scanline); + } + + private static void RunPaethFilterTest() + { + // arrange + byte[] scanline = + { + 62, 23, 186, 150, 174, 4, 205, 59, 153, 134, 158, 86, 240, 173, 191, 58, 111, 183, 77, 37, 85, 23, + 93, 204, 110, 139, 9, 20, 87, 154, 176, 54, 207, 214, 40, 11, 179, 199, 7, 219, 174, 242, 112, 220, + 149, 5, 9, 110, 103, 107, 231, 241, 13, 70, 216, 39, 186, 237, 39, 34, 251, 185, 228, 254 + }; + + byte[] previousScanline = + { + 214, 103, 135, 26, 133, 179, 134, 168, 175, 114, 118, 99, 167, 129, 55, 105, 129, 154, 173, 235, + 179, 191, 41, 137, 253, 0, 81, 198, 159, 228, 224, 245, 14, 113, 5, 45, 126, 239, 233, 179, 229, 62, + 66, 155, 207, 117, 128, 56, 181, 190, 160, 96, 11, 248, 74, 23, 62, 253, 29, 132, 98, 192, 9, 202 + }; + + byte[] expected = + { + 62, 126, 65, 176, 51, 183, 14, 235, 30, 248, 172, 254, 14, 165, 53, 56, 125, 92, 250, 16, 8, 177, + 10, 220, 118, 139, 50, 240, 205, 126, 144, 43, 221, 71, 45, 54, 144, 182, 240, 142, 147, 48, 178, + 106, 40, 122, 187, 166, 143, 41, 162, 151, 24, 111, 34, 135, 248, 92, 68, 169, 243, 21, 1, 200 + }; + + // act + PaethFilter.Decode(scanline, previousScanline, 4); + + // assert + Assert.Equal(expected, scanline); } [Fact] public void AverageFilter_Works() => RunAverageFilterTest(); + [Fact] + public void UpFilter_Works() => RunUpFilterTest(); + + [Fact] + public void SubFilter_Works() => RunSubFilterTest(); + + [Fact] + public void PaethFilter_Works() => RunPaethFilterTest(); + #if SUPPORTS_RUNTIME_INTRINSICS [Fact] public void AverageFilter_WithHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunAverageFilterTest, HwIntrinsics.AllowAll); [Fact] public void AverageFilter_WithoutHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunAverageFilterTest, HwIntrinsics.DisableHWIntrinsic); + + [Fact] + public void UpFilter_WithHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunUpFilterTest, HwIntrinsics.AllowAll); + + [Fact] + public void UpFilter_WithoutAVX2_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunUpFilterTest, HwIntrinsics.DisableAVX2); + + [Fact] + public void UpFilter_WithoutHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunUpFilterTest, HwIntrinsics.DisableHWIntrinsic); + + [Fact] + public void SubFilter_WithHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunSubFilterTest, HwIntrinsics.AllowAll); + + [Fact] + public void SubFilter_WithoutHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunSubFilterTest, HwIntrinsics.DisableHWIntrinsic); + + [Fact] + public void PaethFilter_WithHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunPaethFilterTest, HwIntrinsics.AllowAll); + + [Fact] + public void PaethFilter_WithoutHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunPaethFilterTest, HwIntrinsics.DisableHWIntrinsic); #endif } } From 26a742eb92bdea04268069ca96bfe9db0b90f298 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Fri, 25 Feb 2022 13:41:38 +0100 Subject: [PATCH 45/49] Additional tests for decoding png's with filter --- .../Codecs/Png/DecodeFilteredPng.cs | 6 ++-- .../Formats/Png/PngDecoderTests.cs | 36 +++++++++++++++++-- tests/ImageSharp.Tests/TestImages.cs | 18 +++------- tests/Images/Input/Png/PaethFilter4Bpp.png | 3 ++ tests/Images/Input/Png/SubFilter4Bpp.png | 3 ++ 5 files changed, 48 insertions(+), 18 deletions(-) create mode 100644 tests/Images/Input/Png/PaethFilter4Bpp.png create mode 100644 tests/Images/Input/Png/SubFilter4Bpp.png diff --git a/tests/ImageSharp.Benchmarks/Codecs/Png/DecodeFilteredPng.cs b/tests/ImageSharp.Benchmarks/Codecs/Png/DecodeFilteredPng.cs index 92eb4af07..5f91a050e 100644 --- a/tests/ImageSharp.Benchmarks/Codecs/Png/DecodeFilteredPng.cs +++ b/tests/ImageSharp.Benchmarks/Codecs/Png/DecodeFilteredPng.cs @@ -23,9 +23,9 @@ namespace SixLabors.ImageSharp.Benchmarks.Codecs public void ReadImages() { this.filter0 = File.ReadAllBytes(TestImageFullPath(TestImages.Png.Filter0)); - this.filter1 = File.ReadAllBytes(TestImageFullPath(TestImages.Png.Filter1)); - this.filter2 = File.ReadAllBytes(TestImageFullPath(TestImages.Png.Filter2)); - this.filter3 = File.ReadAllBytes(TestImageFullPath(TestImages.Png.Filter4)); + this.filter1 = File.ReadAllBytes(TestImageFullPath(TestImages.Png.SubFilter3BytesPerPixel)); + this.filter2 = File.ReadAllBytes(TestImageFullPath(TestImages.Png.UpFilter)); + this.filter3 = File.ReadAllBytes(TestImageFullPath(TestImages.Png.PaethFilter3BytesPerPixel)); this.averageFilter3bpp = File.ReadAllBytes(TestImageFullPath(TestImages.Png.AverageFilter3BytesPerPixel)); this.averageFilter4bpp = File.ReadAllBytes(TestImageFullPath(TestImages.Png.AverageFilter4BytesPerPixel)); } diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 7d4708799..752036126 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -112,8 +112,8 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png } [Theory] - [WithFile(TestImages.Png.AverageFilter3BytesPerPixel, PixelTypes.Rgba64)] - [WithFile(TestImages.Png.AverageFilter4BytesPerPixel, PixelTypes.Rgba64)] + [WithFile(TestImages.Png.AverageFilter3BytesPerPixel, PixelTypes.Rgba32)] + [WithFile(TestImages.Png.AverageFilter4BytesPerPixel, PixelTypes.Rgba32)] public void Decode_WithAverageFilter(TestImageProvider provider) where TPixel : unmanaged, IPixel { @@ -122,6 +122,38 @@ namespace SixLabors.ImageSharp.Tests.Formats.Png image.CompareToOriginal(provider, ImageComparer.Exact); } + [Theory] + [WithFile(TestImages.Png.SubFilter3BytesPerPixel, PixelTypes.Rgba32)] + [WithFile(TestImages.Png.SubFilter4BytesPerPixel, PixelTypes.Rgba32)] + public void Decode_WithSubFilter(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); + } + + [Theory] + [WithFile(TestImages.Png.UpFilter, PixelTypes.Rgba32)] + public void Decode_WithUpFilter(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); + } + + [Theory] + [WithFile(TestImages.Png.PaethFilter3BytesPerPixel, PixelTypes.Rgba32)] + [WithFile(TestImages.Png.PaethFilter4BytesPerPixel, PixelTypes.Rgba32)] + public void Decode_WithPaethFilter(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + image.CompareToOriginal(provider, ImageComparer.Exact); + } + [Theory] [WithFile(TestImages.Png.GrayA8Bit, PixelTypes.Rgba32)] [WithFile(TestImages.Png.Gray1BitTrans, PixelTypes.Rgba32)] diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index ad50fc5c7..9ea3c09f1 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -66,12 +66,13 @@ namespace SixLabors.ImageSharp.Tests // Filtered test images from http://www.schaik.com/pngsuite/pngsuite_fil_png.html public const string Filter0 = "Png/filter0.png"; - public const string Filter1 = "Png/filter1.png"; - public const string Filter2 = "Png/filter2.png"; + public const string SubFilter3BytesPerPixel = "Png/filter1.png"; + public const string SubFilter4BytesPerPixel = "Png/SubFilter4Bpp.png"; + public const string UpFilter = "Png/filter2.png"; public const string AverageFilter3BytesPerPixel = "Png/filter3.png"; - public const string Filter4 = "Png/filter4.png"; - public const string AverageFilter4BytesPerPixel = "Png/AverageFilter4Bpp.png"; + public const string PaethFilter3BytesPerPixel = "Png/filter4.png"; + public const string PaethFilter4BytesPerPixel = "Png/PaethFilter4Bpp.png"; // Paletted images also from http://www.schaik.com/pngsuite/pngsuite_fil_png.html public const string PalettedTwoColor = "Png/basn3p01.png"; @@ -156,15 +157,6 @@ namespace SixLabors.ImageSharp.Tests public const string ColorTypeOne = "Png/xc1n0g08.png"; public const string ColorTypeNine = "Png/xc9n2c08.png"; } - - public static readonly string[] All = - { - P1, Pd, Blur, Splash, Cross, - Powerpoint, SplashInterlaced, Interlaced, - Filter0, Filter1, Filter2, AverageFilter3BytesPerPixel, Filter4, - FilterVar, VimImage1, VimImage2, VersioningImage1, - VersioningImage2, Ratio4x1, Ratio1x4 - }; } public static class Jpeg diff --git a/tests/Images/Input/Png/PaethFilter4Bpp.png b/tests/Images/Input/Png/PaethFilter4Bpp.png new file mode 100644 index 000000000..64c9f96ec --- /dev/null +++ b/tests/Images/Input/Png/PaethFilter4Bpp.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:8b2b0a1190854577d5181fe40af61c421d615a1a2727cf9be5ebe727eaafd00d +size 8624 diff --git a/tests/Images/Input/Png/SubFilter4Bpp.png b/tests/Images/Input/Png/SubFilter4Bpp.png new file mode 100644 index 000000000..d9f2c7fa2 --- /dev/null +++ b/tests/Images/Input/Png/SubFilter4Bpp.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:053fac72ff62c66dacb41a6251efa249d5b31567e0222efbf5b1bef912c0bf77 +size 13013 From 3f9331856e5ecaed01744cd88bdba781bafe440a Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Fri, 25 Feb 2022 18:53:27 +0100 Subject: [PATCH 46/49] Add support for decoding gray tiff with jpeg compression --- .../GrayJpegSpectralConverter.cs | 29 +++++++++ .../Decompressors/JpegTiffCompression.cs | 59 +++++++++++++++---- .../Decompressors/RgbJpegSpectralConverter.cs | 1 - 3 files changed, 77 insertions(+), 12 deletions(-) create mode 100644 src/ImageSharp/Formats/Tiff/Compression/Decompressors/GrayJpegSpectralConverter.cs diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/GrayJpegSpectralConverter.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/GrayJpegSpectralConverter.cs new file mode 100644 index 000000000..5b793c35d --- /dev/null +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/GrayJpegSpectralConverter.cs @@ -0,0 +1,29 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder; +using SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder.ColorConverters; +using SixLabors.ImageSharp.PixelFormats; + +namespace SixLabors.ImageSharp.Formats.Tiff.Compression.Decompressors +{ + /// + /// Spectral converter for gray TIFF's which use the JPEG compression. + /// + /// The type of the pixel. + internal sealed class GrayJpegSpectralConverter : SpectralConverter + where TPixel : unmanaged, IPixel + { + /// + /// Initializes a new instance of the class. + /// + /// The configuration. + public GrayJpegSpectralConverter(Configuration configuration) + : base(configuration) + { + } + + /// + protected override JpegColorConverterBase GetColorConverter(JpegFrame frame, IRawJpegData jpegData) => JpegColorConverterBase.GetConverter(JpegColorSpace.Grayscale, frame.Precision); + } +} diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs index ce7820ccf..f73fe508c 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs @@ -9,7 +9,6 @@ using SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder; using SixLabors.ImageSharp.Formats.Tiff.Constants; using SixLabors.ImageSharp.IO; using SixLabors.ImageSharp.Memory; -using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.PixelFormats; namespace SixLabors.ImageSharp.Formats.Tiff.Compression.Decompressors @@ -55,17 +54,43 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Compression.Decompressors { using var jpegDecoder = new JpegDecoderCore(this.configuration, new JpegDecoder()); - // If the PhotometricInterpretation is YCbCr we explicitly assume the JPEG data is in RGB color space. - // There seems no other way to determine that the JPEG data is RGB colorspace (no APP14 marker, componentId's are not RGB). - using SpectralConverter spectralConverter = this.photometricInterpretation == TiffPhotometricInterpretation.YCbCr ? - new RgbJpegSpectralConverter(this.configuration) : new SpectralConverter(this.configuration); - var scanDecoder = new HuffmanScanDecoder(stream, spectralConverter, CancellationToken.None); - jpegDecoder.LoadTables(this.jpegTables, scanDecoder); - scanDecoder.ResetInterval = 0; - jpegDecoder.ParseStream(stream, scanDecoder, CancellationToken.None); + switch (this.photometricInterpretation) + { + case TiffPhotometricInterpretation.BlackIsZero: + case TiffPhotometricInterpretation.WhiteIsZero: + { + using SpectralConverter spectralConverterGray = new GrayJpegSpectralConverter(this.configuration); + var scanDecoderGray = new HuffmanScanDecoder(stream, spectralConverterGray, CancellationToken.None); + jpegDecoder.LoadTables(this.jpegTables, scanDecoderGray); + scanDecoderGray.ResetInterval = 0; + jpegDecoder.ParseStream(stream, scanDecoderGray, CancellationToken.None); - // TODO: Should we pass through the CancellationToken from the tiff decoder? - CopyImageBytesToBuffer(buffer, spectralConverter.GetPixelBuffer(CancellationToken.None)); + // TODO: Should we pass through the CancellationToken from the tiff decoder? + CopyImageBytesToBuffer(buffer, spectralConverterGray.GetPixelBuffer(CancellationToken.None)); + break; + } + + // If the PhotometricInterpretation is YCbCr we explicitly assume the JPEG data is in RGB color space. + // There seems no other way to determine that the JPEG data is RGB colorspace (no APP14 marker, componentId's are not RGB). + case TiffPhotometricInterpretation.YCbCr: + case TiffPhotometricInterpretation.Rgb: + { + using SpectralConverter spectralConverter = this.photometricInterpretation == TiffPhotometricInterpretation.YCbCr ? + new RgbJpegSpectralConverter(this.configuration) : new SpectralConverter(this.configuration); + var scanDecoder = new HuffmanScanDecoder(stream, spectralConverter, CancellationToken.None); + jpegDecoder.LoadTables(this.jpegTables, scanDecoder); + scanDecoder.ResetInterval = 0; + jpegDecoder.ParseStream(stream, scanDecoder, CancellationToken.None); + + // TODO: Should we pass through the CancellationToken from the tiff decoder? + CopyImageBytesToBuffer(buffer, spectralConverter.GetPixelBuffer(CancellationToken.None)); + break; + } + + default: + TiffThrowHelper.ThrowNotSupported($"Jpeg compressed tiff with photometric interpretation {this.photometricInterpretation} is not supported"); + break; + } } else { @@ -86,6 +111,18 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Compression.Decompressors } } + private static void CopyImageBytesToBuffer(Span buffer, Buffer2D pixelBuffer) + { + int offset = 0; + for (int y = 0; y < pixelBuffer.Height; y++) + { + Span pixelRowSpan = pixelBuffer.DangerousGetRowSpan(y); + Span rgbBytes = MemoryMarshal.AsBytes(pixelRowSpan); + rgbBytes.CopyTo(buffer.Slice(offset)); + offset += rgbBytes.Length; + } + } + /// protected override void Dispose(bool disposing) { diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/RgbJpegSpectralConverter.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/RgbJpegSpectralConverter.cs index 001480542..a83518064 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/RgbJpegSpectralConverter.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/RgbJpegSpectralConverter.cs @@ -1,7 +1,6 @@ // Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. -using System.Threading; using SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder; using SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder.ColorConverters; using SixLabors.ImageSharp.PixelFormats; From 3dcb4db49a5034c8d04e5b841b84b12c1e38eb72 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Fri, 25 Feb 2022 18:56:47 +0100 Subject: [PATCH 47/49] Add test for gray tiff jpeg compressed --- tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs | 1 + tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Tiff/JpegCompressedGray.tiff | 3 +++ 3 files changed, 5 insertions(+) create mode 100644 tests/Images/Input/Tiff/JpegCompressedGray.tiff diff --git a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs index ea0544acf..274890066 100644 --- a/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Tiff/TiffDecoderTests.cs @@ -370,6 +370,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Tiff [WithFile(RgbWithStripsJpegCompressed, PixelTypes.Rgba32)] [WithFile(YCbCrJpegCompressed, PixelTypes.Rgba32)] [WithFile(RgbJpegCompressedNoJpegTable, PixelTypes.Rgba32)] + [WithFile(GrayscaleJpegCompressed, PixelTypes.Rgba32)] public void TiffDecoder_CanDecode_JpegCompressed(TestImageProvider provider) where TPixel : unmanaged, IPixel => TestTiffDecoder(provider, useExactComparer: false); diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 5bce99ce1..4830a4308 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -772,6 +772,7 @@ namespace SixLabors.ImageSharp.Tests public const string GrayscaleDeflateMultistrip = "Tiff/grayscale_deflate_multistrip.tiff"; public const string GrayscaleUncompressed = "Tiff/grayscale_uncompressed.tiff"; + public const string GrayscaleJpegCompressed = "Tiff/JpegCompressedGray.tiff"; public const string PaletteDeflateMultistrip = "Tiff/palette_grayscale_deflate_multistrip.tiff"; public const string PaletteUncompressed = "Tiff/palette_uncompressed.tiff"; public const string RgbDeflate = "Tiff/rgb_deflate.tiff"; diff --git a/tests/Images/Input/Tiff/JpegCompressedGray.tiff b/tests/Images/Input/Tiff/JpegCompressedGray.tiff new file mode 100644 index 000000000..e7feed15a --- /dev/null +++ b/tests/Images/Input/Tiff/JpegCompressedGray.tiff @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:868afd018d025ed7636f1155c1b1f64ba8a36153b56c7598e8dee18ce770cd5a +size 539660 From 2fbf56646c6cb1eb86572279d3525133ec20eef4 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 27 Feb 2022 19:13:36 +0100 Subject: [PATCH 48/49] Remove unnecessary setting ResetInterval to zero --- .../Tiff/Compression/Decompressors/JpegTiffCompression.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs index f73fe508c..cfbc32f4f 100644 --- a/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs +++ b/src/ImageSharp/Formats/Tiff/Compression/Decompressors/JpegTiffCompression.cs @@ -62,7 +62,6 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Compression.Decompressors using SpectralConverter spectralConverterGray = new GrayJpegSpectralConverter(this.configuration); var scanDecoderGray = new HuffmanScanDecoder(stream, spectralConverterGray, CancellationToken.None); jpegDecoder.LoadTables(this.jpegTables, scanDecoderGray); - scanDecoderGray.ResetInterval = 0; jpegDecoder.ParseStream(stream, scanDecoderGray, CancellationToken.None); // TODO: Should we pass through the CancellationToken from the tiff decoder? @@ -79,7 +78,6 @@ namespace SixLabors.ImageSharp.Formats.Tiff.Compression.Decompressors new RgbJpegSpectralConverter(this.configuration) : new SpectralConverter(this.configuration); var scanDecoder = new HuffmanScanDecoder(stream, spectralConverter, CancellationToken.None); jpegDecoder.LoadTables(this.jpegTables, scanDecoder); - scanDecoder.ResetInterval = 0; jpegDecoder.ParseStream(stream, scanDecoder, CancellationToken.None); // TODO: Should we pass through the CancellationToken from the tiff decoder? From 7ca5680cd22e6a909d41c76e069127aa4080ff63 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 3 Mar 2022 13:14:10 +1100 Subject: [PATCH 49/49] Allow pad to work for non-alpha pixel formats --- .../Extensions/Transforms/PadExtensions.cs | 5 ++- .../Transforms/Resize/ResizeProcessor.cs | 18 ++------- .../Resize/ResizeProcessor{TPixel}.cs | 28 ++++++++----- src/ImageSharp/Processing/ResizeOptions.cs | 5 +++ .../Processors/Transforms/PadTest.cs | 40 +++++++++---------- .../Processing/Transforms/PadTest.cs | 2 +- .../Processing/Transforms/ResizeTests.cs | 10 ++--- 7 files changed, 54 insertions(+), 54 deletions(-) diff --git a/src/ImageSharp/Processing/Extensions/Transforms/PadExtensions.cs b/src/ImageSharp/Processing/Extensions/Transforms/PadExtensions.cs index 8c3edcadf..ff374a9ac 100644 --- a/src/ImageSharp/Processing/Extensions/Transforms/PadExtensions.cs +++ b/src/ImageSharp/Processing/Extensions/Transforms/PadExtensions.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. namespace SixLabors.ImageSharp.Processing @@ -34,9 +34,10 @@ namespace SixLabors.ImageSharp.Processing Size = new Size(width, height), Mode = ResizeMode.BoxPad, Sampler = KnownResamplers.NearestNeighbor, + PadColor = color }; - return color.Equals(default) ? source.Resize(options) : source.Resize(options).BackgroundColor(color); + return source.Resize(options); } } } diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs index 3d6900683..ef6a15fc9 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor.cs @@ -21,19 +21,12 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms (Size size, Rectangle rectangle) = ResizeHelper.CalculateTargetLocationAndBounds(sourceSize, options); - this.Sampler = options.Sampler; + this.Options = options; this.DestinationWidth = size.Width; this.DestinationHeight = size.Height; this.DestinationRectangle = rectangle; - this.Compand = options.Compand; - this.PremultiplyAlpha = options.PremultiplyAlpha; } - /// - /// Gets the sampler to perform the resize operation. - /// - public IResampler Sampler { get; } - /// /// Gets the destination width. /// @@ -50,14 +43,9 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms public Rectangle DestinationRectangle { get; } /// - /// Gets a value indicating whether to compress or expand individual pixel color values on processing. - /// - public bool Compand { get; } - - /// - /// Gets a value indicating whether to premultiply the alpha (if it exists) during the resize operation. + /// Gets the resize options. /// - public bool PremultiplyAlpha { get; } + public ResizeOptions Options { get; } /// public override ICloningImageProcessor CreatePixelSpecificCloningProcessor(Configuration configuration, Image source, Rectangle sourceRectangle) diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs index b486e4225..c0bf9291e 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeProcessor{TPixel}.cs @@ -4,7 +4,6 @@ using System; using System.Runtime.CompilerServices; using SixLabors.ImageSharp.Advanced; -using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.PixelFormats; @@ -17,12 +16,11 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms internal class ResizeProcessor : TransformProcessor, IResamplingTransformImageProcessor where TPixel : unmanaged, IPixel { + private readonly ResizeOptions options; private readonly int destinationWidth; private readonly int destinationHeight; private readonly IResampler resampler; private readonly Rectangle destinationRectangle; - private readonly bool compand; - private readonly bool premultiplyAlpha; private Image destination; public ResizeProcessor(Configuration configuration, ResizeProcessor definition, Image source, Rectangle sourceRectangle) @@ -31,13 +29,12 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms this.destinationWidth = definition.DestinationWidth; this.destinationHeight = definition.DestinationHeight; this.destinationRectangle = definition.DestinationRectangle; - this.resampler = definition.Sampler; - this.premultiplyAlpha = definition.PremultiplyAlpha; - this.compand = definition.Compand; + this.options = definition.Options; + this.resampler = definition.Options.Sampler; } /// - protected override Size GetDestinationSize() => new Size(this.destinationWidth, this.destinationHeight); + protected override Size GetDestinationSize() => new(this.destinationWidth, this.destinationHeight); /// protected override void BeforeImageApply(Image destination) @@ -62,8 +59,11 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms Image destination = this.destination; Rectangle sourceRectangle = this.SourceRectangle; Rectangle destinationRectangle = this.destinationRectangle; - bool compand = this.compand; - bool premultiplyAlpha = this.premultiplyAlpha; + bool compand = this.options.Compand; + bool premultiplyAlpha = this.options.PremultiplyAlpha; + 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 @@ -91,6 +91,11 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms ImageFrame sourceFrame = source.Frames[i]; ImageFrame destinationFrame = destination.Frames[i]; + if (shouldFill) + { + destinationFrame.Clear(fillColor); + } + ApplyNNResizeFrameTransform( configuration, sourceFrame, @@ -123,6 +128,11 @@ namespace SixLabors.ImageSharp.Processing.Processors.Transforms ImageFrame sourceFrame = source.Frames[i]; ImageFrame destinationFrame = destination.Frames[i]; + if (shouldFill) + { + destinationFrame.Clear(fillColor); + } + ApplyResizeFrameTransform( configuration, sourceFrame, diff --git a/src/ImageSharp/Processing/ResizeOptions.cs b/src/ImageSharp/Processing/ResizeOptions.cs index 4b31998da..62cf8ab23 100644 --- a/src/ImageSharp/Processing/ResizeOptions.cs +++ b/src/ImageSharp/Processing/ResizeOptions.cs @@ -51,5 +51,10 @@ namespace SixLabors.ImageSharp.Processing /// the alpha (if it exists) during the resize operation. /// public bool PremultiplyAlpha { get; set; } = true; + + /// + /// Gets or sets the color to use as a background when padding an image. + /// + public Color PadColor { get; set; } } } diff --git a/tests/ImageSharp.Tests/Processing/Processors/Transforms/PadTest.cs b/tests/ImageSharp.Tests/Processing/Processors/Transforms/PadTest.cs index b1441d109..780758c2b 100644 --- a/tests/ImageSharp.Tests/Processing/Processors/Transforms/PadTest.cs +++ b/tests/ImageSharp.Tests/Processing/Processors/Transforms/PadTest.cs @@ -20,41 +20,37 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Transforms public void ImageShouldPad(TestImageProvider provider) where TPixel : unmanaged, IPixel { - using (Image image = provider.GetImage()) - { - image.Mutate(x => x.Pad(image.Width + 50, image.Height + 50)); - image.DebugSave(provider); + using Image image = provider.GetImage(); + image.Mutate(x => x.Pad(image.Width + 50, image.Height + 50)); + image.DebugSave(provider); - // Check pixels are empty - for (int y = 0; y < 25; y++) + // Check pixels are empty + for (int y = 0; y < 25; y++) + { + for (int x = 0; x < 25; x++) { - for (int x = 0; x < 25; x++) - { - Assert.Equal(default, image[x, y]); - } + Assert.Equal(default, image[x, y]); } } } [Theory] - [WithFileCollection(nameof(CommonTestImages), PixelTypes.Rgba32)] + [WithFileCollection(nameof(CommonTestImages), PixelTypes.Rgba32 | PixelTypes.Rgb24)] public void ImageShouldPadWithBackgroundColor(TestImageProvider provider) where TPixel : unmanaged, IPixel { - var color = Color.Red; + Color color = Color.Red; TPixel expected = color.ToPixel(); - using (Image image = provider.GetImage()) - { - image.Mutate(x => x.Pad(image.Width + 50, image.Height + 50, color)); - image.DebugSave(provider); + using Image image = provider.GetImage(); + image.Mutate(x => x.Pad(image.Width + 50, image.Height + 50, color)); + image.DebugSave(provider); - // Check pixels are filled - for (int y = 0; y < 25; y++) + // Check pixels are filled + for (int y = 0; y < 25; y++) + { + for (int x = 0; x < 25; x++) { - for (int x = 0; x < 25; x++) - { - Assert.Equal(expected, image[x, y]); - } + Assert.Equal(expected, image[x, y]); } } } diff --git a/tests/ImageSharp.Tests/Processing/Transforms/PadTest.cs b/tests/ImageSharp.Tests/Processing/Transforms/PadTest.cs index 227e470d4..3e6726ba0 100644 --- a/tests/ImageSharp.Tests/Processing/Transforms/PadTest.cs +++ b/tests/ImageSharp.Tests/Processing/Transforms/PadTest.cs @@ -23,7 +23,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms Assert.Equal(width, resizeProcessor.DestinationWidth); Assert.Equal(height, resizeProcessor.DestinationHeight); - Assert.Equal(sampler, resizeProcessor.Sampler); + Assert.Equal(sampler, resizeProcessor.Options.Sampler); } } } diff --git a/tests/ImageSharp.Tests/Processing/Transforms/ResizeTests.cs b/tests/ImageSharp.Tests/Processing/Transforms/ResizeTests.cs index 60f7aaa0b..a29f4c035 100644 --- a/tests/ImageSharp.Tests/Processing/Transforms/ResizeTests.cs +++ b/tests/ImageSharp.Tests/Processing/Transforms/ResizeTests.cs @@ -35,7 +35,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms Assert.Equal(width, resizeProcessor.DestinationWidth); Assert.Equal(height, resizeProcessor.DestinationHeight); - Assert.Equal(sampler, resizeProcessor.Sampler); + Assert.Equal(sampler, resizeProcessor.Options.Sampler); } [Fact] @@ -52,8 +52,8 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms Assert.Equal(width, resizeProcessor.DestinationWidth); Assert.Equal(height, resizeProcessor.DestinationHeight); - Assert.Equal(sampler, resizeProcessor.Sampler); - Assert.Equal(compand, resizeProcessor.Compand); + Assert.Equal(sampler, resizeProcessor.Options.Sampler); + Assert.Equal(compand, resizeProcessor.Options.Compand); } [Fact] @@ -78,8 +78,8 @@ namespace SixLabors.ImageSharp.Tests.Processing.Transforms Assert.Equal(width, resizeProcessor.DestinationWidth); Assert.Equal(height, resizeProcessor.DestinationHeight); - Assert.Equal(sampler, resizeProcessor.Sampler); - Assert.Equal(compand, resizeProcessor.Compand); + Assert.Equal(sampler, resizeProcessor.Options.Sampler); + Assert.Equal(compand, resizeProcessor.Options.Compand); // Ensure options are not altered. Assert.Equal(width, resizeOptions.Size.Width);