Makuna / NeoPixelBus

An Arduino NeoPixel support library supporting a large variety of individually addressable LEDs. Please refer to the Wiki for more details. Please use the GitHub Discussions to ask questions as the GitHub Issues feature is used for bug tracking.
GNU Lesser General Public License v3.0
1.18k stars 264 forks source link

All channels in Rgb48Color initialised from R channel in RgbColor #686

Closed blazoncek closed 1 year ago

blazoncek commented 1 year ago

https://github.com/Makuna/NeoPixelBus/blob/d212d3ac5cad3d33785aea838160d11e17111eb2/src/internal/colors/Rgb48Color.h#L60

I may be wrong, but this does not seem correct to me.

Makuna commented 1 year ago

Run the calculation with some sample data:

R = 255 (max uint8_t) = 255 257 = 65535 (max uint16_t) - correct R = 0 (min uint8_t) = 0 257 = 0 (min uint16_t) - correct R = 1 = 1 257 = 257 - close, expect 256 to be precise R = 127 = 127 257 = 32639 - close, expect 32767 to be precise

So, look at the arduino map() method and you will see that it factors down to the same...

long map(long x, long in_min, long in_max, long out_min, long out_max)
{
  return (x - in_min) * (out_max - out_min) / (in_max - in_min) + out_min;
}

call like

uint16_t map (127, 0, 255, 0, 65535);

out = (127 - 0) (65535 - 0) / (255 - 0) + 0; out = 127 65535 / 255; // factor our zeros to get this out = 127 * 257; // factor out consts (other than 127 input) to get this out = 32639!

It's a fast approximation without branching or additional calculations.

Makuna commented 1 year ago

Ok, some days I need to get my coffee before reading/answering. YES, you are correct.

Makuna commented 1 year ago

https://github.com/Makuna/NeoPixelBus/pull/687

blazoncek commented 1 year ago

Ok, some days I need to get my coffee before reading/answering. YES, you are correct.

I know. I am the same. 🤣

Makuna commented 1 year ago

v2.7.5