diff --git a/src/ImageSharp/Common/Helpers/DebugGuard.cs b/src/ImageSharp/Common/Helpers/DebugGuard.cs index dc3cff7a2..e64075db7 100644 --- a/src/ImageSharp/Common/Helpers/DebugGuard.cs +++ b/src/ImageSharp/Common/Helpers/DebugGuard.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics; +// TODO: These should just call the guard equivalents namespace SixLabors.ImageSharp { /// @@ -114,6 +115,28 @@ namespace SixLabors.ImageSharp } } + /// + /// Verifies that the specified value is greater than or equal to a minimum value and less than + /// or equal to a maximum value and throws an exception if it is not. + /// + /// The target value, which should be validated. + /// The minimum value. + /// The maximum value. + /// The name of the parameter that is to be checked. + /// The type of the value. + /// + /// is less than the minimum value of greater than the maximum value. + /// + [Conditional("DEBUG")] + public static void MustBeBetweenOrEqualTo(TValue value, TValue min, TValue max, string parameterName) + where TValue : IComparable + { + if (value.CompareTo(min) < 0 || value.CompareTo(max) > 0) + { + throw new ArgumentOutOfRangeException(parameterName, $"Value {value} must be greater than or equal to {min} and less than or equal to {max}."); + } + } + /// /// Verifies, that the method parameter with specified target value is true /// and throws an exception if it is found to be so. diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 48c8ceb8c..118ec2954 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -5,6 +5,7 @@ using System; using System.Buffers; using System.IO; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Text; using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.Memory; @@ -410,13 +411,12 @@ namespace SixLabors.ImageSharp.Formats.Gif private void ReadFrameColors(ref Image image, ref ImageFrame previousFrame, Span indices, Span colorTable, GifImageDescriptor descriptor) where TPixel : struct, IPixel { + ref byte indicesRef = ref MemoryMarshal.GetReference(indices); int imageWidth = this.logicalScreenDescriptor.Width; int imageHeight = this.logicalScreenDescriptor.Height; ImageFrame prevFrame = null; - ImageFrame currentFrame = null; - ImageFrame imageFrame; if (previousFrame == null) @@ -479,7 +479,6 @@ namespace SixLabors.ImageSharp.Formats.Gif } writeY = interlaceY + descriptor.Top; - interlaceY += interlaceIncrement; } else @@ -487,14 +486,13 @@ namespace SixLabors.ImageSharp.Formats.Gif writeY = y; } - Span rowSpan = imageFrame.GetPixelRowSpan(writeY); - + ref TPixel rowRef = ref MemoryMarshal.GetReference(imageFrame.GetPixelRowSpan(writeY)); var rgba = new Rgba32(0, 0, 0, 255); // #403 The left + width value can be larger than the image width - for (int x = descriptor.Left; x < descriptor.Left + descriptor.Width && x < rowSpan.Length; x++) + for (int x = descriptor.Left; x < descriptor.Left + descriptor.Width && x < imageWidth; x++) { - int index = indices[i]; + int index = Unsafe.Add(ref indicesRef, i); if (this.graphicsControlExtension == null || this.graphicsControlExtension.TransparencyFlag == false || @@ -502,7 +500,7 @@ namespace SixLabors.ImageSharp.Formats.Gif { int indexOffset = index * 3; - ref TPixel pixel = ref rowSpan[x]; + ref TPixel pixel = ref Unsafe.Add(ref rowRef, x); rgba.Rgb = colorTable.GetRgb24(indexOffset); pixel.PackFromRgba32(rgba); diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index 436db636d..cb865e95d 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -4,6 +4,8 @@ using System; using System.IO; using System.Linq; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Text; using SixLabors.ImageSharp.IO; using SixLabors.ImageSharp.Memory; @@ -11,6 +13,9 @@ 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 { /// @@ -45,11 +50,6 @@ namespace SixLabors.ImageSharp.Formats.Gif /// private int bitDepth; - /// - /// Whether the current image has multiple frames. - /// - private bool hasFrames; - /// /// Initializes a new instance of the class. /// @@ -78,8 +78,6 @@ namespace SixLabors.ImageSharp.Formats.Gif // Do not use IDisposable pattern here as we want to preserve the stream. var writer = new EndianBinaryWriter(Endianness.LittleEndian, stream); - this.hasFrames = image.Frames.Count > 1; - // Quantize the image returning a palette. QuantizedFrame quantized = this.quantizer.CreateFrameQuantizer().QuantizeFrame(image.Frames.RootFrame); @@ -98,9 +96,9 @@ namespace SixLabors.ImageSharp.Formats.Gif this.WriteComments(image, writer); // Write additional frames. - if (this.hasFrames) + if (image.Frames.Count > 1) { - this.WriteApplicationExtension(writer, image.MetaData.RepeatCount, image.Frames.Count); + this.WriteApplicationExtension(writer, image.MetaData.RepeatCount); } foreach (ImageFrame frame in image.Frames) @@ -138,11 +136,12 @@ 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); + ref TPixel paletteRef = ref MemoryMarshal.GetReference(quantized.Palette.AsSpan()); for (int i = quantized.Palette.Length - 1; i >= 0; i--) { - quantized.Palette[i].ToRgba32(ref trans); - - if (trans.Equals(default(Rgba32))) + ref TPixel entry = ref Unsafe.Add(ref paletteRef, i); + entry.ToRgba32(ref trans); + if (trans.Equals(default)) { index = i; } @@ -155,6 +154,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// Writes the file header signature and version to the stream. /// /// The writer to write to the stream with. + [MethodImpl(MethodImplOptions.AggressiveInlining)] private void WriteHeader(EndianBinaryWriter writer) { writer.Write(GifConstants.MagicNumber); @@ -201,11 +201,10 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// The writer to write to the stream with. /// The animated image repeat count. - /// The number of image frames. - private void WriteApplicationExtension(EndianBinaryWriter writer, ushort repeatCount, int frames) + private void WriteApplicationExtension(EndianBinaryWriter writer, ushort repeatCount) { // Application Extension Header - if (repeatCount != 1 && frames > 0) + if (repeatCount != 1) { this.buffer[0] = GifConstants.ExtensionIntroducer; this.buffer[1] = GifConstants.ApplicationExtensionLabel; @@ -336,15 +335,14 @@ namespace SixLabors.ImageSharp.Formats.Gif var rgb = default(Rgb24); using (IManagedByteBuffer colorTable = this.memoryManager.AllocateManagedByteBuffer(colorTableLength)) { - Span colorTableSpan = colorTable.Span; + 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++) { - int offset = i * 3; - image.Palette[i].ToRgb24(ref rgb); - colorTableSpan[offset] = rgb.R; - colorTableSpan[offset + 1] = rgb.G; - colorTableSpan[offset + 2] = rgb.B; + ref TPixel entry = ref Unsafe.Add(ref paletteRef, i); + entry.ToRgb24(ref rgb); + Unsafe.Add(ref rgb24Ref, i) = rgb; } writer.Write(colorTable.Array, 0, colorTableLength); diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index 0c73efea4..37daa6de5 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -2,9 +2,9 @@ // Licensed under the Apache License, Version 2.0. using System; -using System.Buffers; using System.IO; - +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using SixLabors.ImageSharp.Memory; namespace SixLabors.ImageSharp.Formats.Gif @@ -115,14 +115,14 @@ namespace SixLabors.ImageSharp.Formats.Gif int data = 0; int first = 0; - Span prefixSpan = this.prefix.Span; - Span suffixSpan = this.suffix.Span; - Span pixelStackSpan = this.pixelStack.Span; + ref int prefixRef = ref MemoryMarshal.GetReference(this.prefix.Span); + ref int suffixRef = ref MemoryMarshal.GetReference(this.suffix.Span); + ref int pixelStackRef = ref MemoryMarshal.GetReference(this.pixelStack.Span); + ref byte pixelsRef = ref MemoryMarshal.GetReference(pixels); for (code = 0; code < clearCode; code++) { - prefixSpan[code] = 0; - suffixSpan[code] = (byte)code; + Unsafe.Add(ref suffixRef, code) = (byte)code; } byte[] buffer = new byte[255]; @@ -176,7 +176,7 @@ namespace SixLabors.ImageSharp.Formats.Gif if (oldCode == NullCode) { - pixelStackSpan[top++] = suffixSpan[code]; + Unsafe.Add(ref pixelStackRef, top++) = Unsafe.Add(ref suffixRef, code); oldCode = code; first = code; continue; @@ -185,27 +185,27 @@ namespace SixLabors.ImageSharp.Formats.Gif int inCode = code; if (code == availableCode) { - pixelStackSpan[top++] = (byte)first; + Unsafe.Add(ref pixelStackRef, top++) = (byte)first; code = oldCode; } while (code > clearCode) { - pixelStackSpan[top++] = suffixSpan[code]; - code = prefixSpan[code]; + Unsafe.Add(ref pixelStackRef, top++) = Unsafe.Add(ref suffixRef, code); + code = Unsafe.Add(ref prefixRef, code); } - first = suffixSpan[code]; - - pixelStackSpan[top++] = suffixSpan[code]; + int suffixCode = Unsafe.Add(ref suffixRef, code); + first = suffixCode; + Unsafe.Add(ref pixelStackRef, top++) = suffixCode; // Fix for Gifs that have "deferred clear code" as per here : // https://bugzilla.mozilla.org/show_bug.cgi?id=55918 if (availableCode < MaxStackSize) { - prefixSpan[availableCode] = oldCode; - suffixSpan[availableCode] = first; + Unsafe.Add(ref prefixRef, availableCode) = oldCode; + Unsafe.Add(ref suffixRef, availableCode) = first; availableCode++; if (availableCode == codeMask + 1 && availableCode < MaxStackSize) { @@ -221,7 +221,7 @@ namespace SixLabors.ImageSharp.Formats.Gif top--; // Clear missing pixels - pixels[xyz++] = (byte)pixelStackSpan[top]; + Unsafe.Add(ref pixelsRef, xyz++) = (byte)Unsafe.Add(ref pixelStackRef, top); } } @@ -238,8 +238,9 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// The buffer to store the block in. /// - /// The . + /// The . /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] private int ReadBlock(byte[] buffer) { int bufferSize = this.stream.ReadByte(); diff --git a/src/ImageSharp/Formats/Gif/LzwEncoder.cs b/src/ImageSharp/Formats/Gif/LzwEncoder.cs index 35c414896..60bc56dc5 100644 --- a/src/ImageSharp/Formats/Gif/LzwEncoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwEncoder.cs @@ -2,9 +2,9 @@ // Licensed under the Apache License, Version 2.0. using System; -using System.Buffers; using System.IO; - +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using SixLabors.ImageSharp.Memory; namespace SixLabors.ImageSharp.Formats.Gif @@ -233,6 +233,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// The number of bits /// See + [MethodImpl(MethodImplOptions.AggressiveInlining)] private static int GetMaxcode(int bitCount) { return (1 << bitCount) - 1; @@ -243,10 +244,12 @@ namespace SixLabors.ImageSharp.Formats.Gif /// flush the packet to disk. /// /// The character to add. + /// The reference to the storage for packat accumulators /// The stream to write to. - private void AddCharacter(byte c, Stream stream) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private void AddCharacter(byte c, ref byte accumulatorsRef, Stream stream) { - this.accumulators[this.accumulatorCount++] = c; + Unsafe.Add(ref accumulatorsRef, this.accumulatorCount++) = c; if (this.accumulatorCount >= 254) { this.FlushPacket(stream); @@ -257,6 +260,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// Table clear for block compress /// /// The output stream. + [MethodImpl(MethodImplOptions.AggressiveInlining)] private void ClearBlock(Stream stream) { this.ResetCodeTable(); @@ -269,15 +273,10 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// Reset the code table. /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] private void ResetCodeTable() { this.hashTable.Span.Fill(-1); - - // Original code: - // for (int i = 0; i < size; ++i) - // { - // this.hashTable[i] = -1; - // } } /// @@ -309,6 +308,7 @@ namespace SixLabors.ImageSharp.Formats.Gif ent = this.NextPixel(); + // TODO: PERF: It looks likt hshift could be calculated once statically. hshift = 0; for (fcode = this.hsize; fcode < 65536; fcode *= 2) { @@ -323,22 +323,22 @@ namespace SixLabors.ImageSharp.Formats.Gif this.Output(this.clearCode, stream); - Span hashTableSpan = this.hashTable.Span; - Span codeTableSpan = this.codeTable.Span; + ref int hashTableRef = ref MemoryMarshal.GetReference(this.hashTable.Span); + ref int codeTableRef = ref MemoryMarshal.GetReference(this.codeTable.Span); while ((c = this.NextPixel()) != Eof) { fcode = (c << this.maxbits) + ent; int i = (c << hshift) ^ ent /* = 0 */; - if (hashTableSpan[i] == fcode) + if (Unsafe.Add(ref hashTableRef, i) == fcode) { - ent = codeTableSpan[i]; + ent = Unsafe.Add(ref codeTableRef, i); continue; } // Non-empty slot - if (hashTableSpan[i] >= 0) + if (Unsafe.Add(ref hashTableRef, i) >= 0) { int disp = hsizeReg - i; if (i == 0) @@ -353,15 +353,15 @@ namespace SixLabors.ImageSharp.Formats.Gif i += hsizeReg; } - if (hashTableSpan[i] == fcode) + if (Unsafe.Add(ref hashTableRef, i) == fcode) { - ent = codeTableSpan[i]; + ent = Unsafe.Add(ref codeTableRef, i); break; } } - while (hashTableSpan[i] >= 0); + while (Unsafe.Add(ref hashTableRef, i) >= 0); - if (hashTableSpan[i] == fcode) + if (Unsafe.Add(ref hashTableRef, i) == fcode) { continue; } @@ -371,8 +371,8 @@ namespace SixLabors.ImageSharp.Formats.Gif ent = c; if (this.freeEntry < this.maxmaxcode) { - codeTableSpan[i] = this.freeEntry++; // code -> hashtable - hashTableSpan[i] = fcode; + Unsafe.Add(ref codeTableRef, i) = this.freeEntry++; // code -> hashtable + Unsafe.Add(ref hashTableRef, i) = fcode; } else { @@ -390,14 +390,12 @@ namespace SixLabors.ImageSharp.Formats.Gif /// Flush the packet to disk, and reset the accumulator. /// /// The output stream. + [MethodImpl(MethodImplOptions.AggressiveInlining)] private void FlushPacket(Stream outStream) { - if (this.accumulatorCount > 0) - { - outStream.WriteByte((byte)this.accumulatorCount); - outStream.Write(this.accumulators, 0, this.accumulatorCount); - this.accumulatorCount = 0; - } + outStream.WriteByte((byte)this.accumulatorCount); + outStream.Write(this.accumulators, 0, this.accumulatorCount); + this.accumulatorCount = 0; } /// @@ -424,6 +422,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// The stream to write to. private void Output(int code, Stream outs) { + ref byte accumulatorsRef = ref MemoryMarshal.GetReference(this.accumulators.AsSpan()); this.currentAccumulator &= Masks[this.currentBits]; if (this.currentBits > 0) @@ -439,7 +438,7 @@ namespace SixLabors.ImageSharp.Formats.Gif while (this.currentBits >= 8) { - this.AddCharacter((byte)(this.currentAccumulator & 0xff), outs); + this.AddCharacter((byte)(this.currentAccumulator & 0xFF), ref accumulatorsRef, outs); this.currentAccumulator >>= 8; this.currentBits -= 8; } @@ -467,12 +466,15 @@ namespace SixLabors.ImageSharp.Formats.Gif // At EOF, write the rest of the buffer. while (this.currentBits > 0) { - this.AddCharacter((byte)(this.currentAccumulator & 0xff), outs); + this.AddCharacter((byte)(this.currentAccumulator & 0xFF), ref accumulatorsRef, outs); this.currentAccumulator >>= 8; this.currentBits -= 8; } - this.FlushPacket(outs); + if (this.accumulatorCount > 0) + { + this.FlushPacket(outs); + } } } diff --git a/src/ImageSharp/Formats/Gif/PackedField.cs b/src/ImageSharp/Formats/Gif/PackedField.cs index 969449a9f..0d3b1539c 100644 --- a/src/ImageSharp/Formats/Gif/PackedField.cs +++ b/src/ImageSharp/Formats/Gif/PackedField.cs @@ -2,6 +2,8 @@ // Licensed under the Apache License, Version 2.0. using System; +using System.Diagnostics; +using System.Runtime.CompilerServices; namespace SixLabors.ImageSharp.Formats.Gif { @@ -21,6 +23,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// public byte Byte { + [MethodImpl(MethodImplOptions.AggressiveInlining)] get { int returnValue = 0; @@ -53,7 +56,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// The public static PackedField FromInt(byte value) { - PackedField packed = default(PackedField); + PackedField packed = default; packed.SetBits(0, 8, value); return packed; } @@ -70,12 +73,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// public void SetBit(int index, bool valueToSet) { - if (index < 0 || index > 7) - { - string message = $"Index must be between 0 and 7. Supplied index: {index}"; - throw new ArgumentOutOfRangeException(nameof(index), message); - } - + DebugGuard.MustBeBetweenOrEqualTo(index, 0, 7, nameof(index)); Bits[index] = valueToSet; } @@ -88,18 +86,8 @@ namespace SixLabors.ImageSharp.Formats.Gif /// The value to set the bits to. public void SetBits(int startIndex, int length, int valueToSet) { - if (startIndex < 0 || startIndex > 7) - { - string message = $"Start index must be between 0 and 7. Supplied index: {startIndex}"; - throw new ArgumentOutOfRangeException(nameof(startIndex), message); - } - - 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); - } + DebugGuard.MustBeBetweenOrEqualTo(startIndex, 0, 7, nameof(startIndex)); + DebugCheckLength(startIndex, length); int bitShift = length - 1; for (int i = startIndex; i < startIndex + length; i++) @@ -121,12 +109,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// public bool GetBit(int index) { - if (index < 0 || index > 7) - { - string message = $"Index must be between 0 and 7. Supplied index: {index}"; - throw new ArgumentOutOfRangeException(nameof(index), message); - } - + DebugGuard.MustBeBetweenOrEqualTo(index, 0, 7, nameof(index)); return Bits[index]; } @@ -140,19 +123,8 @@ namespace SixLabors.ImageSharp.Formats.Gif /// public int GetBits(int startIndex, int length) { - if (startIndex < 0 || startIndex > 7) - { - string message = $"Start index must be between 0 and 7. Supplied index: {startIndex}"; - throw new ArgumentOutOfRangeException(nameof(startIndex), message); - } - - 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); - } + DebugGuard.MustBeBetweenOrEqualTo(startIndex, 1, 8, nameof(startIndex)); + DebugCheckLength(startIndex, length); int returnValue = 0; int bitShift = length - 1; @@ -189,5 +161,17 @@ namespace SixLabors.ImageSharp.Formats.Gif { return this.Byte.GetHashCode(); } + + [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 diff --git a/src/ImageSharp/Processing/Quantization/FrameQuantizers/PaletteFrameQuantizer{TPixel}.cs b/src/ImageSharp/Processing/Quantization/FrameQuantizers/PaletteFrameQuantizer{TPixel}.cs index b3a3eee63..34cb7eb16 100644 --- a/src/ImageSharp/Processing/Quantization/FrameQuantizers/PaletteFrameQuantizer{TPixel}.cs +++ b/src/ImageSharp/Processing/Quantization/FrameQuantizers/PaletteFrameQuantizer{TPixel}.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.PixelFormats; @@ -45,18 +46,18 @@ namespace SixLabors.ImageSharp.Processing.Quantization.FrameQuantizers TPixel sourcePixel = source[0, 0]; TPixel previousPixel = sourcePixel; byte pixelValue = this.QuantizePixel(sourcePixel); - TPixel[] colorPalette = this.GetPalette(); - TPixel transformedPixel = colorPalette[pixelValue]; + ref TPixel colorPaletteRef = ref MemoryMarshal.GetReference(this.GetPalette().AsSpan()); + TPixel transformedPixel = Unsafe.Add(ref colorPaletteRef, pixelValue); for (int y = 0; y < height; y++) { - Span row = source.GetPixelRowSpan(y); + ref TPixel rowRef = ref MemoryMarshal.GetReference(source.GetPixelRowSpan(y)); // And loop through each column for (int x = 0; x < width; x++) { // Get the pixel. - sourcePixel = row[x]; + sourcePixel = Unsafe.Add(ref rowRef, x); // Check if this is the same as the last pixel. If so use that value // rather than calculating it again. This is an inexpensive optimization. @@ -70,7 +71,7 @@ namespace SixLabors.ImageSharp.Processing.Quantization.FrameQuantizers if (this.Dither) { - transformedPixel = colorPalette[pixelValue]; + transformedPixel = Unsafe.Add(ref colorPaletteRef, pixelValue); } } @@ -86,6 +87,7 @@ namespace SixLabors.ImageSharp.Processing.Quantization.FrameQuantizers } /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] protected override TPixel[] GetPalette() { return this.colors; diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs index a2f4806f3..1e0cd948b 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifEncoderTests.cs @@ -2,10 +2,11 @@ // Licensed under the Apache License, Version 2.0. using System.IO; -using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Gif; using SixLabors.ImageSharp.MetaData; using SixLabors.ImageSharp.PixelFormats; +using SixLabors.ImageSharp.Processing.Quantization; +using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison; using Xunit; // ReSharper disable InconsistentNaming @@ -14,6 +15,7 @@ namespace SixLabors.ImageSharp.Tests public class GifEncoderTests { private const PixelTypes TestPixelTypes = PixelTypes.Rgba32 | PixelTypes.RgbaVector | PixelTypes.Argb32; + private static readonly ImageComparer ValidatorComparer = ImageComparer.TolerantPercentage(0.001F); [Theory] [WithTestPatternImages(100, 100, TestPixelTypes)] @@ -22,28 +24,43 @@ namespace SixLabors.ImageSharp.Tests { using (Image image = provider.GetImage()) { - provider.Utility.SaveTestOutputFile(image, "gif", new GifEncoder()); + var encoder = new GifEncoder() + { + // Use the palette quantizer without dithering to ensure results + // are consistant + Quantizer = new PaletteQuantizer(false) + }; + + // Always save as we need to compare the encoded output. + provider.Utility.SaveTestOutputFile(image, "gif", encoder); + } + + // Compare encoded result + string path = provider.Utility.GetTestOutputFileName("gif", null, true); + using (var encoded = Image.Load(path)) + { + encoded.CompareToReferenceOutput(ValidatorComparer, provider, null, "gif"); } } [Fact] public void Encode_IgnoreMetadataIsFalse_CommentsAreWritten() { - GifEncoder options = new GifEncoder() + var options = new GifEncoder() { IgnoreMetadata = false }; - TestFile testFile = TestFile.Create(TestImages.Gif.Rings); + var testFile = TestFile.Create(TestImages.Gif.Rings); using (Image input = testFile.CreateImage()) { - using (MemoryStream memStream = new MemoryStream()) + using (var memStream = new MemoryStream()) { input.Save(memStream, options); memStream.Position = 0; - using (Image output = Image.Load(memStream)) + using (var output = Image.Load(memStream)) { Assert.Equal(1, output.MetaData.Properties.Count); Assert.Equal("Comments", output.MetaData.Properties[0].Name); @@ -56,21 +73,21 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void Encode_IgnoreMetadataIsTrue_CommentsAreNotWritten() { - GifEncoder options = new GifEncoder() + var options = new GifEncoder() { IgnoreMetadata = true }; - TestFile testFile = TestFile.Create(TestImages.Gif.Rings); + var testFile = TestFile.Create(TestImages.Gif.Rings); using (Image input = testFile.CreateImage()) { - using (MemoryStream memStream = new MemoryStream()) + using (var memStream = new MemoryStream()) { input.SaveAsGif(memStream, options); memStream.Position = 0; - using (Image output = Image.Load(memStream)) + using (var output = Image.Load(memStream)) { Assert.Equal(0, output.MetaData.Properties.Count); } @@ -81,17 +98,17 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void Encode_WhenCommentIsTooLong_CommentIsTrimmed() { - using (Image input = new Image(1, 1)) + using (var input = new Image(1, 1)) { string comments = new string('c', 256); input.MetaData.Properties.Add(new ImageProperty("Comments", comments)); - using (MemoryStream memStream = new MemoryStream()) + using (var memStream = new MemoryStream()) { input.Save(memStream, new GifEncoder()); memStream.Position = 0; - using (Image output = Image.Load(memStream)) + using (var output = Image.Load(memStream)) { Assert.Equal(1, output.MetaData.Properties.Count); Assert.Equal("Comments", output.MetaData.Properties[0].Name); diff --git a/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs b/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs index dbae4f85d..7616f89ea 100644 --- a/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs +++ b/tests/ImageSharp.Tests/TestUtilities/TestImageExtensions.cs @@ -87,6 +87,37 @@ namespace SixLabors.ImageSharp.Tests return image; } + /// + /// Saves the image only when not running in the CI server. + /// + /// The pixel format + /// The image + /// The image provider + /// The image encoder + /// Details to be concatenated to the test output file, describing the parameters of the test. + /// A boolean indicating whether to append the pixel type to the output file name. + public static Image DebugSave( + this Image image, + ITestImageProvider provider, + IImageEncoder encoder, + object testOutputDetails = null, + bool appendPixelTypeToFileName = true) + where TPixel : struct, IPixel + { + if (TestEnvironment.RunsOnCI) + { + return image; + } + + // We are running locally then we want to save it out + provider.Utility.SaveTestOutputFile( + image, + encoder: encoder, + testOutputDetails: testOutputDetails, + appendPixelTypeToFileName: appendPixelTypeToFileName); + return image; + } + public static Image DebugSaveMultiFrame( this Image image, ITestImageProvider provider, @@ -168,7 +199,7 @@ namespace SixLabors.ImageSharp.Tests provider, testOutputDetails, extension, - appendPixelTypeToFileName)) + appendPixelTypeToFileName)) { comparer.VerifySimilarity(referenceImage, image); } @@ -272,7 +303,7 @@ namespace SixLabors.ImageSharp.Tests } Image firstTemp = temporaryFrameImages[0]; - + var result = new Image(firstTemp.Width, firstTemp.Height); foreach (Image fi in temporaryFrameImages) @@ -345,7 +376,7 @@ namespace SixLabors.ImageSharp.Tests { return CompareToOriginal(image, provider, ImageComparer.Tolerant()); } - + public static Image CompareToOriginal( this Image image, ITestImageProvider provider, diff --git a/tests/Images/External b/tests/Images/External index 5a66c9c6d..01af5f369 160000 --- a/tests/Images/External +++ b/tests/Images/External @@ -1 +1 @@ -Subproject commit 5a66c9c6da02bf27345f90adc05d415c0d0450ea +Subproject commit 01af5f36912ec7080cae3187a48905d1e54f6ea7