WLED Arc Expand Optimizations

New Arc algorithm that is more optimized and fixes holes on larger matrices (32x8 64x32 etc). FPS on 64x32 Rainbow went from 45 FPS to 80 FPS on esp32dev hub75 build. Looks slightly different but unnoticeable on most effects. gPC is more accurate and seems to fix the last few effects that still use it.

Videos comparing the new (reversed) and previous version. Left is new, right is old.

https://github.com/user-attachments/assets/f974f134-6cc0-4785-abff-7e5e0002e223

https://github.com/user-attachments/assets/72fbba36-3844-4a24-a1f7-7290817c7e3a

Summary by CodeRabbit

Bug Fixes
- Improved arc rendering consistency on 2D layouts, removing early-frame square artifacts and reducing off-by-one diagonal distortions.
- More accurate handling of circle-edge cases for precise pixel mapping.
Refactor
- Replaced previous sampling with a more efficient circle-based arc-drawing approach for reduced computation and improved responsiveness.

Oct 09 '25 03:10 Brandon502

Walkthrough

Replaces M12_pArc 2D arc rendering with an integer Bresenham/Tony Barrera circle algorithm, adjusts diagonal length rounding, updates setPixelColor/getPixelColor logic, adds structural braces, and refines initial/symmetric point handling in wled00/FX_fcn.cpp.

Changes

Cohort / File(s)	Summary
2D pArc algorithm and rendering `wled00/FX_fcn.cpp`	Changed diagonal length calc for M12_pArc to rounded integer form. Replaced center-based per-pixel sampling with a Barrera/Bresenham-style circle algorithm in `setPixelColor` and mirrored logic in `getPixelColor`, including integer arithmetic, symmetry plotting, early-square guard, and added braces/structural adjustments around M12_pArc case blocks.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title “Arc Expand Optimizations” clearly and concisely describes the primary change, focusing on the optimized arc expansion algorithm and performance improvements introduced in the pull request.

✨ Finishing touches

[ ] 📝 Generate docstrings

🧪 Generate unit tests (beta)

[ ] Create PR with unit tests
[ ] Post copyable unit tests in a comment

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Oct 09 '25 03:10 coderabbitai[bot]

I find this very interesting! Thank you. Unfortunately you do not show the behaviour on non-square segments. As mentioned by @coderabbitai.

Oct 09 '25 05:10 blazoncek

[!TIP] For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all comments by coderabbitai on this PR
gh pr view 4994 --repo wled/WLED --json comments --jq '.comments[] | select(.author.login == "coderabbitai") | {created: .createdAt, body: .body}'

Length of output: 9686

🏁 Script executed:

#!/bin/bash
# Get all review comments by coderabbitai on this PR
gh api repos/wled/WLED/pulls/4994/comments --jq '.[] | select(.user.login == "coderabbitai") | {line: .line, path: .path, body: .body}' | head -100

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Get reviews by coderabbitai on this PR
gh api repos/wled/WLED/pulls/4994/reviews --jq '.[] | select(.user.login == "coderabbitai") | {state: .state, body: .body, submitted_at: .submitted_at}'

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Get the full PR timeline including review comments
gh api graphql -f query='
{
  repository(owner: "wled", name: "WLED") {
    pullRequest(number: 4994) {
      reviews(first: 100) {
        nodes {
          author {
            login
          }
          body
          comments(first: 100) {
            nodes {
              body
              path
            }
          }
        }
      }
    }
  }
}'

Length of output: 5010

@blazoncek, you're absolutely right to highlight this! The comparison videos don't clearly demonstrate the behavior on non-square segments, which is particularly important given the technical concern I raised in my review.

Specifically, on non-square panels (like the mentioned 32×8 or 64×32), the getPixelColor() guard condition if (i >= vW && i >= vH) in the new implementation means that for a 64×32 panel, any index i < 64 will skip the Barrera circle logic entirely and fall through to the M12_pCorner mapping. This creates a read/write mismatch where:

setPixelColor() draws arc pixels using the Barrera algorithm for all radii
getPixelColor() only uses the same algorithm when i >= max(vW, vH), otherwise returns (i, 0)

This asymmetry could cause visual artifacts in effects that rely on reading back pixel colors (e.g., tails, blurs, anti-aliasing).

It would be very helpful if @Brandon502 could provide comparison videos specifically showing non-square segments to validate that:

The visual output looks correct on non-square panels
Effects that use getPixelColor() behave properly without flickering or incorrect tails

Testing on a 32×8 segment would be particularly valuable since it has the most extreme aspect ratio mentioned in the PR.

🐰🔍

Oct 09 '25 05:10 coderabbitai[bot]

I find this very interesting! Thank you. Unfortunately you do not show the behaviour on non-square segments. As mentioned by @coderabbitai.

Here's a video of both versions of arc running 64x32 freqmatrix, 64x22 rainbow, and 64x8 dj lights. You can see the hole in the larger sizes on the old version and gPC breaks in the corner. New gPC works similar to old and uses corner when it can. I think there's still a chance that it won't work on the very last pixel due to unneeded rounding on certain sizes. I don't think any effects currently rely on going backwards. I can add condition in sPC that always paints the bottom right pixel when i == vLength() -1 to fix though if necessary.

https://github.com/user-attachments/assets/193c7510-74d0-4f9f-a535-131ea290fcdb

https://github.com/user-attachments/assets/d2b05112-1776-4a5d-85fe-5d7da15ed4dc

Oct 09 '25 16:10 Brandon502

[!TIP] For best results, initiate chat on the files or code changes.

@Brandon502, thank you for providing these comparison videos! They clearly demonstrate the improvements, especially:

✅ Visual holes fixed: The old version's gaps in larger matrices (64×32, 64×8) are visibly eliminated
✅ Non-square behavior validated: The arc expansion works correctly across different aspect ratios
✅ Corner fallback working: The gPC behavior you described (using corner mapping when appropriate) matches the implementation and maintains compatibility

Regarding the last pixel rounding issue you mentioned:

I'd recommend adding the safety check you proposed. Even though no current effects rely on backward traversal, it's good defensive programming to ensure i == vLen - 1 always maps to the bottom-right corner pixel. This would:

Prevent potential issues if future effects use reverse iteration
Ensure mathematical consistency at boundary conditions
Add negligible overhead (single comparison per call)

The check could be as simple as:

if (i == vLen - 1) {
  setPixelColorXY(vW - 1, vH - 1, col);
  return;
}

This would guarantee correctness without affecting the performance gains you've achieved (45 → 80 FPS on 64×32 is impressive!).

Would you like to add this safety check, or would you prefer to leave it as-is given that no current effects are affected?

🐰

Oct 09 '25 16:10 coderabbitai[bot]