From 6e64c4c435a7bf77cf174b0b3c8a03a3b7220c44 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 10:18:10 -0700 Subject: [PATCH 01/31] Write the LogicalScreenDescriptor struct directly to the buffer --- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 7 +- .../Sections/GifLogicalScreenDescriptor.cs | 81 +++++++++---------- 2 files changed, 39 insertions(+), 49 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index 9651cf440..e93e23f70 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -168,13 +168,12 @@ namespace SixLabors.ImageSharp.Formats.Gif private void WriteLogicalScreenDescriptor(Image image, Stream stream, int transparencyIndex) where TPixel : struct, IPixel { + byte packedValue = GifLogicalScreenDescriptor.GetPackedValue(false, this.bitDepth - 1, false, this.bitDepth - 1); + var descriptor = new GifLogicalScreenDescriptor( width: (ushort)image.Width, height: (ushort)image.Height, - bitsPerPixel: 0, - pixelAspectRatio: 0, - globalColorTableFlag: false, // TODO: Always false for now. - globalColorTableSize: this.bitDepth - 1, + packed: packedValue, backgroundColorIndex: unchecked((byte)transparencyIndex)); descriptor.WriteTo(this.buffer); diff --git a/src/ImageSharp/Formats/Gif/Sections/GifLogicalScreenDescriptor.cs b/src/ImageSharp/Formats/Gif/Sections/GifLogicalScreenDescriptor.cs index 4f2a17ddf..35056c6e7 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifLogicalScreenDescriptor.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifLogicalScreenDescriptor.cs @@ -2,7 +2,8 @@ // Licensed under the Apache License, Version 2.0. using System; -using System.Buffers.Binary; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; namespace SixLabors.ImageSharp.Formats.Gif { @@ -11,29 +12,23 @@ namespace SixLabors.ImageSharp.Formats.Gif /// necessary to define the area of the display device /// within which the images will be rendered /// + [StructLayout(LayoutKind.Sequential, Pack = 1)] internal readonly struct GifLogicalScreenDescriptor { - /// - /// The size of the written structure. - /// public const int Size = 7; public GifLogicalScreenDescriptor( ushort width, ushort height, - int bitsPerPixel, + byte packed, byte backgroundColorIndex, - byte pixelAspectRatio, - bool globalColorTableFlag, - int globalColorTableSize) + byte pixelAspectRatio = 0) { this.Width = width; this.Height = height; - this.BitsPerPixel = bitsPerPixel; + this.Packed = packed; this.BackgroundColorIndex = backgroundColorIndex; this.PixelAspectRatio = pixelAspectRatio; - this.GlobalColorTableFlag = globalColorTableFlag; - this.GlobalColorTableSize = globalColorTableSize; } /// @@ -49,9 +44,10 @@ namespace SixLabors.ImageSharp.Formats.Gif public ushort Height { get; } /// - /// Gets the color depth, in number of bits per pixel. + /// Gets the packed value consisting of: + /// globalColorTableFlag, colorResolution, sortFlag, and sizeOfGlobalColorTable. /// - public int BitsPerPixel { get; } + public byte Packed { get; } /// /// Gets the index at the Global Color Table for the Background Color. @@ -61,59 +57,42 @@ namespace SixLabors.ImageSharp.Formats.Gif public byte BackgroundColorIndex { get; } /// - /// Gets the pixel aspect ratio. Default to 0. + /// Gets the pixel aspect ratio. /// public byte PixelAspectRatio { get; } /// /// Gets a value indicating whether a flag denoting the presence of a Global Color Table /// should be set. - /// If the flag is set, the Global Color Table will immediately - /// follow the Logical Screen Descriptor. + /// If the flag is set, the Global Color Table will included after + /// the Logical Screen Descriptor. /// - public bool GlobalColorTableFlag { get; } + public bool GlobalColorTableFlag => ((this.Packed & 0x80) >> 7) == 1; /// /// Gets the global color table size. - /// If the Global Color Table Flag is set to 1, + /// If the Global Color Table Flag is set, /// the value in this field is used to calculate the number of /// bytes contained in the Global Color Table. /// - public int GlobalColorTableSize { get; } - - public byte PackFields() - { - PackedField field = default; + public int GlobalColorTableSize => 2 << (this.Packed & 0x07); - field.SetBit(0, this.GlobalColorTableFlag); // 0 : Global Color Table Flag | 1 bit - field.SetBits(1, 3, this.GlobalColorTableSize); // 1-3 : Color Resolution | 3 bits - field.SetBit(4, false); // 4 : Sort Flag | 1 bits - field.SetBits(5, 3, this.GlobalColorTableSize); // 5-7 : Size of Global Color Table | 3 bits - - return field.Byte; - } + /// + /// Gets the color depth, in number of bits per pixel. + /// The lowest 3 packed bits represent the bit depth minus 1. + /// + public int BitsPerPixel => (this.Packed & 0x07) + 1; public void WriteTo(Span buffer) { - BinaryPrimitives.WriteUInt16LittleEndian(buffer.Slice(0, 2), this.Width); // Logical Screen Width - BinaryPrimitives.WriteUInt16LittleEndian(buffer.Slice(2, 2), this.Height); // Logical Screen Height - buffer[4] = this.PackFields(); // Packed Fields - buffer[5] = this.BackgroundColorIndex; // Background Color Index - buffer[6] = this.PixelAspectRatio; // Pixel Aspect Ratio + ref GifLogicalScreenDescriptor dest = ref Unsafe.As(ref MemoryMarshal.GetReference(buffer)); + + dest = this; } public static GifLogicalScreenDescriptor Parse(ReadOnlySpan buffer) { - byte packed = buffer[4]; - - var result = new GifLogicalScreenDescriptor( - width: BinaryPrimitives.ReadUInt16LittleEndian(buffer.Slice(0, 2)), - height: BinaryPrimitives.ReadUInt16LittleEndian(buffer.Slice(2, 2)), - bitsPerPixel: (buffer[4] & 0x07) + 1, // The lowest 3 bits represent the bit depth minus 1 - backgroundColorIndex: buffer[5], - pixelAspectRatio: buffer[6], - globalColorTableFlag: ((packed & 0x80) >> 7) == 1, - globalColorTableSize: 2 << (packed & 0x07)); + GifLogicalScreenDescriptor result = MemoryMarshal.Cast(buffer)[0]; if (result.GlobalColorTableSize > 255 * 4) { @@ -122,5 +101,17 @@ namespace SixLabors.ImageSharp.Formats.Gif return result; } + + public static byte GetPackedValue(bool globalColorTableFlag, int colorResolution, bool sortFlag, int globalColorTableSize) + { + PackedField field = default; + + field.SetBit(0, globalColorTableFlag); // 0 : Global Color Table Flag | 1 bit + field.SetBits(1, 3, globalColorTableSize); // 1-3 : Color Resolution | 3 bits + field.SetBit(4, sortFlag); // 4 : Sort Flag | 1 bits + field.SetBits(5, 3, globalColorTableSize); // 5-7 : Size of Global Color Table | 3 bits + + return field.Byte; + } } } \ No newline at end of file From 3678f2d08c8a33b7406347398eb81d240d6b0bd0 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 10:28:13 -0700 Subject: [PATCH 02/31] Rename currentStream to stream --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 30 ++++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 1f2cccc6e..4d6f010de 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -32,7 +32,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// The currently loaded stream. /// - private Stream currentStream; + private Stream stream; /// /// The global color table. @@ -236,7 +236,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// private void ReadGraphicalControlExtension() { - this.currentStream.Read(this.buffer, 0, 6); + this.stream.Read(this.buffer, 0, 6); this.graphicsControlExtension = GifGraphicsControlExtension.Parse(this.buffer); } @@ -247,7 +247,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// private GifImageDescriptor ReadImageDescriptor() { - this.currentStream.Read(this.buffer, 0, 9); + this.stream.Read(this.buffer, 0, 9); return GifImageDescriptor.Parse(this.buffer); } @@ -257,7 +257,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// private void ReadLogicalScreenDescriptor() { - this.currentStream.Read(this.buffer, 0, 7); + this.stream.Read(this.buffer, 0, 7); this.logicalScreenDescriptor = GifLogicalScreenDescriptor.Parse(this.buffer); } @@ -268,13 +268,13 @@ namespace SixLabors.ImageSharp.Formats.Gif /// The number of bytes to skip. private void Skip(int length) { - this.currentStream.Skip(length); + this.stream.Skip(length); int flag; - while ((flag = this.currentStream.ReadByte()) != 0) + while ((flag = this.stream.ReadByte()) != 0) { - this.currentStream.Skip(flag); + this.stream.Skip(flag); } } @@ -285,7 +285,7 @@ namespace SixLabors.ImageSharp.Formats.Gif { int length; - while ((length = this.currentStream.ReadByte()) != 0) + while ((length = this.stream.ReadByte()) != 0) { if (length > GifConstants.MaxCommentLength) { @@ -294,13 +294,13 @@ namespace SixLabors.ImageSharp.Formats.Gif if (this.IgnoreMetadata) { - this.currentStream.Seek(length, SeekOrigin.Current); + this.stream.Seek(length, SeekOrigin.Current); continue; } using (IManagedByteBuffer commentsBuffer = this.MemoryManager.AllocateManagedByteBuffer(length)) { - this.currentStream.Read(commentsBuffer.Array, 0, length); + this.stream.Read(commentsBuffer.Array, 0, length); string comments = this.TextEncoding.GetString(commentsBuffer.Array, 0, length); this.metaData.Properties.Add(new ImageProperty(GifConstants.Comments, comments)); } @@ -327,7 +327,7 @@ namespace SixLabors.ImageSharp.Formats.Gif { int length = imageDescriptor.LocalColorTableSize * 3; localColorTable = this.configuration.MemoryManager.AllocateManagedByteBuffer(length, true); - this.currentStream.Read(localColorTable.Array, 0, length); + this.stream.Read(localColorTable.Array, 0, length); } indices = this.configuration.MemoryManager.AllocateManagedByteBuffer(imageDescriptor.Width * imageDescriptor.Height, true); @@ -354,8 +354,8 @@ namespace SixLabors.ImageSharp.Formats.Gif [MethodImpl(MethodImplOptions.AggressiveInlining)] private void ReadFrameIndices(in GifImageDescriptor imageDescriptor, Span indices) { - int dataSize = this.currentStream.ReadByte(); - using (var lzwDecoder = new LzwDecoder(this.configuration.MemoryManager, this.currentStream)) + int dataSize = this.stream.ReadByte(); + using (var lzwDecoder = new LzwDecoder(this.configuration.MemoryManager, this.stream)) { lzwDecoder.DecodePixels(imageDescriptor.Width, imageDescriptor.Height, dataSize, indices); } @@ -526,10 +526,10 @@ namespace SixLabors.ImageSharp.Formats.Gif { this.metaData = new ImageMetaData(); - this.currentStream = stream; + this.stream = stream; // Skip the identifier - this.currentStream.Skip(6); + this.stream.Skip(6); this.ReadLogicalScreenDescriptor(); if (this.logicalScreenDescriptor.GlobalColorTableFlag) From 29d31733d329192689572b0953a0919e4d3a6582 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 11:23:50 -0700 Subject: [PATCH 03/31] Introduce IGifExtension interface --- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 26 +++++- .../Sections/GifGraphicsControlExtension.cs | 81 ++++++++++--------- .../Formats/Gif/Sections/IGifExtension.cs | 22 +++++ 3 files changed, 86 insertions(+), 43 deletions(-) create mode 100644 src/ImageSharp/Formats/Gif/Sections/IGifExtension.cs diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index e93e23f70..d92a0f591 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -254,15 +254,33 @@ namespace SixLabors.ImageSharp.Formats.Gif /// The index of the color in the color palette to make transparent. private void WriteGraphicalControlExtension(ImageFrameMetaData metaData, Stream stream, int transparencyIndex) { - var extension = new GifGraphicsControlExtension( + byte packedValue = GifGraphicsControlExtension.GetPackedValue( disposalMethod: metaData.DisposalMethod, - transparencyFlag: transparencyIndex > -1, + transparencyFlag: transparencyIndex > -1); + + var extension = new GifGraphicsControlExtension( + packed: packedValue, transparencyIndex: unchecked((byte)transparencyIndex), delayTime: (ushort)metaData.FrameDelay); - extension.WriteTo(this.buffer); + this.WriteExtension(extension, stream); + } + + /// + /// Writes the provided extension to the stream. + /// + /// The extension to write to the stream. + /// The stream to write to. + public void WriteExtension(IGifExtension extension, Stream stream) + { + this.buffer[0] = GifConstants.ExtensionIntroducer; + this.buffer[1] = extension.Label; + + int extensionSize = extension.WriteTo(this.buffer.AsSpan(2)); + + this.buffer[extensionSize + 2] = GifConstants.Terminator; - stream.Write(this.buffer, 0, GifGraphicsControlExtension.Size); + stream.Write(this.buffer, 0, 8); } /// diff --git a/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs index bb4c8a59e..230c9cca2 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs @@ -3,6 +3,8 @@ using System; using System.Buffers.Binary; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; namespace SixLabors.ImageSharp.Formats.Gif { @@ -10,34 +12,31 @@ namespace SixLabors.ImageSharp.Formats.Gif /// The Graphic Control Extension contains parameters used when /// processing a graphic rendering block. /// - internal readonly struct GifGraphicsControlExtension + [StructLayout(LayoutKind.Sequential, Pack = 1)] + internal readonly struct GifGraphicsControlExtension : IGifExtension { - public const int Size = 8; + public const int Size = 4; public GifGraphicsControlExtension( - DisposalMethod disposalMethod, - bool transparencyFlag, + byte packed, ushort delayTime, byte transparencyIndex) { - this.DisposalMethod = disposalMethod; - this.TransparencyFlag = transparencyFlag; + this.BlockSize = 4; + this.Packed = packed; this.DelayTime = delayTime; this.TransparencyIndex = transparencyIndex; } /// - /// Gets the disposal method which indicates the way in which the - /// graphic is to be treated after being displayed. + /// Gets the size of the block. /// - public DisposalMethod DisposalMethod { get; } + public int BlockSize { get; } /// - /// Gets a value indicating whether transparency flag is to be set. - /// This indicates whether a transparency index is given in the Transparent Index field. - /// (This field is the least significant bit of the byte.) + /// Gets the packed disposalMethod and transparencyFlag value. /// - public bool TransparencyFlag { get; } + public byte Packed { get; } /// /// Gets the delay time. @@ -54,41 +53,45 @@ namespace SixLabors.ImageSharp.Formats.Gif /// public byte TransparencyIndex { get; } - public byte PackField() - { - PackedField field = default; + /// + /// Gets the disposal method which indicates the way in which the + /// graphic is to be treated after being displayed. + /// + public DisposalMethod DisposalMethod => (DisposalMethod)((this.Packed & 0x1C) >> 2); + + /// + /// Gets a value indicating whether transparency flag is to be set. + /// This indicates whether a transparency index is given in the Transparent Index field. + /// (This field is the least significant bit of the byte.) + /// + public bool TransparencyFlag => (this.Packed & 0x01) == 1; - field.SetBits(3, 3, (int)this.DisposalMethod); // 1-3 : Reserved, 4-6 : Disposal + byte IGifExtension.Label => GifConstants.GraphicControlLabel; - // TODO: Allow this as an option. - field.SetBit(6, false); // 7 : User input - 0 = none - field.SetBit(7, this.TransparencyFlag); // 8: Has transparent. + public int WriteTo(Span buffer) + { + ref GifGraphicsControlExtension dest = ref Unsafe.As(ref MemoryMarshal.GetReference(buffer)); - return field.Byte; + dest = this; + + return 5; } - public void WriteTo(Span buffer) + public static GifGraphicsControlExtension Parse(ReadOnlySpan buffer) { - buffer[0] = GifConstants.ExtensionIntroducer; - buffer[1] = GifConstants.GraphicControlLabel; - buffer[2] = 4; // Block Size - buffer[3] = this.PackField(); // Packed Field - BinaryPrimitives.WriteUInt16LittleEndian(buffer.Slice(4, 2), this.DelayTime); // Delay Time - buffer[6] = this.TransparencyIndex; - buffer[7] = GifConstants.Terminator; + return MemoryMarshal.Cast(buffer)[0]; } - public static GifGraphicsControlExtension Parse(ReadOnlySpan buffer) + public static byte GetPackedValue(DisposalMethod disposalMethod, bool userInputFlag = false, bool transparencyFlag = false) { - // We've already read the Extension Introducer introducer & Graphic Control Label - // Start from the block size (0) - byte packed = buffer[1]; - - return new GifGraphicsControlExtension( - disposalMethod: (DisposalMethod)((packed & 0x1C) >> 2), - delayTime: BinaryPrimitives.ReadUInt16LittleEndian(buffer.Slice(2, 2)), - transparencyIndex: buffer[4], - transparencyFlag: (packed & 0x01) == 1); + PackedField field = default; + + // --------------------------------------- // Reserved | 3 bits + field.SetBits(3, 3, (int)disposalMethod); // Disposal Method | 3 bits + field.SetBit(6, userInputFlag); // User Input Flag | 1 bit + field.SetBit(7, transparencyFlag); // Transparent Color Flag | 1 bit + + return field.Byte; } } } \ No newline at end of file diff --git a/src/ImageSharp/Formats/Gif/Sections/IGifExtension.cs b/src/ImageSharp/Formats/Gif/Sections/IGifExtension.cs new file mode 100644 index 000000000..2fdc233b0 --- /dev/null +++ b/src/ImageSharp/Formats/Gif/Sections/IGifExtension.cs @@ -0,0 +1,22 @@ +using System; + +namespace SixLabors.ImageSharp.Formats.Gif +{ + /// + /// A base interface for GIF extensions. + /// + public interface IGifExtension + { + /// + /// Gets the label identifying the extensions. + /// + byte Label { get; } + + /// + /// Writes the extension data to the buffer. + /// + /// The buffer to write the extension to. + /// The number of bytes written to the buffer. + int WriteTo(Span buffer); + } +} \ No newline at end of file From 7f95ad0a6abeaffd3a2f83a6368318b6d1da8c92 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 11:25:49 -0700 Subject: [PATCH 04/31] Used actual extension size when writing buffer --- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index d92a0f591..105c12585 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -280,7 +280,7 @@ namespace SixLabors.ImageSharp.Formats.Gif this.buffer[extensionSize + 2] = GifConstants.Terminator; - stream.Write(this.buffer, 0, 8); + stream.Write(this.buffer, 0, extensionSize + 3); } /// From 390414c2972a4b4d6e8b8177c368dbdfa396debe Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 11:28:40 -0700 Subject: [PATCH 05/31] Fix block size --- .../Formats/Gif/Sections/GifGraphicsControlExtension.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs index 230c9cca2..53dd055fa 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs @@ -31,7 +31,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// Gets the size of the block. /// - public int BlockSize { get; } + public byte BlockSize { get; } /// /// Gets the packed disposalMethod and transparencyFlag value. From 70cf847bfed8d59c4d5348bd9d4478bb87f951c2 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 11:36:49 -0700 Subject: [PATCH 06/31] Remove incorrect size from GifGraphicsControlExtension --- .../Formats/Gif/Sections/GifGraphicsControlExtension.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs index 53dd055fa..9db73d4e7 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs @@ -15,8 +15,6 @@ namespace SixLabors.ImageSharp.Formats.Gif [StructLayout(LayoutKind.Sequential, Pack = 1)] internal readonly struct GifGraphicsControlExtension : IGifExtension { - public const int Size = 4; - public GifGraphicsControlExtension( byte packed, ushort delayTime, From 6486a457c69d157585384ee03cf0b1caa372529a Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 11:49:37 -0700 Subject: [PATCH 07/31] Increase buffer size and fit the applicaton extension into a single write --- src/ImageSharp/Formats/Gif/GifConstants.cs | 2 +- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 24 ++++++++------------ 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifConstants.cs b/src/ImageSharp/Formats/Gif/GifConstants.cs index ffab45a56..0dbd39b99 100644 --- a/src/ImageSharp/Formats/Gif/GifConstants.cs +++ b/src/ImageSharp/Formats/Gif/GifConstants.cs @@ -54,7 +54,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// The application block size. /// - public const byte ApplicationBlockSize = 0x0b; + public const byte ApplicationBlockSize = 11; /// /// The comment label. diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index 105c12585..cf06cc1e0 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -8,19 +8,15 @@ using System.Linq; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; -using SixLabors.ImageSharp.IO; using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.MetaData; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing.Quantization; -// TODO: This is causing more GC collections than I'm happy with. -// This is likely due to the number of short writes to the stream we are doing. -// We should investigate reducing them since we know the length of the byte array we require for multiple parts. namespace SixLabors.ImageSharp.Formats.Gif { /// - /// Performs the gif encoding operation. + /// Implements the GIF encoding protocol. /// internal sealed class GifEncoderCore { @@ -29,7 +25,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// The temp buffer used to reduce allocations. /// - private readonly byte[] buffer = new byte[16]; + private readonly byte[] buffer = new byte[20]; /// /// Gets the TextEncoding @@ -195,21 +191,21 @@ namespace SixLabors.ImageSharp.Formats.Gif this.buffer[1] = GifConstants.ApplicationExtensionLabel; this.buffer[2] = GifConstants.ApplicationBlockSize; - stream.Write(this.buffer, 0, 3); + // Write NETSCAPE2.0 + GifConstants.ApplicationIdentificationBytes.AsSpan().CopyTo(this.buffer.AsSpan(3, 11)); - stream.Write(GifConstants.ApplicationIdentificationBytes, 0, 11); // NETSCAPE2.0 - - this.buffer[0] = 3; // Application block length - this.buffer[1] = 1; // Data sub-block index (always 1) + // Application Data ---- + this.buffer[14] = 3; // Application block length + this.buffer[15] = 1; // Data sub-block index (always 1) // 0 means loop indefinitely. Count is set as play n + 1 times. repeatCount = (ushort)Math.Max(0, repeatCount - 1); - BinaryPrimitives.WriteUInt16LittleEndian(this.buffer.AsSpan(2, 2), repeatCount); // Repeat count for images. + BinaryPrimitives.WriteUInt16LittleEndian(this.buffer.AsSpan(16, 2), repeatCount); // Repeat count for images. - this.buffer[4] = GifConstants.Terminator; // Terminator + this.buffer[18] = GifConstants.Terminator; // Terminator - stream.Write(this.buffer, 0, 5); + stream.Write(this.buffer, 0, 19); } } From 3acd97cdfe4747eac07da5a4509d7ce523d90208 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 11:57:58 -0700 Subject: [PATCH 08/31] Pass metadata directly to WriteComments to avoid unnessary codegen. --- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index cf06cc1e0..20850cfc2 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -87,7 +87,7 @@ namespace SixLabors.ImageSharp.Formats.Gif this.WriteLogicalScreenDescriptor(image, stream, index); // Write the first frame. - this.WriteComments(image, stream); + this.WriteComments(image.MetaData, stream); // Write additional frames. if (image.Frames.Count > 1) @@ -212,18 +212,16 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// Writes the image comments to the stream. /// - /// The pixel format. - /// The to be encoded. + /// The metadata to be extract the comment data. /// The stream to write to. - private void WriteComments(Image image, Stream stream) - where TPixel : struct, IPixel + private void WriteComments(ImageMetaData metadata, Stream stream) { if (this.ignoreMetadata) { return; } - - ImageProperty property = image.MetaData.Properties.FirstOrDefault(p => p.Name == GifConstants.Comments); + + ImageProperty property = metadata.Properties.FirstOrDefault(p => p.Name == GifConstants.Comments); if (property == null || string.IsNullOrEmpty(property.Value)) { return; From 6f06a07f626d849a183f8be1e141021e8fe57e7b Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 12:03:30 -0700 Subject: [PATCH 09/31] Do not clone ImageProperties We're using a structure that is already copied with two immutable values --- src/ImageSharp/MetaData/ImageMetaData.cs | 3 +-- src/ImageSharp/MetaData/ImageProperty.cs | 15 --------------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/src/ImageSharp/MetaData/ImageMetaData.cs b/src/ImageSharp/MetaData/ImageMetaData.cs index 9613e9b46..df114ce9f 100644 --- a/src/ImageSharp/MetaData/ImageMetaData.cs +++ b/src/ImageSharp/MetaData/ImageMetaData.cs @@ -53,7 +53,7 @@ namespace SixLabors.ImageSharp.MetaData foreach (ImageProperty property in other.Properties) { - this.Properties.Add(new ImageProperty(property)); + this.Properties.Add(property); } this.ExifProfile = other.ExifProfile != null @@ -114,7 +114,6 @@ namespace SixLabors.ImageSharp.MetaData /// /// Gets the list of properties for storing meta information about this image. /// - /// A list of image properties. public IList Properties { get; } = new List(); /// diff --git a/src/ImageSharp/MetaData/ImageProperty.cs b/src/ImageSharp/MetaData/ImageProperty.cs index e4f60e8b3..c67c1f3cf 100644 --- a/src/ImageSharp/MetaData/ImageProperty.cs +++ b/src/ImageSharp/MetaData/ImageProperty.cs @@ -25,21 +25,6 @@ namespace SixLabors.ImageSharp.MetaData this.Value = value; } - /// - /// Initializes a new instance of the struct - /// by making a copy from another property. - /// - /// - /// The other to create this instance from. - /// - internal ImageProperty(ImageProperty other) - { - DebugGuard.NotNull(other, nameof(other)); - - this.Name = other.Name; - this.Value = other.Value; - } - /// /// Gets the name of this indicating which kind of /// information this property stores. From 6d8bc96304f494f981b61466e285a6fe27286bf3 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 12:07:54 -0700 Subject: [PATCH 10/31] Remove EndianBinaryReader & Writer It was factored out over the last few PRs --- src/ImageSharp/IO/EndianBinaryReader.cs | 364 ------------------ src/ImageSharp/IO/EndianBinaryWriter.cs | 303 --------------- .../IO/EndianBinaryReaderWriterTests.cs | 97 ----- 3 files changed, 764 deletions(-) delete mode 100644 src/ImageSharp/IO/EndianBinaryReader.cs delete mode 100644 src/ImageSharp/IO/EndianBinaryWriter.cs delete mode 100644 tests/ImageSharp.Tests/IO/EndianBinaryReaderWriterTests.cs diff --git a/src/ImageSharp/IO/EndianBinaryReader.cs b/src/ImageSharp/IO/EndianBinaryReader.cs deleted file mode 100644 index 6454ff250..000000000 --- a/src/ImageSharp/IO/EndianBinaryReader.cs +++ /dev/null @@ -1,364 +0,0 @@ -// Copyright (c) Six Labors and contributors. -// Licensed under the Apache License, Version 2.0. - -using System; -using System.Buffers.Binary; -using System.IO; -using System.Text; - -namespace SixLabors.ImageSharp.IO -{ - /// - /// Equivalent of , but with either endianness. - /// No data is buffered in the reader; the client may seek within the stream at will. - /// - internal class EndianBinaryReader : IDisposable - { - /// - /// Buffer used for temporary storage before conversion into primitives - /// - private readonly byte[] storageBuffer = new byte[16]; - - /// - /// Whether or not this reader has been disposed yet. - /// - private bool disposed; - - /// - /// The endianness used to read data - /// - private readonly Endianness endianness; - - /// - /// Initializes a new instance of the class. - /// Constructs a new binary reader with the given bit converter, reading - /// to the given stream, using the given encoding. - /// - /// Endianness to use when reading data - /// Stream to read data from - public EndianBinaryReader(Endianness endianness, Stream stream) - { - Guard.NotNull(stream, nameof(stream)); - Guard.IsTrue(stream.CanRead, nameof(stream), "Stream isn't readable"); - - this.BaseStream = stream; - this.endianness = endianness; - } - - /// - /// Gets the underlying stream of the EndianBinaryReader. - /// - public Stream BaseStream { get; } - - /// - /// Closes the reader, including the underlying stream. - /// - public void Close() - { - this.Dispose(); - } - - /// - /// Seeks within the stream. - /// - /// Offset to seek to. - /// Origin of seek operation. - public void Seek(int offset, SeekOrigin origin) - { - this.CheckDisposed(); - this.BaseStream.Seek(offset, origin); - } - - /// - /// Reads a single byte from the stream. - /// - /// The byte read - public byte ReadByte() - { - this.ReadInternal(this.storageBuffer, 1); - return this.storageBuffer[0]; - } - - /// - /// Reads a single signed byte from the stream. - /// - /// The byte read - public sbyte ReadSByte() - { - this.ReadInternal(this.storageBuffer, 1); - return unchecked((sbyte)this.storageBuffer[0]); - } - - /// - /// Reads a boolean from the stream. 1 byte is read. - /// - /// The boolean read - public bool ReadBoolean() - { - this.ReadInternal(this.storageBuffer, 1); - - return this.storageBuffer[0] != 0; - } - - /// - /// Reads a 16-bit signed integer from the stream, using the bit converter - /// for this reader. 2 bytes are read. - /// - /// The 16-bit integer read - public short ReadInt16() - { - this.ReadInternal(this.storageBuffer, 2); - - return (this.endianness == Endianness.BigEndian) - ? BinaryPrimitives.ReadInt16BigEndian(this.storageBuffer) - : BinaryPrimitives.ReadInt16LittleEndian(this.storageBuffer); - } - - /// - /// Reads a 32-bit signed integer from the stream, using the bit converter - /// for this reader. 4 bytes are read. - /// - /// The 32-bit integer read - public int ReadInt32() - { - this.ReadInternal(this.storageBuffer, 4); - - return (this.endianness == Endianness.BigEndian) - ? BinaryPrimitives.ReadInt32BigEndian(this.storageBuffer) - : BinaryPrimitives.ReadInt32LittleEndian(this.storageBuffer); - } - - /// - /// Reads a 64-bit signed integer from the stream, using the bit converter - /// for this reader. 8 bytes are read. - /// - /// The 64-bit integer read - public long ReadInt64() - { - this.ReadInternal(this.storageBuffer, 8); - - return (this.endianness == Endianness.BigEndian) - ? BinaryPrimitives.ReadInt64BigEndian(this.storageBuffer) - : BinaryPrimitives.ReadInt64LittleEndian(this.storageBuffer); - } - - /// - /// Reads a 16-bit unsigned integer from the stream, using the bit converter - /// for this reader. 2 bytes are read. - /// - /// The 16-bit unsigned integer read - public ushort ReadUInt16() - { - this.ReadInternal(this.storageBuffer, 2); - - return (this.endianness == Endianness.BigEndian) - ? BinaryPrimitives.ReadUInt16BigEndian(this.storageBuffer) - : BinaryPrimitives.ReadUInt16LittleEndian(this.storageBuffer); - } - - /// - /// Reads a 32-bit unsigned integer from the stream, using the bit converter - /// for this reader. 4 bytes are read. - /// - /// The 32-bit unsigned integer read - public uint ReadUInt32() - { - this.ReadInternal(this.storageBuffer, 4); - - return (this.endianness == Endianness.BigEndian) - ? BinaryPrimitives.ReadUInt32BigEndian(this.storageBuffer) - : BinaryPrimitives.ReadUInt32LittleEndian(this.storageBuffer); - } - - /// - /// Reads a 64-bit unsigned integer from the stream, using the bit converter - /// for this reader. 8 bytes are read. - /// - /// The 64-bit unsigned integer read - public ulong ReadUInt64() - { - this.ReadInternal(this.storageBuffer, 8); - - return (this.endianness == Endianness.BigEndian) - ? BinaryPrimitives.ReadUInt64BigEndian(this.storageBuffer) - : BinaryPrimitives.ReadUInt64LittleEndian(this.storageBuffer); - } - - /// - /// Reads a single-precision floating-point value from the stream, using the bit converter - /// for this reader. 4 bytes are read. - /// - /// The floating point value read - public unsafe float ReadSingle() - { - int intValue = this.ReadInt32(); - - return *((float*)&intValue); - } - - /// - /// Reads a double-precision floating-point value from the stream, using the bit converter - /// for this reader. 8 bytes are read. - /// - /// The floating point value read - public unsafe double ReadDouble() - { - long value = this.ReadInt64(); - - return *((double*)&value); - } - - /// - /// Reads the specified number of bytes into the given buffer, starting at - /// the given index. - /// - /// The buffer to copy data into - /// The first index to copy data into - /// The number of bytes to read - /// The number of bytes actually read. This will only be less than - /// the requested number of bytes if the end of the stream is reached. - /// - public int Read(byte[] buffer, int index, int count) - { - this.CheckDisposed(); - - Guard.NotNull(this.storageBuffer, nameof(this.storageBuffer)); - Guard.MustBeGreaterThanOrEqualTo(index, 0, nameof(index)); - Guard.MustBeGreaterThanOrEqualTo(count, 0, nameof(count)); - Guard.IsFalse(count + index > buffer.Length, nameof(buffer.Length), "Not enough space in buffer for specified number of bytes starting at specified index."); - - int read = 0; - while (count > 0) - { - int block = this.BaseStream.Read(buffer, index, count); - if (block == 0) - { - return read; - } - - index += block; - read += block; - count -= block; - } - - return read; - } - - /// - /// Reads the specified number of bytes, returning them in a new byte array. - /// If not enough bytes are available before the end of the stream, this - /// method will return what is available. - /// - /// The number of bytes to read - /// The bytes read - public byte[] ReadBytes(int count) - { - this.CheckDisposed(); - Guard.MustBeGreaterThanOrEqualTo(count, 0, nameof(count)); - - byte[] ret = new byte[count]; - int index = 0; - while (index < count) - { - int read = this.BaseStream.Read(ret, index, count - index); - - // Stream has finished half way through. That's fine, return what we've got. - if (read == 0) - { - byte[] copy = new byte[index]; - Buffer.BlockCopy(ret, 0, copy, 0, index); - return copy; - } - - index += read; - } - - return ret; - } - - /// - /// Reads the specified number of bytes, returning them in a new byte array. - /// If not enough bytes are available before the end of the stream, this - /// method will throw an IOException. - /// - /// The number of bytes to read - /// The bytes read - public byte[] ReadBytesOrThrow(int count) - { - byte[] ret = new byte[count]; - this.ReadInternal(ret, count); - return ret; - } - - /// - /// Disposes of the underlying stream. - /// - public void Dispose() - { - if (!this.disposed) - { - this.disposed = true; - ((IDisposable)this.BaseStream).Dispose(); - } - } - - /// - /// Checks whether or not the reader has been disposed, throwing an exception if so. - /// - private void CheckDisposed() - { - if (this.disposed) - { - throw new ObjectDisposedException(nameof(EndianBinaryReader)); - } - } - - /// - /// Reads the given number of bytes from the stream, throwing an exception - /// if they can't all be read. - /// - /// Buffer to read into - /// Number of bytes to read - private void ReadInternal(byte[] data, int size) - { - this.CheckDisposed(); - int index = 0; - while (index < size) - { - int read = this.BaseStream.Read(data, index, size - index); - if (read == 0) - { - throw new EndOfStreamException($"End of stream reached with {size - index} byte{(size - index == 1 ? "s" : string.Empty)} left to read."); - } - - index += read; - } - } - - /// - /// Reads the given number of bytes from the stream if possible, returning - /// the number of bytes actually read, which may be less than requested if - /// (and only if) the end of the stream is reached. - /// - /// Buffer to read into - /// Number of bytes to read - /// Number of bytes actually read - private int TryReadInternal(byte[] data, int size) - { - this.CheckDisposed(); - int index = 0; - while (index < size) - { - int read = this.BaseStream.Read(data, index, size - index); - if (read == 0) - { - return index; - } - - index += read; - } - - return index; - } - } -} \ No newline at end of file diff --git a/src/ImageSharp/IO/EndianBinaryWriter.cs b/src/ImageSharp/IO/EndianBinaryWriter.cs deleted file mode 100644 index 9c42f0b69..000000000 --- a/src/ImageSharp/IO/EndianBinaryWriter.cs +++ /dev/null @@ -1,303 +0,0 @@ -// Copyright (c) Six Labors and contributors. -// Licensed under the Apache License, Version 2.0. - -using System; -using System.Buffers.Binary; -using System.IO; - -namespace SixLabors.ImageSharp.IO -{ - /// - /// Equivalent of , but with either endianness - /// - internal class EndianBinaryWriter : IDisposable - { - /// - /// Buffer used for temporary storage during conversion from primitives - /// - private readonly byte[] buffer = new byte[16]; - - /// - /// The endianness used to write the data - /// - private readonly Endianness endianness; - - /// - /// Whether or not this writer has been disposed yet. - /// - private bool disposed; - - /// - /// Initializes a new instance of the class - /// with the given bit converter, writing to the given stream, using the given encoding. - /// - /// Endianness to use when writing data - /// Stream to write data to - public EndianBinaryWriter(Endianness endianness, Stream stream) - { - Guard.NotNull(stream, nameof(stream)); - Guard.IsTrue(stream.CanWrite, nameof(stream), "Stream isn't writable"); - - this.BaseStream = stream; - this.endianness = endianness; - } - - /// - /// Gets the underlying stream of the EndianBinaryWriter. - /// - public Stream BaseStream { get; } - - /// - /// Closes the writer, including the underlying stream. - /// - public void Close() - { - this.Dispose(); - } - - /// - /// Flushes the underlying stream. - /// - public void Flush() - { - this.CheckDisposed(); - this.BaseStream.Flush(); - } - - /// - /// Seeks within the stream. - /// - /// Offset to seek to. - /// Origin of seek operation. - public void Seek(int offset, SeekOrigin origin) - { - this.CheckDisposed(); - this.BaseStream.Seek(offset, origin); - } - - /// - /// Writes a boolean value to the stream. 1 byte is written. - /// - /// The value to write - public void Write(bool value) - { - this.buffer[0] = value ? (byte)1 : (byte)0; - - this.WriteInternal(this.buffer, 1); - } - - /// - /// Writes a 16-bit signed integer to the stream, using the bit converter - /// for this writer. 2 bytes are written. - /// - /// The value to write - public void Write(short value) - { - if (this.endianness == Endianness.BigEndian) - { - BinaryPrimitives.WriteInt16BigEndian(this.buffer, value); - } - else - { - BinaryPrimitives.WriteInt16LittleEndian(this.buffer, value); - } - - this.WriteInternal(this.buffer, 2); - } - - /// - /// Writes a 32-bit signed integer to the stream, using the bit converter - /// for this writer. 4 bytes are written. - /// - /// The value to write - public void Write(int value) - { - if (this.endianness == Endianness.BigEndian) - { - BinaryPrimitives.WriteInt32BigEndian(this.buffer, value); - } - else - { - BinaryPrimitives.WriteInt32LittleEndian(this.buffer, value); - } - - this.WriteInternal(this.buffer, 4); - } - - /// - /// Writes a 64-bit signed integer to the stream, using the bit converter - /// for this writer. 8 bytes are written. - /// - /// The value to write - public void Write(long value) - { - if (this.endianness == Endianness.BigEndian) - { - BinaryPrimitives.WriteInt64BigEndian(this.buffer, value); - } - else - { - BinaryPrimitives.WriteInt64LittleEndian(this.buffer, value); - } - - this.WriteInternal(this.buffer, 8); - } - - /// - /// Writes a 16-bit unsigned integer to the stream, using the bit converter - /// for this writer. 2 bytes are written. - /// - /// The value to write - public void Write(ushort value) - { - if (this.endianness == Endianness.BigEndian) - { - BinaryPrimitives.WriteUInt16BigEndian(this.buffer, value); - } - else - { - BinaryPrimitives.WriteUInt16LittleEndian(this.buffer, value); - } - - this.WriteInternal(this.buffer, 2); - } - - /// - /// Writes a 32-bit unsigned integer to the stream, using the bit converter - /// for this writer. 4 bytes are written. - /// - /// The value to write - public void Write(uint value) - { - if (this.endianness == Endianness.BigEndian) - { - BinaryPrimitives.WriteUInt32BigEndian(this.buffer, value); - } - else - { - BinaryPrimitives.WriteUInt32LittleEndian(this.buffer, value); - } - - this.WriteInternal(this.buffer, 4); - } - - /// - /// Writes a 64-bit unsigned integer to the stream, using the bit converter - /// for this writer. 8 bytes are written. - /// - /// The value to write - public void Write(ulong value) - { - if (this.endianness == Endianness.BigEndian) - { - BinaryPrimitives.WriteUInt64BigEndian(this.buffer, value); - } - else - { - BinaryPrimitives.WriteUInt64LittleEndian(this.buffer, value); - } - - this.WriteInternal(this.buffer, 8); - } - - /// - /// Writes a single-precision floating-point value to the stream, using the bit converter - /// for this writer. 4 bytes are written. - /// - /// The value to write - public unsafe void Write(float value) - { - this.Write(*((int*)&value)); - } - - /// - /// Writes a double-precision floating-point value to the stream, using the bit converter - /// for this writer. 8 bytes are written. - /// - /// The value to write - public unsafe void Write(double value) - { - this.Write(*((long*)&value)); - } - - /// - /// Writes a signed byte to the stream. - /// - /// The value to write - public void Write(byte value) - { - this.buffer[0] = value; - this.WriteInternal(this.buffer, 1); - } - - /// - /// Writes an unsigned byte to the stream. - /// - /// The value to write - public void Write(sbyte value) - { - this.buffer[0] = unchecked((byte)value); - this.WriteInternal(this.buffer, 1); - } - - /// - /// Writes an array of bytes to the stream. - /// - /// The values to write - /// value is null - public void Write(byte[] value) - { - Guard.NotNull(value, nameof(value)); - this.WriteInternal(value, value.Length); - } - - /// - /// Writes a portion of an array of bytes to the stream. - /// - /// An array containing the bytes to write - /// The index of the first byte to write within the array - /// The number of bytes to write - /// value is null - public void Write(byte[] value, int offset, int count) - { - this.CheckDisposed(); - this.BaseStream.Write(value, offset, count); - } - - /// - /// Disposes of the underlying stream. - /// - public void Dispose() - { - if (!this.disposed) - { - this.Flush(); - this.disposed = true; - ((IDisposable)this.BaseStream).Dispose(); - } - } - - /// - /// Checks whether or not the writer has been disposed, throwing an exception if so. - /// - private void CheckDisposed() - { - if (this.disposed) - { - throw new ObjectDisposedException(nameof(EndianBinaryWriter)); - } - } - - /// - /// Writes the specified number of bytes from the start of the given byte array, - /// after checking whether or not the writer has been disposed. - /// - /// The array of bytes to write from - /// The number of bytes to write - private void WriteInternal(byte[] bytes, int length) - { - this.CheckDisposed(); - this.BaseStream.Write(bytes, 0, length); - } - } -} \ No newline at end of file diff --git a/tests/ImageSharp.Tests/IO/EndianBinaryReaderWriterTests.cs b/tests/ImageSharp.Tests/IO/EndianBinaryReaderWriterTests.cs deleted file mode 100644 index 6e22b1689..000000000 --- a/tests/ImageSharp.Tests/IO/EndianBinaryReaderWriterTests.cs +++ /dev/null @@ -1,97 +0,0 @@ -// Copyright (c) Six Labors and contributors. -// Licensed under the Apache License, Version 2.0. - -using System; -using System.IO; -using SixLabors.ImageSharp.IO; -using Xunit; - -namespace SixLabors.ImageSharp.Tests.IO -{ - public class EndianBinaryReaderWriterTests - { - [Fact] - public void RoundtripSingles() - { - foreach ((Endianness endianness, byte[] bytes) in new[] { - (Endianness.BigEndian, new byte[] { 64, 73, 15, 219 }), - (Endianness.LittleEndian, new byte[] { 219, 15, 73, 64 }) - }) - { - var stream = new MemoryStream(); - - using (var writer = new EndianBinaryWriter(endianness, stream)) - { - writer.Write((float)Math.PI); - - Assert.Equal(bytes, stream.ToArray()); - } - } - } - - [Fact] - public void RoundtripDoubles() - { - foreach ((Endianness endianness, byte[] bytes) in new[] { - (Endianness.BigEndian, new byte[] { 64, 9, 33, 251, 84, 68, 45, 24 }), - (Endianness.LittleEndian, new byte[] { 24, 45, 68, 84, 251, 33, 9, 64 }) - }) - { - var stream = new MemoryStream(); - - using (var writer = new EndianBinaryWriter(endianness, stream)) - { - writer.Write(Math.PI); - - Assert.Equal(bytes, stream.ToArray()); - } - } - } - - /// - /// Ensures that the data written through a binary writer can be read back through the reader - /// - [Fact] - public void RoundtripValues() - { - foreach (Endianness endianness in new[] { Endianness.BigEndian, Endianness.LittleEndian }) - { - var stream = new MemoryStream(); - - var writer = new EndianBinaryWriter(endianness, stream); - - writer.Write(true); // Bool - writer.Write((byte)1); // Byte - writer.Write((short)1); // Int16 - writer.Write(1); // Int32 - writer.Write(1L); // Int64 - writer.Write(1f); // Single - writer.Write(1d); // Double - writer.Write((sbyte)1); // SByte - writer.Write((ushort)1); // UInt16 - writer.Write((uint)1); // UInt32 - writer.Write(1UL); // ULong - - Assert.Equal(43, stream.Length); - - stream.Position = 0; - - var reader = new EndianBinaryReader(endianness, stream); - - Assert.True(reader.ReadBoolean()); // Bool - Assert.Equal((byte)1, reader.ReadByte()); // Byte - Assert.Equal((short)1, reader.ReadInt16()); // Int16 - Assert.Equal(1, reader.ReadInt32()); // Int32 - Assert.Equal(1L, reader.ReadInt64()); // Int64 - Assert.Equal(1f, reader.ReadSingle()); // Single - Assert.Equal(1d, reader.ReadDouble()); // Double - Assert.Equal((sbyte)1, reader.ReadSByte()); // SByte - Assert.Equal((ushort)1, reader.ReadUInt16()); // UInt16 - Assert.Equal((uint)1, reader.ReadUInt32()); // UInt32 - Assert.Equal(1UL, reader.ReadUInt64()); // ULong - - stream.Dispose(); - } - } - } -} \ No newline at end of file From bfc4a685514460121cfd88d15ec7c526cdc2914d Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 12:10:35 -0700 Subject: [PATCH 11/31] Make ImageMetaData(ImageMetaData other) constructor private Ensure we use the Clone() method. This also lets us remove the nullability check. --- src/ImageSharp/MetaData/ImageMetaData.cs | 4 +--- .../MetaData/ImageMetaDataTests.cs | 18 ++++++++---------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/ImageSharp/MetaData/ImageMetaData.cs b/src/ImageSharp/MetaData/ImageMetaData.cs index df114ce9f..01b53a3fd 100644 --- a/src/ImageSharp/MetaData/ImageMetaData.cs +++ b/src/ImageSharp/MetaData/ImageMetaData.cs @@ -43,10 +43,8 @@ namespace SixLabors.ImageSharp.MetaData /// /// The other to create this instance from. /// - internal ImageMetaData(ImageMetaData other) + private ImageMetaData(ImageMetaData other) { - DebugGuard.NotNull(other, nameof(other)); - this.HorizontalResolution = other.HorizontalResolution; this.VerticalResolution = other.VerticalResolution; this.RepeatCount = other.RepeatCount; diff --git a/tests/ImageSharp.Tests/MetaData/ImageMetaDataTests.cs b/tests/ImageSharp.Tests/MetaData/ImageMetaDataTests.cs index 43c53570a..255451e0e 100644 --- a/tests/ImageSharp.Tests/MetaData/ImageMetaDataTests.cs +++ b/tests/ImageSharp.Tests/MetaData/ImageMetaDataTests.cs @@ -1,8 +1,6 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. -using SixLabors.ImageSharp.Formats; -using SixLabors.ImageSharp.Formats.Gif; using SixLabors.ImageSharp.MetaData; using SixLabors.ImageSharp.MetaData.Profiles.Exif; using SixLabors.ImageSharp.PixelFormats; @@ -20,10 +18,10 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void ConstructorImageMetaData() { - ImageMetaData metaData = new ImageMetaData(); + var metaData = new ImageMetaData(); - ExifProfile exifProfile = new ExifProfile(); - ImageProperty imageProperty = new ImageProperty("name", "value"); + var exifProfile = new ExifProfile(); + var imageProperty = new ImageProperty("name", "value"); metaData.ExifProfile = exifProfile; metaData.HorizontalResolution = 4; @@ -31,7 +29,7 @@ namespace SixLabors.ImageSharp.Tests metaData.Properties.Add(imageProperty); metaData.RepeatCount = 1; - ImageMetaData clone = new ImageMetaData(metaData); + ImageMetaData clone = metaData.Clone(); Assert.Equal(exifProfile.ToByteArray(), clone.ExifProfile.ToByteArray()); Assert.Equal(4, clone.HorizontalResolution); @@ -43,7 +41,7 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void HorizontalResolution() { - ImageMetaData metaData = new ImageMetaData(); + var metaData = new ImageMetaData(); Assert.Equal(96, metaData.HorizontalResolution); metaData.HorizontalResolution=0; @@ -59,7 +57,7 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void VerticalResolution() { - ImageMetaData metaData = new ImageMetaData(); + var metaData = new ImageMetaData(); Assert.Equal(96, metaData.VerticalResolution); metaData.VerticalResolution = 0; @@ -75,11 +73,11 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void SyncProfiles() { - ExifProfile exifProfile = new ExifProfile(); + var exifProfile = new ExifProfile(); exifProfile.SetValue(ExifTag.XResolution, new Rational(200)); exifProfile.SetValue(ExifTag.YResolution, new Rational(300)); - Image image = new Image(1, 1); + var image = new Image(1, 1); image.MetaData.ExifProfile = exifProfile; image.MetaData.HorizontalResolution = 400; image.MetaData.VerticalResolution = 500; From ae5b135fadaf91e2d6c337bdaecefb95ab8e07d7 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 12:15:17 -0700 Subject: [PATCH 12/31] Format code --- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 2 +- .../Formats/Gif/Sections/GifGraphicsControlExtension.cs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index 20850cfc2..910e348bb 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -220,7 +220,7 @@ namespace SixLabors.ImageSharp.Formats.Gif { return; } - + ImageProperty property = metadata.Properties.FirstOrDefault(p => p.Name == GifConstants.Comments); if (property == null || string.IsNullOrEmpty(property.Value)) { diff --git a/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs index 9db73d4e7..a040aa900 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. using System; -using System.Buffers.Binary; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; From ee5ec977334b411a93929222d190e0d4f8d31853 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 12:16:42 -0700 Subject: [PATCH 13/31] Remove unused PackField methods --- src/ImageSharp/Formats/Gif/PackedField.cs | 64 +---------------------- 1 file changed, 1 insertion(+), 63 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/PackedField.cs b/src/ImageSharp/Formats/Gif/PackedField.cs index 0d3b1539c..e3c1ef0e7 100644 --- a/src/ImageSharp/Formats/Gif/PackedField.cs +++ b/src/ImageSharp/Formats/Gif/PackedField.cs @@ -11,7 +11,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// Represents a byte of data in a GIF data stream which contains a number /// of data items. /// - internal readonly struct PackedField : IEquatable + internal readonly struct PackedField { /// /// The individual bits representing the packed byte. @@ -48,19 +48,6 @@ namespace SixLabors.ImageSharp.Formats.Gif } } - /// - /// Returns a new with the bits in the packed fields to - /// the corresponding bits from the supplied byte. - /// - /// The value to pack. - /// The - public static PackedField FromInt(byte value) - { - PackedField packed = default; - packed.SetBits(0, 8, value); - return packed; - } - /// /// Sets the specified bit within the packed fields to the supplied /// value. @@ -113,55 +100,6 @@ namespace SixLabors.ImageSharp.Formats.Gif return Bits[index]; } - /// - /// Gets the value of the specified bits within the byte. - /// - /// The zero-based index of the first bit to get. - /// The number of bits to get. - /// - /// The value of the specified bits within the byte. - /// - public int GetBits(int startIndex, int length) - { - DebugGuard.MustBeBetweenOrEqualTo(startIndex, 1, 8, nameof(startIndex)); - DebugCheckLength(startIndex, length); - - int returnValue = 0; - int bitShift = length - 1; - for (int i = startIndex; i < startIndex + length; i++) - { - int bitValue = (Bits[i] ? 1 : 0) << bitShift; - returnValue += bitValue; - bitShift--; - } - - return returnValue; - } - - /// - public override bool Equals(object obj) - { - return obj is PackedField other && this.Equals(other); - } - - /// - public bool Equals(PackedField other) - { - return this.Byte.Equals(other.Byte); - } - - /// - public override string ToString() - { - return $"PackedField [ Byte={this.Byte} ]"; - } - - /// - public override int GetHashCode() - { - return this.Byte.GetHashCode(); - } - [Conditional("DEBUG")] private static void DebugCheckLength(int startIndex, int length) { From 242b61fb40a9dc10ed956cf619164e47bd0d6ddc Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 13:23:37 -0700 Subject: [PATCH 14/31] Pack the bits directly in GifImageDescriptor This avoids the array allocation in PackedField --- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 2 +- .../Gif/Sections/GifImageDescriptor.cs | 38 ++++++++++++++----- .../Gif/Sections/GifImageDescriptorTests.cs | 25 ++++++++++++ 3 files changed, 55 insertions(+), 10 deletions(-) create mode 100644 tests/ImageSharp.Tests/Formats/Gif/Sections/GifImageDescriptorTests.cs diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index 910e348bb..ecc684306 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -290,7 +290,7 @@ namespace SixLabors.ImageSharp.Formats.Gif localColorTableFlag: true, interfaceFlag: false, sortFlag: false, - localColorTableSize: this.bitDepth); // Note: we subtract 1 from the colorTableSize writing + localColorTableSize: (byte)this.bitDepth); // Note: we subtract 1 from the colorTableSize writing var descriptor = new GifImageDescriptor( left: 0, diff --git a/src/ImageSharp/Formats/Gif/Sections/GifImageDescriptor.cs b/src/ImageSharp/Formats/Gif/Sections/GifImageDescriptor.cs index 5af3ed814..c5360729e 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifImageDescriptor.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifImageDescriptor.cs @@ -81,16 +81,36 @@ namespace SixLabors.ImageSharp.Formats.Gif return MemoryMarshal.Cast(buffer)[0]; } - public static byte GetPackedValue(bool localColorTableFlag, bool interfaceFlag, bool sortFlag, int localColorTableSize) + public static byte GetPackedValue(bool localColorTableFlag, bool interfaceFlag, bool sortFlag, byte localColorTableSize) { - var field = default(PackedField); - - field.SetBit(0, localColorTableFlag); // 0: Local color table flag = 1 (LCT used) - field.SetBit(1, interfaceFlag); // 1: Interlace flag 0 - field.SetBit(2, sortFlag); // 2: Sort flag 0 - field.SetBits(5, 3, localColorTableSize - 1); // 3-4: Reserved, 5-7 : LCT size. 2^(N+1) - - return field.Byte; + /* + Local Color Table Flag | 1 Bit + Interlace Flag | 1 Bit + Sort Flag | 1 Bit + Reserved | 2 Bits + Size of Local Color Table | 3 Bits + */ + + byte value = 0; + + if (localColorTableFlag) + { + value |= 1 << 7; + } + + if (interfaceFlag) + { + value |= 1 << 6; + } + + if (sortFlag) + { + value |= 1 << 5; + } + + value |= (byte)(localColorTableSize - 1); + + return value; } } } \ No newline at end of file diff --git a/tests/ImageSharp.Tests/Formats/Gif/Sections/GifImageDescriptorTests.cs b/tests/ImageSharp.Tests/Formats/Gif/Sections/GifImageDescriptorTests.cs new file mode 100644 index 000000000..5eed47b9c --- /dev/null +++ b/tests/ImageSharp.Tests/Formats/Gif/Sections/GifImageDescriptorTests.cs @@ -0,0 +1,25 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +using SixLabors.ImageSharp.Formats.Gif; + +using Xunit; + +namespace SixLabors.ImageSharp.Tests.Formats.Gif +{ + public class GifImageDescriptorTests + { + + [Fact] + public void TestPackedValue() + { + Assert.Equal(128, GifImageDescriptor.GetPackedValue(true, false, false, 1)); // localColorTable + Assert.Equal(64, GifImageDescriptor.GetPackedValue(false, true, false, 1)); // interfaceFlag + Assert.Equal(32, GifImageDescriptor.GetPackedValue(false, false, true, 1)); // sortFlag + Assert.Equal(224, GifImageDescriptor.GetPackedValue(true, true, true, 1)); // all + Assert.Equal(7, GifImageDescriptor.GetPackedValue(false, false, false, 8)); + Assert.Equal(227, GifImageDescriptor.GetPackedValue(true, true, true, 4)); + Assert.Equal(231, GifImageDescriptor.GetPackedValue(true, true, true, 8)); + } + } +} \ No newline at end of file From 029564ac0a52cd746b1dd5504e1750c6687c116c Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 15:02:02 -0700 Subject: [PATCH 15/31] Update test SDK note: v15.7 continues to work on older VS versions --- tests/ImageSharp.Tests/ImageSharp.Tests.csproj | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/ImageSharp.Tests/ImageSharp.Tests.csproj b/tests/ImageSharp.Tests/ImageSharp.Tests.csproj index 765d9b32e..6028180fd 100644 --- a/tests/ImageSharp.Tests/ImageSharp.Tests.csproj +++ b/tests/ImageSharp.Tests/ImageSharp.Tests.csproj @@ -33,22 +33,16 @@ - + - - - - From 512e36dddb6de751144c94e7c2f6a3fc0285877a Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 15:04:08 -0700 Subject: [PATCH 16/31] Update Moq Also removes the duplicate PackageReference --- tests/ImageSharp.Tests/ImageSharp.Tests.csproj | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/ImageSharp.Tests/ImageSharp.Tests.csproj b/tests/ImageSharp.Tests/ImageSharp.Tests.csproj index 6028180fd..d6851be28 100644 --- a/tests/ImageSharp.Tests/ImageSharp.Tests.csproj +++ b/tests/ImageSharp.Tests/ImageSharp.Tests.csproj @@ -32,11 +32,10 @@ - - + From c102c0039e070a692ea0bf63af4f69b954df0d33 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 15:12:29 -0700 Subject: [PATCH 17/31] Remove unused service include --- src/ImageSharp/ImageSharp.csproj | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/ImageSharp/ImageSharp.csproj b/src/ImageSharp/ImageSharp.csproj index 8bb0442a1..63b1f6170 100644 --- a/src/ImageSharp/ImageSharp.csproj +++ b/src/ImageSharp/ImageSharp.csproj @@ -86,9 +86,6 @@ TextTemplatingFileGenerator - - - True From 26ffb7705f336777c272773190743f128eac08ee Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 15:23:37 -0700 Subject: [PATCH 18/31] Rename GifGraphicControlExtension extension to match spec --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 4 ++-- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 4 ++-- ...xtension.cs => GifGraphicControlExtension.cs} | 16 +++++++--------- 3 files changed, 11 insertions(+), 13 deletions(-) rename src/ImageSharp/Formats/Gif/Sections/{GifGraphicsControlExtension.cs => GifGraphicControlExtension.cs} (80%) diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 4d6f010de..1900d0df0 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -57,7 +57,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// The graphics control extension. /// - private GifGraphicsControlExtension graphicsControlExtension; + private GifGraphicControlExtension graphicsControlExtension; /// /// The metadata @@ -238,7 +238,7 @@ namespace SixLabors.ImageSharp.Formats.Gif { this.stream.Read(this.buffer, 0, 6); - this.graphicsControlExtension = GifGraphicsControlExtension.Parse(this.buffer); + this.graphicsControlExtension = GifGraphicControlExtension.Parse(this.buffer); } /// diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index ecc684306..9c0ae9821 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -248,11 +248,11 @@ namespace SixLabors.ImageSharp.Formats.Gif /// The index of the color in the color palette to make transparent. private void WriteGraphicalControlExtension(ImageFrameMetaData metaData, Stream stream, int transparencyIndex) { - byte packedValue = GifGraphicsControlExtension.GetPackedValue( + byte packedValue = GifGraphicControlExtension.GetPackedValue( disposalMethod: metaData.DisposalMethod, transparencyFlag: transparencyIndex > -1); - var extension = new GifGraphicsControlExtension( + var extension = new GifGraphicControlExtension( packed: packedValue, transparencyIndex: unchecked((byte)transparencyIndex), delayTime: (ushort)metaData.FrameDelay); diff --git a/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs similarity index 80% rename from src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs rename to src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs index a040aa900..b7b5b090c 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifGraphicsControlExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs @@ -12,9 +12,9 @@ namespace SixLabors.ImageSharp.Formats.Gif /// processing a graphic rendering block. /// [StructLayout(LayoutKind.Sequential, Pack = 1)] - internal readonly struct GifGraphicsControlExtension : IGifExtension + internal readonly struct GifGraphicControlExtension : IGifExtension { - public GifGraphicsControlExtension( + public GifGraphicControlExtension( byte packed, ushort delayTime, byte transparencyIndex) @@ -36,9 +36,8 @@ namespace SixLabors.ImageSharp.Formats.Gif public byte Packed { get; } /// - /// Gets the delay time. - /// If not 0, this field specifies the number of hundredths (1/100) of a second to - /// wait before continuing with the processing of the Data Stream. + /// Gets the delay time in of hundredths (1/100) of a second + /// to wait before continuing with the processing of the Data Stream. /// The clock starts ticking immediately after the graphic is rendered. /// public ushort DelayTime { get; } @@ -59,7 +58,6 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// Gets a value indicating whether transparency flag is to be set. /// This indicates whether a transparency index is given in the Transparent Index field. - /// (This field is the least significant bit of the byte.) /// public bool TransparencyFlag => (this.Packed & 0x01) == 1; @@ -67,16 +65,16 @@ namespace SixLabors.ImageSharp.Formats.Gif public int WriteTo(Span buffer) { - ref GifGraphicsControlExtension dest = ref Unsafe.As(ref MemoryMarshal.GetReference(buffer)); + ref GifGraphicControlExtension dest = ref Unsafe.As(ref MemoryMarshal.GetReference(buffer)); dest = this; return 5; } - public static GifGraphicsControlExtension Parse(ReadOnlySpan buffer) + public static GifGraphicControlExtension Parse(ReadOnlySpan buffer) { - return MemoryMarshal.Cast(buffer)[0]; + return MemoryMarshal.Cast(buffer)[0]; } public static byte GetPackedValue(DisposalMethod disposalMethod, bool userInputFlag = false, bool transparencyFlag = false) From 09e14e1c929e509d7ffac1c57c46554056dba2aa Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 15:26:54 -0700 Subject: [PATCH 19/31] Pack GifGraphicControlExtension value directly --- .../Sections/GifGraphicControlExtension.cs | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs index b7b5b090c..7ec5f2030 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs @@ -79,14 +79,28 @@ namespace SixLabors.ImageSharp.Formats.Gif public static byte GetPackedValue(DisposalMethod disposalMethod, bool userInputFlag = false, bool transparencyFlag = false) { - PackedField field = default; + /* + Reserved | 3 Bits + Disposal Method | 3 Bits + User Input Flag | 1 Bit + Transparent Color Flag | 1 Bit + */ - // --------------------------------------- // Reserved | 3 bits - field.SetBits(3, 3, (int)disposalMethod); // Disposal Method | 3 bits - field.SetBit(6, userInputFlag); // User Input Flag | 1 bit - field.SetBit(7, transparencyFlag); // Transparent Color Flag | 1 bit + byte value = 0; - return field.Byte; + value |= (byte)((int)disposalMethod << 2); + + if (userInputFlag) + { + value |= 1 << 1; + } + + if (transparencyFlag) + { + value |= 1; + } + + return value; } } } \ No newline at end of file From c9b56f85acc7a5e4ea7a115c2927f63508daeeab Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 15:40:50 -0700 Subject: [PATCH 20/31] Pack GifLogicalScreen value directly --- .../Sections/GifLogicalScreenDescriptor.cs | 28 +++++++++++++++---- .../GifLogicalScreenDescriptorTests.cs | 23 +++++++++++++++ 2 files changed, 45 insertions(+), 6 deletions(-) create mode 100644 tests/ImageSharp.Tests/Formats/Gif/Sections/GifLogicalScreenDescriptorTests.cs diff --git a/src/ImageSharp/Formats/Gif/Sections/GifLogicalScreenDescriptor.cs b/src/ImageSharp/Formats/Gif/Sections/GifLogicalScreenDescriptor.cs index 35056c6e7..1cfec4763 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifLogicalScreenDescriptor.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifLogicalScreenDescriptor.cs @@ -104,14 +104,30 @@ namespace SixLabors.ImageSharp.Formats.Gif public static byte GetPackedValue(bool globalColorTableFlag, int colorResolution, bool sortFlag, int globalColorTableSize) { - PackedField field = default; + /* + Global Color Table Flag | 1 Bit + Color Resolution | 3 Bits + Sort Flag | 1 Bit + Size of Global Color Table | 3 Bits + */ - field.SetBit(0, globalColorTableFlag); // 0 : Global Color Table Flag | 1 bit - field.SetBits(1, 3, globalColorTableSize); // 1-3 : Color Resolution | 3 bits - field.SetBit(4, sortFlag); // 4 : Sort Flag | 1 bits - field.SetBits(5, 3, globalColorTableSize); // 5-7 : Size of Global Color Table | 3 bits + byte value = 0; - return field.Byte; + if (globalColorTableFlag) + { + value |= 1 << 7; + } + + value |= (byte)(colorResolution << 4); + + if (sortFlag) + { + value |= 1 << 3; + } + + value |= (byte)globalColorTableSize; + + return value; } } } \ No newline at end of file diff --git a/tests/ImageSharp.Tests/Formats/Gif/Sections/GifLogicalScreenDescriptorTests.cs b/tests/ImageSharp.Tests/Formats/Gif/Sections/GifLogicalScreenDescriptorTests.cs new file mode 100644 index 000000000..c6458d22f --- /dev/null +++ b/tests/ImageSharp.Tests/Formats/Gif/Sections/GifLogicalScreenDescriptorTests.cs @@ -0,0 +1,23 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +using SixLabors.ImageSharp.Formats.Gif; + +using Xunit; + +namespace SixLabors.ImageSharp.Tests.Formats.Gif +{ + public class GifLogicalScreenDescriptorTests + { + [Fact] + public void TestPackedValue() + { + Assert.Equal(0, GifLogicalScreenDescriptor.GetPackedValue(false, 0, false, 0)); + Assert.Equal(128, GifLogicalScreenDescriptor.GetPackedValue(true, 0, false, 0)); // globalColorTableFlag + Assert.Equal(8, GifLogicalScreenDescriptor.GetPackedValue(false, 0, true, 0)); // sortFlag + Assert.Equal(48, GifLogicalScreenDescriptor.GetPackedValue(false, 3, false, 0)); + Assert.Equal(155, GifLogicalScreenDescriptor.GetPackedValue(true, 1, true, 3)); + Assert.Equal(55, GifLogicalScreenDescriptor.GetPackedValue(false, 3, false, 7)); + } + } +} \ No newline at end of file From d14b31763275ba69483343f128c24a327f1fcce3 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 15:41:50 -0700 Subject: [PATCH 21/31] Remove packed field --- src/ImageSharp/Formats/Gif/PackedField.cs | 115 ---------------------- 1 file changed, 115 deletions(-) delete mode 100644 src/ImageSharp/Formats/Gif/PackedField.cs diff --git a/src/ImageSharp/Formats/Gif/PackedField.cs b/src/ImageSharp/Formats/Gif/PackedField.cs deleted file mode 100644 index e3c1ef0e7..000000000 --- a/src/ImageSharp/Formats/Gif/PackedField.cs +++ /dev/null @@ -1,115 +0,0 @@ -// Copyright (c) Six Labors and contributors. -// Licensed under the Apache License, Version 2.0. - -using System; -using System.Diagnostics; -using System.Runtime.CompilerServices; - -namespace SixLabors.ImageSharp.Formats.Gif -{ - /// - /// Represents a byte of data in a GIF data stream which contains a number - /// of data items. - /// - internal readonly struct PackedField - { - /// - /// The individual bits representing the packed byte. - /// - private static readonly bool[] Bits = new bool[8]; - - /// - /// Gets the byte which represents the data items held in this instance. - /// - public byte Byte - { - [MethodImpl(MethodImplOptions.AggressiveInlining)] - get - { - int returnValue = 0; - int bitShift = 7; - foreach (bool bit in Bits) - { - int bitValue; - if (bit) - { - bitValue = 1 << bitShift; - } - else - { - bitValue = 0; - } - - returnValue |= bitValue; - bitShift--; - } - - return Convert.ToByte(returnValue & 0xFF); - } - } - - /// - /// Sets the specified bit within the packed fields to the supplied - /// value. - /// - /// - /// The zero-based index within the packed fields of the bit to set. - /// - /// - /// The value to set the bit to. - /// - public void SetBit(int index, bool valueToSet) - { - DebugGuard.MustBeBetweenOrEqualTo(index, 0, 7, nameof(index)); - Bits[index] = valueToSet; - } - - /// - /// Sets the specified bits within the packed fields to the supplied - /// value. - /// - /// The zero-based index within the packed fields of the first bit to set. - /// The number of bits to set. - /// The value to set the bits to. - public void SetBits(int startIndex, int length, int valueToSet) - { - DebugGuard.MustBeBetweenOrEqualTo(startIndex, 0, 7, nameof(startIndex)); - DebugCheckLength(startIndex, length); - - int bitShift = length - 1; - for (int i = startIndex; i < startIndex + length; i++) - { - int bitValueIfSet = 1 << bitShift; - int bitValue = valueToSet & bitValueIfSet; - int bitIsSet = bitValue >> bitShift; - Bits[i] = bitIsSet == 1; - bitShift--; - } - } - - /// - /// Gets the value of the specified bit within the byte. - /// - /// The zero-based index of the bit to get. - /// - /// The value of the specified bit within the byte. - /// - public bool GetBit(int index) - { - DebugGuard.MustBeBetweenOrEqualTo(index, 0, 7, nameof(index)); - return Bits[index]; - } - - [Conditional("DEBUG")] - private static void DebugCheckLength(int startIndex, int length) - { - if (length < 1 || startIndex + length > 8) - { - string message = "Length must be greater than zero and the sum of length and start index must be less than 8. " - + $"Supplied length: {length}. Supplied start index: {startIndex}"; - - throw new ArgumentOutOfRangeException(nameof(length), message); - } - } - } -} \ No newline at end of file From d56c8dfaba175e078cf34548cdf145cfb28f3791 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 15:42:07 -0700 Subject: [PATCH 22/31] Update namespace of gif tests --- tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs | 2 +- tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index 82d281d85..ceb60ae5c 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -10,7 +10,7 @@ using System.IO; using SixLabors.ImageSharp.Advanced; // ReSharper disable InconsistentNaming -namespace SixLabors.ImageSharp.Tests +namespace SixLabors.ImageSharp.Tests.Formats.Gif { using System.Collections.Generic; diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs index 1e0cd948b..918d39021 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs @@ -10,7 +10,7 @@ using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; using Xunit; // ReSharper disable InconsistentNaming -namespace SixLabors.ImageSharp.Tests +namespace SixLabors.ImageSharp.Tests.Formats.Gif { public class GifEncoderTests { From 7e446064dccfcb2dd2f22daf2d53877ee6aaad27 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 15:42:31 -0700 Subject: [PATCH 23/31] Add GifGraphicControlExtensionTests --- .../GifGraphicControlExtensionTests.cs | 21 +++++++++++++++++++ .../Gif/Sections/GifImageDescriptorTests.cs | 1 - 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 tests/ImageSharp.Tests/Formats/Gif/Sections/GifGraphicControlExtensionTests.cs diff --git a/tests/ImageSharp.Tests/Formats/Gif/Sections/GifGraphicControlExtensionTests.cs b/tests/ImageSharp.Tests/Formats/Gif/Sections/GifGraphicControlExtensionTests.cs new file mode 100644 index 000000000..2790b1a57 --- /dev/null +++ b/tests/ImageSharp.Tests/Formats/Gif/Sections/GifGraphicControlExtensionTests.cs @@ -0,0 +1,21 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +using SixLabors.ImageSharp.Formats.Gif; + +using Xunit; + +namespace SixLabors.ImageSharp.Tests.Formats.Gif +{ + public class GifGraphicControlExtensionTests + { + [Fact] + public void TestPackedValue() + { + Assert.Equal(0, GifGraphicControlExtension.GetPackedValue(DisposalMethod.Unspecified, false, false)); + Assert.Equal(11, GifGraphicControlExtension.GetPackedValue(DisposalMethod.RestoreToBackground, true, true)); + Assert.Equal(4, GifGraphicControlExtension.GetPackedValue(DisposalMethod.NotDispose, false, false)); + Assert.Equal(14, GifGraphicControlExtension.GetPackedValue(DisposalMethod.RestoreToPrevious, true, false)); + } + } +} \ No newline at end of file diff --git a/tests/ImageSharp.Tests/Formats/Gif/Sections/GifImageDescriptorTests.cs b/tests/ImageSharp.Tests/Formats/Gif/Sections/GifImageDescriptorTests.cs index 5eed47b9c..4ef4c12d9 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/Sections/GifImageDescriptorTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/Sections/GifImageDescriptorTests.cs @@ -9,7 +9,6 @@ namespace SixLabors.ImageSharp.Tests.Formats.Gif { public class GifImageDescriptorTests { - [Fact] public void TestPackedValue() { From 370bb68096d92d3ca959a17136e7f7981af50d3c Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 15:57:59 -0700 Subject: [PATCH 24/31] Optimize LzwEncoder --- src/ImageSharp/Formats/Gif/LzwEncoder.cs | 33 +++++++++++------------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/LzwEncoder.cs b/src/ImageSharp/Formats/Gif/LzwEncoder.cs index 60bc56dc5..32ba6015b 100644 --- a/src/ImageSharp/Formats/Gif/LzwEncoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwEncoder.cs @@ -59,7 +59,7 @@ namespace SixLabors.ImageSharp.Formats.Gif }; /// - /// The working pixel array + /// The working pixel array. /// private readonly byte[] pixelArray; @@ -99,7 +99,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// The current pixel /// - private int currentPixel; + private int position; /// /// Number of bits/code @@ -212,7 +212,7 @@ namespace SixLabors.ImageSharp.Formats.Gif // Write "initial code size" byte stream.WriteByte((byte)this.initialCodeSize); - this.currentPixel = 0; + this.position = 0; // Compress and write the pixel data this.Compress(this.initialCodeSize + 1, stream); @@ -221,13 +221,6 @@ namespace SixLabors.ImageSharp.Formats.Gif stream.WriteByte(GifConstants.Terminator); } - /// - public void Dispose() - { - // Do not change this code. Put cleanup code in Dispose(bool disposing) above. - this.Dispose(true); - } - /// /// Gets the maximum code value /// @@ -326,8 +319,10 @@ namespace SixLabors.ImageSharp.Formats.Gif ref int hashTableRef = ref MemoryMarshal.GetReference(this.hashTable.Span); ref int codeTableRef = ref MemoryMarshal.GetReference(this.codeTable.Span); - while ((c = this.NextPixel()) != Eof) + while (this.position < this.pixelArray.Length) { + c = this.NextPixel(); + fcode = (c << this.maxbits) + ent; int i = (c << hshift) ^ ent /* = 0 */; @@ -404,15 +399,10 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// The /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] private int NextPixel() { - if (this.currentPixel == this.pixelArray.Length) - { - return Eof; - } - - this.currentPixel++; - return this.pixelArray[this.currentPixel - 1] & 0xff; + return this.pixelArray[this.position++] & 0xff; } /// @@ -478,6 +468,13 @@ namespace SixLabors.ImageSharp.Formats.Gif } } + /// + public void Dispose() + { + // Do not change this code. Put cleanup code in Dispose(bool disposing) above. + this.Dispose(true); + } + /// /// Disposes the object and frees resources for the Garbage Collector. /// From 0b4e614c5420400c023138471d7da60b5523d737 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 15:59:58 -0700 Subject: [PATCH 25/31] Remove the unused finalizer pattern from LzwEncoder The pattern is not relevant without a finalizer. --- src/ImageSharp/Formats/Gif/LzwEncoder.cs | 37 ++---------------------- 1 file changed, 2 insertions(+), 35 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/LzwEncoder.cs b/src/ImageSharp/Formats/Gif/LzwEncoder.cs index 32ba6015b..4f78daf9d 100644 --- a/src/ImageSharp/Formats/Gif/LzwEncoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwEncoder.cs @@ -83,19 +83,6 @@ namespace SixLabors.ImageSharp.Formats.Gif /// private readonly byte[] accumulators = new byte[256]; - /// - /// A value indicating whether this instance of the given entity has been disposed. - /// - /// if this instance has been disposed; otherwise, . - /// - /// If the entity is disposed, it must not be disposed a second - /// time. The isDisposed field is set the first time the entity - /// is disposed. If the isDisposed field is true, then the Dispose() - /// method will not dispose again. This help not to prolong the entity's - /// life in the Garbage Collector. - /// - private bool isDisposed; - /// /// The current pixel /// @@ -471,28 +458,8 @@ namespace SixLabors.ImageSharp.Formats.Gif /// public void Dispose() { - // Do not change this code. Put cleanup code in Dispose(bool disposing) above. - this.Dispose(true); - } - - /// - /// Disposes the object and frees resources for the Garbage Collector. - /// - /// If true, the object gets disposed. - private void Dispose(bool disposing) - { - if (this.isDisposed) - { - return; - } - - if (disposing) - { - this.hashTable?.Dispose(); - this.codeTable?.Dispose(); - } - - this.isDisposed = true; + this.hashTable?.Dispose(); + this.codeTable?.Dispose(); } } } \ No newline at end of file From 0dc94609beff8e19bcdd7000ad5f48936ca0808b Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 16:07:10 -0700 Subject: [PATCH 26/31] Cleanup LzwEncoder --- src/ImageSharp/Formats/Gif/LzwEncoder.cs | 31 ++++++++++-------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/LzwEncoder.cs b/src/ImageSharp/Formats/Gif/LzwEncoder.cs index 4f78daf9d..9adf48843 100644 --- a/src/ImageSharp/Formats/Gif/LzwEncoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwEncoder.cs @@ -34,11 +34,6 @@ namespace SixLabors.ImageSharp.Formats.Gif /// internal sealed class LzwEncoder : IDisposable { - /// - /// The end-of-file marker - /// - private const int Eof = -1; - /// /// The maximum number of bits. /// @@ -84,7 +79,7 @@ namespace SixLabors.ImageSharp.Formats.Gif private readonly byte[] accumulators = new byte[256]; /// - /// The current pixel + /// The current position within the pixelArray. /// private int position; @@ -96,12 +91,12 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// User settable max # bits/code /// - private int maxbits = Bits; + private int maxBits = Bits; /// /// maximum code, given bitCount /// - private int maxcode; + private int maxCode; /// /// should NEVER generate this code @@ -209,7 +204,7 @@ namespace SixLabors.ImageSharp.Formats.Gif } /// - /// Gets the maximum code value + /// Gets the maximum code value. /// /// The number of bits /// See @@ -237,7 +232,7 @@ namespace SixLabors.ImageSharp.Formats.Gif } /// - /// Table clear for block compress + /// Table clear for block compress. /// /// The output stream. [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -272,13 +267,13 @@ namespace SixLabors.ImageSharp.Formats.Gif int hsizeReg; int hshift; - // Set up the globals: globalInitialBits - initial number of bits + // Set up the globals: globalInitialBits - initial number of bits this.globalInitialBits = intialBits; // Set up the necessary values this.clearFlag = false; this.bitCount = this.globalInitialBits; - this.maxcode = GetMaxcode(this.bitCount); + this.maxCode = GetMaxcode(this.bitCount); this.clearCode = 1 << (intialBits - 1); this.eofCode = this.clearCode + 1; @@ -310,7 +305,7 @@ namespace SixLabors.ImageSharp.Formats.Gif { c = this.NextPixel(); - fcode = (c << this.maxbits) + ent; + fcode = (c << this.maxBits) + ent; int i = (c << hshift) ^ ent /* = 0 */; if (Unsafe.Add(ref hashTableRef, i) == fcode) @@ -369,7 +364,7 @@ namespace SixLabors.ImageSharp.Formats.Gif } /// - /// Flush the packet to disk, and reset the accumulator. + /// Flush the packet to disk and reset the accumulator. /// /// The output stream. [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -381,7 +376,7 @@ namespace SixLabors.ImageSharp.Formats.Gif } /// - /// Return the next pixel from the image + /// Reads the next pixel from the image. /// /// /// The @@ -422,17 +417,17 @@ namespace SixLabors.ImageSharp.Formats.Gif // If the next entry is going to be too big for the code size, // then increase it, if possible. - if (this.freeEntry > this.maxcode || this.clearFlag) + if (this.freeEntry > this.maxCode || this.clearFlag) { if (this.clearFlag) { - this.maxcode = GetMaxcode(this.bitCount = this.globalInitialBits); + this.maxCode = GetMaxcode(this.bitCount = this.globalInitialBits); this.clearFlag = false; } else { ++this.bitCount; - this.maxcode = this.bitCount == this.maxbits + this.maxCode = this.bitCount == this.maxBits ? this.maxmaxcode : GetMaxcode(this.bitCount); } From 07d3b9dcecfc4655361b0a3d1f3917c7e54326b2 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 16:10:50 -0700 Subject: [PATCH 27/31] Remove unused support for configuring the maximium number of bits in the LzwEncoder. --- src/ImageSharp/Formats/Gif/LzwEncoder.cs | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/LzwEncoder.cs b/src/ImageSharp/Formats/Gif/LzwEncoder.cs index 9adf48843..6c3ede379 100644 --- a/src/ImageSharp/Formats/Gif/LzwEncoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwEncoder.cs @@ -34,11 +34,6 @@ namespace SixLabors.ImageSharp.Formats.Gif /// internal sealed class LzwEncoder : IDisposable { - /// - /// The maximum number of bits. - /// - private const int Bits = 12; - /// /// 80% occupancy /// @@ -53,6 +48,11 @@ namespace SixLabors.ImageSharp.Formats.Gif 0x01FF, 0x03FF, 0x07FF, 0x0FFF, 0x1FFF, 0x3FFF, 0x7FFF, 0xFFFF }; + /// + /// The maximium number of bits/code. + /// + private const int MaxBits = 12; + /// /// The working pixel array. /// @@ -88,11 +88,6 @@ namespace SixLabors.ImageSharp.Formats.Gif /// private int bitCount; - /// - /// User settable max # bits/code - /// - private int maxBits = Bits; - /// /// maximum code, given bitCount /// @@ -101,7 +96,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// should NEVER generate this code /// - private int maxmaxcode = 1 << Bits; + private int maxmaxcode = 1 << MaxBits; /// /// For dynamic table sizing @@ -305,7 +300,7 @@ namespace SixLabors.ImageSharp.Formats.Gif { c = this.NextPixel(); - fcode = (c << this.maxBits) + ent; + fcode = (c << MaxBits) + ent; int i = (c << hshift) ^ ent /* = 0 */; if (Unsafe.Add(ref hashTableRef, i) == fcode) @@ -427,7 +422,7 @@ namespace SixLabors.ImageSharp.Formats.Gif else { ++this.bitCount; - this.maxCode = this.bitCount == this.maxBits + this.maxCode = this.bitCount == MaxBits ? this.maxmaxcode : GetMaxcode(this.bitCount); } From cb44bbb633454182c7001ab39a6e6056d12c79f9 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 16:14:06 -0700 Subject: [PATCH 28/31] Make MaxMaxCode const --- src/ImageSharp/Formats/Gif/LzwEncoder.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/LzwEncoder.cs b/src/ImageSharp/Formats/Gif/LzwEncoder.cs index 6c3ede379..1dc7e99e8 100644 --- a/src/ImageSharp/Formats/Gif/LzwEncoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwEncoder.cs @@ -53,6 +53,11 @@ namespace SixLabors.ImageSharp.Formats.Gif /// private const int MaxBits = 12; + /// + /// Should NEVER generate this code. + /// + private const int MaxMaxCode = 1 << MaxBits; + /// /// The working pixel array. /// @@ -93,11 +98,6 @@ namespace SixLabors.ImageSharp.Formats.Gif /// private int maxCode; - /// - /// should NEVER generate this code - /// - private int maxmaxcode = 1 << MaxBits; - /// /// For dynamic table sizing /// @@ -341,7 +341,7 @@ namespace SixLabors.ImageSharp.Formats.Gif this.Output(ent, stream); ent = c; - if (this.freeEntry < this.maxmaxcode) + if (this.freeEntry < MaxMaxCode) { Unsafe.Add(ref codeTableRef, i) = this.freeEntry++; // code -> hashtable Unsafe.Add(ref hashTableRef, i) = fcode; @@ -423,7 +423,7 @@ namespace SixLabors.ImageSharp.Formats.Gif { ++this.bitCount; this.maxCode = this.bitCount == MaxBits - ? this.maxmaxcode + ? MaxMaxCode : GetMaxcode(this.bitCount); } } From 0f6943656416c340fb74db1688d21dc8cad0e42b Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 16:18:03 -0700 Subject: [PATCH 29/31] Cleanup LzwDecoder --- src/ImageSharp/Formats/Gif/LzwDecoder.cs | 43 +++--------------------- 1 file changed, 5 insertions(+), 38 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index 37daa6de5..9f9e070e2 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -44,19 +44,6 @@ namespace SixLabors.ImageSharp.Formats.Gif /// private readonly IBuffer pixelStack; - /// - /// A value indicating whether this instance of the given entity has been disposed. - /// - /// if this instance has been disposed; otherwise, . - /// - /// If the entity is disposed, it must not be disposed a second - /// time. The isDisposed field is set the first time the entity - /// is disposed. If the isDisposed field is true, then the Dispose() - /// method will not dispose again. This help not to prolong the entity's - /// life in the Garbage Collector. - /// - private bool isDisposed; - /// /// Initializes a new instance of the class /// and sets the stream, where the compressed data should be read from. @@ -225,13 +212,6 @@ namespace SixLabors.ImageSharp.Formats.Gif } } - /// - public void Dispose() - { - // Do not change this code. Put cleanup code in Dispose(bool disposing) above. - this.Dispose(true); - } - /// /// Reads the next data block from the stream. A data block begins with a byte, /// which defines the size of the block, followed by the block itself. @@ -253,25 +233,12 @@ namespace SixLabors.ImageSharp.Formats.Gif return count != bufferSize ? 0 : bufferSize; } - /// - /// Disposes the object and frees resources for the Garbage Collector. - /// - /// If true, the object gets disposed. - private void Dispose(bool disposing) + /// + public void Dispose() { - if (this.isDisposed) - { - return; - } - - if (disposing) - { - this.prefix?.Dispose(); - this.suffix?.Dispose(); - this.pixelStack?.Dispose(); - } - - this.isDisposed = true; + this.prefix.Dispose(); + this.suffix.Dispose(); + this.pixelStack.Dispose(); } } } \ No newline at end of file From 49d41fff88c313e6b6ff5696592fc0efbfbd80ba Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 16:39:24 -0700 Subject: [PATCH 30/31] Add TryGetProperty to ImageMetadata and remove Linq call from GifEncoder --- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 18 +++++++-------- .../Formats/Gif/IGifEncoderOptions.cs | 2 +- src/ImageSharp/MetaData/ImageMetaData.cs | 23 +++++++++++++++++++ 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index 9c0ae9821..14bfa6fd0 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -4,7 +4,6 @@ using System; using System.Buffers.Binary; using System.IO; -using System.Linq; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; @@ -23,7 +22,7 @@ namespace SixLabors.ImageSharp.Formats.Gif private readonly MemoryManager memoryManager; /// - /// The temp buffer used to reduce allocations. + /// A reusable buffer used to reduce allocations. /// private readonly byte[] buffer = new byte[20]; @@ -129,7 +128,8 @@ namespace SixLabors.ImageSharp.Formats.Gif { // Transparent pixels are much more likely to be found at the end of a palette int index = -1; - var trans = default(Rgba32); + Rgba32 trans = default; + ref TPixel paletteRef = ref MemoryMarshal.GetReference(quantized.Palette.AsSpan()); for (int i = quantized.Palette.Length - 1; i >= 0; i--) { @@ -221,8 +221,7 @@ namespace SixLabors.ImageSharp.Formats.Gif return; } - ImageProperty property = metadata.Properties.FirstOrDefault(p => p.Name == GifConstants.Comments); - if (property == null || string.IsNullOrEmpty(property.Value)) + if (!metadata.TryGetProperty(GifConstants.Comments, out ImageProperty property) || string.IsNullOrEmpty(property.Value)) { return; } @@ -313,17 +312,15 @@ namespace SixLabors.ImageSharp.Formats.Gif private void WriteColorTable(QuantizedFrame image, Stream stream) where TPixel : struct, IPixel { - // Grab the palette and write it to the stream. int pixelCount = image.Palette.Length; - // Get max colors for bit depth. - int colorTableLength = (int)Math.Pow(2, this.bitDepth) * 3; - var rgb = default(Rgb24); + int colorTableLength = (int)Math.Pow(2, this.bitDepth) * 3; // The maximium number of colors for the bit depth + Rgb24 rgb = default; + using (IManagedByteBuffer colorTable = this.memoryManager.AllocateManagedByteBuffer(colorTableLength)) { ref TPixel paletteRef = ref MemoryMarshal.GetReference(image.Palette.AsSpan()); ref Rgb24 rgb24Ref = ref Unsafe.As(ref MemoryMarshal.GetReference(colorTable.Span)); - for (int i = 0; i < pixelCount; i++) { ref TPixel entry = ref Unsafe.Add(ref paletteRef, i); @@ -331,6 +328,7 @@ namespace SixLabors.ImageSharp.Formats.Gif Unsafe.Add(ref rgb24Ref, i) = rgb; } + // Write the palette to the stream stream.Write(colorTable.Array, 0, colorTableLength); } } diff --git a/src/ImageSharp/Formats/Gif/IGifEncoderOptions.cs b/src/ImageSharp/Formats/Gif/IGifEncoderOptions.cs index 1f1875789..f7bc5f4ed 100644 --- a/src/ImageSharp/Formats/Gif/IGifEncoderOptions.cs +++ b/src/ImageSharp/Formats/Gif/IGifEncoderOptions.cs @@ -7,7 +7,7 @@ using SixLabors.ImageSharp.Processing.Quantization; namespace SixLabors.ImageSharp.Formats.Gif { /// - /// The configuration options used for encoding gifs + /// The configuration options used for encoding gifs. /// internal interface IGifEncoderOptions { diff --git a/src/ImageSharp/MetaData/ImageMetaData.cs b/src/ImageSharp/MetaData/ImageMetaData.cs index 01b53a3fd..af3cc5f5f 100644 --- a/src/ImageSharp/MetaData/ImageMetaData.cs +++ b/src/ImageSharp/MetaData/ImageMetaData.cs @@ -120,6 +120,29 @@ namespace SixLabors.ImageSharp.MetaData /// public ushort RepeatCount { get; set; } + /// + /// Looks up a property with the provided name. + /// + /// The name of the property to lookup. + /// The property, if found, with the provided name. + /// Whether the property was found. + internal bool TryGetProperty(string name, out ImageProperty result) + { + foreach (ImageProperty property in this.Properties) + { + if (property.Name == name) + { + result = property; + + return true; + } + } + + result = default; + + return false; + } + /// /// Clones this into a new instance /// From 82765ced95ef005ac1e9ab7d9f0bdd0a879a8232 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 18 Apr 2018 16:50:54 -0700 Subject: [PATCH 31/31] Tidy up comments --- src/ImageSharp/Formats/Gif/DisposalMethod.cs | 13 ++++++++----- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 8 ++++---- .../Formats/Gif/GifImageFormatDetector.cs | 8 +------- src/ImageSharp/Formats/Gif/IGifDecoderOptions.cs | 2 +- src/ImageSharp/Formats/Gif/IGifEncoderOptions.cs | 4 ++-- src/ImageSharp/Formats/Gif/ImageExtensions.cs | 6 ++---- 6 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/DisposalMethod.cs b/src/ImageSharp/Formats/Gif/DisposalMethod.cs index f553c204b..5371fc0fa 100644 --- a/src/ImageSharp/Formats/Gif/DisposalMethod.cs +++ b/src/ImageSharp/Formats/Gif/DisposalMethod.cs @@ -11,23 +11,26 @@ namespace SixLabors.ImageSharp.Formats.Gif public enum DisposalMethod { /// - /// No disposal specified. The decoder is not required to take any action. + /// No disposal specified. + /// The decoder is not required to take any action. /// Unspecified = 0, /// - /// Do not dispose. The graphic is to be left in place. + /// Do not dispose. + /// The graphic is to be left in place. /// NotDispose = 1, /// - /// Restore to background color. The area used by the graphic must be restored to - /// the background color. + /// Restore to background color. + /// The area used by the graphic must be restored to the background color. /// RestoreToBackground = 2, /// - /// Restore to previous. The decoder is required to restore the area overwritten by the + /// Restore to previous. + /// The decoder is required to restore the area overwritten by the /// graphic with what was there prior to rendering the graphic. /// RestoreToPrevious = 3 diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index 14bfa6fd0..747867c80 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -27,22 +27,22 @@ namespace SixLabors.ImageSharp.Formats.Gif private readonly byte[] buffer = new byte[20]; /// - /// Gets the TextEncoding + /// Gets the text encoding used to write comments. /// private readonly Encoding textEncoding; /// - /// Gets or sets the quantizer for reducing the color count. + /// Gets or sets the quantizer used to generate the color palette. /// private readonly IQuantizer quantizer; /// - /// Gets or sets a value indicating whether the metadata should be ignored when the image is being decoded. + /// A flag indicating whether to ingore the metadata when writing the image. /// private readonly bool ignoreMetadata; /// - /// The number of bits requires to store the image palette. + /// The number of bits requires to store the color palette. /// private int bitDepth; diff --git a/src/ImageSharp/Formats/Gif/GifImageFormatDetector.cs b/src/ImageSharp/Formats/Gif/GifImageFormatDetector.cs index 36346f606..bfbd334b0 100644 --- a/src/ImageSharp/Formats/Gif/GifImageFormatDetector.cs +++ b/src/ImageSharp/Formats/Gif/GifImageFormatDetector.cs @@ -16,17 +16,11 @@ namespace SixLabors.ImageSharp.Formats.Gif /// public IImageFormat DetectFormat(ReadOnlySpan header) { - if (this.IsSupportedFileFormat(header)) - { - return ImageFormats.Gif; - } - - return null; + return this.IsSupportedFileFormat(header) ? ImageFormats.Gif : null; } private bool IsSupportedFileFormat(ReadOnlySpan header) { - // TODO: This should be in constants return header.Length >= this.HeaderSize && header[0] == 0x47 && // G header[1] == 0x49 && // I diff --git a/src/ImageSharp/Formats/Gif/IGifDecoderOptions.cs b/src/ImageSharp/Formats/Gif/IGifDecoderOptions.cs index a2288f30a..e99f09add 100644 --- a/src/ImageSharp/Formats/Gif/IGifDecoderOptions.cs +++ b/src/ImageSharp/Formats/Gif/IGifDecoderOptions.cs @@ -16,7 +16,7 @@ namespace SixLabors.ImageSharp.Formats.Gif bool IgnoreMetadata { get; } /// - /// Gets the encoding that should be used when reading comments. + /// Gets the text encoding that should be used when reading comments. /// Encoding TextEncoding { get; } diff --git a/src/ImageSharp/Formats/Gif/IGifEncoderOptions.cs b/src/ImageSharp/Formats/Gif/IGifEncoderOptions.cs index f7bc5f4ed..44dd19db6 100644 --- a/src/ImageSharp/Formats/Gif/IGifEncoderOptions.cs +++ b/src/ImageSharp/Formats/Gif/IGifEncoderOptions.cs @@ -17,12 +17,12 @@ namespace SixLabors.ImageSharp.Formats.Gif bool IgnoreMetadata { get; } /// - /// Gets the encoding that should be used when writing comments. + /// Gets the text encoding used to write comments. /// Encoding TextEncoding { get; } /// - /// Gets the quantizer for reducing the color count. + /// Gets the quantizer used to generate the color palette. /// IQuantizer Quantizer { get; } } diff --git a/src/ImageSharp/Formats/Gif/ImageExtensions.cs b/src/ImageSharp/Formats/Gif/ImageExtensions.cs index 78acadb4b..1c41285a9 100644 --- a/src/ImageSharp/Formats/Gif/ImageExtensions.cs +++ b/src/ImageSharp/Formats/Gif/ImageExtensions.cs @@ -1,10 +1,8 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. -using System; using System.IO; using SixLabors.ImageSharp.Advanced; -using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Gif; using SixLabors.ImageSharp.PixelFormats; @@ -16,7 +14,7 @@ namespace SixLabors.ImageSharp public static partial class ImageExtensions { /// - /// Saves the image to the given stream with the gif format. + /// Saves the image to the given stream in the gif format. /// /// The pixel format. /// The image this method extends. @@ -27,7 +25,7 @@ namespace SixLabors.ImageSharp => source.SaveAsGif(stream, null); /// - /// Saves the image to the given stream with the gif format. + /// Saves the image to the given stream in the gif format. /// /// The pixel format. /// The image this method extends.