Fix boxed quantization and update refs

SixLabors · Nov 16, 2023 · 8d7ac6e · 8d7ac6e
1 parent afe2133
commit 8d7ac6e
Show file tree

Hide file tree

Showing 29 changed files with 89 additions and 114 deletions.
diff --git a/src/ImageSharp/Processing/Processors/Quantization/OctreeQuantizer{TPixel}.cs b/src/ImageSharp/Processing/Processors/Quantization/OctreeQuantizer{TPixel}.cs
@@ -60,7 +60,7 @@ public OctreeQuantizer(Configuration configuration, QuantizerOptions options)
     public QuantizerOptions Options { get; }
 
     /// <inheritdoc/>
-    public ReadOnlyMemory<TPixel> Palette
+    public readonly ReadOnlyMemory<TPixel> Palette
     {
         get
         {
@@ -72,16 +72,14 @@ public ReadOnlyMemory<TPixel> Palette
     /// <inheritdoc/>
     public void AddPaletteColors(Buffer2DRegion<TPixel> pixelRegion)
     {
-        Rectangle bounds = pixelRegion.Rectangle;
-        Buffer2D<TPixel> source = pixelRegion.Buffer;
-        using (IMemoryOwner<Rgba32> buffer = this.Configuration.MemoryAllocator.Allocate<Rgba32>(bounds.Width))
+        using (IMemoryOwner<Rgba32> buffer = this.Configuration.MemoryAllocator.Allocate<Rgba32>(pixelRegion.Width))
         {
             Span<Rgba32> bufferSpan = buffer.GetSpan();
 
             // Loop through each row
-            for (int y = bounds.Top; y < bounds.Bottom; y++)
+            for (int y = 0; y < pixelRegion.Height; y++)
             {
-                Span<TPixel> row = source.DangerousGetRowSpan(y).Slice(bounds.Left, bounds.Width);
+                Span<TPixel> row = pixelRegion.DangerousGetRowSpan(y);
                 PixelOperations<TPixel>.Instance.ToRgba32(this.Configuration, row, bufferSpan);
 
                 for (int x = 0; x < bufferSpan.Length; x++)

diff --git a/src/ImageSharp/Processing/Processors/Quantization/QuantizeProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Quantization/QuantizeProcessor{TPixel}.cs
@@ -32,7 +32,7 @@ public QuantizeProcessor(Configuration configuration, IQuantizer quantizer, Imag
     /// <inheritdoc />
     protected override void OnFrameApply(ImageFrame<TPixel> source)
     {
-        var interest = Rectangle.Intersect(source.Bounds(), this.SourceRectangle);
+        Rectangle interest = Rectangle.Intersect(source.Bounds(), this.SourceRectangle);
 
         Configuration configuration = this.Configuration;
         using IQuantizer<TPixel> frameQuantizer = this.quantizer.CreatePixelSpecificQuantizer<TPixel>(configuration);
@@ -43,14 +43,14 @@ protected override void OnFrameApply(ImageFrame<TPixel> source)
         int offsetX = interest.Left;
         Buffer2D<TPixel> sourceBuffer = source.PixelBuffer;
 
-        for (int y = interest.Y; y < interest.Height; y++)
+        for (int y = 0; y < quantized.Height; y++)
         {
-            Span<TPixel> row = sourceBuffer.DangerousGetRowSpan(y);
-            ReadOnlySpan<byte> quantizedRow = quantized.DangerousGetRowSpan(y - offsetY);
+            ReadOnlySpan<byte> quantizedRow = quantized.DangerousGetRowSpan(y);
+            Span<TPixel> row = sourceBuffer.DangerousGetRowSpan(y + offsetY);
 
-            for (int x = interest.Left; x < interest.Right; x++)
+            for (int x = 0; x < quantized.Width; x++)
             {
-                row[x] = paletteSpan[quantizedRow[x - offsetX]];
+                row[x + offsetX] = paletteSpan[quantizedRow[x]];
             }
         }
     }

diff --git a/src/ImageSharp/Processing/Processors/Quantization/QuantizerUtilities.cs b/src/ImageSharp/Processing/Processors/Quantization/QuantizerUtilities.cs
@@ -154,14 +154,14 @@ private static void SecondPass<TFrameQuantizer, TPixel>(
             int offsetY = bounds.Top;
             int offsetX = bounds.Left;
 
-            for (int y = bounds.Y; y < bounds.Height; y++)
+            for (int y = 0; y < destination.Height; y++)
             {
-                Span<TPixel> sourceRow = sourceBuffer.DangerousGetRowSpan(y);
-                Span<byte> destinationRow = destination.GetWritablePixelRowSpanUnsafe(y - offsetY);
+                Span<TPixel> sourceRow = sourceBuffer.DangerousGetRowSpan(y + offsetY);
+                Span<byte> destinationRow = destination.GetWritablePixelRowSpanUnsafe(y);
 
-                for (int x = bounds.Left; x < bounds.Right; x++)
+                for (int x = 0; x < destination.Width; x++)
                 {
-                    destinationRow[x - offsetX] = Unsafe.AsRef(quantizer).GetQuantizedColor(sourceRow[x], out TPixel _);
+                    destinationRow[x] = Unsafe.AsRef(quantizer).GetQuantizedColor(sourceRow[x + offsetX], out TPixel _);
                 }
             }
 

diff --git a/src/ImageSharp/Processing/Processors/Quantization/WuQuantizer{TPixel}.cs b/src/ImageSharp/Processing/Processors/Quantization/WuQuantizer{TPixel}.cs
@@ -123,10 +123,11 @@ public readonly ReadOnlyMemory<TPixel> Palette
     /// <inheritdoc/>
     public void AddPaletteColors(Buffer2DRegion<TPixel> pixelRegion)
     {
-        Rectangle bounds = pixelRegion.Rectangle;
-        Buffer2D<TPixel> source = pixelRegion.Buffer;
-
-        this.Build3DHistogram(source, bounds);
+        // TODO: Something is destroying the existing palette when adding new colors.
+        // When the QuantizingImageEncoder.PixelSamplingStrategy is DefaultPixelSamplingStrategy
+        // this leads to performance issues + the palette is not preserved.
+        // https://github.com/SixLabors/ImageSharp/issues/2498
+        this.Build3DHistogram(pixelRegion);
         this.Get3DMoments(this.memoryAllocator);
         this.BuildCube();
 
@@ -360,19 +361,18 @@ private static Moment Top(ref Box cube, int direction, int position, ReadOnlySpa
     /// <summary>
     /// Builds a 3-D color histogram of <c>counts, r/g/b, c^2</c>.
     /// </summary>
-    /// <param name="source">The source data.</param>
-    /// <param name="bounds">The bounds within the source image to quantize.</param>
-    private readonly void Build3DHistogram(Buffer2D<TPixel> source, Rectangle bounds)
+    /// <param name="source">The source pixel data.</param>
+    private readonly void Build3DHistogram(Buffer2DRegion<TPixel> source)
     {
         Span<Moment> momentSpan = this.momentsOwner.GetSpan();
 
         // Build up the 3-D color histogram
-        using IMemoryOwner<Rgba32> buffer = this.memoryAllocator.Allocate<Rgba32>(bounds.Width);
+        using IMemoryOwner<Rgba32> buffer = this.memoryAllocator.Allocate<Rgba32>(source.Width);
         Span<Rgba32> bufferSpan = buffer.GetSpan();
 
-        for (int y = bounds.Top; y < bounds.Bottom; y++)
+        for (int y = 0; y < source.Height; y++)
         {
-            Span<TPixel> row = source.DangerousGetRowSpan(y).Slice(bounds.Left, bounds.Width);
+            Span<TPixel> row = source.DangerousGetRowSpan(y);
             PixelOperations<TPixel>.Instance.ToRgba32(this.Configuration, row, bufferSpan);
 
             for (int x = 0; x < bufferSpan.Length; x++)

diff --git a/tests/ImageSharp.Tests/Processing/Processors/Quantization/QuantizerTests.cs b/tests/ImageSharp.Tests/Processing/Processors/Quantization/QuantizerTests.cs
@@ -12,74 +12,66 @@ namespace SixLabors.ImageSharp.Tests.Processing.Processors.Quantization;
 [Trait("Category", "Processors")]
 public class QuantizerTests
 {
-    /// <summary>
-    /// Something is causing tests to fail on NETFX in CI.
-    /// Could be a JIT error as everything runs well and is identical to .NET Core output.
-    /// Not worth investigating for now.
-    /// <see href="https://github.com/SixLabors/ImageSharp/pull/1114/checks?check_run_id=448891164#step:11:631"/>
-    /// </summary>
-    private static readonly bool SkipAllQuantizerTests = TestEnvironment.IsFramework;
-
     public static readonly string[] CommonTestImages =
     {
         TestImages.Png.CalliphoraPartial,
         TestImages.Png.Bike
     };
 
-    private static readonly QuantizerOptions NoDitherOptions = new QuantizerOptions { Dither = null };
-    private static readonly QuantizerOptions DiffuserDitherOptions = new QuantizerOptions { Dither = KnownDitherings.FloydSteinberg };
-    private static readonly QuantizerOptions OrderedDitherOptions = new QuantizerOptions { Dither = KnownDitherings.Bayer8x8 };
+    private static readonly QuantizerOptions NoDitherOptions = new() { Dither = null };
+    private static readonly QuantizerOptions DiffuserDitherOptions = new() { Dither = KnownDitherings.FloydSteinberg };
+    private static readonly QuantizerOptions OrderedDitherOptions = new() { Dither = KnownDitherings.Bayer8x8 };
 
-    private static readonly QuantizerOptions Diffuser0_ScaleDitherOptions = new QuantizerOptions
+    private static readonly QuantizerOptions Diffuser0_ScaleDitherOptions = new()
     {
         Dither = KnownDitherings.FloydSteinberg,
         DitherScale = 0F
     };
 
-    private static readonly QuantizerOptions Diffuser0_25_ScaleDitherOptions = new QuantizerOptions
+    private static readonly QuantizerOptions Diffuser0_25_ScaleDitherOptions = new()
     {
         Dither = KnownDitherings.FloydSteinberg,
         DitherScale = .25F
     };
 
-    private static readonly QuantizerOptions Diffuser0_5_ScaleDitherOptions = new QuantizerOptions
+    private static readonly QuantizerOptions Diffuser0_5_ScaleDitherOptions = new()
     {
         Dither = KnownDitherings.FloydSteinberg,
         DitherScale = .5F
     };
 
-    private static readonly QuantizerOptions Diffuser0_75_ScaleDitherOptions = new QuantizerOptions
+    private static readonly QuantizerOptions Diffuser0_75_ScaleDitherOptions = new()
     {
         Dither = KnownDitherings.FloydSteinberg,
         DitherScale = .75F
     };
 
-    private static readonly QuantizerOptions Ordered0_ScaleDitherOptions = new QuantizerOptions
+    private static readonly QuantizerOptions Ordered0_ScaleDitherOptions = new()
     {
         Dither = KnownDitherings.Bayer8x8,
         DitherScale = 0F
     };
 
-    private static readonly QuantizerOptions Ordered0_25_ScaleDitherOptions = new QuantizerOptions
+    private static readonly QuantizerOptions Ordered0_25_ScaleDitherOptions = new()
     {
         Dither = KnownDitherings.Bayer8x8,
         DitherScale = .25F
     };
 
-    private static readonly QuantizerOptions Ordered0_5_ScaleDitherOptions = new QuantizerOptions
+    private static readonly QuantizerOptions Ordered0_5_ScaleDitherOptions = new()
     {
         Dither = KnownDitherings.Bayer8x8,
         DitherScale = .5F
     };
 
-    private static readonly QuantizerOptions Ordered0_75_ScaleDitherOptions = new QuantizerOptions
+    private static readonly QuantizerOptions Ordered0_75_ScaleDitherOptions = new()
     {
         Dither = KnownDitherings.Bayer8x8,
         DitherScale = .75F
     };
 
     public static readonly TheoryData<IQuantizer> Quantizers
-    = new TheoryData<IQuantizer>
+    = new()
     {
         // Known uses error diffusion by default.
         KnownQuantizers.Octree,
@@ -97,7 +89,7 @@ public static readonly TheoryData<IQuantizer> Quantizers
     };
 
     public static readonly TheoryData<IQuantizer> DitherScaleQuantizers
-    = new TheoryData<IQuantizer>
+    = new()
     {
         new OctreeQuantizer(Diffuser0_ScaleDitherOptions),
         new WebSafePaletteQuantizer(Diffuser0_ScaleDitherOptions),
@@ -151,7 +143,7 @@ public static readonly TheoryData<IQuantizer> DitherScaleQuantizers
     };
 
     public static readonly TheoryData<IDither> DefaultInstanceDitherers
-        = new TheoryData<IDither>
+        = new()
         {
             default(ErrorDither),
             default(OrderedDither)
@@ -164,11 +156,6 @@ public static readonly TheoryData<IDither> DefaultInstanceDitherers
     public void ApplyQuantizationInBox<TPixel>(TestImageProvider<TPixel> provider, IQuantizer quantizer)
         where TPixel : unmanaged, IPixel<TPixel>
     {
-        if (SkipAllQuantizerTests)
-        {
-            return;
-        }
-
         string quantizerName = quantizer.GetType().Name;
         string ditherName = quantizer.Options.Dither?.GetType()?.Name ?? "NoDither";
         string testOutputDetails = $"{quantizerName}_{ditherName}";
@@ -185,11 +172,6 @@ public void ApplyQuantizationInBox<TPixel>(TestImageProvider<TPixel> provider, I
     public void ApplyQuantization<TPixel>(TestImageProvider<TPixel> provider, IQuantizer quantizer)
         where TPixel : unmanaged, IPixel<TPixel>
     {
-        if (SkipAllQuantizerTests)
-        {
-            return;
-        }
-
         string quantizerName = quantizer.GetType().Name;
         string ditherName = quantizer.Options.Dither?.GetType()?.Name ?? "NoDither";
         string testOutputDetails = $"{quantizerName}_{ditherName}";
@@ -206,11 +188,6 @@ public void ApplyQuantization<TPixel>(TestImageProvider<TPixel> provider, IQuant
     public void ApplyQuantizationWithDitheringScale<TPixel>(TestImageProvider<TPixel> provider, IQuantizer quantizer)
         where TPixel : unmanaged, IPixel<TPixel>
     {
-        if (SkipAllQuantizerTests)
-        {
-            return;
-        }
-
         string quantizerName = quantizer.GetType().Name;
         string ditherName = quantizer.Options.Dither.GetType().Name;
         float ditherScale = quantizer.Options.DitherScale;
@@ -229,8 +206,8 @@ public void ShouldThrowForDefaultDitherInstance(IDither dither)
     {
         void Command()
         {
-            using var image = new Image<Rgba32>(10, 10);
-            var quantizer = new WebSafePaletteQuantizer();
+            using Image<Rgba32> image = new(10, 10);
+            WebSafePaletteQuantizer quantizer = new();
             quantizer.Options.Dither = dither;
             image.Mutate(x => x.Quantize(quantizer));
         }

diff --git a/...tput/QuantizerTests/ApplyQuantizationInBox_Bike_OctreeQuantizer_ErrorDither.png b/...tput/QuantizerTests/ApplyQuantizationInBox_Bike_OctreeQuantizer_ErrorDither.png
diff --git a/...eOutput/QuantizerTests/ApplyQuantizationInBox_Bike_OctreeQuantizer_NoDither.png b/...eOutput/QuantizerTests/ApplyQuantizationInBox_Bike_OctreeQuantizer_NoDither.png
diff --git a/...ut/QuantizerTests/ApplyQuantizationInBox_Bike_OctreeQuantizer_OrderedDither.png b/...ut/QuantizerTests/ApplyQuantizationInBox_Bike_OctreeQuantizer_OrderedDither.png
diff --git a/...ntizerTests/ApplyQuantizationInBox_Bike_WebSafePaletteQuantizer_ErrorDither.png b/...ntizerTests/ApplyQuantizationInBox_Bike_WebSafePaletteQuantizer_ErrorDither.png
diff --git a/...QuantizerTests/ApplyQuantizationInBox_Bike_WebSafePaletteQuantizer_NoDither.png b/...QuantizerTests/ApplyQuantizationInBox_Bike_WebSafePaletteQuantizer_NoDither.png
diff --git a/...izerTests/ApplyQuantizationInBox_Bike_WebSafePaletteQuantizer_OrderedDither.png b/...izerTests/ApplyQuantizationInBox_Bike_WebSafePaletteQuantizer_OrderedDither.png
diff --git a/...antizerTests/ApplyQuantizationInBox_Bike_WernerPaletteQuantizer_ErrorDither.png b/...antizerTests/ApplyQuantizationInBox_Bike_WernerPaletteQuantizer_ErrorDither.png
diff --git a/.../QuantizerTests/ApplyQuantizationInBox_Bike_WernerPaletteQuantizer_NoDither.png b/.../QuantizerTests/ApplyQuantizationInBox_Bike_WernerPaletteQuantizer_NoDither.png
diff --git a/...tizerTests/ApplyQuantizationInBox_Bike_WernerPaletteQuantizer_OrderedDither.png b/...tizerTests/ApplyQuantizationInBox_Bike_WernerPaletteQuantizer_OrderedDither.png
diff --git a/...ceOutput/QuantizerTests/ApplyQuantizationInBox_Bike_WuQuantizer_ErrorDither.png b/...ceOutput/QuantizerTests/ApplyQuantizationInBox_Bike_WuQuantizer_ErrorDither.png
diff --git a/...renceOutput/QuantizerTests/ApplyQuantizationInBox_Bike_WuQuantizer_NoDither.png b/...renceOutput/QuantizerTests/ApplyQuantizationInBox_Bike_WuQuantizer_NoDither.png
diff --git a/...Output/QuantizerTests/ApplyQuantizationInBox_Bike_WuQuantizer_OrderedDither.png b/...Output/QuantizerTests/ApplyQuantizationInBox_Bike_WuQuantizer_OrderedDither.png
diff --git a/...rTests/ApplyQuantizationInBox_CalliphoraPartial_OctreeQuantizer_ErrorDither.png b/...rTests/ApplyQuantizationInBox_CalliphoraPartial_OctreeQuantizer_ErrorDither.png
diff --git a/...izerTests/ApplyQuantizationInBox_CalliphoraPartial_OctreeQuantizer_NoDither.png b/...izerTests/ApplyQuantizationInBox_CalliphoraPartial_OctreeQuantizer_NoDither.png
diff --git a/...ests/ApplyQuantizationInBox_CalliphoraPartial_OctreeQuantizer_OrderedDither.png b/...ests/ApplyQuantizationInBox_CalliphoraPartial_OctreeQuantizer_OrderedDither.png
diff --git a/...pplyQuantizationInBox_CalliphoraPartial_WebSafePaletteQuantizer_ErrorDither.png b/...pplyQuantizationInBox_CalliphoraPartial_WebSafePaletteQuantizer_ErrorDither.png
diff --git a/...s/ApplyQuantizationInBox_CalliphoraPartial_WebSafePaletteQuantizer_NoDither.png b/...s/ApplyQuantizationInBox_CalliphoraPartial_WebSafePaletteQuantizer_NoDither.png
diff --git a/...lyQuantizationInBox_CalliphoraPartial_WebSafePaletteQuantizer_OrderedDither.png b/...lyQuantizationInBox_CalliphoraPartial_WebSafePaletteQuantizer_OrderedDither.png
diff --git a/...ApplyQuantizationInBox_CalliphoraPartial_WernerPaletteQuantizer_ErrorDither.png b/...ApplyQuantizationInBox_CalliphoraPartial_WernerPaletteQuantizer_ErrorDither.png
diff --git a/...ts/ApplyQuantizationInBox_CalliphoraPartial_WernerPaletteQuantizer_NoDither.png b/...ts/ApplyQuantizationInBox_CalliphoraPartial_WernerPaletteQuantizer_NoDither.png
diff --git a/...plyQuantizationInBox_CalliphoraPartial_WernerPaletteQuantizer_OrderedDither.png b/...plyQuantizationInBox_CalliphoraPartial_WernerPaletteQuantizer_OrderedDither.png