Lava Lamp FX in the user_fx usermod #5253
Conversation
WalkthroughA new 2D Lava Lamp FX mode is added to the user effects module with particle system logic, palette wrapping helpers, and effect registration. The implementation includes particle state management, soft-edged additive blending, and blob attraction support. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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: 2
🧹 Nitpick comments (2)
usermods/user_fx/user_fx.cpp (2)
5-8: Remove unused macro definitions.These palette wrap macros are defined but never used in the file. Consider removing them unless they're intended for future use.
🔎 Proposed fix to remove unused macros
-// paletteBlend: 0 - wrap when moving, 1 - always wrap, 2 - never wrap, 3 - none (undefined) -#define PALETTE_SOLID_WRAP (strip.paletteBlend == 1 || strip.paletteBlend == 3) -#define PALETTE_MOVING_WRAP !(strip.paletteBlend == 2 || (strip.paletteBlend == 0 && SEGMENT.speed == 0)) -
187-187: Use hw_random8() consistently for hue generation.Both branches generate a random 0-255 value, making the conditional redundant. Since
hw_random8()is already used elsewhere in WLED for better entropy, use it consistently for both color modes:- lavaParticles[i].hue = SEGMENT.check1 ? hw_random8() : random16(256); + lavaParticles[i].hue = hw_random8();
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/user_fx/user_fx.cpp
🧰 Additional context used
🧠 Learnings (10)
📓 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.
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.
📚 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:
usermods/user_fx/user_fx.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:
usermods/user_fx/user_fx.cpp
📚 Learning: 2025-09-02T19:42:57.475Z
Learnt from: freakintoddles2
Repo: wled/WLED PR: 4904
File: wled00/FX.cpp:3915-3930
Timestamp: 2025-09-02T19:42:57.475Z
Learning: WLED wled00/FX.cpp::mode_percent — For time-based options (% of Minute/Hour/Day), behavior is clock-like: at rollover (elapsed==0) intentionally show SEGLEN lit for one tick to represent completion, then move to 1 LED at the next second. This is by design (not a progress bar) and should not be “fixed”.
Applied to files:
usermods/user_fx/user_fx.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:
usermods/user_fx/user_fx.cpp
📚 Learning: 2025-12-28T14:06:48.764Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.764Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
Applied to files:
usermods/user_fx/user_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:
usermods/user_fx/user_fx.cpp
📚 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:
usermods/user_fx/user_fx.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:
usermods/user_fx/user_fx.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:
usermods/user_fx/user_fx.cpp
🔇 Additional comments (4)
usermods/user_fx/user_fx.cpp (4)
122-133: Initialization logic is correct assuming the static array issue is resolved.The 2D validation and particle initialization on first call follows standard WLED patterns.
217-238: Blob attraction logic is correct.The condition at line 224 correctly implements "attract only when moving in opposite vertical directions" by skipping pairs where both particles move in the same direction.
240-330: LGTM: Rendering implementation is solid.The physics simulation, boundary handling, and soft-edged additive rendering create the intended lava lamp effect. The use of
qadd8()for additive blending and distance-based intensity falloff are appropriate techniques for organic blob merging.
334-334: LGTM: Effect metadata and registration follow WLED conventions.The metadata string properly defines sliders, checkboxes, 2D flag, and defaults. Registration with id=255 is correct for user effects.
Also applies to: 348-348
| bool active; // will not be displayed if false | ||
| } LavaParticle; | ||
|
|
||
| static LavaParticle lavaParticles[MAX_LAVA_PARTICLES]; |
There was a problem hiding this comment.
Critical: Static global array breaks multi-segment support.
The static global lavaParticles array is shared across all segments. If multiple segments run this effect simultaneously, they will corrupt each other's particle state, causing visual artifacts and incorrect behavior.
WLED effects should store per-segment state using SEGENV.allocateData() or manage buffers in SEGENV.data. This is consistent with the Diffusion Fire effect above (lines 36-37) which properly allocates per-segment storage.
🔎 Recommended approach
Allocate particle array in SEGENV.data using SEGENV.allocateData(sizeof(LavaParticle) * MAX_LAVA_PARTICLES) and cast the pointer:
LavaParticle* lavaParticles = reinterpret_cast<LavaParticle*>(SEGENV.data);Initialize on first call or when allocation succeeds. This ensures each segment has its own independent particle state.
🤖 Prompt for AI Agents
In usermods/user_fx/user_fx.cpp around line 120, remove the static global
lavaParticles array and instead allocate per-segment storage via
SEGENV.allocateData(sizeof(LavaParticle) * MAX_LAVA_PARTICLES); cast SEGENV.data
to LavaParticle* (e.g., LavaParticle* lavaParticles =
reinterpret_cast<LavaParticle*>(SEGENV.data)); ensure allocation is done once
per-segment (check return of allocateData or a per-seg init flag) and initialize
the particles when allocation succeeds; replace all uses of the former global
with the per-segment pointer so each segment has independent particle state.
| static uint8_t lastSizeControl = 128; | ||
| uint8_t currentSizeControl = SEGMENT.custom1; | ||
| bool sizeChanged = (currentSizeControl != lastSizeControl); | ||
|
|
||
| if (sizeChanged) { | ||
| // Recalculate size range based on new slider value | ||
| float minSize = cols * 0.15f; | ||
| float maxSize = cols * 0.4f; | ||
| float newRange = (maxSize - minSize) * (currentSizeControl / 255.0f); | ||
|
|
||
| for (int i = 0; i < MAX_LAVA_PARTICLES; i++) { | ||
| if (lavaParticles[i].active) { | ||
| // Assign new random size within the new range | ||
| lavaParticles[i].size = minSize + random16((int)(newRange * 10)) / 10.0f; | ||
| // Ensure minimum size | ||
| if (lavaParticles[i].size < minSize) lavaParticles[i].size = minSize; | ||
| } | ||
| } | ||
|
|
||
| lastSizeControl = currentSizeControl; | ||
| } |
There was a problem hiding this comment.
Static variable breaks per-segment size tracking.
The static lastSizeControl variable is shared across all segments. When multiple segments run this effect with different custom1 values, they will incorrectly detect size changes and interfere with each other's size calculations.
Use segment-specific storage such as SEGENV.aux0 to track the last size control value:
uint8_t lastSizeControl = SEGENV.aux0;
uint8_t currentSizeControl = SEGMENT.custom1;
bool sizeChanged = (currentSizeControl != lastSizeControl);Then update at line 163:
SEGENV.aux0 = currentSizeControl;🤖 Prompt for AI Agents
In usermods/user_fx/user_fx.cpp around lines 144 to 164, the static variable
lastSizeControl is shared across all segments causing cross-segment
interference; replace the static with segment-local storage by reading
lastSizeControl from SEGENV.aux0, compare it to SEGMENT.custom1, and after
applying size changes write the new currentSizeControl back to SEGENV.aux0 so
each segment tracks its own last size control value.
| // Extract RGB | ||
| uint8_t r = (color >> 16) & 0xFF; | ||
| uint8_t g = (color >> 8) & 0xFF; | ||
| uint8_t b = color & 0xFF; |
There was a problem hiding this comment.
you can use the R() G() B() and W() macros here, they do the same thing
There was a problem hiding this comment.
better yet: use the methods from colors.cpp for scaling and blending, no need to implement them here again.
| // Apply life/opacity | ||
| r = (r * p->life) >> 8; | ||
| g = (g * p->life) >> 8; | ||
| b = (b * p->life) >> 8; |
There was a problem hiding this comment.
did you intend to omit the "white" channel?
| uint32_t existing = SEGMENT.getPixelColorXY(px, py); | ||
| uint8_t er = (existing >> 16) & 0xFF; | ||
| uint8_t eg = (existing >> 8) & 0xFF; | ||
| uint8_t eb = existing & 0xFF; |
There was a problem hiding this comment.
same as above - recommended to use R() G() B() and W() macros
|
|
||
| er = qadd8(er, br); | ||
| eg = qadd8(eg, bg); | ||
| eb = qadd8(eb, bb); |
There was a problem hiding this comment.
the faster alternative here would be to use color_add() or color_blend(). These functions handle all color components at once.
There was a problem hiding this comment.
exactly, see my comment above.
3 participants
This PR will add the Lava Lamp effect into the new user_fx usermod instead of FX.cpp