Skip to content
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.

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

Optimzation: code consolidation in drawCircle() #4302

Open
wants to merge 1 commit into
base: 0_15
Choose a base branch
from
Open
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
50 changes: 25 additions & 25 deletions wled00/FX_2Dfcn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ void Segment::drawCircle(uint16_t cx, uint16_t cy, uint8_t radius, uint32_t col,
if (!isActive() || radius == 0) return; // not active
if (soft) {
// Xiaolin Wu’s algorithm
int rsq = radius*radius;
const int rsq = radius*radius;
int x = 0;
int y = radius;
unsigned oldFade = 0;
Expand All @@ -516,37 +516,37 @@ void Segment::drawCircle(uint16_t cx, uint16_t cy, uint8_t radius, uint32_t col,
unsigned fade = float(0xFFFF) * (ceilf(yf) - yf); // how much color to keep
if (oldFade > fade) y--;
oldFade = fade;
setPixelColorXY(cx+x, cy+y, color_blend(col, getPixelColorXY(cx+x, cy+y), fade, true));
setPixelColorXY(cx-x, cy+y, color_blend(col, getPixelColorXY(cx-x, cy+y), fade, true));
setPixelColorXY(cx+x, cy-y, color_blend(col, getPixelColorXY(cx+x, cy-y), fade, true));
setPixelColorXY(cx-x, cy-y, color_blend(col, getPixelColorXY(cx-x, cy-y), fade, true));
setPixelColorXY(cx+y, cy+x, color_blend(col, getPixelColorXY(cx+y, cy+x), fade, true));
setPixelColorXY(cx-y, cy+x, color_blend(col, getPixelColorXY(cx-y, cy+x), fade, true));
setPixelColorXY(cx+y, cy-x, color_blend(col, getPixelColorXY(cx+y, cy-x), fade, true));
setPixelColorXY(cx-y, cy-x, color_blend(col, getPixelColorXY(cx-y, cy-x), fade, true));
setPixelColorXY(cx+x, cy+y-1, color_blend(getPixelColorXY(cx+x, cy+y-1), col, fade, true));
setPixelColorXY(cx-x, cy+y-1, color_blend(getPixelColorXY(cx-x, cy+y-1), col, fade, true));
setPixelColorXY(cx+x, cy-y+1, color_blend(getPixelColorXY(cx+x, cy-y+1), col, fade, true));
setPixelColorXY(cx-x, cy-y+1, color_blend(getPixelColorXY(cx-x, cy-y+1), col, fade, true));
setPixelColorXY(cx+y-1, cy+x, color_blend(getPixelColorXY(cx+y-1, cy+x), col, fade, true));
setPixelColorXY(cx-y+1, cy+x, color_blend(getPixelColorXY(cx-y+1, cy+x), col, fade, true));
setPixelColorXY(cx+y-1, cy-x, color_blend(getPixelColorXY(cx+y-1, cy-x), col, fade, true));
setPixelColorXY(cx-y+1, cy-x, color_blend(getPixelColorXY(cx-y+1, cy-x), col, fade, true));
int px, py;
for (uint8_t i = 0; i < 16; i++) {
int swaps = (i & 0x4 ? 1 : 0); // 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0, 1, 1, 1, 1
int adj = (i < 8) ? 0 : 1; // 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1
int dx = (i & 1) ? -1 : 1; // 1, -1, 1, -1, 1, -1, 1, -1, 1, -1, 1, -1, 1, -1, 1, -1
int dy = (i & 2) ? -1 : 1; // 1, 1, -1, -1, 1, 1, -1, -1, 1, 1, -1, -1, 1, 1, -1, -1
if (swaps) {
px = cx + (y - adj) * dx;
py = cy + x * dy;
} else {
px = cx + x * dx;
py = cy + (y - adj) * dy;
}
uint32_t pixCol = getPixelColorXY(px, py);
setPixelColorXY(px, py, adj ?
color_blend(pixCol, col, fade, true) :
color_blend(col, pixCol, fade, true));
}
x++;
}
} else {
// Bresenham’s Algorithm
int d = 3 - (2*radius);
int y = radius, x = 0;
while (y >= x) {
setPixelColorXY(cx+x, cy+y, col);
setPixelColorXY(cx-x, cy+y, col);
setPixelColorXY(cx+x, cy-y, col);
setPixelColorXY(cx-x, cy-y, col);
setPixelColorXY(cx+y, cy+x, col);
setPixelColorXY(cx-y, cy+x, col);
setPixelColorXY(cx+y, cy-x, col);
setPixelColorXY(cx-y, cy-x, col);
for (int i = 0; i < 4; i++) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

My suggestion would be to compact the "soft" Wu's algo (as you did), but keep Bresenham's code untouched.
Why: in some cases speed does matter, so I'd like to have the basic circle drawing as fast as possible. Also you did not save much code on Bresenham.

The "soft" algorithm is much slower any way, so I'd agree with you that making the code more compact here is better for readability.

Copy link
Collaborator Author

@DedeHai DedeHai Nov 22, 2024

Choose a reason for hiding this comment

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

I ran a few more tests: there is no measureable difference in speed using the Bresenham's code, the "overhead" added by the loop is really small when comparing it to a setPixelColorXY() call. I got the exact same FPS with and without the changes using a modified ripple FX drawing about 100 circles. Also: there is no FX currently using the Bresenham's circle version.

With the Wu's algorithm, there is a slight drop in speed. Using the same modified ripple FX frame rate drops by 0.6FPS from 42FPS to 41.4FPS worst case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also: there is no FX currently using the Bresenham's circle version.

There's an open PR that uses it.

int dx = (i & 1) ? -x : x;
int dy = (i & 2) ? -y : y;
setPixelColorXY(cx + dx, cy + dy, col);
setPixelColorXY(cx + dy, cy + dx, col);
}
x++;
if (d > 0) {
y--;
Expand Down