From 8f2716b567542d7fa1638592f5d964d4ff3aa815 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 26 May 2026 14:01:08 +1000 Subject: [PATCH 1/2] GIF: background handling & quantizer overflow fix (#3133) * GIF: background handling & quantizer overflow fix * Fix tests --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 6 +- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 74 ++++-------- src/ImageSharp/Formats/Gif/GifMetadata.cs | 8 +- .../HexadecatreeQuantizer{TPixel}.cs | 25 ++-- tests/ImageSharp.Tests/Issues/Issue_396.cs | 111 ++++++++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 3 + ...tColor_WithHexadecatreeQuantizer_rgb32.bmp | 2 +- ...de_8BitColor_WithOctreeQuantizer_rgb32.bmp | 3 - ...Bike_HexadecatreeQuantizer_ErrorDither.png | 4 +- .../Input/Png/issues/Issue_396_dragon.png | 3 + .../Input/Png/issues/Issue_396_hand_1.png | 3 + .../Input/Png/issues/Issue_396_hand_2.png | 3 + 12 files changed, 171 insertions(+), 74 deletions(-) create mode 100644 tests/ImageSharp.Tests/Issues/Issue_396.cs delete mode 100644 tests/Images/External/ReferenceOutput/BmpEncoderTests/Encode_8BitColor_WithOctreeQuantizer_rgb32.bmp create mode 100644 tests/Images/Input/Png/issues/Issue_396_dragon.png create mode 100644 tests/Images/Input/Png/issues/Issue_396_hand_1.png create mode 100644 tests/Images/Input/Png/issues/Issue_396_hand_2.png diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index c087b66811..dc6ce1846b 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -990,7 +990,11 @@ internal sealed class GifDecoderCore : ImageDecoderCore byte index = this.logicalScreenDescriptor.BackgroundColorIndex; this.backgroundColorIndex = index; - this.gifMetadata.BackgroundColorIndex = index; + ReadOnlyMemory? globalColorTable = this.gifMetadata.GlobalColorTable; + if (globalColorTable.HasValue && index < globalColorTable.Value.Length) + { + this.gifMetadata.BackgroundColor = globalColorTable.Value.Span[index]; + } } private unsafe struct ScratchBuffer diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index 49640fae08..dcba3953d5 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -45,14 +45,6 @@ internal sealed class GifEncoderCore /// private readonly IPixelSamplingStrategy pixelSamplingStrategy; - /// - /// The default background color of the canvas when animating. - /// This color may be used to fill the unused space on the canvas around the frames, - /// as well as the transparent pixels of the first frame. - /// The background color is also used when a frame disposal mode is . - /// - private readonly Color? backgroundColor; - /// /// The number of times any animation is repeated. /// @@ -76,7 +68,6 @@ internal sealed class GifEncoderCore this.skipMetadata = encoder.SkipMetadata; this.colorTableMode = encoder.ColorTableMode; this.pixelSamplingStrategy = encoder.PixelSamplingStrategy; - this.backgroundColor = encoder.BackgroundColor; this.repeatCount = encoder.RepeatCount; this.transparentColorMode = encoder.TransparentColorMode; } @@ -113,7 +104,17 @@ internal sealed class GifEncoderCore TransparentColorMode mode = this.transparentColorMode; // Create a new quantizer options instance augmenting the transparent color mode to match the encoder. - QuantizerOptions options = (this.encoder.Quantizer?.Options ?? new QuantizerOptions()).DeepClone(o => o.TransparentColorMode = mode); + QuantizerOptions options = (this.encoder.Quantizer?.Options ?? new QuantizerOptions()).DeepClone(o => + { + o.TransparentColorMode = mode; + + // Animated GIF delta frames can use one padded color-table index as transparency. + // Express that through MaxColors so custom quantizers receive the same budget. + if (image.Frames.Count > 1 && o.MaxColors == QuantizerConstants.MaxColors) + { + o.MaxColors = QuantizerConstants.MaxColors - 1; + } + }); if (globalQuantizer is null) { @@ -145,6 +146,11 @@ internal sealed class GifEncoderCore IPixelSamplingStrategy strategy = this.pixelSamplingStrategy; ImageFrame encodingFrame = image.Frames.RootFrame; + + // This color is encoded as the logical-screen background index and is also + // used when de-duplicating frames that restore to the GIF background. + Color backgroundColor = this.encoder.BackgroundColor ?? gifMetadata.BackgroundColor ?? Color.Transparent; + byte backgroundIndex = 0; if (useGlobalTableForFirstFrame) { using IQuantizer firstFrameQuantizer = globalQuantizer.CreatePixelSpecificQuantizer(this.configuration, options); @@ -158,6 +164,8 @@ internal sealed class GifEncoderCore } quantized = firstFrameQuantizer.QuantizeFrame(encodingFrame, encodingFrame.Bounds); + TPixel backgroundPixel = backgroundColor.ToPixel(); + backgroundIndex = firstFrameQuantizer.GetQuantizedColor(backgroundPixel, out _); } else { @@ -184,8 +192,6 @@ internal sealed class GifEncoderCore frameMetadata.TransparencyIndex = ClampIndex(transparencyIndex); } - byte backgroundIndex = GetBackgroundIndex(quantized, gifMetadata, this.backgroundColor); - // Get the number of bits. int bitDepth = ColorNumerics.GetBitsNeededForColorDepth(quantized.Palette.Length); this.WriteLogicalScreenDescriptor(image.Metadata, image.Width, image.Height, backgroundIndex, useGlobalTable, bitDepth, stream); @@ -222,6 +228,7 @@ internal sealed class GifEncoderCore image, globalQuantizer, globalFrameQuantizer, + backgroundColor, transparencyIndex, frameMetadata.DisposalMode, cancellationToken); @@ -253,6 +260,7 @@ internal sealed class GifEncoderCore Image image, IQuantizer globalQuantizer, PaletteQuantizer globalFrameQuantizer, + Color backgroundColor, int globalTransparencyIndex, FrameDisposalMode previousDisposalMode, CancellationToken cancellationToken) @@ -284,6 +292,7 @@ internal sealed class GifEncoderCore globalFrameQuantizer, useLocal, gifMetadata, + backgroundColor, previousDisposalMode); previousFrame = currentFrame; @@ -327,6 +336,7 @@ internal sealed class GifEncoderCore PaletteQuantizer globalFrameQuantizer, bool useLocal, GifFrameMetadata metadata, + Color backgroundColor, FrameDisposalMode previousDisposalMode) where TPixel : unmanaged, IPixel { @@ -347,7 +357,7 @@ internal sealed class GifEncoderCore previous.Metadata.GetGifMetadata().DisposalMode; Color background = !useTransparency && disposalMode == FrameDisposalMode.RestoreToBackground - ? this.backgroundColor ?? Color.Transparent + ? backgroundColor : Color.Transparent; // Deduplicate and quantize the frame capturing only required parts. @@ -534,44 +544,6 @@ internal sealed class GifEncoderCore return index; } - /// - /// Returns the index of the background color in the palette. - /// - /// The current quantized frame. - /// The gif metadata - /// The background color to match. - /// The pixel format. - /// The index of the background color. - private static byte GetBackgroundIndex(IndexedImageFrame? quantized, GifMetadata metadata, Color? background) - where TPixel : unmanaged, IPixel - { - int match = -1; - if (quantized != null) - { - if (background.HasValue) - { - TPixel backgroundPixel = background.Value.ToPixel(); - ReadOnlySpan palette = quantized.Palette.Span; - for (int i = 0; i < palette.Length; i++) - { - if (!backgroundPixel.Equals(palette[i])) - { - continue; - } - - match = i; - break; - } - } - else if (metadata.BackgroundColorIndex < quantized.Palette.Length) - { - match = metadata.BackgroundColorIndex; - } - } - - return ClampIndex(match); - } - /// /// Writes the file header signature and version to the stream. /// diff --git a/src/ImageSharp/Formats/Gif/GifMetadata.cs b/src/ImageSharp/Formats/Gif/GifMetadata.cs index 978209b23b..50a29775d0 100644 --- a/src/ImageSharp/Formats/Gif/GifMetadata.cs +++ b/src/ImageSharp/Formats/Gif/GifMetadata.cs @@ -26,7 +26,7 @@ public class GifMetadata : IFormatMetadata { this.RepeatCount = other.RepeatCount; this.ColorTableMode = other.ColorTableMode; - this.BackgroundColorIndex = other.BackgroundColorIndex; + this.BackgroundColor = other.BackgroundColor; if (other.GlobalColorTable?.Length > 0) { @@ -59,10 +59,9 @@ public class GifMetadata : IFormatMetadata public ReadOnlyMemory? GlobalColorTable { get; set; } /// - /// Gets or sets the index at the for the background color. - /// The background color is the color used for those pixels on the screen that are not covered by an image. + /// Gets or sets the background color used for pixels on the screen that are not covered by an image. /// - public byte BackgroundColorIndex { get; set; } + public Color? BackgroundColor { get; set; } /// /// Gets or sets the collection of comments about the graphics, credits, descriptions or any @@ -101,6 +100,7 @@ public class GifMetadata : IFormatMetadata { AnimateRootFrame = true, ColorTableMode = this.ColorTableMode, + BackgroundColor = this.BackgroundColor ?? Color.Transparent, PixelTypeInfo = this.GetPixelTypeInfo(), RepeatCount = this.RepeatCount, }; diff --git a/src/ImageSharp/Processing/Processors/Quantization/HexadecatreeQuantizer{TPixel}.cs b/src/ImageSharp/Processing/Processors/Quantization/HexadecatreeQuantizer{TPixel}.cs index b5d39d73ec..71c1b84571 100644 --- a/src/ImageSharp/Processing/Processors/Quantization/HexadecatreeQuantizer{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Quantization/HexadecatreeQuantizer{TPixel}.cs @@ -392,11 +392,11 @@ public struct HexadecatreeQuantizer : IQuantizer internal struct Node { public bool Leaf; - public int PixelCount; - public int Red; - public int Green; - public int Blue; - public int Alpha; + public long PixelCount; + public long Red; + public long Green; + public long Blue; + public long Alpha; public short PaletteIndex; public short NextReducibleIndex; private InlineArray16 children; @@ -510,12 +510,13 @@ public struct HexadecatreeQuantizer : IQuantizer return; } - // Now merge the (presumably reduced) children. - int pixelCount = 0; - int sumRed = 0; - int sumGreen = 0; - int sumBlue = 0; - int sumAlpha = 0; + // Allocation fallback can accumulate samples on an interior node. Seed the merge + // with this node's own sums so reduction preserves those samples with its children. + long pixelCount = this.PixelCount; + long sumRed = this.Red; + long sumGreen = this.Green; + long sumBlue = this.Blue; + long sumAlpha = this.Alpha; Span children = this.Children; for (int i = 0; i < children.Length; i++) @@ -524,7 +525,7 @@ public struct HexadecatreeQuantizer : IQuantizer if (childIndex != -1) { ref Node child = ref tree.Nodes[childIndex]; - int pixels = child.PixelCount; + long pixels = child.PixelCount; sumRed += child.Red; sumGreen += child.Green; sumBlue += child.Blue; diff --git a/tests/ImageSharp.Tests/Issues/Issue_396.cs b/tests/ImageSharp.Tests/Issues/Issue_396.cs new file mode 100644 index 0000000000..0d76f0c808 --- /dev/null +++ b/tests/ImageSharp.Tests/Issues/Issue_396.cs @@ -0,0 +1,111 @@ +// Copyright (c) Six Labors. +// Licensed under the Six Labors Split License. + +using SixLabors.ImageSharp.Formats; +using SixLabors.ImageSharp.Formats.Gif; +using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Processing; + +namespace SixLabors.ImageSharp.Tests.Issues; + +// https://github.com/SixLabors/ImageSharp.Drawing/discussions/396 +public class Issue_396 +{ + [Theory] + [WithFile(TestImages.Png.Issue396Dragon, PixelTypes.Rgba32)] + public void GeneratesGifForInspection(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + Image[] handImages = + [ + Image.Load(TestFile.Create(TestImages.Png.Issue396Hand1).Bytes), + Image.Load(TestFile.Create(TestImages.Png.Issue396Hand2).Bytes) + ]; + + try + { + using Image inputImage = provider.GetImage(); + using Image outputImage = new(handImages[0].Width, handImages[0].Height); + + const float scaleFactor = 0.6F; + const float squashStepFactor = 0.22F; + + for (int i = 0; i < handImages.Length; i++) + { + using Image frameImage = new(handImages[i].Width, handImages[i].Height); + float squashFactorY = 1 - (i * squashStepFactor); + + frameImage.ProcessPixelRows(accessor => + { + Rgba32 gray = Color.Gray.ToPixel(); + + for (int y = 0; y < accessor.Height; y++) + { + accessor.GetRowSpan(y).Fill(gray); + } + }); + + Rectangle targetRect = new( + (int)((frameImage.Width - (frameImage.Width * scaleFactor)) / 2), + (int)(frameImage.Height * (((1 - scaleFactor) / 2) + (scaleFactor * (1 - squashFactorY)))), + (int)(frameImage.Width * scaleFactor), + (int)(frameImage.Height * scaleFactor * squashFactorY)); + + using Image dragonFrame = inputImage.CloneAs(); + dragonFrame.Mutate(context => context.Resize(targetRect.Size)); + + frameImage.Mutate(context => + { + context.DrawImage(dragonFrame, targetRect.Location, 1F); + context.DrawImage(handImages[i], Point.Empty, 1F); + }); + + ImageFrame outputFrame = outputImage.Frames.AddFrame(frameImage.Frames.RootFrame); + GifFrameMetadata gifFrameMetadata = outputFrame.Metadata.GetGifMetadata(); + gifFrameMetadata.FrameDelay = 40; + gifFrameMetadata.DisposalMode = FrameDisposalMode.RestoreToBackground; + } + + for (int i = handImages.Length - 1; i > 0; i--) + { + outputImage.Frames.AddFrame(outputImage.Frames[i]); + } + + outputImage.Frames.RemoveFrame(0); + GifMetadata gifMetadata = outputImage.Metadata.GetGifMetadata(); + gifMetadata.RepeatCount = 0; + + Assert.Equal((handImages.Length * 2) - 1, outputImage.Frames.Count); + foreach (ImageFrame frame in outputImage.Frames) + { + Assert.Equal(FrameDisposalMode.RestoreToBackground, frame.Metadata.GetGifMetadata().DisposalMode); + } + + outputImage.DebugSaveMultiFrame(provider, "Issue396-source-frames", appendPixelTypeToFileName: false); + provider.Utility.SaveTestOutputFile( + outputImage, + "gif", + new GifEncoder(), + "Issue396-encoded", + appendPixelTypeToFileName: false, + appendSourceFileOrDescription: false); + + // Save and decode the GIF so the encoder's optimized frame diffs can be inspected separately. + using MemoryStream gifStream = new(); + outputImage.SaveAsGif(gifStream); + gifStream.Position = 0; + + using Image decodedImage = Image.Load(gifStream); + decodedImage.DebugSaveMultiFrame(provider, "Issue396-decoded-frames", appendPixelTypeToFileName: false); + + Assert.Equal(outputImage.Frames.Count, decodedImage.Frames.Count); + } + finally + { + foreach (Image handImage in handImages) + { + handImage.Dispose(); + } + } + } +} diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 2624d1cdad..d74db0eeb7 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -79,6 +79,9 @@ public static class TestImages public const string AnimatedFrameCount = "Png/animated/issue-animated-frame-count.png"; public const string Issue2666 = "Png/issues/Issue_2666.png"; public const string Issue2882 = "Png/issues/Issue_2882.png"; + public const string Issue396Dragon = "Png/issues/Issue_396_dragon.png"; + public const string Issue396Hand1 = "Png/issues/Issue_396_hand_1.png"; + public const string Issue396Hand2 = "Png/issues/Issue_396_hand_2.png"; // Filtered test images from http://www.schaik.com/pngsuite/pngsuite_fil_png.html public const string Filter0 = "Png/filter0.png"; diff --git a/tests/Images/External/ReferenceOutput/BmpEncoderTests/Encode_8BitColor_WithHexadecatreeQuantizer_rgb32.bmp b/tests/Images/External/ReferenceOutput/BmpEncoderTests/Encode_8BitColor_WithHexadecatreeQuantizer_rgb32.bmp index f4ae3b9b68..7f79c6f3a5 100644 --- a/tests/Images/External/ReferenceOutput/BmpEncoderTests/Encode_8BitColor_WithHexadecatreeQuantizer_rgb32.bmp +++ b/tests/Images/External/ReferenceOutput/BmpEncoderTests/Encode_8BitColor_WithHexadecatreeQuantizer_rgb32.bmp @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:a98b1ec707af066f77fad7d1a64b858d460986beb6d27682717dd5e221310fd4 +oid sha256:fff91e78a87c2b2db661609dd70af2d7a74e6a0de18924ab2b6272e12c60977d size 9270 diff --git a/tests/Images/External/ReferenceOutput/BmpEncoderTests/Encode_8BitColor_WithOctreeQuantizer_rgb32.bmp b/tests/Images/External/ReferenceOutput/BmpEncoderTests/Encode_8BitColor_WithOctreeQuantizer_rgb32.bmp deleted file mode 100644 index f4ae3b9b68..0000000000 --- a/tests/Images/External/ReferenceOutput/BmpEncoderTests/Encode_8BitColor_WithOctreeQuantizer_rgb32.bmp +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:a98b1ec707af066f77fad7d1a64b858d460986beb6d27682717dd5e221310fd4 -size 9270 diff --git a/tests/Images/External/ReferenceOutput/QuantizerTests/ApplyQuantization_Bike_HexadecatreeQuantizer_ErrorDither.png b/tests/Images/External/ReferenceOutput/QuantizerTests/ApplyQuantization_Bike_HexadecatreeQuantizer_ErrorDither.png index e9782bc076..b17bce9e3e 100644 --- a/tests/Images/External/ReferenceOutput/QuantizerTests/ApplyQuantization_Bike_HexadecatreeQuantizer_ErrorDither.png +++ b/tests/Images/External/ReferenceOutput/QuantizerTests/ApplyQuantization_Bike_HexadecatreeQuantizer_ErrorDither.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:b380eda5646fe97ee217ef711103001e54ee023fb8d95f7f3bbad19d886130da -size 83702 +oid sha256:822f4ae1d8676e2231a0c938296931be42a74c0a51b4ec63fdf6dcd17d64dc7e +size 83924 diff --git a/tests/Images/Input/Png/issues/Issue_396_dragon.png b/tests/Images/Input/Png/issues/Issue_396_dragon.png new file mode 100644 index 0000000000..4e8b2d8917 --- /dev/null +++ b/tests/Images/Input/Png/issues/Issue_396_dragon.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:203aca58e0aa19cbb7ab0820ef55005af830bf4c9bfd1dd4d9967c7e1bc1d51a +size 494160 diff --git a/tests/Images/Input/Png/issues/Issue_396_hand_1.png b/tests/Images/Input/Png/issues/Issue_396_hand_1.png new file mode 100644 index 0000000000..1624bce043 --- /dev/null +++ b/tests/Images/Input/Png/issues/Issue_396_hand_1.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:4ab791784ac46f9e50a63bd255f39ff8af236a922aab3510a29b473bd93bffd5 +size 16295 diff --git a/tests/Images/Input/Png/issues/Issue_396_hand_2.png b/tests/Images/Input/Png/issues/Issue_396_hand_2.png new file mode 100644 index 0000000000..d12fa097ba --- /dev/null +++ b/tests/Images/Input/Png/issues/Issue_396_hand_2.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:dbf54bcef6730b68b18770105a15448f984aaca6910693d4e65106a526af58d8 +size 17756 From 8e6cab957b80a56d29ecfbdc3b8f628d09c16d81 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 26 May 2026 14:06:11 +1000 Subject: [PATCH 2/2] Validate PBM max pixel value --- src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs | 5 +++++ .../ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs | 13 +++++++++++++ 2 files changed, 18 insertions(+) diff --git a/src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs b/src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs index 989eadda7a..5451d3d461 100644 --- a/src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs +++ b/src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs @@ -155,6 +155,11 @@ internal sealed class PbmDecoderCore : ImageDecoderCore ThrowPrematureEof(); } + if (this.maxPixelValue <= 0 || this.maxPixelValue >= 65536) + { + throw new InvalidImageContentException("Invalid max pixel value."); + } + if (this.maxPixelValue > 255) { this.componentType = PbmComponentType.Short; diff --git a/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs index 476538ccdf..5c52c9785d 100644 --- a/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs @@ -123,6 +123,19 @@ public class PbmDecoderTests appendPixelTypeToFileName: false); } + [Theory] + [InlineData("P2")] + [InlineData("P3")] + [InlineData("P5")] + [InlineData("P6")] + public void Decode_MaxPixelValueZero_ThrowsInvalidImageContentException(string magic) + { + byte[] bytes = Encoding.ASCII.GetBytes($"{magic} 1 1 0\n0"); + using MemoryStream stream = new(bytes); + + Assert.Throws(() => Image.Load(stream)); + } + [Fact] public void PlainText_PrematureEof() {