diff --git a/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs b/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs index 9bea6867a..568eea00a 100644 --- a/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs +++ b/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs @@ -105,7 +105,7 @@ internal sealed class BmpDecoderCore : IImageDecoderInternals /// private readonly bool skipFileHeader; - /// + /// private readonly bool isDoubleHeight; /// @@ -120,7 +120,7 @@ internal sealed class BmpDecoderCore : IImageDecoderInternals this.memoryAllocator = this.configuration.MemoryAllocator; this.processedAlphaMask = options.ProcessedAlphaMask; this.skipFileHeader = options.SkipFileHeader; - this.isDoubleHeight = options.IsDoubleHeight; + this.isDoubleHeight = options.UseDoubleHeight; } /// diff --git a/src/ImageSharp/Formats/Bmp/BmpDecoderOptions.cs b/src/ImageSharp/Formats/Bmp/BmpDecoderOptions.cs index 17d37cab2..158a9d479 100644 --- a/src/ImageSharp/Formats/Bmp/BmpDecoderOptions.cs +++ b/src/ImageSharp/Formats/Bmp/BmpDecoderOptions.cs @@ -18,10 +18,10 @@ public sealed class BmpDecoderOptions : ISpecializedDecoderOptions public RleSkippedPixelHandling RleSkippedPixelHandling { get; init; } /// - /// Gets a value indicating whether the additional AlphaMask is processed at decoding time. + /// Gets a value indicating whether the additional alpha mask is processed at decoding time. /// /// - /// It will be used at IcoDecoder. + /// Used by the icon decoder. /// internal bool ProcessedAlphaMask { get; init; } @@ -29,15 +29,15 @@ public sealed class BmpDecoderOptions : ISpecializedDecoderOptions /// Gets a value indicating whether to skip loading the BMP file header. /// /// - /// It will be used at IcoDecoder. + /// Used by the icon decoder. /// internal bool SkipFileHeader { get; init; } /// - /// Gets a value indicating whether the height is double of true height. + /// Gets a value indicating whether to treat the height as double of true height. /// /// - /// It will be used at IcoDecoder. + /// Used by the icon decoder. /// - internal bool IsDoubleHeight { get; init; } + internal bool UseDoubleHeight { get; init; } } diff --git a/src/ImageSharp/Formats/Icon/Cur/CurDecoderCore.cs b/src/ImageSharp/Formats/Icon/Cur/CurDecoderCore.cs index 8b08f127d..44fcc8fcc 100644 --- a/src/ImageSharp/Formats/Icon/Cur/CurDecoderCore.cs +++ b/src/ImageSharp/Formats/Icon/Cur/CurDecoderCore.cs @@ -3,6 +3,8 @@ using SixLabors.ImageSharp.Metadata; +// TODO: flatten namespace. +// namespace SixLabors.ImageSharp.Formats.Cur; namespace SixLabors.ImageSharp.Formats.Icon.Cur; internal sealed class CurDecoderCore : IconDecoderCore diff --git a/src/ImageSharp/Formats/Icon/Cur/CurFrameMetadata.cs b/src/ImageSharp/Formats/Icon/Cur/CurFrameMetadata.cs index c94afdd3a..60ced16ea 100644 --- a/src/ImageSharp/Formats/Icon/Cur/CurFrameMetadata.cs +++ b/src/ImageSharp/Formats/Icon/Cur/CurFrameMetadata.cs @@ -4,7 +4,7 @@ namespace SixLabors.ImageSharp.Formats.Icon.Cur; /// -/// IcoFrameMetadata +/// IcoFrameMetadata. TODO: Remove base class and merge into this class. /// public class CurFrameMetadata : IconFrameMetadata, IDeepCloneable, IDeepCloneable { diff --git a/src/ImageSharp/Formats/Icon/Ico/IcoDecoderCore.cs b/src/ImageSharp/Formats/Icon/Ico/IcoDecoderCore.cs index 0782c2128..677a9c07a 100644 --- a/src/ImageSharp/Formats/Icon/Ico/IcoDecoderCore.cs +++ b/src/ImageSharp/Formats/Icon/Ico/IcoDecoderCore.cs @@ -3,6 +3,8 @@ using SixLabors.ImageSharp.Metadata; +// TODO: flatten namespace. +// namespace SixLabors.ImageSharp.Formats.Ico; namespace SixLabors.ImageSharp.Formats.Icon.Ico; internal sealed class IcoDecoderCore : IconDecoderCore diff --git a/src/ImageSharp/Formats/Icon/Ico/IcoFrameMetadata.cs b/src/ImageSharp/Formats/Icon/Ico/IcoFrameMetadata.cs index 7c903facd..4f557715d 100644 --- a/src/ImageSharp/Formats/Icon/Ico/IcoFrameMetadata.cs +++ b/src/ImageSharp/Formats/Icon/Ico/IcoFrameMetadata.cs @@ -4,7 +4,7 @@ namespace SixLabors.ImageSharp.Formats.Icon.Ico; /// -/// IcoFrameMetadata +/// IcoFrameMetadata. TODO: Remove base class and merge into this class. /// public class IcoFrameMetadata : IconFrameMetadata, IDeepCloneable, IDeepCloneable { @@ -38,11 +38,9 @@ public class IcoFrameMetadata : IconFrameMetadata, IDeepCloneable - /// Gets or sets Specifies bits per pixel. + /// Gets or sets the bits per pixel. + /// TODO: This needs to be constrained and calculated using the metadata returned from the png/bmp. /// - /// - /// It may used by Encoder. - /// public ushort BitCount { get => this.Field2; set => this.Field2 = value; } /// diff --git a/src/ImageSharp/Formats/Icon/IconAssert.cs b/src/ImageSharp/Formats/Icon/IconAssert.cs index bcb427c1a..04a9527b9 100644 --- a/src/ImageSharp/Formats/Icon/IconAssert.cs +++ b/src/ImageSharp/Formats/Icon/IconAssert.cs @@ -32,12 +32,4 @@ internal class IconAssert return v; } - - internal static void NotSquare(in Size size) - { - if (size.Width != size.Height) - { - throw new FormatException("This image is not square."); - } - } } diff --git a/src/ImageSharp/Formats/Icon/IconDecoderCore.cs b/src/ImageSharp/Formats/Icon/IconDecoderCore.cs index d9a578ff2..d7fc25c69 100644 --- a/src/ImageSharp/Formats/Icon/IconDecoderCore.cs +++ b/src/ImageSharp/Formats/Icon/IconDecoderCore.cs @@ -2,7 +2,8 @@ // Licensed under the Six Labors Split License. using System.Runtime.InteropServices; -using SixLabors.ImageSharp; +using SixLabors.ImageSharp.Formats.Bmp; +using SixLabors.ImageSharp.Formats.Png; using SixLabors.ImageSharp.IO; using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.PixelFormats; @@ -30,50 +31,63 @@ internal abstract class IconDecoderCore : IImageDecoderInternals long basePosition = stream.Position; this.ReadHeader(stream); - Span flag = stackalloc byte[Png.PngConstants.HeaderBytes.Length]; + Span flag = stackalloc byte[PngConstants.HeaderBytes.Length]; + Image result = new(this.Dimensions.Width, this.Dimensions.Height); - List<(Image Image, bool IsPng, int Index)> frames = new(this.Entries.Length); for (int i = 0; i < this.Entries.Length; i++) { - _ = IconAssert.EndOfStream(stream.Seek(basePosition + this.Entries[i].ImageOffset, SeekOrigin.Begin), basePosition + this.Entries[i].ImageOffset); - _ = IconAssert.EndOfStream(stream.Read(flag), Png.PngConstants.HeaderBytes.Length); - _ = stream.Seek(-Png.PngConstants.HeaderBytes.Length, SeekOrigin.Current); + ref IconDirEntry entry = ref this.Entries[i]; - bool isPng = flag.SequenceEqual(Png.PngConstants.HeaderBytes); + // If we hit the end of the stream we should break. + if (stream.Seek(basePosition + entry.ImageOffset, SeekOrigin.Begin) >= stream.Length) + { + break; + } - Image img = this.GetDecoder(isPng).Decode(stream, cancellationToken); - IconAssert.NotSquare(img.Size); - frames.Add((img, isPng, i)); - if (isPng && img.Size.Width > this.Dimensions.Width) + // There should always be enough bytes for this regardless of the entry type. + if (stream.Read(flag) != PngConstants.HeaderBytes.Length) { - this.Dimensions = img.Size; + break; } - } - ImageMetadata metadata = new(); - return new(this.Options.Configuration, metadata, frames.Select(i => - { - ImageFrame target = new(this.Options.Configuration, this.Dimensions); - ImageFrame source = i.Image.Frames.RootFrameUnsafe; + // Reset the stream position. + stream.Seek(-PngConstants.HeaderBytes.Length, SeekOrigin.Current); + + bool isPng = flag.SequenceEqual(PngConstants.HeaderBytes); + using Image temp = this.GetDecoder(isPng).Decode(stream, cancellationToken); + + ImageFrame source = temp.Frames.RootFrameUnsafe; + ImageFrame target = i == 0 ? result.Frames.RootFrameUnsafe : result.Frames.CreateFrame(); + + // Draw the new frame at position 0,0. We capture the dimensions for cropping during encoding + // via the icon entry. for (int h = 0; h < source.Height; h++) { source.PixelBuffer.DangerousGetRowSpan(h).CopyTo(target.PixelBuffer.DangerousGetRowSpan(h)); } - if (i.IsPng) + // Copy the format specific metadata to the image. + if (isPng) { - target.Metadata.UnsafeSetFormatMetadata(Png.PngFormat.Instance, i.Image.Metadata.GetPngMetadata()); + if (i == 0) + { + result.Metadata.SetFormatMetadata(PngFormat.Instance, temp.Metadata.GetPngMetadata()); + } + + target.Metadata.SetFormatMetadata(PngFormat.Instance, target.Metadata.GetPngFrameMetadata()); } - else + else if (i == 0) { - target.Metadata.UnsafeSetFormatMetadata(Bmp.BmpFormat.Instance, i.Image.Metadata.GetBmpMetadata()); + // Bmp does not contain frame specific metadata. + result.Metadata.SetFormatMetadata(BmpFormat.Instance, temp.Metadata.GetBmpMetadata()); } - this.GetFrameMetadata(target.Metadata).FromIconDirEntry(this.Entries[i.Index]); + // 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(entry); + } - i.Image.Dispose(); - return target; - }).ToArray()); + return result; } public ImageInfo Identify(BufferedReadStream stream, CancellationToken cancellationToken) @@ -84,11 +98,14 @@ internal abstract class IconDecoderCore : IImageDecoderInternals ImageFrameMetadata[] frames = new ImageFrameMetadata[this.FileHeader.Count]; for (int i = 0; i < frames.Length; i++) { + // 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]); } + // TODO: Use real values from the metadata. return new(new(32), new(0), metadata, frames); } @@ -96,6 +113,7 @@ internal abstract class IconDecoderCore : IImageDecoderInternals protected void ReadHeader(Stream stream) { + // TODO: Check length and throw if the header cannot be read. _ = Read(stream, out this.fileHeader, IconDir.Size); this.Entries = new IconDirEntry[this.FileHeader.Count]; for (int i = 0; i < this.Entries.Length; i++) @@ -103,15 +121,39 @@ internal abstract class IconDecoderCore : IImageDecoderInternals _ = Read(stream, out this.Entries[i], IconDirEntry.Size); } - this.Dimensions = new( - this.Entries.Max(i => i.Width), - this.Entries.Max(i => i.Height)); + int width = 0; + int height = 0; + foreach (IconDirEntry entry in this.Entries) + { + if (entry.Width == 0) + { + width = 256; + } + + if (entry.Height == 0) + { + height = 256; + } + + if (width == 256 && height == 256) + { + break; + } + + width = Math.Max(width, entry.Width); + height = Math.Max(height, entry.Height); + } + + this.Dimensions = new(width, height); } private static int Read(Stream stream, out T data, int size) where T : unmanaged { + // TODO: Use explicit parsing methods for each T type. + // See PngHeader.Parse() for example. Span buffer = stackalloc byte[size]; + _ = IconAssert.EndOfStream(stream.Read(buffer), size); data = MemoryMarshal.Cast(buffer)[0]; return size; @@ -121,15 +163,16 @@ internal abstract class IconDecoderCore : IImageDecoderInternals { if (isPng) { - return new Png.PngDecoderCore(this.Options); + return new PngDecoderCore(this.Options); } else { - return new Bmp.BmpDecoderCore(new() + return new BmpDecoderCore(new() { + GeneralOptions = this.Options, ProcessedAlphaMask = true, SkipFileHeader = true, - IsDoubleHeight = true, + UseDoubleHeight = true, }); } } diff --git a/src/ImageSharp/Formats/Icon/IconFrameCompression.cs b/src/ImageSharp/Formats/Icon/IconFrameCompression.cs index f6509f40c..5c772c3fe 100644 --- a/src/ImageSharp/Formats/Icon/IconFrameCompression.cs +++ b/src/ImageSharp/Formats/Icon/IconFrameCompression.cs @@ -8,11 +8,6 @@ namespace SixLabors.ImageSharp.Formats.Icon; /// public enum IconFrameCompression { - /// - /// Unknown - /// - Unknown, - /// /// Bmp /// diff --git a/src/ImageSharp/Formats/Icon/IconFrameMetadata.cs b/src/ImageSharp/Formats/Icon/IconFrameMetadata.cs index 17e641c71..5c23388e7 100644 --- a/src/ImageSharp/Formats/Icon/IconFrameMetadata.cs +++ b/src/ImageSharp/Formats/Icon/IconFrameMetadata.cs @@ -5,6 +5,7 @@ namespace SixLabors.ImageSharp.Formats.Icon; /// /// IconFrameMetadata +/// TODO: Delete this. Treat the individual metadata types as separate types so we can avoid Field1, Field2 and use strong typing with constaints. /// public abstract class IconFrameMetadata : IDeepCloneable { diff --git a/src/ImageSharp/Metadata/ImageFrameMetadata.cs b/src/ImageSharp/Metadata/ImageFrameMetadata.cs index 0567c8916..562e47803 100644 --- a/src/ImageSharp/Metadata/ImageFrameMetadata.cs +++ b/src/ImageSharp/Metadata/ImageFrameMetadata.cs @@ -99,9 +99,9 @@ public sealed class ImageFrameMetadata : IDeepCloneable return newMeta; } - internal void UnsafeSetFormatMetadata( - IImageFormat key, - IDeepCloneable value) + internal void SetFormatMetadata(IImageFormat key, TFormatFrameMetadata value) + where TFormatMetadata : class + where TFormatFrameMetadata : class, IDeepCloneable => this.formatMetadata[key] = value; /// diff --git a/src/ImageSharp/Metadata/ImageMetadata.cs b/src/ImageSharp/Metadata/ImageMetadata.cs index 6b62be08f..d9aba6631 100644 --- a/src/ImageSharp/Metadata/ImageMetadata.cs +++ b/src/ImageSharp/Metadata/ImageMetadata.cs @@ -215,6 +215,9 @@ public sealed class ImageMetadata : IDeepCloneable metadata = default; return false; } + internal void SetFormatMetadata(IImageFormat key, TFormatMetadata value) + where TFormatMetadata : class, IDeepCloneable + => this.formatMetadata[key] = value; /// public ImageMetadata DeepClone() => new(this); diff --git a/tests/ImageSharp.Tests/Formats/Icon/Cur/CurDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Icon/Cur/CurDecoderTests.cs index c3c9ad1c4..f78c04cdf 100644 --- a/tests/ImageSharp.Tests/Formats/Icon/Cur/CurDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Icon/Cur/CurDecoderTests.cs @@ -2,7 +2,6 @@ // Licensed under the Six Labors Split License. using SixLabors.ImageSharp.Formats.Icon.Cur; -using SixLabors.ImageSharp.Formats.Tiff; using SixLabors.ImageSharp.PixelFormats; using static SixLabors.ImageSharp.Tests.TestImages.Cur; @@ -18,6 +17,10 @@ public class CurDecoderTests { using Image image = provider.GetImage(CurDecoder.Instance); - image.DebugSave(provider, extension: "tiff", encoder: new TiffEncoder()); + image.DebugSaveMultiFrame(provider, extension: "png"); + + image.DebugSaveMultiFrame(provider, extension: "png"); + + // TODO: Assert metadata, frame count, etc } } diff --git a/tests/ImageSharp.Tests/Formats/Icon/Ico/IcoDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Icon/Ico/IcoDecoderTests.cs index 6e7b35111..86f8b003b 100644 --- a/tests/ImageSharp.Tests/Formats/Icon/Ico/IcoDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Icon/Ico/IcoDecoderTests.cs @@ -2,7 +2,6 @@ // Licensed under the Six Labors Split License. using SixLabors.ImageSharp.Formats.Icon.Ico; -using SixLabors.ImageSharp.Formats.Tiff; using SixLabors.ImageSharp.PixelFormats; using static SixLabors.ImageSharp.Tests.TestImages.Ico; @@ -18,6 +17,8 @@ public class IcoDecoderTests { using Image image = provider.GetImage(IcoDecoder.Instance); - image.DebugSave(provider, extension: "tiff", encoder: new TiffEncoder()); + image.DebugSaveMultiFrame(provider, extension: "png"); + + // TODO: Assert metadata, frame count, etc } }