Forum

I noted some strange color problems and, after some investigations, I found that, in some cases, CExtBitmap::stat_RGBtoHSL does not work. This happens only in one of my PCs and in release builds: the problem seems that the == comparisons between floating point numbers ("if( r == cmax )" and ("if( g == cmax )") do not return the expected result.
I guess this may be due to some optimizations of the compiler (it happens only in release builds) and/or the behavior of a given coprocessor (I can reproduce it only in one PC).
In general it is not a good practice to compare floating numbers with ==, but in this case there should not be any problem, since the numbers that are compared should be identical when the corresponding r,g,b values are identical. However, just to be more sure, the function may be modified using tolerances or (I prefer this second option) comparing the r,g,b integer values.
As a matter of fact, modifying the function as follows, I was no longer able to reproduce the problem. Please let me know what you think.

void _RGBtoHSL( COLORREF rgb, double *H, double *S, double *L ) {   
	
int nR = GetRValue(rgb);
int nG = GetGValue(rgb);
int nB = GetBValue(rgb);
int nMin = min(nR,min(nG,nB));
int nMax = max(nR,min(nG,nB));
double delta;
double r = (double)nR/255;
double g = (double)nG/255;
double b = (double)nB/255;   
double cmax = (double)nMax/255;
double cmin = (double)nMin/255;

	*L = (cmax + cmin) / 2.0;   
	
	if(nMax==nMin) 
	{
		*S = 0;      
		*H = 0; // it’s really undefined   
	}
	else 
	{
		if( *L < 0.5 ) 
			*S = (cmax-cmin)/(cmax+cmin);      
		else
			*S = (cmax-cmin)/(2.0-cmax-cmin);      
		delta = cmax - cmin;
		if( nR == nMax ) 
			*H = (g-b)/delta;      
		else if( nG == nMax )
			*H = 2.0 +(b-r)/delta;
		else
			*H = 4.0 + (r-g)/delta;
		
		*H /= 6.0; 

		if( *H < 0.0 )
			*H += 1;  
	}
}

Sorry, there is an error in the previous code: it should be

int nMax = max(nR,max(nG,nB));

This function is based on this article. We have checked the source code and cannot confirm that there is a problem with this. First of all the CExtBitmap::stat_RGBtoHSL() method’s source code looks different than the method in your message and has no problem with min/max function usage.

This is the function I have. Please let me know if it is different from the one in your latest sources.
As you can see, it uses floating point numbers, while the MS sample doesn’t.

void CExtBitmap::stat_RGBtoHSL( COLORREF rgb, double *H, double *S, double *L )
{   
double delta;
double r = (double)GetRValue(rgb)/255;
double g = (double)GetGValue(rgb)/255;
double b = (double)GetBValue(rgb)/255;   
double cmax = max(r,max(g,b));
double cmin = min(r,min(g,b));   
	*L = (cmax + cmin) / 2.0;   
	
	if(cmax==cmin) 
	{
		*S = 0;      
		*H = 0; // it’s really undefined   
	}
	else 
	{
		if( *L < 0.5 ) 
			*S = (cmax-cmin)/(cmax+cmin);      
		else
			*S = (cmax-cmin)/(2.0-cmax-cmin);      
		delta = cmax - cmin;
		if( r == cmax ) 
			*H = (g-b)/delta;      
		else if( g == cmax )
			*H = 2.0 +(b-r)/delta;
		else          
			*H = 4.0 + (r-g)/delta;
		*H /= 6.0; 
		if( *H < 0.0 )
			*H += 1;  
	}
}

The following updated version should work on any computer:

void CExtBitmap::stat_RGBtoHSL( COLORREF rgb, double *H, double *S, double *L )

{   

INT nr = GetRValue( rgb );

INT ng = GetGValue( rgb );

INT nb = GetBValue( rgb );

double delta;

double r = double( nr ) / 255.0;

double g = double( ng ) / 255.0;

double b = double( nb ) / 255.0;

INT ncmax = max( nr, max( ng, nb ) );

INT ncmin = min( nr, min( ng, nb ) );

double cmax = double( ncmax ) / 255.0;

double cmin = double( ncmin ) / 255.0;

    *L = ( cmax + cmin ) / 2.0;

    if( ncmax == ncmin )

    {

        *S = 0;

        *H = 0; // undefined

    }

    else

    {

        if( *L < 0.5 )

            *S = ( cmax - cmin ) / ( cmax + cmin );

        else

            *S = ( cmax - cmin ) / ( 2.0 - cmax - cmin );

        delta = cmax - cmin;

        if( r == cmax )

            *H = ( g -b ) / delta;

        else if( g == cmax )

            *H = 2.0 +( b - r ) / delta;

        else

            *H = 4.0 + ( r - g) / delta;

        *H /= 6.0;

        if( *H < 0.0 )

            *H += 1;

    }

}

You are still comparing float numbers with == . Please further modify "if( r == cmax )" in " if( nr == ncmax )" and "else if( g == cmax )" in "else if( ng == ncmax )" , as I suggested at the beginning.
I think the problem may occur when the compiler optimizes the code placing one of the variables (e.g. r) in the coprocessor stack (80 bit) and another (e.g. cmax) in memory (64 bit).

Thank you for this comment. We followed your suggestion and replaced two floating point variable comparisons with identical numeric variable comparisons. Now the method is:

void CExtBitmap::stat_RGBtoHSL( COLORREF rgb, double *H, double *S, double *L )

{   

INT nr = GetRValue( rgb );

INT ng = GetGValue( rgb );

INT nb = GetBValue( rgb );

double delta;

double r = double( nr ) / 255.0;

double g = double( ng ) / 255.0;

double b = double( nb ) / 255.0;

INT ncmax = max( nr, max( ng, nb ) );

INT ncmin = min( nr, min( ng, nb ) );

double cmax = double( ncmax ) / 255.0;

double cmin = double( ncmin ) / 255.0;

    *L = ( cmax + cmin ) / 2.0;

    if( ncmax == ncmin )

    {

        *S = 0;

        *H = 0; // undefined

    }

    else

    {

        if( *L < 0.5 )

            *S = ( cmax - cmin ) / ( cmax + cmin );

        else

            *S = ( cmax - cmin ) / ( 2.0 - cmax - cmin );

        delta = cmax - cmin;

        if( nr == ncmax )

            *H = ( g -b ) / delta;

        else if( ng == ncmax )

            *H = 2.0 +( b - r ) / delta;

        else

            *H = 4.0 + ( r - g) / delta;

        *H /= 6.0;

        if( *H < 0.0 )

            *H += 1;

    }

}

test