Javascript: Converting HSV to RGB, output correct but shuffled

Question

I've built a function to convert RGB into HSV, using the formula I found on Wikipedia. While the numbers in the output seem correct, their position keeps changing depending on which one's bigger.

Examples:

   Given HSV: [204, 100,  94]
Expected RGB: [  0, 144, 240]
  Output RGB: [240,   0, 144]

   Given HSV: [240, 100,  94]
Expected RGB: [  0,   0, 240]
  Output RGB: [240,   0,   0]

   Given HSV: [120, 100,  94]
Expected RGB: [  0, 240,   0]
  Output RGB: [240,   0,   0]

And this one's only correct because red happens to be the biggest number:

   Given HSV: [  0, 100,  94]
Expected RGB: [240,   0,   0]
  Output RGB: [240,   0,   0]

Here's the function:

function hsvRGB(h, s, v) {
    h /= 60, s /= 100, v /= 100;    // Convert [deg, %, %] to ranges 0-6, 0-1, 0-1

    var r, g, b;                    // Set up for later
    var r1, g1, b1;                 // Set up for later

    var c = v*s;                    // Chroma

    var x = c*(1-Math.abs(h%2-1));

    if ( h <= 0 )       [r1, g1, b1] = [0, 0, 0];
    if ( 0 <= h <= 1 )  [r1, g1, b1] = [c, x, 0];
    if ( 1 < h <= 2 )   [r1, g1, b1] = [x, c, 0];
    if ( 2 < h <= 3 )   [r1, g1, b1] = [0, c, x];
    if ( 3 < h <= 4 )   [r1, g1, b1] = [0, x, c];
    if ( 4 < h <= 5 )   [r1, g1, b1] = [x, 0, c];
    if ( 5 < h <= 6 )   [r1, g1, b1] = [c, 0, x];
    var m = v-c;
    [r, g, b] = [r1 + m, g1 + m, b1 + m];

    return [r*255, g*255, b*255];   // Output 0-255 instead of 0-1
}

I've been checking and re-checking my function against the formula on Wikipedia, and I can't see anything it does that I don't, but I've been looking at this for so long that I may just need a second pair of eyes.

`0 <= h <= 1` does not do what it looks like it might do. You want `0 <= h && h <= 1` — Pointy, Nov 28 '18 at 15:05
Could you not use a `if..else if` there as well? That way you wouldn't need the first check — George, Nov 28 '18 at 15:11
@Pointy Dang, `0 <= h <= 1` seemed to work when I tested it alone, but I guess I didn't test thoroughly enough. You're correct. @George I'm not sure what else I'd use. None of them are `else`. — TanzNukeTerror, Nov 28 '18 at 15:12
@TanzNukeTerror What I mean is do `if(h <= o){} else if (h <=1){} etc` then you wouldn't need the first part `0 <= h` as it would've been caught by the previous block if that was the case — George, Nov 28 '18 at 15:14
@George Oh, I see, I thought you meant "could you not" as in "you're using that when you shouldn't be", my bad. What's there was mostly a straight conversion of the formula on Wikipedia. I was going to work on that once it worked properly. — TanzNukeTerror, Nov 28 '18 at 15:18
@George the algorithm is looking at ranges of values; that's the way the conversion works. Probably the *simplest* thing to do is just check `if (Math.floor(h) == 0)`, `else if (Math.floor(h) == 1)`, etc. — Pointy, Nov 28 '18 at 15:31
@Pointy Couldn't you also use `switch(Math.floor(h))` and `case 0:`, etc.? — TanzNukeTerror, Nov 28 '18 at 15:44
@TanzNukeTerror yes sure, that would work too. The negative `h` case could be handled outside the `switch`, though I'm not sure how/why `h` would be negative. If the incoming `h` is negative it could be corrected by adding `360` since it's an angular value. — Pointy, Nov 28 '18 at 15:47
@Pointy Makes sense. And I _am_ only using 0-360 for the hue. So I guess I could move the negative case above the `\= 60`. I can't mark comments as answers, so if you want to repeat your first comment as an answer, I'll mark that. — TanzNukeTerror, Nov 28 '18 at 16:03

score 1 · Accepted Answer · answered Nov 28 '18 at 16:33

First, the most important issue is that although

if ( 0 <= h <= 1 )

looks reasonable, it doesn't work the way one might think. It's interpreted by JavaScript as if it were written

if ((0 <= h) <= 1)

The JavaScript comparison operators return boolean results, so while it's syntactically correct it's doing something completely different than checking whether h is between 0 and 1.

Because the algorithm is using the H angle of the input value to pick between one of six different scenarios, the whole thing could be done a little bit more simply with a switch statement (as mentioned in a comment on the question). First, the code as written handles the case of h being negative, which is fine, but since it's an angular value it's safe to just force it into the range [0, 360):

function hsvRGB(h, s, v) {
    while (h < 0) h += 360; // could be smarter but just for illustration
    h = h % 360;
    h /= 60, s /= 100, v /= 100;    // Convert [deg, %, %] to ranges 0-6, 0-1, 0-1

So now h is between 0 and 6; it might be 0 but it won't ever be exactly 6. We can then use a switch to distinguish the cases:

    switch (Math.floor(h)) {
      case 0: [r1, g1, b1] = [c, x, 0]; break;
      case 1: [r1, g1, b1] = [x, c, 0]; break;
      case 2: [r1, g1, b1] = [0, c, x]; break;
      case 3: [r1, g1, b1] = [0, x, c]; break;
      case 4: [r1, g1, b1] = [x, 0, c]; break;
      case 5: [r1, g1, b1] = [c, 0, x]; break;
    }

Javascript: Converting HSV to RGB, output correct but shuffled

1 Answers1