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

src/ImageSharp/Formats/Gif/GifDecoderCore.cs

@@ -89,6 +89,9 @@ internal sealed class GifDecoderCore : ImageDecoderCore
     /// </summary>
     private GifMetadata? gifMetadata;
 
+    // The background color used to fill the frame.
+    private Color backgroundColor;
+
     /// <summary>
     /// Initializes a new instance of the <see cref="GifDecoderCore"/> class.
     /// </summary>
@@ -108,9 +111,12 @@ protected override Image<TPixel> Decode<TPixel>(BufferedReadStream stream, Cance
         uint frameCount = 0;
         Image<TPixel>? image = null;
         ImageFrame<TPixel>? previousFrame = null;
+        GifDisposalMethod? previousDisposalMethod = null;
+
         try
         {
             this.ReadLogicalScreenDescriptorAndGlobalColorTable(stream);
+            TPixel backgroundPixel = this.backgroundColor.ToPixel<TPixel>();
 
             // Loop though the respective gif parts and read the data.
             int nextFlag = stream.ReadByte();
@@ -123,7 +129,7 @@ protected override Image<TPixel> Decode<TPixel>(BufferedReadStream stream, Cance
                         break;
                     }
 
-                    this.ReadFrame(stream, ref image, ref previousFrame);
+                    this.ReadFrame(stream, ref image, ref previousFrame, ref previousDisposalMethod, backgroundPixel);
 
                     // Reset per-frame state.
                     this.imageDescriptor = default;
@@ -417,7 +423,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="backgroundPixel">The background color pixel.</param>
+    private void ReadFrame<TPixel>(
+        BufferedReadStream stream,
+        ref Image<TPixel>? image,
+        ref ImageFrame<TPixel>? previousFrame,
+        ref GifDisposalMethod? previousDisposalMethod,
+        TPixel backgroundPixel)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         this.ReadImageDescriptor(stream);
@@ -444,7 +457,7 @@ 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);
+        this.ReadFrameColors(stream, ref image, ref previousFrame, ref previousDisposalMethod, colorTable, this.imageDescriptor, backgroundPixel);
 
         // Skip any remaining blocks
         SkipBlock(stream);
@@ -457,57 +470,58 @@ 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;
 
-        ImageFrame<TPixel>? prevFrame = null;
-        ImageFrame<TPixel>? currentFrame = null;
-        ImageFrame<TPixel> imageFrame;
+        GifDisposalMethod disposalMethod = this.graphicsControlExtension.DisposalMethod;
 
+        ImageFrame<TPixel> currentFrame;
         if (previousFrame is null)
         {
-            if (!transFlag)
-            {
-                image = new Image<TPixel>(this.configuration, imageWidth, imageHeight, Color.Black.ToPixel<TPixel>(), this.metadata);
-            }
-            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);
-            }
+            image = transFlag
+                ? new Image<TPixel>(this.configuration, imageWidth, imageHeight, this.metadata)
+                : new Image<TPixel>(this.configuration, imageWidth, imageHeight, backgroundPixel, this.metadata);
 
             this.SetFrameMetadata(image.Frames.RootFrame.Metadata);
-
-            imageFrame = image.Frames.RootFrame;
+            currentFrame = image.Frames.RootFrame;
         }
         else
         {
-            if (this.graphicsControlExtension.DisposalMethod == GifDisposalMethod.RestoreToPrevious)
-            {
-                prevFrame = 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);
 
             this.SetFrameMetadata(currentFrame.Metadata);
 
-            imageFrame = currentFrame;
+            if (previousDisposalMethod == GifDisposalMethod.RestoreToBackground)
+            {
+                this.RestoreToBackground(currentFrame, backgroundPixel, transFlag);
+            }
+        }
+
+        Rectangle interest = Rectangle.Intersect(image.Bounds, new(descriptor.Left, descriptor.Top, descriptor.Width, descriptor.Height));
+        previousFrame = currentFrame;
+        previousDisposalMethod = disposalMethod;
 
-            this.RestoreToBackground(imageFrame);
+        if (disposalMethod == GifDisposalMethod.RestoreToBackground)
+        {
+            this.restoreArea = interest;
         }
 
         if (colorTable.Length == 0)
@@ -573,7 +587,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 +619,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 +639,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 +668,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 +680,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 +802,18 @@ private void ReadLogicalScreenDescriptorAndGlobalColorTable(BufferedReadStream s
             }
         }
 
-        this.gifMetadata.BackgroundColorIndex = this.logicalScreenDescriptor.BackgroundColorIndex;
+        // If the global color table is present, we can set the background color
+        // otherwise we default to transparent to match browser behavior.
+        ReadOnlyMemory<Color>? table = this.gifMetadata.GlobalColorTable;
+        byte index = this.logicalScreenDescriptor.BackgroundColorIndex;
+        if (table is not null && index < table.Value.Length)
+        {
+            this.backgroundColor = table.Value.Span[index];
+        }
+        else
+        {
+            this.backgroundColor = Color.Transparent;
+        }
     }
 
     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();
         }
     }
 

src/ImageSharp/Formats/Gif/MetadataExtensions.cs

@@ -60,21 +60,15 @@ public static bool TryGetGifMetadata(this ImageFrameMetadata source, [NotNullWhe
         => source.TryGetFormatMetadata(GifFormat.Instance, out metadata);
 
     internal static AnimatedImageMetadata ToAnimatedImageMetadata(this GifMetadata source)
-    {
-        Color background = Color.Transparent;
-        if (source.GlobalColorTable != null)
-        {
-            background = source.GlobalColorTable.Value.Span[source.BackgroundColorIndex];
-        }
 
-        return new()
+        // We cannot trust the global GIF palette so don't use it.
+        // https://github.com/SixLabors/ImageSharp/issues/2866
+        => new()
         {
-            ColorTable = source.GlobalColorTable,
             ColorTableMode = source.ColorTableMode == GifColorTableMode.Global ? FrameColorTableMode.Global : FrameColorTableMode.Local,
             RepeatCount = source.RepeatCount,
-            BackgroundColor = background,
+            BackgroundColor = Color.Transparent,
         };
-    }
 
     internal static AnimatedImageFrameMetadata ToAnimatedImageFrameMetadata(this GifFrameMetadata source)
     {

src/ImageSharp/Formats/Webp/BitReader/BitReaderBase.cs

@@ -32,7 +32,7 @@ protected BitReaderBase(Stream inputStream, int imageDataSize, MemoryAllocator m
     /// <param name="memoryAllocator">Used for allocating memory during reading data from the stream.</param>
     protected static IMemoryOwner<byte> ReadImageDataFromStream(Stream input, int bytesToRead, MemoryAllocator memoryAllocator)
     {
-        IMemoryOwner<byte> data = memoryAllocator.Allocate<byte>(bytesToRead);
+        IMemoryOwner<byte> data = memoryAllocator.Allocate<byte>(bytesToRead, AllocationOptions.Clean);
         Span<byte> dataSpan = data.Memory.Span;
         input.Read(dataSpan[..bytesToRead], 0, bytesToRead);
 

src/ImageSharp/Formats/Webp/Lossy/Vp8Decoder.cs

@@ -67,14 +67,14 @@ public Vp8Decoder(Vp8FrameHeader frameHeader, Vp8PictureHeader pictureHeader, Vp
         int extraY = extraRows * this.CacheYStride;
         int extraUv = extraRows / 2 * this.CacheUvStride;
         this.YuvBuffer = memoryAllocator.Allocate<byte>((WebpConstants.Bps * 17) + (WebpConstants.Bps * 9) + extraY);
-        this.CacheY = memoryAllocator.Allocate<byte>((16 * this.CacheYStride) + extraY);
+        this.CacheY = memoryAllocator.Allocate<byte>((16 * this.CacheYStride) + extraY, AllocationOptions.Clean);
         int cacheUvSize = (16 * this.CacheUvStride) + extraUv;
-        this.CacheU = memoryAllocator.Allocate<byte>(cacheUvSize);
-        this.CacheV = memoryAllocator.Allocate<byte>(cacheUvSize);
-        this.TmpYBuffer = memoryAllocator.Allocate<byte>((int)width);
-        this.TmpUBuffer = memoryAllocator.Allocate<byte>((int)width);
-        this.TmpVBuffer = memoryAllocator.Allocate<byte>((int)width);
-        this.Pixels = memoryAllocator.Allocate<byte>((int)(width * height * 4));
+        this.CacheU = memoryAllocator.Allocate<byte>(cacheUvSize, AllocationOptions.Clean);
+        this.CacheV = memoryAllocator.Allocate<byte>(cacheUvSize, AllocationOptions.Clean);
+        this.TmpYBuffer = memoryAllocator.Allocate<byte>((int)width, AllocationOptions.Clean);
+        this.TmpUBuffer = memoryAllocator.Allocate<byte>((int)width, AllocationOptions.Clean);
+        this.TmpVBuffer = memoryAllocator.Allocate<byte>((int)width, AllocationOptions.Clean);
+        this.Pixels = memoryAllocator.Allocate<byte>((int)(width * height * 4), AllocationOptions.Clean);
 
 #if DEBUG
         // Filling those buffers with 205, is only useful for debugging,