ImageStatisticsHSL's check for black pixel isn't proper.

[GoogleCodeExporter]

@ Line 169, ImageStatisticsHSL.cs

The correct line should read:

if ((rgb.Red != 0) || (rgb.Green != 0) || (rgb.Blue != 0))

instead of:

if ((hsl.Hue != 0.0) || (hsl.Luminance != 0.0) || (hsl.Saturation != 0.0))

Reason:::
I was upgrading the code to include Hue histogram in the class. This line 
checks whether pixel is black. The calculation inside if block needs to be 
done only if pixel is not black.
Check for Hue!=0.0 is not required (Hue doesn't have any significance for 
the check). Instead, RGB pixel components available in same code section 
could be used.

Original issue reported on code.google.com by piyoo...@gmail.com on 9 May 2008 at 7:48

[GoogleCodeExporter]

I might be wrong saying that here Hue doesn't matter. Pls. confirm. Thanks.

Original comment by piyoo...@gmail.com on 9 May 2008 at 8:51

[GoogleCodeExporter]

Original comment by andrew.k...@gmail.com on 13 May 2008 at 6:33

• Changed state: Accepted
• Added labels: Milestone-Coming, Project-Imaging

[GoogleCodeExporter]

In this particular case there is no issue. Since the HSL value is calculated 
from 
RGB, then all non black pixels will form non zero components in HSL. If at 
least one 
HSL component is not equal to zero, then original RGB value represents non 
black 
pixel. In generic case of course I agree – the fact that Saturation or Hue is 
not 
set to zero does not mean that pixel is non black (luminance makes the 
decision).

But, you are right. It is possible to make some tuning. In HSL color space 
black is 
represented by Luminance equal to zero (independent of other components). If 
luminance is not zero, then it is not black and it represents RGB other then 
(0,0,0). So I've simplified the check to:

if ( hsl.Luminance != 0.0 )

Fixed in revision 473. Will be released in AForge.NET 1.6.3.

Original comment by andrew.k...@gmail.com on 13 May 2008 at 7:14

• Changed state: Fixed

[GoogleCodeExporter]

Original comment by andrew.k...@gmail.com on 19 May 2008 at 6:01

• Changed state: Verified
• Removed labels: Milestone-Coming

[GoogleCodeExporter]

Original comment by andrew.k...@gmail.com on 19 May 2008 at 6:08

• Changed state: Released

[GoogleCodeExporter]

How about adding Hue histogram too to the class?

Original comment by piyoo...@gmail.com on 19 May 2008 at 6:38

[GoogleCodeExporter]

How do think it should work ? :)

Hue is cyclic value, so such values like mean, median and std. dev. will give 
you 0 
information.

Suppose you have 2 value on the histogram: 0 and 359. Both of them are for red 
color. So, if mean will be 180 will it be true ? No.

Original comment by andrew.k...@gmail.com on 19 May 2008 at 7:20

[GoogleCodeExporter]

I agree...
However, histograms may not be used only to get statistical values. Histogram 
simply 
represents a collection of frequencies for various bins (which may not be 
numerical 
e.g. days of week). For cases when there is no definition of statistical values 
like 
mean, we can use such GenericHistogram<T>.
Addition of GenericHistogram would be an enhancement to the AForge class 
hierarchy 
allowing the use of a base type for handling any histograms without frills of 
specialized functions (statistical, etc).
Hence, my suggestion would be to derive the other Histogram classes from 
GenericHistogram<T>. The classes like ImageStatisticsHSL can expose the 
corresponding derived histogram types or the generic type as allowed by their 
semantics.

Original comment by piyoo...@gmail.com on 19 May 2008 at 8:30

[GoogleCodeExporter]

If you would like, you may register a new issue requesting new feature. This 
particular issue is fixed and closed.

Original comment by andrew.k...@gmail.com on 19 May 2008 at 5:38