WLED icon indicating copy to clipboard operation
WLED copied to clipboard

Make rainbow effects more colorful

Open TripleWhy opened this issue 1 year ago • 14 comments

I was playing around with some rainbow effects and was wondering why there was no real yellow color. Cyan and magenta are also mysteriously underrepresented. I think rainbow effects should use the full rainbow spectrum.

If you agree, that is what this PR does.
Here are two images displaying the Rainbow effect on 256 LEDs, captured using the web UI:

rainbows

Top: 0_15 (874b24fb320c6dc16e509514b70965bb4ca7964b)
Bottom: mine (760a1d45124903070a62944faf6f0f3e95e9bbaf)

This works by replacing the linear transitions R -> G -> B in color_wheel by the HSV transition 0° -> 360°.

TripleWhy avatar Jan 14 '24 16:01 TripleWhy

Bravo! That boggled me as well. Approved.

blazoncek avatar Jan 14 '24 17:01 blazoncek

One more thing: As to avoid angering certain users please make the new color wheel calculation optional. Make an entry in LED settings (something like "Use HSV transition for color wheel") and a global selector (WLED_GLOBAL bool hsvColorWheel _INIT(false); ).

blazoncek avatar Jan 15 '24 16:01 blazoncek

Update: Please check colors.cpp and function colorHStoRGB(). You could optimize the code to reuse that function (which itself could be optimised to not use float).

blazoncek avatar Jan 17 '24 08:01 blazoncek

Tested this today but have to say that I fail to see any real difference on my LEDs.

blazoncek avatar Jan 17 '24 16:01 blazoncek

Hm. You are right. There is a difference for me, but I can't really say that one better than the other. My version certainly is a bit brighter (obviously, now that I think about it). I have the cheapest LEDs I could find though.

Possibly there might be a greater difference with higher quality or future LEDs or setups. I mean there clearly is a difference on an LCD screen.

TripleWhy avatar Jan 18 '24 20:01 TripleWhy

Update: Please check colors.cpp and function colorHStoRGB(). You could optimize the code to reuse that function (which itself could be optimised to not use float).

I did actually look at that when I worked on color_wheel and decided against it:

  1. As you said, colorHStoRGB is far from optimal.
  2. colorHStoRGB doesn't preserve the W value... I think?
  3. The computations I used are highly optimized to work with the given assumptions. The computations are reduced so far they are not the direct result of math transformations anymore, they are new formulas that happen to produce the correct result for every possible input. These assumptions that can't be made when colorHStoRGB is used, even if I managed to optimize colorHStoRGB a bit.

If you wish, (and if you want to keep my version in the first place,) I can have a look at what's possible, but it won't be as optimal. It might result in an ever so slightly smaller binary though.
But for that size goal I'd first decide on one version and only keep one.

TripleWhy avatar Jan 18 '24 20:01 TripleWhy

I've already updated code to utilize both as an option. Please review and comment.

blazoncek avatar Jan 18 '24 21:01 blazoncek

I saw that. Some more explicit feedback:

  • I think I wouldn't keep both versions at the same time.
  • Configuration option: I have no idea how this works in WLED, so I can't really comment on that.
  • The setting itself is pretty non-descriptive, so I'd want some more information as a user.
  • colorHStoRGB(): Looks good. Probably doesn't change much though ^^

TripleWhy avatar Jan 18 '24 21:01 TripleWhy

Another option would be to keep the original function for ESP8266, and use the hsv function for ESP32.

I made some comparisons between the color computation of these functions:

D1 Mini

original hsv delta %
exec/ns 429.43 546.26 116.83 127.21 %
flash/bytes 874987 875035 48 100.01 %

WT32-ETH01

original hsv delta %
exec/ns 159.56 170.03 10.47 106.56 %
flash/bytes 1290933 1290957 24 100.00 %

TripleWhy avatar Jan 19 '24 13:01 TripleWhy

I made some comparisons between the color computation of these functions

Are you talking about color_wheel() and colorHStoRGB() or about old vs. new color_wheel()?

blazoncek avatar Jan 19 '24 13:01 blazoncek

color_wheel original vs. mine. The versions of the function I benchmarked didn't contain the first two lines that read palette and currentColor, that's why I said this is about the color computation only.

TripleWhy avatar Jan 19 '24 13:01 TripleWhy

Perhaps change % 256 into & 0xFF and test again. It should provide same result.

blazoncek avatar Jan 19 '24 13:01 blazoncek

According to godbolt's compiler explorer, that produces the same binary

TripleWhy avatar Jan 19 '24 14:01 TripleWhy

According to godbolt's compiler explorer, that produces the same binary

Nice to learn something new. Quite a different ASM to Z80. ;)

So simpler C code produces 27% overhead. Who would've thought.

blazoncek avatar Jan 19 '24 15:01 blazoncek