From 58e8efac20aa0c9eee16ad8d1451c81069e89df3 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Fri, 15 Mar 2024 20:02:00 +1000 Subject: [PATCH 01/26] Ensure VP8X alpha flag is updated correctly. --- src/ImageSharp/Common/Helpers/RiffHelper.cs | 17 +++++++++++- .../Formats/Webp/BitWriter/BitWriterBase.cs | 26 +++++++++++++----- .../Formats/Webp/Chunks/WebpVp8X.cs | 23 +++++++++++++++- .../Formats/Webp/Lossless/Vp8LEncoder.cs | 20 +++++++++----- .../Formats/Webp/Lossy/Vp8Encoder.cs | 24 +++++++++++------ .../Formats/Webp/WebpChunkParsingUtils.cs | 1 - .../Formats/Webp/WebpDecoderCore.cs | 2 +- .../Formats/Webp/WebpEncoderCore.cs | 27 ++++++++++++------- 8 files changed, 105 insertions(+), 35 deletions(-) diff --git a/src/ImageSharp/Common/Helpers/RiffHelper.cs b/src/ImageSharp/Common/Helpers/RiffHelper.cs index 8f06e5886..667a47fc9 100644 --- a/src/ImageSharp/Common/Helpers/RiffHelper.cs +++ b/src/ImageSharp/Common/Helpers/RiffHelper.cs @@ -3,6 +3,7 @@ using System.Buffers.Binary; using System.Text; +using SixLabors.ImageSharp.Formats.Webp.Chunks; namespace SixLabors.ImageSharp.Common.Helpers; @@ -107,6 +108,7 @@ internal static class RiffHelper position++; } + // Add the size of the encoded file to the Riff header. BinaryPrimitives.WriteUInt32LittleEndian(buffer, dataSize); stream.Position = sizePosition; stream.Write(buffer); @@ -120,5 +122,18 @@ internal static class RiffHelper return sizePosition; } - public static void EndWriteRiffFile(Stream stream, long sizePosition) => EndWriteChunk(stream, sizePosition); + public static void EndWriteRiffFile(Stream stream, in WebpVp8X vp8x, bool updateVp8x, long sizePosition) + { + EndWriteChunk(stream, sizePosition + 4); + + // Write the VP8X chunk if necessary. + if (updateVp8x) + { + long position = stream.Position; + + stream.Position = sizePosition + 12; + vp8x.WriteTo(stream); + stream.Position = position; + } + } } diff --git a/src/ImageSharp/Formats/Webp/BitWriter/BitWriterBase.cs b/src/ImageSharp/Formats/Webp/BitWriter/BitWriterBase.cs index 9ffda0f51..651e68011 100644 --- a/src/ImageSharp/Formats/Webp/BitWriter/BitWriterBase.cs +++ b/src/ImageSharp/Formats/Webp/BitWriter/BitWriterBase.cs @@ -88,7 +88,8 @@ internal abstract class BitWriterBase /// The color profile. /// Flag indicating, if a alpha channel is present. /// Flag indicating, if an animation parameter is present. - public static void WriteTrunksBeforeData( + /// A or a default instance. + public static WebpVp8X WriteTrunksBeforeData( Stream stream, uint width, uint height, @@ -102,16 +103,19 @@ internal abstract class BitWriterBase RiffHelper.BeginWriteRiffFile(stream, WebpConstants.WebpFourCc); // Write VP8X, header if necessary. + WebpVp8X vp8x = default; bool isVp8X = exifProfile != null || xmpProfile != null || iccProfile != null || hasAlpha || hasAnimation; if (isVp8X) { - WriteVp8XHeader(stream, exifProfile, xmpProfile, iccProfile, width, height, hasAlpha, hasAnimation); + vp8x = WriteVp8XHeader(stream, exifProfile, xmpProfile, iccProfile, width, height, hasAlpha, hasAnimation); if (iccProfile != null) { RiffHelper.WriteChunk(stream, (uint)WebpChunkType.Iccp, iccProfile.ToByteArray()); } } + + return vp8x; } /// @@ -124,10 +128,16 @@ internal abstract class BitWriterBase /// Write the trunks after data trunk. /// /// The stream to write to. - /// The exif profile. + /// The VP8X chunk. + /// Whether to update the chunk. + /// The initial position of the stream before encoding. + /// The EXIF profile. /// The XMP profile. public static void WriteTrunksAfterData( Stream stream, + in WebpVp8X vp8x, + bool updateVp8x, + long initialPosition, ExifProfile? exifProfile, XmpProfile? xmpProfile) { @@ -141,7 +151,7 @@ internal abstract class BitWriterBase RiffHelper.WriteChunk(stream, (uint)WebpChunkType.Xmp, xmpProfile.Data); } - RiffHelper.EndWriteRiffFile(stream, 4); + RiffHelper.EndWriteRiffFile(stream, in vp8x, updateVp8x, initialPosition); } /// @@ -186,19 +196,21 @@ internal abstract class BitWriterBase /// Writes a VP8X header to the stream. /// /// The stream to write to. - /// A exif profile or null, if it does not exist. - /// A XMP profile or null, if it does not exist. + /// An EXIF profile or null, if it does not exist. + /// An XMP profile or null, if it does not exist. /// The color profile. /// The width of the image. /// The height of the image. /// Flag indicating, if a alpha channel is present. /// Flag indicating, if an animation parameter is present. - protected static void WriteVp8XHeader(Stream stream, ExifProfile? exifProfile, XmpProfile? xmpProfile, IccProfile? iccProfile, uint width, uint height, bool hasAlpha, bool hasAnimation) + protected static WebpVp8X WriteVp8XHeader(Stream stream, ExifProfile? exifProfile, XmpProfile? xmpProfile, IccProfile? iccProfile, uint width, uint height, bool hasAlpha, bool hasAnimation) { WebpVp8X chunk = new(hasAnimation, xmpProfile != null, exifProfile != null, hasAlpha, iccProfile != null, width, height); chunk.Validate(MaxDimension, MaxCanvasPixels); chunk.WriteTo(stream); + + return chunk; } } diff --git a/src/ImageSharp/Formats/Webp/Chunks/WebpVp8X.cs b/src/ImageSharp/Formats/Webp/Chunks/WebpVp8X.cs index 70d6870ce..41fe4dd07 100644 --- a/src/ImageSharp/Formats/Webp/Chunks/WebpVp8X.cs +++ b/src/ImageSharp/Formats/Webp/Chunks/WebpVp8X.cs @@ -5,7 +5,7 @@ using SixLabors.ImageSharp.Common.Helpers; namespace SixLabors.ImageSharp.Formats.Webp.Chunks; -internal readonly struct WebpVp8X +internal readonly struct WebpVp8X : IEquatable { public WebpVp8X(bool hasAnimation, bool hasXmp, bool hasExif, bool hasAlpha, bool hasIcc, uint width, uint height) { @@ -53,6 +53,24 @@ internal readonly struct WebpVp8X /// public uint Height { get; } + public static bool operator ==(WebpVp8X left, WebpVp8X right) => left.Equals(right); + + public static bool operator !=(WebpVp8X left, WebpVp8X right) => !(left == right); + + public override bool Equals(object? obj) => obj is WebpVp8X x && this.Equals(x); + + public bool Equals(WebpVp8X other) + => this.HasAnimation == other.HasAnimation + && this.HasXmp == other.HasXmp + && this.HasExif == other.HasExif + && this.HasAlpha == other.HasAlpha + && this.HasIcc == other.HasIcc + && this.Width == other.Width + && this.Height == other.Height; + + public override int GetHashCode() + => HashCode.Combine(this.HasAnimation, this.HasXmp, this.HasExif, this.HasAlpha, this.HasIcc, this.Width, this.Height); + public void Validate(uint maxDimension, ulong maxCanvasPixels) { if (this.Width > maxDimension || this.Height > maxDimension) @@ -67,6 +85,9 @@ internal readonly struct WebpVp8X } } + public WebpVp8X WithAlpha(bool hasAlpha) + => new(this.HasAnimation, this.HasXmp, this.HasExif, hasAlpha, this.HasIcc, this.Width, this.Height); + public void WriteTo(Stream stream) { byte flags = 0; diff --git a/src/ImageSharp/Formats/Webp/Lossless/Vp8LEncoder.cs b/src/ImageSharp/Formats/Webp/Lossless/Vp8LEncoder.cs index 518c09ff4..485225ab8 100644 --- a/src/ImageSharp/Formats/Webp/Lossless/Vp8LEncoder.cs +++ b/src/ImageSharp/Formats/Webp/Lossless/Vp8LEncoder.cs @@ -237,7 +237,7 @@ internal class Vp8LEncoder : IDisposable /// public Vp8LHashChain HashChain { get; } - public void EncodeHeader(Image image, Stream stream, bool hasAnimation) + public WebpVp8X EncodeHeader(Image image, Stream stream, bool hasAnimation) where TPixel : unmanaged, IPixel { // Write bytes from the bit-writer buffer to the stream. @@ -247,7 +247,8 @@ internal class Vp8LEncoder : IDisposable ExifProfile exifProfile = this.skipMetadata ? null : metadata.ExifProfile; XmpProfile xmpProfile = this.skipMetadata ? null : metadata.XmpProfile; - BitWriterBase.WriteTrunksBeforeData( + // The alpha flag is updated following encoding. + WebpVp8X vp8x = BitWriterBase.WriteTrunksBeforeData( stream, (uint)image.Width, (uint)image.Height, @@ -262,9 +263,11 @@ internal class Vp8LEncoder : IDisposable WebpMetadata webpMetadata = WebpCommonUtils.GetWebpMetadata(image); BitWriterBase.WriteAnimationParameter(stream, webpMetadata.BackgroundColor, webpMetadata.RepeatCount); } + + return vp8x; } - public void EncodeFooter(Image image, Stream stream) + public void EncodeFooter(Image image, in WebpVp8X vp8x, bool hasAlpha, Stream stream, long initialPosition) where TPixel : unmanaged, IPixel { // Write bytes from the bit-writer buffer to the stream. @@ -273,7 +276,9 @@ internal class Vp8LEncoder : IDisposable ExifProfile exifProfile = this.skipMetadata ? null : metadata.ExifProfile; XmpProfile xmpProfile = this.skipMetadata ? null : metadata.XmpProfile; - BitWriterBase.WriteTrunksAfterData(stream, exifProfile, xmpProfile); + bool updateVp8x = hasAlpha && vp8x != default; + WebpVp8X updated = updateVp8x ? vp8x.WithAlpha(true) : vp8x; + BitWriterBase.WriteTrunksAfterData(stream, in updated, updateVp8x, initialPosition, exifProfile, xmpProfile); } /// @@ -285,7 +290,8 @@ internal class Vp8LEncoder : IDisposable /// The frame metadata. /// The to encode the image data to. /// Flag indicating, if an animation parameter is present. - public void Encode(ImageFrame frame, Rectangle bounds, WebpFrameMetadata frameMetadata, Stream stream, bool hasAnimation) + /// A indicating whether the frame contains an alpha channel. + public bool Encode(ImageFrame frame, Rectangle bounds, WebpFrameMetadata frameMetadata, Stream stream, bool hasAnimation) where TPixel : unmanaged, IPixel { // Convert image pixels to bgra array. @@ -324,6 +330,8 @@ internal class Vp8LEncoder : IDisposable { RiffHelper.EndWriteChunk(stream, prevPosition); } + + return hasAlpha; } /// @@ -502,7 +510,7 @@ internal class Vp8LEncoder : IDisposable /// The type of the pixels. /// The frame pixel buffer to convert. /// true, if the image is non opaque. - private bool ConvertPixelsToBgra(Buffer2DRegion pixels) + public bool ConvertPixelsToBgra(Buffer2DRegion pixels) where TPixel : unmanaged, IPixel { bool nonOpaque = false; diff --git a/src/ImageSharp/Formats/Webp/Lossy/Vp8Encoder.cs b/src/ImageSharp/Formats/Webp/Lossy/Vp8Encoder.cs index 2b74c300a..7317527f7 100644 --- a/src/ImageSharp/Formats/Webp/Lossy/Vp8Encoder.cs +++ b/src/ImageSharp/Formats/Webp/Lossy/Vp8Encoder.cs @@ -311,7 +311,7 @@ internal class Vp8Encoder : IDisposable /// private int MbHeaderLimit { get; } - public void EncodeHeader(Image image, Stream stream, bool hasAlpha, bool hasAnimation) + public WebpVp8X EncodeHeader(Image image, Stream stream, bool hasAlpha, bool hasAnimation) where TPixel : unmanaged, IPixel { // Write bytes from the bitwriter buffer to the stream. @@ -321,7 +321,7 @@ internal class Vp8Encoder : IDisposable ExifProfile exifProfile = this.skipMetadata ? null : metadata.ExifProfile; XmpProfile xmpProfile = this.skipMetadata ? null : metadata.XmpProfile; - BitWriterBase.WriteTrunksBeforeData( + WebpVp8X vp8x = BitWriterBase.WriteTrunksBeforeData( stream, (uint)image.Width, (uint)image.Height, @@ -336,9 +336,11 @@ internal class Vp8Encoder : IDisposable WebpMetadata webpMetadata = WebpCommonUtils.GetWebpMetadata(image); BitWriterBase.WriteAnimationParameter(stream, webpMetadata.BackgroundColor, webpMetadata.RepeatCount); } + + return vp8x; } - public void EncodeFooter(Image image, Stream stream) + public void EncodeFooter(Image image, in WebpVp8X vp8x, bool hasAlpha, Stream stream, long initialPosition) where TPixel : unmanaged, IPixel { // Write bytes from the bitwriter buffer to the stream. @@ -347,7 +349,9 @@ internal class Vp8Encoder : IDisposable ExifProfile exifProfile = this.skipMetadata ? null : metadata.ExifProfile; XmpProfile xmpProfile = this.skipMetadata ? null : metadata.XmpProfile; - BitWriterBase.WriteTrunksAfterData(stream, exifProfile, xmpProfile); + bool updateVp8x = hasAlpha && vp8x != default; + WebpVp8X updated = updateVp8x ? vp8x.WithAlpha(true) : vp8x; + BitWriterBase.WriteTrunksAfterData(stream, in updated, updateVp8x, initialPosition, exifProfile, xmpProfile); } /// @@ -358,9 +362,10 @@ internal class Vp8Encoder : IDisposable /// The stream to encode the image data to. /// The region of interest within the frame to encode. /// The frame metadata. - public void EncodeAnimation(ImageFrame frame, Stream stream, Rectangle bounds, WebpFrameMetadata frameMetadata) - where TPixel : unmanaged, IPixel => - this.Encode(stream, frame, bounds, frameMetadata, true, null); + /// A indicating whether the frame contains an alpha channel. + public bool EncodeAnimation(ImageFrame frame, Stream stream, Rectangle bounds, WebpFrameMetadata frameMetadata) + where TPixel : unmanaged, IPixel + => this.Encode(stream, frame, bounds, frameMetadata, true, null); /// /// Encodes the static image frame to the specified stream. @@ -385,7 +390,8 @@ internal class Vp8Encoder : IDisposable /// The frame metadata. /// Flag indicating, if an animation parameter is present. /// The image to encode from. - private void Encode(Stream stream, ImageFrame frame, Rectangle bounds, WebpFrameMetadata frameMetadata, bool hasAnimation, Image image) + /// A indicating whether the frame contains an alpha channel. + private bool Encode(Stream stream, ImageFrame frame, Rectangle bounds, WebpFrameMetadata frameMetadata, bool hasAnimation, Image image) where TPixel : unmanaged, IPixel { int width = bounds.Width; @@ -515,6 +521,8 @@ internal class Vp8Encoder : IDisposable { encodedAlphaData?.Dispose(); } + + return hasAlpha; } /// diff --git a/src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs b/src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs index 80ffe8a99..07f09d45e 100644 --- a/src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs +++ b/src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs @@ -2,7 +2,6 @@ // Licensed under the Six Labors Split License. using System.Buffers.Binary; -using System.Drawing; using SixLabors.ImageSharp.Formats.Webp.BitReader; using SixLabors.ImageSharp.Formats.Webp.Lossy; using SixLabors.ImageSharp.IO; diff --git a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs index 69a0afcd9..21a25860c 100644 --- a/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs +++ b/src/ImageSharp/Formats/Webp/WebpDecoderCore.cs @@ -54,7 +54,7 @@ internal sealed class WebpDecoderCore : IImageDecoderInternals, IDisposable /// /// The flag to decide how to handle the background color in the Animation Chunk. /// - private BackgroundColorHandling backgroundColorHandling; + private readonly BackgroundColorHandling backgroundColorHandling; /// /// Initializes a new instance of the class. diff --git a/src/ImageSharp/Formats/Webp/WebpEncoderCore.cs b/src/ImageSharp/Formats/Webp/WebpEncoderCore.cs index e37c1d179..d29759f9a 100644 --- a/src/ImageSharp/Formats/Webp/WebpEncoderCore.cs +++ b/src/ImageSharp/Formats/Webp/WebpEncoderCore.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using SixLabors.ImageSharp.Formats.Webp.Chunks; using SixLabors.ImageSharp.Formats.Webp.Lossless; using SixLabors.ImageSharp.Formats.Webp.Lossy; using SixLabors.ImageSharp.Memory; @@ -143,12 +144,14 @@ internal sealed class WebpEncoderCore : IImageEncoderInternals this.nearLossless, this.nearLosslessQuality); - encoder.EncodeHeader(image, stream, hasAnimation); + long initialPosition = stream.Position; + bool hasAlpha = false; + WebpVp8X vp8x = encoder.EncodeHeader(image, stream, hasAnimation); // Encode the first frame. ImageFrame previousFrame = image.Frames.RootFrame; WebpFrameMetadata frameMetadata = WebpCommonUtils.GetWebpFrameMetadata(previousFrame); - encoder.Encode(previousFrame, previousFrame.Bounds(), frameMetadata, stream, hasAnimation); + hasAlpha |= encoder.Encode(previousFrame, previousFrame.Bounds(), frameMetadata, stream, hasAnimation); if (hasAnimation) { @@ -190,14 +193,14 @@ internal sealed class WebpEncoderCore : IImageEncoderInternals this.nearLossless, this.nearLosslessQuality); - animatedEncoder.Encode(encodingFrame, bounds, frameMetadata, stream, hasAnimation); + hasAlpha |= animatedEncoder.Encode(encodingFrame, bounds, frameMetadata, stream, hasAnimation); previousFrame = currentFrame; previousDisposal = frameMetadata.DisposalMethod; } } - encoder.EncodeFooter(image, stream); + encoder.EncodeFooter(image, in vp8x, hasAlpha, stream, initialPosition); } else { @@ -214,17 +217,20 @@ internal sealed class WebpEncoderCore : IImageEncoderInternals this.spatialNoiseShaping, this.alphaCompression); + long initialPosition = stream.Position; + bool hasAlpha = false; + WebpVp8X vp8x = default; if (image.Frames.Count > 1) { - // TODO: What about alpha here? - encoder.EncodeHeader(image, stream, false, true); + // The alpha flag is updated following encoding. + vp8x = encoder.EncodeHeader(image, stream, false, true); // Encode the first frame. ImageFrame previousFrame = image.Frames.RootFrame; WebpFrameMetadata frameMetadata = WebpCommonUtils.GetWebpFrameMetadata(previousFrame); WebpDisposalMethod previousDisposal = frameMetadata.DisposalMethod; - encoder.EncodeAnimation(previousFrame, stream, previousFrame.Bounds(), frameMetadata); + hasAlpha |= encoder.EncodeAnimation(previousFrame, stream, previousFrame.Bounds(), frameMetadata); // Encode additional frames // This frame is reused to store de-duplicated pixel buffers. @@ -263,18 +269,19 @@ internal sealed class WebpEncoderCore : IImageEncoderInternals this.spatialNoiseShaping, this.alphaCompression); - animatedEncoder.EncodeAnimation(encodingFrame, stream, bounds, frameMetadata); + hasAlpha |= animatedEncoder.EncodeAnimation(encodingFrame, stream, bounds, frameMetadata); previousFrame = currentFrame; previousDisposal = frameMetadata.DisposalMethod; } + + encoder.EncodeFooter(image, in vp8x, hasAlpha, stream, initialPosition); } else { encoder.EncodeStatic(stream, image); + encoder.EncodeFooter(image, in vp8x, hasAlpha, stream, initialPosition); } - - encoder.EncodeFooter(image, stream); } } } From a197f222a685b3e4b08bf7d83e06cc95b68f09ee Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 17 Mar 2024 11:05:56 +1000 Subject: [PATCH 02/26] Update WebpVp8X.cs --- src/ImageSharp/Formats/Webp/Chunks/WebpVp8X.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Webp/Chunks/WebpVp8X.cs b/src/ImageSharp/Formats/Webp/Chunks/WebpVp8X.cs index 41fe4dd07..fc88f8faa 100644 --- a/src/ImageSharp/Formats/Webp/Chunks/WebpVp8X.cs +++ b/src/ImageSharp/Formats/Webp/Chunks/WebpVp8X.cs @@ -68,7 +68,7 @@ internal readonly struct WebpVp8X : IEquatable && this.Width == other.Width && this.Height == other.Height; - public override int GetHashCode() + public override int GetHashCode() => HashCode.Combine(this.HasAnimation, this.HasXmp, this.HasExif, this.HasAlpha, this.HasIcc, this.Width, this.Height); public void Validate(uint maxDimension, ulong maxCanvasPixels) From 623ad5defdbf8b62fe9c5fd2d1fc05e1be674316 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 19 Mar 2024 00:30:20 +1000 Subject: [PATCH 03/26] Only exit after multiple EOF hits --- .../Jpeg/Components/Decoder/JpegBitReader.cs | 20 +++++++++++++++---- .../Formats/Jpg/JpegDecoderTests.cs | 11 ++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Jpg/issues/Issue2638.jpg | 3 +++ 4 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 tests/Images/Input/Jpg/issues/Issue2638.jpg diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs index 0877dbc92..c2b0cb6e0 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs @@ -22,6 +22,9 @@ internal struct JpegBitReader // Whether there is no more good data to pull from the stream for the current mcu. private bool badData; + // How many times have we hit the eof. + private int eofHitCount; + public JpegBitReader(BufferedReadStream stream) { this.stream = stream; @@ -31,6 +34,7 @@ internal struct JpegBitReader this.MarkerPosition = 0; this.badData = false; this.NoData = false; + this.eofHitCount = 0; } /// @@ -80,6 +84,9 @@ internal struct JpegBitReader [MethodImpl(InliningOptions.ShortMethod)] public bool HasBadMarker() => this.Marker != JpegConstants.Markers.XFF && !this.HasRestartMarker(); + [MethodImpl(InliningOptions.ShortMethod)] + public bool HasEndMarker() => this.Marker == JpegConstants.Markers.EOI; + [MethodImpl(InliningOptions.AlwaysInline)] public void FillBuffer() { @@ -219,11 +226,16 @@ internal struct JpegBitReader // we know we have hit the EOI and completed decoding the scan buffer. if (value == -1 || (this.badData && this.data == 0 && this.stream.Position >= this.stream.Length)) { - // We've encountered the end of the file stream which means there's no EOI marker + // We've passed the end of the file stream which means there's no EOI marker // in the image or the SOS marker has the wrong dimensions set. - this.badData = true; - this.NoData = true; - value = 0; + if (this.eofHitCount > JpegConstants.Huffman.FetchLoop) + { + this.badData = true; + this.NoData = true; + value = 0; + } + + this.eofHitCount++; } return value; diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index c8d93f6e9..6a94a98ac 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -364,4 +364,15 @@ public partial class JpegDecoderTests image.DebugSave(provider); image.CompareToOriginal(provider); } + + // https://github.com/SixLabors/ImageSharp/issues/2638 + [Theory] + [WithFile(TestImages.Jpeg.Issues.Issue2638, PixelTypes.Rgba32)] + public void Issue2638_DecodeWorks(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(JpegDecoder.Instance); + image.DebugSave(provider); + image.CompareToOriginal(provider); + } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 00f7370f3..fe633fddc 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -315,6 +315,7 @@ public static class TestImages public const string Issue2564 = "Jpg/issues/issue-2564.jpg"; public const string HangBadScan = "Jpg/issues/Hang_C438A851.jpg"; public const string Issue2517 = "Jpg/issues/issue2517-bad-d7.jpg"; + public const string Issue2638 = "Jpg/issues/Issue2638.jpg"; public static class Fuzz { diff --git a/tests/Images/Input/Jpg/issues/Issue2638.jpg b/tests/Images/Input/Jpg/issues/Issue2638.jpg new file mode 100644 index 000000000..f42d67b0e --- /dev/null +++ b/tests/Images/Input/Jpg/issues/Issue2638.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:208d5b0b727bbef120a7e090e020a48f99c9e264c2d3939ba749f8620853c1fe +size 70876 From 4f289f9e126d39191f747bd53ae32cb325f01178 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 19 Mar 2024 00:31:57 +1000 Subject: [PATCH 04/26] Remove unused code. --- .../Formats/Jpeg/Components/Decoder/JpegBitReader.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs index c2b0cb6e0..babd2ff4d 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs @@ -84,9 +84,6 @@ internal struct JpegBitReader [MethodImpl(InliningOptions.ShortMethod)] public bool HasBadMarker() => this.Marker != JpegConstants.Markers.XFF && !this.HasRestartMarker(); - [MethodImpl(InliningOptions.ShortMethod)] - public bool HasEndMarker() => this.Marker == JpegConstants.Markers.EOI; - [MethodImpl(InliningOptions.AlwaysInline)] public void FillBuffer() { From ea12c0d480510fedda02afeb420d623a8e5901a7 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 19 Mar 2024 00:35:23 +1000 Subject: [PATCH 05/26] Update comment --- src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs index babd2ff4d..e71d86a1d 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs @@ -223,7 +223,7 @@ internal struct JpegBitReader // we know we have hit the EOI and completed decoding the scan buffer. if (value == -1 || (this.badData && this.data == 0 && this.stream.Position >= this.stream.Length)) { - // We've passed the end of the file stream which means there's no EOI marker + // We've hit the end of the file stream more times than allowed which means there's no EOI marker // in the image or the SOS marker has the wrong dimensions set. if (this.eofHitCount > JpegConstants.Huffman.FetchLoop) { From 92b82779ac8e7a032989533bb9a01ef092186b2e Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Thu, 28 Mar 2024 02:36:45 +0100 Subject: [PATCH 06/26] Limit all allocations --- .../Memory/Allocators/MemoryAllocator.cs | 51 ++++++++++++++++--- .../Allocators/MemoryAllocatorOptions.cs | 21 +++++++- .../Allocators/SimpleGcMemoryAllocator.cs | 10 +++- ...iformUnmanagedMemoryPoolMemoryAllocator.cs | 25 ++++----- .../Memory/InvalidMemoryOperationException.cs | 6 +++ 5 files changed, 93 insertions(+), 20 deletions(-) diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs index 2bd9cb5ee..194449cfc 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs @@ -2,6 +2,8 @@ // Licensed under the Six Labors Split License. using System.Buffers; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; namespace SixLabors.ImageSharp.Memory; @@ -10,6 +12,8 @@ namespace SixLabors.ImageSharp.Memory; /// public abstract class MemoryAllocator { + private const int OneGigabyte = 1 << 30; + /// /// Gets the default platform-specific global instance that /// serves as the default value for . @@ -20,6 +24,12 @@ public abstract class MemoryAllocator /// public static MemoryAllocator Default { get; } = Create(); + internal long MemoryGroupAllocationLimitBytes { get; private set; } = Environment.Is64BitProcess ? + 4L * OneGigabyte : + OneGigabyte; + + internal int SingleBufferAllocationLimitBytes { get; private set; } = OneGigabyte; + /// /// Gets the length of the largest contiguous buffer that can be handled by this allocator instance in bytes. /// @@ -30,16 +40,24 @@ public abstract class MemoryAllocator /// Creates a default instance of a optimized for the executing platform. /// /// The . - public static MemoryAllocator Create() => - new UniformUnmanagedMemoryPoolMemoryAllocator(null); + public static MemoryAllocator Create() => Create(default); /// /// Creates the default using the provided options. /// /// The . /// The . - public static MemoryAllocator Create(MemoryAllocatorOptions options) => - new UniformUnmanagedMemoryPoolMemoryAllocator(options.MaximumPoolSizeMegabytes); + public static MemoryAllocator Create(MemoryAllocatorOptions options) + { + UniformUnmanagedMemoryPoolMemoryAllocator allocator = new(options.MaximumPoolSizeMegabytes); + if (options.AllocationLimitMegabytes.HasValue) + { + allocator.MemoryGroupAllocationLimitBytes = options.AllocationLimitMegabytes.Value * 1024 * 1024; + allocator.SingleBufferAllocationLimitBytes = (int)Math.Min(allocator.SingleBufferAllocationLimitBytes, allocator.MemoryGroupAllocationLimitBytes); + } + + return allocator; + } /// /// Allocates an , holding a of length . @@ -69,10 +87,31 @@ public abstract class MemoryAllocator /// The . /// A new . /// Thrown when 'blockAlignment' converted to bytes is greater than the buffer capacity of the allocator. - internal virtual MemoryGroup AllocateGroup( + internal MemoryGroup AllocateGroup( long totalLength, int bufferAlignment, AllocationOptions options = AllocationOptions.None) where T : struct - => MemoryGroup.Allocate(this, totalLength, bufferAlignment, options); + { + long totalLengthInBytes = totalLength * Unsafe.SizeOf(); + if (totalLengthInBytes < 0) + { + ThrowNotRepresentable(); + } + + if (totalLengthInBytes > this.MemoryGroupAllocationLimitBytes) + { + InvalidMemoryOperationException.ThrowAllocationOverLimitException(totalLengthInBytes, this.MemoryGroupAllocationLimitBytes); + } + + return this.AllocateGroupCore(totalLengthInBytes, totalLength, bufferAlignment, options); + + [DoesNotReturn] + static void ThrowNotRepresentable() => + throw new InvalidMemoryOperationException("Attempted to allocate a MemoryGroup of a size that is not representable."); + } + + internal virtual MemoryGroup AllocateGroupCore(long totalLengthInElements, long totalLengthInBytes, int bufferAlignment, AllocationOptions options) + where T : struct + => MemoryGroup.Allocate(this, totalLengthInElements, bufferAlignment, options); } diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs index 5a821fd04..d9ba62c1e 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Six Labors Split License. namespace SixLabors.ImageSharp.Memory; @@ -9,6 +9,7 @@ namespace SixLabors.ImageSharp.Memory; public struct MemoryAllocatorOptions { private int? maximumPoolSizeMegabytes; + private int? allocationLimitMegabytes; /// /// Gets or sets a value defining the maximum size of the 's internal memory pool @@ -27,4 +28,22 @@ public struct MemoryAllocatorOptions this.maximumPoolSizeMegabytes = value; } } + + /// + /// Gets or sets a value defining the maximum (discontiguous) buffer size that can be allocated by the allocator in Megabytes. + /// means platform default: 1GB on 32-bit processes, 4GB on 64-bit processes. + /// + public int? AllocationLimitMegabytes + { + get => this.allocationLimitMegabytes; + set + { + if (value.HasValue) + { + Guard.MustBeGreaterThan(value.Value, 0, nameof(this.AllocationLimitMegabytes)); + } + + this.allocationLimitMegabytes = value; + } + } } diff --git a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs index 41730d967..e60462195 100644 --- a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs @@ -1,7 +1,8 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Six Labors Split License. using System.Buffers; +using System.Runtime.CompilerServices; using SixLabors.ImageSharp.Memory.Internals; namespace SixLabors.ImageSharp.Memory; @@ -19,6 +20,13 @@ public sealed class SimpleGcMemoryAllocator : MemoryAllocator { Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length)); + int lengthInBytes = length * Unsafe.SizeOf(); + + if (lengthInBytes > this.SingleBufferAllocationLimitBytes) + { + InvalidMemoryOperationException.ThrowAllocationOverLimitException(lengthInBytes, this.SingleBufferAllocationLimitBytes); + } + return new BasicArrayBuffer(new T[length]); } } diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index 0bae19363..585d4717a 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -2,6 +2,7 @@ // Licensed under the Six Labors Split License. using System.Buffers; +using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using SixLabors.ImageSharp.Memory.Internals; @@ -86,6 +87,11 @@ internal sealed class UniformUnmanagedMemoryPoolMemoryAllocator : MemoryAllocato Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length)); int lengthInBytes = length * Unsafe.SizeOf(); + if (lengthInBytes > this.SingleBufferAllocationLimitBytes) + { + InvalidMemoryOperationException.ThrowAllocationOverLimitException(lengthInBytes, this.SingleBufferAllocationLimitBytes); + } + if (lengthInBytes <= this.sharedArrayPoolThresholdInBytes) { var buffer = new SharedArrayPoolBuffer(length); @@ -111,20 +117,15 @@ internal sealed class UniformUnmanagedMemoryPoolMemoryAllocator : MemoryAllocato } /// - internal override MemoryGroup AllocateGroup( - long totalLength, + internal override MemoryGroup AllocateGroupCore( + long totalLengthInElements, + long totalLengthInBytes, int bufferAlignment, AllocationOptions options = AllocationOptions.None) { - long totalLengthInBytes = totalLength * Unsafe.SizeOf(); - if (totalLengthInBytes < 0) - { - throw new InvalidMemoryOperationException("Attempted to allocate a MemoryGroup of a size that is not representable."); - } - if (totalLengthInBytes <= this.sharedArrayPoolThresholdInBytes) { - var buffer = new SharedArrayPoolBuffer((int)totalLength); + var buffer = new SharedArrayPoolBuffer((int)totalLengthInElements); return MemoryGroup.CreateContiguous(buffer, options.Has(AllocationOptions.Clean)); } @@ -134,18 +135,18 @@ internal sealed class UniformUnmanagedMemoryPoolMemoryAllocator : MemoryAllocato UnmanagedMemoryHandle mem = this.pool.Rent(); if (mem.IsValid) { - UnmanagedBuffer buffer = this.pool.CreateGuardedBuffer(mem, (int)totalLength, options.Has(AllocationOptions.Clean)); + UnmanagedBuffer buffer = this.pool.CreateGuardedBuffer(mem, (int)totalLengthInElements, options.Has(AllocationOptions.Clean)); return MemoryGroup.CreateContiguous(buffer, options.Has(AllocationOptions.Clean)); } } // Attempt to rent the whole group from the pool, allocate a group of unmanaged buffers if the attempt fails: - if (MemoryGroup.TryAllocate(this.pool, totalLength, bufferAlignment, options, out MemoryGroup? poolGroup)) + if (MemoryGroup.TryAllocate(this.pool, totalLengthInElements, bufferAlignment, options, out MemoryGroup? poolGroup)) { return poolGroup; } - return MemoryGroup.Allocate(this.nonPoolAllocator, totalLength, bufferAlignment, options); + return MemoryGroup.Allocate(this.nonPoolAllocator, totalLengthInElements, bufferAlignment, options); } public override void ReleaseRetainedResources() => this.pool.Release(); diff --git a/src/ImageSharp/Memory/InvalidMemoryOperationException.cs b/src/ImageSharp/Memory/InvalidMemoryOperationException.cs index 6a5547223..0dc8106b6 100644 --- a/src/ImageSharp/Memory/InvalidMemoryOperationException.cs +++ b/src/ImageSharp/Memory/InvalidMemoryOperationException.cs @@ -1,6 +1,8 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Diagnostics.CodeAnalysis; + namespace SixLabors.ImageSharp.Memory; /// @@ -24,4 +26,8 @@ public class InvalidMemoryOperationException : InvalidOperationException public InvalidMemoryOperationException() { } + + [DoesNotReturn] + internal static void ThrowAllocationOverLimitException(long length, long limit) => + throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of length={length} that exceeded the limit {limit}."); } From 7dd3c43a00fffc7c9e70b6ed53e24c936f4b38a8 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Thu, 28 Mar 2024 02:40:52 +0100 Subject: [PATCH 07/26] reduce UnmanagedMemoryHandle.MaxAllocationAttempts --- .../Memory/Allocators/Internals/UnmanagedMemoryHandle.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs index c13fa754e..6b31cadf4 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs @@ -11,7 +11,7 @@ namespace SixLabors.ImageSharp.Memory.Internals; internal struct UnmanagedMemoryHandle : IEquatable { // Number of allocation re-attempts when detecting OutOfMemoryException. - private const int MaxAllocationAttempts = 1000; + private const int MaxAllocationAttempts = 10; // Track allocations for testing purposes: private static int totalOutstandingHandles; From 417667a12c5ba1871b3b33d623e2aaa21020e1d9 Mon Sep 17 00:00:00 2001 From: SpaceCheetah Date: Thu, 28 Mar 2024 12:33:23 -1000 Subject: [PATCH 08/26] Add test --- tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs | 3 ++- tests/ImageSharp.Tests/TestImages.cs | 1 + .../Decode_VerifyAllFrames_Rgba32_frame-offset.png/00.png | 3 +++ .../Decode_VerifyAllFrames_Rgba32_frame-offset.png/01.png | 3 +++ .../Decode_VerifyAllFrames_Rgba32_frame-offset.png/02.png | 3 +++ .../Decode_VerifyAllFrames_Rgba32_frame-offset.png/03.png | 3 +++ .../Decode_VerifyAllFrames_Rgba32_frame-offset.png/04.png | 3 +++ tests/Images/Input/Png/animated/frame-offset.png | 3 +++ 8 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/00.png create mode 100644 tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/01.png create mode 100644 tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/02.png create mode 100644 tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/03.png create mode 100644 tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/04.png create mode 100644 tests/Images/Input/Png/animated/frame-offset.png diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 9b165526e..336b89376 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -87,7 +87,8 @@ public partial class PngDecoderTests TestImages.Png.DisposeBackgroundRegion, TestImages.Png.DisposePreviousFirst, TestImages.Png.DisposeBackgroundBeforeRegion, - TestImages.Png.BlendOverMultiple + TestImages.Png.BlendOverMultiple, + TestImages.Png.FrameOffset }; [Theory] diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index fe633fddc..7e6ac9e8d 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -73,6 +73,7 @@ public static class TestImages public const string DisposeBackgroundRegion = "Png/animated/15-dispose-background-region.png"; public const string DisposePreviousFirst = "Png/animated/12-dispose-prev-first.png"; public const string BlendOverMultiple = "Png/animated/21-blend-over-multiple.png"; + public const string FrameOffset = "Png/animated/frame-offset.png"; public const string Issue2666 = "Png/issues/Issue_2666.png"; // Filtered test images from http://www.schaik.com/pngsuite/pngsuite_fil_png.html diff --git a/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/00.png b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/00.png new file mode 100644 index 000000000..b9fa24c93 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/00.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:84e2353264e3488122f4d488d7c4b198ff5192ad0c662c7fb0a369c957ecc7ea +size 353 diff --git a/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/01.png b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/01.png new file mode 100644 index 000000000..6f3a27187 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/01.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:2916711d3f4d72eb66a5cfc2b40a3318eb4cce5b367658cfc7e3b573fd39cc33 +size 693 diff --git a/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/02.png b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/02.png new file mode 100644 index 000000000..50911cce5 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/02.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:7f9d5503414ccefa6b66661b1e93c2c3f6e4491f14af006a71153cecf43b52f5 +size 806 diff --git a/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/03.png b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/03.png new file mode 100644 index 000000000..89d2f9570 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/03.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:fe42b7dc6524d5589ad680650f4bcd181319b40b258b31e0932d6e936818e980 +size 570 diff --git a/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/04.png b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/04.png new file mode 100644 index 000000000..c3f2b99b8 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/04.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:8002ff5b3451b348f285eec15dd7a093c62d11d8b77c3ead9ac89ca6eb29977d +size 669 diff --git a/tests/Images/Input/Png/animated/frame-offset.png b/tests/Images/Input/Png/animated/frame-offset.png new file mode 100644 index 000000000..4eebb44a3 --- /dev/null +++ b/tests/Images/Input/Png/animated/frame-offset.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:3c019073841b48b02cb07c779fed8654c6052aee700e7620d07f5d775d97088f +size 2156 From d933db75ed5fff7e3ada9ded4d5562b2982ca260 Mon Sep 17 00:00:00 2001 From: SpaceCheetah Date: Thu, 28 Mar 2024 13:50:40 -1000 Subject: [PATCH 09/26] Fix ProcessInterlacedPaletteScanline not obeying frameControl.XOffset --- src/ImageSharp/Formats/Png/PngScanlineProcessor.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs b/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs index f217515e3..b327ed21b 100644 --- a/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs +++ b/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs @@ -198,8 +198,9 @@ internal static class PngScanlineProcessor ref byte scanlineSpanRef = ref MemoryMarshal.GetReference(scanlineSpan); ref TPixel rowSpanRef = ref MemoryMarshal.GetReference(rowSpan); ref Color paletteBase = ref MemoryMarshal.GetReference(palette.Value.Span); + uint offset = pixelOffset + frameControl.XOffset; - for (nuint x = pixelOffset, o = 0; x < frameControl.XMax; x += increment, o++) + for (nuint x = offset, o = 0; x < frameControl.XMax; x += increment, o++) { uint index = Unsafe.Add(ref scanlineSpanRef, o); pixel.FromRgba32(Unsafe.Add(ref paletteBase, index).ToRgba32()); From 63b7ecd87e1d6659874123ac7c8e5baefdc3dc53 Mon Sep 17 00:00:00 2001 From: SpaceCheetah Date: Thu, 28 Mar 2024 17:49:33 -1000 Subject: [PATCH 10/26] Fix frame dispose operation handling --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 25 +++++++++++-------- .../00.png | 4 +-- .../01.png | 4 +-- .../02.png | 4 +-- .../03.png | 4 +-- .../04.png | 4 +-- 6 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index a96c53c10..caa104c6e 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -240,8 +240,13 @@ internal sealed class PngDecoderCore : IImageDecoderInternals currentFrameControl.Value, cancellationToken); - previousFrame = currentFrame; - previousFrameControl = currentFrameControl; + // if current frame dispose is restore to previous, then from future frame's perspective, it never happened + if (currentFrameControl.Value.DisposeOperation != PngDisposalMethod.RestoreToPrevious) + { + previousFrame = currentFrame; + previousFrameControl = currentFrameControl; + } + break; case PngChunkType.Data: @@ -639,18 +644,18 @@ internal sealed class PngDecoderCore : IImageDecoderInternals out ImageFrame frame) where TPixel : unmanaged, IPixel { - // We create a clone of the previous frame and add it. - // We will overpaint the difference of pixels on the current frame to create a complete image. - // This ensures that we have enough pixel data to process without distortion. #2450 frame = image.Frames.AddFrame(previousFrame ?? image.Frames.RootFrame); - // If the first `fcTL` chunk uses a `dispose_op` of APNG_DISPOSE_OP_PREVIOUS it should be treated as APNG_DISPOSE_OP_BACKGROUND. - if (previousFrameControl.DisposeOperation == PngDisposalMethod.RestoreToBackground - || (previousFrame is null && previousFrameControl.DisposeOperation == PngDisposalMethod.RestoreToPrevious)) + // if restoring to before first frame, restore to background + if (previousFrame is null && previousFrameControl.DisposeOperation == PngDisposalMethod.RestoreToPrevious) + { + Buffer2DRegion pixelRegion = frame.PixelBuffer.GetRegion(); + pixelRegion.Clear(); + } + else if (previousFrameControl.DisposeOperation == PngDisposalMethod.RestoreToBackground) { Rectangle restoreArea = previousFrameControl.Bounds; - Rectangle interest = Rectangle.Intersect(frame.Bounds(), restoreArea); - Buffer2DRegion pixelRegion = frame.PixelBuffer.GetRegion(interest); + Buffer2DRegion pixelRegion = frame.PixelBuffer.GetRegion(restoreArea); pixelRegion.Clear(); } diff --git a/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/00.png b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/00.png index b9fa24c93..870ed61a4 100644 --- a/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/00.png +++ b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/00.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:84e2353264e3488122f4d488d7c4b198ff5192ad0c662c7fb0a369c957ecc7ea -size 353 +oid sha256:b85aaf7153e0ca538856a58d7b069bcc13fadc468ea603c85f8782cc691f86c3 +size 387 diff --git a/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/01.png b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/01.png index 6f3a27187..cab85d946 100644 --- a/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/01.png +++ b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/01.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:2916711d3f4d72eb66a5cfc2b40a3318eb4cce5b367658cfc7e3b573fd39cc33 -size 693 +oid sha256:fcb83d6893dcfd869b764ff9846c259eaa0caf26cec3f0fc2cbae2c26f2eeaa5 +size 660 diff --git a/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/02.png b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/02.png index 50911cce5..1a2c5adcf 100644 --- a/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/02.png +++ b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/02.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:7f9d5503414ccefa6b66661b1e93c2c3f6e4491f14af006a71153cecf43b52f5 -size 806 +oid sha256:562ec382f6d2af68e66092bf6949f66147d5f608d3c618eea5a7c1ea400737ff +size 768 diff --git a/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/03.png b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/03.png index 89d2f9570..d850459ee 100644 --- a/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/03.png +++ b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/03.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:fe42b7dc6524d5589ad680650f4bcd181319b40b258b31e0932d6e936818e980 -size 570 +oid sha256:d12a7791b960072e32b78bd9aaf456dc99341eea1c66ea05050433d8c082c6ac +size 579 diff --git a/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/04.png b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/04.png index c3f2b99b8..000b0567d 100644 --- a/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/04.png +++ b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_frame-offset.png/04.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:8002ff5b3451b348f285eec15dd7a093c62d11d8b77c3ead9ac89ca6eb29977d -size 669 +oid sha256:2db38d7ffcc95c23a5c94a06f10c6cc67406ae581a955c99ede4af97b1a044f8 +size 628 From d15a2e79a4fbfbaa150e436e6afa938da131f05d Mon Sep 17 00:00:00 2001 From: SpaceCheetah Date: Thu, 28 Mar 2024 18:14:23 -1000 Subject: [PATCH 11/26] Add test for default image not animated --- tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs | 3 ++- tests/ImageSharp.Tests/TestImages.cs | 1 + .../00.png | 3 +++ .../01.png | 3 +++ tests/Images/Input/Png/animated/default-not-animated.png | 3 +++ 5 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_default-not-animated.png/00.png create mode 100644 tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_default-not-animated.png/01.png create mode 100644 tests/Images/Input/Png/animated/default-not-animated.png diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 336b89376..9ff368d4e 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -88,7 +88,8 @@ public partial class PngDecoderTests TestImages.Png.DisposePreviousFirst, TestImages.Png.DisposeBackgroundBeforeRegion, TestImages.Png.BlendOverMultiple, - TestImages.Png.FrameOffset + TestImages.Png.FrameOffset, + TestImages.Png.DefaultNotAnimated }; [Theory] diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 7e6ac9e8d..afb13363d 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -74,6 +74,7 @@ public static class TestImages public const string DisposePreviousFirst = "Png/animated/12-dispose-prev-first.png"; public const string BlendOverMultiple = "Png/animated/21-blend-over-multiple.png"; public const string FrameOffset = "Png/animated/frame-offset.png"; + public const string DefaultNotAnimated = "Png/animated/default-not-animated.png"; public const string Issue2666 = "Png/issues/Issue_2666.png"; // Filtered test images from http://www.schaik.com/pngsuite/pngsuite_fil_png.html diff --git a/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_default-not-animated.png/00.png b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_default-not-animated.png/00.png new file mode 100644 index 000000000..4c5ea8169 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_default-not-animated.png/00.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:8d4716e18655be53630d6d50daebe8c38e0eedb2432c7a73840b55d1473d5944 +size 1050 diff --git a/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_default-not-animated.png/01.png b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_default-not-animated.png/01.png new file mode 100644 index 000000000..790fe45e4 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/PngDecoderTests/Decode_VerifyAllFrames_Rgba32_default-not-animated.png/01.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:9b5a6d3cf1a777f6b719c2a1cf79bffe2251355d75e6c0f7ce7a973b3d033419 +size 1177 diff --git a/tests/Images/Input/Png/animated/default-not-animated.png b/tests/Images/Input/Png/animated/default-not-animated.png new file mode 100644 index 000000000..1ed72698d --- /dev/null +++ b/tests/Images/Input/Png/animated/default-not-animated.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:647d484c8f320b55824b9219270524df3edc434a4793e1627e0ee14af8d6e4f8 +size 1689 From 9b8ef108d41877464418a16451aac87eec37c8bb Mon Sep 17 00:00:00 2001 From: SpaceCheetah Date: Thu, 28 Mar 2024 18:28:03 -1000 Subject: [PATCH 12/26] Fix handling of case where default image isn't animated --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 22 +++++++++++--------- src/ImageSharp/Formats/Png/PngMetadata.cs | 5 +++++ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index caa104c6e..3bc2ccf49 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -228,8 +228,7 @@ internal sealed class PngDecoderCore : IImageDecoderInternals PngThrowHelper.ThrowMissingFrameControl(); } - previousFrameControl ??= new((uint)this.header.Width, (uint)this.header.Height); - this.InitializeFrame(previousFrameControl.Value, currentFrameControl.Value, image, previousFrame, out currentFrame); + this.InitializeFrame(previousFrameControl, currentFrameControl.Value, image, previousFrame, out currentFrame); this.currentStream.Position += 4; this.ReadScanlines( @@ -249,7 +248,7 @@ internal sealed class PngDecoderCore : IImageDecoderInternals break; case PngChunkType.Data: - + pngMetadata.DefaultImageAnimated = currentFrameControl != null; currentFrameControl ??= new((uint)this.header.Width, (uint)this.header.Height); if (image is null) { @@ -266,9 +265,12 @@ internal sealed class PngDecoderCore : IImageDecoderInternals this.ReadNextDataChunk, currentFrameControl.Value, cancellationToken); + if (pngMetadata.DefaultImageAnimated) + { + previousFrame = currentFrame; + previousFrameControl = currentFrameControl; + } - previousFrame = currentFrame; - previousFrameControl = currentFrameControl; break; case PngChunkType.Palette: this.palette = chunk.Data.GetSpan().ToArray(); @@ -637,7 +639,7 @@ internal sealed class PngDecoderCore : IImageDecoderInternals /// The previous frame. /// The created frame private void InitializeFrame( - FrameControl previousFrameControl, + FrameControl? previousFrameControl, FrameControl currentFrameControl, Image image, ImageFrame? previousFrame, @@ -646,15 +648,15 @@ internal sealed class PngDecoderCore : IImageDecoderInternals { frame = image.Frames.AddFrame(previousFrame ?? image.Frames.RootFrame); - // if restoring to before first frame, restore to background - if (previousFrame is null && previousFrameControl.DisposeOperation == PngDisposalMethod.RestoreToPrevious) + // If restoring to before first frame, restore to background. Same if first frame (previousFrameControl null). + if (previousFrameControl == null || (previousFrame is null && previousFrameControl.Value.DisposeOperation == PngDisposalMethod.RestoreToPrevious)) { Buffer2DRegion pixelRegion = frame.PixelBuffer.GetRegion(); pixelRegion.Clear(); } - else if (previousFrameControl.DisposeOperation == PngDisposalMethod.RestoreToBackground) + else if (previousFrameControl.Value.DisposeOperation == PngDisposalMethod.RestoreToBackground) { - Rectangle restoreArea = previousFrameControl.Bounds; + Rectangle restoreArea = previousFrameControl.Value.Bounds; Buffer2DRegion pixelRegion = frame.PixelBuffer.GetRegion(restoreArea); pixelRegion.Clear(); } diff --git a/src/ImageSharp/Formats/Png/PngMetadata.cs b/src/ImageSharp/Formats/Png/PngMetadata.cs index 93ddcf263..c4ff3bbe2 100644 --- a/src/ImageSharp/Formats/Png/PngMetadata.cs +++ b/src/ImageSharp/Formats/Png/PngMetadata.cs @@ -83,6 +83,11 @@ public class PngMetadata : IDeepCloneable /// public uint RepeatCount { get; set; } = 1; + /// + /// Gets or sets a value indicating whether the default image is shown as part of the animated sequence + /// + public bool DefaultImageAnimated { get; set; } + /// public IDeepCloneable DeepClone() => new PngMetadata(this); From 85fd4311f1caa30bb37c990842f977ed9a1dec81 Mon Sep 17 00:00:00 2001 From: SpaceCheetah Date: Thu, 28 Mar 2024 19:23:35 -1000 Subject: [PATCH 13/26] Fix PngMetadata copy --- src/ImageSharp/Formats/Png/PngMetadata.cs | 3 ++- tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngMetadata.cs b/src/ImageSharp/Formats/Png/PngMetadata.cs index c4ff3bbe2..766377f7c 100644 --- a/src/ImageSharp/Formats/Png/PngMetadata.cs +++ b/src/ImageSharp/Formats/Png/PngMetadata.cs @@ -29,6 +29,7 @@ public class PngMetadata : IDeepCloneable this.InterlaceMethod = other.InterlaceMethod; this.TransparentColor = other.TransparentColor; this.RepeatCount = other.RepeatCount; + this.DefaultImageAnimated = other.DefaultImageAnimated; if (other.ColorTable?.Length > 0) { @@ -86,7 +87,7 @@ public class PngMetadata : IDeepCloneable /// /// Gets or sets a value indicating whether the default image is shown as part of the animated sequence /// - public bool DefaultImageAnimated { get; set; } + public bool DefaultImageAnimated { get; set; } = true; /// public IDeepCloneable DeepClone() => new PngMetadata(this); diff --git a/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs index b3c122a7a..4f9ba9abe 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs @@ -32,7 +32,8 @@ public class PngMetadataTests InterlaceMethod = PngInterlaceMode.Adam7, Gamma = 2, TextData = new List { new PngTextData("name", "value", "foo", "bar") }, - RepeatCount = 123 + RepeatCount = 123, + DefaultImageAnimated = false }; PngMetadata clone = (PngMetadata)meta.DeepClone(); @@ -44,6 +45,7 @@ public class PngMetadataTests Assert.False(meta.TextData.Equals(clone.TextData)); Assert.True(meta.TextData.SequenceEqual(clone.TextData)); Assert.True(meta.RepeatCount == clone.RepeatCount); + Assert.True(meta.DefaultImageAnimated == clone.DefaultImageAnimated); clone.BitDepth = PngBitDepth.Bit2; clone.ColorType = PngColorType.Palette; From addec72467bacd9fc2152549f9ce22e1b6074006 Mon Sep 17 00:00:00 2001 From: SpaceCheetah Date: Thu, 28 Mar 2024 20:11:43 -1000 Subject: [PATCH 14/26] Make PngEncoder respect DefaultImageAnimated --- src/ImageSharp/Formats/Png/PngEncoderCore.cs | 43 +++++++++++++------ .../Formats/Png/PngEncoderTests.cs | 23 ++++++++++ 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index ea94270f2..d1a600737 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -161,6 +161,7 @@ internal sealed class PngEncoderCore : IImageEncoderInternals, IDisposable ImageFrame? clonedFrame = null; ImageFrame currentFrame = image.Frames.RootFrame; + int currentFrameIndex = 0; bool clearTransparency = this.encoder.TransparentColorMode is PngTransparentColorMode.Clear; if (clearTransparency) @@ -190,28 +191,49 @@ internal sealed class PngEncoderCore : IImageEncoderInternals, IDisposable if (image.Frames.Count > 1) { this.WriteAnimationControlChunk(stream, (uint)image.Frames.Count, pngMetadata.RepeatCount); + } + + // If the first frame isn't animated, write it as usual and skip it when writing animated frames + if (!pngMetadata.DefaultImageAnimated || image.Frames.Count == 1) + { + FrameControl frameControl = new((uint)this.width, (uint)this.height); + this.WriteDataChunks(frameControl, currentFrame.PixelBuffer.GetRegion(), quantized, stream, false); + currentFrameIndex++; + } - // Write the first frame. + if (image.Frames.Count > 1) + { + // Write the first animated frame. + currentFrame = image.Frames[currentFrameIndex]; PngFrameMetadata frameMetadata = GetPngFrameMetadata(currentFrame); PngDisposalMethod previousDisposal = frameMetadata.DisposalMethod; FrameControl frameControl = this.WriteFrameControlChunk(stream, frameMetadata, currentFrame.Bounds(), 0); - this.WriteDataChunks(frameControl, currentFrame.PixelBuffer.GetRegion(), quantized, stream, false); + uint sequenceNumber = 1; + if (pngMetadata.DefaultImageAnimated) + { + this.WriteDataChunks(frameControl, currentFrame.PixelBuffer.GetRegion(), quantized, stream, false); + } + else + { + sequenceNumber += this.WriteDataChunks(frameControl, currentFrame.PixelBuffer.GetRegion(), quantized, stream, true); + } + + currentFrameIndex++; // Capture the global palette for reuse on subsequent frames. ReadOnlyMemory? previousPalette = quantized?.Palette.ToArray(); // Write following frames. - uint increment = 0; ImageFrame previousFrame = image.Frames.RootFrame; // This frame is reused to store de-duplicated pixel buffers. using ImageFrame encodingFrame = new(image.Configuration, previousFrame.Size()); - for (int i = 1; i < image.Frames.Count; i++) + for (; currentFrameIndex < image.Frames.Count; currentFrameIndex++) { ImageFrame? prev = previousDisposal == PngDisposalMethod.RestoreToBackground ? null : previousFrame; - currentFrame = image.Frames[i]; - ImageFrame? nextFrame = i < image.Frames.Count - 1 ? image.Frames[i + 1] : null; + currentFrame = image.Frames[currentFrameIndex]; + ImageFrame? nextFrame = currentFrameIndex < image.Frames.Count - 1 ? image.Frames[currentFrameIndex + 1] : null; frameMetadata = GetPngFrameMetadata(currentFrame); bool blend = frameMetadata.BlendMethod == PngBlendMethod.Over; @@ -232,22 +254,17 @@ internal sealed class PngEncoderCore : IImageEncoderInternals, IDisposable } // Each frame control sequence number must be incremented by the number of frame data chunks that follow. - frameControl = this.WriteFrameControlChunk(stream, frameMetadata, bounds, (uint)i + increment); + frameControl = this.WriteFrameControlChunk(stream, frameMetadata, bounds, sequenceNumber); // Dispose of previous quantized frame and reassign. quantized?.Dispose(); quantized = this.CreateQuantizedImageAndUpdateBitDepth(pngMetadata, encodingFrame, bounds, previousPalette); - increment += this.WriteDataChunks(frameControl, encodingFrame.PixelBuffer.GetRegion(bounds), quantized, stream, true); + sequenceNumber += this.WriteDataChunks(frameControl, encodingFrame.PixelBuffer.GetRegion(bounds), quantized, stream, true) + 1; previousFrame = currentFrame; previousDisposal = frameMetadata.DisposalMethod; } } - else - { - FrameControl frameControl = new((uint)this.width, (uint)this.height); - this.WriteDataChunks(frameControl, currentFrame.PixelBuffer.GetRegion(), quantized, stream, false); - } this.WriteEndChunk(stream); diff --git a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs index 950f1d2e3..bbd7ff4e4 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs @@ -586,6 +586,29 @@ public partial class PngEncoderTests } } + [Theory] + [WithFile(TestImages.Png.DefaultNotAnimated, PixelTypes.Rgba32)] + public void Encode_DefaultNotAnimated(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(PngDecoder.Instance); + using MemoryStream memStream = new(); + image.Save(memStream, PngEncoder); + memStream.Position = 0; + + image.DebugSave(provider: provider, encoder: PngEncoder, null, false); + + using Image output = Image.Load(memStream); + ImageComparer.Exact.VerifySimilarity(output, image); + + Assert.Equal(2, image.Frames.Count); + Assert.Equal(image.Frames.Count, output.Frames.Count); + + PngMetadata originalMetadata = image.Metadata.GetPngMetadata(); + PngMetadata outputMetadata = output.Metadata.GetPngMetadata(); + Assert.Equal(originalMetadata.DefaultImageAnimated, outputMetadata.DefaultImageAnimated); + } + [Theory] [MemberData(nameof(PngTrnsFiles))] public void Encode_PreserveTrns(string imagePath, PngBitDepth pngBitDepth, PngColorType pngColorType) From 9ae8be4c8afc28f6c648f913b558ba23e6a8e191 Mon Sep 17 00:00:00 2001 From: SpaceCheetah Date: Fri, 29 Mar 2024 21:34:48 -1000 Subject: [PATCH 15/26] Add more tests --- .../Formats/Png/PngEncoderTests.Chunks.cs | 33 +++++++++++++++++++ .../Formats/Png/PngMetadataTests.cs | 20 +++++++++++ 2 files changed, 53 insertions(+) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs index 044da2193..2e3390298 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs @@ -3,6 +3,7 @@ using System.Buffers.Binary; using SixLabors.ImageSharp.Formats.Png; +using SixLabors.ImageSharp.Formats.Png.Chunks; using SixLabors.ImageSharp.PixelFormats; // ReSharper disable InconsistentNaming @@ -59,6 +60,38 @@ public partial class PngEncoderTests } } + [Theory] + [WithFile(TestImages.Png.DefaultNotAnimated, PixelTypes.Rgba32)] + [WithFile(TestImages.Png.APng, PixelTypes.Rgba32)] + public void AcTL_CorrectlyWritten(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(PngDecoder.Instance); + PngMetadata metadata = image.Metadata.GetPngMetadata(); + int correctFrameCount = image.Frames.Count - (metadata.DefaultImageAnimated ? 0 : 1); + using MemoryStream memStream = new(); + image.Save(memStream, PngEncoder); + memStream.Position = 0; + Span bytesSpan = memStream.ToArray().AsSpan(8); // Skip header. + bool foundAcTl = false; + while (bytesSpan.Length > 0 && !foundAcTl) + { + int length = BinaryPrimitives.ReadInt32BigEndian(bytesSpan[..4]); + PngChunkType type = (PngChunkType)BinaryPrimitives.ReadInt32BigEndian(bytesSpan.Slice(4, 4)); + if (type == PngChunkType.AnimationControl) + { + AnimationControl control = AnimationControl.Parse(bytesSpan[8..]); + foundAcTl = true; + Assert.True(control.NumberFrames == correctFrameCount); + Assert.True(control.NumberPlays == metadata.RepeatCount); + } + + bytesSpan = bytesSpan[(4 + 4 + length + 4)..]; + } + + Assert.True(foundAcTl); + } + [Theory] [InlineData(PngChunkType.Gamma)] [InlineData(PngChunkType.Chroma)] diff --git a/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs index 4f9ba9abe..8308935d0 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs @@ -146,6 +146,26 @@ public class PngMetadataTests VerifyExifDataIsPresent(exif); } + [Theory] + [WithFile(TestImages.Png.DefaultNotAnimated, PixelTypes.Rgba32)] + public void Decode_IdentifiesDefaultFrameNotAnimated(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(PngDecoder.Instance); + PngMetadata meta = image.Metadata.GetFormatMetadata(PngFormat.Instance); + Assert.False(meta.DefaultImageAnimated); + } + + [Theory] + [WithFile(TestImages.Png.APng, PixelTypes.Rgba32)] + public void Decode_IdentifiesDefaultFrameAnimated(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(PngDecoder.Instance); + PngMetadata meta = image.Metadata.GetFormatMetadata(PngFormat.Instance); + Assert.True(meta.DefaultImageAnimated); + } + [Theory] [WithFile(TestImages.Png.PngWithMetadata, PixelTypes.Rgba32)] public void Decode_IgnoresExifData_WhenIgnoreMetadataIsTrue(TestImageProvider provider) From ceb7dc0e7e26ee289cb8e2743ceb5b6793f65c41 Mon Sep 17 00:00:00 2001 From: SpaceCheetah Date: Fri, 29 Mar 2024 21:35:53 -1000 Subject: [PATCH 16/26] Fix incorrect acTL frame count --- src/ImageSharp/Formats/Png/PngEncoderCore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index d1a600737..e0f7ed948 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -190,7 +190,7 @@ internal sealed class PngEncoderCore : IImageEncoderInternals, IDisposable if (image.Frames.Count > 1) { - this.WriteAnimationControlChunk(stream, (uint)image.Frames.Count, pngMetadata.RepeatCount); + this.WriteAnimationControlChunk(stream, (uint)(image.Frames.Count - (pngMetadata.DefaultImageAnimated ? 0 : 1)), pngMetadata.RepeatCount); } // If the first frame isn't animated, write it as usual and skip it when writing animated frames From 4698e8ca3b4a6a453a45cace6554f40148c53d3e Mon Sep 17 00:00:00 2001 From: SpaceCheetah Date: Mon, 1 Apr 2024 17:03:25 -0700 Subject: [PATCH 17/26] re-add comment --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 3bc2ccf49..dd2ce9eb1 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -648,7 +648,8 @@ internal sealed class PngDecoderCore : IImageDecoderInternals { frame = image.Frames.AddFrame(previousFrame ?? image.Frames.RootFrame); - // If restoring to before first frame, restore to background. Same if first frame (previousFrameControl null). + // If the first `fcTL` chunk uses a `dispose_op` of APNG_DISPOSE_OP_PREVIOUS it should be treated as APNG_DISPOSE_OP_BACKGROUND. + // So, if restoring to before first frame, clear entire area. Same if first frame (previousFrameControl null). if (previousFrameControl == null || (previousFrame is null && previousFrameControl.Value.DisposeOperation == PngDisposalMethod.RestoreToPrevious)) { Buffer2DRegion pixelRegion = frame.PixelBuffer.GetRegion(); From b1a5593b20e89786343d5d7d71a2c08dd606b1e9 Mon Sep 17 00:00:00 2001 From: SpaceCheetah Date: Mon, 1 Apr 2024 17:28:27 -0700 Subject: [PATCH 18/26] Add test for case with frame offsets --- .../Formats/Png/PngEncoderTests.cs | 31 ++++--------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs index bbd7ff4e4..6c4047be8 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs @@ -447,6 +447,8 @@ public partial class PngEncoderTests [Theory] [WithFile(TestImages.Png.APng, PixelTypes.Rgba32)] + [WithFile(TestImages.Png.DefaultNotAnimated, PixelTypes.Rgba32)] + [WithFile(TestImages.Png.FrameOffset, PixelTypes.Rgba32)] public void Encode_APng(TestImageProvider provider) where TPixel : unmanaged, IPixel { @@ -458,15 +460,17 @@ public partial class PngEncoderTests image.DebugSave(provider: provider, encoder: PngEncoder, null, false); using Image output = Image.Load(memStream); - ImageComparer.Exact.VerifySimilarity(output, image); - Assert.Equal(5, image.Frames.Count); + // some loss from original, due to compositing + ImageComparer.TolerantPercentage(0.01f).VerifySimilarity(output, image); + Assert.Equal(image.Frames.Count, output.Frames.Count); PngMetadata originalMetadata = image.Metadata.GetPngMetadata(); PngMetadata outputMetadata = output.Metadata.GetPngMetadata(); Assert.Equal(originalMetadata.RepeatCount, outputMetadata.RepeatCount); + Assert.Equal(originalMetadata.DefaultImageAnimated, outputMetadata.DefaultImageAnimated); for (int i = 0; i < image.Frames.Count; i++) { @@ -586,29 +590,6 @@ public partial class PngEncoderTests } } - [Theory] - [WithFile(TestImages.Png.DefaultNotAnimated, PixelTypes.Rgba32)] - public void Encode_DefaultNotAnimated(TestImageProvider provider) - where TPixel : unmanaged, IPixel - { - using Image image = provider.GetImage(PngDecoder.Instance); - using MemoryStream memStream = new(); - image.Save(memStream, PngEncoder); - memStream.Position = 0; - - image.DebugSave(provider: provider, encoder: PngEncoder, null, false); - - using Image output = Image.Load(memStream); - ImageComparer.Exact.VerifySimilarity(output, image); - - Assert.Equal(2, image.Frames.Count); - Assert.Equal(image.Frames.Count, output.Frames.Count); - - PngMetadata originalMetadata = image.Metadata.GetPngMetadata(); - PngMetadata outputMetadata = output.Metadata.GetPngMetadata(); - Assert.Equal(originalMetadata.DefaultImageAnimated, outputMetadata.DefaultImageAnimated); - } - [Theory] [MemberData(nameof(PngTrnsFiles))] public void Encode_PreserveTrns(string imagePath, PngBitDepth pngBitDepth, PngColorType pngColorType) From 848013921281a7381ea4328f4f3d37f3587b01bf Mon Sep 17 00:00:00 2001 From: SpaceCheetah Date: Mon, 1 Apr 2024 17:56:21 -0700 Subject: [PATCH 19/26] re-add rest of comment --- 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 dd2ce9eb1..1afb65566 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -646,6 +646,9 @@ internal sealed class PngDecoderCore : IImageDecoderInternals out ImageFrame frame) where TPixel : unmanaged, IPixel { + // We create a clone of the previous frame and add it. + // We will overpaint the difference of pixels on the current frame to create a complete image. + // This ensures that we have enough pixel data to process without distortion. #2450 frame = image.Frames.AddFrame(previousFrame ?? image.Frames.RootFrame); // If the first `fcTL` chunk uses a `dispose_op` of APNG_DISPOSE_OP_PREVIOUS it should be treated as APNG_DISPOSE_OP_BACKGROUND. From 63d4b2028a370debe463077fb7fa7fe8469676f7 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 3 Apr 2024 15:24:05 +1000 Subject: [PATCH 20/26] Fix args and add tests --- .../ComponentProcessors/ComponentProcessor.cs | 2 +- .../Components/Encoder/ComponentProcessor.cs | 2 +- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 3 ++ src/ImageSharp/Formats/Tga/TgaDecoderCore.cs | 2 +- .../Memory/Allocators/MemoryAllocator.cs | 21 +++++--------- .../Allocators/MemoryAllocatorOptions.cs | 4 +-- .../Allocators/SimpleGcMemoryAllocator.cs | 10 ++++--- ...iformUnmanagedMemoryPoolMemoryAllocator.cs | 14 +++++---- .../DiscontiguousBuffers/MemoryGroup{T}.cs | 13 +++++---- .../Memory/InvalidMemoryOperationException.cs | 11 ++++++- .../Memory/MemoryAllocatorExtensions.cs | 14 ++++----- .../Transforms/Resize/ResizeKernelMap.cs | 2 +- .../Transforms/Resize/ResizeWorker.cs | 2 +- .../Formats/Bmp/BmpDecoderTests.cs | 9 ++++++ .../Formats/Jpg/JpegDecoderTests.cs | 18 +++--------- .../SimpleGcMemoryAllocatorTests.cs | 16 ++++++---- ...niformUnmanagedPoolMemoryAllocatorTests.cs | 13 ++++++++- .../ImageSharp.Tests/Memory/Buffer2DTests.cs | 29 +++++++++++++++++-- .../MemoryGroupTests.Allocate.cs | 12 ++++++-- tests/ImageSharp.Tests/TestImages.cs | 2 ++ tests/Images/Input/Bmp/issue-2696.bmp | 3 ++ 21 files changed, 131 insertions(+), 71 deletions(-) create mode 100644 tests/Images/Input/Bmp/issue-2696.bmp diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs index f0cc4d5e8..3bdaf2dc2 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs @@ -16,7 +16,7 @@ internal abstract class ComponentProcessor : IDisposable this.Component = component; this.BlockAreaSize = component.SubSamplingDivisors * blockSize; - this.ColorBuffer = memoryAllocator.Allocate2DOveraligned( + this.ColorBuffer = memoryAllocator.Allocate2DOverAligned( postProcessorBufferSize.Width, postProcessorBufferSize.Height, this.BlockAreaSize.Height); diff --git a/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs b/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs index c33a8a196..9346b11e7 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs @@ -28,7 +28,7 @@ internal class ComponentProcessor : IDisposable this.blockAreaSize = component.SubSamplingDivisors * 8; // alignment of 8 so each block stride can be sampled from a single 'ref pointer' - this.ColorBuffer = memoryAllocator.Allocate2DOveraligned( + this.ColorBuffer = memoryAllocator.Allocate2DOverAligned( postProcessorBufferSize.Width, postProcessorBufferSize.Height, 8, diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index a96c53c10..abd5cadeb 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -1968,6 +1968,9 @@ internal sealed class PngDecoderCore : IImageDecoderInternals } // We rent the buffer here to return it afterwards in Decode() + // We don't want to throw a degenerated memory exception here as we want to allow partial decoding + // so limit the length. + length = (int)Math.Min(length, this.currentStream.Length - this.currentStream.Position); IMemoryOwner buffer = this.configuration.MemoryAllocator.Allocate(length, AllocationOptions.Clean); this.currentStream.Read(buffer.GetSpan(), 0, length); diff --git a/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs b/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs index 26e057bff..34f4c2bcf 100644 --- a/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs +++ b/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs @@ -84,7 +84,7 @@ internal sealed class TgaDecoderCore : IImageDecoderInternals throw new UnknownImageFormatException("Width or height cannot be 0"); } - Image image = Image.CreateUninitialized(this.configuration, this.fileHeader.Width, this.fileHeader.Height, this.metadata); + Image image = new(this.configuration, this.fileHeader.Width, this.fileHeader.Height, this.metadata); Buffer2D pixels = image.GetRootFramePixelBuffer(); if (this.fileHeader.ColorMapType == 1) diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs index 194449cfc..b56bedd04 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs @@ -2,7 +2,6 @@ // Licensed under the Six Labors Split License. using System.Buffers; -using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; namespace SixLabors.ImageSharp.Memory; @@ -24,9 +23,7 @@ public abstract class MemoryAllocator /// public static MemoryAllocator Default { get; } = Create(); - internal long MemoryGroupAllocationLimitBytes { get; private set; } = Environment.Is64BitProcess ? - 4L * OneGigabyte : - OneGigabyte; + internal long MemoryGroupAllocationLimitBytes { get; private set; } = Environment.Is64BitProcess ? 4L * OneGigabyte : OneGigabyte; internal int SingleBufferAllocationLimitBytes { get; private set; } = OneGigabyte; @@ -82,6 +79,7 @@ public abstract class MemoryAllocator /// /// Allocates a . /// + /// The type of element to allocate. /// The total length of the buffer. /// The expected alignment (eg. to make sure image rows fit into single buffers). /// The . @@ -93,22 +91,19 @@ public abstract class MemoryAllocator AllocationOptions options = AllocationOptions.None) where T : struct { - long totalLengthInBytes = totalLength * Unsafe.SizeOf(); - if (totalLengthInBytes < 0) + if (totalLength < 0) { - ThrowNotRepresentable(); + InvalidMemoryOperationException.ThrowNegativeAllocationException(totalLength); } - if (totalLengthInBytes > this.MemoryGroupAllocationLimitBytes) + ulong totalLengthInBytes = (ulong)totalLength * (ulong)Unsafe.SizeOf(); + if (totalLengthInBytes > (ulong)this.MemoryGroupAllocationLimitBytes) { InvalidMemoryOperationException.ThrowAllocationOverLimitException(totalLengthInBytes, this.MemoryGroupAllocationLimitBytes); } - return this.AllocateGroupCore(totalLengthInBytes, totalLength, bufferAlignment, options); - - [DoesNotReturn] - static void ThrowNotRepresentable() => - throw new InvalidMemoryOperationException("Attempted to allocate a MemoryGroup of a size that is not representable."); + // Cast to long is safe because we already checked that the total length is within the limit. + return this.AllocateGroupCore(totalLength, (long)totalLengthInBytes, bufferAlignment, options); } internal virtual MemoryGroup AllocateGroupCore(long totalLengthInElements, long totalLengthInBytes, int bufferAlignment, AllocationOptions options) diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs index d9ba62c1e..9f2581660 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs @@ -17,7 +17,7 @@ public struct MemoryAllocatorOptions /// public int? MaximumPoolSizeMegabytes { - get => this.maximumPoolSizeMegabytes; + readonly get => this.maximumPoolSizeMegabytes; set { if (value.HasValue) @@ -35,7 +35,7 @@ public struct MemoryAllocatorOptions /// public int? AllocationLimitMegabytes { - get => this.allocationLimitMegabytes; + readonly get => this.allocationLimitMegabytes; set { if (value.HasValue) diff --git a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs index e60462195..675afe8b9 100644 --- a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs @@ -18,11 +18,13 @@ public sealed class SimpleGcMemoryAllocator : MemoryAllocator /// public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length)); - - int lengthInBytes = length * Unsafe.SizeOf(); + if (length < 0) + { + InvalidMemoryOperationException.ThrowNegativeAllocationException(length); + } - if (lengthInBytes > this.SingleBufferAllocationLimitBytes) + ulong lengthInBytes = (ulong)length * (ulong)Unsafe.SizeOf(); + if (lengthInBytes > (ulong)this.SingleBufferAllocationLimitBytes) { InvalidMemoryOperationException.ThrowAllocationOverLimitException(lengthInBytes, this.SingleBufferAllocationLimitBytes); } diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index 585d4717a..621073a3d 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -2,7 +2,6 @@ // Licensed under the Six Labors Split License. using System.Buffers; -using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using SixLabors.ImageSharp.Memory.Internals; @@ -84,15 +83,18 @@ internal sealed class UniformUnmanagedMemoryPoolMemoryAllocator : MemoryAllocato int length, AllocationOptions options = AllocationOptions.None) { - Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length)); - int lengthInBytes = length * Unsafe.SizeOf(); + if (length < 0) + { + InvalidMemoryOperationException.ThrowNegativeAllocationException(length); + } - if (lengthInBytes > this.SingleBufferAllocationLimitBytes) + ulong lengthInBytes = (ulong)length * (ulong)Unsafe.SizeOf(); + if (lengthInBytes > (ulong)this.SingleBufferAllocationLimitBytes) { InvalidMemoryOperationException.ThrowAllocationOverLimitException(lengthInBytes, this.SingleBufferAllocationLimitBytes); } - if (lengthInBytes <= this.sharedArrayPoolThresholdInBytes) + if (lengthInBytes <= (ulong)this.sharedArrayPoolThresholdInBytes) { var buffer = new SharedArrayPoolBuffer(length); if (options.Has(AllocationOptions.Clean)) @@ -103,7 +105,7 @@ internal sealed class UniformUnmanagedMemoryPoolMemoryAllocator : MemoryAllocato return buffer; } - if (lengthInBytes <= this.poolBufferSizeInBytes) + if (lengthInBytes <= (ulong)this.poolBufferSizeInBytes) { UnmanagedMemoryHandle mem = this.pool.Rent(); if (mem.IsValid) diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs index 9695c7f98..03c29a723 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs @@ -83,15 +83,16 @@ internal abstract partial class MemoryGroup : IMemoryGroup, IDisposable { int bufferCapacityInBytes = allocator.GetBufferCapacityInBytes(); Guard.NotNull(allocator, nameof(allocator)); - Guard.MustBeGreaterThanOrEqualTo(totalLengthInElements, 0, nameof(totalLengthInElements)); - Guard.MustBeGreaterThanOrEqualTo(bufferAlignmentInElements, 0, nameof(bufferAlignmentInElements)); - int blockCapacityInElements = bufferCapacityInBytes / ElementSize; + if (totalLengthInElements < 0) + { + InvalidMemoryOperationException.ThrowNegativeAllocationException(totalLengthInElements); + } - if (bufferAlignmentInElements > blockCapacityInElements) + int blockCapacityInElements = bufferCapacityInBytes / ElementSize; + if (bufferAlignmentInElements < 0 || bufferAlignmentInElements > blockCapacityInElements) { - throw new InvalidMemoryOperationException( - $"The buffer capacity of the provided MemoryAllocator is insufficient for the requested buffer alignment: {bufferAlignmentInElements}."); + InvalidMemoryOperationException.ThrowInvalidAlignmentException(bufferAlignmentInElements); } if (totalLengthInElements == 0) diff --git a/src/ImageSharp/Memory/InvalidMemoryOperationException.cs b/src/ImageSharp/Memory/InvalidMemoryOperationException.cs index 0dc8106b6..81210f13d 100644 --- a/src/ImageSharp/Memory/InvalidMemoryOperationException.cs +++ b/src/ImageSharp/Memory/InvalidMemoryOperationException.cs @@ -28,6 +28,15 @@ public class InvalidMemoryOperationException : InvalidOperationException } [DoesNotReturn] - internal static void ThrowAllocationOverLimitException(long length, long limit) => + internal static void ThrowNegativeAllocationException(long length) => + throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of negative length={length}."); + + [DoesNotReturn] + internal static void ThrowInvalidAlignmentException(long alignment) => + throw new InvalidMemoryOperationException( + $"The buffer capacity of the provided MemoryAllocator is insufficient for the requested buffer alignment: {alignment}."); + + [DoesNotReturn] + internal static void ThrowAllocationOverLimitException(ulong length, long limit) => throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of length={length} that exceeded the limit {limit}."); } diff --git a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs index ff306e1e4..e0dc8aab3 100644 --- a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs +++ b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs @@ -18,20 +18,20 @@ public static class MemoryAllocatorExtensions /// The memory allocator. /// The buffer width. /// The buffer height. - /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . + /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . /// The allocation options. /// The . public static Buffer2D Allocate2D( this MemoryAllocator memoryAllocator, int width, int height, - bool preferContiguosImageBuffers, + bool preferContiguousImageBuffers, AllocationOptions options = AllocationOptions.None) where T : struct { long groupLength = (long)width * height; MemoryGroup memoryGroup; - if (preferContiguosImageBuffers && groupLength < int.MaxValue) + if (preferContiguousImageBuffers && groupLength < int.MaxValue) { IMemoryOwner buffer = memoryAllocator.Allocate((int)groupLength, options); memoryGroup = MemoryGroup.CreateContiguous(buffer, false); @@ -69,16 +69,16 @@ public static class MemoryAllocatorExtensions /// The type of buffer items to allocate. /// The memory allocator. /// The buffer size. - /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . + /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . /// The allocation options. /// The . public static Buffer2D Allocate2D( this MemoryAllocator memoryAllocator, Size size, - bool preferContiguosImageBuffers, + bool preferContiguousImageBuffers, AllocationOptions options = AllocationOptions.None) where T : struct => - Allocate2D(memoryAllocator, size.Width, size.Height, preferContiguosImageBuffers, options); + Allocate2D(memoryAllocator, size.Width, size.Height, preferContiguousImageBuffers, options); /// /// Allocates a buffer of value type objects interpreted as a 2D region @@ -96,7 +96,7 @@ public static class MemoryAllocatorExtensions where T : struct => Allocate2D(memoryAllocator, size.Width, size.Height, false, options); - internal static Buffer2D Allocate2DOveraligned( + internal static Buffer2D Allocate2DOverAligned( this MemoryAllocator memoryAllocator, int width, int height, diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs index c1907bb52..da26cbefc 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs @@ -50,7 +50,7 @@ internal partial class ResizeKernelMap : IDisposable this.sourceLength = sourceLength; this.DestinationLength = destinationLength; this.MaxDiameter = (radius * 2) + 1; - this.data = memoryAllocator.Allocate2D(this.MaxDiameter, bufferHeight, preferContiguosImageBuffers: true, AllocationOptions.Clean); + this.data = memoryAllocator.Allocate2D(this.MaxDiameter, bufferHeight, preferContiguousImageBuffers: true, AllocationOptions.Clean); this.pinHandle = this.data.DangerousGetSingleMemory().Pin(); this.kernels = new ResizeKernel[destinationLength]; this.tempValues = new double[this.MaxDiameter]; diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs index cce27a401..fd13d7b5a 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs @@ -83,7 +83,7 @@ internal sealed class ResizeWorker : IDisposable this.transposedFirstPassBuffer = configuration.MemoryAllocator.Allocate2D( this.workerHeight, targetWorkingRect.Width, - preferContiguosImageBuffers: true, + preferContiguousImageBuffers: true, options: AllocationOptions.Clean); this.tempRowBuffer = configuration.MemoryAllocator.Allocate(this.sourceRectangle.Width); diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs index e76f21d0e..be2fa4b68 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs @@ -558,4 +558,13 @@ public class BmpDecoderTests // Compare to reference output instead. image.CompareToReferenceOutput(provider, extension: "png"); } + + [Theory] + [WithFile(Issue2696, PixelTypes.Rgba32)] + public void BmpDecoder_ThrowsException_Issue2696(TestImageProvider provider) + where TPixel : unmanaged, IPixel + => Assert.Throws(() => + { + using Image image = provider.GetImage(BmpDecoder.Instance); + }); } diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 6a94a98ac..2fe428260 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -338,21 +338,11 @@ public partial class JpegDecoderTests } [Theory] - [WithFile(TestImages.Jpeg.Issues.HangBadScan, PixelTypes.L8)] - public void DecodeHang(TestImageProvider provider) + [WithFile(TestImages.Jpeg.Issues.HangBadScan, PixelTypes.Rgb24)] + public void DecodeHang_ThrowsException(TestImageProvider provider) where TPixel : unmanaged, IPixel - { - if (TestEnvironment.IsWindows && - TestEnvironment.RunsOnCI) - { - // Windows CI runs consistently fail with OOM. - return; - } - - using Image image = provider.GetImage(JpegDecoder.Instance); - Assert.Equal(65503, image.Width); - Assert.Equal(65503, image.Height); - } + => Assert.Throws( + () => { using Image image = provider.GetImage(JpegDecoder.Instance); }); // https://github.com/SixLabors/ImageSharp/issues/2517 [Theory] diff --git a/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs index 780ba7f20..665f34a34 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs @@ -19,13 +19,17 @@ public class SimpleGcMemoryAllocatorTests protected SimpleGcMemoryAllocator MemoryAllocator { get; } = new SimpleGcMemoryAllocator(); - [Theory] - [InlineData(-1)] - public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length) + public static TheoryData InvalidLengths { get; set; } = new() { - ArgumentOutOfRangeException ex = Assert.Throws(() => this.MemoryAllocator.Allocate(length)); - Assert.Equal("length", ex.ParamName); - } + { -1 }, + { (1 << 30) + 1 } + }; + + [Theory] + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException(int length) + => Assert.Throws( + () => this.MemoryAllocator.Allocate(length)); [Fact] public unsafe void Allocate_MemoryIsPinnableMultipleTimes() diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index 7e7c13663..6187b536e 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -111,9 +111,20 @@ public class UniformUnmanagedPoolMemoryAllocatorTests public void AllocateGroup_SizeInBytesOverLongMaxValue_ThrowsInvalidMemoryOperationException() { var allocator = new UniformUnmanagedMemoryPoolMemoryAllocator(null); - Assert.Throws(() => allocator.AllocateGroup(int.MaxValue * (long)int.MaxValue, int.MaxValue)); + Assert.Throws(() => allocator.AllocateGroup(int.MaxValue * (long)int.MaxValue, int.MaxValue)); } + public static TheoryData InvalidLengths { get; set; } = new() + { + { -1 }, + { (1 << 30) + 1 } + }; + + [Theory] + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException(int length) + => Assert.Throws(() => new UniformUnmanagedMemoryPoolMemoryAllocator(null).Allocate(length)); + [Fact] public unsafe void Allocate_MemoryIsPinnableMultipleTimes() { diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs index 5364de065..3ef4fc540 100644 --- a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs @@ -4,6 +4,7 @@ using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using SixLabors.ImageSharp.Memory; +using SixLabors.ImageSharp.PixelFormats; // ReSharper disable InconsistentNaming namespace SixLabors.ImageSharp.Tests.Memory; @@ -72,11 +73,11 @@ public partial class Buffer2DTests using Buffer2D buffer = useSizeOverload ? this.MemoryAllocator.Allocate2D( new Size(200, 200), - preferContiguosImageBuffers: true) : + preferContiguousImageBuffers: true) : this.MemoryAllocator.Allocate2D( 200, 200, - preferContiguosImageBuffers: true); + preferContiguousImageBuffers: true); Assert.Equal(1, buffer.FastMemoryGroup.Count); Assert.Equal(200 * 200, buffer.FastMemoryGroup.TotalLength); } @@ -87,7 +88,7 @@ public partial class Buffer2DTests { this.MemoryAllocator.BufferCapacityInBytes = sizeof(int) * bufferCapacity; - using Buffer2D buffer = this.MemoryAllocator.Allocate2DOveraligned(width, height, alignmentMultiplier); + using Buffer2D buffer = this.MemoryAllocator.Allocate2DOverAligned(width, height, alignmentMultiplier); MemoryGroup memoryGroup = buffer.FastMemoryGroup; int expectedAlignment = width * alignmentMultiplier; @@ -337,4 +338,26 @@ public partial class Buffer2DTests Assert.False(mgBefore.IsValid); Assert.NotSame(mgBefore, buffer1.MemoryGroup); } + + public static TheoryData InvalidLengths { get; set; } = new() + { + { new(-1, -1) }, + { new(32768, 32769) }, + { new(32769, 32768) } + }; + + [Theory] + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException(Size size) + => Assert.Throws(() => this.MemoryAllocator.Allocate2D(size.Width, size.Height)); + + [Theory] + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException_Size(Size size) + => Assert.Throws(() => this.MemoryAllocator.Allocate2D(new Size(size))); + + [Theory] + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException_OverAligned(Size size) + => Assert.Throws(() => this.MemoryAllocator.Allocate2DOverAligned(size.Width, size.Height, 1)); } diff --git a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs index 936e482cb..4c7de5412 100644 --- a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs +++ b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Six Labors Split License. using System.Runtime.CompilerServices; @@ -219,11 +219,17 @@ public partial class MemoryGroupTests [StructLayout(LayoutKind.Sequential, Size = 5)] internal struct S5 { - public override string ToString() => "S5"; + public override readonly string ToString() => nameof(S5); } [StructLayout(LayoutKind.Sequential, Size = 4)] internal struct S4 { - public override string ToString() => "S4"; + public override readonly string ToString() => nameof(S4); +} + +[StructLayout(LayoutKind.Explicit, Size = 512)] +internal struct S512 +{ + public override readonly string ToString() => nameof(S512); } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index fe633fddc..42778e6f1 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -439,6 +439,8 @@ public static class TestImages public const string Rgba321010102 = "Bmp/rgba32-1010102.bmp"; public const string RgbaAlphaBitfields = "Bmp/rgba32abf.bmp"; + public const string Issue2696 = "Bmp/issue-2696.bmp"; + public const string BlackWhitePalletDataMatrix = "Bmp/bit1datamatrix.bmp"; public static readonly string[] BitFields = diff --git a/tests/Images/Input/Bmp/issue-2696.bmp b/tests/Images/Input/Bmp/issue-2696.bmp new file mode 100644 index 000000000..4a42e456c --- /dev/null +++ b/tests/Images/Input/Bmp/issue-2696.bmp @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:de7e7ec0454a55f6a76c859f356b240a3d4cb56ca50dfa209a1813dd09e12076 +size 143 From c8eab788dd8d496703ba7c82bd595fe2fea81d83 Mon Sep 17 00:00:00 2001 From: SpaceCheetah Date: Tue, 2 Apr 2024 23:00:15 -0700 Subject: [PATCH 21/26] Rename DefaultImageAnimated to AnimateRootFrame --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 4 ++-- src/ImageSharp/Formats/Png/PngEncoderCore.cs | 6 +++--- src/ImageSharp/Formats/Png/PngMetadata.cs | 6 +++--- .../Formats/Png/PngEncoderTests.Chunks.cs | 2 +- tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs | 2 +- tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs | 8 ++++---- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 1afb65566..c4b10b4dc 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -248,7 +248,7 @@ internal sealed class PngDecoderCore : IImageDecoderInternals break; case PngChunkType.Data: - pngMetadata.DefaultImageAnimated = currentFrameControl != null; + pngMetadata.AnimateRootFrame = currentFrameControl != null; currentFrameControl ??= new((uint)this.header.Width, (uint)this.header.Height); if (image is null) { @@ -265,7 +265,7 @@ internal sealed class PngDecoderCore : IImageDecoderInternals this.ReadNextDataChunk, currentFrameControl.Value, cancellationToken); - if (pngMetadata.DefaultImageAnimated) + if (pngMetadata.AnimateRootFrame) { previousFrame = currentFrame; previousFrameControl = currentFrameControl; diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs index e0f7ed948..802e4dd6a 100644 --- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs @@ -190,11 +190,11 @@ internal sealed class PngEncoderCore : IImageEncoderInternals, IDisposable if (image.Frames.Count > 1) { - this.WriteAnimationControlChunk(stream, (uint)(image.Frames.Count - (pngMetadata.DefaultImageAnimated ? 0 : 1)), pngMetadata.RepeatCount); + this.WriteAnimationControlChunk(stream, (uint)(image.Frames.Count - (pngMetadata.AnimateRootFrame ? 0 : 1)), pngMetadata.RepeatCount); } // If the first frame isn't animated, write it as usual and skip it when writing animated frames - if (!pngMetadata.DefaultImageAnimated || image.Frames.Count == 1) + if (!pngMetadata.AnimateRootFrame || image.Frames.Count == 1) { FrameControl frameControl = new((uint)this.width, (uint)this.height); this.WriteDataChunks(frameControl, currentFrame.PixelBuffer.GetRegion(), quantized, stream, false); @@ -209,7 +209,7 @@ internal sealed class PngEncoderCore : IImageEncoderInternals, IDisposable PngDisposalMethod previousDisposal = frameMetadata.DisposalMethod; FrameControl frameControl = this.WriteFrameControlChunk(stream, frameMetadata, currentFrame.Bounds(), 0); uint sequenceNumber = 1; - if (pngMetadata.DefaultImageAnimated) + if (pngMetadata.AnimateRootFrame) { this.WriteDataChunks(frameControl, currentFrame.PixelBuffer.GetRegion(), quantized, stream, false); } diff --git a/src/ImageSharp/Formats/Png/PngMetadata.cs b/src/ImageSharp/Formats/Png/PngMetadata.cs index 766377f7c..d9028dd80 100644 --- a/src/ImageSharp/Formats/Png/PngMetadata.cs +++ b/src/ImageSharp/Formats/Png/PngMetadata.cs @@ -29,7 +29,7 @@ public class PngMetadata : IDeepCloneable this.InterlaceMethod = other.InterlaceMethod; this.TransparentColor = other.TransparentColor; this.RepeatCount = other.RepeatCount; - this.DefaultImageAnimated = other.DefaultImageAnimated; + this.AnimateRootFrame = other.AnimateRootFrame; if (other.ColorTable?.Length > 0) { @@ -85,9 +85,9 @@ public class PngMetadata : IDeepCloneable public uint RepeatCount { get; set; } = 1; /// - /// Gets or sets a value indicating whether the default image is shown as part of the animated sequence + /// Gets or sets a value indicating whether the root frame is shown as part of the animated sequence /// - public bool DefaultImageAnimated { get; set; } = true; + public bool AnimateRootFrame { get; set; } = true; /// public IDeepCloneable DeepClone() => new PngMetadata(this); diff --git a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs index 2e3390298..76fd260dd 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.Chunks.cs @@ -68,7 +68,7 @@ public partial class PngEncoderTests { using Image image = provider.GetImage(PngDecoder.Instance); PngMetadata metadata = image.Metadata.GetPngMetadata(); - int correctFrameCount = image.Frames.Count - (metadata.DefaultImageAnimated ? 0 : 1); + int correctFrameCount = image.Frames.Count - (metadata.AnimateRootFrame ? 0 : 1); using MemoryStream memStream = new(); image.Save(memStream, PngEncoder); memStream.Position = 0; diff --git a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs index 6c4047be8..ca5aae961 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs @@ -470,7 +470,7 @@ public partial class PngEncoderTests PngMetadata outputMetadata = output.Metadata.GetPngMetadata(); Assert.Equal(originalMetadata.RepeatCount, outputMetadata.RepeatCount); - Assert.Equal(originalMetadata.DefaultImageAnimated, outputMetadata.DefaultImageAnimated); + Assert.Equal(originalMetadata.AnimateRootFrame, outputMetadata.AnimateRootFrame); for (int i = 0; i < image.Frames.Count; i++) { diff --git a/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs index 8308935d0..225e4deef 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs @@ -33,7 +33,7 @@ public class PngMetadataTests Gamma = 2, TextData = new List { new PngTextData("name", "value", "foo", "bar") }, RepeatCount = 123, - DefaultImageAnimated = false + AnimateRootFrame = false }; PngMetadata clone = (PngMetadata)meta.DeepClone(); @@ -45,7 +45,7 @@ public class PngMetadataTests Assert.False(meta.TextData.Equals(clone.TextData)); Assert.True(meta.TextData.SequenceEqual(clone.TextData)); Assert.True(meta.RepeatCount == clone.RepeatCount); - Assert.True(meta.DefaultImageAnimated == clone.DefaultImageAnimated); + Assert.True(meta.AnimateRootFrame == clone.AnimateRootFrame); clone.BitDepth = PngBitDepth.Bit2; clone.ColorType = PngColorType.Palette; @@ -153,7 +153,7 @@ public class PngMetadataTests { using Image image = provider.GetImage(PngDecoder.Instance); PngMetadata meta = image.Metadata.GetFormatMetadata(PngFormat.Instance); - Assert.False(meta.DefaultImageAnimated); + Assert.False(meta.AnimateRootFrame); } [Theory] @@ -163,7 +163,7 @@ public class PngMetadataTests { using Image image = provider.GetImage(PngDecoder.Instance); PngMetadata meta = image.Metadata.GetFormatMetadata(PngFormat.Instance); - Assert.True(meta.DefaultImageAnimated); + Assert.True(meta.AnimateRootFrame); } [Theory] From da497882fed5080ef78fd65364e14d34be6b9a22 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 6 Apr 2024 09:18:23 +1000 Subject: [PATCH 22/26] Revert additional changes --- .../ComponentProcessors/ComponentProcessor.cs | 2 +- .../Jpeg/Components/Encoder/ComponentProcessor.cs | 2 +- src/ImageSharp/Formats/Tga/TgaDecoderCore.cs | 2 +- .../Memory/Allocators/MemoryAllocatorOptions.cs | 4 ++-- src/ImageSharp/Memory/MemoryAllocatorExtensions.cs | 14 +++++++------- .../Transforms/Resize/ResizeKernelMap.cs | 2 +- .../Processors/Transforms/Resize/ResizeWorker.cs | 2 +- tests/ImageSharp.Tests/Memory/Buffer2DTests.cs | 8 ++++---- 8 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs index 3bdaf2dc2..f0cc4d5e8 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs @@ -16,7 +16,7 @@ internal abstract class ComponentProcessor : IDisposable this.Component = component; this.BlockAreaSize = component.SubSamplingDivisors * blockSize; - this.ColorBuffer = memoryAllocator.Allocate2DOverAligned( + this.ColorBuffer = memoryAllocator.Allocate2DOveraligned( postProcessorBufferSize.Width, postProcessorBufferSize.Height, this.BlockAreaSize.Height); diff --git a/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs b/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs index 9346b11e7..c33a8a196 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs @@ -28,7 +28,7 @@ internal class ComponentProcessor : IDisposable this.blockAreaSize = component.SubSamplingDivisors * 8; // alignment of 8 so each block stride can be sampled from a single 'ref pointer' - this.ColorBuffer = memoryAllocator.Allocate2DOverAligned( + this.ColorBuffer = memoryAllocator.Allocate2DOveraligned( postProcessorBufferSize.Width, postProcessorBufferSize.Height, 8, diff --git a/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs b/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs index 34f4c2bcf..26e057bff 100644 --- a/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs +++ b/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs @@ -84,7 +84,7 @@ internal sealed class TgaDecoderCore : IImageDecoderInternals throw new UnknownImageFormatException("Width or height cannot be 0"); } - Image image = new(this.configuration, this.fileHeader.Width, this.fileHeader.Height, this.metadata); + Image image = Image.CreateUninitialized(this.configuration, this.fileHeader.Width, this.fileHeader.Height, this.metadata); Buffer2D pixels = image.GetRootFramePixelBuffer(); if (this.fileHeader.ColorMapType == 1) diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs index 9f2581660..d9ba62c1e 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs @@ -17,7 +17,7 @@ public struct MemoryAllocatorOptions /// public int? MaximumPoolSizeMegabytes { - readonly get => this.maximumPoolSizeMegabytes; + get => this.maximumPoolSizeMegabytes; set { if (value.HasValue) @@ -35,7 +35,7 @@ public struct MemoryAllocatorOptions /// public int? AllocationLimitMegabytes { - readonly get => this.allocationLimitMegabytes; + get => this.allocationLimitMegabytes; set { if (value.HasValue) diff --git a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs index e0dc8aab3..ff306e1e4 100644 --- a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs +++ b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs @@ -18,20 +18,20 @@ public static class MemoryAllocatorExtensions /// The memory allocator. /// The buffer width. /// The buffer height. - /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . + /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . /// The allocation options. /// The . public static Buffer2D Allocate2D( this MemoryAllocator memoryAllocator, int width, int height, - bool preferContiguousImageBuffers, + bool preferContiguosImageBuffers, AllocationOptions options = AllocationOptions.None) where T : struct { long groupLength = (long)width * height; MemoryGroup memoryGroup; - if (preferContiguousImageBuffers && groupLength < int.MaxValue) + if (preferContiguosImageBuffers && groupLength < int.MaxValue) { IMemoryOwner buffer = memoryAllocator.Allocate((int)groupLength, options); memoryGroup = MemoryGroup.CreateContiguous(buffer, false); @@ -69,16 +69,16 @@ public static class MemoryAllocatorExtensions /// The type of buffer items to allocate. /// The memory allocator. /// The buffer size. - /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . + /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . /// The allocation options. /// The . public static Buffer2D Allocate2D( this MemoryAllocator memoryAllocator, Size size, - bool preferContiguousImageBuffers, + bool preferContiguosImageBuffers, AllocationOptions options = AllocationOptions.None) where T : struct => - Allocate2D(memoryAllocator, size.Width, size.Height, preferContiguousImageBuffers, options); + Allocate2D(memoryAllocator, size.Width, size.Height, preferContiguosImageBuffers, options); /// /// Allocates a buffer of value type objects interpreted as a 2D region @@ -96,7 +96,7 @@ public static class MemoryAllocatorExtensions where T : struct => Allocate2D(memoryAllocator, size.Width, size.Height, false, options); - internal static Buffer2D Allocate2DOverAligned( + internal static Buffer2D Allocate2DOveraligned( this MemoryAllocator memoryAllocator, int width, int height, diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs index da26cbefc..c1907bb52 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs @@ -50,7 +50,7 @@ internal partial class ResizeKernelMap : IDisposable this.sourceLength = sourceLength; this.DestinationLength = destinationLength; this.MaxDiameter = (radius * 2) + 1; - this.data = memoryAllocator.Allocate2D(this.MaxDiameter, bufferHeight, preferContiguousImageBuffers: true, AllocationOptions.Clean); + this.data = memoryAllocator.Allocate2D(this.MaxDiameter, bufferHeight, preferContiguosImageBuffers: true, AllocationOptions.Clean); this.pinHandle = this.data.DangerousGetSingleMemory().Pin(); this.kernels = new ResizeKernel[destinationLength]; this.tempValues = new double[this.MaxDiameter]; diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs index fd13d7b5a..cce27a401 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs @@ -83,7 +83,7 @@ internal sealed class ResizeWorker : IDisposable this.transposedFirstPassBuffer = configuration.MemoryAllocator.Allocate2D( this.workerHeight, targetWorkingRect.Width, - preferContiguousImageBuffers: true, + preferContiguosImageBuffers: true, options: AllocationOptions.Clean); this.tempRowBuffer = configuration.MemoryAllocator.Allocate(this.sourceRectangle.Width); diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs index 3ef4fc540..8ba3bf70a 100644 --- a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs @@ -73,11 +73,11 @@ public partial class Buffer2DTests using Buffer2D buffer = useSizeOverload ? this.MemoryAllocator.Allocate2D( new Size(200, 200), - preferContiguousImageBuffers: true) : + preferContiguosImageBuffers: true) : this.MemoryAllocator.Allocate2D( 200, 200, - preferContiguousImageBuffers: true); + preferContiguosImageBuffers: true); Assert.Equal(1, buffer.FastMemoryGroup.Count); Assert.Equal(200 * 200, buffer.FastMemoryGroup.TotalLength); } @@ -88,7 +88,7 @@ public partial class Buffer2DTests { this.MemoryAllocator.BufferCapacityInBytes = sizeof(int) * bufferCapacity; - using Buffer2D buffer = this.MemoryAllocator.Allocate2DOverAligned(width, height, alignmentMultiplier); + using Buffer2D buffer = this.MemoryAllocator.Allocate2DOveraligned(width, height, alignmentMultiplier); MemoryGroup memoryGroup = buffer.FastMemoryGroup; int expectedAlignment = width * alignmentMultiplier; @@ -359,5 +359,5 @@ public partial class Buffer2DTests [Theory] [MemberData(nameof(InvalidLengths))] public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException_OverAligned(Size size) - => Assert.Throws(() => this.MemoryAllocator.Allocate2DOverAligned(size.Width, size.Height, 1)); + => Assert.Throws(() => this.MemoryAllocator.Allocate2DOveraligned(size.Width, size.Height, 1)); } From 26d44ef5f127c9869454e6c648bdea248d26a4ac Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 6 Apr 2024 11:29:51 +1000 Subject: [PATCH 23/26] Clear pixel buffers on decode. --- .../Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs | 3 ++- src/ImageSharp/Formats/Tga/TgaDecoderCore.cs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs index 5add4c6f0..7f89baba5 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/SpectralConverter{TPixel}.cs @@ -201,7 +201,8 @@ internal class SpectralConverter : SpectralConverter, IDisposable this.pixelBuffer = allocator.Allocate2D( pixelSize.Width, pixelSize.Height, - this.Configuration.PreferContiguousImageBuffers); + this.Configuration.PreferContiguousImageBuffers, + AllocationOptions.Clean); this.paddedProxyPixelRow = allocator.Allocate(pixelSize.Width + 3); // Component processors from spectral to RGB diff --git a/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs b/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs index 26e057bff..34f4c2bcf 100644 --- a/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs +++ b/src/ImageSharp/Formats/Tga/TgaDecoderCore.cs @@ -84,7 +84,7 @@ internal sealed class TgaDecoderCore : IImageDecoderInternals throw new UnknownImageFormatException("Width or height cannot be 0"); } - Image image = Image.CreateUninitialized(this.configuration, this.fileHeader.Width, this.fileHeader.Height, this.metadata); + Image image = new(this.configuration, this.fileHeader.Width, this.fileHeader.Height, this.metadata); Buffer2D pixels = image.GetRootFramePixelBuffer(); if (this.fileHeader.ColorMapType == 1) From fb64b3367fa7e2c68213c94606a9c96037991850 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 6 Apr 2024 12:44:45 +1000 Subject: [PATCH 24/26] Clamp read palette indices. --- src/ImageSharp/Formats/Png/PngScanlineProcessor.cs | 3 ++- tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs | 8 ++++++++ tests/ImageSharp.Tests/TestImages.cs | 2 ++ tests/Images/Input/Png/issues/Issue_2714.png | 3 +++ 4 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 tests/Images/Input/Png/issues/Issue_2714.png diff --git a/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs b/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs index b327ed21b..c653e71c6 100644 --- a/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs +++ b/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs @@ -199,11 +199,12 @@ internal static class PngScanlineProcessor ref TPixel rowSpanRef = ref MemoryMarshal.GetReference(rowSpan); ref Color paletteBase = ref MemoryMarshal.GetReference(palette.Value.Span); uint offset = pixelOffset + frameControl.XOffset; + int maxIndex = palette.Value.Length - 1; for (nuint x = offset, o = 0; x < frameControl.XMax; x += increment, o++) { uint index = Unsafe.Add(ref scanlineSpanRef, o); - pixel.FromRgba32(Unsafe.Add(ref paletteBase, index).ToRgba32()); + pixel.FromRgba32(Unsafe.Add(ref paletteBase, (int)Math.Min(index, maxIndex)).ToRgba32()); Unsafe.Add(ref rowSpanRef, x) = pixel; } } diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 9ff368d4e..8d4bc3f46 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -693,4 +693,12 @@ public partial class PngDecoderTests string path = Path.GetFullPath(Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, file)); _ = Image.Identify(path); } + + [Theory] + [InlineData(TestImages.Png.Bad.Issue2714BadPalette)] + public void Decode_BadPalette(string file) + { + string path = Path.GetFullPath(Path.Combine(TestEnvironment.InputImagesDirectoryFullPath, file)); + using Image image = Image.Load(path); + } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index afb13363d..f925f1a90 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -192,6 +192,8 @@ public static class TestImages public const string BadZTXT = "Png/issues/bad-ztxt.png"; public const string BadZTXT2 = "Png/issues/bad-ztxt2.png"; + + public const string Issue2714BadPalette = "Png/issues/Issue_2714.png"; } } diff --git a/tests/Images/Input/Png/issues/Issue_2714.png b/tests/Images/Input/Png/issues/Issue_2714.png new file mode 100644 index 000000000..9bb231dd9 --- /dev/null +++ b/tests/Images/Input/Png/issues/Issue_2714.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:9a4b6efc3090dbd70ae9efe97ea817464845263536beea4e80fd7c884dee6c5a +size 128 From d1cc651e19def81df66779985a50f9372db332bc Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Wed, 10 Apr 2024 23:18:45 +0200 Subject: [PATCH 25/26] fix BmpDecoder_ThrowsException_Issue2696 --- tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs | 11 +++++++---- tests/Images/Input/Bmp/issue-2696.bmp | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs index be2fa4b68..1ce794e44 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs @@ -563,8 +563,11 @@ public class BmpDecoderTests [WithFile(Issue2696, PixelTypes.Rgba32)] public void BmpDecoder_ThrowsException_Issue2696(TestImageProvider provider) where TPixel : unmanaged, IPixel - => Assert.Throws(() => - { - using Image image = provider.GetImage(BmpDecoder.Instance); - }); + { + InvalidImageContentException ex = Assert.Throws(() => + { + using Image image = provider.GetImage(BmpDecoder.Instance); + }); + Assert.IsType(ex.InnerException); + } } diff --git a/tests/Images/Input/Bmp/issue-2696.bmp b/tests/Images/Input/Bmp/issue-2696.bmp index 4a42e456c..6770dd946 100644 --- a/tests/Images/Input/Bmp/issue-2696.bmp +++ b/tests/Images/Input/Bmp/issue-2696.bmp @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:de7e7ec0454a55f6a76c859f356b240a3d4cb56ca50dfa209a1813dd09e12076 -size 143 +oid sha256:bc42cda9bac8fc73351ad03bf55214069bb8d31ea5bdd806321a8cc8b56c282e +size 126 From 572366e07806bfb6a269c9a132c9afbd2a9191d0 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Wed, 10 Apr 2024 23:33:34 +0200 Subject: [PATCH 26/26] test allocation limits --- ...niformUnmanagedPoolMemoryAllocatorTests.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index 6187b536e..077d0afed 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -418,4 +418,28 @@ public class UniformUnmanagedPoolMemoryAllocatorTests _ = MemoryAllocator.Create(); } } + + [Fact] + public void Allocate_OverLimit_ThrowsInvalidMemoryOperationException() + { + MemoryAllocator allocator = MemoryAllocator.Create(new MemoryAllocatorOptions() + { + AllocationLimitMegabytes = 4 + }); + const int oneMb = 1 << 20; + allocator.Allocate(4 * oneMb).Dispose(); // Should work + Assert.Throws(() => allocator.Allocate(5 * oneMb)); + } + + [Fact] + public void AllocateGroup_OverLimit_ThrowsInvalidMemoryOperationException() + { + MemoryAllocator allocator = MemoryAllocator.Create(new MemoryAllocatorOptions() + { + AllocationLimitMegabytes = 4 + }); + const int oneMb = 1 << 20; + allocator.AllocateGroup(4 * oneMb, 1024).Dispose(); // Should work + Assert.Throws(() => allocator.AllocateGroup(5 * oneMb, 1024)); + } }