Fix WEBP animation disposal and blending

JimBobSquarePants · JimBobSquarePants · commit ef8c79d1c267 · 2025-02-07T09:40:24.000+10:00
diff --git a/src/ImageSharp/Formats/Webp/BitReader/BitReaderBase.cs b/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);
 
diff --git a/src/ImageSharp/Formats/Webp/Lossy/Vp8Decoder.cs b/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,
diff --git a/src/ImageSharp/Formats/Webp/WebpAnimationDecoder.cs b/src/ImageSharp/Formats/Webp/WebpAnimationDecoder.cs
@@ -81,16 +81,29 @@ public WebpAnimationDecoder(MemoryAllocator memoryAllocator, Configuration confi
     /// <param name="width">The width of the image.</param>
     /// <param name="height">The height of the image.</param>
     /// <param name="completeDataSize">The size of the image data in bytes.</param>
-    public Image<TPixel> Decode<TPixel>(BufferedReadStream stream, WebpFeatures features, uint width, uint height, uint completeDataSize)
+    public Image<TPixel> Decode<TPixel>(
+        BufferedReadStream stream,
+        WebpFeatures features,
+        uint width,
+        uint height,
+        uint completeDataSize)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         Image<TPixel>? image = null;
         ImageFrame<TPixel>? previousFrame = null;
+        WebpFrameData? prevFrameData = null;
 
         this.metadata = new ImageMetadata();
         this.webpMetadata = this.metadata.GetWebpMetadata();
         this.webpMetadata.RepeatCount = features.AnimationLoopCount;
 
+        Color backgroundColor = this.backgroundColorHandling == BackgroundColorHandling.Ignore
+            ? Color.Transparent
+            : features.AnimationBackgroundColor!.Value;
+
+        this.webpMetadata.BackgroundColor = backgroundColor;
+        TPixel backgroundPixel = backgroundColor.ToPixel<TPixel>();
+
         Span<byte> buffer = stackalloc byte[4];
         uint frameCount = 0;
         int remainingBytes = (int)completeDataSize;
@@ -101,10 +114,15 @@ public Image<TPixel> Decode<TPixel>(BufferedReadStream stream, WebpFeatures feat
             switch (chunkType)
             {
                 case WebpChunkType.FrameData:
-                    Color backgroundColor = this.backgroundColorHandling == BackgroundColorHandling.Ignore
-                        ? new Color(new Bgra32(0, 0, 0, 0))
-                        : features.AnimationBackgroundColor!.Value;
-                    uint dataSize = this.ReadFrame(stream, ref image, ref previousFrame, width, height, backgroundColor);
+                    uint dataSize = this.ReadFrame(
+                        stream,
+                        ref image,
+                        ref previousFrame,
+                        ref prevFrameData,
+                        width,
+                        height,
+                        backgroundPixel);
+
                     remainingBytes -= (int)dataSize;
                     break;
                 case WebpChunkType.Xmp:
@@ -132,10 +150,18 @@ public Image<TPixel> Decode<TPixel>(BufferedReadStream stream, WebpFeatures feat
     /// <param name="stream">The stream, where the image should be decoded from. Cannot be null.</param>
     /// <param name="image">The image to decode the information to.</param>
     /// <param name="previousFrame">The previous frame.</param>
+    /// <param name="prevFrameData">The previous frame data.</param>
     /// <param name="width">The width of the image.</param>
     /// <param name="height">The height of the image.</param>
     /// <param name="backgroundColor">The default background color of the canvas in.</param>
-    private uint ReadFrame<TPixel>(BufferedReadStream stream, ref Image<TPixel>? image, ref ImageFrame<TPixel>? previousFrame, uint width, uint height, Color backgroundColor)
+    private uint ReadFrame<TPixel>(
+        BufferedReadStream stream,
+        ref Image<TPixel>? image,
+        ref ImageFrame<TPixel>? previousFrame,
+        ref WebpFrameData? prevFrameData,
+        uint width,
+        uint height,
+        TPixel backgroundColor)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         WebpFrameData frameData = WebpFrameData.Parse(stream);
@@ -174,40 +200,54 @@ private uint ReadFrame<TPixel>(BufferedReadStream stream, ref Image<TPixel>? ima
                 break;
         }
 
-        ImageFrame<TPixel>? currentFrame = null;
-        ImageFrame<TPixel> imageFrame;
+        bool isKeyFrame = false;
+        ImageFrame<TPixel> currentFrame;
         if (previousFrame is null)
         {
-            image = new Image<TPixel>(this.configuration, (int)width, (int)height, backgroundColor.ToPixel<TPixel>(), this.metadata);
+            image = new Image<TPixel>(this.configuration, (int)width, (int)height, backgroundColor, this.metadata);
 
             SetFrameMetadata(image.Frames.RootFrame.Metadata, frameData);
 
-            imageFrame = image.Frames.RootFrame;
+            currentFrame = image.Frames.RootFrame;
+            isKeyFrame = true;
         }
         else
         {
-            currentFrame = image!.Frames.AddFrame(previousFrame); // This clones the frame and adds it the collection.
+            // If the frame is a key frame we do not need to clone the frame or clear it.
+            isKeyFrame = prevFrameData?.DisposalMethod is WebpDisposalMethod.RestoreToBackground
+                && this.restoreArea == image!.Bounds;
 
-            SetFrameMetadata(currentFrame.Metadata, frameData);
+            if (isKeyFrame)
+            {
+                currentFrame = image!.Frames.CreateFrame(backgroundColor);
+            }
+            else
+            {
+                // This clones the frame and adds it the collection.
+                currentFrame = image!.Frames.AddFrame(previousFrame);
+                if (prevFrameData?.DisposalMethod is WebpDisposalMethod.RestoreToBackground)
+                {
+                    this.RestoreToBackground(currentFrame, backgroundColor);
+                }
+            }
 
-            imageFrame = currentFrame;
+            SetFrameMetadata(currentFrame.Metadata, frameData);
         }
 
-        Rectangle regionRectangle = frameData.Bounds;
+        Rectangle interest = frameData.Bounds;
+        bool blend = previousFrame != null && frameData.BlendingMethod == WebpBlendMethod.Over;
+        using Buffer2D<TPixel> pixelData = this.DecodeImageFrameData<TPixel>(frameData, webpInfo);
+        DrawDecodedImageFrameOnCanvas(pixelData, currentFrame, interest, blend);
+
+        webpInfo?.Dispose();
+        previousFrame = currentFrame;
+        prevFrameData = frameData;
 
         if (frameData.DisposalMethod is WebpDisposalMethod.RestoreToBackground)
         {
-            this.RestoreToBackground(imageFrame, backgroundColor);
+            this.restoreArea = interest;
         }
 
-        using Buffer2D<TPixel> decodedImageFrame = this.DecodeImageFrameData<TPixel>(frameData, webpInfo);
-
-        bool blend = previousFrame != null && frameData.BlendingMethod == WebpBlendMethod.Over;
-        DrawDecodedImageFrameOnCanvas(decodedImageFrame, imageFrame, regionRectangle, blend);
-
-        previousFrame = currentFrame ?? image.Frames.RootFrame;
-        this.restoreArea = regionRectangle;
-
         return (uint)(stream.Position - streamStartPosition);
     }
 
@@ -257,31 +297,27 @@ private Buffer2D<TPixel> DecodeImageFrameData<TPixel>(WebpFrameData frameData, W
 
         try
         {
-            Buffer2D<TPixel> pixelBufferDecoded = decodedFrame.PixelBuffer;
+            Buffer2D<TPixel> decodeBuffer = decodedFrame.PixelBuffer;
             if (webpInfo.IsLossless)
             {
                 WebpLosslessDecoder losslessDecoder =
                     new(webpInfo.Vp8LBitReader, this.memoryAllocator, this.configuration);
-                losslessDecoder.Decode(pixelBufferDecoded, (int)webpInfo.Width, (int)webpInfo.Height);
+                losslessDecoder.Decode(decodeBuffer, (int)frameData.Width, (int)frameData.Height);
             }
             else
             {
                 WebpLossyDecoder lossyDecoder =
                     new(webpInfo.Vp8BitReader, this.memoryAllocator, this.configuration);
-                lossyDecoder.Decode(pixelBufferDecoded, (int)webpInfo.Width, (int)webpInfo.Height, webpInfo, this.alphaData);
+                lossyDecoder.Decode(decodeBuffer, (int)frameData.Width, (int)frameData.Height, webpInfo, this.alphaData);
             }
 
-            return pixelBufferDecoded;
+            return decodeBuffer;
         }
         catch
         {
             decodedFrame?.Dispose();
             throw;
         }
-        finally
-        {
-            webpInfo.Dispose();
-        }
     }
 
     /// <summary>
@@ -290,17 +326,17 @@ private Buffer2D<TPixel> DecodeImageFrameData<TPixel>(WebpFrameData frameData, W
     /// <typeparam name="TPixel">The type of the pixel.</typeparam>
     /// <param name="decodedImageFrame">The decoded image.</param>
     /// <param name="imageFrame">The image frame to draw into.</param>
-    /// <param name="restoreArea">The area of the frame.</param>
+    /// <param name="interest">The area of the frame to draw to.</param>
     /// <param name="blend">Whether to blend the decoded frame data onto the target frame.</param>
     private static void DrawDecodedImageFrameOnCanvas<TPixel>(
         Buffer2D<TPixel> decodedImageFrame,
         ImageFrame<TPixel> imageFrame,
-        Rectangle restoreArea,
+        Rectangle interest,
         bool blend)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         // Trim the destination frame to match the restore area. The source frame is already trimmed.
-        Buffer2DRegion<TPixel> imageFramePixels = imageFrame.PixelBuffer.GetRegion(restoreArea);
+        Buffer2DRegion<TPixel> imageFramePixels = imageFrame.PixelBuffer.GetRegion(interest);
         if (blend)
         {
             // The destination frame has already been prepopulated with the pixel data from the previous frame
@@ -309,21 +345,21 @@ private static void DrawDecodedImageFrameOnCanvas<TPixel>(
             PixelBlender<TPixel> blender =
                 PixelOperations<TPixel>.Instance.GetPixelBlender(PixelColorBlendingMode.Normal, PixelAlphaCompositionMode.SrcOver);
 
-            for (int y = 0; y < restoreArea.Height; y++)
+            for (int y = 0; y < interest.Height; y++)
             {
                 Span<TPixel> framePixelRow = imageFramePixels.DangerousGetRowSpan(y);
-                Span<TPixel> decodedPixelRow = decodedImageFrame.DangerousGetRowSpan(y)[..restoreArea.Width];
+                Span<TPixel> decodedPixelRow = decodedImageFrame.DangerousGetRowSpan(y);
 
                 blender.Blend<TPixel>(imageFrame.Configuration, framePixelRow, framePixelRow, decodedPixelRow, 1f);
             }
 
             return;
         }
 
-        for (int y = 0; y < restoreArea.Height; y++)
+        for (int y = 0; y < interest.Height; y++)
         {
             Span<TPixel> framePixelRow = imageFramePixels.DangerousGetRowSpan(y);
-            Span<TPixel> decodedPixelRow = decodedImageFrame.DangerousGetRowSpan(y)[..restoreArea.Width];
+            Span<TPixel> decodedPixelRow = decodedImageFrame.DangerousGetRowSpan(y);
             decodedPixelRow.CopyTo(framePixelRow);
         }
     }
@@ -335,7 +371,7 @@ private static void DrawDecodedImageFrameOnCanvas<TPixel>(
     /// <typeparam name="TPixel">The pixel format.</typeparam>
     /// <param name="imageFrame">The image frame.</param>
     /// <param name="backgroundColor">Color of the background.</param>
-    private void RestoreToBackground<TPixel>(ImageFrame<TPixel> imageFrame, Color backgroundColor)
+    private void RestoreToBackground<TPixel>(ImageFrame<TPixel> imageFrame, TPixel backgroundColor)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         if (!this.restoreArea.HasValue)
@@ -345,8 +381,7 @@ private void RestoreToBackground<TPixel>(ImageFrame<TPixel> imageFrame, Color ba
 
         Rectangle interest = Rectangle.Intersect(imageFrame.Bounds(), this.restoreArea.Value);
         Buffer2DRegion<TPixel> pixelRegion = imageFrame.PixelBuffer.GetRegion(interest);
-        TPixel backgroundPixel = backgroundColor.ToPixel<TPixel>();
-        pixelRegion.Fill(backgroundPixel);
+        pixelRegion.Fill(backgroundColor);
     }
 
     /// <inheritdoc/>
diff --git a/src/ImageSharp/Formats/Webp/WebpBlendMethod.cs b/src/ImageSharp/Formats/Webp/WebpBlendMethod.cs
@@ -1,4 +1,4 @@
-﻿// Copyright (c) Six Labors.
+// Copyright (c) Six Labors.
 // Licensed under the Six Labors Split License.
 
 namespace SixLabors.ImageSharp.Formats.Webp;
@@ -12,11 +12,11 @@ public enum WebpBlendMethod
     /// Do not blend. After disposing of the previous frame,
     /// render the current frame on the canvas by overwriting the rectangle covered by the current frame.
     /// </summary>
-    Source = 0,
+    Source = 1,
 
     /// <summary>
     /// Use alpha blending. After disposing of the previous frame, render the current frame on the canvas using alpha-blending.
     /// If the current frame does not have an alpha channel, assume alpha value of 255, effectively replacing the rectangle.
     /// </summary>
-    Over = 1,
+    Over = 0,
 }
diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpDecoderTests.cs
@@ -450,6 +450,22 @@ public void WebpDecoder_CanDecode_Issue2670<TPixel>(TestImageProvider<TPixel> pr
         image.CompareToOriginal(provider, ReferenceDecoder);
     }
 
+    // https://github.com/SixLabors/ImageSharp/issues/2866
+    [Theory]
+    [WithFile(Lossy.Issue2866, PixelTypes.Rgba32)]
+    public void WebpDecoder_CanDecode_Issue2866<TPixel>(TestImageProvider<TPixel> provider)
+        where TPixel : unmanaged, IPixel<TPixel>
+    {
+        // Web
+        using Image<TPixel> image = provider.GetImage(
+            WebpDecoder.Instance,
+            new WebpDecoderOptions() { BackgroundColorHandling = BackgroundColorHandling.Ignore });
+
+        // We can't use the reference decoder here.
+        // It creates frames of different size without blending the frames.
+        image.DebugSave(provider, extension: "webp", encoder: new WebpEncoder());
+    }
+
     [Theory]
     [WithFile(Lossless.LossLessCorruptImage3, PixelTypes.Rgba32)]
     public void WebpDecoder_ThrowImageFormatException_OnInvalidImages<TPixel>(TestImageProvider<TPixel> provider)
diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpEncoderTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpEncoderTests.cs
@@ -582,21 +582,6 @@ public async Task CanSave_NonSeekableStream_Async<TPixel>(TestImageProvider<TPix
         Assert.True(seekable.ToArray().SequenceEqual(memoryStream.ToArray()));
     }
 
-    // https://github.com/SixLabors/ImageSharp/issues/2866
-    [Theory]
-    [WithFile(Lossy.Issue2866, PixelTypes.Rgba32)]
-    public void WebpDecoder_CanDecode_Issue2866<TPixel>(TestImageProvider<TPixel> provider)
-        where TPixel : unmanaged, IPixel<TPixel>
-    {
-        WebpEncoder encoder = new()
-        {
-            Quality = 100
-        };
-
-        using Image<TPixel> image = provider.GetImage();
-        image.VerifyEncoder(provider, "webp", string.Empty, encoder, ImageComparer.TolerantPercentage(0.0994F));
-    }
-
     private static ImageComparer GetComparer(int quality)
     {
         float tolerance = 0.01f; // ~1.0%
diff --git a/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs b/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs

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