Skip to content

Arc Expand Optimizations #4994

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
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Arc Expand Optimizations #4994

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
30ee87a
Arc Expand Optmizations
Brandon502 Oct 9, 2025
8c087b9
Arc Optimizations 2
Brandon502 Oct 10, 2025
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
52 changes: 24 additions & 28 deletions wled00/FX_fcn.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ uint16_t Segment::virtualLength() const {
vLen = max(vW,vH); // get the longest dimension
break;
case M12_pArc:
vLen = sqrt32_bw(vH*vH + vW*vW); // use diagonal
vLen = (sqrt32_bw((vH*vH + vW*vW) << 2) + 1) >> 1; // diagonal rounded
break;
case M12_sPinwheel:
vLen = getPinwheelLength(vW, vH);
Expand Down Expand Up @@ -742,36 +742,25 @@ void WLED_O2_ATTR Segment::setPixelColor(int i, uint32_t col) const
if (vStrip > 0) setPixelColorRaw(XY(vStrip - 1, vH - i - 1), col);
else for (int x = 0; x < vW; x++) setPixelColorRaw(XY(x, vH - i - 1), col);
break;
case M12_pArc:
case M12_pArc: {
// expand in circular fashion from center
if (i == 0)
setPixelColorRaw(XY(0, 0), col);
else {
float r = i;
float step = HALF_PI / (2.8284f * r + 4); // we only need (PI/4)/(r/sqrt(2)+1) steps
for (float rad = 0.0f; rad <= (HALF_PI/2)+step/2; rad += step) {
int x = roundf(sin_t(rad) * r);
int y = roundf(cos_t(rad) * r);
// exploit symmetry
if (i == 0) setPixelColorXY(0, 0, col);
else if (i == 2) setPixelColorXY(1, 1, col);

int x = 0, y = i; // i is the radius
int d = -(i >> 1); // Initial decision parameter

// Barrera's circle algorithm
while (x <= y) {
if (!(i == x && i == y)) { // prevent early square
setPixelColorXY(x, y, col);
setPixelColorXY(y, x, col);
}
// Bresenham’s Algorithm (may not fill every pixel)
//int d = 3 - (2*i);
//int y = i, x = 0;
//while (y >= x) {
// setPixelColorXY(x, y, col);
// setPixelColorXY(y, x, col);
// x++;
// if (d > 0) {
// y--;
// d += 4 * (x - y) + 10;
// } else {
// d += 4 * x + 6;
// }
//}
if (d <= 0) d += ++x;
else d -= --y;
}
break;
}
case M12_pCorner:
for (int x = 0; x <= i; x++) setPixelColorXY(x, i, col); // note: <= to include i=0. Relies on overflow check in sPC()
for (int y = 0; y < i; y++) setPixelColorXY(i, y, col);
Expand Down Expand Up @@ -947,9 +936,16 @@ uint32_t WLED_O2_ATTR Segment::getPixelColor(int i) const
else { y = vH - i - 1; };
break;
case M12_pArc:
if (i > vW && i > vH) {
x = y = sqrt32_bw(i*i/2);
break; // use diagonal
if (i >= vW && i >= vH) { // Barrera's circle algorithm
int x2 = 0, y2 = i, d = -(i >> 1);
int validCount = 0; // return 2nd non mirrored pixel if available
while (x2 <= y2 && validCount < 2) {
if (y2 < vH && x2 < vW) {x = x2, y = y2; validCount++;}
else if (y2 < vW && x2 < vH) {x = y2, y = x2; validCount++;}

if (d <= 0) d += ++x2; else d -= --y2;
}
break;
Comment on lines 942 to 952
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 9, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix getPixelColor guard for pArc (breaks mapping)

setPixelColor() now draws the Barrera circle for every radius i, but getPixelColor() only runs the same loop when i >= vW && i >= vH. On typical panels (e.g. 64×32), all indices i < 64 fall straight through to the M12_pCorner fallback, so callers read back (i,0) even though we actually painted an arc of pixels. Any 1D effect that relies on getPixelColor() (tails, blurs, AA, etc.) will misbehave or flicker because read/write mappings no longer agree.

Please drop that guard (or at least require only i >= min(vW, vH)) and fall back only when the loop fails to find an in-bounds coordinate. For example:

-      case M12_pArc:
-        if (i >= vW && i >= vH) {
+      case M12_pArc: {
+        int hitCount = 0;
+        int xSel = 0, ySel = 0;
         int x2 = 0, y2 = i, d = -(i >> 1);
-        int validCount = 0;
-        while (x2 <= y2 && validCount < 2) {
-          if      (y2 < vH && x2 < vW) {x = x2, y = y2; validCount++;}
-          else if (y2 < vW && x2 < vH) {x = y2, y = x2; validCount++;}
+        while (x2 <= y2 && hitCount < 2) {
+          if (y2 < vH && x2 < vW) { xSel = x2; ySel = y2; hitCount++; }
+          else if (y2 < vW && x2 < vH) { xSel = y2; ySel = x2; hitCount++; }
           if (d <= 0) d += ++x2; else d -= --y2;
         }
-        break;
-      }
+        if (hitCount) { x = xSel; y = ySel; break; }
+        // otherwise fall through
+      }

That keeps read/write symmetry for all radii while still falling back when nothing lands inside bounds.

🤖 Prompt for AI Agents 
In wled00/FX_fcn.cpp around lines 939 to 948, remove the restrictive guard "if
(i >= vW && i >= vH)" so the Barrera circle search runs for all radii (or change
it to "i >= min(vW, vH)" if you prefer the looser check), and only fall back to
the M12_pCorner mapping when the loop completes without finding any in-bounds
coordinate (i.e., validCount remains zero). Ensure the loop sets x,y on the
first/second valid pixel as before, breaks out once the desired validCount is
reached, and after the loop you perform the fallback only when no valid pixel
was found so read/write mapping stays symmetric.

}
// otherwise fallthrough
case M12_pCorner:
Expand Down