Fix quantization transparent pixel allocation

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)
+                if (trans.Equals(default(Rgba32)))
-                {
-                    continue;
-                }
-                else
                 {
                     index = i;
-                    break;
                 }
             }
 

src/ImageSharp/Quantizers/OctreeQuantizer{TPixel}.cs

@@ -23,11 +23,6 @@ namespace SixLabors.ImageSharp.Quantizers
         /// </summary>
         private readonly Dictionary<TPixel, byte> colorMap = new Dictionary<TPixel, byte>();
 
-        /// <summary>
-        /// The pixel buffer, used to reduce allocations.
-        /// </summary>
-        private readonly byte[] pixelBuffer = new byte[4];
-
         /// <summary>
         /// Stores the tree
         /// </summary>
@@ -43,6 +38,11 @@ namespace SixLabors.ImageSharp.Quantizers
         /// </summary>
         private TPixel[] palette;
 
+        /// <summary>
+        /// The transparent index
+        /// </summary>
+        private byte transparentIndex;
+
         /// <summary>
         /// Initializes a new instance of the <see cref="OctreeQuantizer{TPixel}"/> class.
         /// </summary>
@@ -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);
         }
 
         /// <inheritdoc/>
         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;
+        }
+
+        /// <summary>
+        /// Returns the index of the first instance of the transparent color in the palette.
+        /// </summary>
+        /// <returns>
+        /// The <see cref="int"/>.
+        /// </returns>
+        [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;
         }
 
         /// <summary>
         /// Process the pixel in the second pass of the algorithm
         /// </summary>
         /// <param name="pixel">The pixel to quantize</param>
+        /// <param name="rgba">The color to compare against</param>
         /// <returns>
         /// The quantized value
         /// </returns>
         [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);
+            pixel.ToRgba32(ref rgba);
-            if (this.pixelBuffer[3] == 0)
+            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);
         }
 
         /// <summary>
@@ -233,8 +267,8 @@ namespace SixLabors.ImageSharp.Quantizers
             /// Add a given color value to the Octree
             /// </summary>
             /// <param name="pixel">The pixel data.</param>
-            /// <param name="buffer">The buffer array.</param>
+            /// <param name="rgba">The color.</param>
-            public void AddColor(TPixel pixel, byte[] buffer)
+            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
             /// </summary>
             /// <param name="pixel">The pixel data.</param>
-            /// <param name="buffer">The buffer array.</param>
+            /// <param name="rgba">The color to map to.</param>
             /// <returns>
             /// The <see cref="int"/>.
             /// </returns>
-            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);
             }
 
             /// <summary>
@@ -415,17 +449,17 @@ namespace SixLabors.ImageSharp.Quantizers
                 /// <summary>
                 /// Add a color into the tree
                 /// </summary>
-                /// <param name="pixel">The color</param>
+                /// <param name="pixel">The pixel color</param>
                 /// <param name="colorBits">The number of significant color bits</param>
                 /// <param name="level">The level in the tree</param>
                 /// <param name="octree">The tree to which this node belongs</param>
-                /// <param name="buffer">The buffer array.</param>
+                /// <param name="rgba">The color to map to.</param>
-                public void AddColor(TPixel pixel, int colorBits, int level, Octree octree, byte[] buffer)
+                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)) |
+                        int index = ((rgba.B & Mask[level]) >> (shift - 2)) |
-                                    ((buffer[1] & Mask[level]) >> (shift - 1)) |
+                                    ((rgba.G & Mask[level]) >> (shift - 1)) |
-                                    ((buffer[0] & Mask[level]) >> shift);
+                                    ((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
                 /// </summary>
                 /// <param name="pixel">The pixel data.</param>
                 /// <param name="level">The level.</param>
-                /// <param name="buffer">The buffer array.</param>
+                /// <param name="rgba">The color to map to.</param>
                 /// <returns>
                 /// The <see cref="int"/> representing the index of the pixel in the palette.
                 /// </returns>
-                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)) |
+                        int pixelIndex = ((rgba.B & Mask[level]) >> (shift - 2)) |
-                                         ((buffer[1] & Mask[level]) >> (shift - 1)) |
+                                         ((rgba.G & Mask[level]) >> (shift - 1)) |
-                                         ((buffer[0] & Mask[level]) >> shift);
+                                         ((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
                 /// </summary>
                 /// <param name="pixel">The pixel to add.</param>
-                /// <param name="buffer">The buffer array.</param>
+                /// <param name="rgba">The color to map to.</param>
-                public void Increment(TPixel pixel, byte[] buffer)
+                public void Increment(TPixel pixel, ref Rgba32 rgba)
                 {
-                    pixel.ToXyzwBytes(buffer, 0);
+                    pixel.ToRgba32(ref rgba);
                     this.pixelCount++;
-                    this.red += buffer[0];
+                    this.red += rgba.R;
-                    this.green += buffer[1];
+                    this.green += rgba.G;
-                    this.blue += buffer[2];
+                    this.blue += rgba.B;
                 }
             }
         }

src/ImageSharp/Quantizers/QuantizerBase{TPixel}.cs

@@ -40,7 +40,7 @@ namespace SixLabors.ImageSharp.Quantizers.Base
         }
 
         /// <inheritdoc />
-        public bool Dither { get; set; } = true;
+        public bool Dither { get; set; } = false;
 
         /// <inheritdoc />
         public IErrorDiffuser DitherType { get; set; } = new FloydSteinbergDiffuser();

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<TPixel>(TestImageProvider<TPixel> provider, bool dither)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage())
+            {
+                Assert.True(image[0, 0].Equals(default(TPixel)));
+
+                IQuantizer<TPixel> quantizer = new PaletteQuantizer<TPixel> { Dither = dither };
+
+                foreach (ImageFrame<TPixel> frame in image.Frames)
+                {
+                    QuantizedImage<TPixel> 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<TPixel>(TestImageProvider<TPixel> provider, bool dither)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage())
+            {
+                Assert.True(image[0, 0].Equals(default(TPixel)));
+
+                IQuantizer<TPixel> quantizer = new OctreeQuantizer<TPixel> { Dither = dither };
+
+                foreach (ImageFrame<TPixel> frame in image.Frames)
+                {
+                    QuantizedImage<TPixel> 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<TPixel>(TestImageProvider<TPixel> provider, bool dither)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage())
+            {
+                Assert.True(image[0, 0].Equals(default(TPixel)));
+
+                IQuantizer<TPixel> quantizer = new WuQuantizer<TPixel> { Dither = dither };
+
+                foreach (ImageFrame<TPixel> frame in image.Frames)
+                {
+                    QuantizedImage<TPixel> quantized = quantizer.Quantize(frame, 256);
+
+                    int index = this.GetTransparentIndex(quantized);
+                    Assert.Equal(index, quantized.Pixels[0]);
+                }
+            }
+        }
+
+        private int GetTransparentIndex<TPixel>(QuantizedImage<TPixel> quantized)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            // 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;
+        }
+    }
+}