From b044fa95a719310d9d670f240f91a017d861d90b Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Mon, 12 Aug 2019 20:10:32 +0200 Subject: [PATCH 1/5] Remove unnecessary tileX and tileY declarations --- ...AdaptiveHistogramEqualizationProcessor{TPixel}.cs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs index 7d8ba6208..5d3e865ed 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs @@ -126,11 +126,11 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization 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, 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, tileHeight, xStart: rightBorderStartX, xEnd: sourceWidth, luminanceLevels); // Fix top row ProcessBorderRow(ref pixelsBase, cdfData, 0, sourceWidth, tileWidth, yStart: 0, yEnd: halfTileHeight, luminanceLevels); @@ -201,7 +201,6 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization /// The X index of the lookup table to use. /// The source image width. /// The source image height. - /// The width of a tile. /// The height of a tile. /// X start position in the image. /// X end position of the image. @@ -215,13 +214,11 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization int cdfX, int sourceWidth, int sourceHeight, - int tileWidth, int tileHeight, int xStart, int xEnd, int luminanceLevels) { - int halfTileWidth = tileWidth / 2; int halfTileHeight = tileHeight / 2; int cdfY = 0; @@ -232,13 +229,11 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization for (int dy = y; dy < yLimit; dy++) { int dyOffSet = dy * sourceWidth; - int tileX = halfTileWidth; for (int dx = xStart; dx < xEnd; dx++) { ref TPixel pixel = ref Unsafe.Add(ref pixelBase, dyOffSet + dx); float luminanceEqualized = InterpolateBetweenTwoTiles(pixel, cdfData, cdfX, cdfY, cdfX, cdfY + 1, tileY, tileHeight, luminanceLevels); pixel.FromVector4(new Vector4(luminanceEqualized, luminanceEqualized, luminanceEqualized, pixel.ToVector4().W)); - tileX++; } tileY++; @@ -277,7 +272,6 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization int cdfX = 0; for (int x = halfTileWidth; x < sourceWidth - halfTileWidth; x += tileWidth) { - int tileY = 0; for (int dy = yStart; dy < yEnd; dy++) { int dyOffSet = dy * sourceWidth; @@ -290,8 +284,6 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization pixel.FromVector4(new Vector4(luminanceEqualized, luminanceEqualized, luminanceEqualized, pixel.ToVector4().W)); tileX++; } - - tileY++; } cdfX++; From 76e7b3dbaad491d11a5e10eff65478191cb3118e Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Tue, 13 Aug 2019 19:47:09 +0200 Subject: [PATCH 2/5] Add comment about when to use clipping --- .../Normalization/HistogramEqualizationOptions.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationOptions.cs b/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationOptions.cs index 1d9d5c986..8ddb4834d 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/HistogramEqualizationOptions.cs +++ b/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; /// - /// 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. /// public bool ClipHistogram { get; set; } = false; From 84fc4320e7c5095625c1f95b709b4bbee28800ca Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 15 Aug 2019 19:10:05 +0200 Subject: [PATCH 3/5] Changed processing of border rows and columns to use the tile count and process only a maximum of tileCount - 1 tiles --- ...eHistogramEqualizationProcessor{TPixel}.cs | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs b/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs index 5d3e865ed..1c7130dfa 100644 --- a/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs +++ b/src/ImageSharp/Processing/Processors/Normalization/AdaptiveHistogramEqualizationProcessor{TPixel}.cs @@ -65,10 +65,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( @@ -87,7 +89,8 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization ref TPixel sourceBase = ref source.GetPixelReference(0, 0); cdfX = 0; - for (int x = halfTileWidth; x < sourceWidth - halfTileWidth; x += tileWidth) + int x = halfTileWidth; + for (int tile = 0; tile < tileCount - 1; tile++) { tileY = 0; int yEnd = Math.Min(y + tileHeight, sourceHeight); @@ -120,24 +123,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, 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, 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); @@ -201,6 +205,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization /// The X index of the lookup table to use. /// The source image width. /// The source image height. + /// The number of vertical tiles. /// The height of a tile. /// X start position in the image. /// X end position of the image. @@ -214,6 +219,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization int cdfX, int sourceWidth, int sourceHeight, + int tileCount, int tileHeight, int xStart, int xEnd, @@ -222,7 +228,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; @@ -240,6 +247,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization } cdfY++; + y += tileHeight; } } @@ -250,6 +258,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization /// The pre-computed lookup tables to remap the grey values for each tiles. /// The Y index of the lookup table to use. /// The source image width. + /// The number of horizontal tiles. /// The width of a tile. /// Y start position in the image. /// Y end position of the image. @@ -262,6 +271,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization CdfTileData cdfData, int cdfY, int sourceWidth, + int tileCount, int tileWidth, int yStart, int yEnd, @@ -270,7 +280,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++) { @@ -287,6 +298,7 @@ namespace SixLabors.ImageSharp.Processing.Processors.Normalization } cdfX++; + x += tileWidth; } } From d15022007f2aec75ea64ec7f2b2f65ee28542a36 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Thu, 22 Aug 2019 14:54:05 +0200 Subject: [PATCH 4/5] Add regression test for Issue984 --- .../HistogramEqualizationTests.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs b/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs index 5d8a155f0..3610f9683 100644 --- a/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs +++ b/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); } } + + /// + /// 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 + /// + [Theory] + [WithTestPatternImages(110, 110, PixelTypes.Rgba32)] + [WithTestPatternImages(170, 170, PixelTypes.Rgba32)] + public void Issue984_DoesNotThrowException(TestImageProvider provider) + where TPixel : struct, IPixel + { + using (Image image = provider.GetImage()) + { + var options = new HistogramEqualizationOptions() + { + Method = HistogramEqualizationMethod.AdaptiveTileInterpolation, + LuminanceLevels = 256, + ClipHistogram = true, + NumberOfTiles = 10 + }; + System.Exception ex = Record.Exception(() => image.Mutate(x => x.HistogramEqualization(options))); + Assert.Null(ex); + } + } } } From 1845f5e14218b3b35dcb65c6b74110003dede6c9 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Sun, 1 Sep 2019 22:54:13 +0200 Subject: [PATCH 5/5] debug save the output for Issue984 --- .../Processing/Normalization/HistogramEqualizationTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs b/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs index 3610f9683..c71232524 100644 --- a/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs +++ b/tests/ImageSharp.Tests/Processing/Normalization/HistogramEqualizationTests.cs @@ -123,7 +123,7 @@ namespace SixLabors.ImageSharp.Tests.Processing.Normalization [Theory] [WithTestPatternImages(110, 110, PixelTypes.Rgba32)] [WithTestPatternImages(170, 170, PixelTypes.Rgba32)] - public void Issue984_DoesNotThrowException(TestImageProvider provider) + public void Issue984(TestImageProvider provider) where TPixel : struct, IPixel { using (Image image = provider.GetImage()) @@ -135,8 +135,8 @@ namespace SixLabors.ImageSharp.Tests.Processing.Normalization ClipHistogram = true, NumberOfTiles = 10 }; - System.Exception ex = Record.Exception(() => image.Mutate(x => x.HistogramEqualization(options))); - Assert.Null(ex); + image.Mutate(x => x.HistogramEqualization(options)); + image.DebugSave(provider); } } }