Merge branch 'main' into js/colorspace-converter

Author: JimBobSquarePants

.github/workflows/build-and-test.yml

@@ -133,7 +133,7 @@ jobs:
           XUNIT_PATH: .\tests\ImageSharp.Tests # Required for xunit
 
       - name: Export Failed Output
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         if: failure()
         with:
           name: actual_output_${{ runner.os }}_${{ matrix.options.framework }}${{ matrix.options.runtime }}.zip

.github/workflows/code-coverage.yml

@@ -74,7 +74,7 @@ jobs:
           XUNIT_PATH: .\tests\ImageSharp.Tests # Required for xunit
 
       - name: Export Failed Output
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         if: failure()
         with:
           name: actual_output_${{ runner.os }}_${{ matrix.options.framework }}${{ matrix.options.runtime }}.zip

src/ImageSharp/Formats/Jpeg/Components/Decoder/JpegBitReader.cs

@@ -22,6 +22,9 @@ internal struct JpegBitReader
     // Whether there is no more good data to pull from the stream for the current mcu.
     private bool badData;
 
+    // How many times have we hit the eof.
+    private int eofHitCount;
+
     public JpegBitReader(BufferedReadStream stream)
     {
         this.stream = stream;
@@ -31,6 +34,7 @@ public JpegBitReader(BufferedReadStream stream)
         this.MarkerPosition = 0;
         this.badData = false;
         this.NoData = false;
+        this.eofHitCount = 0;
     }
 
     /// <summary>
@@ -219,11 +223,16 @@ private int ReadStream()
         // we know we have hit the EOI and completed decoding the scan buffer.
         if (value == -1 || (this.badData && this.data == 0 && this.stream.Position >= this.stream.Length))
         {
-            // We've encountered the end of the file stream which means there's no EOI marker
+            // We've hit the end of the file stream more times than allowed which means there's no EOI marker
             // in the image or the SOS marker has the wrong dimensions set.
-            this.badData = true;
-            this.NoData = true;
-            value = 0;
+            if (this.eofHitCount > JpegConstants.Huffman.FetchLoop)
+            {
+                this.badData = true;
+                this.NoData = true;
+                value = 0;
+            }
+
+            this.eofHitCount++;
         }
 
         return value;

src/ImageSharp/Formats/Png/PngDecoderCore.cs

@@ -234,8 +234,7 @@ public Image<TPixel> Decode<TPixel>(BufferedReadStream stream, CancellationToken
                                 PngThrowHelper.ThrowMissingFrameControl();
                             }
 
-                            previousFrameControl ??= new((uint)this.header.Width, (uint)this.header.Height);
-                            this.InitializeFrame(previousFrameControl.Value, currentFrameControl.Value, image, previousFrame, out currentFrame);
+                            this.InitializeFrame(previousFrameControl, currentFrameControl.Value, image, previousFrame, out currentFrame);
 
                             this.currentStream.Position += 4;
                             this.ReadScanlines(
@@ -246,11 +245,16 @@ public Image<TPixel> Decode<TPixel>(BufferedReadStream stream, CancellationToken
                                 currentFrameControl.Value,
                                 cancellationToken);
 
-                            previousFrame = currentFrame;
-                            previousFrameControl = currentFrameControl;
+                            // if current frame dispose is restore to previous, then from future frame's perspective, it never happened
+                            if (currentFrameControl.Value.DisposeOperation != PngDisposalMethod.RestoreToPrevious)
+                            {
+                                previousFrame = currentFrame;
+                                previousFrameControl = currentFrameControl;
+                            }
+
                             break;
                         case PngChunkType.Data:
-
+                            pngMetadata.AnimateRootFrame = currentFrameControl != null;
                             currentFrameControl ??= new((uint)this.header.Width, (uint)this.header.Height);
                             if (image is null)
                             {
@@ -267,9 +271,12 @@ public Image<TPixel> Decode<TPixel>(BufferedReadStream stream, CancellationToken
                                 this.ReadNextDataChunk,
                                 currentFrameControl.Value,
                                 cancellationToken);
+                            if (pngMetadata.AnimateRootFrame)
+                            {
+                                previousFrame = currentFrame;
+                                previousFrameControl = currentFrameControl;
+                            }
 
-                            previousFrame = currentFrame;
-                            previousFrameControl = currentFrameControl;
                             break;
                         case PngChunkType.Palette:
                             this.palette = chunk.Data.GetSpan().ToArray();
@@ -638,7 +645,7 @@ private void InitializeImage<TPixel>(ImageMetadata metadata, FrameControl frameC
     /// <param name="previousFrame">The previous frame.</param>
     /// <param name="frame">The created frame</param>
     private void InitializeFrame<TPixel>(
-        FrameControl previousFrameControl,
+        FrameControl? previousFrameControl,
         FrameControl currentFrameControl,
         Image<TPixel> image,
         ImageFrame<TPixel>? previousFrame,
@@ -651,12 +658,16 @@ private void InitializeFrame<TPixel>(
         frame = image.Frames.AddFrame(previousFrame ?? image.Frames.RootFrame);
 
         // If the first `fcTL` chunk uses a `dispose_op` of APNG_DISPOSE_OP_PREVIOUS it should be treated as APNG_DISPOSE_OP_BACKGROUND.
-        if (previousFrameControl.DisposeOperation == PngDisposalMethod.RestoreToBackground
-            || (previousFrame is null && previousFrameControl.DisposeOperation == PngDisposalMethod.RestoreToPrevious))
+        // So, if restoring to before first frame, clear entire area. Same if first frame (previousFrameControl null).
+        if (previousFrameControl == null || (previousFrame is null && previousFrameControl.Value.DisposeOperation == PngDisposalMethod.RestoreToPrevious))
+        {
+            Buffer2DRegion<TPixel> pixelRegion = frame.PixelBuffer.GetRegion();
+            pixelRegion.Clear();
+        }
+        else if (previousFrameControl.Value.DisposeOperation == PngDisposalMethod.RestoreToBackground)
         {
-            Rectangle restoreArea = previousFrameControl.Bounds;
-            Rectangle interest = Rectangle.Intersect(frame.Bounds(), restoreArea);
-            Buffer2DRegion<TPixel> pixelRegion = frame.PixelBuffer.GetRegion(interest);
+            Rectangle restoreArea = previousFrameControl.Value.Bounds;
+            Buffer2DRegion<TPixel> pixelRegion = frame.PixelBuffer.GetRegion(restoreArea);
             pixelRegion.Clear();
         }
 

src/ImageSharp/Formats/Png/PngEncoderCore.cs

@@ -167,6 +167,7 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
 
         ImageFrame<TPixel>? clonedFrame = null;
         ImageFrame<TPixel> currentFrame = image.Frames.RootFrame;
+        int currentFrameIndex = 0;
 
         bool clearTransparency = this.encoder.TransparentColorMode is PngTransparentColorMode.Clear;
         if (clearTransparency)
@@ -195,29 +196,50 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
 
         if (image.Frames.Count > 1)
         {
-            this.WriteAnimationControlChunk(stream, (uint)image.Frames.Count, pngMetadata.RepeatCount);
+            this.WriteAnimationControlChunk(stream, (uint)(image.Frames.Count - (pngMetadata.AnimateRootFrame ? 0 : 1)), pngMetadata.RepeatCount);
+        }
+
+        // If the first frame isn't animated, write it as usual and skip it when writing animated frames
+        if (!pngMetadata.AnimateRootFrame || image.Frames.Count == 1)
+        {
+            FrameControl frameControl = new((uint)this.width, (uint)this.height);
+            this.WriteDataChunks(frameControl, currentFrame.PixelBuffer.GetRegion(), quantized, stream, false);
+            currentFrameIndex++;
+        }
 
-            // Write the first frame.
+        if (image.Frames.Count > 1)
+        {
+            // Write the first animated frame.
+            currentFrame = image.Frames[currentFrameIndex];
             PngFrameMetadata frameMetadata = GetPngFrameMetadata(currentFrame);
             PngDisposalMethod previousDisposal = frameMetadata.DisposalMethod;
             FrameControl frameControl = this.WriteFrameControlChunk(stream, frameMetadata, currentFrame.Bounds(), 0);
-            this.WriteDataChunks(frameControl, currentFrame.PixelBuffer.GetRegion(), quantized, stream, false);
+            uint sequenceNumber = 1;
+            if (pngMetadata.AnimateRootFrame)
+            {
+                this.WriteDataChunks(frameControl, currentFrame.PixelBuffer.GetRegion(), quantized, stream, false);
+            }
+            else
+            {
+                sequenceNumber += this.WriteDataChunks(frameControl, currentFrame.PixelBuffer.GetRegion(), quantized, stream, true);
+            }
+
+            currentFrameIndex++;
 
             // Capture the global palette for reuse on subsequent frames.
             ReadOnlyMemory<TPixel>? previousPalette = quantized?.Palette.ToArray();
 
             // Write following frames.
-            uint increment = 0;
             ImageFrame<TPixel> previousFrame = image.Frames.RootFrame;
 
             // This frame is reused to store de-duplicated pixel buffers.
             using ImageFrame<TPixel> encodingFrame = new(image.Configuration, previousFrame.Size());
 
-            for (int i = 1; i < image.Frames.Count; i++)
+            for (; currentFrameIndex < image.Frames.Count; currentFrameIndex++)
             {
                 ImageFrame<TPixel>? prev = previousDisposal == PngDisposalMethod.RestoreToBackground ? null : previousFrame;
-                currentFrame = image.Frames[i];
-                ImageFrame<TPixel>? nextFrame = i < image.Frames.Count - 1 ? image.Frames[i + 1] : null;
+                currentFrame = image.Frames[currentFrameIndex];
+                ImageFrame<TPixel>? nextFrame = currentFrameIndex < image.Frames.Count - 1 ? image.Frames[currentFrameIndex + 1] : null;
 
                 frameMetadata = GetPngFrameMetadata(currentFrame);
                 bool blend = frameMetadata.BlendMethod == PngBlendMethod.Over;
@@ -238,22 +260,17 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
                 }
 
                 // Each frame control sequence number must be incremented by the number of frame data chunks that follow.
-                frameControl = this.WriteFrameControlChunk(stream, frameMetadata, bounds, (uint)i + increment);
+                frameControl = this.WriteFrameControlChunk(stream, frameMetadata, bounds, sequenceNumber);
 
                 // Dispose of previous quantized frame and reassign.
                 quantized?.Dispose();
                 quantized = this.CreateQuantizedImageAndUpdateBitDepth(pngMetadata, encodingFrame, bounds, previousPalette);
-                increment += this.WriteDataChunks(frameControl, encodingFrame.PixelBuffer.GetRegion(bounds), quantized, stream, true);
+                sequenceNumber += this.WriteDataChunks(frameControl, encodingFrame.PixelBuffer.GetRegion(bounds), quantized, stream, true) + 1;
 
                 previousFrame = currentFrame;
                 previousDisposal = frameMetadata.DisposalMethod;
             }
         }
-        else
-        {
-            FrameControl frameControl = new((uint)this.width, (uint)this.height);
-            this.WriteDataChunks(frameControl, currentFrame.PixelBuffer.GetRegion(), quantized, stream, false);
-        }
 
         this.WriteEndChunk(stream);
 

src/ImageSharp/Formats/Png/PngMetadata.cs

@@ -29,6 +29,7 @@ private PngMetadata(PngMetadata other)
         this.InterlaceMethod = other.InterlaceMethod;
         this.TransparentColor = other.TransparentColor;
         this.RepeatCount = other.RepeatCount;
+        this.AnimateRootFrame = other.AnimateRootFrame;
 
         if (other.ColorTable?.Length > 0)
         {
@@ -83,6 +84,11 @@ private PngMetadata(PngMetadata other)
     /// </summary>
     public uint RepeatCount { get; set; } = 1;
 
+    /// <summary>
+    ///  Gets or sets a value indicating whether the root frame is shown as part of the animated sequence
+    /// </summary>
+    public bool AnimateRootFrame { get; set; } = true;
+
     /// <inheritdoc/>
     public IDeepCloneable DeepClone() => new PngMetadata(this);
 

src/ImageSharp/Formats/Png/PngScanlineProcessor.cs

@@ -180,8 +180,9 @@ public static void ProcessInterlacedPaletteScanline<TPixel>(
         ref byte scanlineSpanRef = ref MemoryMarshal.GetReference(scanlineSpan);
         ref TPixel rowSpanRef = ref MemoryMarshal.GetReference(rowSpan);
         ref Color paletteBase = ref MemoryMarshal.GetReference(palette.Value.Span);
+        uint offset = pixelOffset + frameControl.XOffset;
 
-        for (nuint x = pixelOffset, o = 0; x < frameControl.XMax; x += increment, o++)
+        for (nuint x = offset, o = 0; x < frameControl.XMax; x += increment, o++)
         {
             uint index = Unsafe.Add(ref scanlineSpanRef, o);
             Unsafe.Add(ref rowSpanRef, x) = TPixel.FromRgba32(Unsafe.Add(ref paletteBase, index).ToPixel<Rgba32>());

src/ImageSharp/Formats/Webp/BitWriter/BitWriterBase.cs

@@ -88,7 +88,8 @@ protected void ResizeBuffer(int maxBytes, int sizeRequired)
     /// <param name="iccProfile">The color profile.</param>
     /// <param name="hasAlpha">Flag indicating, if a alpha channel is present.</param>
     /// <param name="hasAnimation">Flag indicating, if an animation parameter is present.</param>
-    public static void WriteTrunksBeforeData(
+    /// <returns>A <see cref="WebpVp8X"/> or a default instance.</returns>
+    public static WebpVp8X WriteTrunksBeforeData(
         Stream stream,
         uint width,
         uint height,
@@ -102,16 +103,19 @@ public static void WriteTrunksBeforeData(
         RiffHelper.BeginWriteRiffFile(stream, WebpConstants.WebpFourCc);
 
         // Write VP8X, header if necessary.
+        WebpVp8X vp8x = default;
         bool isVp8X = exifProfile != null || xmpProfile != null || iccProfile != null || hasAlpha || hasAnimation;
         if (isVp8X)
         {
-            WriteVp8XHeader(stream, exifProfile, xmpProfile, iccProfile, width, height, hasAlpha, hasAnimation);
+            vp8x = WriteVp8XHeader(stream, exifProfile, xmpProfile, iccProfile, width, height, hasAlpha, hasAnimation);
 
             if (iccProfile != null)
             {
                 RiffHelper.WriteChunk(stream, (uint)WebpChunkType.Iccp, iccProfile.ToByteArray());
             }
         }
+
+        return vp8x;
     }
 
     /// <summary>
@@ -124,10 +128,16 @@ public static void WriteTrunksBeforeData(
     /// Write the trunks after data trunk.
     /// </summary>
     /// <param name="stream">The stream to write to.</param>
-    /// <param name="exifProfile">The exif profile.</param>
+    /// <param name="vp8x">The VP8X chunk.</param>
+    /// <param name="updateVp8x">Whether to update the chunk.</param>
+    /// <param name="initialPosition">The initial position of the stream before encoding.</param>
+    /// <param name="exifProfile">The EXIF profile.</param>
     /// <param name="xmpProfile">The XMP profile.</param>
     public static void WriteTrunksAfterData(
         Stream stream,
+        in WebpVp8X vp8x,
+        bool updateVp8x,
+        long initialPosition,
         ExifProfile? exifProfile,
         XmpProfile? xmpProfile)
     {
@@ -141,7 +151,7 @@ public static void WriteTrunksAfterData(
             RiffHelper.WriteChunk(stream, (uint)WebpChunkType.Xmp, xmpProfile.Data);
         }
 
-        RiffHelper.EndWriteRiffFile(stream, 4);
+        RiffHelper.EndWriteRiffFile(stream, in vp8x, updateVp8x, initialPosition);
     }
 
     /// <summary>
@@ -186,19 +196,21 @@ public static void WriteAlphaChunk(Stream stream, Span<byte> dataBytes, bool alp
     /// Writes a VP8X header to the stream.
     /// </summary>
     /// <param name="stream">The stream to write to.</param>
-    /// <param name="exifProfile">A exif profile or null, if it does not exist.</param>
-    /// <param name="xmpProfile">A XMP profile or null, if it does not exist.</param>
+    /// <param name="exifProfile">An EXIF profile or null, if it does not exist.</param>
+    /// <param name="xmpProfile">An XMP profile or null, if it does not exist.</param>
     /// <param name="iccProfile">The color profile.</param>
     /// <param name="width">The width of the image.</param>
     /// <param name="height">The height of the image.</param>
     /// <param name="hasAlpha">Flag indicating, if a alpha channel is present.</param>
     /// <param name="hasAnimation">Flag indicating, if an animation parameter is present.</param>
-    protected static void WriteVp8XHeader(Stream stream, ExifProfile? exifProfile, XmpProfile? xmpProfile, IccProfile? iccProfile, uint width, uint height, bool hasAlpha, bool hasAnimation)
+    protected static WebpVp8X WriteVp8XHeader(Stream stream, ExifProfile? exifProfile, XmpProfile? xmpProfile, IccProfile? iccProfile, uint width, uint height, bool hasAlpha, bool hasAnimation)
     {
         WebpVp8X chunk = new(hasAnimation, xmpProfile != null, exifProfile != null, hasAlpha, iccProfile != null, width, height);
 
         chunk.Validate(MaxDimension, MaxCanvasPixels);
 
         chunk.WriteTo(stream);
+
+        return chunk;
     }
 }

src/ImageSharp/Formats/Webp/Chunks/WebpVp8X.cs

@@ -3,7 +3,7 @@
 
 namespace SixLabors.ImageSharp.Formats.Webp.Chunks;
 
-internal readonly struct WebpVp8X
+internal readonly struct WebpVp8X : IEquatable<WebpVp8X>
 {
     public WebpVp8X(bool hasAnimation, bool hasXmp, bool hasExif, bool hasAlpha, bool hasIcc, uint width, uint height)
     {
@@ -51,6 +51,24 @@ public WebpVp8X(bool hasAnimation, bool hasXmp, bool hasExif, bool hasAlpha, boo
     /// </summary>
     public uint Height { get; }
 
+    public static bool operator ==(WebpVp8X left, WebpVp8X right) => left.Equals(right);
+
+    public static bool operator !=(WebpVp8X left, WebpVp8X right) => !(left == right);
+
+    public override bool Equals(object? obj) => obj is WebpVp8X x && this.Equals(x);
+
+    public bool Equals(WebpVp8X other)
+        => this.HasAnimation == other.HasAnimation
+        && this.HasXmp == other.HasXmp
+        && this.HasExif == other.HasExif
+        && this.HasAlpha == other.HasAlpha
+        && this.HasIcc == other.HasIcc
+        && this.Width == other.Width
+        && this.Height == other.Height;
+
+    public override int GetHashCode()
+        => HashCode.Combine(this.HasAnimation, this.HasXmp, this.HasExif, this.HasAlpha, this.HasIcc, this.Width, this.Height);
+
     public void Validate(uint maxDimension, ulong maxCanvasPixels)
     {
         if (this.Width > maxDimension || this.Height > maxDimension)
@@ -65,6 +83,9 @@ public void Validate(uint maxDimension, ulong maxCanvasPixels)
         }
     }
 
+    public WebpVp8X WithAlpha(bool hasAlpha)
+        => new(this.HasAnimation, this.HasXmp, this.HasExif, hasAlpha, this.HasIcc, this.Width, this.Height);
+
     public void WriteTo(Stream stream)
     {
         byte flags = 0;