Aircoookie / WLED

Control WS2812B and many more types of digital RGB LEDs with an ESP8266 or ESP32 over WiFi!
https://kno.wled.ge
MIT License
14.61k stars 3.14k forks source link

FastLED CRGB[] (leds) support #2662

Open Aircoookie opened 2 years ago

Aircoookie commented 2 years ago

Is your feature request related to a problem? Please describe. It is some effort for users to port effects written for the FastLED library to WLED.

Describe the solution you'd like Add support for using the CRGB leds[] array from within WLED effect functions, calling setPixelColor() and getPixelColor() automatically via [] operator override

Describe alternatives you've considered Current WLED effect code flavor and upcoming custom FX scripting does not make it easier to port FastLED effects.

Additional context https://stackoverflow.com/questions/37043078/c-overloading-array-operator

blazoncek commented 2 years ago

leds[] array is in fact double-buffering. It can be global/permanent or local/effect specific (temporary).

Unfortunately most often leds[] represent RGB array stripping away W support WLED has.

Still, using leds[], global or local, can benefit effect porting since LED/RGB data is retained (losslessly) between invocations of the effect function which is it's most common use.

Actually there is no need to overload [] operator since we only need an index to leds[]. Currently most 2D implementations use XY() function that produces an index from X and Y coordinates of a pixel. This requires a knowlege of pixel position on a strip in relation to the layout of the matrix in case global leds[] is used which can be cumbersome. Using local leds[] removes that requirement but requires knowing segment dimensions (which are available since segment is a member class of a strip!). In such case XY() only needs to know width of a segment.

I have implemented 2D support for WLED that uses local leds[] array in a sample 2D effect bundled with it.

Aircoookie commented 2 years ago

Of course leds[] in its standard form is double buffering. This overload idea would allow for using leds[] without using double buffering or allocating extra memory. I think double buffering is essential to have, especially with 2D where you often have to limit brightness, but I'm not yet sure whether it should occur on the effect or bus level:

Effect level benefits:

Bus level benefits:

blazoncek commented 2 years ago

Sorry, I misunderstood the original post. 😄

Overloading leds[XY(x,y)] with a setPixelColor/getPixelColor is a nice idea. Though, rarely needed since leds[] serves its own purpose IMO. If using global leds[], strip.show() could do the setPixelColor and getPixelColor could be replaced to return leds[] (since it is used in handful effects). If using local leds[] a for() loop is needed to do setPixelColor at the end of effect function, which can be shortened as SEGMENT.show().

blazoncek commented 2 years ago

I have been porting FastLED effects (25 currently) to WLED using dynamic leds[] array and it only takes 8 lines of code to make simple effects work.

blazoncek commented 2 years ago

One more note regading global leds[].

WLED handles color in uint32_t whereas CRGB is effectively uint24_t and as such discarding/loosing W channel.

If leds[] is implemented as a global to allow benefits from above and to serve as a double buffer it will mean sacrificing W channel for portability. This may not be desirable in every circumstance.

The additional benefit of global leds[] may be (depending on effect implementation) support for overlapping segmens and blendinng their effects.

blazoncek commented 2 years ago

@ewoudwijma has started work on this here.

So the recommendation would be to overload [] and =operators to support setPixelColor and getPixelColor

ewoudwijma commented 2 years ago

Eventually it turned out we removed leds from effect functions all together and only have leds under the hood in sPC and gPC. So using fastled code into WLED requires changing to sPC and gPC. Good thing is gPC is not applying brightness on earlier sPC (due to leds array under the hood).

Question is if this is enough if we still need to add leds overloads? I personally think not as we keep code more straightforward by only allowing sPC and gPC. What do you think?

blazoncek commented 2 years ago

Although I would prefer WLED way of writing effects (using setPixelColor() & getPixelColor()) most people familiar with FastLED will find it awkward. Unfortunately it is true that converting FastLED effect for WLED is not really straightforward and leds[] alone will not make it possible to port anything but simplest effects.

blazoncek commented 1 year ago

Double buffering done in #3280

blazoncek commented 10 months ago

@Aircoookie do we keep this open? I know setPixelColor/getPixelColoris not real leds[] substitute but IMO adding C++ overloaded operators will be too much work for too little effect as FastLED effects cannot all be directly ported to WLED.

softhack007 commented 10 months ago

My opinion is, let's close it.

Even if we can use some tricks to implement an "array index" operator, it won't allow to do all the fancy things that are possible with FastLED leds[].

blazoncek commented 10 months ago

Just a thought: If we ditch global brightness then we no longer need to care about lossy pixel restoration. And we can play with pixel arrays directly.