V3 : Fix GIF, PNG, and WEBP Edge Case Handling

src/ImageSharp/Formats/Gif/GifDecoderCore.cs

@@ -89,6 +89,11 @@ internal sealed class GifDecoderCore : ImageDecoderCore
     /// </summary>
     private GifMetadata? gifMetadata;
 
+    /// <summary>
+    /// The background color index.
+    /// </summary>
+    private byte backgroundColorIndex;
+
     /// <summary>
     /// Initializes a new instance of the <see cref="GifDecoderCore"/> class.
     /// </summary>
@@ -108,6 +113,10 @@ protected override Image<TPixel> Decode<TPixel>(BufferedReadStream stream, Cance
         uint frameCount = 0;
         Image<TPixel>? image = null;
         ImageFrame<TPixel>? previousFrame = null;
+        GifDisposalMethod? previousDisposalMethod = null;
+        bool globalColorTableUsed = false;
+        Color backgroundColor = Color.Transparent;
+
         try
         {
             this.ReadLogicalScreenDescriptorAndGlobalColorTable(stream);
@@ -123,7 +132,7 @@ protected override Image<TPixel> Decode<TPixel>(BufferedReadStream stream, Cance
                         break;
                     }
 
-                    this.ReadFrame(stream, ref image, ref previousFrame);
+                    globalColorTableUsed |= this.ReadFrame(stream, ref image, ref previousFrame, ref previousDisposalMethod, ref backgroundColor);
 
                     // Reset per-frame state.
                     this.imageDescriptor = default;
@@ -158,6 +167,13 @@ protected override Image<TPixel> Decode<TPixel>(BufferedReadStream stream, Cance
                     break;
                 }
             }
+
+            // We cannot always trust the global GIF palette has actually been used.
+            // https://github.com/SixLabors/ImageSharp/issues/2866
+            if (!globalColorTableUsed)
+            {
+                this.gifMetadata.ColorTableMode = GifColorTableMode.Local;
+            }
         }
         finally
         {
@@ -417,7 +433,14 @@ private void ReadComments(BufferedReadStream stream)
     /// <param name="stream">The <see cref="BufferedReadStream"/> containing image data.</param>
     /// <param name="image">The image to decode the information to.</param>
     /// <param name="previousFrame">The previous frame.</param>
-    private void ReadFrame<TPixel>(BufferedReadStream stream, ref Image<TPixel>? image, ref ImageFrame<TPixel>? previousFrame)
+    /// <param name="previousDisposalMethod">The previous disposal method.</param>
+    /// <param name="backgroundColor">The background color.</param>
+    private bool ReadFrame<TPixel>(
+        BufferedReadStream stream,
+        ref Image<TPixel>? image,
+        ref ImageFrame<TPixel>? previousFrame,
+        ref GifDisposalMethod? previousDisposalMethod,
+        ref Color backgroundColor)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         this.ReadImageDescriptor(stream);
@@ -444,10 +467,52 @@ private void ReadFrame<TPixel>(BufferedReadStream stream, ref Image<TPixel>? ima
         }
 
         ReadOnlySpan<Rgb24> colorTable = MemoryMarshal.Cast<byte, Rgb24>(rawColorTable);
-        this.ReadFrameColors(stream, ref image, ref previousFrame, colorTable, this.imageDescriptor);
+
+        // First frame
+        if (image is null)
+        {
+            if (this.backgroundColorIndex < colorTable.Length)
+            {
+                backgroundColor = colorTable[this.backgroundColorIndex];
+            }
+            else
+            {
+                backgroundColor = Color.Transparent;
+            }
+
+            if (this.graphicsControlExtension.TransparencyFlag)
+            {
+                backgroundColor = backgroundColor.WithAlpha(0);
+            }
+        }
+
+        this.ReadFrameColors(stream, ref image, ref previousFrame, ref previousDisposalMethod, colorTable, this.imageDescriptor, backgroundColor.ToPixel<TPixel>());
+
+        // Update from newly decoded frame.
+        if (this.graphicsControlExtension.DisposalMethod != GifDisposalMethod.RestoreToPrevious)
+        {
+            if (this.backgroundColorIndex < colorTable.Length)
+            {
+                backgroundColor = colorTable[this.backgroundColorIndex];
+            }
+            else
+            {
+                backgroundColor = Color.Transparent;
+            }
+
+            // TODO: I don't understand why this is always set to alpha of zero.
+            // This should be dependent on the transparency flag of the graphics
+            // control extension. ImageMagick does the same.
+            // if (this.graphicsControlExtension.TransparencyFlag)
+            {
+                backgroundColor = backgroundColor.WithAlpha(0);
+            }
+        }
 
         // Skip any remaining blocks
         SkipBlock(stream);
+
+        return !hasLocalColorTable;
     }
 
     /// <summary>
@@ -457,57 +522,74 @@ private void ReadFrame<TPixel>(BufferedReadStream stream, ref Image<TPixel>? ima
     /// <param name="stream">The <see cref="BufferedReadStream"/> containing image data.</param>
     /// <param name="image">The image to decode the information to.</param>
     /// <param name="previousFrame">The previous frame.</param>
+    /// <param name="previousDisposalMethod">The previous disposal method.</param>
     /// <param name="colorTable">The color table containing the available colors.</param>
     /// <param name="descriptor">The <see cref="GifImageDescriptor"/></param>
+    /// <param name="backgroundPixel">The background color pixel.</param>
     private void ReadFrameColors<TPixel>(
         BufferedReadStream stream,
         ref Image<TPixel>? image,
         ref ImageFrame<TPixel>? previousFrame,
+        ref GifDisposalMethod? previousDisposalMethod,
         ReadOnlySpan<Rgb24> colorTable,
-        in GifImageDescriptor descriptor)
+        in GifImageDescriptor descriptor,
+        TPixel backgroundPixel)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         int imageWidth = this.logicalScreenDescriptor.Width;
         int imageHeight = this.logicalScreenDescriptor.Height;
         bool transFlag = this.graphicsControlExtension.TransparencyFlag;
+        GifDisposalMethod disposalMethod = this.graphicsControlExtension.DisposalMethod;
+        ImageFrame<TPixel> currentFrame;
+        ImageFrame<TPixel>? restoreFrame = null;
 
-        ImageFrame<TPixel>? prevFrame = null;
-        ImageFrame<TPixel>? currentFrame = null;
-        ImageFrame<TPixel> imageFrame;
+        if (previousFrame is null && previousDisposalMethod is null)
+        {
+            image = transFlag
+                ? new Image<TPixel>(this.configuration, imageWidth, imageHeight, this.metadata)
+                : new Image<TPixel>(this.configuration, imageWidth, imageHeight, backgroundPixel, this.metadata);
 
-        if (previousFrame is null)
+            this.SetFrameMetadata(image.Frames.RootFrame.Metadata);
+            currentFrame = image.Frames.RootFrame;
+        }
+        else
         {
-            if (!transFlag)
+            if (previousFrame != null)
             {
-                image = new Image<TPixel>(this.configuration, imageWidth, imageHeight, Color.Black.ToPixel<TPixel>(), this.metadata);
+                currentFrame = image!.Frames.AddFrame(previousFrame);
             }
             else
             {
-                // This initializes the image to become fully transparent because the alpha channel is zero.
-                image = new Image<TPixel>(this.configuration, imageWidth, imageHeight, this.metadata);
+                currentFrame = image!.Frames.CreateFrame(backgroundPixel);
             }
 
-            this.SetFrameMetadata(image.Frames.RootFrame.Metadata);
+            this.SetFrameMetadata(currentFrame.Metadata);
 
-            imageFrame = image.Frames.RootFrame;
-        }
-        else
-        {
             if (this.graphicsControlExtension.DisposalMethod == GifDisposalMethod.RestoreToPrevious)
             {
-                prevFrame = previousFrame;
+                restoreFrame = previousFrame;
             }
 
-            // We create a clone of the 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
-            currentFrame = image!.Frames.AddFrame(previousFrame);
+            if (previousDisposalMethod == GifDisposalMethod.RestoreToBackground)
+            {
+                this.RestoreToBackground(currentFrame, backgroundPixel, transFlag);
+            }
+        }
 
-            this.SetFrameMetadata(currentFrame.Metadata);
+        if (this.graphicsControlExtension.DisposalMethod == GifDisposalMethod.RestoreToPrevious)
+        {
+            previousFrame = restoreFrame;
+        }
+        else
+        {
+            previousFrame = currentFrame;
+        }
 
-            imageFrame = currentFrame;
+        previousDisposalMethod = disposalMethod;
 
-            this.RestoreToBackground(imageFrame);
+        if (disposalMethod == GifDisposalMethod.RestoreToBackground)
+        {
+            this.restoreArea = Rectangle.Intersect(image.Bounds, new(descriptor.Left, descriptor.Top, descriptor.Width, descriptor.Height));
         }
 
         if (colorTable.Length == 0)
@@ -573,7 +655,7 @@ private void ReadFrameColors<TPixel>(
                 }
 
                 lzwDecoder.DecodePixelRow(indicesRow);
-                ref TPixel rowRef = ref MemoryMarshal.GetReference(imageFrame.PixelBuffer.DangerousGetRowSpan(writeY));
+                ref TPixel rowRef = ref MemoryMarshal.GetReference(currentFrame.PixelBuffer.DangerousGetRowSpan(writeY));
 
                 if (!transFlag)
                 {
@@ -605,19 +687,6 @@ private void ReadFrameColors<TPixel>(
                 }
             }
         }
-
-        if (prevFrame != null)
-        {
-            previousFrame = prevFrame;
-            return;
-        }
-
-        previousFrame = currentFrame ?? image.Frames.RootFrame;
-
-        if (this.graphicsControlExtension.DisposalMethod == GifDisposalMethod.RestoreToBackground)
-        {
-            this.restoreArea = new Rectangle(descriptor.Left, descriptor.Top, descriptor.Width, descriptor.Height);
-        }
     }
 
     /// <summary>
@@ -638,6 +707,11 @@ private void ReadFrameMetadata(BufferedReadStream stream, List<ImageFrameMetadat
             this.currentLocalColorTable ??= this.configuration.MemoryAllocator.Allocate<byte>(768, AllocationOptions.Clean);
             stream.Read(this.currentLocalColorTable.GetSpan()[..length]);
         }
+        else
+        {
+            this.currentLocalColorTable = null;
+            this.currentLocalColorTableSize = 0;
+        }
 
         // Skip the frame indices. Pixels length + mincode size.
         // The gif format does not tell us the length of the compressed data beforehand.
@@ -662,7 +736,9 @@ private void ReadFrameMetadata(BufferedReadStream stream, List<ImageFrameMetadat
     /// </summary>
     /// <typeparam name="TPixel">The pixel format.</typeparam>
     /// <param name="frame">The frame.</param>
-    private void RestoreToBackground<TPixel>(ImageFrame<TPixel> frame)
+    /// <param name="background">The background color.</param>
+    /// <param name="transparent">Whether the background is transparent.</param>
+    private void RestoreToBackground<TPixel>(ImageFrame<TPixel> frame, TPixel background, bool transparent)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         if (this.restoreArea is null)
@@ -672,7 +748,14 @@ private void RestoreToBackground<TPixel>(ImageFrame<TPixel> frame)
 
         Rectangle interest = Rectangle.Intersect(frame.Bounds(), this.restoreArea.Value);
         Buffer2DRegion<TPixel> pixelRegion = frame.PixelBuffer.GetRegion(interest);
-        pixelRegion.Clear();
+        if (transparent)
+        {
+            pixelRegion.Clear();
+        }
+        else
+        {
+            pixelRegion.Fill(background);
+        }
 
         this.restoreArea = null;
     }
@@ -787,7 +870,9 @@ private void ReadLogicalScreenDescriptorAndGlobalColorTable(BufferedReadStream s
             }
         }
 
-        this.gifMetadata.BackgroundColorIndex = this.logicalScreenDescriptor.BackgroundColorIndex;
+        byte index = this.logicalScreenDescriptor.BackgroundColorIndex;
+        this.backgroundColorIndex = index;
+        this.gifMetadata.BackgroundColorIndex = index;
     }
 
     private unsafe struct ScratchBuffer

src/ImageSharp/Formats/Gif/GifEncoderCore.cs

@@ -88,18 +88,22 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
         GifMetadata gifMetadata = GetGifMetadata(image);
         this.colorTableMode ??= gifMetadata.ColorTableMode;
         bool useGlobalTable = this.colorTableMode == GifColorTableMode.Global;
-
-        // Quantize the first image frame returning a palette.
-        IndexedImageFrame<TPixel>? quantized = null;
+        bool useGlobalTableForFirstFrame = useGlobalTable;
 
         // Work out if there is an explicit transparent index set for the frame. We use that to ensure the
         // correct value is set for the background index when quantizing.
         GifFrameMetadata frameMetadata = GetGifFrameMetadata(image.Frames.RootFrame, -1);
+        if (frameMetadata.ColorTableMode == GifColorTableMode.Local)
+        {
+            useGlobalTableForFirstFrame = false;
+        }
 
+        // Quantize the first image frame returning a palette.
+        IndexedImageFrame<TPixel>? quantized = null;
         if (this.quantizer is null)
         {
             // Is this a gif with color information. If so use that, otherwise use octree.
-            if (gifMetadata.ColorTableMode == GifColorTableMode.Global && gifMetadata.GlobalColorTable?.Length > 0)
+            if (useGlobalTable && gifMetadata.GlobalColorTable?.Length > 0)
             {
                 // We avoid dithering by default to preserve the original colors.
                 int transparencyIndex = GetTransparentIndex(quantized, frameMetadata);
@@ -118,8 +122,9 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
             }
         }
 
-        using (IQuantizer<TPixel> frameQuantizer = this.quantizer.CreatePixelSpecificQuantizer<TPixel>(this.configuration))
+        if (useGlobalTableForFirstFrame)
         {
+            using IQuantizer<TPixel> frameQuantizer = this.quantizer.CreatePixelSpecificQuantizer<TPixel>(this.configuration);
             if (useGlobalTable)
             {
                 frameQuantizer.BuildPalette(this.pixelSamplingStrategy, image);
@@ -131,6 +136,17 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
                 quantized = frameQuantizer.QuantizeFrame(image.Frames.RootFrame, image.Bounds);
             }
         }
+        else
+        {
+            quantized = this.QuantizeAdditionalFrameAndUpdateMetadata(
+                image.Frames.RootFrame,
+                image.Frames.RootFrame.Bounds(),
+                frameMetadata,
+                true,
+                default,
+                false,
+                frameMetadata.HasTransparency ? frameMetadata.TransparencyIndex : -1);
+        }
 
         // Write the header.
         WriteHeader(stream);
@@ -243,8 +259,8 @@ private void EncodeAdditionalFrames<TPixel>(
             return;
         }
 
-        PaletteQuantizer<TPixel> paletteQuantizer = default;
-        bool hasPaletteQuantizer = false;
+        PaletteQuantizer<TPixel> globalPaletteQuantizer = default;
+        bool hasGlobalPaletteQuantizer = false;
 
         // Store the first frame as a reference for de-duplication comparison.
         ImageFrame<TPixel> previousFrame = image.Frames.RootFrame;
@@ -260,14 +276,14 @@ private void EncodeAdditionalFrames<TPixel>(
             GifFrameMetadata gifMetadata = GetGifFrameMetadata(currentFrame, globalTransparencyIndex);
             bool useLocal = this.colorTableMode == GifColorTableMode.Local || (gifMetadata.ColorTableMode == GifColorTableMode.Local);
 
-            if (!useLocal && !hasPaletteQuantizer && i > 0)
+            if (!useLocal && !hasGlobalPaletteQuantizer && i > 0)
             {
                 // The palette quantizer can reuse the same global pixel map across multiple frames since the palette is unchanging.
                 // This allows a reduction of memory usage across multi-frame gifs using a global palette
                 // and also allows use to reuse the cache from previous runs.
                 int transparencyIndex = gifMetadata.HasTransparency ? gifMetadata.TransparencyIndex : -1;
-                paletteQuantizer = new(this.configuration, this.quantizer!.Options, globalPalette, transparencyIndex);
-                hasPaletteQuantizer = true;
+                globalPaletteQuantizer = new(this.configuration, this.quantizer!.Options, globalPalette, transparencyIndex);
+                hasGlobalPaletteQuantizer = true;
             }
 
             this.EncodeAdditionalFrame(
@@ -278,16 +294,16 @@ private void EncodeAdditionalFrames<TPixel>(
                 encodingFrame,
                 useLocal,
                 gifMetadata,
-                paletteQuantizer,
+                globalPaletteQuantizer,
                 previousDisposalMethod);
 
             previousFrame = currentFrame;
             previousDisposalMethod = gifMetadata.DisposalMethod;
         }
 
-        if (hasPaletteQuantizer)
+        if (hasGlobalPaletteQuantizer)
         {
-            paletteQuantizer.Dispose();
+            globalPaletteQuantizer.Dispose();
         }
     }