From 403b7759df4c30b60517972a6d3eb21473e0cf55 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 8 Oct 2017 13:25:36 +1100 Subject: [PATCH 1/7] Remove PixelAccessor and cleanup --- .../Processors/DrawImageProcessor.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/ImageSharp.Drawing/Processors/DrawImageProcessor.cs b/src/ImageSharp.Drawing/Processors/DrawImageProcessor.cs index 213ab1b4a..a88a0a6d0 100644 --- a/src/ImageSharp.Drawing/Processors/DrawImageProcessor.cs +++ b/src/ImageSharp.Drawing/Processors/DrawImageProcessor.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. using System; -using System.Numerics; using System.Threading.Tasks; using SixLabors.ImageSharp.Advanced; using SixLabors.ImageSharp.Helpers; @@ -42,7 +41,7 @@ namespace SixLabors.ImageSharp.Drawing.Processors /// /// Gets the image to blend. /// - public Image Image { get; private set; } + public Image Image { get; } /// /// Gets the alpha percentage value. @@ -84,9 +83,7 @@ namespace SixLabors.ImageSharp.Drawing.Processors maxY = Math.Min(this.Location.Y + this.Size.Height, maxY); int width = maxX - minX; - using (Buffer amount = new Buffer(width)) - using (PixelAccessor toBlendPixels = targetImage.Lock()) - using (PixelAccessor sourcePixels = source.Lock()) + using (var amount = new Buffer(width)) { for (int i = 0; i < width; i++) { @@ -99,8 +96,8 @@ namespace SixLabors.ImageSharp.Drawing.Processors configuration.ParallelOptions, y => { - Span background = sourcePixels.GetRowSpan(y).Slice(minX, width); - Span foreground = toBlendPixels.GetRowSpan(y - this.Location.Y).Slice(0, width); + Span background = source.GetPixelRowSpan(y).Slice(minX, width); + Span foreground = targetImage.GetPixelRowSpan(y - this.Location.Y).Slice(0, width); this.blender.Blend(background, background, foreground, amount); }); } From 55d688387ee976f35e620144131c118efc1598ef Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 8 Oct 2017 16:55:29 +1100 Subject: [PATCH 2/7] Correctly handle negative x location. Fix #252 --- src/ImageSharp.Drawing/Processors/DrawImageProcessor.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp.Drawing/Processors/DrawImageProcessor.cs b/src/ImageSharp.Drawing/Processors/DrawImageProcessor.cs index a88a0a6d0..47763c0aa 100644 --- a/src/ImageSharp.Drawing/Processors/DrawImageProcessor.cs +++ b/src/ImageSharp.Drawing/Processors/DrawImageProcessor.cs @@ -72,10 +72,11 @@ namespace SixLabors.ImageSharp.Drawing.Processors } // Align start/end positions. - Rectangle bounds = this.Image.Bounds(); + Rectangle bounds = targetImage.Bounds(); int minX = Math.Max(this.Location.X, sourceRectangle.X); int maxX = Math.Min(this.Location.X + bounds.Width, sourceRectangle.Width); maxX = Math.Min(this.Location.X + this.Size.Width, maxX); + int targetX = minX - this.Location.X; int minY = Math.Max(this.Location.Y, sourceRectangle.Y); int maxY = Math.Min(this.Location.Y + bounds.Height, sourceRectangle.Bottom); @@ -97,7 +98,7 @@ namespace SixLabors.ImageSharp.Drawing.Processors y => { Span background = source.GetPixelRowSpan(y).Slice(minX, width); - Span foreground = targetImage.GetPixelRowSpan(y - this.Location.Y).Slice(0, width); + Span foreground = targetImage.GetPixelRowSpan(y - this.Location.Y).Slice(targetX, width); this.blender.Blend(background, background, foreground, amount); }); } From f53a047a073aadab38c8e38004e93f6ecb836d40 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 8 Oct 2017 17:17:40 +1100 Subject: [PATCH 3/7] Cleanup gif decoder classes --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 42 +++++++++---------- .../Formats/Gif/IGifDecoderOptions.cs | 4 -- 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index bfd441259..7fa3aa04a 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -95,7 +95,7 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// Gets the text encoding /// - public Encoding TextEncoding { get; private set; } + public Encoding TextEncoding { get; } /// /// Decodes the stream to the image. @@ -311,10 +311,9 @@ namespace SixLabors.ImageSharp.Formats.Gif try { // Determine the color table for this frame. If there is a local one, use it otherwise use the global color table. - int length = this.globalColorTableLength; if (imageDescriptor.LocalColorTableFlag) { - length = imageDescriptor.LocalColorTableSize * 3; + int length = imageDescriptor.LocalColorTableSize * 3; localColorTable = ArrayPool.Shared.Rent(length); this.currentStream.Read(localColorTable, 0, length); } @@ -322,7 +321,7 @@ namespace SixLabors.ImageSharp.Formats.Gif indices = ArrayPool.Shared.Rent(imageDescriptor.Width * imageDescriptor.Height); this.ReadFrameIndices(imageDescriptor, indices); - this.ReadFrameColors(indices, localColorTable ?? this.globalColorTable, length, imageDescriptor); + this.ReadFrameColors(indices, localColorTable ?? this.globalColorTable, imageDescriptor); // Skip any remaining blocks this.Skip(0); @@ -358,18 +357,17 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// The indexed pixels. /// The color table containing the available colors. - /// The color table length. /// The - private unsafe void ReadFrameColors(byte[] indices, byte[] colorTable, int colorTableLength, GifImageDescriptor descriptor) + private void ReadFrameColors(byte[] indices, byte[] colorTable, GifImageDescriptor descriptor) { int imageWidth = this.logicalScreenDescriptor.Width; int imageHeight = this.logicalScreenDescriptor.Height; - ImageFrame previousFrame = null; + ImageFrame prevFrame = null; ImageFrame currentFrame = null; - ImageFrame image; + ImageFrame imageFrame; if (this.previousFrame == null) { @@ -378,23 +376,23 @@ namespace SixLabors.ImageSharp.Formats.Gif this.SetFrameMetaData(this.image.Frames.RootFrame.MetaData); - image = this.image.Frames.RootFrame; + imageFrame = this.image.Frames.RootFrame; } else { if (this.graphicsControlExtension != null && this.graphicsControlExtension.DisposalMethod == DisposalMethod.RestoreToPrevious) { - previousFrame = this.previousFrame; + prevFrame = this.previousFrame; } - currentFrame = this.image.Frames.AddFrame(this.previousFrame); // this clones the frame and adds it the collection + currentFrame = this.image.Frames.AddFrame(this.previousFrame); // This clones the frame and adds it the collection this.SetFrameMetaData(currentFrame.MetaData); - image = currentFrame; + imageFrame = currentFrame; - this.RestoreToBackground(image); + this.RestoreToBackground(imageFrame); } int i = 0; @@ -439,9 +437,9 @@ namespace SixLabors.ImageSharp.Formats.Gif writeY = y; } - Span rowSpan = image.GetPixelRowSpan(writeY); + Span rowSpan = imageFrame.GetPixelRowSpan(writeY); - Rgba32 rgba = new Rgba32(0, 0, 0, 255); + var rgba = new Rgba32(0, 0, 0, 255); for (int x = descriptor.Left; x < descriptor.Left + descriptor.Width; x++) { @@ -463,13 +461,13 @@ namespace SixLabors.ImageSharp.Formats.Gif } } - if (previousFrame != null) + if (prevFrame != null) { - this.previousFrame = previousFrame; + this.previousFrame = prevFrame; return; } - this.previousFrame = currentFrame == null ? this.image.Frames.RootFrame : currentFrame; + this.previousFrame = currentFrame ?? this.image.Frames.RootFrame; if (this.graphicsControlExtension != null && this.graphicsControlExtension.DisposalMethod == DisposalMethod.RestoreToBackground) @@ -518,18 +516,18 @@ namespace SixLabors.ImageSharp.Formats.Gif /// /// Sets the frames metadata. /// - /// The meta data. + /// The meta data. [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void SetFrameMetaData(ImageFrameMetaData metaData) + private void SetFrameMetaData(ImageFrameMetaData meta) { if (this.graphicsControlExtension != null) { if (this.graphicsControlExtension.DelayTime > 0) { - metaData.FrameDelay = this.graphicsControlExtension.DelayTime; + meta.FrameDelay = this.graphicsControlExtension.DelayTime; } - metaData.DisposalMethod = this.graphicsControlExtension.DisposalMethod; + meta.DisposalMethod = this.graphicsControlExtension.DisposalMethod; } } } diff --git a/src/ImageSharp/Formats/Gif/IGifDecoderOptions.cs b/src/ImageSharp/Formats/Gif/IGifDecoderOptions.cs index 60c39f936..fd4cc82c9 100644 --- a/src/ImageSharp/Formats/Gif/IGifDecoderOptions.cs +++ b/src/ImageSharp/Formats/Gif/IGifDecoderOptions.cs @@ -1,11 +1,7 @@ // Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. -using System; -using System.Collections.Generic; -using System.IO; using System.Text; -using SixLabors.ImageSharp.PixelFormats; namespace SixLabors.ImageSharp.Formats.Gif { From e14bf0889fb3e95989ac8815c5aef2f8ad706396 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 9 Oct 2017 00:01:43 +1100 Subject: [PATCH 4/7] Fix quantization transparent pixel allocation --- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 10 +- .../Quantizers/OctreeQuantizer{TPixel}.cs | 122 +++++++++++------- .../Quantizers/QuantizerBase{TPixel}.cs | 2 +- .../Quantization/QuantizedImageTests.cs | 95 ++++++++++++++ 4 files changed, 177 insertions(+), 52 deletions(-) create mode 100644 tests/ImageSharp.Tests/Quantization/QuantizedImageTests.cs diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index b10f8a2e0..1b145a79e 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -154,18 +154,14 @@ 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); for (int i = quantized.Palette.Length - 1; i >= 0; i--) { - quantized.Palette[i].ToXyzwBytes(this.buffer, 0); + quantized.Palette[i].ToRgba32(ref trans); - if (this.buffer[3] > 0) - { - continue; - } - else + if (trans.Equals(default(Rgba32))) { index = i; - break; } } diff --git a/src/ImageSharp/Quantizers/OctreeQuantizer{TPixel}.cs b/src/ImageSharp/Quantizers/OctreeQuantizer{TPixel}.cs index 8766f1042..d646a680e 100644 --- a/src/ImageSharp/Quantizers/OctreeQuantizer{TPixel}.cs +++ b/src/ImageSharp/Quantizers/OctreeQuantizer{TPixel}.cs @@ -23,11 +23,6 @@ namespace SixLabors.ImageSharp.Quantizers /// private readonly Dictionary colorMap = new Dictionary(); - /// - /// The pixel buffer, used to reduce allocations. - /// - private readonly byte[] pixelBuffer = new byte[4]; - /// /// Stores the tree /// @@ -43,6 +38,11 @@ namespace SixLabors.ImageSharp.Quantizers /// private TPixel[] palette; + /// + /// The transparent index + /// + private byte transparentIndex; + /// /// Initializes a new instance of the class. /// @@ -73,7 +73,8 @@ namespace SixLabors.ImageSharp.Quantizers // pass of the algorithm by avoiding transforming rows of identical color. TPixel sourcePixel = source[0, 0]; TPixel previousPixel = sourcePixel; - byte pixelValue = this.QuantizePixel(sourcePixel); + var rgba = default(Rgba32); + byte pixelValue = this.QuantizePixel(sourcePixel, ref rgba); TPixel[] colorPalette = this.GetPalette(); TPixel transformedPixel = colorPalette[pixelValue]; @@ -92,7 +93,7 @@ namespace SixLabors.ImageSharp.Quantizers if (!previousPixel.Equals(sourcePixel)) { // Quantize the pixel - pixelValue = this.QuantizePixel(sourcePixel); + pixelValue = this.QuantizePixel(sourcePixel, ref rgba); // And setup the previous pointer previousPixel = sourcePixel; @@ -118,24 +119,57 @@ namespace SixLabors.ImageSharp.Quantizers protected override void InitialQuantizePixel(TPixel pixel) { // Add the color to the Octree - this.octree.AddColor(pixel, this.pixelBuffer); + var rgba = default(Rgba32); + this.octree.AddColor(pixel, ref rgba); } /// protected override TPixel[] GetPalette() { - return this.palette ?? (this.palette = this.octree.Palletize(Math.Max(this.colors, (byte)1))); + if (this.palette == null) + { + this.palette = this.octree.Palletize(Math.Max(this.colors, (byte)1)); + this.transparentIndex = this.GetTransparentIndex(); + } + + return this.palette; + } + + /// + /// Returns the index of the first instance of the transparent color in the palette. + /// + /// + /// The . + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private byte GetTransparentIndex() + { + // Transparent pixels are much more likely to be found at the end of a palette + int index = this.colors; + var trans = default(Rgba32); + for (int i = this.palette.Length - 1; i >= 0; i--) + { + this.palette[i].ToRgba32(ref trans); + + if (trans.Equals(default(Rgba32))) + { + index = i; + } + } + + return (byte)index; } /// /// Process the pixel in the second pass of the algorithm /// /// The pixel to quantize + /// The color to compare against /// /// The quantized value /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - private byte QuantizePixel(TPixel pixel) + private byte QuantizePixel(TPixel pixel, ref Rgba32 rgba) { if (this.Dither) { @@ -144,13 +178,13 @@ namespace SixLabors.ImageSharp.Quantizers return this.GetClosestPixel(pixel, this.palette, this.colorMap); } - pixel.ToXyzwBytes(this.pixelBuffer, 0); - if (this.pixelBuffer[3] == 0) + pixel.ToRgba32(ref rgba); + if (rgba.Equals(default(Rgba32))) { - return this.colors; + return this.transparentIndex; } - return (byte)this.octree.GetPaletteIndex(pixel, this.pixelBuffer); + return (byte)this.octree.GetPaletteIndex(pixel, ref rgba); } /// @@ -233,8 +267,8 @@ namespace SixLabors.ImageSharp.Quantizers /// Add a given color value to the Octree /// /// The pixel data. - /// The buffer array. - public void AddColor(TPixel pixel, byte[] buffer) + /// The color. + public void AddColor(TPixel pixel, ref Rgba32 rgba) { // Check if this request is for the same color as the last if (this.previousColor.Equals(pixel)) @@ -244,18 +278,18 @@ namespace SixLabors.ImageSharp.Quantizers if (this.previousNode == null) { this.previousColor = pixel; - this.root.AddColor(pixel, this.maxColorBits, 0, this, buffer); + this.root.AddColor(pixel, this.maxColorBits, 0, this, ref rgba); } else { // Just update the previous node - this.previousNode.Increment(pixel, buffer); + this.previousNode.Increment(pixel, ref rgba); } } else { this.previousColor = pixel; - this.root.AddColor(pixel, this.maxColorBits, 0, this, buffer); + this.root.AddColor(pixel, this.maxColorBits, 0, this, ref rgba); } } @@ -287,13 +321,13 @@ namespace SixLabors.ImageSharp.Quantizers /// Get the palette index for the passed color /// /// The pixel data. - /// The buffer array. + /// The color to map to. /// /// The . /// - public int GetPaletteIndex(TPixel pixel, byte[] buffer) + public int GetPaletteIndex(TPixel pixel, ref Rgba32 rgba) { - return this.root.GetPaletteIndex(pixel, 0, buffer); + return this.root.GetPaletteIndex(pixel, 0, ref rgba); } /// @@ -415,17 +449,17 @@ namespace SixLabors.ImageSharp.Quantizers /// /// Add a color into the tree /// - /// The color + /// The pixel color /// The number of significant color bits /// The level in the tree /// The tree to which this node belongs - /// The buffer array. - public void AddColor(TPixel pixel, int colorBits, int level, Octree octree, byte[] buffer) + /// The color to map to. + public void AddColor(TPixel pixel, int colorBits, int level, Octree octree, ref Rgba32 rgba) { // Update the color information if this is a leaf if (this.leaf) { - this.Increment(pixel, buffer); + this.Increment(pixel, ref rgba); // Setup the previous node octree.TrackPrevious(this); @@ -434,11 +468,11 @@ namespace SixLabors.ImageSharp.Quantizers { // Go to the next level down in the tree int shift = 7 - level; - pixel.ToXyzwBytes(buffer, 0); + pixel.ToRgba32(ref rgba); - int index = ((buffer[2] & Mask[level]) >> (shift - 2)) | - ((buffer[1] & Mask[level]) >> (shift - 1)) | - ((buffer[0] & Mask[level]) >> shift); + int index = ((rgba.B & Mask[level]) >> (shift - 2)) | + ((rgba.G & Mask[level]) >> (shift - 1)) | + ((rgba.R & Mask[level]) >> shift); OctreeNode child = this.children[index]; @@ -450,7 +484,7 @@ namespace SixLabors.ImageSharp.Quantizers } // Add the color to the child node - child.AddColor(pixel, colorBits, level + 1, octree, buffer); + child.AddColor(pixel, colorBits, level + 1, octree, ref rgba); } } @@ -524,26 +558,26 @@ namespace SixLabors.ImageSharp.Quantizers /// /// The pixel data. /// The level. - /// The buffer array. + /// The color to map to. /// /// The representing the index of the pixel in the palette. /// - public int GetPaletteIndex(TPixel pixel, int level, byte[] buffer) + public int GetPaletteIndex(TPixel pixel, int level, ref Rgba32 rgba) { int index = this.paletteIndex; if (!this.leaf) { int shift = 7 - level; - pixel.ToXyzwBytes(buffer, 0); + pixel.ToRgba32(ref rgba); - int pixelIndex = ((buffer[2] & Mask[level]) >> (shift - 2)) | - ((buffer[1] & Mask[level]) >> (shift - 1)) | - ((buffer[0] & Mask[level]) >> shift); + int pixelIndex = ((rgba.B & Mask[level]) >> (shift - 2)) | + ((rgba.G & Mask[level]) >> (shift - 1)) | + ((rgba.R & Mask[level]) >> shift); if (this.children[pixelIndex] != null) { - index = this.children[pixelIndex].GetPaletteIndex(pixel, level + 1, buffer); + index = this.children[pixelIndex].GetPaletteIndex(pixel, level + 1, ref rgba); } else { @@ -558,14 +592,14 @@ namespace SixLabors.ImageSharp.Quantizers /// Increment the pixel count and add to the color information /// /// The pixel to add. - /// The buffer array. - public void Increment(TPixel pixel, byte[] buffer) + /// The color to map to. + public void Increment(TPixel pixel, ref Rgba32 rgba) { - pixel.ToXyzwBytes(buffer, 0); + pixel.ToRgba32(ref rgba); this.pixelCount++; - this.red += buffer[0]; - this.green += buffer[1]; - this.blue += buffer[2]; + this.red += rgba.R; + this.green += rgba.G; + this.blue += rgba.B; } } } diff --git a/src/ImageSharp/Quantizers/QuantizerBase{TPixel}.cs b/src/ImageSharp/Quantizers/QuantizerBase{TPixel}.cs index 20ba2e637..d57865c97 100644 --- a/src/ImageSharp/Quantizers/QuantizerBase{TPixel}.cs +++ b/src/ImageSharp/Quantizers/QuantizerBase{TPixel}.cs @@ -40,7 +40,7 @@ namespace SixLabors.ImageSharp.Quantizers.Base } /// - public bool Dither { get; set; } = true; + public bool Dither { get; set; } = false; /// public IErrorDiffuser DitherType { get; set; } = new FloydSteinbergDiffuser(); diff --git a/tests/ImageSharp.Tests/Quantization/QuantizedImageTests.cs b/tests/ImageSharp.Tests/Quantization/QuantizedImageTests.cs new file mode 100644 index 000000000..a0b14b09b --- /dev/null +++ b/tests/ImageSharp.Tests/Quantization/QuantizedImageTests.cs @@ -0,0 +1,95 @@ +namespace SixLabors.ImageSharp.Tests +{ + using SixLabors.ImageSharp.PixelFormats; + using SixLabors.ImageSharp.Quantizers; + + using Xunit; + + public class QuantizedImageTests + { + [Theory] + [WithFile(TestImages.Gif.Giphy, PixelTypes.Rgba32, true)] + [WithFile(TestImages.Gif.Giphy, PixelTypes.Rgba32, false)] + public void PaletteQuantizerYieldsCorrectTransparentPixel(TestImageProvider provider, bool dither) + where TPixel : struct, IPixel + { + using (Image image = provider.GetImage()) + { + Assert.True(image[0, 0].Equals(default(TPixel))); + + IQuantizer quantizer = new PaletteQuantizer { Dither = dither }; + + foreach (ImageFrame frame in image.Frames) + { + QuantizedImage quantized = quantizer.Quantize(frame, 256); + + int index = this.GetTransparentIndex(quantized); + Assert.Equal(index, quantized.Pixels[0]); + } + } + } + + [Theory] + [WithFile(TestImages.Gif.Giphy, PixelTypes.Rgba32, true)] + [WithFile(TestImages.Gif.Giphy, PixelTypes.Rgba32, false)] + public void OctreeQuantizerYieldsCorrectTransparentPixel(TestImageProvider provider, bool dither) + where TPixel : struct, IPixel + { + using (Image image = provider.GetImage()) + { + Assert.True(image[0, 0].Equals(default(TPixel))); + + IQuantizer quantizer = new OctreeQuantizer { Dither = dither }; + + foreach (ImageFrame frame in image.Frames) + { + QuantizedImage quantized = quantizer.Quantize(frame, 256); + + int index = this.GetTransparentIndex(quantized); + Assert.Equal(index, quantized.Pixels[0]); + } + } + } + + [Theory] + [WithFile(TestImages.Gif.Giphy, PixelTypes.Rgba32, true)] + [WithFile(TestImages.Gif.Giphy, PixelTypes.Rgba32, false)] + public void WuQuantizerYieldsCorrectTransparentPixel(TestImageProvider provider, bool dither) + where TPixel : struct, IPixel + { + using (Image image = provider.GetImage()) + { + Assert.True(image[0, 0].Equals(default(TPixel))); + + IQuantizer quantizer = new WuQuantizer { Dither = dither }; + + foreach (ImageFrame frame in image.Frames) + { + QuantizedImage quantized = quantizer.Quantize(frame, 256); + + int index = this.GetTransparentIndex(quantized); + Assert.Equal(index, quantized.Pixels[0]); + } + } + } + + private int GetTransparentIndex(QuantizedImage quantized) + where TPixel : struct, IPixel + { + // Transparent pixels are much more likely to be found at the end of a palette + int index = -1; + var trans = default(Rgba32); + for (int i = quantized.Palette.Length - 1; i >= 0; i--) + { + quantized.Palette[i].ToRgba32(ref trans); + + if (trans.Equals(default(Rgba32))) + { + index = i; + } + } + + return index; + } + } +} \ No newline at end of file From e8dfbccaf9ea1a39e936e4e74bfac21ce7cbfb98 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 9 Oct 2017 00:55:05 +1100 Subject: [PATCH 5/7] Add FrameDecodingMode. Fix #255 --- .../Formats/Gif/FrameDecodingMode.cs | 21 +++++++++++ src/ImageSharp/Formats/Gif/GifDecoder.cs | 5 +++ src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 19 ++++++---- .../Formats/Gif/IGifDecoderOptions.cs | 5 +++ .../Formats/Gif/GifDecoderTests.cs | 37 +++++++++++++++---- 5 files changed, 71 insertions(+), 16 deletions(-) create mode 100644 src/ImageSharp/Formats/Gif/FrameDecodingMode.cs diff --git a/src/ImageSharp/Formats/Gif/FrameDecodingMode.cs b/src/ImageSharp/Formats/Gif/FrameDecodingMode.cs new file mode 100644 index 000000000..05791c92e --- /dev/null +++ b/src/ImageSharp/Formats/Gif/FrameDecodingMode.cs @@ -0,0 +1,21 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + +namespace SixLabors.ImageSharp.Formats.Gif +{ + /// + /// Enumerated frame process modes to apply to multi-frame images. + /// + public enum FrameDecodingMode + { + /// + /// Decodes all the frames of a multi-frame image. + /// + All, + + /// + /// Decodes only the first frame of a multi-frame image. + /// + First + } +} \ No newline at end of file diff --git a/src/ImageSharp/Formats/Gif/GifDecoder.cs b/src/ImageSharp/Formats/Gif/GifDecoder.cs index 5ded251e2..11b5b57fa 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoder.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoder.cs @@ -24,6 +24,11 @@ namespace SixLabors.ImageSharp.Formats.Gif /// public Encoding TextEncoding { get; set; } = GifConstants.DefaultEncoding; + /// + /// Gets or sets the decoding mode for multi-frame images + /// + public FrameDecodingMode DecodingMode { get; set; } = FrameDecodingMode.All; + /// public Image Decode(Configuration configuration, Stream stream) where TPixel : struct, IPixel diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 7fa3aa04a..453197b0c 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -84,6 +84,7 @@ namespace SixLabors.ImageSharp.Formats.Gif { this.TextEncoding = options.TextEncoding ?? GifConstants.DefaultEncoding; this.IgnoreMetadata = options.IgnoreMetadata; + this.DecodingMode = options.DecodingMode; this.configuration = configuration ?? Configuration.Default; } @@ -97,6 +98,11 @@ namespace SixLabors.ImageSharp.Formats.Gif /// public Encoding TextEncoding { get; } + /// + /// Gets the decoding mode for multi-frame images + /// + public FrameDecodingMode DecodingMode { get; } + /// /// Decodes the stream to the image. /// @@ -129,6 +135,11 @@ namespace SixLabors.ImageSharp.Formats.Gif { if (nextFlag == GifConstants.ImageLabel) { + if (this.previousFrame != null && this.DecodingMode == FrameDecodingMode.First) + { + break; + } + this.ReadFrame(); } else if (nextFlag == GifConstants.ExtensionIntroducer) @@ -238,14 +249,6 @@ namespace SixLabors.ImageSharp.Formats.Gif { throw new ImageFormatException($"Invalid gif colormap size '{this.logicalScreenDescriptor.GlobalColorTableSize}'"); } - - /* // No point doing this as the max width/height is always int.Max and that always bigger than the max size of a gif which is stored in a short. - if (this.logicalScreenDescriptor.Width > Image.MaxWidth || this.logicalScreenDescriptor.Height > Image.MaxHeight) - { - throw new ArgumentOutOfRangeException( - $"The input gif '{this.logicalScreenDescriptor.Width}x{this.logicalScreenDescriptor.Height}' is bigger then the max allowed size '{Image.MaxWidth}x{Image.MaxHeight}'"); - } - */ } /// diff --git a/src/ImageSharp/Formats/Gif/IGifDecoderOptions.cs b/src/ImageSharp/Formats/Gif/IGifDecoderOptions.cs index fd4cc82c9..a2288f30a 100644 --- a/src/ImageSharp/Formats/Gif/IGifDecoderOptions.cs +++ b/src/ImageSharp/Formats/Gif/IGifDecoderOptions.cs @@ -19,5 +19,10 @@ namespace SixLabors.ImageSharp.Formats.Gif /// Gets the encoding that should be used when reading comments. /// Encoding TextEncoding { get; } + + /// + /// Gets the decoding mode for multi-frame images + /// + FrameDecodingMode DecodingMode { get; } } } diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index d04c49a98..cd78add75 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. using System.Text; -using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Gif; using SixLabors.ImageSharp.PixelFormats; using SixLabors.Primitives; @@ -45,12 +44,12 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void Decode_IgnoreMetadataIsFalse_CommentsAreRead() { - GifDecoder options = new GifDecoder() + var options = new GifDecoder { IgnoreMetadata = false }; - TestFile testFile = TestFile.Create(TestImages.Gif.Rings); + var testFile = TestFile.Create(TestImages.Gif.Rings); using (Image image = testFile.CreateImage(options)) { @@ -63,12 +62,12 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void Decode_IgnoreMetadataIsTrue_CommentsAreIgnored() { - GifDecoder options = new GifDecoder() + var options = new GifDecoder { IgnoreMetadata = true }; - TestFile testFile = TestFile.Create(TestImages.Gif.Rings); + var testFile = TestFile.Create(TestImages.Gif.Rings); using (Image image = testFile.CreateImage(options)) { @@ -79,12 +78,12 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void Decode_TextEncodingSetToUnicode_TextIsReadWithCorrectEncoding() { - GifDecoder options = new GifDecoder() + var options = new GifDecoder { TextEncoding = Encoding.Unicode }; - TestFile testFile = TestFile.Create(TestImages.Gif.Rings); + var testFile = TestFile.Create(TestImages.Gif.Rings); using (Image image = testFile.CreateImage(options)) { @@ -92,5 +91,27 @@ namespace SixLabors.ImageSharp.Tests Assert.Equal("浉条卥慨灲", image.MetaData.Properties[0].Value); } } + + [Theory] + [WithFile(TestImages.Gif.Giphy, PixelTypes.Rgba32)] + public void CanDecodeJustOneFrame(TestImageProvider provider) + where TPixel : struct, IPixel + { + using (Image image = provider.GetImage(new GifDecoder { DecodingMode = FrameDecodingMode.First })) + { + Assert.Equal(1, image.Frames.Count); + } + } + + [Theory] + [WithFile(TestImages.Gif.Giphy, PixelTypes.Rgba32)] + public void CanDecodeAllFrames(TestImageProvider provider) + where TPixel : struct, IPixel + { + using (Image image = provider.GetImage(new GifDecoder { DecodingMode = FrameDecodingMode.All })) + { + Assert.True(image.Frames.Count > 1); + } + } } -} +} \ No newline at end of file From e53618c1f75017cadda2dd380cffe4cb10e2232f Mon Sep 17 00:00:00 2001 From: JimBobSquarePants Date: Mon, 9 Oct 2017 09:48:45 +1100 Subject: [PATCH 6/7] Add missing unit tests --- .../ImageSharp.Tests/Drawing/DrawImageTest.cs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/ImageSharp.Tests/Drawing/DrawImageTest.cs b/tests/ImageSharp.Tests/Drawing/DrawImageTest.cs index 94dd903b4..53986e6d5 100644 --- a/tests/ImageSharp.Tests/Drawing/DrawImageTest.cs +++ b/tests/ImageSharp.Tests/Drawing/DrawImageTest.cs @@ -9,6 +9,8 @@ using Xunit; namespace SixLabors.ImageSharp.Tests { + using System; + public class DrawImageTest : FileTestBase { private const PixelTypes PixelTypes = Tests.PixelTypes.Rgba32; @@ -42,5 +44,45 @@ namespace SixLabors.ImageSharp.Tests image.DebugSave(provider, new { mode }); } } + + [Theory] + [WithFile(TestImages.Bmp.Car, PixelTypes.Rgba32, TestImages.Png.Splash)] + public void ImageShouldHandleNegativeLocation(TestImageProvider provider, string backgroundPath) + { + using (Image background = TestFile.Create(backgroundPath).CreateImage()) + using (Image overlay = provider.GetImage()) + { + int xy = -25; + Rgba32 backgroundPixel = background[0, 0]; + Rgba32 overlayPixel = overlay[Math.Abs(xy) + 1, Math.Abs(xy) + 1]; + + background.Mutate(x => x.DrawImage(overlay, PixelBlenderMode.Normal, 1F, new Size(overlay.Width, overlay.Height), new Point(xy, xy))); + + Assert.Equal(default(Rgba32), backgroundPixel); + Assert.Equal(overlayPixel, background[0, 0]); + + background.DebugSave(provider, new[] { "Negative" }); + } + } + + [Theory] + [WithFile(TestImages.Bmp.Car, PixelTypes.Rgba32, TestImages.Png.Splash)] + public void ImageShouldHandlePositiveLocation(TestImageProvider provider, string backgroundPath) + { + using (Image background = TestFile.Create(backgroundPath).CreateImage()) + using (Image overlay = provider.GetImage()) + { + int xy = 25; + Rgba32 backgroundPixel = background[xy - 1, xy - 1]; + Rgba32 overlayPixel = overlay[0, 0]; + + background.Mutate(x => x.DrawImage(overlay, PixelBlenderMode.Normal, 1F, new Size(overlay.Width, overlay.Height), new Point(xy, xy))); + + Assert.Equal(default(Rgba32), backgroundPixel); + Assert.Equal(overlayPixel, background[xy, xy]); + + background.DebugSave(provider, new[] { "Positive" }); + } + } } } From ab1dc518b4dbfbc234f4546d05df19771752c876 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 9 Oct 2017 20:55:41 +1100 Subject: [PATCH 7/7] More lightweight tests --- .../ImageSharp.Tests/Drawing/DrawImageTest.cs | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/tests/ImageSharp.Tests/Drawing/DrawImageTest.cs b/tests/ImageSharp.Tests/Drawing/DrawImageTest.cs index 53986e6d5..2e3a730fc 100644 --- a/tests/ImageSharp.Tests/Drawing/DrawImageTest.cs +++ b/tests/ImageSharp.Tests/Drawing/DrawImageTest.cs @@ -46,19 +46,21 @@ namespace SixLabors.ImageSharp.Tests } [Theory] - [WithFile(TestImages.Bmp.Car, PixelTypes.Rgba32, TestImages.Png.Splash)] - public void ImageShouldHandleNegativeLocation(TestImageProvider provider, string backgroundPath) + [WithSolidFilledImages(100, 100, 255, 255, 255, PixelTypes.Rgba32)] + public void ImageShouldHandleNegativeLocation(TestImageProvider provider) { - using (Image background = TestFile.Create(backgroundPath).CreateImage()) - using (Image overlay = provider.GetImage()) + using (Image background = provider.GetImage()) + using (var overlay = new Image(50, 50)) { + overlay.Mutate(x => x.Fill(Rgba32.Black)); + int xy = -25; Rgba32 backgroundPixel = background[0, 0]; Rgba32 overlayPixel = overlay[Math.Abs(xy) + 1, Math.Abs(xy) + 1]; background.Mutate(x => x.DrawImage(overlay, PixelBlenderMode.Normal, 1F, new Size(overlay.Width, overlay.Height), new Point(xy, xy))); - Assert.Equal(default(Rgba32), backgroundPixel); + Assert.Equal(Rgba32.White, backgroundPixel); Assert.Equal(overlayPixel, background[0, 0]); background.DebugSave(provider, new[] { "Negative" }); @@ -66,19 +68,21 @@ namespace SixLabors.ImageSharp.Tests } [Theory] - [WithFile(TestImages.Bmp.Car, PixelTypes.Rgba32, TestImages.Png.Splash)] - public void ImageShouldHandlePositiveLocation(TestImageProvider provider, string backgroundPath) + [WithSolidFilledImages(100, 100, 255, 255, 255, PixelTypes.Rgba32)] + public void ImageShouldHandlePositiveLocation(TestImageProvider provider) { - using (Image background = TestFile.Create(backgroundPath).CreateImage()) - using (Image overlay = provider.GetImage()) + using (Image background = provider.GetImage()) + using (var overlay = new Image(50, 50)) { + overlay.Mutate(x => x.Fill(Rgba32.Black)); + int xy = 25; Rgba32 backgroundPixel = background[xy - 1, xy - 1]; Rgba32 overlayPixel = overlay[0, 0]; background.Mutate(x => x.DrawImage(overlay, PixelBlenderMode.Normal, 1F, new Size(overlay.Width, overlay.Height), new Point(xy, xy))); - Assert.Equal(default(Rgba32), backgroundPixel); + Assert.Equal(Rgba32.White, backgroundPixel); Assert.Equal(overlayPixel, background[xy, xy]); background.DebugSave(provider, new[] { "Positive" });