WLED icon indicating copy to clipboard operation
WLED copied to clipboard

Squashed commit of FXparticleSystem

Open DedeHai opened this issue 1 year ago • 30 comments

FX particle system ready for review.

DedeHai avatar Mar 14 '24 15:03 DedeHai

Unfortunately on my dev build:

Linking .pio/build/wemos_d1_mini32_debug/firmware.elf
Retrieving maximum program size .pio/build/wemos_d1_mini32_debug/firmware.elf
Checking size .pio/build/wemos_d1_mini32_debug/firmware.elf
Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
Error: The program size (1578750 bytes) is greater than maximum allowed (1572864 bytes)
RAM:   [===       ]  26.6% (used 87072 bytes from 327680 bytes)
Flash: [===*** [checkprogsize] Explicit exit, status 1
=======]  100.4% (used 1578750 bytes from 1572864 bytes)

blazoncek avatar Mar 16 '24 14:03 blazoncek

Screenshot 2024-03-16 at 15 50 59 A bit long names...

blazoncek avatar Mar 16 '24 14:03 blazoncek

Suggestions:

  • fire tends to burn on the left side of the matrix
  • waterfall tends to splay on the right hand side
  • rotating spray would benefit of random alternating rotation
  • volcano never fills entire height
  • fireworks might be better to have white firing pojectile

blazoncek avatar Mar 16 '24 15:03 blazoncek

Looks extremely good. The only downside is hefty increase in image size and slow FPS. None of the effects achieve more than 50-58FPS on 20x20 matrix on ESP32 (desired FPS was 80).

blazoncek avatar Mar 16 '24 15:03 blazoncek

-On my build (ESP 32 S3 with AR) the increase in code is 1% or about 16kB. Do you have a map file of sorts? 1.5MB of code is huge for an embedded system... do you know what uses the largest slices of space? Maybe there is room to optimize somewhere else to make it fit? -FPS can be increased by limiting the number of particles. I did not target 80FPS but 40FPS which IMHO is enough for the effects to look smooth. But parameters can be adjusted to increase it. Should I take a look at that? I can test on ESP32 S3 with 32x16. -Effect Names are TBD I put 'particle' in every name just so I can easily filter them in the list. Open to suggestions. -I will take a look at your suggestions (some may be bugs: fire, waterfall) -Fireworks in white is a good idea, as 'exhaust' is only orange if using rainbow palette (I added this effect before adding the possibility to set saturation) -Volcano: I can adjust the settings range, I tested on 16x16 where it goes to top. Any matrix target size to optimize for? I assumed 8x8 as a minimum but no maximum height. suggestion: slider settings such that 32x32 (I can test on w16 x h32) is still filled (some other animations may also need adjustment for that)

DedeHai avatar Mar 17 '24 06:03 DedeHai

I looked at your other points:

  • fire tends to burn on the left side of the matrix

cannot reproduce. any specific settings this happens?

  • waterfall tends to splay on the right hand side

this is something I already noticed, 'inequality' happens also in other animations. I think the reason is somewhere in the collision handling and integer math favouring that direction. So far I could not figure out what it is exactly. but it has nothing to do with the 'pushing' part as it also happens if I disable particle pushing.

  • rotating spray would benefit of random alternating rotation

There is an option checkbox left in that animation, so I can add it. I put it on my todo list.

DedeHai avatar Mar 17 '24 15:03 DedeHai

any specific settings this happens?

no. fire was definitely more prominent on the left side of the matrix. default values.

blazoncek avatar Mar 17 '24 20:03 blazoncek

  • volcano never fills entire height

I set my 32x16 matrix to vertical (w15 x h32) and volcano fills to the top easily if particles speed is set high. But depends on collision settings and bounceX setting.

DedeHai avatar Mar 18 '24 18:03 DedeHai

I looked at the possibility to limit the total amount of particles to make transistions faster but that is not really an option. fire on a wide matrix (32 pixels in my case) uses A LOT of particles to make it look good, over 1000 (this may be excessive, can probably be somewhat optimized). Also the rotating particlespray uses 700 particles (I tuned this with 16x16 matrix). So it could be limited to about 1000 but that will not help if transitioning between effects that use collisions and less particles (like particle box and volcano for example). So not a good solution without crippling some of the FX...

DedeHai avatar Mar 18 '24 18:03 DedeHai

Let's see if @Aircoookie agrees on this PR. It gets my approval, except for the long names.

IMO the next iteration would be to try to replace older effects (1D) with particle system if the resulting code size or performance would be better.

blazoncek avatar Mar 19 '24 13:03 blazoncek

any specific settings this happens?

no. fire was definitely more prominent on the left side of the matrix. default values.

I was able to reproduce this now. I think I fixed it, will be in upcoming updates. Turns out random16() is not random enough for some things

DedeHai avatar Mar 29 '24 09:03 DedeHai

Unfortunately on my dev build:

Linking .pio/build/wemos_d1_mini32_debug/firmware.elf
Retrieving maximum program size .pio/build/wemos_d1_mini32_debug/firmware.elf
Checking size .pio/build/wemos_d1_mini32_debug/firmware.elf
Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
Error: The program size (1578750 bytes) is greater than maximum allowed (1572864 bytes)
RAM:   [===       ]  26.6% (used 87072 bytes from 327680 bytes)
Flash: [===*** [checkprogsize] Explicit exit, status 1
=======]  100.4% (used 1578750 bytes from 1572864 bytes)

Similar observation on the 8266 github build - this feature pushed FLASH (program) size over the limits.

I think it would be best to have a build flag to disable the particle system feartures - similar to WLED_DISABLE_2D.

This would allow to build some environments without particle system, and have some program space left for other things like usermods.

softhack007 avatar Apr 08 '24 19:04 softhack007

The latest push will not build successfully on ESP8266.

blazoncek avatar May 06 '24 06:05 blazoncek

I think it would be best to have a build flag to disable the particle system feartures - similar to WLED_DISABLE_2D.

I added WLED_DISABLE_PARTICLESYSTEM that can be used as a build flag.

DedeHai avatar May 07 '24 20:05 DedeHai

Please undo changes in platformio.ini and FX_fcn.cpp as they are irrelevant for this PR.

blazoncek avatar May 22 '24 06:05 blazoncek

It's been a while... I promise to take a second look when I get back from vacation next week.

@DedeHai can you check and update any outstanding issues until then?

blazoncek avatar Jun 22 '24 08:06 blazoncek

You are doing something wrong when you are merging upstream into your fork. You are messing commit history as it appears you are overwriting upstream files.

blazoncek avatar Jul 06 '24 08:07 blazoncek

You are doing something wrong when you are merging upstream into your fork. You are messing commit history as it appears you are overwriting upstream files.

it's probably the order I do this. I first pull latest to my working branch, then do a squash commit to the PR branch. so history is preserved in my working branch but gets messed on the squash commit. What is the proper way to do the squash? OR: can you live with it for now? I can do a final, correct 'release' PR once it is all ready.

DedeHai avatar Jul 06 '24 08:07 DedeHai

then do a squash commit to the PR branch

Oh! You don't do that. Squash commit is only for the upstream PR. Squash commit strips all history. We'll fix that later. I hope to get to it today or tomorrow.

blazoncek avatar Jul 06 '24 08:07 blazoncek

Unfortunately the code is too big for my dev environment. Will need to strip something.

Flash: [====*** [checkprogsize] Explicit exit, status 1
======]  101.2% (used 1591370 bytes from 1572864 bytes)

blazoncek avatar Jul 07 '24 19:07 blazoncek

@DedeHai a while ago I managed to do some testing and found:

  • Spray
  • Chase
  • Dancing Shadows
  • Fireworks 1D
  • Ghost Rider To behave somewhat odd and nowhere near original counterparts (if they have them). Still, it was just a brief testing.

The good thing is there were no crashes and all effects worked. The sad reality is that the code size is still huge.

blazoncek avatar Aug 05 '24 19:08 blazoncek

Hi @DedeHai, thanks for making this cool feature availeable to WLED 😃

I've just started to try particlefx, and I'd like to work with you on the code to make it fit for merging into 0_15. Maybe we find a few optimizations wrt. speed or size, let's see..

Actually I'm a bit busy IRL for the next weeks, so my comments and proposals might come spread over time.

A few observations for a start

  • I think something went wrong with re-basing the PR to our 0_15 branch, maybe @blazoncek you could help out?
  • 1D effects do not work well in 1D2D expand mode "pixels" - often just the first 128 to 256 pixels (or so) get animated, the rest stays black.
  • some 2D effects do not use full panel height (tested on 64x64): ps fire, ps fireworks, ps volcano, ps equalizer
  • ps waterfall is a bit slow, maybe the default for "intensity" should be reduced.
  • speed and size: usually it better that local variables are only int or unsigned. In our experience with other effects, the 16bit and 8bit types make the firmware bigger and slower. I guess the reason is that the compiler has to emulate 8bit registers, so any op on 8bit or 16bit actually requires additional instructions to guarantee exact 8bit behaviour.

For discussion : the particle systems code seems to push some builds over the "100% program space" limits. I think we can do something for esp32 by using link-time optimizations (-flto). Not sure if it works with the "old" 3.x framework, too. The problem is 8266. 8266 uses -flto by default, so I don't see how to make room for the particle code on 8266. I would suggest to only enable particle effects on esp32 and variants, and not include them on 8266. @DedeHai @blazoncek what do you think?

softhack007 avatar Aug 13 '24 18:08 softhack007

My first suggestion is to remove PixelMagic tool from the build. Then we can start optimising other stuff, but it is going to be a tough call if you want to include any usermods.

@softhack007 the problem is with squash commits that @DedeHai performed. IMO the best way to fix this will be to manually port modifications (using stashes and diffs) to a new branch or try to merge 0_15 normally into this branch. I can try the second method in a few days.

blazoncek avatar Aug 13 '24 19:08 blazoncek

Thanks for the feedback @softhack007, much appreciated. Can't comment too much RN as I am on vacation. 1D effects are not as 'ripe' as 2D as I have limited testing capability (no long strips). Parameters of FX could use some fine tuning. 2D FX I tested up to 32x16 only, on larger ones CPU and number of particles (ram limit) may be insufficient and also need optimization.

DedeHai avatar Aug 13 '24 19:08 DedeHai

IMO the best way to fix this will be to manually port modifications (using stashes and diffs) to a new branch or try to merge 0_15 normally into this branch. I can try the second method in a few days.

I have my dev branch with correct history, I can create a branch from that and squash it properly next week.

DedeHai avatar Aug 13 '24 19:08 DedeHai

@softhack007 I am back. let me comment some more. 64x64 may be pushing it in general, for the particle FX to look good at least one particle per 2 pixels would be nice, but the total number is limited. I could easily increase it but then during transitions, FX assigned ram will be depleted. Currently I limited the maximum amount of particles such that at least to PS animations can run during transitions. If that is dropped, the amount can be doubled. Fire also does not look good on my 40x30 matrix, so I have plans to ditch that limit, not sure how transitions are going to look... regarding 1D effects: any specific settings I should look at? also @blazoncek mentioned, some of the replacement effects do not look like the originals (they are a close match on my test setup).

Regarding code size: as I mentioned before, when looking at the elf file there seems to be room for optimisation, especially the 'printf' variants use quite a lot of flash, over 200k IIRC. maybe these can somehow be compacted.

DedeHai avatar Aug 21 '24 17:08 DedeHai

@softhack007 since flash space is still scarce I am thinking about moving the particle system into a usermod. That way it could be officially released as a beta version for more users to test. If we later find a good way to reduce code size the UM could just be part of the standard build or it could be moved back. What do you think?

DedeHai avatar Sep 05 '24 05:09 DedeHai

I am thinking about moving the particle system into a usermod.

Hi @DedeHai, I think the more future-proof solution would be to change effect IDs from byte to uint16_t. This way we have enough room so that any usermod effects could get their own ID's, maybe we could even allow for "effect ID namespaces" where each usermod has it's own apportionment (using the usermod ID as part of the higher byte).

For a short term solution, just allowing uint16_t for effect ID's would enable us to go ahead without creating a dedicated usermod for particle effects. It should still be possible (my preference) to disable your effects at compile time, for custom builds where particleFX might be "too much" for a specific use case.

It would cost us one byte per segment, so in total of 32 bytes extra. Maybe the effect table needs a redesign, too.

@blazoncek @DedeHai what do you think?

softhack007 avatar Sep 29 '24 21:09 softhack007

For a short term solution, just allowing uint16_t for effect ID's would enable us to go ahead

The main issue are not IDs but flash use. Latest optimisations in #4138 and #4167 look promising to include PS into main branch as they are.

As for the future, effects are stored in vector, currently with gaps included, which may allocate a lot of RAM.

blazoncek avatar Sep 30 '24 17:09 blazoncek

If we change from byte to uint16 then we will also need to change the dmx code as effect mode is limited to 255.

I would be in favour of the change as with the Animatrix usermod enabled I think that already takes us up into the 240 effects range

You could also introduce the idea of "pages" so we have up to 255 effects per page and that page is a certain category of effect

netmindz avatar Oct 02 '24 08:10 netmindz