Adding Particle System with many new FX #4506
Conversation
not yet final version but working
update from another commit that got lost
this somehow also got lost from an earlier commit
particle box now is more random in random mode (still a work in progress) firework is now more configurable by sliders
particle attractor animation does not work, somthing wrong with pointer allocation, it worked with static variables
still unknown, why more than 256 particles are needed in memory allocation to not make it crash, but it works for now
at the expense of more ram usage, animations now have more options for color control (already used in fireworks now)
also fixed a bug in fire animation
improved efficiency for stackup (pushback), added code to correctly determine direction if particles meed (probably overkill but now its there)
untested, need to verify it works
…ables to 32bit for faster calculation 32bit variables are faster on ESP32, so use them whenever a variable is used a lot, it saves one instruction per access.
it made very little difference in performance, but for ESP8266 it may matter so it is set permanently there. graphically the difference is also very small (sometimes a particle gets brighter or less saturated)
also tweaked some of the FX
particle pile-up did not work correctly, now fixed
…ptions to perlin noise
|
tested combinations that work with ESP8266 without touching JSON buffer size: the only thing that does not work is enabling both 1D and 2D particle system, which is now prohibited by an #ifdef. Enabling the replaced FX still produces a working UI. |
|
@netmindz regarding
the way I understand the |
|
Updated to play nice with blending styles. @blazoncek regarding
that (intermediate) disabling is necessary to not have one FX fade out and the other fade in as that is not how PS transitions work anymore when "fade" style is set. It is also only disabled during the buffer transfer. I don't see a better way except if another blending style specifically for the PS is added, but that is just confusing. If you have any other good ideas, I am open to suggestions. |
I do not know if it is a "good idea" but am just thinking out loud: If done outside of PS it is as simple* as |
|
thanks for the input. I agree it would be better to do blending in one place BUT: blending between FX inside the PS is only done for one segment, not multiple. The reason it does that is to share the buffer AND the particles for one segment: if you had your proposed scenario, each segment would require two render buffers and twice the particles, which would exceed RAM already at 32x32 and multiple segments + transitions. Disabling the "particle blending" is as easy as changing two or three if's as it already disables it when not using 'fade' style. The "two PS FX can interact" part is more of an addition than a necessity, it could be a user selectable option as well. |
|
@netmindz feel free to squash-merge this |
…, minor tweaks - collisions are now also velocity based in 1D, there was a bug that prevented that from working well (wrong collision distance calculation) - improvement and bugfix in 2D collision distance calculation - added distance based pushing in 2D (instead of only using the dotproduct) the combination of improved distance calculation and proper pushing make collisions a lot better in all tested FX
Sorry I wasn't sure what you meant by this and got distracted before I replied. Was this just a note to use squash when I merge, but not yet or a request for merge with squash now? |
|
Just a suggestion to squash merge so commits are compacted. Merge when you took a look at it, any remaining issues are minor and can be worked out with more feedback from users |
|
@netmindz regarding the max number of FX: I think increasing |
|
@DedeHai there are numerous old effects that can be removed (and replaced with other existing effects with added sliders and options) some of which you already modified or consolidated. I would recommend to do that. These are: 11, 14, 19, 52, 53, 60, 77, 93, 94, 99, 102, 103, 117 |
Are you aware of all consequences? I guess not otherwise you wouldn't make such a bold claim. |
I am not. please do enlighten me: which is the tricky part? |
|
Sync etc. |
if a PS FX is partially overlapping, it will render in overlay mode
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
You can disable this status message by setting the WalkthroughThe changes update the build configuration and particle effects support. The platformio.ini file now includes new build flags to disable particle systems in various environments. The FX module has been enhanced by replacing the previous MODE_COUNT definition with a new particle mode and adding multiple new particle effect macros. A new method (loadOldPalette) was added to the Segment class, and a new header file introduces ParticleSystem2D and ParticleSystem1D classes with associated update, emission, and allocation methods. Additionally, a minor formatting adjustment was made in the configuration file. Changes
Sequence Diagram(s)sequenceDiagram
participant Core as WLED Core
participant Seg as Segment
participant PS as Particle System
Core->>Seg: Trigger animation update
Seg->>Seg: Check transition state
alt Transition Active
Seg->>Seg: Call loadOldPalette() → loadPalette()
end
Seg->>PS: Update particle effects
PS->>PS: Process update (emit particles, apply forces, update state)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
wled00/FXparticleSystem.h (4)
1-9: Consider renaming the file header.
The file name and initial comment mention "FXparticleSystem.cpp," but the actual file appears to be a header (".h"). Ensuring consistency between file name, top-level comment, and included references will help avoid confusion.
33-44: Evaluate bitfield usage for booleans.
In the 'partMem' struct, multiple booleans are used (e.g., finalTransfer, transferParticles), but only some entries are grouped into bitfields (watchdog is an 8-bit integer). Converting more booleans into bitfields or combining them could reduce memory footprints, which is crucial on ESP8266.
158-163: Consider a destructor or explicit memory freeing.
The ParticleSystem2D constructor sets up external memory pointers (particles, sources, etc.). Although this is typically managed externally, ensure there's no scenario where the system is allocated but never freed.
210-220: Reduce the size of collision logic for performance.
Methods like handleCollisions() could be large, especially if the device must handle many particles. Consider partitioning collisions into spatial buckets or using more efficient data structures if performance degrades with high particle counts.wled00/FX.h (3)
325-353: Document newly introduced particle effect modes.
These macros (FX_MODE_PARTICLEVOLCANO to FX_MODE_PS1DSONICSTREAM) add numerous effect modes. In future maintenance, ensure they are well-documented in user-facing material (UI labels, docs) so that users understand their distinctive behaviors.
510-510: Clarify the padding purpose.
This inline comment merely notes a padding byte in the Transition struct. Consider expanding the comment to indicate why it is necessary (alignment vs. future usage) so future maintainers do not remove it inadvertently.
606-607: Rename getmodeBlend() for consistency.
The static function getmodeBlend() returns a boolean. For clarity, consider renaming it to isModeBlend(), better reflecting its true/false return.-inline static bool getmodeBlend(void) { return _modeBlend; } +inline static bool isModeBlend(void) { return _modeBlend; }wled00/FX_fcn.cpp (2)
14-14: Consider moving memory service definition to FX.h as suggested by TODO comment.The TODO comment indicates that memory service functionality should be defined in FX.h. This would improve code organization by keeping related functionality together.
474-478: Add null pointer check in loadOldPalette.While the implementation is clean and focused, consider adding a null pointer check before accessing
_tto prevent potential crashes.void Segment::loadOldPalette(void) { - if(isInTransition()) + if(isInTransition() && _t != nullptr) loadPalette(_currentPalette, _t->_palTid); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
platformio.ini(6 hunks)wled00/FX.h(5 hunks)wled00/FX_fcn.cpp(4 hunks)wled00/FXparticleSystem.h(1 hunks)wled00/cfg.cpp(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- wled00/cfg.cpp
🔇 Additional comments (8)
wled00/FXparticleSystem.h (5)
56-58: Validate speed limiting range.
The limitSpeed() function clamps speed between ±PS_P_MAXSPEED (120). Verify that 120 is an acceptable limit for all hardware targets and does not break any intended behaviors for advanced or large displays.
94-102: Review struct alignment and potential padding in PSparticle.
This struct is 10 bytes. Depending on compiler alignment rules, it might insert padding if used in arrays or forced alignment. Carefully confirm how the memory manager and transitions handle partial alignment to avoid subtle bugs.
135-140: Review advanced particle size logic.
The PSsizeControl struct introduces multiple bitfields and flags (e.g., grow, shrink, pulsate). Verify all size manipulations are tested thoroughly, especially edge cases with maxsize or 0.
200-205: Confirm that advanced pointers gather correct offsets.
PSadvancedParticle and PSsizeControl pointers are marked nullable. Validate that code paths where they are null do not lead to accidental dereferencing.
249-253: Validate memory constraints for large 2D arrays.
allocateParticleSystemMemory2D can allocate up to thousands of particles for large segments. For lower-memory devices (1M/2M ESP8266), this might cause out-of-memory errors. Confirm compile-time guards or runtime checks are sufficient.wled00/FX.h (1)
655-655: Ensure backward compatibility in loadOldPalette().
This new method loads an “old FX palette” into the current palette. Verify it correctly handles older configurations or user data to prevent color mismatches, especially during transitions.wled00/FX_fcn.cpp (1)
1556-1558: LGTM! Good use of conditional compilation.The particle system memory handling is properly guarded by conditional compilation, allowing it to be disabled when not needed.
platformio.ini (1)
370-370: LGTM! Appropriate handling of memory constraints.The build flags to disable particle systems are correctly applied to memory-constrained environments, particularly ESP8266 boards. This aligns well with the PR objectives to manage memory usage.
Also applies to: 380-380, 388-388, 397-398, 407-408, 415-416, 426-427, 436-437, 445-446
this replaces the previous PR (#3823) since I made too many changes and a rebase was needed.
Some info on the particle system:
there are three compile time defines
Replaced FX
to save on flash, effects that have a particle system based replacement are disabled. This saves 12k of flash. Users who want the originals back can custom compile using compile flag.
Known issues:
Summary by CodeRabbit