Merge pull request #984 from brianpopow/feature/fixAdaptiveHistIssue

Fix adaptive histogram out of bounds bug

src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs

@@ -73,10 +73,12 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization
 
                 var tileYStartPositions = new List<(int y, int cdfY)>();
                 int cdfY = 0;
-                for (int y = halfTileHeight; y < sourceHeight - halfTileHeight; y += tileHeight)
+                int yStart = halfTileHeight;
+                for (int tile = 0; tile < tileCount - 1; tile++)
                 {
-                    tileYStartPositions.Add((y, cdfY));
+                    tileYStartPositions.Add((yStart, cdfY));
                     cdfY++;
+                    yStart += tileHeight;
                 }
 
                 Parallel.For(
@@ -92,7 +94,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization
                             ref TPixel sourceBase = ref source.GetPixelReference(0, 0);
 
                             int cdfX = 0;
-                            for (int x = halfTileWidth; x < sourceWidth - halfTileWidth; x += tileWidth)
+                            int x = halfTileWidth;
+                            for (int tile = 0; tile < tileCount - 1; tile++)
                             {
                                 int tileY = 0;
                                 int yEnd = Math.Min(y + tileHeight, sourceHeight);
@@ -125,24 +128,25 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization
                                 }
 
                                 cdfX++;
+                                x += tileWidth;
                             }
                         });
 
                 ref TPixel pixelsBase = ref source.GetPixelReference(0, 0);
 
                 // Fix left column
-                ProcessBorderColumn(ref pixelsBase, cdfData, 0, sourceWidth, sourceHeight, tileWidth, tileHeight, xStart: 0, xEnd: halfTileWidth, luminanceLevels);
+                ProcessBorderColumn(ref pixelsBase, cdfData, 0, sourceWidth, sourceHeight, this.Tiles, tileHeight, xStart: 0, xEnd: halfTileWidth, luminanceLevels);
 
                 // Fix right column
                 int rightBorderStartX = ((this.Tiles - 1) * tileWidth) + halfTileWidth;
-                ProcessBorderColumn(ref pixelsBase, cdfData, this.Tiles - 1, sourceWidth, sourceHeight, tileWidth, tileHeight, xStart: rightBorderStartX, xEnd: sourceWidth, luminanceLevels);
+                ProcessBorderColumn(ref pixelsBase, cdfData, this.Tiles - 1, sourceWidth, sourceHeight, this.Tiles, tileHeight, xStart: rightBorderStartX, xEnd: sourceWidth, luminanceLevels);
 
                 // Fix top row
-                ProcessBorderRow(ref pixelsBase, cdfData, 0, sourceWidth, tileWidth, yStart: 0, yEnd: halfTileHeight, luminanceLevels);
+                ProcessBorderRow(ref pixelsBase, cdfData, 0, sourceWidth, this.Tiles, tileWidth, yStart: 0, yEnd: halfTileHeight, luminanceLevels);
 
                 // Fix bottom row
                 int bottomBorderStartY = ((this.Tiles - 1) * tileHeight) + halfTileHeight;
-                ProcessBorderRow(ref pixelsBase, cdfData, this.Tiles - 1, sourceWidth, tileWidth, yStart: bottomBorderStartY, yEnd: sourceHeight, luminanceLevels);
+                ProcessBorderRow(ref pixelsBase, cdfData, this.Tiles - 1, sourceWidth, this.Tiles, tileWidth, yStart: bottomBorderStartY, yEnd: sourceHeight, luminanceLevels);
 
                 // Left top corner
                 ProcessCornerTile(ref pixelsBase, cdfData, sourceWidth, 0, 0, xStart: 0, xEnd: halfTileWidth, yStart: 0, yEnd: halfTileHeight, luminanceLevels);
@@ -206,7 +210,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization
         /// <param name="cdfX">The X index of the lookup table to use.</param>
         /// <param name="sourceWidth">The source image width.</param>
         /// <param name="sourceHeight">The source image height.</param>
-        /// <param name="tileWidth">The width of a tile.</param>
+        /// <param name="tileCount">The number of vertical tiles.</param>
         /// <param name="tileHeight">The height of a tile.</param>
         /// <param name="xStart">X start position in the image.</param>
         /// <param name="xEnd">X end position of the image.</param>
@@ -220,7 +224,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization
             int cdfX,
             int sourceWidth,
             int sourceHeight,
-            int tileWidth,
+            int tileCount,
             int tileHeight,
             int xStart,
             int xEnd,
@@ -229,7 +233,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization
             int halfTileHeight = tileHeight / 2;
 
             int cdfY = 0;
-            for (int y = halfTileHeight; y < sourceHeight - halfTileHeight; y += tileHeight)
+            int y = halfTileHeight;
+            for (int tile = 0; tile < tileCount - 1; tile++)
             {
                 int yLimit = Math.Min(y + tileHeight, sourceHeight - 1);
                 int tileY = 0;
@@ -247,6 +252,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization
                 }
 
                 cdfY++;
+                y += tileHeight;
             }
         }
 
@@ -257,6 +263,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization
         /// <param name="cdfData">The pre-computed lookup tables to remap the grey values for each tiles.</param>
         /// <param name="cdfY">The Y index of the lookup table to use.</param>
         /// <param name="sourceWidth">The source image width.</param>
+        /// <param name="tileCount">The number of horizontal tiles.</param>
         /// <param name="tileWidth">The width of a tile.</param>
         /// <param name="yStart">Y start position in the image.</param>
         /// <param name="yEnd">Y end position of the image.</param>
@@ -269,6 +276,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization
             CdfTileData cdfData,
             int cdfY,
             int sourceWidth,
+            int tileCount,
             int tileWidth,
             int yStart,
             int yEnd,
@@ -277,7 +285,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization
             int halfTileWidth = tileWidth / 2;
 
             int cdfX = 0;
-            for (int x = halfTileWidth; x < sourceWidth - halfTileWidth; x += tileWidth)
+            int x = halfTileWidth;
+            for (int tile = 0; tile < tileCount - 1; tile++)
             {
                 for (int dy = yStart; dy < yEnd; dy++)
                 {
@@ -294,6 +303,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization
                 }
 
                 cdfX++;
+                x += tileWidth;
             }
         }
 

src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationOptions.cs

@@ -1,4 +1,4 @@
-// Copyright (c) Six Labors and contributors.
+// Copyright (c) Six Labors and contributors.
 // Licensed under the Apache License, Version 2.0.
 
 namespace SixLabors.ImageSharp.Processing.Processors.Normalization
@@ -25,7 +25,9 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization
         public int LuminanceLevels { get; set; } = 256;
 
         /// <summary>
-        /// Gets or sets a value indicating whether to clip the histogram bins at a specific value. Defaults to false.
+        /// Gets or sets a value indicating whether to clip the histogram bins at a specific value.
+        /// It is recommended to use clipping when the AdaptiveTileInterpolation method is used, to suppress artifacts which can occur on the borders of the tiles.
+        /// Defaults to false.
         /// </summary>
         public bool ClipHistogram { get; set; } = false;
 

tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs

@@ -9,6 +9,7 @@ using Xunit;
 
 namespace SixLabors.ImageSharp.Tests.Processing.Normalization
 {
+    // ReSharper disable InconsistentNaming
     public class HistogramEqualizationTests
     {
         private static readonly ImageComparer ValidatorComparer = ImageComparer.TolerantPercentage(0.0456F);
@@ -113,5 +114,30 @@ namespace SixLabors.ImageSharp.Tests.Processing.Normalization
                 image.CompareToReferenceOutput(ValidatorComparer, provider);
             }
         }
+
+        /// <summary>
+        /// This is regression test for a bug with the calculation of the y-start positions,
+        /// where it could happen that one too much start position was calculated in some cases.
+        /// See: https://github.com/SixLabors/ImageSharp/pull/984
+        /// </summary>
+        [Theory]
+        [WithTestPatternImages(110, 110, PixelTypes.Rgba32)]
+        [WithTestPatternImages(170, 170, PixelTypes.Rgba32)]
+        public void Issue984<TPixel>(TestImageProvider<TPixel> provider)
+            where TPixel : struct, IPixel<TPixel>
+        {
+            using (Image<TPixel> image = provider.GetImage())
+            {
+                var options = new HistogramEqualizationOptions()
+                              {
+                                  Method = HistogramEqualizationMethod.AdaptiveTileInterpolation,
+                                  LuminanceLevels = 256,
+                                  ClipHistogram = true,
+                                  NumberOfTiles = 10
+                              };
+                image.Mutate(x => x.HistogramEqualization(options));
+                image.DebugSave(provider);
+            }
+        }
     }
 }