Remove IconFrameMetadata

Signed-off-by: 舰队的偶像-岛风酱! <frg2089@outlook.com>

src/ImageSharp/Formats/Icon/Cur/CurDecoderCore.cs

@@ -14,5 +14,5 @@ internal sealed class CurDecoderCore : IconDecoderCore
     {
     }
 
-    protected override IconFrameMetadata GetFrameMetadata(ImageFrameMetadata metadata) => metadata.GetCurMetadata();
+    protected override void SetFrameMetadata(ImageFrameMetadata metadata, in IconDirEntry entry) => metadata.GetCurMetadata().FromIconDirEntry(entry);
 }

src/ImageSharp/Formats/Icon/Cur/CurFrameMetadata.cs

@@ -4,9 +4,9 @@
 namespace SixLabors.ImageSharp.Formats.Icon.Cur;
 
 /// <summary>
-/// IcoFrameMetadata. TODO: Remove base class and merge into this class.
+/// IcoFrameMetadata.
 /// </summary>
-public class CurFrameMetadata : IconFrameMetadata, IDeepCloneable<CurFrameMetadata>, IDeepCloneable
+public class CurFrameMetadata : IDeepCloneable<CurFrameMetadata>, IDeepCloneable
 {
     /// <summary>
     /// Initializes a new instance of the <see cref="CurFrameMetadata"/> class.
@@ -15,38 +15,92 @@ public class CurFrameMetadata : IconFrameMetadata, IDeepCloneable<CurFrameMetada
     {
     }
 
-    /// <summary>
-    /// Initializes a new instance of the <see cref="CurFrameMetadata"/> class.
-    /// </summary>
-    /// <param name="metadata">metadata</param>
-    public CurFrameMetadata(IconFrameMetadata metadata)
-        : base(metadata)
-    {
-    }
-
     /// <summary>
     /// Initializes a new instance of the <see cref="CurFrameMetadata"/> class.
     /// </summary>
     /// <param name="width">width</param>
     /// <param name="height">height</param>
     /// <param name="colorCount">colorCount</param>
-    /// <param name="field1">field1</param>
-    /// <param name="field2">field2</param>
-    public CurFrameMetadata(byte width, byte height, byte colorCount, ushort field1, ushort field2)
-        : base(width, height, colorCount, field1, field2)
+    /// <param name="hotspotX">hotspotX</param>
+    /// <param name="hotspotY">hotspotY</param>
+    public CurFrameMetadata(byte width, byte height, byte colorCount, ushort hotspotX, ushort hotspotY)
     {
+        this.EncodingWidth = width;
+        this.EncodingHeight = height;
+        this.ColorCount = colorCount;
+        this.HotspotX = hotspotX;
+        this.HotspotY = hotspotY;
     }
 
+    /// <inheritdoc cref="CurFrameMetadata()"/>
+    public CurFrameMetadata(CurFrameMetadata metadata)
+    {
+        this.EncodingWidth = metadata.EncodingWidth;
+        this.EncodingHeight = metadata.EncodingHeight;
+        this.ColorCount = metadata.ColorCount;
+        this.HotspotX = metadata.HotspotX;
+        this.HotspotY = metadata.HotspotY;
+        this.Compression = metadata.Compression;
+    }
+
+    /// <summary>
+    /// Gets or sets icoFrameCompression.
+    /// </summary>
+    public IconFrameCompression Compression { get; set; }
+
+    /// <summary>
+    /// Gets or sets ColorCount field. <br />
+    /// Specifies number of colors in the color palette. Should be 0 if the image does not use a color palette.
+    /// </summary>
+    // TODO: BmpMetadata does not supported palette yet.
+    public byte ColorCount { get; set; }
+
     /// <summary>
     /// Gets or sets Specifies the horizontal coordinates of the hotspot in number of pixels from the left.
     /// </summary>
-    public ushort HotspotX { get => this.Field1; set => this.Field1 = value; }
+    public ushort HotspotX { get; set; }
 
     /// <summary>
     /// Gets or sets Specifies the vertical coordinates of the hotspot in number of pixels from the top.
     /// </summary>
-    public ushort HotspotY { get => this.Field2; set => this.Field2 = value; }
+    public ushort HotspotY { get; set; }
+
+    /// <summary>
+    /// Gets or sets Height field. <br />
+    /// Specifies image height in pixels. Can be any number between 0 and 255. Value 0 means image height is 256 pixels.
+    /// </summary>
+    public byte EncodingHeight { get; set; }
+
+    /// <summary>
+    /// Gets or sets Width field. <br />
+    /// Specifies image width in pixels. Can be any number between 0 and 255. Value 0 means image width is 256 pixels.
+    /// </summary>
+    public byte EncodingWidth { get; set; }
+
+    /// <inheritdoc cref="Bmp.BmpMetadata.BitsPerPixel" />
+    public Bmp.BmpBitsPerPixel BitsPerPixel { get; set; } = Bmp.BmpBitsPerPixel.Pixel24;
 
     /// <inheritdoc/>
-    public override CurFrameMetadata DeepClone() => new(this);
+    public CurFrameMetadata DeepClone() => new(this);
+
+    /// <inheritdoc/>
+    IDeepCloneable IDeepCloneable.DeepClone() => this.DeepClone();
+
+    internal void FromIconDirEntry(in IconDirEntry entry)
+    {
+        this.EncodingWidth = entry.Width;
+        this.EncodingHeight = entry.Height;
+        this.ColorCount = entry.ColorCount;
+        this.HotspotX = entry.Planes;
+        this.HotspotY = entry.BitCount;
+    }
+
+    internal IconDirEntry ToIconDirEntry() => new()
+    {
+        Width = this.EncodingWidth,
+        Height = this.EncodingHeight,
+        ColorCount = this.ColorCount,
+        Planes = this.HotspotX,
+        BitCount = this.HotspotY,
+    };
 }

src/ImageSharp/Formats/Icon/Ico/IcoDecoderCore.cs

@@ -14,5 +14,6 @@ internal sealed class IcoDecoderCore : IconDecoderCore
     {
     }
 
-    protected override IconFrameMetadata GetFrameMetadata(ImageFrameMetadata metadata) => metadata.GetIcoMetadata();
+    protected override void SetFrameMetadata(ImageFrameMetadata metadata, in IconDirEntry entry)
+        => metadata.GetIcoMetadata().FromIconDirEntry(entry);
 }

src/ImageSharp/Formats/Icon/Ico/IcoFrameMetadata.cs

@@ -6,7 +6,7 @@ namespace SixLabors.ImageSharp.Formats.Icon.Ico;
 /// <summary>
 /// IcoFrameMetadata. TODO: Remove base class and merge into this class.
 /// </summary>
-public class IcoFrameMetadata : IconFrameMetadata, IDeepCloneable<IcoFrameMetadata>, IDeepCloneable
+public class IcoFrameMetadata : IDeepCloneable<IcoFrameMetadata>, IDeepCloneable
 {
     /// <summary>
     /// Initializes a new instance of the <see cref="IcoFrameMetadata"/> class.
@@ -15,34 +15,78 @@ public class IcoFrameMetadata : IconFrameMetadata, IDeepCloneable<IcoFrameMetada
     {
     }
 
-    /// <summary>
-    /// Initializes a new instance of the <see cref="IcoFrameMetadata"/> class.
-    /// </summary>
-    /// <param name="metadata">metadata</param>
-    public IcoFrameMetadata(IconFrameMetadata metadata)
-        : base(metadata)
-    {
-    }
-
     /// <summary>
     /// Initializes a new instance of the <see cref="IcoFrameMetadata"/> class.
     /// </summary>
     /// <param name="width">width</param>
     /// <param name="height">height</param>
     /// <param name="colorCount">colorCount</param>
-    /// <param name="field1">field1</param>
-    /// <param name="field2">field2</param>
-    public IcoFrameMetadata(byte width, byte height, byte colorCount, ushort field1, ushort field2)
-        : base(width, height, colorCount, field1, field2)
+    public IcoFrameMetadata(byte width, byte height, byte colorCount)
     {
+        this.EncodingWidth = width;
+        this.EncodingHeight = height;
+        this.ColorCount = colorCount;
     }
 
+    /// <inheritdoc cref="IcoFrameMetadata()"/>
+    public IcoFrameMetadata(IcoFrameMetadata metadata)
+    {
+        this.EncodingWidth = metadata.EncodingWidth;
+        this.EncodingHeight = metadata.EncodingHeight;
+        this.ColorCount = metadata.ColorCount;
+        this.Compression = metadata.Compression;
+    }
+
+    /// <summary>
+    /// Gets or sets icoFrameCompression.
+    /// </summary>
+    public IconFrameCompression Compression { get; set; }
+
+    /// <summary>
+    /// Gets or sets ColorCount field. <br />
+    /// Specifies number of colors in the color palette. Should be 0 if the image does not use a color palette.
+    /// </summary>
+    // TODO: BmpMetadata does not supported palette yet.
+    public byte ColorCount { get; set; }
+
     /// <summary>
-    /// Gets or sets the bits per pixel.
-    /// TODO: This needs to be constrained and calculated using the metadata returned from the png/bmp.
+    /// Gets or sets Height field. <br />
+    /// Specifies image height in pixels. Can be any number between 0 and 255. Value 0 means image height is 256 pixels.
     /// </summary>
-    public ushort BitCount { get => this.Field2; set => this.Field2 = value; }
+    public byte EncodingHeight { get; set; }
+
+    /// <summary>
+    /// Gets or sets Width field. <br />
+    /// Specifies image width in pixels. Can be any number between 0 and 255. Value 0 means image width is 256 pixels.
+    /// </summary>
+    public byte EncodingWidth { get; set; }
+
+    /// <inheritdoc cref="Bmp.BmpMetadata.BitsPerPixel" />
+    public Bmp.BmpBitsPerPixel BitsPerPixel { get; set; } = Bmp.BmpBitsPerPixel.Pixel24;
 
     /// <inheritdoc/>
-    public override IcoFrameMetadata DeepClone() => new(this);
+    public IcoFrameMetadata DeepClone() => new(this);
+
+    /// <inheritdoc/>
+    IDeepCloneable IDeepCloneable.DeepClone() => this.DeepClone();
+
+    internal void FromIconDirEntry(in IconDirEntry entry)
+    {
+        this.EncodingWidth = entry.Width;
+        this.EncodingHeight = entry.Height;
+        this.ColorCount = entry.ColorCount;
+    }
+
+    internal IconDirEntry ToIconDirEntry() => new()
+    {
+        Width = this.EncodingWidth,
+        Height = this.EncodingHeight,
+        ColorCount = this.ColorCount,
+        Planes = 1,
+        BitCount = this.Compression switch
+        {
+            IconFrameCompression.Bmp => (ushort)this.BitsPerPixel,
+            _ => 0,
+        },
+    };
 }

src/ImageSharp/Formats/Icon/IconDecoderCore.cs

@@ -93,9 +93,7 @@ internal abstract class IconDecoderCore : IImageDecoderInternals
                 bmpMetadata = x.Image.Metadata.GetBmpMetadata();
             }
 
-            // TODO: The inheriting decoder should be responsible for setting the actual data (FromIconDirEntry)
-            // so we can avoid the protected Field1 and Field2 properties and use strong typing.
-            this.GetFrameMetadata(target.Metadata).FromIconDirEntry(this.Entries[x.Index]);
+            this.SetFrameMetadata(target.Metadata, this.Entries[x.Index]);
 
             x.Image.Dispose();
 
@@ -127,15 +125,14 @@ internal abstract class IconDecoderCore : IImageDecoderInternals
             // TODO: Use the Identify methods in each decoder to return the
             // format specific metadata for the image and frame.
             frames[i] = new();
-            IconFrameMetadata icoFrameMetadata = this.GetFrameMetadata(frames[i]);
-            icoFrameMetadata.FromIconDirEntry(this.Entries[i]);
+            this.SetFrameMetadata(frames[i], this.Entries[i]);
         }
 
         // TODO: Use real values from the metadata.
         return new(new(32), new(0), metadata, frames);
     }
 
-    protected abstract IconFrameMetadata GetFrameMetadata(ImageFrameMetadata metadata);
+    protected abstract void SetFrameMetadata(ImageFrameMetadata metadata, in IconDirEntry entry);
 
     protected void ReadHeader(Stream stream)
     {

src/ImageSharp/Formats/Icon/IconFrameMetadata.cs

@@ -1,104 +0,0 @@
-// Copyright (c) Six Labors.
-// Licensed under the Six Labors Split License.
-
-namespace SixLabors.ImageSharp.Formats.Icon;
-
-/// <summary>
-/// IconFrameMetadata
-/// TODO: Delete this. Treat the individual metadata types as separate types so we can avoid Field1, Field2 and use strong typing with constaints.
-/// </summary>
-public abstract class IconFrameMetadata : IDeepCloneable
-{
-    /// <summary>
-    /// Initializes a new instance of the <see cref="IconFrameMetadata"/> class.
-    /// </summary>
-    protected IconFrameMetadata()
-    {
-    }
-
-    /// <summary>
-    /// Initializes a new instance of the <see cref="IconFrameMetadata"/> class.
-    /// </summary>
-    /// <param name="width">width</param>
-    /// <param name="height">height</param>
-    /// <param name="colorCount">colorCount</param>
-    /// <param name="field1">field1</param>
-    /// <param name="field2">field2</param>
-    protected IconFrameMetadata(byte width, byte height, byte colorCount, ushort field1, ushort field2)
-    {
-        this.EncodingWidth = width;
-        this.EncodingHeight = height;
-        this.ColorCount = colorCount;
-        this.Field1 = field1;
-        this.Field2 = field2;
-    }
-
-    /// <inheritdoc cref="IconFrameMetadata()"/>
-    protected IconFrameMetadata(IconFrameMetadata metadata)
-    {
-        this.EncodingWidth = metadata.EncodingWidth;
-        this.EncodingHeight = metadata.EncodingHeight;
-        this.ColorCount = metadata.ColorCount;
-        this.Field1 = metadata.Field1;
-        this.Field2 = metadata.Field2;
-    }
-
-    /// <summary>
-    /// Gets or sets icoFrameCompression.
-    /// </summary>
-    public IconFrameCompression Compression { get; protected set; }
-
-    /// <summary>
-    /// Gets or sets ColorCount field. <br />
-    /// Specifies number of colors in the color palette. Should be 0 if the image does not use a color palette.
-    /// </summary>
-    // TODO: BmpMetadata does not supported palette yet.
-    public byte ColorCount { get; set; }
-
-    /// <summary>
-    /// Gets or sets Planes field. <br />
-    /// In ICO format: Specifies color planes. Should be 0 or 1. <br />
-    /// In CUR format: Specifies the horizontal coordinates of the hotspot in number of pixels from the left.
-    /// </summary>
-    protected ushort Field1 { get; set; }
-
-    /// <summary>
-    /// Gets or sets BitCount field. <br />
-    /// In ICO format: Specifies bits per pixel. <br />
-    /// In CUR format: Specifies the vertical coordinates of the hotspot in number of pixels from the top.
-    /// </summary>
-    protected ushort Field2 { get; set; }
-
-    /// <summary>
-    /// Gets or sets Height field. <br />
-    /// Specifies image height in pixels. Can be any number between 0 and 255. Value 0 means image height is 256 pixels.
-    /// </summary>
-    public byte EncodingHeight { get; set; }
-
-    /// <summary>
-    /// Gets or sets Width field. <br />
-    /// Specifies image width in pixels. Can be any number between 0 and 255. Value 0 means image width is 256 pixels.
-    /// </summary>
-    public byte EncodingWidth { get; set; }
-
-    /// <inheritdoc/>
-    public abstract IDeepCloneable DeepClone();
-
-    internal void FromIconDirEntry(in IconDirEntry metadata)
-    {
-        this.EncodingWidth = metadata.Width;
-        this.EncodingHeight = metadata.Height;
-        this.ColorCount = metadata.ColorCount;
-        this.Field1 = metadata.Planes;
-        this.Field2 = metadata.BitCount;
-    }
-
-    internal IconDirEntry ToIconDirEntry() => new()
-    {
-        Width = this.EncodingWidth,
-        Height = this.EncodingHeight,
-        ColorCount = this.ColorCount,
-        Planes = this.Field1,
-        BitCount = this.Field2,
-    };
-}