Merge branch 'png-cgbi' of https://github.com/Erik-White/ImageSharp into png-cgbi

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<Color>? globalColorTable = this.gifMetadata.GlobalColorTable;
+        if (globalColorTable.HasValue && index < globalColorTable.Value.Length)
+        {
+            this.gifMetadata.BackgroundColor = globalColorTable.Value.Span[index];
+        }
     }
 
     private unsafe struct ScratchBuffer

src/ImageSharp/Formats/Gif/GifEncoderCore.cs

@@ -45,14 +45,6 @@ internal sealed class GifEncoderCore
     /// </summary>
     private readonly IPixelSamplingStrategy pixelSamplingStrategy;
 
-    /// <summary>
-    /// 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 <see cref="FrameDisposalMode.RestoreToBackground"/>.
-    /// </summary>
-    private readonly Color? backgroundColor;
-
     /// <summary>
     /// The number of times any animation is repeated.
     /// </summary>
@@ -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<TPixel> 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<TPixel> firstFrameQuantizer = globalQuantizer.CreatePixelSpecificQuantizer<TPixel>(this.configuration, options);
@@ -158,6 +164,8 @@ internal sealed class GifEncoderCore
             }
 
             quantized = firstFrameQuantizer.QuantizeFrame(encodingFrame, encodingFrame.Bounds);
+            TPixel backgroundPixel = backgroundColor.ToPixel<TPixel>();
+            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<TPixel> image,
         IQuantizer globalQuantizer,
         PaletteQuantizer<TPixel> 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<TPixel> globalFrameQuantizer,
         bool useLocal,
         GifFrameMetadata metadata,
+        Color backgroundColor,
         FrameDisposalMode previousDisposalMode)
         where TPixel : unmanaged, IPixel<TPixel>
     {
@@ -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;
     }
 
-    /// <summary>
-    /// Returns the index of the background color in the palette.
-    /// </summary>
-    /// <param name="quantized">The current quantized frame.</param>
-    /// <param name="metadata">The gif metadata</param>
-    /// <param name="background">The background color to match.</param>
-    /// <typeparam name="TPixel">The pixel format.</typeparam>
-    /// <returns>The <see cref="byte"/> index of the background color.</returns>
-    private static byte GetBackgroundIndex<TPixel>(IndexedImageFrame<TPixel>? quantized, GifMetadata metadata, Color? background)
-        where TPixel : unmanaged, IPixel<TPixel>
-    {
-        int match = -1;
-        if (quantized != null)
-        {
-            if (background.HasValue)
-            {
-                TPixel backgroundPixel = background.Value.ToPixel<TPixel>();
-                ReadOnlySpan<TPixel> 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);
-    }
-
     /// <summary>
     /// Writes the file header signature and version to the stream.
     /// </summary>

src/ImageSharp/Formats/Gif/GifMetadata.cs

@@ -26,7 +26,7 @@ public class GifMetadata : IFormatMetadata<GifMetadata>
     {
         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<GifMetadata>
     public ReadOnlyMemory<Color>? GlobalColorTable { get; set; }
 
     /// <summary>
-    /// Gets or sets the index at the <see cref="GlobalColorTable"/> 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.
     /// </summary>
-    public byte BackgroundColorIndex { get; set; }
+    public Color? BackgroundColor { get; set; }
 
     /// <summary>
     /// Gets or sets the collection of comments about the graphics, credits, descriptions or any
@@ -101,6 +100,7 @@ public class GifMetadata : IFormatMetadata<GifMetadata>
         {
             AnimateRootFrame = true,
             ColorTableMode = this.ColorTableMode,
+            BackgroundColor = this.BackgroundColor ?? Color.Transparent,
             PixelTypeInfo = this.GetPixelTypeInfo(),
             RepeatCount = this.RepeatCount,
         };

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;

src/ImageSharp/Processing/Processors/Quantization/HexadecatreeQuantizer{TPixel}.cs

@@ -392,11 +392,11 @@ public struct HexadecatreeQuantizer<TPixel> : IQuantizer<TPixel>
         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<short> children;
@@ -510,12 +510,13 @@ public struct HexadecatreeQuantizer<TPixel> : IQuantizer<TPixel>
                     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<short> children = this.Children;
 
                 for (int i = 0; i < children.Length; i++)
@@ -524,7 +525,7 @@ public struct HexadecatreeQuantizer<TPixel> : IQuantizer<TPixel>
                     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;

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<InvalidImageContentException>(() => Image.Load<Rgba32>(stream));
+    }
+
     [Fact]
     public void PlainText_PrematureEof()
     {

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<TPixel>(TestImageProvider<TPixel> provider)
+        where TPixel : unmanaged, IPixel<TPixel>
+    {
+        Image<Rgba32>[] handImages =
+        [
+            Image.Load<Rgba32>(TestFile.Create(TestImages.Png.Issue396Hand1).Bytes),
+            Image.Load<Rgba32>(TestFile.Create(TestImages.Png.Issue396Hand2).Bytes)
+        ];
+
+        try
+        {
+            using Image<TPixel> inputImage = provider.GetImage();
+            using Image<Rgba32> 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<Rgba32> frameImage = new(handImages[i].Width, handImages[i].Height);
+                float squashFactorY = 1 - (i * squashStepFactor);
+
+                frameImage.ProcessPixelRows(accessor =>
+                {
+                    Rgba32 gray = Color.Gray.ToPixel<Rgba32>();
+
+                    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<Rgba32> dragonFrame = inputImage.CloneAs<Rgba32>();
+                dragonFrame.Mutate(context => context.Resize(targetRect.Size));
+
+                frameImage.Mutate(context =>
+                {
+                    context.DrawImage(dragonFrame, targetRect.Location, 1F);
+                    context.DrawImage(handImages[i], Point.Empty, 1F);
+                });
+
+                ImageFrame<Rgba32> 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<Rgba32> 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<Rgba32> decodedImage = Image.Load<Rgba32>(gifStream);
+            decodedImage.DebugSaveMultiFrame(provider, "Issue396-decoded-frames", appendPixelTypeToFileName: false);
+
+            Assert.Equal(outputImage.Frames.Count, decodedImage.Frames.Count);
+        }
+        finally
+        {
+            foreach (Image<Rgba32> handImage in handImages)
+            {
+                handImage.Dispose();
+            }
+        }
+    }
+}

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";

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

tests/Images/External/ReferenceOutput/BmpEncoderTests/Encode_8BitColor_WithOctreeQuantizer_rgb32.bmp

@@ -1,3 +0,0 @@
-version https://git-lfs.github.com/spec/v1
-oid sha256:a98b1ec707af066f77fad7d1a64b858d460986beb6d27682717dd5e221310fd4
-size 9270

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

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

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

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