Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deleting segments is not handled properly #4304

Open
1 task done
DedeHai opened this issue Nov 22, 2024 · 4 comments
Open
1 task done

Deleting segments is not handled properly #4304

DedeHai opened this issue Nov 22, 2024 · 4 comments
Labels
backburner low priority compared to other issues bug

Comments

@DedeHai
Copy link
Collaborator

DedeHai commented Nov 22, 2024

What happened?

When having multiple segments, deleting one of them results in broken segment configuration with one exception: if it is the last segment that is deleted.

To Reproduce Bug

I tested like this on a 16x16 matrix, setting-up 3 overlapping segments:

  • Segment 0: initial segment is 16x16
  • Segment 1: overlapping segment 8x16 (start at x=0)
  • Segment 2: overlapping segment 8x16 (start at x=8)

setup any FX you like, I used "Black Hole" on all three segments
delete the middle segment -> the last segment gets broken and has to be deleted before it can work again

Expected Behavior

deleting a segment should not mess up other segments, it should be properly handled.

Install Method

Binary from WLED.me

What version of WLED?

Not working on 14.4 and 15 B7

Which microcontroller/board are you seeing the problem on?

ESP32-C3

Relevant log/trace output

No response

Anything else?

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@DedeHai DedeHai added the bug label Nov 22, 2024
@softhack007
Copy link
Collaborator

This might be a case of "it was always like this", however would be good to understand the root cause before releasing 0.15.0-RC1

@DedeHai
Copy link
Collaborator Author

DedeHai commented Nov 23, 2024

I stumbled upon this when working on the new memory management for the particle system. When deleting the middle segment, there is a particle memory request (which is done when initializing a new FX i.e. call==0) with a segment of nonsensical size of x=0, y=16 which I confirmed is created by deserializeSegment calling the segment setUp() function. So the root cause may be even coming from the UI or incorrect handling of the UI decoding.

@netmindz
Copy link
Collaborator

Sounds like not a blocker for rc1 then

@blazoncek
Copy link
Collaborator

FYI segments are not deleted from memory when user removes them form UI (or using API). Instead they are deactivated.
This behaviour is the same as it was since 0.10 (stop becomes 0), the difference is that since 0.14 segments are stored in vector and not in fixed size array. This is also where isActive() method comes from.
If a user later re-activates such segment (using API) no memory intensive operations happen.

Removing segment when it is deactivated is possible, just remove it from vector and you are done. Be aware that this will trigger copying/moving active segments in memory (from one vector to a new one). Pruning/purging is done if more than half of segments are deactivated to release some memory.

One possible way to work around the issue may be to manually condense vector by overwriting deactivated segments with still active segments (using move constructor) and then prune remaining inactive segments.

As for non-working third segment from example above, only debugging can show what's happening with it. I have the feeling that root issue lies in UI rather than segments themselves. But I may be wrong.

@softhack007 softhack007 added this to the 0.16.0 candidate milestone Nov 27, 2024
@DedeHai DedeHai added the backburner low priority compared to other issues label Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backburner low priority compared to other issues bug
Projects
None yet
Development

No branches or pull requests

4 participants