Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
6 changes: 5 additions & 1 deletion wled00/FX.h
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,8 @@ typedef enum mapping1D2D {
M12_pBar = 1,
M12_pArc = 2,
M12_pCorner = 3,
M12_sPinwheel = 4
M12_sPinwheel = 4,
M12_Cube = 5
} mapping1D2D_t;

class WS2812FX;
Expand Down Expand Up @@ -546,6 +547,7 @@ class Segment {
// transition functions
void stopTransition(); // ends transition mode by destroying transition structure (does nothing if not in transition)
void updateTransitionProgress() const; // sets transition progress (0-65535) based on time passed since transition start
void _applyCubeWrapping(int &x, int &y) const;
Comment thread
palatter marked this conversation as resolved.
Outdated
Comment thread
palatter marked this conversation as resolved.
Outdated
inline void handleTransition() {
updateTransitionProgress();
if (isInTransition() && progress() == 0xFFFFU) stopTransition();
Expand Down Expand Up @@ -826,6 +828,7 @@ class WS2812FX {
now(millis()),
timebase(0),
isMatrix(false),
isCube(false),
#ifdef WLED_AUTOSEGMENTS
autoSegments(true),
#else
Expand Down Expand Up @@ -1007,6 +1010,7 @@ class WS2812FX {
bool autoSegments : 1;
bool correctWB : 1;
bool cctFromRgb : 1;
bool isCube : 1;
};

Segment *_currentSegment;
Expand Down
58 changes: 58 additions & 0 deletions wled00/FX_2Dfcn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,66 @@ bool Segment::isPixelXYClipped(int x, int y) const {
return false;
}

void Segment::_applyCubeWrapping(int &x, int &y) const
{
// AI: below section was generated by an AI
if (!strip.isCube || map1D2D != M12_Cube) return;
const uint16_t vH = vHeight();
const uint16_t vW = vWidth();
if (vH % 3 != 0 || vW != (vH / 3) * 4) return;

int n = vH / 3;
// AI: end
// Horizontal belt wrap (Left, Front, Right, Back)
if (y >= n && y < 2 * n) {
x %= (4 * n); if (x < 0) x += (4 * n);
return;
}

// AI: below section was generated by an AI
// Handle vertical transitions and corner gaps
int fx = (x >= 0) ? (x / n) : -1;
int fy = (y >= 0) ? (y / n) : -1;
int u = (x % n + n) % n;
int v = (y % n + n) % n;

// 1. Moving UP from Top face (Face 0) or belt faces into Top gap
if (y < n) {
if (fx == 1) { // On Top face
if (y < 0) { // Move UP to Back face (Face 4)
x = 4 * n - 1 - u; y = n + v; // Back face top meets Top face top (inverted)
}
} else { // In gaps or outside
if (fx == 0) { // Top-Left gap -> map to Top face left edge
x = n + v; y = u;
} else if (fx == 2) { // Top-Right gap -> map to Top face right edge
x = 2 * n - 1 - v; y = n - 1 - u;
} else { // Back face top part
x = (4 * n) - 1 - u; y = (n - 1) - v;
}
}
}
// 2. Moving DOWN from Bottom face (Face 5) or belt faces into Bottom gap
else if (y >= 2 * n) {
if (fx == 1) { // On Bottom face
if (y >= 3 * n) { // Move DOWN to Back face (Face 4)
x = 4 * n - 1 - u; y = 2 * n - 1 - (y - 3 * n);
}
} else { // In gaps
if (fx == 0) { // Bottom-Left gap -> map to Bottom face left edge
x = n + (n - 1 - v); y = 2 * n + u;
} else if (fx == 2) { // Bottom-Right gap -> map to Bottom face right edge
x = 2 * n - 1 - (n - 1 - v); y = 2 * n + (n - 1 - u);
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}
// AI: end
}
Comment on lines +188 to +223
Copy link
Copy Markdown
Contributor

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

fx != 1 branches silently remap out-of-range x instead of letting the caller drop it.

Inside the fy == 0 and fy == 2 blocks, the third branch uses fx != 1 with a comment indicating the intent is fx == 3 (Top/Bottom-Back gap). However, fx can also be -1 (when x < 0) or >= 4 (when x >= 4*n), because nothing earlier in the function bounds x. For those cases, this branch still fires and remaps the coordinate onto a valid Top/Bottom face cell:

  • x = -1, y in [0, n)fx = -1, hits fx != 1, mapped onto top face's left column.
  • x >= 4n, y in [0, n)fx >= 4, hits fx != 1, mapped onto top face.
  • Symmetric cases for fy == 2.

This contradicts the design that out-of-cube-net coordinates should remain out of range so the caller's (unsigned)x >= vWidth() check discards them (the same rationale used to remove the earlier [0,4n) x [0,3n) clamp). Effects that occasionally probe just off the segment edges with cube mode active may paint stray pixels onto Top/Bottom face borders.

Tightening to an exact fx == 3 keeps the back-gap fix but lets truly out-of-range writes/reads fall through to the caller's bounds check:

🛠️ Proposed fix
   } else if (fy == 0) {
     if (fx == 0) { // Top-Left gap -> maps to Top face left edge
       x = n + (n - 1 - v); y = u;
     } else if (fx == 2) { // Top-Right gap -> maps to Top face right edge
       x = n + v; y = n - 1 - u;
-    } else if (fx != 1) { // fx == 3 (Top-Back gap) -> maps to Top face top edge
+    } else if (fx == 3) { // Top-Back gap -> maps to Top face top edge
       x = 2 * n - 1 - u; y = n - 1 - v;
     }
   } else if (fy == 2) {
     if (fx == 0) { // Bottom-Left gap -> maps to Bottom face left edge
       x = n + v; y = 3 * n - 1 - u;
     } else if (fx == 2) { // Bottom-Right gap -> maps to Bottom face right edge
       x = 2 * n - 1 - v; y = 2 * n + u;
-    } else if (fx != 1) { // fx == 3 (Bottom-Back gap) -> maps to Bottom face bottom edge
+    } else if (fx == 3) { // Bottom-Back gap -> maps to Bottom face bottom edge
       x = 2 * n - 1 - u; y = 3 * n - 1 - v;
     }
   }
🤖 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 `@wled00/FX_2Dfcn.cpp` around lines 188 - 223, The fx != 1 checks in
Segment::applyCubeWrapping silently remap out-of-range x values (fx == -1 or fx
>= 4) instead of letting the caller drop them; change both occurrences inside
the fy == 0 and fy == 2 branches to test fx == 3 (the intended "Back" gap) so
only the legitimate back-gap case is remapped and true out-of-range coordinates
fall through to the caller's bounds check; update the two branches that
currently say "fx != 1" to "fx == 3" (keeping the same remap assignments to x
and y).


void IRAM_ATTR_YN Segment::setPixelColorXY(int x, int y, uint32_t col) const
{
if (!isActive()) return; // not active
if (strip.isCube && map1D2D == M12_Cube) _applyCubeWrapping(x, y);
if ((unsigned)x >= vWidth() || (unsigned)y >= vHeight()) return; // if pixel would fall out of virtual segment just exit
setPixelColorXYRaw(x, y, col);
}
Expand Down Expand Up @@ -238,6 +295,7 @@ void Segment::setPixelColorXY(float x, float y, uint32_t col, bool aa) const
// returns RGBW values of pixel
uint32_t IRAM_ATTR_YN Segment::getPixelColorXY(int x, int y) const {
if (!isActive()) return 0; // not active
if (strip.isCube && map1D2D == M12_Cube) _applyCubeWrapping(x, y);
if ((unsigned)x >= vWidth() || (unsigned)y >= vHeight()) return 0; // if pixel would fall out of virtual segment just exit
return getPixelColorXYRaw(x,y);
}
Expand Down
2 changes: 2 additions & 0 deletions wled00/cfg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ bool deserializeConfig(JsonObject doc, bool fromFS) {
JsonObject matrix = hw_led[F("matrix")];
if (!matrix.isNull()) {
strip.isMatrix = true;
CJSON(strip.isCube, matrix["isCube"]);
unsigned numPanels = matrix[F("mpc")] | 1;
numPanels = constrain(numPanels, 1, WLED_MAX_PANELS);
strip.panel.clear();
Expand Down Expand Up @@ -954,6 +955,7 @@ void serializeConfig(JsonObject root) {
// 2D Matrix Settings
if (strip.isMatrix) {
JsonObject matrix = hw_led.createNestedObject(F("matrix"));
matrix["isCube"] = strip.isCube;
matrix[F("mpc")] = strip.panel.size();
JsonArray panels = matrix.createNestedArray(F("panels"));
for (size_t i = 0; i < strip.panel.size(); i++) {
Expand Down
1 change: 1 addition & 0 deletions wled00/data/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,7 @@ function populateSegments(s)
`<option value="2" ${inst.m12==2?' selected':''}>Arc</option>`+
`<option value="3" ${inst.m12==3?' selected':''}>Corner</option>`+
`<option value="4" ${inst.m12==4?' selected':''}>Pinwheel</option>`+
`<option value="5" ${inst.m12==5?' selected':''}>Cube</option>`+
`</select></div>`+
`</div>`;
let blend = `<div class="lbl-l">Blend mode<br>`+
Expand Down
27 changes: 26 additions & 1 deletion wled00/data/settings_2D.htm
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,29 @@
UI(); // Update the preview after generating panels
}

function genCube() {
resetPanels();
var pw = parseInt(Sf.PW.value);
var ph = parseInt(Sf.PH.value);
let count = Math.min(6, maxPanels);
Sf.MPC.value = count;
for (let i = 0; i < count; i++) addPanel(i);

if (count > 0) { Sf.P0X.value = pw; Sf.P0Y.value = 0; }
if (count > 1) { Sf.P1X.value = 0; Sf.P1Y.value = ph; }
if (count > 2) { Sf.P2X.value = pw; Sf.P2Y.value = ph; }
if (count > 3) { Sf.P3X.value = 2*pw; Sf.P3Y.value = ph; }
if (count > 4) { Sf.P4X.value = 3*pw; Sf.P4Y.value = ph; }
if (count > 5) { Sf.P5X.value = pw; Sf.P5Y.value = 2*ph; }

for (let i = 0; i < count; i++) {
Sf["P"+i+"W"].value = pw;
Sf["P"+i+"H"].value = ph;
}
Sf.ISCB.checked = true;
UI();
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

function expand(o,i)
{
i.style.display = i.style.display!=="none" ? "none" : "";
Expand Down Expand Up @@ -260,7 +283,8 @@ <h2>2D setup</h2>
<select id="somp" name="SOMP" onchange="resetPanels();addPanels();UI();" >
<option value="0">1D Strip</option>
<option value="1">2D Matrix</option>
</select>
</select><br>
3D Cube: <input type="checkbox" name="ISCB" onchange="UI()">
</div>
<div id="mpdiv" style="display:none;">
<div class="sec">
Expand All @@ -284,6 +308,7 @@ <h3>Matrix Generator <button type="button" id="expGen" onclick="expand(this,gId(
<i class="warn">Pressing Populate will create LED panel layout with pre-arranged matrix.<br>Values above <i>will not</i> affect final layout.<br>
WARNING: You may need to update each panel parameters after they are generated.</i><br>
<button type="button" onclick="gen();expand(gId('expGen'),gId('mxGen'));">Populate</button>
<button type="button" onclick="genCube();expand(gId('expGen'),gId('mxGen'));">Populate Cube</button>
</div>
</div>
<div class="sec">
Expand Down
1 change: 1 addition & 0 deletions wled00/set.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -843,6 +843,7 @@ void handleSettingsSet(AsyncWebServerRequest *request, byte subPage)
if (subPage == SUBPAGE_2D)
{
strip.isMatrix = request->arg(F("SOMP")).toInt();
strip.isCube = request->hasArg(F("ISCB"));
strip.panel.clear();
if (strip.isMatrix) {
unsigned panels = constrain(request->arg(F("MPC")).toInt(), 1, WLED_MAX_PANELS);
Expand Down
1 change: 1 addition & 0 deletions wled00/xml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,7 @@ void getSettingsJS(byte subPage, Print& settingsScript)
if (subPage == SUBPAGE_2D) // 2D matrices
{
printSetFormValue(settingsScript,PSTR("SOMP"),strip.isMatrix);
printSetFormCheckbox(settingsScript,PSTR("ISCB"),strip.isCube);
#ifndef WLED_DISABLE_2D
settingsScript.printf_P(PSTR("maxPanels=%d;resetPanels();"),WLED_MAX_PANELS);
if (strip.isMatrix) {
Expand Down
Loading