Can't convert from HSL to RGB

Question

I have a problem with converting a color from HSL to RGB. I've written the following function:

struct RGB {
    float r, g, b;
};
RGB hslToRgb( float hue, float saturation, float lightness ) {

    RGB rgb1, rgbResult;

    float chroma = ( 1.0 - (float) abs( 2.0 * lightness - 1.0 ) ) * saturation;
    float h1 = hue / 60.0;
    float x = chroma * ( 1.0 - (float) abs( (float) ( (int) h1 % 2 ) - 1.0 ) );

    if ( ( 0 <= h1 ) && ( h1 < 1 ) ) {

        rgb1.r = chroma;
        rgb1.g = x;
        rgb1.b = 0.0;

    } else if ( ( 1 <= h1 ) && ( h1 < 2 ) ) {

        rgb1.r = x;
        rgb1.g = chroma;
        rgb1.b = 0.0;

    } else if ( ( 2 <= h1 ) && ( h1 < 3 ) ) {

        rgb1.r = 0.0;
        rgb1.g = chroma;
        rgb1.b = x;

    } else if ( ( 3 <= h1 ) && ( h1 < 4 ) ) {

        rgb1.r = 0.0;
        rgb1.g = x;
        rgb1.b = chroma;

    } else if ( ( 4 <= h1 ) && ( h1 < 5 ) ) {

        rgb1.r = x;
        rgb1.g = 0.0;
        rgb1.b = chroma;

    } else if ( ( 5 <= h1 ) && ( h1 < 6 ) ) {

        rgb1.r = chroma;
        rgb1.g = 0;
        rgb1.b = x;

    } else {

        rgb1.r = 0.0;
        rgb1.g = 0.0;
        rgb1.b = 0.0;

    }

    float m = lightness - 0.5 * chroma;

    rgbResult.r = rgb1.r + m;
    rgbResult.g = rgb1.g + m;
    rgbResult.b = rgb1.b + m;

    return rgbResult;

}

here it is its test:

float cHue = 0.0;
while ( cHue < 360 ) {
    RGB rgb1 = hslToRgb( (int) cHue, 1.0, 0.5 ); // max on saturation and a middle value for lightness
    printf( "r = %f, g = %f, b = %f\n", rgb1.r, rgb1.g, rgb1.b );
    cHue += 1.0;
}

but I get only 1.0 and 0.0 when I need to get all range between this "integers".

r = 1.000000, g = 0.000000, b = 1.000000
r = 1.000000, g = 0.000000, b = 1.000000
r = 1.000000, g = 0.000000, b = 1.000000
r = 1.000000, g = 0.000000, b = 0.000000
r = 1.000000, g = 0.000000, b = 0.000000
r = 1.000000, g = 0.000000, b = 0.000000

Can anyone help me with this code? Formulas from: http://en.wikipedia.org/wiki/HSL_and_HSV

The `( 1 <= h1 )` tests are redundant and unnecessary since you're using an else if. — Shmiddty, Nov 15 '12 at 22:37
@Shmiddty, why...? For example, if we have h1 == 3.4, it runs the fourth block of "if"-code. Doesn't it? — JavaRunner, Nov 15 '12 at 22:42
You could simplify it like this: `if (h1 < 1) {...} else if (h1 < 2){...}` etc since the first test will fail if `h1 >= 1`, which is the same as `1 <= h1`. The test is redundant. — Shmiddty, Nov 15 '12 at 22:45

score 4 · Accepted Answer · answered Nov 15 '12 at 22:47

4

Use floating point modulo fmodf as:

float x = chroma * ( 1.0 - (float) abs( fmodf(h1, 2.0) - 1.0 ));

answered Nov 15 '12 at 22:47

Michael Shmalko

710
4
16

Wow. Excellent solution! It works great. I didn't know about this function :) Thanks! – JavaRunner Nov 15 '12 at 22:50
1

You can also use fabsf instead of abs to avoid casting to/from double – Michael Shmalko Nov 15 '12 at 22:55

score 1 · Answer 2 · answered Nov 15 '12 at 22:47

1

Stop using C-style casts (like (int) and (float)) in C++ code. Use fabs when you need a floating point absolute function. Break complex formulas down into many steps, you don't gain any efficiency by doing it all on one line.

Do 1 calculation per line. Store the result in a variable of the type you think you need, with a descriptive name. See if you can avoid ever using an int unless you are supposed to explicitly round.

answered Nov 15 '12 at 22:47

Yakk - Adam Nevraumont

262,606
27
330
524

instead of `fabs`, you should really use `std::abs`, which is properly overloaded for every type, albeit in different headers: the floating point overloads are in [``](http://en.cppreference.com/w/cpp/numeric/math/fabs), the integer version is in [``](http://en.cppreference.com/w/cpp/numeric/math/abs) and the `complex` and `valarray` templates are in their respective class headers. – rubenvb Mar 04 '13 at 15:51

score 0 · Answer 3 · answered Nov 15 '12 at 22:49

0

Your chroma value seems off. You have it as

float chroma = ( 1.0 - (float) abs( 2.0 * lightness - 1.0 ) ) * saturation;

but I think it should be

float chroma = saturation * lightness;

since Wikipedia says,

From HSV

Given a color with hue H ∈ [0°, 360°), saturation SHSV ∈ [0, 1], and value V ∈ [0, 1],
we first find chroma:

C = V \times S_{HSV}

answered Nov 15 '12 at 22:49

LSerni

55,617
10
65
107

it's for HSV, but I deal with HSL :) – JavaRunner Nov 15 '12 at 22:57

score -1 · Answer 4 · edited May 23 '17 at 11:51

-1

On your line:

float x = chroma * ( 1.0 - (float) abs( (float) ( (int) h1 % 2 ) - 1.0 ) );

Your casting of (int) for the modulus causes it to remove all the decimals of the float. Thereby making this a binary function.

You need a floating point modulus function like Michael Sh described.

edited May 23 '17 at 11:51

Community

1
1

answered Nov 15 '12 at 22:50

Thymine

8,775
2
35
47

Can't convert from HSL to RGB

4 Answers4