Skip to content
Open
Show file tree
Hide file tree
Changes from all 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;
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
39 changes: 39 additions & 0 deletions wled00/FX_2Dfcn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,47 @@ 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;
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;

if (fy < 0 || fy > 2) { // Above Top or Below Bottom
x = 4 * n - 1 - u;
y = 2 * n - 1 - v;
} 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
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
x = 2 * n - 1 - u; y = 3 * n - 1 - v;
}
}
// 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 +276,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