Merge pull request #1672 from SixLabors/js/gifoptimize

Allow clearing and reusing a pixel map.

src/ImageSharp/Formats/Gif/GifEncoderCore.cs

@@ -7,7 +7,6 @@ using System.IO;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using System.Threading;
-using System.Threading.Tasks;
 using SixLabors.ImageSharp.Advanced;
 using SixLabors.ImageSharp.Memory;
 using SixLabors.ImageSharp.Metadata;
@@ -54,7 +53,7 @@ namespace SixLabors.ImageSharp.Formats.Gif
         /// <summary>
         /// The pixel sampling strategy for global quantization.
         /// </summary>
-        private IPixelSamplingStrategy pixelSamplingStrategy;
+        private readonly IPixelSamplingStrategy pixelSamplingStrategy;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="GifEncoderCore"/> class.
@@ -150,8 +149,8 @@ namespace SixLabors.ImageSharp.Formats.Gif
             // The palette quantizer can reuse the same pixel map across multiple frames
             // since the palette is unchanging. This allows a reduction of memory usage across
             // multi frame gifs using a global palette.
-            EuclideanPixelMap<TPixel> pixelMap = default;
-            bool pixelMapHasValue = false;
+            PaletteQuantizer<TPixel> paletteFrameQuantizer = default;
+            bool quantizerInitialized = false;
             for (int i = 0; i < image.Frames.Count; i++)
             {
                 ImageFrame<TPixel> frame = image.Frames[i];
@@ -166,22 +165,18 @@ namespace SixLabors.ImageSharp.Formats.Gif
                 }
                 else
                 {
-                    if (!pixelMapHasValue)
+                    if (!quantizerInitialized)
                     {
-                        pixelMapHasValue = true;
-                        pixelMap = new EuclideanPixelMap<TPixel>(this.configuration, quantized.Palette);
+                        quantizerInitialized = true;
+                        paletteFrameQuantizer = new PaletteQuantizer<TPixel>(this.configuration, this.quantizer.Options, quantized.Palette);
                     }
 
-                    using var paletteFrameQuantizer = new PaletteQuantizer<TPixel>(this.configuration, this.quantizer.Options, pixelMap, true);
                     using IndexedImageFrame<TPixel> paletteQuantized = paletteFrameQuantizer.QuantizeFrame(frame, frame.Bounds());
                     this.WriteImageData(paletteQuantized, stream);
                 }
             }
 
-            if (pixelMapHasValue)
-            {
-                pixelMap.Dispose();
-            }
+            paletteFrameQuantizer.Dispose();
         }
 
         private void EncodeLocal<TPixel>(Image<TPixel> image, IndexedImageFrame<TPixel> quantized, Stream stream)
@@ -310,7 +305,7 @@ namespace SixLabors.ImageSharp.Formats.Gif
                     }
                     else
                     {
-                        ratio = (byte)(((1 / vr) * 64) - 15);
+                        ratio = (byte)((1 / vr * 64) - 15);
                     }
                 }
             }
@@ -354,7 +349,7 @@ namespace SixLabors.ImageSharp.Formats.Gif
                 return;
             }
 
-            for (var i = 0; i < metadata.Comments.Count; i++)
+            for (int i = 0; i < metadata.Comments.Count; i++)
             {
                 string comment = metadata.Comments[i];
                 this.buffer[0] = GifConstants.ExtensionIntroducer;

src/ImageSharp/Processing/Processors/Quantization/EuclideanPixelMap{TPixel}.cs

@@ -18,20 +18,21 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
     /// This class is not threadsafe and should not be accessed in parallel.
     /// Doing so will result in non-idempotent results.
     /// </para>
-    internal readonly struct EuclideanPixelMap<TPixel> : IDisposable
+    internal sealed class EuclideanPixelMap<TPixel> : IDisposable
         where TPixel : unmanaged, IPixel<TPixel>
     {
-        private readonly Rgba32[] rgbaPalette;
+        private Rgba32[] rgbaPalette;
         private readonly ColorDistanceCache cache;
+        private readonly Configuration configuration;
 
         /// <summary>
-        /// Initializes a new instance of the <see cref="EuclideanPixelMap{TPixel}"/> struct.
+        /// Initializes a new instance of the <see cref="EuclideanPixelMap{TPixel}"/> class.
         /// </summary>
         /// <param name="configuration">The configuration.</param>
         /// <param name="palette">The color palette to map from.</param>
-        [MethodImpl(InliningOptions.ShortMethod)]
         public EuclideanPixelMap(Configuration configuration, ReadOnlyMemory<TPixel> palette)
         {
+            this.configuration = configuration;
             this.Palette = palette;
             this.rgbaPalette = new Rgba32[palette.Length];
             this.cache = new ColorDistanceCache(configuration.MemoryAllocator);
@@ -46,6 +47,9 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
         {
             [MethodImpl(InliningOptions.ShortMethod)]
             get;
+
+            [MethodImpl(InliningOptions.ShortMethod)]
+            private set;
         }
 
         /// <summary>
@@ -72,6 +76,18 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
             return index;
         }
 
+        /// <summary>
+        /// Clears the map, resetting it to use the given palette.
+        /// </summary>
+        /// <param name="palette">The color palette to map from.</param>
+        public void Clear(ReadOnlyMemory<TPixel> palette)
+        {
+            this.Palette = palette;
+            this.rgbaPalette = new Rgba32[palette.Length];
+            PixelOperations<TPixel>.Instance.ToRgba32(this.configuration, this.Palette.Span, this.rgbaPalette);
+            this.cache.Clear();
+        }
+
         [MethodImpl(InliningOptions.ShortMethod)]
         private int GetClosestColorSlow(Rgba32 rgba, ref TPixel paletteRef, out TPixel match)
         {
@@ -177,6 +193,12 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
                 return match > -1;
             }
 
+            /// <summary>
+            /// Clears the cache resetting each entry to empty.
+            /// </summary>
+            [MethodImpl(InliningOptions.ShortMethod)]
+            public void Clear() => this.table.GetSpan().Fill(-1);
+
             [MethodImpl(InliningOptions.ShortMethod)]
             private static int GetPaletteIndex(int r, int g, int b, int a)
                 => (r << ((IndexBits << 1) + IndexAlphaBits))

src/ImageSharp/Processing/Processors/Quantization/OctreeQuantizer{TPixel}.cs

@@ -25,7 +25,6 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
         private IMemoryOwner<TPixel> paletteOwner;
         private ReadOnlyMemory<TPixel> palette;
         private EuclideanPixelMap<TPixel> pixelMap;
-        private bool pixelMapHasValue;
         private readonly bool isDithering;
         private bool isDisposed;
 
@@ -48,7 +47,6 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
             this.octree = new Octree(this.bitDepth);
             this.paletteOwner = configuration.MemoryAllocator.Allocate<TPixel>(this.maxColors, AllocationOptions.Clean);
             this.pixelMap = default;
-            this.pixelMapHasValue = false;
             this.palette = default;
             this.isDithering = !(this.Options.Dither is null);
             this.isDisposed = false;
@@ -112,15 +110,17 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
             this.octree.Palletize(paletteSpan, max, ref paletteIndex);
             ReadOnlyMemory<TPixel> result = this.paletteOwner.Memory.Slice(0, paletteSpan.Length);
 
-            // When called by QuantizerUtilities.BuildPalette this prevents
-            // mutiple instances of the map being created but not disposed.
-            if (this.pixelMapHasValue)
+            // When called multiple times by QuantizerUtilities.BuildPalette
+            // this prevents memory churn caused by reallocation.
+            if (this.pixelMap is null)
             {
-                this.pixelMap.Dispose();
+                this.pixelMap = new EuclideanPixelMap<TPixel>(this.Configuration, result);
+            }
+            else
+            {
+                this.pixelMap.Clear(result);
             }
 
-            this.pixelMap = new EuclideanPixelMap<TPixel>(this.Configuration, result);
-            this.pixelMapHasValue = true;
             this.palette = result;
         }
 
@@ -153,9 +153,10 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
             if (!this.isDisposed)
             {
                 this.isDisposed = true;
-                this.paletteOwner.Dispose();
+                this.paletteOwner?.Dispose();
                 this.paletteOwner = null;
-                this.pixelMap.Dispose();
+                this.pixelMap?.Dispose();
+                this.pixelMap = null;
             }
         }
 

src/ImageSharp/Processing/Processors/Quantization/PaletteQuantizer.cs

@@ -58,9 +58,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
             var palette = new TPixel[length];
 
             Color.ToPixel(configuration, this.colorPalette.Span, palette.AsSpan());
-
-            var pixelMap = new EuclideanPixelMap<TPixel>(configuration, palette);
-            return new PaletteQuantizer<TPixel>(configuration, options, pixelMap, false);
+            return new PaletteQuantizer<TPixel>(configuration, options, palette);
         }
     }
 }

src/ImageSharp/Processing/Processors/Quantization/PaletteQuantizer{TPixel}.cs

@@ -16,32 +16,23 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
     internal struct PaletteQuantizer<TPixel> : IQuantizer<TPixel>
         where TPixel : unmanaged, IPixel<TPixel>
     {
-        private readonly EuclideanPixelMap<TPixel> pixelMap;
-        private readonly bool leaveMap;
+        private EuclideanPixelMap<TPixel> pixelMap;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="PaletteQuantizer{TPixel}"/> struct.
         /// </summary>
         /// <param name="configuration">The configuration which allows altering default behaviour or extending the library.</param>
         /// <param name="options">The quantizer options defining quantization rules.</param>
-        /// <param name="pixelMap">The pixel map for looking up color matches from a predefined palette.</param>
-        /// <param name="leaveMap">
-        /// <see langword="true"/> to leave the pixel map undisposed after disposing the <see cref="PaletteQuantizer{TPixel}"/> object; otherwise, <see langword="false"/>.
-        /// </param>
+        /// <param name="palette">The palette to use.</param>
         [MethodImpl(InliningOptions.ShortMethod)]
-        public PaletteQuantizer(
-            Configuration configuration,
-            QuantizerOptions options,
-            EuclideanPixelMap<TPixel> pixelMap,
-            bool leaveMap)
+        public PaletteQuantizer(Configuration configuration, QuantizerOptions options, ReadOnlyMemory<TPixel> palette)
         {
             Guard.NotNull(configuration, nameof(configuration));
             Guard.NotNull(options, nameof(options));
 
             this.Configuration = configuration;
             this.Options = options;
-            this.pixelMap = pixelMap;
-            this.leaveMap = leaveMap;
+            this.pixelMap = new EuclideanPixelMap<TPixel>(configuration, palette);
         }
 
         /// <inheritdoc/>
@@ -72,10 +63,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
         /// <inheritdoc/>
         public void Dispose()
         {
-            if (!this.leaveMap)
-            {
-                this.pixelMap.Dispose();
-            }
+            this.pixelMap?.Dispose();
+            this.pixelMap = null;
         }
     }
 }

src/ImageSharp/Processing/Processors/Quantization/WuQuantizer{TPixel}.cs

@@ -72,7 +72,6 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
         private int maxColors;
         private readonly Box[] colorCube;
         private EuclideanPixelMap<TPixel> pixelMap;
-        private bool pixelMapHasValue;
         private readonly bool isDithering;
         private bool isDisposed;
 
@@ -97,7 +96,6 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
             this.colorCube = new Box[this.maxColors];
             this.isDisposed = false;
             this.pixelMap = default;
-            this.pixelMapHasValue = false;
             this.palette = default;
             this.isDithering = this.isDithering = !(this.Options.Dither is null);
         }
@@ -147,15 +145,16 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
             ReadOnlyMemory<TPixel> result = this.paletteOwner.Memory.Slice(0, paletteSpan.Length);
             if (this.isDithering)
             {
-                // When called by QuantizerUtilities.BuildPalette this prevents
-                // mutiple instances of the map being created but not disposed.
-                if (this.pixelMapHasValue)
+                // When called multiple times by QuantizerUtilities.BuildPalette
+                // this prevents memory churn caused by reallocation.
+                if (this.pixelMap is null)
                 {
-                    this.pixelMap.Dispose();
+                    this.pixelMap = new EuclideanPixelMap<TPixel>(this.Configuration, result);
+                }
+                else
+                {
+                    this.pixelMap.Clear(result);
                 }
-
-                this.pixelMap = new EuclideanPixelMap<TPixel>(this.Configuration, result);
-                this.pixelMapHasValue = true;
             }
 
             this.palette = result;
@@ -201,7 +200,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
                 this.momentsOwner = null;
                 this.tagsOwner = null;
                 this.paletteOwner = null;
-                this.pixelMap.Dispose();
+                this.pixelMap?.Dispose();
+                this.pixelMap = null;
             }
         }
 
@@ -215,16 +215,14 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
         /// <returns>The index.</returns>
         [MethodImpl(InliningOptions.ShortMethod)]
         private static int GetPaletteIndex(int r, int g, int b, int a)
-        {
-            return (r << ((IndexBits * 2) + IndexAlphaBits))
-                + (r << (IndexBits + IndexAlphaBits + 1))
-                + (g << (IndexBits + IndexAlphaBits))
-                + (r << (IndexBits * 2))
-                + (r << (IndexBits + 1))
-                + (g << IndexBits)
-                + ((r + g + b) << IndexAlphaBits)
-                + r + g + b + a;
-        }
+            => (r << ((IndexBits * 2) + IndexAlphaBits))
+            + (r << (IndexBits + IndexAlphaBits + 1))
+            + (g << (IndexBits + IndexAlphaBits))
+            + (r << (IndexBits * 2))
+            + (r << (IndexBits + 1))
+            + (g << IndexBits)
+            + ((r + g + b) << IndexAlphaBits)
+            + r + g + b + a;
 
         /// <summary>
         /// Computes sum over a box of any given statistic.
@@ -233,24 +231,22 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
         /// <param name="moments">The moment.</param>
         /// <returns>The result.</returns>
         private static Moment Volume(ref Box cube, ReadOnlySpan<Moment> moments)
-        {
-            return moments[GetPaletteIndex(cube.RMax, cube.GMax, cube.BMax, cube.AMax)]
-                 - moments[GetPaletteIndex(cube.RMax, cube.GMax, cube.BMax, cube.AMin)]
-                 - moments[GetPaletteIndex(cube.RMax, cube.GMax, cube.BMin, cube.AMax)]
-                 + moments[GetPaletteIndex(cube.RMax, cube.GMax, cube.BMin, cube.AMin)]
-                 - moments[GetPaletteIndex(cube.RMax, cube.GMin, cube.BMax, cube.AMax)]
-                 + moments[GetPaletteIndex(cube.RMax, cube.GMin, cube.BMax, cube.AMin)]
-                 + moments[GetPaletteIndex(cube.RMax, cube.GMin, cube.BMin, cube.AMax)]
-                 - moments[GetPaletteIndex(cube.RMax, cube.GMin, cube.BMin, cube.AMin)]
-                 - moments[GetPaletteIndex(cube.RMin, cube.GMax, cube.BMax, cube.AMax)]
-                 + moments[GetPaletteIndex(cube.RMin, cube.GMax, cube.BMax, cube.AMin)]
-                 + moments[GetPaletteIndex(cube.RMin, cube.GMax, cube.BMin, cube.AMax)]
-                 - moments[GetPaletteIndex(cube.RMin, cube.GMax, cube.BMin, cube.AMin)]
-                 + moments[GetPaletteIndex(cube.RMin, cube.GMin, cube.BMax, cube.AMax)]
-                 - moments[GetPaletteIndex(cube.RMin, cube.GMin, cube.BMax, cube.AMin)]
-                 - moments[GetPaletteIndex(cube.RMin, cube.GMin, cube.BMin, cube.AMax)]
-                 + moments[GetPaletteIndex(cube.RMin, cube.GMin, cube.BMin, cube.AMin)];
-        }
+            => moments[GetPaletteIndex(cube.RMax, cube.GMax, cube.BMax, cube.AMax)]
+            - moments[GetPaletteIndex(cube.RMax, cube.GMax, cube.BMax, cube.AMin)]
+            - moments[GetPaletteIndex(cube.RMax, cube.GMax, cube.BMin, cube.AMax)]
+            + moments[GetPaletteIndex(cube.RMax, cube.GMax, cube.BMin, cube.AMin)]
+            - moments[GetPaletteIndex(cube.RMax, cube.GMin, cube.BMax, cube.AMax)]
+            + moments[GetPaletteIndex(cube.RMax, cube.GMin, cube.BMax, cube.AMin)]
+            + moments[GetPaletteIndex(cube.RMax, cube.GMin, cube.BMin, cube.AMax)]
+            - moments[GetPaletteIndex(cube.RMax, cube.GMin, cube.BMin, cube.AMin)]
+            - moments[GetPaletteIndex(cube.RMin, cube.GMax, cube.BMax, cube.AMax)]
+            + moments[GetPaletteIndex(cube.RMin, cube.GMax, cube.BMax, cube.AMin)]
+            + moments[GetPaletteIndex(cube.RMin, cube.GMax, cube.BMin, cube.AMax)]
+            - moments[GetPaletteIndex(cube.RMin, cube.GMax, cube.BMin, cube.AMin)]
+            + moments[GetPaletteIndex(cube.RMin, cube.GMin, cube.BMax, cube.AMax)]
+            - moments[GetPaletteIndex(cube.RMin, cube.GMin, cube.BMax, cube.AMin)]
+            - moments[GetPaletteIndex(cube.RMin, cube.GMin, cube.BMin, cube.AMax)]
+            + moments[GetPaletteIndex(cube.RMin, cube.GMin, cube.BMin, cube.AMin)];
 
         /// <summary>
         /// Computes part of Volume(cube, moment) that doesn't depend on RMax, GMax, BMax, or AMax (depending on direction).
@@ -835,7 +831,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
             public int Volume;
 
             /// <inheritdoc/>
-            public readonly override bool Equals(object obj)
+            public override readonly bool Equals(object obj)
                 => obj is Box box
                 && this.Equals(box);
 
@@ -852,7 +848,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Quantization
                 && this.Volume == other.Volume;
 
             /// <inheritdoc/>
-            public readonly override int GetHashCode()
+            public override readonly int GetHashCode()
             {
                 HashCode hash = default;
                 hash.Add(this.RMin);