Merge pull request #1568 from SixLabors/js/fix-dither

Fix ordered dithering for small palette lengths.
SixLabors · Mar 2, 2021 · 36018bd · 36018bd
2 parents 9959d51 + 58c5bf1
commit 36018bd
Show file tree

Hide file tree

Showing 57 changed files with 145 additions and 122 deletions.
diff --git a/src/ImageSharp/ImageSharp.csproj b/src/ImageSharp/ImageSharp.csproj
@@ -31,8 +31,8 @@
     <None Include="..\..\shared-infrastructure\branding\icons\imagesharp\sixlabors.imagesharp.128.png" Pack="true" PackagePath="" />
   </ItemGroup>
 
-  <ItemGroup Condition=" '$(TargetFramework)' == 'netcoreapp2.1' ">
-    <PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="4.7.1" />
+  <ItemGroup>
+    <PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="5.0.0" />
   </ItemGroup>
 
   <ItemGroup Condition=" $(TargetFramework.StartsWith('netstandard')) OR '$(TargetFramework)' == 'net472'">

diff --git a/src/ImageSharp/Processing/KnownDitherings.cs b/src/ImageSharp/Processing/KnownDitherings.cs
@@ -30,6 +30,11 @@ public static class KnownDitherings
         /// </summary>
         public static IDither Bayer8x8 { get; } = OrderedDither.Bayer8x8;
 
+        /// <summary>
+        /// Gets the order ditherer using the 16x16 Bayer dithering matrix
+        /// </summary>
+        public static IDither Bayer16x16 { get; } = OrderedDither.Bayer16x16;
+
         /// <summary>
         /// Gets the error Dither that implements the Atkinson algorithm.
         /// </summary>

diff --git a/src/ImageSharp/Processing/Processors/Dithering/ErrorDither.cs b/src/ImageSharp/Processing/Processors/Dithering/ErrorDither.cs
@@ -5,7 +5,6 @@
 using System.Numerics;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
-using SixLabors.ImageSharp.Advanced;
 using SixLabors.ImageSharp.PixelFormats;
 using SixLabors.ImageSharp.Processing.Processors.Quantization;
 

diff --git a/src/ImageSharp/Processing/Processors/Dithering/OrderedDither.KnownTypes.cs b/src/ImageSharp/Processing/Processors/Dithering/OrderedDither.KnownTypes.cs
@@ -23,6 +23,11 @@ public readonly partial struct OrderedDither
         /// </summary>
         public static OrderedDither Bayer8x8 = new OrderedDither(8);
 
+        /// <summary>
+        /// Applies order dithering using the 16x16 Bayer dithering matrix.
+        /// </summary>
+        public static OrderedDither Bayer16x16 = new OrderedDither(16);
+
         /// <summary>
         /// Applies order dithering using the 3x3 ordered dithering matrix.
         /// </summary>

diff --git a/src/ImageSharp/Processing/Processors/Dithering/OrderedDither.cs b/src/ImageSharp/Processing/Processors/Dithering/OrderedDither.cs
@@ -3,7 +3,6 @@
 
 using System;
 using System.Runtime.CompilerServices;
-using System.Runtime.InteropServices;
 using SixLabors.ImageSharp.Advanced;
 using SixLabors.ImageSharp.PixelFormats;
 using SixLabors.ImageSharp.Processing.Processors.Quantization;
@@ -145,24 +144,27 @@ in Unsafe.AsRef(this),
                 in ditherOperation);
         }
 
+        // Spread assumes an even colorspace distribution and precision.
+        // TODO: Cubed root is currently used to represent 3 color channels
+        // but we should introduce something to PixelTypeInfo.
+        // https://bisqwit.iki.fi/story/howto/dither/jy/
+        // https://en.wikipedia.org/wiki/Ordered_dithering#Algorithm
+        internal static int CalculatePaletteSpread(int colors)
+            => (int)(255 / Math.Max(1, Math.Pow(colors, 1.0 / 3) - 1));
+
         [MethodImpl(InliningOptions.ShortMethod)]
         internal TPixel Dither<TPixel>(
             TPixel source,
             int x,
             int y,
-            int bitDepth,
+            int spread,
             float scale)
             where TPixel : unmanaged, IPixel<TPixel>
         {
-            Rgba32 rgba = default;
+            Unsafe.SkipInit(out Rgba32 rgba);
             source.ToRgba32(ref rgba);
-            Rgba32 attempt;
+            Unsafe.SkipInit(out Rgba32 attempt);
 
-            // Spread assumes an even colorspace distribution and precision.
-            // Calculated as 0-255/component count. 256 / bitDepth
-            // https://bisqwit.iki.fi/story/howto/dither/jy/
-            // https://en.wikipedia.org/wiki/Ordered_dithering#Algorithm
-            int spread = 256 / bitDepth;
             float factor = spread * this.thresholdMatrix[y % this.modulusY, x % this.modulusX] * scale;
 
             attempt.R = (byte)Numerics.Clamp(rgba.R + factor, byte.MinValue, byte.MaxValue);
@@ -203,7 +205,7 @@ public override int GetHashCode()
             private readonly ImageFrame<TPixel> source;
             private readonly IndexedImageFrame<TPixel> destination;
             private readonly Rectangle bounds;
-            private readonly int bitDepth;
+            private readonly int spread;
 
             [MethodImpl(InliningOptions.ShortMethod)]
             public QuantizeDitherRowOperation(
@@ -218,23 +220,24 @@ public QuantizeDitherRowOperation(
                 this.source = source;
                 this.destination = destination;
                 this.bounds = bounds;
-                this.bitDepth = ColorNumerics.GetBitsNeededForColorDepth(destination.Palette.Length);
+                this.spread = CalculatePaletteSpread(destination.Palette.Length);
             }
 
             [MethodImpl(InliningOptions.ShortMethod)]
             public void Invoke(int y)
             {
-                int offsetY = this.bounds.Top;
-                int offsetX = this.bounds.Left;
+                ref TFrameQuantizer quantizer = ref Unsafe.AsRef(this.quantizer);
+                int spread = this.spread;
                 float scale = this.quantizer.Options.DitherScale;
 
-                ref TPixel sourceRowRef = ref MemoryMarshal.GetReference(this.source.GetPixelRowSpan(y));
-                ref byte destinationRowRef = ref MemoryMarshal.GetReference(this.destination.GetWritablePixelRowSpanUnsafe(y - offsetY));
+                ReadOnlySpan<TPixel> sourceRow = this.source.GetPixelRowSpan(y).Slice(this.bounds.X, this.bounds.Width);
+                Span<byte> destRow =
+                    this.destination.GetWritablePixelRowSpanUnsafe(y - this.bounds.Y).Slice(0, sourceRow.Length);
 
-                for (int x = this.bounds.Left; x < this.bounds.Right; x++)
+                for (int x = 0; x < sourceRow.Length; x++)
                 {
-                    TPixel dithered = this.dither.Dither(Unsafe.Add(ref sourceRowRef, x), x, y, this.bitDepth, scale);
-                    Unsafe.Add(ref destinationRowRef, x - offsetX) = Unsafe.AsRef(this.quantizer).GetQuantizedColor(dithered, out TPixel _);
+                    TPixel dithered = this.dither.Dither(sourceRow[x], x, y, spread, scale);
+                    destRow[x] = quantizer.GetQuantizedColor(dithered, out TPixel _);
                 }
             }
         }
@@ -248,7 +251,7 @@ public void Invoke(int y)
             private readonly ImageFrame<TPixel> source;
             private readonly Rectangle bounds;
             private readonly float scale;
-            private readonly int bitDepth;
+            private readonly int spread;
 
             [MethodImpl(InliningOptions.ShortMethod)]
             public PaletteDitherRowOperation(
@@ -262,19 +265,23 @@ public PaletteDitherRowOperation(
                 this.source = source;
                 this.bounds = bounds;
                 this.scale = processor.DitherScale;
-                this.bitDepth = ColorNumerics.GetBitsNeededForColorDepth(processor.Palette.Length);
+                this.spread = CalculatePaletteSpread(processor.Palette.Length);
             }
 
             [MethodImpl(InliningOptions.ShortMethod)]
             public void Invoke(int y)
             {
-                ref TPixel sourceRowRef = ref MemoryMarshal.GetReference(this.source.GetPixelRowSpan(y));
+                ref TPaletteDitherImageProcessor processor = ref Unsafe.AsRef(this.processor);
+                int spread = this.spread;
+                float scale = this.scale;
+
+                Span<TPixel> row = this.source.GetPixelRowSpan(y).Slice(this.bounds.X, this.bounds.Width);
 
-                for (int x = this.bounds.Left; x < this.bounds.Right; x++)
+                for (int x = 0; x < row.Length; x++)
                 {
-                    ref TPixel sourcePixel = ref Unsafe.Add(ref sourceRowRef, x);
-                    TPixel dithered = this.dither.Dither(sourcePixel, x, y, this.bitDepth, this.scale);
-                    sourcePixel = Unsafe.AsRef(this.processor).GetPaletteColor(dithered);
+                    ref TPixel sourcePixel = ref row[x];
+                    TPixel dithered = this.dither.Dither(sourcePixel, x, y, spread, scale);
+                    sourcePixel = processor.GetPaletteColor(dithered);
                 }
             }
         }

diff --git a/tests/Directory.Build.targets b/tests/Directory.Build.targets
@@ -21,7 +21,7 @@
     <PackageReference Update="BenchmarkDotNet" Version="0.12.1" />
     <PackageReference Update="BenchmarkDotNet.Diagnostics.Windows" Version="0.12.1" Condition="'$(IsWindows)'=='true'" />
     <PackageReference Update="Colourful" Version="2.0.5" />
-    <PackageReference Update="Magick.NET-Q16-AnyCPU" Version="7.22.0" />
+    <PackageReference Update="Magick.NET-Q16-AnyCPU" Version="7.23.2.1" />
     <PackageReference Update="Microsoft.DotNet.RemoteExecutor" Version="6.0.0-beta.20513.1" />
     <PackageReference Update="Microsoft.DotNet.XUnitExtensions" Version="6.0.0-beta.20513.1" />
     <PackageReference Update="Moq" Version="4.14.6" />

diff --git a/tests/ImageSharp.Tests/Processing/Processors/Dithering/DitherTests.cs b/tests/ImageSharp.Tests/Processing/Processors/Dithering/DitherTests.cs
@@ -37,6 +37,7 @@ public static readonly TheoryData<IDither, string> OrderedDitherers
                 { KnownDitherings.Bayer2x2, nameof(KnownDitherings.Bayer2x2) },
                 { KnownDitherings.Bayer4x4, nameof(KnownDitherings.Bayer4x4) },
                 { KnownDitherings.Bayer8x8, nameof(KnownDitherings.Bayer8x8) },
+                { KnownDitherings.Bayer16x16, nameof(KnownDitherings.Bayer16x16) },
                 { KnownDitherings.Ordered3x3, nameof(KnownDitherings.Ordered3x3) }
             };
 

diff --git a/tests/ImageSharp.Tests/Processing/Processors/Quantization/QuantizerTests.cs b/tests/ImageSharp.Tests/Processing/Processors/Quantization/QuantizerTests.cs
@@ -169,8 +169,8 @@ public void ApplyQuantizationInBox<TPixel>(TestImageProvider<TPixel> provider, I
 
             provider.RunRectangleConstrainedValidatingProcessorTest(
                 (x, rect) => x.Quantize(quantizer, rect),
-                comparer: ValidatorComparer,
                 testOutputDetails: testOutputDetails,
+                comparer: ValidatorComparer,
                 appendPixelTypeToFileName: false);
         }
 

diff --git a/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs b/tests/ImageSharp.Tests/TestUtilities/ReferenceCodecs/MagickReferenceDecoder.cs
@@ -7,7 +7,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using ImageMagick;
-using ImageMagick.Formats.Bmp;
+using ImageMagick.Formats;
 using SixLabors.ImageSharp.Formats;
 using SixLabors.ImageSharp.Memory;
 using SixLabors.ImageSharp.PixelFormats;

diff --git a/tests/ImageSharp.Tests/TestUtilities/TestUtils.cs b/tests/ImageSharp.Tests/TestUtilities/TestUtils.cs
@@ -240,7 +240,7 @@ internal static void RunValidatingProcessorTest<TPixel>(
             using (Image<TPixel> image = provider.GetImage())
             {
                 FormattableString testOutputDetails = $"";
-                image.Mutate(ctx => { testOutputDetails = processAndGetTestOutputDetails(ctx); });
+                image.Mutate(ctx => testOutputDetails = processAndGetTestOutputDetails(ctx));
 
                 image.DebugSave(
                     provider,

diff --git a/...ReferenceOutput/DitherTests/ApplyDitherFilterInBox_Rgba32_CalliphoraPartial.png b/...ReferenceOutput/DitherTests/ApplyDitherFilterInBox_Rgba32_CalliphoraPartial.png
diff --git a/...ut/DitherTests/DitherFilter_ShouldNotDependOnSinglePixelType_Bgra32_filter0.png b/...ut/DitherTests/DitherFilter_ShouldNotDependOnSinglePixelType_Bgra32_filter0.png
diff --git a/...put/DitherTests/DitherFilter_ShouldNotDependOnSinglePixelType_Rgb24_filter0.png b/...put/DitherTests/DitherFilter_ShouldNotDependOnSinglePixelType_Rgb24_filter0.png
diff --git a/...ut/DitherTests/DitherFilter_ShouldNotDependOnSinglePixelType_Rgba32_filter0.png b/...ut/DitherTests/DitherFilter_ShouldNotDependOnSinglePixelType_Rgba32_filter0.png
diff --git a/...itherTests/DitherFilter_ShouldNotDependOnSinglePixelType_RgbaVector_filter0.png b/...itherTests/DitherFilter_ShouldNotDependOnSinglePixelType_RgbaVector_filter0.png
diff --git a/...erenceOutput/DitherTests/DitherFilter_WorksWithAllDitherers_Bike_Bayer16x16.png b/...erenceOutput/DitherTests/DitherFilter_WorksWithAllDitherers_Bike_Bayer16x16.png
diff --git a/...eferenceOutput/DitherTests/DitherFilter_WorksWithAllDitherers_Bike_Bayer2x2.png b/...eferenceOutput/DitherTests/DitherFilter_WorksWithAllDitherers_Bike_Bayer2x2.png
diff --git a/...eferenceOutput/DitherTests/DitherFilter_WorksWithAllDitherers_Bike_Bayer4x4.png b/...eferenceOutput/DitherTests/DitherFilter_WorksWithAllDitherers_Bike_Bayer4x4.png
diff --git a/...eferenceOutput/DitherTests/DitherFilter_WorksWithAllDitherers_Bike_Bayer8x8.png b/...eferenceOutput/DitherTests/DitherFilter_WorksWithAllDitherers_Bike_Bayer8x8.png
diff --git a/...erenceOutput/DitherTests/DitherFilter_WorksWithAllDitherers_Bike_Ordered3x3.png b/...erenceOutput/DitherTests/DitherFilter_WorksWithAllDitherers_Bike_Ordered3x3.png
diff --git a/...DitherTests/DitherFilter_WorksWithAllDitherers_CalliphoraPartial_Bayer16x16.png b/...DitherTests/DitherFilter_WorksWithAllDitherers_CalliphoraPartial_Bayer16x16.png
diff --git a/...t/DitherTests/DitherFilter_WorksWithAllDitherers_CalliphoraPartial_Bayer2x2.png b/...t/DitherTests/DitherFilter_WorksWithAllDitherers_CalliphoraPartial_Bayer2x2.png
diff --git a/...t/DitherTests/DitherFilter_WorksWithAllDitherers_CalliphoraPartial_Bayer4x4.png b/...t/DitherTests/DitherFilter_WorksWithAllDitherers_CalliphoraPartial_Bayer4x4.png
diff --git a/...t/DitherTests/DitherFilter_WorksWithAllDitherers_CalliphoraPartial_Bayer8x8.png b/...t/DitherTests/DitherFilter_WorksWithAllDitherers_CalliphoraPartial_Bayer8x8.png
diff --git a/...DitherTests/DitherFilter_WorksWithAllDitherers_CalliphoraPartial_Ordered3x3.png b/...DitherTests/DitherFilter_WorksWithAllDitherers_CalliphoraPartial_Ordered3x3.png
diff --git a/...ut/QuantizerTests/ApplyQuantizationInBox_Bike_OctreeQuantizer_OrderedDither.png b/...ut/QuantizerTests/ApplyQuantizationInBox_Bike_OctreeQuantizer_OrderedDither.png
diff --git a/...izerTests/ApplyQuantizationInBox_Bike_WebSafePaletteQuantizer_OrderedDither.png b/...izerTests/ApplyQuantizationInBox_Bike_WebSafePaletteQuantizer_OrderedDither.png
diff --git a/...tizerTests/ApplyQuantizationInBox_Bike_WernerPaletteQuantizer_OrderedDither.png b/...tizerTests/ApplyQuantizationInBox_Bike_WernerPaletteQuantizer_OrderedDither.png
diff --git a/...Output/QuantizerTests/ApplyQuantizationInBox_Bike_WuQuantizer_OrderedDither.png b/...Output/QuantizerTests/ApplyQuantizationInBox_Bike_WuQuantizer_OrderedDither.png
diff --git a/...ests/ApplyQuantizationInBox_CalliphoraPartial_OctreeQuantizer_OrderedDither.png b/...ests/ApplyQuantizationInBox_CalliphoraPartial_OctreeQuantizer_OrderedDither.png
diff --git a/...lyQuantizationInBox_CalliphoraPartial_WebSafePaletteQuantizer_OrderedDither.png b/...lyQuantizationInBox_CalliphoraPartial_WebSafePaletteQuantizer_OrderedDither.png
diff --git a/...plyQuantizationInBox_CalliphoraPartial_WernerPaletteQuantizer_OrderedDither.png b/...plyQuantizationInBox_CalliphoraPartial_WernerPaletteQuantizer_OrderedDither.png
diff --git a/...zerTests/ApplyQuantizationInBox_CalliphoraPartial_WuQuantizer_OrderedDither.png b/...zerTests/ApplyQuantizationInBox_CalliphoraPartial_WuQuantizer_OrderedDither.png
diff --git a/...pplyQuantizationWithDitheringScale_david_OctreeQuantizer_OrderedDither_0.25.png b/...pplyQuantizationWithDitheringScale_david_OctreeQuantizer_OrderedDither_0.25.png
diff --git a/...ApplyQuantizationWithDitheringScale_david_OctreeQuantizer_OrderedDither_0.5.png b/...ApplyQuantizationWithDitheringScale_david_OctreeQuantizer_OrderedDither_0.5.png
diff --git a/...pplyQuantizationWithDitheringScale_david_OctreeQuantizer_OrderedDither_0.75.png b/...pplyQuantizationWithDitheringScale_david_OctreeQuantizer_OrderedDither_0.75.png
diff --git a/...s/ApplyQuantizationWithDitheringScale_david_OctreeQuantizer_OrderedDither_1.png b/...s/ApplyQuantizationWithDitheringScale_david_OctreeQuantizer_OrderedDither_1.png
diff --git a/...tizationWithDitheringScale_david_WebSafePaletteQuantizer_OrderedDither_0.25.png b/...tizationWithDitheringScale_david_WebSafePaletteQuantizer_OrderedDither_0.25.png
diff --git a/...ntizationWithDitheringScale_david_WebSafePaletteQuantizer_OrderedDither_0.5.png b/...ntizationWithDitheringScale_david_WebSafePaletteQuantizer_OrderedDither_0.5.png
diff --git a/...tizationWithDitheringScale_david_WebSafePaletteQuantizer_OrderedDither_0.75.png b/...tizationWithDitheringScale_david_WebSafePaletteQuantizer_OrderedDither_0.75.png
diff --git a/...uantizationWithDitheringScale_david_WebSafePaletteQuantizer_OrderedDither_1.png b/...uantizationWithDitheringScale_david_WebSafePaletteQuantizer_OrderedDither_1.png