RGB to HSL and back, calculation problems

Question

I'm trying to convert RGB to HSL and I also want to convert from HSL to RGB, I have written a class for it but if I do RGB->HSL->RGB to try if it works I get a different value.

Example case: if you create a HSLColor object by doing HSLColor MyTestConversion = HSLColor.FromRGB(Colors.Green); and then do Color ExpectedGreenHere = MyTestConversion.ToRGB() you get a different color than Colors.Green while it was the original input so something goes wrong..

This is the code i'm using:

public class HSLColor
{
    public float Hue;
    public float Saturation;
    public float Luminosity;

    public HSLColor(float H, float S, float L)
    {
        Hue = H;
        Saturation = S;
        Luminosity = L;
    }

    public static HSLColor FromRGB(Color Clr)
    {
        return FromRGB(Clr.R, Clr.G, Clr.B);
    }

    public static HSLColor FromRGB(Byte R, Byte G, Byte B)
    {
        float _R = (R / 255f);
        float _G = (G / 255f);
        float _B = (B / 255f);

        float _Min = Math.Min(Math.Min(_R, _G), _B);
        float _Max = Math.Max(Math.Max(_R, _G), _B);
        float _Delta = _Max - _Min;

        float H = 0;
        float S = 0;
        float L = (float)((_Max + _Min) / 2.0f);

        if (_Delta != 0)
        {
            if (L < 0.5f)
            {
                S = (float)(_Delta / (_Max + _Min));
            }
            else
            {
                S = (float)(_Delta / (2.0f - _Max - _Min));
            }

            float _Delta_R = (float)(((_Max - _R) / 6.0f + (_Delta / 2.0f)) / _Delta);
            float _Delta_G = (float)(((_Max - _G) / 6.0f + (_Delta / 2.0f)) / _Delta);
            float _Delta_B = (float)(((_Max - _B) / 6.0f + (_Delta / 2.0f)) / _Delta);

            if (_R == _Max)
            {
                H = _Delta_B - _Delta_G;
            }
            else if (_G == _Max)
            {
                H = (1.0f / 3.0f) + _Delta_R - _Delta_B;
            }
            else if (_B == _Max)
            {
                H = (2.0f / 3.0f) + _Delta_G - _Delta_R;
            }

            if (H < 0) H += 1.0f;
            if (H > 1) H -= 1.0f;
        }

        return new HSLColor(H, S, L);
    }

    private float Hue_2_RGB(float v1, float v2, float vH)
    {
        if (vH < 0) vH += 1;
        if (vH > 1) vH -= 1;
        if ((6 * vH) < 1) return (v1 + (v2 - v1) * 6 * vH);
        if ((2 * vH) < 1) return (v2);
        if ((3 * vH) < 2) return (v1 + (v2 - v1) * ((2 / 3) - vH) * 6);
        return (v1);
    }

    public Color ToRGB()
    {
        Color Clr = new Color();
        float var_1, var_2;

        if (Saturation == 0)
        {
            Clr.R = (Byte)(Luminosity * 255);
            Clr.G = (Byte)(Luminosity * 255);
            Clr.B = (Byte)(Luminosity * 255);
        }
        else
        {
            if (Luminosity < 0.5) var_2 = Luminosity * (1 + Saturation);
            else var_2 = (Luminosity + Saturation) - (Saturation * Luminosity);

            var_1 = 2 * Luminosity - var_2;

            Clr.R = (Byte)(255 * Hue_2_RGB(var_1, var_2, Hue + (1 / 3)));
            Clr.G = (Byte)(255 * Hue_2_RGB(var_1, var_2, Hue));
            Clr.B = (Byte)(255 * Hue_2_RGB(var_1, var_2, Hue - (1 / 3)));
        }

        return Clr;
    }
}

Used reference: EasyRGB Color Math

Solution 1

Besides the precision issues I think your actual algorithm is incorrect. This should be your FromRGB:

    public static HSLColor FromRGB(Byte R, Byte G, Byte B)
    {
        float _R = (R / 255f);
        float _G = (G / 255f);
        float _B = (B / 255f);

        float _Min = Math.Min(Math.Min(_R, _G), _B);
        float _Max = Math.Max(Math.Max(_R, _G), _B);
        float _Delta = _Max - _Min;

        float H = 0;
        float S = 0;
        float L = (float)((_Max + _Min) / 2.0f);

        if (_Delta != 0)
        {
            if (L < 0.5f)
            {
                S = (float)(_Delta / (_Max + _Min));
            }
            else
            {
                S = (float)(_Delta / (2.0f - _Max - _Min));
            }


            if (_R == _Max)
            {
                H = (_G - _B) / _Delta;
            }
            else if (_G == _Max)
            {
                H = 2f + (_B - _R) / _Delta;
            }
            else if (_B == _Max)
            {
                H = 4f + (_R - _G) / _Delta;
            }
        }

        return new HSLColor(H, S, L);
    }

The next thing you need to understand is that we're taking integer RGB values from 0 to 255 and converting them to decimal values from 0 to 1. The HSL that we get back will thus need to be converted to the normal degree/percent/percent that you're used to. The H value returned should be from 0 to 6 so to convert it to degrees you just multiply by 60. H can actually be negative sometimes so if it is just add 360;

            //Convert to degrees
            H = H * 60f;
            if (H < 0) H += 360;

S and L also need to be multiplied by 100 to give you a percentage from 0 to 100.

UPDATE

This code should get you from HSL to RGB. It assumes that the HSL values are still in their decimal format. Also, I used double instead of float in the code below for better precision.

    public Color ToRGB()
    {
        byte r, g, b;
        if (Saturation == 0)
        {
            r = (byte)Math.Round(Luminosity * 255d);
            g = (byte)Math.Round(Luminosity * 255d);
            b = (byte)Math.Round(Luminosity * 255d);
        }
        else
        {
            double t1, t2;
            double th = Hue / 6.0d;

            if (Luminosity < 0.5d)
            {
                t2 = Luminosity * (1d + Saturation);
            }
            else
            {
                t2 = (Luminosity + Saturation) - (Luminosity * Saturation);
            }
            t1 = 2d * Luminosity - t2;

            double tr, tg, tb;
            tr = th + (1.0d / 3.0d);
            tg = th;
            tb = th - (1.0d / 3.0d);

            tr = ColorCalc(tr, t1, t2);
            tg = ColorCalc(tg, t1, t2);
            tb = ColorCalc(tb, t1, t2);
            r = (byte)Math.Round(tr * 255d);
            g = (byte)Math.Round(tg * 255d);
            b = (byte)Math.Round(tb * 255d);
        }
        return Color.FromArgb(r, g, b);
    }
    private static double ColorCalc(double c, double t1, double t2)
    {

        if (c < 0) c += 1d;
        if (c > 1) c -= 1d;
        if (6.0d * c < 1.0d) return t1 + (t2 - t1) * 6.0d * c;
        if (2.0d * c < 1.0d) return t2;
        if (3.0d * c < 2.0d) return t1 + (t2 - t1) * (2.0d / 3.0d - c) * 6.0d;
        return t1;
    }

Solution 2

Common bug. You've got

    public static HSLColor FromRGB(Byte R, Byte G, Byte B)
    {
        float _R = (R / 255);
        float _G = (G / 255);
        float _B = (B / 255);

Tell me precisely what values of R can result in _R not being 0. (Hint: there's only one).

Edit: you've got the same problem in ToRGB() with 1/3.

Solution 3

The problem I see in your code is the following:

float _R = (R / 255);

You are basically doing integer division here, so you are losing tons of precision.

Try changing it to:

float _R = (R / 255f);

(and the same for the other 2 lines).

Also, to increase precision even more, better to use double instead of float.

Comments

[user1703401]

He already got this answer, just copied his question without using it. stackoverflow.com/questions/4784040/…

[bastijn]

+1 for not directly giving the answer but trying to let the TS see his own fault; which, in the case of a simple answer is in my eyes a better way of securing the TS will not make that fault again :).

[Mervin]

I tried your suggestions but If i put in as example RGB(0,128,0) i get RGB(100,100,100) back after the conversion to and from HSL

[Chris Haas]

@Mervin, let's divide and conquer. Running what I gave you should have given you HSL 2/1/0.25 which is 120° 100% 25%, right? If so, we know that works. Now you just need to fix the HSL to RGB equation. I don't have time to look into that part now but this is the site that I used to fix the RGB to HSL 130.113.54.154/~monger/hsl-rgb.html

[Mervin]

@Hans Passant that is a different question as this one is also about converting back to RGB to follow the HSL and RGB standards correctly.

[Peter Taylor]

@Mervin, two things. Firstly, did you read, understand, and act upon the line starting "Edit"? Secondly, precisely how are the results not good? Unless you give input, expected output, and actual output it's not reasonable to expect anyone to reproduce your problem.

[Mervin]

@ It does indeed return 120,100,25 now , you currently have given me the most useful answer. I'll have a look at the website

[Jörgen Sigvardsson]

I just want to point out that you are comparing floating values for equality, which is extremely unpredictable. Here's a good article: randomascii.wordpress.com/2012/02/25/… TL;DR; Never compare floating point numbers for equality. Use a "near enough" value (a.k.a epsilon): if (Math.Abs(a - b) < epsilon) Console.WriteLine("For all intents and purposes, a equals b")

[Chris Haas]

@JörgenSigvardsson, the only equality comparison that I see is done is as a result of Math.Max() and Math.Min(). Can those functions return floats that differ from their input values in any way? There is also a zero comparison but that's just to avoid a divide by zero exception.

[Jörgen Sigvardsson]

The equality test I see that may be at very big risk is _Delta != 0. I don't believe Math.Max() et al returns anything different from what it got in. I'd find it very strange if they did!

[Bruce Dawson]

Given that the input and output both have 8-bit precision there is absolutely no reason to prefer double over float. float has more than enough precision to get perfect 8-bit results. Of course, all of the math errors and programming errors have to be corrected, and the conversion from float to double needs to use rounding instead of truncation. As an example, if you do this using fixed-point math then as long as the fixed-point constants you use are chosen carefully they only need to have ~8-10 bits of precision -- far fewer than the 24 bits of float or the 53 bits of double.