Fix transparency handling and optimize encoding

src/ImageSharp/Formats/Gif/GifDecoderCore.cs

@@ -479,7 +479,7 @@ internal sealed class GifDecoderCore : IImageDecoderInternals
                 prevFrame = previousFrame;
             }
 
-            currentFrame = image.Frames.AddFrame(previousFrame); // This clones the frame and adds it the collection
+            currentFrame = image.Frames.AddFrame(new ImageFrame<TPixel>(this.configuration, previousFrame.Width, previousFrame.Height));
 
             this.SetFrameMetadata(currentFrame.Metadata, false);
 
@@ -548,7 +548,7 @@ internal sealed class GifDecoderCore : IImageDecoderInternals
                 // #403 The left + width value can be larger than the image width
                 for (int x = descriptorLeft; x < descriptorRight && x < imageWidth; x++)
                 {
-                    int index = Numerics.Clamp(Unsafe.Add(ref indicesRowRef, x - descriptorLeft), 0, colorTableMaxIdx);
+                    int index = Numerics.Clamp(Unsafe.Add(ref indicesRowRef, x - descriptorLeft), 0, Math.Max(transIndex, colorTableMaxIdx));
                     ref TPixel pixel = ref Unsafe.Add(ref rowRef, x);
                     Rgb24 rgb = colorTable[index];
                     pixel.FromRgb24(rgb);
@@ -558,7 +558,7 @@ internal sealed class GifDecoderCore : IImageDecoderInternals
             {
                 for (int x = descriptorLeft; x < descriptorRight && x < imageWidth; x++)
                 {
-                    int index = Numerics.Clamp(Unsafe.Add(ref indicesRowRef, x - descriptorLeft), 0, colorTableMaxIdx);
+                    int index = Numerics.Clamp(Unsafe.Add(ref indicesRowRef, x - descriptorLeft), 0, Math.Max(transIndex, colorTableMaxIdx));
                     if (transIndex != index)
                     {
                         ref TPixel pixel = ref Unsafe.Add(ref rowRef, x);
@@ -596,7 +596,7 @@ internal sealed class GifDecoderCore : IImageDecoderInternals
             return;
         }
 
-        var interest = Rectangle.Intersect(frame.Bounds(), this.restoreArea.Value);
+        Rectangle interest = Rectangle.Intersect(frame.Bounds(), this.restoreArea.Value);
         Buffer2DRegion<TPixel> pixelRegion = frame.PixelBuffer.GetRegion(interest);
         pixelRegion.Clear();
 
@@ -616,6 +616,7 @@ internal sealed class GifDecoderCore : IImageDecoderInternals
             && this.logicalScreenDescriptor.GlobalColorTableSize > 0)
         {
             GifFrameMetadata gifMeta = meta.GetGifMetadata();
+            gifMeta.ColorTableMode = GifColorTableMode.Global;
             gifMeta.ColorTableLength = this.logicalScreenDescriptor.GlobalColorTableSize;
         }
 
@@ -623,6 +624,7 @@ internal sealed class GifDecoderCore : IImageDecoderInternals
             && this.imageDescriptor.LocalColorTableSize > 0)
         {
             GifFrameMetadata gifMeta = meta.GetGifMetadata();
+            gifMeta.ColorTableMode = GifColorTableMode.Local;
             gifMeta.ColorTableLength = this.imageDescriptor.LocalColorTableSize;
         }
 

src/ImageSharp/Formats/Gif/GifEncoderCore.cs

@@ -93,7 +93,6 @@ internal sealed class GifEncoderCore : IImageEncoderInternals
 
         // Quantize the image returning a palette.
         IndexedImageFrame<TPixel> quantized;
-
         using (IQuantizer<TPixel> frameQuantizer = this.quantizer.CreatePixelSpecificQuantizer<TPixel>(this.configuration))
         {
             if (useGlobalTable)
@@ -133,114 +132,102 @@ internal sealed class GifEncoderCore : IImageEncoderInternals
             this.WriteApplicationExtensions(stream, image.Frames.Count, gifMetadata.RepeatCount, xmpProfile);
         }
 
-        if (useGlobalTable)
+        this.EncodeFrames(stream, image, quantized, quantized.Palette.ToArray());
-        {
-            this.EncodeGlobal(image, quantized, index, stream);
-        }
-        else
-        {
-            this.EncodeLocal(image, quantized, stream);
-        }
-
-        // Clean up.
-        quantized.Dispose();
 
         stream.WriteByte(GifConstants.EndIntroducer);
     }
 
-    private void EncodeGlobal<TPixel>(Image<TPixel> image, IndexedImageFrame<TPixel> quantized, int transparencyIndex, Stream stream)
+    private void EncodeFrames<TPixel>(
+        Stream stream,
+        Image<TPixel> image,
+        IndexedImageFrame<TPixel> quantized,
+        ReadOnlyMemory<TPixel> palette)
         where TPixel : unmanaged, IPixel<TPixel>
     {
-        // The palette quantizer can reuse the same pixel map across multiple frames
+        PaletteQuantizer<TPixel> paletteQuantizer = default;
-        // since the palette is unchanging. This allows a reduction of memory usage across
+        bool hasPaletteQuantizer = false;
-        // multi frame gifs using a global palette.
-        PaletteQuantizer<TPixel> paletteFrameQuantizer = default;
-        bool quantizerInitialized = false;
         for (int i = 0; i < image.Frames.Count; i++)
         {
+            // Gather the metadata for this frame.
             ImageFrame<TPixel> frame = image.Frames[i];
             ImageFrameMetadata metadata = frame.Metadata;
+            bool hasMetadata = metadata.TryGetGifMetadata(out GifFrameMetadata frameMetadata);
+            bool useLocal = this.colorTableMode == GifColorTableMode.Local || (hasMetadata && frameMetadata.ColorTableMode == GifColorTableMode.Local);
 
-            bool hasMeta = metadata.TryGetGifMetadata(out GifFrameMetadata frameMetadata);
+            if (!useLocal && !hasPaletteQuantizer && i > 0)
-            if (hasMeta || transparencyIndex > -1)
             {
-                frameMetadata ??= metadata.GetGifMetadata();
+                // The palette quantizer can reuse the same pixel map across multiple frames
-                this.WriteGraphicalControlExtension(frameMetadata, transparencyIndex, stream);
+                // since the palette is unchanging. This allows a reduction of memory usage across
+                // multi frame gifs using a global palette.
+                hasPaletteQuantizer = true;
+                paletteQuantizer = new(this.configuration, this.quantizer.Options, palette);
             }
 
-            this.WriteImageDescriptor(frame, false, stream);
+            this.EncodeFrame(stream, frame, i, useLocal, frameMetadata, ref quantized, ref paletteQuantizer);
 
-            if (i == 0)
+            // Clean up for the next run.
-            {
+            quantized.Dispose();
-                this.WriteImageData(quantized, stream);
+            quantized = null;
-            }
-            else
-            {
-                if (!quantizerInitialized)
-                {
-                    quantizerInitialized = true;
-                    paletteFrameQuantizer = new PaletteQuantizer<TPixel>(this.configuration, this.quantizer.Options, quantized.Palette);
-                }
-
-                using IndexedImageFrame<TPixel> paletteQuantized = paletteFrameQuantizer.QuantizeFrame(frame, frame.Bounds());
-                this.WriteImageData(paletteQuantized, stream);
-            }
         }
 
-        paletteFrameQuantizer.Dispose();
+        paletteQuantizer.Dispose();
     }
 
-    private void EncodeLocal<TPixel>(Image<TPixel> image, IndexedImageFrame<TPixel> quantized, Stream stream)
+    private void EncodeFrame<TPixel>(
+        Stream stream,
+        ImageFrame<TPixel> frame,
+        int frameIndex,
+        bool useLocal,
+        GifFrameMetadata metadata,
+        ref IndexedImageFrame<TPixel> quantized,
+        ref PaletteQuantizer<TPixel> paletteQuantizer)
         where TPixel : unmanaged, IPixel<TPixel>
     {
-        ImageFrame<TPixel> previousFrame = null;
+        // The first frame has already been quantized so we do not need to do so again.
-        GifFrameMetadata previousMeta = null;
+        if (frameIndex > 0)
-        for (int i = 0; i < image.Frames.Count; i++)
         {
-            ImageFrame<TPixel> frame = image.Frames[i];
+            if (useLocal)
-            ImageFrameMetadata metadata = frame.Metadata;
-            bool hasMetadata = metadata.TryGetGifMetadata(out GifFrameMetadata frameMetadata);
-            if (quantized is null)
             {
-                // Allow each frame to be encoded at whatever color depth the frame designates if set.
+                // Reassign using the current frame and details.
-                if (previousFrame != null && frameMetadata != null
+                QuantizerOptions options = null;
-                    && previousMeta.ColorTableLength != frameMetadata.ColorTableLength
+                int colorTableLength = metadata?.ColorTableLength ?? 0;
-                    && frameMetadata.ColorTableLength > 0)
+                if (colorTableLength > 0)
                 {
-                    QuantizerOptions options = new()
+                    options = new()
                     {
                         Dither = this.quantizer.Options.Dither,
                         DitherScale = this.quantizer.Options.DitherScale,
-                        MaxColors = frameMetadata.ColorTableLength
+                        MaxColors = colorTableLength
                     };
-
-                    using IQuantizer<TPixel> frameQuantizer = this.quantizer.CreatePixelSpecificQuantizer<TPixel>(this.configuration, options);
-                    quantized = frameQuantizer.BuildPaletteAndQuantizeFrame(frame, frame.Bounds());
-                }
-                else
-                {
-                    using IQuantizer<TPixel> frameQuantizer = this.quantizer.CreatePixelSpecificQuantizer<TPixel>(this.configuration);
-                    quantized = frameQuantizer.BuildPaletteAndQuantizeFrame(frame, frame.Bounds());
                 }
-            }
 
-            this.bitDepth = ColorNumerics.GetBitsNeededForColorDepth(quantized.Palette.Length);
+                using IQuantizer<TPixel> frameQuantizer = this.quantizer.CreatePixelSpecificQuantizer<TPixel>(this.configuration, options ?? this.quantizer.Options);
-            int index = GetTransparentIndex(quantized);
+                quantized = frameQuantizer.BuildPaletteAndQuantizeFrame(frame, frame.Bounds());
-            if (hasMetadata || index > -1)
+            }
+            else
             {
-                frameMetadata ??= metadata.GetGifMetadata();
+                // Quantize the image using the global palette.
-                this.WriteGraphicalControlExtension(frameMetadata, index, stream);
+                quantized = paletteQuantizer.QuantizeFrame(frame, frame.Bounds());
             }
 
-            this.WriteImageDescriptor(frame, true, stream);
+            this.bitDepth = ColorNumerics.GetBitsNeededForColorDepth(quantized.Palette.Length);
-            this.WriteColorTable(quantized, stream);
+        }
-            this.WriteImageData(quantized, stream);
 
-            quantized.Dispose();
+        // Do we have extension information to write?
-            quantized = null; // So next frame can regenerate it
+        int index = GetTransparentIndex(quantized);
-            previousFrame = frame;
+        if (metadata != null || index > -1)
-            previousMeta = frameMetadata;
+        {
+            this.WriteGraphicalControlExtension(metadata ?? new(), index, stream);
+        }
+
+        this.WriteImageDescriptor(frame, useLocal, stream);
+
+        if (useLocal)
+        {
+            this.WriteColorTable(quantized, stream);
         }
+
+        this.WriteImageData(quantized, stream);
     }
 
     /// <summary>

src/ImageSharp/Formats/Gif/GifFrameMetadata.cs

@@ -21,13 +21,19 @@ public class GifFrameMetadata : IDeepCloneable
     /// <param name="other">The metadata to create an instance from.</param>
     private GifFrameMetadata(GifFrameMetadata other)
     {
+        this.ColorTableMode = other.ColorTableMode;
         this.ColorTableLength = other.ColorTableLength;
         this.FrameDelay = other.FrameDelay;
         this.DisposalMethod = other.DisposalMethod;
     }
 
     /// <summary>
-    /// Gets or sets the length of the color table for paletted images.
+    /// Gets or sets the color table mode.
+    /// </summary>
+    public GifColorTableMode ColorTableMode { get; set; }
+
+    /// <summary>
+    /// Gets or sets the length of the color table.
     /// If not 0, then this field indicates the maximum number of colors to use when quantizing the
     /// image frame.
     /// </summary>

src/ImageSharp/Formats/Gif/GifMetadata.cs

@@ -50,7 +50,7 @@ public class GifMetadata : IDeepCloneable
     public int GlobalColorTableLength { get; set; }
 
     /// <summary>
-    /// Gets or sets the the collection of comments about the graphics, credits, descriptions or any
+    /// Gets or sets the collection of comments about the graphics, credits, descriptions or any
     /// other type of non-control and non-graphic data.
     /// </summary>
     public IList<string> Comments { get; set; } = new List<string>();

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

@@ -106,7 +106,7 @@ public struct OctreeQuantizer<TPixel> : IQuantizer<TPixel>
         // for higher bit depths. Lower bit depths will correctly reduce the palette.
         // TODO: Investigate more evenly reduced palette reduction.
         int max = this.maxColors;
-        if (this.bitDepth == 8)
+        if (this.bitDepth >= 4)
         {
             max--;
         }

src/ImageSharp/Processing/Processors/Quantization/PaletteQuantizer.cs

@@ -49,12 +49,8 @@ public class PaletteQuantizer : IQuantizer
     {
         Guard.NotNull(options, nameof(options));
 
-        // The palette quantizer can reuse the same pixel map across multiple frames
+        // Always use the palette length over options since the palette cannot be reduced.
-        // since the palette is unchanging. This allows a reduction of memory usage across
+        TPixel[] palette = new TPixel[this.colorPalette.Length];
-        // multi frame gifs using a global palette.
-        int length = Math.Min(this.colorPalette.Length, options.MaxColors);
-        TPixel[] palette = new TPixel[length];
-
         Color.ToPixel(configuration, this.colorPalette.Span, palette.AsSpan());
         return new PaletteQuantizer<TPixel>(configuration, options, palette);
     }

tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs

@@ -204,6 +204,7 @@ public class GifEncoderTests
 
     [Theory]
     [WithFile(TestImages.Gif.Issues.Issue2288OptionalExtension, PixelTypes.Rgba32)]
+    [WithFile(TestImages.Gif.Issues.Issue2288OptionalExtension2, PixelTypes.Rgba32)]
     public void OptionalExtensionsShouldBeHandledProperly<TPixel>(TestImageProvider<TPixel> provider)
         where TPixel : unmanaged, IPixel<TPixel>
     {

tests/ImageSharp.Tests/TestImages.cs

@@ -477,6 +477,7 @@ public static class TestImages
             public const string Issue2012EmptyXmp = "Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif";
             public const string Issue2012BadMinCode = "Gif/issues/issue2012_drona1.gif";
             public const string Issue2288OptionalExtension = "Gif/issues/issue_2288.gif";
+            public const string Issue2288OptionalExtension2 = "Gif/issues/issue_2288_2.gif";
         }
 
         public static readonly string[] All = { Rings, Giphy, Cheers, Trans, Kumin, Leo, Ratio4x1, Ratio1x4 };

tests/Images/Input/Gif/issues/issue_2288_2.gif

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