probonopd / ESP8266HueEmulator

Emulate a Philips Hue bridge running on an ESP8266 using the Arduino IDE.
MIT License
411 stars 93 forks source link

Wrong color conversion? #79

Closed GitHobi closed 6 years ago

GitHobi commented 6 years ago

Hi. I'm just playing around with this emulator. So far everything seems to work fine. However, when using the Hue App, it seems that the colors are somehow "messed up". It is hardly impossible to set a desired color using the Hue App. As far as I can tell, the problem is in the getXYtoRGB function. From my understanding this function should translate the XY values to rgb values in the range of 0.0 to 1.0. - which then, multiplied with the saturation, give rgb values in the range of 0-255. However, the calculation give values outside of this range.

As example: Trying to set dark red: Hue App sends for xy 0.690456 / 0.295907, which is close to a absolute red. Doing the math I'm getting as r=1.6559, g=0.5179 and b=0.2311 ... which end's up in these RBG values red=166, green=132 and blue=59 ... all at all not even a red, but more of a "white with a touch of red". It is no surprise, cause the calculated values are > 255: red = $01A6, green=$0084, blue=$003A - but when converted to uint8_t, only the lower 8 bits are used ...

I double checked with various other pages, but the math is always (more or less) the same ... What am I missing here?

probonopd commented 6 years ago

Not sure, but in any case, Pull Requests welcome if you can figure it out.

mariusmotea commented 6 years ago

This is the function that i use in my lights to convert xy to rgb. It has more conditions copied from Philips SDK:

void convert_xy(uint8_t light)
{
  float Y = bri[light] / 250.0f;

  float z = 1.0f - x[light] - y[light];

  float X = (Y / y[light]) * x[light];
  float Z = (Y / y[light]) * z;

  // sRGB D65 conversion
  float r =  X * 1.656492f - Y * 0.354851f - Z * 0.255038f;
  float g = -X * 0.707196f + Y * 1.655397f + Z * 0.036152f;
  float b =  X * 0.051713f - Y * 0.121364f + Z * 1.011530f;

  if (r > b && r > g && r > 1.0f) {
    // red is too big
    g = g / r;
    b = b / r;
    r = 1.0f;
  }
  else if (g > b && g > r && g > 1.0f) {
    // green is too big
    r = r / g;
    b = b / g;
    g = 1.0f;
  }
  else if (b > r && b > g && b > 1.0f) {
    // blue is too big
    r = r / b;
    g = g / b;
    b = 1.0f;
  }

  // Apply gamma correction
  r = r <= 0.0031308f ? 12.92f * r : (1.0f + 0.055f) * pow(r, (1.0f / 2.4f)) - 0.055f;
  g = g <= 0.0031308f ? 12.92f * g : (1.0f + 0.055f) * pow(g, (1.0f / 2.4f)) - 0.055f;
  b = b <= 0.0031308f ? 12.92f * b : (1.0f + 0.055f) * pow(b, (1.0f / 2.4f)) - 0.055f;

  if (r > b && r > g) {
    // red is biggest
    if (r > 1.0f) {
      g = g / r;
      b = b / r;
      r = 1.0f;
    }
  }
  else if (g > b && g > r) {
    // green is biggest
    if (g > 1.0f) {
      r = r / g;
      b = b / g;
      g = 1.0f;
    }
  }
  else if (b > r && b > g) {
    // blue is biggest
    if (b > 1.0f) {
      r = r / b;
      g = g / b;
      b = 1.0f;
    }
  }

  r = r < 0 ? 0 : r;
  g = g < 0 ? 0 : g;
  b = b < 0 ? 0 : b;

  rgb[light][0] = (int) (r * 255.0f); rgb[light][1] = (int) (g * 255.0f); rgb[light][2] = (int) (b * 255.0f);
}

I will like to help with a PR but in my house all lights use SK6812 neopixels and i don't have any ws2812b in order to test.

probonopd commented 6 years ago

Thanks for your help. As long as the conversion is mathematically resulting in the correct RGB values it should not matter which kind of LCD strip we are using, right?

Yes, a PR would be great.

mariusmotea commented 6 years ago

I will create a PR, hope i will not made any mistakes.

Schischu commented 6 years ago

I'm using the following values (in my own huebridge emulator) which are creating more intensive colors. Meaning pure red is really red, pure green is really green, ... I used the values from Wiki SRGB "The forward transformation (CIE XYZ to sRGB)"

  float r =  X * 3.2406f - Y * 1.5372f - Z * 0.4986f;
  float g = -X * 0.9689f + Y * 1.8758f + Z * 0.0415f;
  float b =  X * 0.0557f - Y * 0.2040f + Z * 1.0570f;

The rest of your code is the same as mine, just these 9 values are different.

Trace (New=myValues, Old=yourValues):

Green:
xyToRGB x:0.211727 y:0.689969
RGB-New   0 254   0
RGB-Old  80 254  45

Red:
xyToRGB x:0.68768 y:0.294417
RGB-New 254   0   0
RGB-Old 254   5  35

Blue:
xyToRGB x:0.153775 y:0.086632
RGB-New   0  67 254
RGB-Old  55  80 254
probonopd commented 6 years ago

Thanks. Want to send a pull request @Schischu?

mariusmotea commented 6 years ago

I also tested the above formula and i'm really impressed. The colors are more accurate.

Schischu commented 6 years ago

Done. :-)

probonopd commented 6 years ago

Merged.