Problem with Arc option (1d expand FX)

[mitrokun]

What happened?

After the 0.15 b4 update to release I got a bug for the 2D matrix.
The arc modifier stopped working adequately for some
sound effects (waterfall, matripix). The diodes light up only at the edge and there is no animation.
I tried a clean installation and installing the night version 0.16, but it did not help.
Can anyone confirm or deny the existence of the problem?

To Reproduce Bug

Enable the Waterfall effect on the 2D matrix (you can use sine instead mic) and compare the animation with "1d expand FX" modifier "corner" and "arc".

Expected Behavior

Animation on the entire matrix area for both options

Install Method

Binary from WLED.me

What version of WLED?

WLED 0.15.0 (build 2412100)

Which microcontroller/board are you seeing the problem on?

ESP32

Relevant log/trace output

No response

Anything else?

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[mitrokun]

doc_2024-12-22_18-27-50.mp4

[softhack007]

@mitrokun I think this part of the video is your 2D layout, correct?

So the matrix you have is 15 x 13 pixels, and the segment "reverse X" option is enabled.

[softhack007]

@blazoncek it looks like a commit from you has last modified the "arc" logic e82f38e

any ideas?

[mitrokun]

I think this part of the video is your 2D layout, correct?

yes

the segment "reverse X" option is enabled.

You can use any combination of "mirror" and "reverse" settings, the problem still remains. The initial position changes, but the arc waves are missing

[blazoncek]

any ideas?

Yes.

Reverse is at issue here. Why? Because Waterfall (and some other effects) use getPixelColor() for shifting pixels. As Arc is expanding in circular fashion it is not well suited for rectangular matrices as you have to deal with corners somehow. If you want to fill corner in a circular fashion it needs to be encompased within drawing range (and we know that from Pitagora c^2 = a^2 + b^2). But when a (or b) is larger than W (or H) it falls out of draing range.

The change you pointed fixed a complaint that Arc was not filling entire matrix, i.e. corners.

If you want to fix this issue you can either reduce Arc expansion range (and then have reopened original issue when Arc was not filling entire matrix/segment) or fix forementioned effect(s).

[softhack007]

@blazoncek thanks, yes I agree it's most likely the "shifting" (combined with reverse) that does not work in arc with this very small 2D resolution. So basicially it's the same problem that we are fighting in the "pinwheel" expansion - exact getPixelColor is not working sometimes because the reference pixel might actually overlap with other pixels.

I think the best solution would be to add a dedicated pixel buffer for arc and pinwheel, so getPixelColor could retrieve the exact color that was previously sef with setPixelColor.

[blazoncek]

There is another fix possible which will consume a bit more resources. That is - getting pixel at x,y coordinates instead of x,0 (or y,0).
The problem with missing pixels (gaps or ledmap with -1) will still remain though.
BTW pinwheel has its own issues.

[blazoncek]

I think the best solution would be to add a dedicated pixel buffer for arc and pinwheel, so getPixelColor could retrieve the exact color that was previously sef with setPixelColor.

I don't think so. That would add too much complexity.
The simplest solution was already in-place way back in first versions of 0.14 - setUpLeds(). Any effect that relies on previous pixel values should allocate its own buffer to maintain those values independent of pixel mapping (gaps or expansion). To (try to) reduce memory fragmentation an if could be added to check if gaps/ledmaps are present or expansion takes place.

I can prepare such PR if you wish.

[softhack007]

That would add too much complexity.
The simplest solution was already in-place way back in first versions of 0.14 - setUpLeds().

I see your point - and a setupLeds() would also be necessary to further improve drawing speed - I was thinking (for long time) that the complications added by segment level buffering are not worth it. However the truth is, today each pixel has to "go down the rabbit hole" until we deliver it at the bus driver level, and each gPC requires the same deep dive.

Experience from MM - and also from particle effects by @DedeHai - is that we can expect around 30% of speedup, plus we will never again need to worry about gaps that are not mapped to any physical pixels.

Maybe setupLeds() 2.0 should be aware of 1D2D mapping, and create a buffer of SEGLEN pixels as this will also help when using 1D2D expand (for any expand that does not map 1:1 to 2d pixels).

Let's continue the topic after Xmas days 👍 (grandpa currently has higher duties 🎅 )

[blazoncek]

Maybe setupLeds() 2.0 should be aware of 1D2D mapping, and create a buffer of SEGLEN pixels as this will also help when using 1D2D expand (for any expand that does not map 1:1 to 2d pixels).

It does not need to be (explicitly) as virtualLength() deals with that.
The other option is to use effect data buffer (as it was designed for this purpose). Unfortunate consequence may be heap exhaustion in extreme cases or not big enough buffer.

And yes, triple buffering has always been an option. Expensive, though.

[blazoncek]

@mitrokun while you are at it, please check other effect if anyone else also exhibits similar issue.

BTW I have fix for Waterfall ready.

[mitrokun]

BTW I have fix for Waterfall ready.

Tnx

check other effect

Ок.
Surprisingly, the problem is clearly visible only on the two effects I mentioned at the beginning - waterfall and matripix. And both effects were in my presets🫡.