Add tests and fix issues.

SixLabors · Nov 27, 2024 · 502a354 · 502a354
1 parent 8dd4f35
commit 502a354
Show file tree

Hide file tree

Showing 26 changed files with 784 additions and 208 deletions.
diff --git a/src/ImageSharp/Formats/Cur/CurFrameMetadata.cs b/src/ImageSharp/Formats/Cur/CurFrameMetadata.cs
@@ -48,13 +48,13 @@ private CurFrameMetadata(CurFrameMetadata other)
     /// Gets or sets the encoding width. <br />
     /// Can be any number between 0 and 255. Value 0 means a frame height of 256 pixels or greater.
     /// </summary>
-    public byte EncodingWidth { get; set; }
+    public byte? EncodingWidth { get; set; }
 
     /// <summary>
     /// Gets or sets the encoding height. <br />
     /// Can be any number between 0 and 255. Value 0 means a frame height of 256 pixels or greater.
     /// </summary>
-    public byte EncodingHeight { get; set; }
+    public byte? EncodingHeight { get; set; }
 
     /// <summary>
     /// Gets or sets the number of bits per pixel.<br/>
@@ -80,20 +80,6 @@ public static CurFrameMetadata FromFormatConnectingFrameMetadata(FormatConnectin
             };
         }
 
-        byte encodingWidth = metadata.EncodingWidth switch
-        {
-            > 255 => 0,
-            <= 255 and >= 1 => (byte)metadata.EncodingWidth,
-            _ => 0
-        };
-
-        byte encodingHeight = metadata.EncodingHeight switch
-        {
-            > 255 => 0,
-            <= 255 and >= 1 => (byte)metadata.EncodingHeight,
-            _ => 0
-        };
-
         int bpp = metadata.PixelTypeInfo.Value.BitsPerPixel;
         BmpBitsPerPixel bbpp = bpp switch
         {
@@ -116,8 +102,8 @@ public static CurFrameMetadata FromFormatConnectingFrameMetadata(FormatConnectin
         {
             BmpBitsPerPixel = bbpp,
             Compression = compression,
-            EncodingWidth = encodingWidth,
-            EncodingHeight = encodingHeight,
+            EncodingWidth = ClampEncodingDimension(metadata.EncodingWidth),
+            EncodingHeight = ClampEncodingDimension(metadata.EncodingHeight),
             ColorTable = compression == IconFrameCompression.Bmp ? metadata.ColorTable : null
         };
     }
@@ -138,8 +124,8 @@ public void AfterFrameApply<TPixel>(ImageFrame<TPixel> source, ImageFrame<TPixel
     {
         float ratioX = destination.Width / (float)source.Width;
         float ratioY = destination.Height / (float)source.Height;
-        this.EncodingWidth = Scale(this.EncodingWidth, destination.Width, ratioX);
-        this.EncodingHeight = Scale(this.EncodingHeight, destination.Height, ratioY);
+        this.EncodingWidth = ScaleEncodingDimension(this.EncodingWidth, destination.Width, ratioX);
+        this.EncodingHeight = ScaleEncodingDimension(this.EncodingHeight, destination.Height, ratioY);
     }
 
     /// <inheritdoc/>
@@ -156,16 +142,16 @@ internal void FromIconDirEntry(IconDirEntry entry)
         this.HotspotY = entry.BitCount;
     }
 
-    internal IconDirEntry ToIconDirEntry()
+    internal IconDirEntry ToIconDirEntry(Size size)
     {
         byte colorCount = this.Compression == IconFrameCompression.Png || this.BmpBitsPerPixel > BmpBitsPerPixel.Bit8
             ? (byte)0
             : (byte)ColorNumerics.GetColorCountForBitDepth((int)this.BmpBitsPerPixel);
 
         return new()
         {
-            Width = this.EncodingWidth,
-            Height = this.EncodingHeight,
+            Width = ClampEncodingDimension(this.EncodingWidth ?? size.Width),
+            Height = ClampEncodingDimension(this.EncodingHeight ?? size.Height),
             Planes = this.HotspotX,
             BitCount = this.HotspotY,
             ColorCount = colorCount
@@ -233,13 +219,22 @@ private PixelTypeInfo GetPixelTypeInfo()
         };
     }
 
-    private static byte Scale(byte? value, int destination, float ratio)
+    private static byte ScaleEncodingDimension(byte? value, int destination, float ratio)
     {
         if (value is null)
         {
-            return (byte)Math.Clamp(destination, 0, 255);
+            return ClampEncodingDimension(destination);
         }
 
-        return Math.Min((byte)MathF.Ceiling(value.Value * ratio), (byte)Math.Clamp(destination, 0, 255));
+        return ClampEncodingDimension(MathF.Ceiling(value.Value * ratio));
     }
+
+    private static byte ClampEncodingDimension(float? dimension)
+        => dimension switch
+        {
+            // Encoding dimensions can be between 0-256 where 0 means 256 or greater.
+            > 255 => 0,
+            <= 255 and >= 1 => (byte)dimension,
+            _ => 0
+        };
 }
diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs
@@ -207,22 +207,29 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream, CancellationToken
             this.WriteApplicationExtensions(stream, image.Frames.Count, this.repeatCount ?? gifMetadata.RepeatCount, xmpProfile);
         }
 
-        this.EncodeFirstFrame(stream, frameMetadata, quantized);
-
-        // Capture the global palette for reuse on subsequent frames and cleanup the quantized frame.
-        TPixel[] globalPalette = image.Frames.Count == 1 ? [] : quantized.Palette.ToArray();
-
-        this.EncodeAdditionalFrames(
-            stream,
-            image,
-            globalPalette,
-            derivedTransparencyIndex,
-            frameMetadata.DisposalMode,
-            cancellationToken);
-
-        stream.WriteByte(GifConstants.EndIntroducer);
+        // If the token is cancelled during encoding of frames we must ensure the
+        // quantized frame is disposed.
+        try
+        {
+            this.EncodeFirstFrame(stream, frameMetadata, quantized, cancellationToken);
+
+            // Capture the global palette for reuse on subsequent frames and cleanup the quantized frame.
+            TPixel[] globalPalette = image.Frames.Count == 1 ? [] : quantized.Palette.ToArray();
+
+            this.EncodeAdditionalFrames(
+                stream,
+                image,
+                globalPalette,
+                derivedTransparencyIndex,
+                frameMetadata.DisposalMode,
+                cancellationToken);
+        }
+        finally
+        {
+            stream.WriteByte(GifConstants.EndIntroducer);
 
-        quantized?.Dispose();
+            quantized?.Dispose();
+        }
     }
 
     private static GifFrameMetadata GetGifFrameMetadata<TPixel>(ImageFrame<TPixel> frame, int transparencyIndex)
@@ -310,9 +317,12 @@ private void EncodeAdditionalFrames<TPixel>(
     private void EncodeFirstFrame<TPixel>(
         Stream stream,
         GifFrameMetadata metadata,
-        IndexedImageFrame<TPixel> quantized)
+        IndexedImageFrame<TPixel> quantized,
+        CancellationToken cancellationToken)
         where TPixel : unmanaged, IPixel<TPixel>
     {
+        cancellationToken.ThrowIfCancellationRequested();
+
         this.WriteGraphicalControlExtension(metadata, stream);
 
         Buffer2D<byte> indices = ((IPixelSource)quantized).PixelBuffer;

diff --git a/src/ImageSharp/Formats/Ico/IcoFrameMetadata.cs b/src/ImageSharp/Formats/Ico/IcoFrameMetadata.cs
@@ -41,13 +41,13 @@ private IcoFrameMetadata(IcoFrameMetadata other)
     /// Gets or sets the encoding width. <br />
     /// Can be any number between 0 and 255. Value 0 means a frame height of 256 pixels or greater.
     /// </summary>
-    public byte EncodingWidth { get; set; }
+    public byte? EncodingWidth { get; set; }
 
     /// <summary>
     /// Gets or sets the encoding height. <br />
     /// Can be any number between 0 and 255. Value 0 means a frame height of 256 pixels or greater.
     /// </summary>
-    public byte EncodingHeight { get; set; }
+    public byte? EncodingHeight { get; set; }
 
     /// <summary>
     /// Gets or sets the number of bits per pixel.<br/>
@@ -73,20 +73,6 @@ public static IcoFrameMetadata FromFormatConnectingFrameMetadata(FormatConnectin
             };
         }
 
-        byte encodingWidth = metadata.EncodingWidth switch
-        {
-            > 255 => 0,
-            <= 255 and >= 1 => (byte)metadata.EncodingWidth,
-            _ => 0
-        };
-
-        byte encodingHeight = metadata.EncodingHeight switch
-        {
-            > 255 => 0,
-            <= 255 and >= 1 => (byte)metadata.EncodingHeight,
-            _ => 0
-        };
-
         int bpp = metadata.PixelTypeInfo.Value.BitsPerPixel;
         BmpBitsPerPixel bbpp = bpp switch
         {
@@ -109,8 +95,8 @@ public static IcoFrameMetadata FromFormatConnectingFrameMetadata(FormatConnectin
         {
             BmpBitsPerPixel = bbpp,
             Compression = compression,
-            EncodingWidth = encodingWidth,
-            EncodingHeight = encodingHeight,
+            EncodingWidth = ClampEncodingDimension(metadata.EncodingWidth),
+            EncodingHeight = ClampEncodingDimension(metadata.EncodingHeight),
             ColorTable = compression == IconFrameCompression.Bmp ? metadata.ColorTable : null
         };
     }
@@ -131,8 +117,8 @@ public void AfterFrameApply<TPixel>(ImageFrame<TPixel> source, ImageFrame<TPixel
     {
         float ratioX = destination.Width / (float)source.Width;
         float ratioY = destination.Height / (float)source.Height;
-        this.EncodingWidth = Scale(this.EncodingWidth, destination.Width, ratioX);
-        this.EncodingHeight = Scale(this.EncodingHeight, destination.Height, ratioY);
+        this.EncodingWidth = ScaleEncodingDimension(this.EncodingWidth, destination.Width, ratioX);
+        this.EncodingHeight = ScaleEncodingDimension(this.EncodingHeight, destination.Height, ratioY);
     }
 
     /// <inheritdoc/>
@@ -147,16 +133,16 @@ internal void FromIconDirEntry(IconDirEntry entry)
         this.EncodingHeight = entry.Height;
     }
 
-    internal IconDirEntry ToIconDirEntry()
+    internal IconDirEntry ToIconDirEntry(Size size)
     {
         byte colorCount = this.Compression == IconFrameCompression.Png || this.BmpBitsPerPixel > BmpBitsPerPixel.Bit8
             ? (byte)0
             : (byte)ColorNumerics.GetColorCountForBitDepth((int)this.BmpBitsPerPixel);
 
         return new()
         {
-            Width = this.EncodingWidth,
-            Height = this.EncodingHeight,
+            Width = ClampEncodingDimension(this.EncodingWidth ?? size.Width),
+            Height = ClampEncodingDimension(this.EncodingHeight ?? size.Height),
             Planes = 1,
             ColorCount = colorCount,
             BitCount = this.Compression switch
@@ -228,13 +214,22 @@ private PixelTypeInfo GetPixelTypeInfo()
         };
     }
 
-    private static byte Scale(byte? value, int destination, float ratio)
+    private static byte ScaleEncodingDimension(byte? value, int destination, float ratio)
     {
         if (value is null)
         {
-            return (byte)Math.Clamp(destination, 0, 255);
+            return ClampEncodingDimension(destination);
         }
 
-        return Math.Min((byte)MathF.Ceiling(value.Value * ratio), (byte)Math.Clamp(destination, 0, 255));
+        return ClampEncodingDimension(MathF.Ceiling(value.Value * ratio));
     }
+
+    private static byte ClampEncodingDimension(float? dimension)
+        => dimension switch
+        {
+            // Encoding dimensions can be between 0-256 where 0 means 256 or greater.
+            > 255 => 0,
+            <= 255 and >= 1 => (byte)dimension,
+            _ => 0
+        };
 }
diff --git a/src/ImageSharp/Formats/Icon/IconEncoderCore.cs b/src/ImageSharp/Formats/Icon/IconEncoderCore.cs
@@ -123,13 +123,13 @@ private void InitHeader(Image image)
             image.Frames.Select(i =>
             {
                 IcoFrameMetadata metadata = i.Metadata.GetIcoMetadata();
-                return new EncodingFrameMetadata(metadata.Compression, metadata.BmpBitsPerPixel, metadata.ColorTable, metadata.ToIconDirEntry());
+                return new EncodingFrameMetadata(metadata.Compression, metadata.BmpBitsPerPixel, metadata.ColorTable, metadata.ToIconDirEntry(i.Size));
             }).ToArray(),
             IconFileType.CUR =>
             image.Frames.Select(i =>
             {
                 CurFrameMetadata metadata = i.Metadata.GetCurMetadata();
-                return new EncodingFrameMetadata(metadata.Compression, metadata.BmpBitsPerPixel, metadata.ColorTable, metadata.ToIconDirEntry());
+                return new EncodingFrameMetadata(metadata.Compression, metadata.BmpBitsPerPixel, metadata.ColorTable, metadata.ToIconDirEntry(i.Size));
             }).ToArray(),
             _ => throw new NotSupportedException(),
         };