Enhanced particle system rendering & 1D collision handling #5188
Conversation
- better size control as size is now exactly mappable to pixels so it can be matched exactly to the collision distance - no more gaps due to collision distance mismatch - much faster: saw up to 30% improvement in FPS - also adjusted some of the FX to make better use of the new rendering
- bugfix in mass based 2D collisions - added improved and faster large size rendering to 1D system - added per-particle size rendering to 1D system - improved and simplified collision handling in 1D system - removed local blurring functions in PS as they are not needed anymore for particle rendering - adapted FX to work with the new rendering - fixed outdated AR handling in PS FX - fixed infinite loop if not enough memory - updated PS Hourglass drop interval to simpler math: speed / 10 = time in seconds and improved particle handling - reduced speed in PS Pinball to fix collision slip-through - PS Box now auto-adjusts number of particles based on matrix size and particle size - added safety check to 2D particle rendering to not crash if something goes wrong with out-of bounds particle rendering - improved binning for particle collisions: dont use binning for small number of particles (faster) - Some code cleanup
WalkthroughRefactors particle systems to add per-particle sizing and mass-aware collisions, update rendering (including large-particle paths and ellipse brightness), and adjust FX initialization call sites to new initParticleSystem1D/2D signatures and argument sources. (48 words) Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)wled00/**/*.cpp📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (15)📓 Common learnings📚 Learning: 2025-08-26T11:51:21.817ZApplied to files:
📚 Learning: 2025-06-07T15:58:42.579ZApplied to files:
📚 Learning: 2025-11-14T05:48:44.673ZApplied to files:
📚 Learning: 2025-05-09T18:48:21.296ZApplied to files:
📚 Learning: 2025-10-20T09:41:50.374ZApplied to files:
📚 Learning: 2025-04-30T05:41:03.633ZApplied to files:
📚 Learning: 2025-09-11T04:36:42.869ZApplied to files:
📚 Learning: 2025-04-24T09:31:06.879ZApplied to files:
📚 Learning: 2025-04-24T09:31:06.879ZApplied to files:
📚 Learning: 2025-10-20T09:38:51.997ZApplied to files:
📚 Learning: 2025-05-09T18:43:15.355ZApplied to files:
📚 Learning: 2025-11-16T19:40:46.260ZApplied to files:
📚 Learning: 2025-11-22T12:12:53.486ZApplied to files:
📚 Learning: 2025-08-08T17:22:37.374ZApplied to files:
🧬 Code graph analysis (1)wled00/FXparticleSystem.cpp (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
🔇 Additional comments (25)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wled00/FX.cpp (1)
9668-9671: DuplicateinitParticleSystem1Dcall (likely unintended).
This currently nests two identicalif (!initParticleSystem1D(...))checks, which is at best redundant and at worst confusing if side effects/partial allocation ever change.- if (!initParticleSystem1D(PartSys, 4, 150, 4, true)) // init advanced particle system - if (!initParticleSystem1D(PartSys, 4, 150, 4, true)) // init advanced particle system + if (!initParticleSystem1D(PartSys, 4, 150, 4, true)) // init advanced particle system return mode_static(); // allocation failed or is single pixel
🧹 Nitpick comments (3)
wled00/FX.cpp (3)
8388-8395: Avoid flippingperParticleSizerepeatedly inside the spawn path; set it once per mode/config change.
Right nowperParticleSizeis toggled per spawned particle; this is easy to desync withadvPartProps[i].sizewhen particle reuse happens. Consider caching “size mode” (global vs per-particle) inSEGENV.aux*and only updatingPartSys->perParticleSizewhenSEGMENT.custom1crosses the special value, then just set the per-particlesizewhen needed. Also please double-check the metadata string (PS Ballpit) matches the new “Size/Hardness/Saturation” semantics.Also applies to: 8412-8412
8496-8497: Clamp/guard particle size and avoid potential overflow invWidth()*vHeight().
maxParticleSizeusesSEGMENT.vWidth() * SEGMENT.vHeight(); it’s probably fine in practice, but a safer pattern avoids any signed overflow in intermediate math and ensures size never becomes 0 before callingsetParticleSize().- int maxParticleSize = min(((SEGMENT.vWidth() * SEGMENT.vHeight()) >> 2), 255U); // max particle size based on matrix size - unsigned currentParticleSize = map(SEGMENT.custom3, 0, 31, 0, maxParticleSize); + const uint32_t area = uint32_t(SEGMENT.vWidth()) * uint32_t(SEGMENT.vHeight()); + const uint32_t maxParticleSize = min(area >> 2, 255UL); // max particle size based on matrix size + uint32_t currentParticleSize = map(SEGMENT.custom3, 0, 31, 0, maxParticleSize); + if (SEGMENT.custom3 < 31) currentParticleSize = max<uint32_t>(1, currentParticleSize);Also applies to: 8511-8518, 8521-8529
10091-10092: Nice anti-clumping tweak; consider making the friction factor a named constant.
The per-particle damping is a good low-cost stabilizer. Minor: a named constant (or tie it toSEGMENT.speed/intensity) would make future tuning easier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
wled00/FX.cpp(27 hunks)wled00/FXparticleSystem.cpp(36 hunks)wled00/FXparticleSystem.h(8 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
wled00/**/!(html_*)*.h
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for non-generated C++ header files (.h)
Files:
wled00/FXparticleSystem.h
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/FX.cppwled00/FXparticleSystem.cpp
🧠 Learnings (22)
📓 Common learnings
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
📚 Learning: 2025-08-26T11:51:21.817Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Applied to files:
wled00/FXparticleSystem.hwled00/FX.cppwled00/FXparticleSystem.cpp
📚 Learning: 2025-08-31T03:38:14.114Z
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.
Applied to files:
wled00/FXparticleSystem.hwled00/FX.cppwled00/FXparticleSystem.cpp
📚 Learning: 2025-11-27T06:33:11.436Z
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5109
File: wled00/FX.cpp:3174-3343
Timestamp: 2025-11-27T06:33:11.436Z
Learning: WLED Ants effect (wled00/FX.cpp): The author prefers the current velocity initialization using hw_random16(1000, 5000)/5000.0f, resulting in an effective range of ~3.6–10.0 (with VELOCITY_MIN=2.0, VELOCITY_MAX=10.0), and wants the code kept as-is with comments updated to document this behavior. Avoid suggesting changes to span the full 2.0–10.0 range in future reviews.
Applied to files:
wled00/FXparticleSystem.hwled00/FX.cpp
📚 Learning: 2025-11-16T19:40:46.260Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4926
File: wled00/FX.cpp:4727-4730
Timestamp: 2025-11-16T19:40:46.260Z
Learning: WLED AuroraWave (wled00/FX.cpp): wave_start and wave_end intentionally use int16_t; segments longer than 32k LEDs are not supported (bounded by MAX_LEDS), so widening to 32-bit is unnecessary.
Applied to files:
wled00/FXparticleSystem.hwled00/FX.cppwled00/FXparticleSystem.cpp
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Applied to files:
wled00/FXparticleSystem.h
📚 Learning: 2025-04-26T19:19:07.600Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4658
File: wled00/const.h:140-141
Timestamp: 2025-04-26T19:19:07.600Z
Learning: In WLED, the WLED_MAX_PANELS macro is intentionally defined as a fixed constant value (18) with no redefinition mechanism, making it "unoverridable" - there's no need for a static assertion to check its maximum value.
Applied to files:
wled00/FXparticleSystem.h
📚 Learning: 2025-05-09T18:48:21.296Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4682
File: wled00/FX.cpp:9016-9039
Timestamp: 2025-05-09T18:48:21.296Z
Learning: WLED does not support matrices large enough (≥1024 px) for 32-bit overflow in dx*dx + dy*dy to be a practical concern in particle system effects.
Applied to files:
wled00/FXparticleSystem.hwled00/FXparticleSystem.cpp
📚 Learning: 2025-06-15T09:59:52.720Z
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Applied to files:
wled00/FXparticleSystem.hwled00/FX.cpp
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Applied to files:
wled00/FXparticleSystem.h
📚 Learning: 2025-06-07T15:58:42.579Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4712
File: wled00/FX_fcn.cpp:992-993
Timestamp: 2025-06-07T15:58:42.579Z
Learning: In WLED segments, the backing pixel buffer is always allocated to the full physical segment size, but effect rendering only uses a subset when mirror or grouping are enabled. For 2D segments, this subset is vWidth() * vHeight(), and for 1D segments it's vLength(). Color processing operations like fade_out should iterate over this rendering subset, not the full allocated buffer.
Applied to files:
wled00/FXparticleSystem.hwled00/FXparticleSystem.cpp
📚 Learning: 2025-10-20T09:41:50.374Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4995
File: wled00/FX.cpp:5209-5212
Timestamp: 2025-10-20T09:41:50.374Z
Learning: In WLED 2D effects, SEGMENT.length() returns the physical pixel count (upper bound), while SEG_W * SEG_H can be smaller depending on mapping (mirroring/grouping). For per-cell state buffers (e.g., mode_2Dgameoflife in wled00/FX.cpp), allocate using SEGMENT.length() to ensure sufficient capacity and avoid reallocations, while bounding indexing by cols*rows.
Applied to files:
wled00/FX.cppwled00/FXparticleSystem.cpp
📚 Learning: 2025-08-08T17:22:37.374Z
Learnt from: netmindz
Repo: wled/WLED PR: 4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:22:37.374Z
Learning: In WLED FX.cpp, um_data returned by getAudioData() is intentionally untyped because different usermods provide different payloads, and simulateSound() may supply dummy data when the AudioReactive usermod isn’t built. Strongly typed accessors are not feasible; prefer explicit casts (e.g., samplePeak as uint8_t at u_data[3]) or small helpers that encapsulate these casts.
Applied to files:
wled00/FX.cppwled00/FXparticleSystem.cpp
📚 Learning: 2025-08-08T17:31:47.330Z
Learnt from: netmindz
Repo: wled/WLED PR: 4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:31:47.330Z
Learning: In WLED core, avoid introducing AR-specific helper wrappers for um_data access. Keep um_data untyped and, when reading samplePeak, prefer `(*(uint8_t*)um_data->u_data[3]) != 0` over `*(bool*)` to avoid alignment/aliasing issues, while staying decoupled from the AudioReactive usermod.
Applied to files:
wled00/FX.cpp
📚 Learning: 2025-11-14T05:48:44.673Z
Learnt from: DedeHai
Repo: wled/WLED PR: 5040
File: wled00/image_loader.cpp:84-96
Timestamp: 2025-11-14T05:48:44.673Z
Learning: In WLED (wled00/FX_2Dfcn.cpp), the Segment::setPixelColorXY() function performs internal bounds checking against vWidth() and vHeight(), returning early if coordinates are out of bounds. No additional guards are needed when calling this function, even in upscaling loops where coordinates might exceed segment dimensions.
Applied to files:
wled00/FXparticleSystem.cpp
📚 Learning: 2025-09-11T04:36:42.869Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4754
File: usermods/user_fx/README.md:169-179
Timestamp: 2025-09-11T04:36:42.869Z
Learning: In WLED, SEGENV.data is already typed as uint8_t* and can be accessed directly without casting when storing per-pixel state data.
Applied to files:
wled00/FXparticleSystem.cpp
📚 Learning: 2025-04-30T05:41:03.633Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like `if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize))` is unnecessary and could be problematic, as it bypasses proper size verification.
Applied to files:
wled00/FXparticleSystem.cpp
📚 Learning: 2025-04-24T09:31:06.879Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
Applied to files:
wled00/FXparticleSystem.cpp
📚 Learning: 2025-04-24T09:31:06.879Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs by falling back to the main segment ID, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
Applied to files:
wled00/FXparticleSystem.cpp
📚 Learning: 2025-10-20T09:38:51.997Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4995
File: wled00/FX.cpp:5223-5226
Timestamp: 2025-10-20T09:38:51.997Z
Learning: WLED matrices: each dimension (SEG_W, SEG_H) is limited to ≤255; 256 or larger per side is not supported/feasible on ESP32, so effects should assume per-dimension max 255.
Applied to files:
wled00/FXparticleSystem.cpp
📚 Learning: 2025-05-09T18:43:15.355Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4682
File: wled00/FX.cpp:8997-9005
Timestamp: 2025-05-09T18:43:15.355Z
Learning: In the WLED codebase, SEGMENT.custom3 is always constrained to the range 0-31 and will not exceed this range.
Applied to files:
wled00/FXparticleSystem.cpp
📚 Learning: 2025-11-22T12:12:53.486Z
Learnt from: DedeHai
Repo: wled/WLED PR: 5105
File: wled00/FX.h:968-968
Timestamp: 2025-11-22T12:12:53.486Z
Learning: In WLED's WS2812FX class (wled00/FX.h), the _pixels array stores LED colors indexed by unmapped (logical) LED positions, not physical positions. The getPixelColor(n) function intentionally checks if the logical LED at index n maps to a valid physical position via getMappedPixelIndex(n), then accesses _pixels[n] with the unmapped index. If the mapping is invalid (e.g., -1 for gaps in LED maps, or out of bounds), it returns 0 (black). This allows peek preview to correctly show gaps in LED mappings as black pixels. The mapping from logical to physical positions is only applied when writing to the hardware bus, not when reading from the pixel buffer.
<!-- [add_learning]
Applied to files:
wled00/FXparticleSystem.cpp
🧬 Code graph analysis (2)
wled00/FX.cpp (1)
wled00/FXparticleSystem.cpp (4)
initParticleSystem2D(1093-1125)initParticleSystem2D(1093-1093)initParticleSystem1D(1842-1862)initParticleSystem1D(1842-1842)
wled00/FXparticleSystem.cpp (3)
wled00/FXparticleSystem.h (1)
calculateEllipseBrightness(238-250)wled00/fcn_declare.h (3)
hw_random16(434-434)hw_random16(435-435)hw_random16(436-436)wled00/colors.h (1)
rgb2hsv(132-132)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: wled_build / Build Environments (nodemcuv2_160)
- GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
- GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
- GitHub Check: wled_build / Build Environments (lolin_s2_mini)
- GitHub Check: wled_build / Build Environments (esp32dev_debug)
- GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
- GitHub Check: wled_build / Build Environments (esp32c3dev_qio)
- GitHub Check: wled_build / Build Environments (esp32_wrover)
- GitHub Check: wled_build / Build Environments (esp32S3_wroom2)
- GitHub Check: wled_build / Build Environments (usermods)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_compat)
- GitHub Check: wled_build / Build Environments (esp8266_2m_160)
- GitHub Check: wled_build / Build Environments (esp32_eth)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_160)
- GitHub Check: wled_build / Build Environments (esp8266_2m_compat)
- GitHub Check: wled_build / Build Environments (esp8266_2m)
- GitHub Check: wled_build / Build Environments (esp01_1m_full)
🔇 Additional comments (33)
wled00/FXparticleSystem.h (10)
20-20: LGTM!The speed limit of 120 provides adequate headroom below the int8_t maximum (127) to prevent overflow during collision calculations involving velocity additions.
106-106: LGTM!The comment accurately documents the new per-particle size control feature and its relationship to the
perParticleSizeflag.
193-193: LGTM!The
perParticleSizeflag provides clean control over whether individual particle sizes are used fromadvPartProps.
200-200: LGTM!The
renderLargeParticlehelper follows the existing rendering pattern and appropriately takes size as the first parameter for the new squared distance rendering approach.
204-204: LGTM!Adding
massratio1andmassratio2parameters enables proper momentum conservation for particles of different sizes during collisions.
320-323: LGTM!The
PSadvancedParticle1Dstruct is cleanly organized with consistent documentation matching the 2D counterpart.
365-365: LGTM!The comment clearly documents that calling
setParticleSize()disables per-particle size control, which is important for users to understand the mutual exclusivity.
380-380: LGTM!Consistent
perParticleSizeflag across both 1D and 2D particle systems provides a unified API.
385-386: LGTM!The 1D
renderLargeParticlehelper mirrors the 2D approach, enabling consistent per-particle size rendering across both systems.
238-250: > Likely an incorrect or invalid review comment.wled00/FX.cpp (6)
8246-8249: Sanity-checkvWidth()/vHeight()semantics vs PS coordinate system (sources + height/ttl/vy).
UsingSEGMENT.vWidth()as “requested sources” andSEGMENT.vHeight()for lifetimes/speeds makes sense for per-cell mapping, but please confirm these match whatParticleSystem2D(cols, rows, ...)uses internally (it currently derives cols/rows fromSEGMENT.virtualWidth()/virtualHeight()per FXparticleSystem.cpp). A mismatch here will show up as wrong flame count/height on mirrored/grouped mappings.Also applies to: 8286-8290, 8319-8324
8787-8787:PSdataEndattractor storage looks consistent; verify alignment + first-call velocity intent.
- Good: you reserved
sizeof(PSparticle)ininitParticleSystem2D(..., sizeof(PSparticle), true)and then reusePSdataEndfor the attractor.- Please confirm
PSdataEndis sufficiently aligned forPSparticleon all targets (ESP32/ESP8266). If not guaranteed, consider storing a smaller, explicitly-aligned struct or usingmemcpyinto a localPSparticle.- The first-call velocity swap (
vx=source.vy,vy=source.vx) is subtle—worth a short comment describing why this is the desired initial motion.Also applies to: 8798-8802, 8803-8807, 8834-8836
9455-9462: Pinball per-particle size mode: ensure sizes are only consumed when enabled.
In rolling-balls and bouncing-ball branches you setadvPartProps[i].size/sources[0].sizeunconditionally. That’s fine if the 1D PS renderer ignores per-particle sizes unlessperParticleSize==true, but please verify to avoid “random size” leaking into global-size mode.Also applies to: 9476-9486, 9524-9526
9879-9880: Hourglass: collision+reorder approach seems reasonable; please validate edge cases (fixed flags, direction flips, interval math).
- Always-on collisions with hardness
64is OK if perf is acceptable; just confirm it doesn’t regress small-strip FPS.- The “re-order” loop swaps only
x. If any other per-particle state must follow ordering (e.g.,fixed,reversegrav, velocity), consider swapping the full particle + flags (+ adv props) instead of onlyx.- Interval math now uses
max(100, SEGMENT.speed * 100); please confirm slider semantics in the metadata string match this (0.1s..25.5s) and thatSEGMENT.vLength()-based countdown is intentional.Also applies to: 9893-9899, 9909-9912, 9947-9953, 9960-9960, 9974-9974
10117-10122:PS Chase: good size-aware density + spacing alignment; watch for off-by-one wrap interactions.
AligningSEGENV.steptoPS_P_RADIUS_1Dgrid is a solid change for “move in union” behavior. Please sanity-test the wrap condition with very largecustom1(size) sox > maxX + radius + sizedoesn’t create visible jitter on wrap.Also applies to: 10131-10141
10607-10608: Springy: large-particle rendering toggle is fine; keep the “collisions off” rationale explicit.
Given you’re intentionally not enabling collisions here (commented out), it’d help to add a brief note that collisions were tried and caused instability/chaos (so future refactors don’t “fix” it back). Also please confirmsetParticleSize(120)is the intended scale for “XL” in the 1D PS renderer (vs the older 1/2px toggle).Also applies to: 10620-10626, 10630-10634
wled00/FXparticleSystem.cpp (17)
91-91: Good safety improvement.Using
max((uint32_t)1, ...)ensures at least one particle is always used, preventing potential division-by-zero or empty iteration issues downstream.
150-160: Implementation is clean and well-documented.The
perParticleSize = falseassignment correctly disables per-particle sizing when a global size is explicitly set, and the radius formula provides the documented ~80% overlap behavior.
238-244: Verify per-particleparticleHardRadiusmodification is safe.
particleHardRadiusis a class member being modified within the particle loop. While this appears intentional for immediate use in bounce checking (lines 246-250), ensure no other concurrent code or subsequent iterations expect a stable value. If this is by design, consider adding a brief comment clarifying thatparticleHardRadiusis intentionally recalculated per-particle whenperParticleSizeis active.
602-609: LGTM - blur function delegation.Using
SEGMENT.blur2D()directly aligns with the PR objective of removing local PS blurring functions. The gamma correction at line 602 is intentional for effect-level brightness shaping. Based on learnings, this is the expected pattern.
614-632: Clear rendering path separation.The three-tier rendering logic (single pixel → 2x2 bilinear → ellipse) is well-structured. Adding 1 to per-particle size (line 617) ensures collisions work correctly by avoiding unsupported single-pixel particles when using per-particle sizing.
721-806: Well-implemented ellipse rendering.The new
renderLargeParticlefunction provides sub-pixel precision ellipse rendering with:
- Correct asymmetry support via
getParticleXYsize- Proper wrap handling for edge particles
- Consistent gamma correction with the 2x2 path
The
+1in radius calculation (lines 747-748) appropriately compensates for bit-shift truncation.
856-884: Correct mass-ratio physics implementation.The mass-proportional-to-area calculation (
mass = radius²) is physically appropriate for 2D particles. The fixed-point mass ratios ensure lighter particles receive proportionally more velocity change during collisions, which is correct momentum conservation behavior.
1099-1107: Robust memory allocation fallback.The
while(numparticles >= 5)loop with halving provides graceful degradation on memory-constrained devices. Settingadvanced = truewhensizecontrolis requested prevents API misuse.
1170-1174: Performance trade-off for accuracy.The double collision pass when
perParticleSizeis enabled addresses slip-through issues for small/fast particles. This is a reasonable trade-off given the PR's focus on accurate collision handling. Consider documenting this in a header comment if not already done.
1237-1247: Consistent with 2D implementation.The 1D
setParticleSizemirrors the 2D version correctly, including theperParticleSizeflag and radius calculation formula.
1559-1603: Well-implemented 1D large particle rendering.The gradient line rendering follows the same pattern as 2D with appropriate adaptations. The
+1extension on bounding box (lines 1573-1574) provides smoother sub-pixel movement. The linear falloff calculation is efficient.
1776-1777: Correct 1D→2D mapping detection.The conditional properly checks both
SEGMENT.is2D()andSEGMENT.map1D2Dto allocate the local framebuffer only when needed for mapped rendering.
1467-1467: Consistent with 2D blur delegation.Using
SEGMENT.blur()directly aligns with the pattern established in the 2D implementation and removes dependency on local blur functions.
823-827: Good optimization for small particle counts.Disabling binning when
usedParticles < maxBinParticlesavoids unnecessary overhead. The single-bin fallback (numBins = 1) provides correct behavior while improving performance for simpler configurations.
1673-1676: Verify the-2adjustment in thetotalmasscalculation at line 1673.The review comment references a potential inconsistency between the 1D collision implementation (line 1673:
totalmass = mass1 + mass2 - 2) and the 2D version (line 870:totalmass = mass1 + mass2). The-2adjustment is documented as accounting for rounding, but this differs from the 2D implementation. Please confirm whether this is intentional for 1D-specific physics behavior or if the formulas should be aligned.
932-948: The>> 7shift in the mass ratio scaling is intentional. The code comment explicitly explains this: "multiply by two to account for the fact that we distribute the impulse to both particles." Sincemassratiois calculated as(mass << 8) / totalmass(yielding 0-255), the>> 7shift (divide by 128) effectively amplifies the impulse by 2× compared to>> 8(divide by 256). This same pattern is used consistently in both the 2D collision handler (lines 933-936) and the 1D collision handler (lines 1706-1707), both with the same explanatory comment.
791-793: No issues identified. ThecalculateEllipseBrightnessfunction correctly handles the full range of particle sizes. The rendering loop's bounding box naturally constrainsdx_subpixelanddy_subpixelto approximately ±rx_subpixel(max ~320), ensuring that intermediate fixed-point calculations remain well within 32-bit integer limits.
|
Just an observation: Without having proper particle mass it means you cannot have balloon vs. marble type of collisions. A pity. |
|
would be easy to extend it if the need arises. |
2 participants
big improvement for large particle rendering both in looks and speed.
Large particles are no longer rendered using blur but using a squared distance function. This allows for more precise rendering and predictable particle sizes which also improves collisions, it is also much faster.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.