-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Updates to particle system #4630
base: main
Are you sure you want to change the base?
Conversation
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
touch is implemented differently on S2 and S3, these changes make touch buttons work on S2 and S3
now better fits the default threshold value of 32
This reverts commit d21ad8e.
This reverts commit 0904155.
-removed particle box 'rocking boat' (buggy) and replaced with random sloshing. -removed comments -changed some variables into 32bit for speed boost on ESP32 -added link to KB at the top of FX.cpp
…, 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
if a PS FX is partially overlapping, it will render in overlay mode
- slight improvement to 2D collision code efficiency - added faster "division" to C3/ESP8266 by using a right shift trick (biasing towards 0 also for negative numbers by applying proper rounding)
- improved Balance and Hourglass FX - bugfix in 1D collisions - minor improvements / fixes
added velcity lookahead, improved calculation speed for 1D collisions (only about 1% FPS increase but still an improvement)
WalkthroughThis pull request updates the particle effects system. In the primary effects file, the fireworks mode now uses hardware-based randomization for velocity and saturation, and two new modes—sonic boom and springy—are introduced. The corresponding header file updates the effect mode definitions and the overall mode count. In the particle system implementation, function signatures have been refined by changing parameter types (e.g., from Changes
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.31.1)wled00/FX.cpp📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Definitions (1)wled00/FX.cpp (5)
⏰ Context from checks skipped due to timeout of 90000ms (16)
🔇 Additional comments (18)
✨ Finishing Touches
🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
wled00/FX.cpp (1)
9776-9776
: Minor typo in comment.There's a typo in the comment "anable motion blur" instead of "enable motion blur".
-PartSys->setMotionBlur(7 + ((SEGMENT.custom3) << 3)); // anable motion blur +PartSys->setMotionBlur(7 + ((SEGMENT.custom3) << 3)); // enable motion blurwled00/FXparticleSystem.h (1)
346-346
: Inconsistency in function formattingThe function signature changed but doesn't appear to have substantive changes to the parameters. This might be just a code formatting change for consistency.
wled00/FXparticleSystem.cpp (1)
914-917
: Enhanced collision detection with velocity lookahead.The collision detection has been significantly improved by including velocity components in proximity calculations. Instead of just comparing current positions, the code now predicts where particles will be in the next frame by adding velocity to position. This results in more accurate collision detection, especially for fast-moving particles that might otherwise pass through each other between frames.
Consider adding a comment explaining the lookahead approach to make the intention clear to future maintainers.
- int32_t dx = (particles[idx_j].x + particles[idx_j].vx) - (particles[idx_i].x + particles[idx_i].vx); // distance with lookahead + // Calculate predicted position difference using lookahead (position + velocity) for more accurate collision detection + int32_t dx = (particles[idx_j].x + particles[idx_j].vx) - (particles[idx_i].x + particles[idx_i].vx); // distance with lookahead
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
wled00/FX.cpp
(13 hunks)wled00/FX.h
(1 hunks)wled00/FXparticleSystem.cpp
(57 hunks)wled00/FXparticleSystem.h
(3 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
wled00/FX.cpp (4)
wled00/fcn_declare.h (15)
hw_random16
(539-539)hw_random16
(539-539)hw_random16
(540-540)hw_random16
(540-540)hw_random16
(541-541)hw_random16
(541-541)sin16_t
(574-574)sqrt32_bw
(587-587)perlin8
(525-525)perlin8
(526-526)perlin8
(527-527)hw_random
(536-536)hw_random
(536-536)hw_random
(537-537)hw_random
(538-538)wled00/FXparticleSystem.h (1)
initParticleSystem1D
(413-413)wled00/wled_math.cpp (4)
sin16_t
(65-76)sin16_t
(65-65)sqrt32_bw
(225-246)sqrt32_bw
(225-225)wled00/util.cpp (10)
perlin8
(785-787)perlin8
(785-785)perlin8
(789-791)perlin8
(789-789)perlin8
(793-795)perlin8
(793-793)hw_random
(608-612)hw_random
(608-608)hw_random
(614-620)hw_random
(614-614)
wled00/FXparticleSystem.cpp (1)
wled00/FXparticleSystem.h (17)
part
(163-163)part
(169-169)part
(170-170)part
(173-173)part
(176-176)part
(344-344)part
(346-346)part
(348-348)particleindex
(171-171)particleindex
(174-174)particleindex
(178-178)particleindex
(214-214)particleindex
(381-381)x
(185-185)x
(354-354)particle1
(218-218)particle1
(386-386)
🪛 Cppcheck (2.10-2)
wled00/FX.cpp
[error] 10320-10320: Invalid abs() argument nr 1. A non-boolean value is required.
(invalidFunctionArgBool)
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: wled_build / Build Environments (nodemcuv2_compat)
- GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
- GitHub Check: wled_build / Build Environments (nodemcuv2_160)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_160)
- GitHub Check: wled_build / Build Environments (esp32dev)
- GitHub Check: wled_build / Build Environments (esp32_wrover)
- GitHub Check: wled_build / Build Environments (esp8266_2m_compat)
- GitHub Check: wled_build / Build Environments (esp32dev_V4)
- GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
- GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
- GitHub Check: wled_build / Build Environments (usermods)
- GitHub Check: wled_build / Build Environments (lolin_s2_mini)
- GitHub Check: wled_build / Build Environments (esp32c3dev)
- GitHub Check: wled_build / Build Environments (esp8266_2m_160)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_compat)
- GitHub Check: wled_build / Build Environments (nodemcuv2)
- GitHub Check: wled_build / Build Environments (esp01_1m_full)
- GitHub Check: wled_build / Build Environments (esp8266_2m)
🔇 Additional comments (30)
wled00/FX.cpp (17)
7821-7821
: Good use of hardware randomization for velocity.The change to use
hw_random16
for rocket velocity provides better randomization than software-based methods. This should deliver more natural-looking firework behavior.
7891-7891
: Nice enhancement to particle saturation range.Using
hw_random16(156)
with a base of 100 ensures vibrant colors (saturation between 100-255) while maintaining randomness.
9517-9520
: Good improvements to particle system configuration.The updated intensity-based particle count calculation and collision detection with an experimentally-derived hardness value will improve the visual appeal and physics behavior.
9534-9539
: Well-structured lambda function for target position calculation.Using a lambda for calculating target positions improves code readability and maintenance by centralizing this logic.
9543-9546
: Improved collision detection with look-ahead logic.The new approach with a
closeToTarget
boolean improves readability and provides better control over when particles are considered "arrived" at their destinations.
9571-9576
: Good addition of particle re-ordering after collisions.This ensures particles maintain their intended visual arrangement even after physics interactions cause position changes.
9584-9584
: Consistent use of the target position calculation.Using the
calcTargetPos
lambda for setting initial positions ensures consistency with the rest of the code.
9712-9721
: Particle re-ordering with wrap-around handling.The additional check for wrap-around conditions is a good enhancement that prevents incorrect swapping of particles that are actually on opposite ends of the display.
9763-9763
: Enhanced particle system initialization.Setting
advanced
totrue
enables more sophisticated particle features that will improve the visual quality of the effect.
9777-9779
: Improved particle count calculation.The new logic for calculating the number of particles and hue steps based on intensity and other parameters will provide more consistent and visually pleasing results.
9799-9801
: Great addition of dynamic color spread in "playful mode".The sine-based modulation of hue step and particle size creates more dynamic and engaging visual patterns.
Also applies to: 9807-9809
9818-9842
: Excellent implementation of "playful mode".This new mode with its dynamic changes to hue, size, speed, and density creates a more engaging and varied visual experience. The combination of different timing mechanisms for each parameter provides complex but coherent motion.
10087-10087
: Improved audio reactive behavior.The updates to threshold calculation and saturation handling will provide better response to audio input and more consistent visuals.
Also applies to: 10095-10095
10106-10109
: Nice addition of mid-frequency color modulation.Using mid-frequency audio data with Perlin noise to modulate colors creates rich, responsive visual patterns that enhance the audio-reactive experience.
10159-10243
: Great new "Sonic Boom" effect.This effect creates explosion particles triggered by audio beats with multiple position modes (fixed, alternating, random) and color options. The implementation uses audio data effectively to create dynamic, music-responsive visuals.
10249-10411
: Impressive "Springy" effect with realistic physics.The new spring physics effect is well-implemented with:
- Configurable spring constants and length
- Advanced damping mechanisms to control oscillations
- Multiple excitation methods (pulses, sine waves, random)
- Smart handling of particle ordering and physics calculations
- Various color mapping options based on particle physics properties
This effect adds significant value with its highly customizable parameters and realistic physics simulation.
🧰 Tools
🪛 Cppcheck (2.10-2)
[error] 10320-10320: Invalid abs() argument nr 1. A non-boolean value is required.
(invalidFunctionArgBool)
10681-10683
: Good integration of new effects.The new effects are properly registered with the system, making them available to users with appropriate metadata.
wled00/FX.h (1)
354-356
: Added two new particle system effectsThese changes introduce two new particle system effects ("PS1DSONICBOOM" and "PS1DSPRINGY") and update the MODE_COUNT accordingly. The new effects align with the PR objectives of adding "Sonic Boom" (an AR experience with firework-like explosions) and "PS Springy" (particles connected by springs).
wled00/FXparticleSystem.h (3)
214-214
: Optimized parameter type for brightnessChanged the brightness parameter from
uint32_t
touint8_t
. This is a good optimization since brightness values typically range from 0-255, making uint8_t sufficient. This reduces memory usage and aligns with the PR's performance optimization goals.
381-381
: Optimized parameter type for brightnessSimilar to the 2D version, the brightness parameter has been changed from
uint32_t
touint8_t
, which is an appropriate data type for brightness values (0-255). This optimization improves memory usage.
386-386
: Improved collision detection parametersChanged the collision parameters from using relative velocity (
relativeVx
) to using absolute distance (dx_abs
). This modification likely improves the precision of collision detection and aligns with the PR objective of enhancing collision handling through a look-ahead mechanism.wled00/FXparticleSystem.cpp (9)
21-22
: Type correction for color scale parameters.The parameter type changes from
uint32_t
touint8_t
for scale values in these functions is appropriate since color scaling factors typically range from 0-255. This improves type correctness and reduces memory usage while maintaining the same functionality.
390-398
: Performance optimization using bit shifts instead of division.The calculation of deviation and angle transformations now use bit shifts (
>>
) instead of division, which is more efficient on embedded hardware. The addition of 255 before shifting right by 8 bits (equivalent to dividing by 256) ensures proper rounding.
697-699
: Type correction for brightness parameters.Changing the type of
brightness
parameter fromuint32_t
touint8_t
is appropriate since brightness values are in the 0-255 range. Similarly, changing thepxlbrightness
array type reduces memory usage. These changes make the code more type-correct and efficient.Also applies to: 709-709
1771-1774
: Improved 1D particle collision detection with lookahead.Similar to the 2D system, the 1D particle collision detection now includes velocity in its calculations, which improves collision accuracy. The type change from
int32_t dx_abs
touint32_t dx_abs
is also appropriate since absolute values are always non-negative.
1783-1795
: Optimized collision calculation for different architectures.The collision calculation has been improved and optimized for different chip architectures. The code now uses bit shifts with proper rounding for ESP32C3 and ESP8266 platforms, while using division for other ESP32 platforms based on their performance characteristics.
2030-2030
: Optimized maximum color value calculation.The implementation for finding the maximum of three color values has been improved using chained comparisons, which according to the comment is faster than using
std::max()
or XOR operations. This small optimization can have a significant impact in a function called frequently during rendering.Also applies to: 2043-2045
2098-2098
: Memory management safety improvements.Several safety checks have been added to prevent potential crashes:
- Null pointer check before freeing memory
- Improved minimum particle allocation logic
- Safety checks before particle transfers and buffer offset calculations
These changes enhance the robustness of the memory management system, reducing the risk of crashes due to invalid memory operations.
Also applies to: 2164-2166, 2189-2190, 2192-2195
2326-2330
: Buffer handling robustness improvements.The buffer allocation and deallocation logic has been enhanced with better checks:
- Improved conditional checks for buffer allocation
- Proper null checks before deallocating buffers
- Null check before transferring buffer data
These changes improve the reliability of the rendering system by preventing operations on invalid buffers.
Also applies to: 2364-2373, 2379-2379
2344-2347
: Skipping frozen segments in memory management.Adding a check to skip frozen segments when handling memory prevents unnecessary incrementing of the watchdog counter for inactive segments, which would otherwise lead to memory deallocation and potential crashes when the segment becomes active again.
You used your original particle branch which was squash-merged into main. |
thanks for the clarification, makes sense as git does not know all old commits. I will do future updates branched from main then and use squash merge on this one. |
Assuming there are no objections to this PR: |
FX.cpp is a beast I don't like very much. |
that is true and a good option to reduce FX.cpp. Some restructuring of files in general is something that may need to be discussed. |
New PS Effects:
edit:
not sure why this PR shows so many commits, maybe I should have rebased the branch before cherry-picking? It's actually only 8 commits. Is it better to start a new PR or just squash-merge this one?
Summary by CodeRabbit