Skip to content

Perlinscape FX (2D) in the user_fx usermod #5587

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.

Sign up for GitHub

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

Open
BobLoeffler68 wants to merge 10 commits into
base: main
Choose a base branch
from
+96 −0
Open

Perlinscape FX (2D) in the user_fx usermod #5587

Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
73ca612
Minor changes to bring it up to the latest version on github
BobLoeffler68 Apr 3, 2026
427e61a
Merge branch 'main' of https://github.com/Aircoookie/WLED
BobLoeffler68 Apr 5, 2026
c58ccb8
Merge branch 'main' of https://github.com/Aircoookie/WLED
BobLoeffler68 Apr 9, 2026
5c65919
Merge branch 'main' of https://github.com/Aircoookie/WLED
BobLoeffler68 Apr 24, 2026
e76d4af
Adding white space back in to match WLED main branch
BobLoeffler68 Apr 25, 2026
1af6f89
Merge branch 'main' of https://github.com/Aircoookie/WLED
BobLoeffler68 May 10, 2026
8d8e107
Added zooming/scaling as suggested by blazoncek
BobLoeffler68 May 10, 2026
0dcdbaa
Fixed two issues mentioned by the rabbit: - fixed memory allocation. …
BobLoeffler68 May 10, 2026
4d1d818
Perlinscape: Will now use Perlin color if Default palette is used but…
BobLoeffler68 May 16, 2026
b41d994
Comment change
BobLoeffler68 May 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 98 additions & 0 deletions usermods/user_fx/user_fx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1258,6 +1258,103 @@ static void mode_morsecode(void) {
static const char _data_FX_MODE_MORSECODE[] PROGMEM = "Morse Code@Speed,,,,Color mode,Color by Word,Punctuation,EndOfMessage;;!;1;sx=192,c3=8,o1=1,o2=1";


/*
/ Perlinscape effect - a Perlin noise Landscape
* Created by stepko as part of Stepko Land on soulmatelights.com
* Adapted to WLED by Bob Loeffler with additional features (and help from Claude)
* First slider is for speed
* Second slider is for zooming in/out (Perlin scaling)
* Third slider is the X multiplier
* Fourth slider is the Y multiplier
* First checkbox will use the selected palette
* Second checkbox will rotate the image
* Third checkbox will randomize the horizonal and vertical directions
*/
static void mode_2D_perlinscape(void) {
if (!strip.isMatrix || !SEGMENT.is2D()) FX_FALLBACK_STATIC; // not a 2D set-up
const uint16_t width = SEG_W;
const uint16_t height = SEG_H;
if (!SEGENV.allocateData(5 * sizeof(float) + width * height)) FX_FALLBACK_STATIC;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

uint32_t speedDiv = map(SEGMENT.speed, 0, 255, 20, 1);
uint32_t t = strip.now / speedDiv;
uint8_t Xmult = map(SEGMENT.custom1, 0, 255, 0, 64);
uint8_t Ymult = map(SEGMENT.custom2, 0, 255, 0, 64);

float *offX = reinterpret_cast<float*>(SEGENV.data) + 0;
float *offY = reinterpret_cast<float*>(SEGENV.data) + 1;
float *stepX = reinterpret_cast<float*>(SEGENV.data) + 2;
float *stepY = reinterpret_cast<float*>(SEGENV.data) + 3;
float *prevT = reinterpret_cast<float*>(SEGENV.data) + 4;

if (SEGENV.call == 0) {
SEGENV.aux0 = hw_random16(5000, 10000);
SEGENV.aux1 = 0b00;
*offX = 0.0f;
*offY = 0.0f;
*stepX = 1.0f;
*stepY = 1.0f;
*prevT = (float)t;
}
Comment on lines +1295 to +1296
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify timestamp-delta patterns in this file (read-only audit)
rg -n -C2 'float\s+dt\s*=|strip\.now\s*-|lastBumpUpdate|prevT' usermods/user_fx/user_fx.cpp

Repository: wled/WLED

Length of output: 3778

🏁 Script executed:

#!/bin/bash
# Get detailed view of lines 1280-1320 to see full context and data allocation
sed -n '1275,1320p' usermods/user_fx/user_fx.cpp | cat -n

Repository: wled/WLED

Length of output: 1837

Change prevT storage from float to uint32_t and compute timestamp delta as signed integer before float conversion to prevent precision loss and wraparound artifacts.

The current code (line 1312) subtracts two separately-cast floats: (float)t - *prevT. When the uint32_t t value wraps (after ~49 days), casting each to float independently loses precision. The fix: store prevT as uint32_t, then compute the delta as a signed integer first before converting to float—matching the correct pattern already used elsewhere in this file (e.g., lines 934, 956, 980).

Suggested changes 
-  if (!SEGENV.allocateData(5 * sizeof(float))) FX_FALLBACK_STATIC;
+  if (!SEGENV.allocateData(4 * sizeof(float) + sizeof(uint32_t))) FX_FALLBACK_STATIC;
...
-  float *prevT = reinterpret_cast<float*>(SEGENV.data) + 4;
+  uint32_t *prevT = reinterpret_cast<uint32_t*>(reinterpret_cast<float*>(SEGENV.data) + 4);
...
-    *prevT = (float)t;
+    *prevT = t;
...
-  float dt = (float)t - *prevT;
+  float dt = float(int32_t(t - *prevT));
...
-  *prevT = (float)t;
+  *prevT = t;
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@usermods/user_fx/user_fx.cpp` around lines 1295 - 1296, Change the prevT
storage from float to uint32_t and compute the timestamp delta as a signed
integer before converting to float: keep prevT as an unsigned 32-bit value,
compute int32_t delta = (int32_t)t - (int32_t)*prevT (or equivalent signed
subtraction), then convert that delta to float for use (e.g., float dt =
(float)delta) and finally update *prevT = t; apply this change where prevT is
used (the same pattern as around lines that use prevT elsewhere) to avoid float
precision loss and wraparound artifacts.

Copy link
Copy Markdown
Member

@softhack007 softhack007 May 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this finding is right. float is stored in IEEE754 format (22 bit mantissa, 10bit exponent). It means that in worst case (near the uint32 roll-over) , float is only accurate for 22bits and you lose 10bits = 1-2 seconds of accuracy.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@softhack007, exactly right — thank you for spelling out the precision math. To summarize the complete impact:

  • t = strip.now / speedDiv is a derived uint32_t that can reach values in the hundreds of millions.
  • Storing it as float (23-bit mantissa, ~7 decimal significant digits) means once t > 2²³ ≈ 8 388 608, the least-significant bit of the float representation covers more than one millisecond — and near the 32-bit rollover (~4 billion) that grows to the 1–2 second error you describe.
  • The delta dt = (float)t - *prevT can therefore be significantly wrong, causing jitter or jumps in the animation offset.

The correct fix (as noted in the suggestion) is to keep prevT as a uint32_t and compute the signed integer delta before converting:

// allocation: 4 floats + 1 uint32_t
if (!SEGENV.allocateData(4 * sizeof(float) + sizeof(uint32_t))) FX_FALLBACK_STATIC;

uint32_t *prevT = reinterpret_cast<uint32_t*>(reinterpret_cast<float*>(SEGENV.data) + 4);

// on init:
*prevT = t;

// each frame:
float dt = float(int32_t(t - *prevT));
*offX += *stepX * dt;
*offY += *stepY * dt;
*prevT = t;

This matches the pattern already used in the Ants effect for lastBumpUpdate (e.g., float(int(strip.now - ants[i].lastBumpUpdate))).

@BobLoeffler68 — this is a clear correctness bug worth fixing before merging. Would you like to apply the change?

🐇 📐

✏️ Learnings added 
Learnt from: softhack007
Repo: wled/WLED PR: 5587
File: usermods/user_fx/user_fx.cpp:1295-1296
Timestamp: 2026-05-16T08:21:02.410Z
Learning: In WLED effect functions, never store a uint32_t timestamp (e.g., `t = strip.now / speedDiv`, or any millisecond counter) as `float` for use as "previous time" in a delta calculation. IEEE 754 single-precision float has only 23 bits of mantissa, so values > ~8 million lose sub-second precision, and near uint32_t rollover the rounding error reaches 1–2 seconds. Instead, store the timestamp as `uint32_t` and compute the delta as `float(int32_t(newT - *prevT))` before converting to float. Apply this pattern throughout usermods/user_fx/user_fx.cpp and any other WLED effect files that compute per-frame time deltas.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
Learnt from: softhack007
Repo: wled/WLED PR: 5251
File: usermods/user_fx/user_fx.cpp:225-227
Timestamp: 2026-01-01T13:18:10.072Z
Learning: In WLED, when computing time differences between unsigned long timestamps (e.g., strip.now, lastBumpUpdate) for ESP32-C3 (RISC-V), cast the subtraction result to int before converting to float. Use float(int(timestamp1 - timestamp2)) instead of float(timestamp1 - timestamp2) to avoid wraparound issues and ensure correct behavior. Apply this pattern in usermods/user_fx/user_fx.cpp and similar timestamp-difference calculations in related modules where unsigned long subtraction could overflow before conversion.

Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5293
File: usermods/user_fx/user_fx.cpp:149-151
Timestamp: 2026-01-24T18:51:02.052Z
Learning: In Spinning Wheel style effects (e.g., WLED's Spinning Wheel), ensure that the color scale (palette mapping) matches the position modulo range. If position is computed as pos = (pos_fixed >> 16) % SEGLEN, then set the color scale auxiliary value aux1 to (255 << 8) / SEGLEN so the palette distributes evenly across the full position range. Do not use vstripLen for color scaling when position uses SEGLEN, as that causes the palette colors to cycle too quickly and appear incomplete. Apply this pattern to files in the usermods/user_fx directory (e.g., user_fx.cpp and related files) to maintain consistent palette distribution across segments.

Learnt from: softhack007
Repo: wled/WLED PR: 5355
File: wled00/util.cpp:635-638
Timestamp: 2026-02-07T16:06:08.677Z
Learning: PSRAM-related compilation guards should enable PSRAM code only for ESP32 variants that actually include PSRAM: ESP32-C61, ESP32-C5, and ESP32-P4. Exclude ESP32-C3, ESP32-C6, and ESP8266 from these guards. Apply this rule across the codebase (not just wled00/util.cpp) by reviewing and updating PSRAM guards/macros in all relevant files (C/C++ headers and sources).

Learnt from: softhack007
Repo: wled/WLED PR: 4838
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHIMethod.cpp:30-35
Timestamp: 2026-03-27T12:33:48.499Z
Learning: In C/C++ preprocessor conditionals (`#if`, `#elif`) GCC/Clang treat `&&` as short-circuit evaluated during preprocessing. This means guards like `#if defined(ARDUINO_ARCH_ESP32) && ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)` are safe even if the macro/function-like macro on the RHS (e.g., `ESP_IDF_VERSION_VAL`) is not defined on some targets, because the RHS will not be expanded when the LHS is false (e.g., `defined(...)` evaluates to 0). During code review, avoid flagging such cases as “undefined function-like macro invocation” if they are protected by short-circuiting `defined(...) && ...`/`||` logic; some tools like cppcheck may not model this and can produce false positives. Also, don’t suggest refactoring that moves ESP32-specific includes/headers (e.g., `esp_idf_version.h`) outside of these guarded preprocessor blocks, since that will break targets (e.g., ESP8266) where the headers don’t exist.

Learnt from: softhack007
Repo: wled/WLED PR: 4843
File: usermods/elastic_collisions/Elastic_Collisions.cpp:1-1
Timestamp: 2026-05-06T22:10:27.517Z
Learning: In C/C++, do not flag precedence issues for expressions that mix additive operators (`+`, `-`) with shift operators (`<<`, `>>`) based on the assumption that shifts bind tighter. Per C/C++ operator precedence, `+`/`-` have higher precedence than `<<`/`>>` (e.g., `x - edge0 << 8` parses as `(x - edge0) << 8`). When reviewing WLED (and other) fixed-point/bit-manipulation code, confirm the intended parse using cppreference before reporting a precedence bug for mixed `-`/`+` and `<<`/`>>` expressions.

Learnt from: softhack007
Repo: wled/WLED PR: 5599
File: usermods/audioreactive/audio_reactive.cpp:1303-1308
Timestamp: 2026-05-11T16:00:13.574Z
Learning: When targeting arduino-esp32 v3.x (ESP-IDF 5.x), treat `WiFiUDP::flush()` as deprecated and ineffective for RX draining; it should not be used to clear/drain the UDP receive buffer. Instead, use `WiFiUDP::clear()`, which was introduced in arduino-esp32 v3.0 and is marked as the deprecated replacement via `NetworkUdp.h`.

If the code must compile and behave correctly across both arduino-esp32 v2.x (ESP-IDF < 5) and v3.x (ESP-IDF >= 5), use conditional compilation:
- for `ESP_IDF_VERSION_MAJOR < 5`: use `udp.flush()`
- for `ESP_IDF_VERSION_MAJOR >= 5`: use `udp.clear()`

Do not flag `WiFiUDP::clear()` usages for ESP-IDF >= 5 as non-existent—this is the documented correct replacement.


if (SEGMENT.check3 && (strip.now - SEGENV.step > SEGENV.aux0)) {
SEGENV.aux0 = hw_random16(5000, 10000);
SEGENV.aux1 = hw_random8(4);
SEGENV.step = strip.now;
}

bool flipX = SEGMENT.check3 ? (SEGENV.aux1 & 0x01) : false;
bool flipY = SEGMENT.check3 ? (SEGENV.aux1 & 0x02) : false;

float targetX = flipX ? -1.0f : 1.0f;
float targetY = flipY ? -1.0f : 1.0f;
*stepX += (targetX - *stepX) * 0.05f;
*stepY += (targetY - *stepY) * 0.05f;

float dt = (float)t - *prevT;
*offX += *stepX * dt;
*offY += *stepY * dt;
*prevT = (float)t;

int32_t tX = (int32_t)*offX;
int32_t tY = (int32_t)*offY;

// Rotation
float cosA = 1.0f, sinA = 0.0f;
float cx = width * 0.5f;
float cy = height * 0.5f;

if (SEGMENT.check2) {
float angle = strip.now / 5000.0f;
cosA = cosf(angle);
sinA = sinf(angle);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}

float scale = map(SEGMENT.intensity, 0, 255, 10, 200) / 100.0f; // range 0.1 to 2.0

for (byte x = 0; x < width; x++) {
for (byte y = 0; y < height; y++) {
Comment on lines +1333 to +1334
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Prevent loop-counter overflow on wider matrices.

Line 1333 and Line 1334 use byte loop counters against uint16_t dimensions. If width or height is >= 256, these loops can wrap and never terminate.

Suggested fix 
-  for (byte x = 0; x < width; x++) {
-    for (byte y = 0; y < height; y++) {
+  for (uint16_t x = 0; x < width; x++) {
+    for (uint16_t y = 0; y < height; y++) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for (byte x = 0; x < width; x++) {
for (byte y = 0; y < height; y++) {
for (uint16_t x = 0; x < width; x++) {
for (uint16_t y = 0; y < height; y++) {
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@usermods/user_fx/user_fx.cpp` around lines 1333 - 1334, The nested loops use
byte-typed counters `x` and `y` while iterating up to `width` and `height`
(which are `uint16_t`), causing overflow when a dimension >= 256; change the
loop counters to a wider integer type (e.g., `uint16_t` or `int`) wherever `for
(byte x = 0; x < width; x++)` and `for (byte y = 0; y < height; y++)` appear (in
the user_fx loops) so they can represent the full range of `width`/`height`, and
update any related variables/headers if necessary to keep types consistent.

Copy link
Copy Markdown
Member

@softhack007 softhack007 May 16, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BobLoeffler68 in the main FX.cpp we usually don't use 8bit types at all. They are not faster, and just add subtle bugs like the "fails when width > 255" highlighted by the rabbit. It's true that wled currently does not work with a panel width > 255, but there are plans to change this in future. Simply changing "byte" to "unsigned" in both loops should do the trick.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

float rx = cosA * (x - cx) - sinA * (y - cy) + cx;
float ry = sinA * (x - cx) + cosA * (y - cy) + cy;

float scaled_x = rx * Xmult * scale;
float scaled_y = ry * Ymult * scale;

if (SEGMENT.check1) {
// Palette mode
uint8_t paletteIndex = perlin8(scaled_x, scaled_y, t);
uint8_t brightness = perlin8(scaled_x + tX, scaled_y + tY);
SEGMENT.setPixelColorXY(x, y, SEGMENT.color_from_palette(paletteIndex, false, PALETTE_SOLID_WRAP, brightness));
} else {
// Raw RGB mode
SEGMENT.setPixelColorXY(x, y, perlin8(scaled_x, scaled_y, t), perlin8(scaled_x, scaled_y + tY), perlin8(scaled_x + tX, scaled_y));
}
}
}
}
static const char _data_FX_MODE_2D_PERLINSCAPE[] PROGMEM = "Perlinscape@!,Zoom (+/-),X multiplier,Y multiplier,,Palettes,Rotation,Random direction;;!;2;";


/////////////////////
// UserMod Class //
/////////////////////
Expand All @@ -1272,6 +1369,7 @@ class UserFxUsermod : public Usermod {
strip.addEffect(255, &mode_2D_magma, _data_FX_MODE_2D_MAGMA);
strip.addEffect(255, &mode_ants, _data_FX_MODE_ANTS);
strip.addEffect(255, &mode_morsecode, _data_FX_MODE_MORSECODE);
strip.addEffect(255, &mode_2D_perlinscape, _data_FX_MODE_2D_PERLINSCAPE);

////////////////////////////////////////
// add your effect function(s) here //
Expand Down
Loading