Skip to content

speed optimisations, fix for restoreColorLossy, code cleanup #4895

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
+138 −151
Open

speed optimisations, fix for restoreColorLossy, code cleanup #4895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3641db2
speed optimizations, fix for restoreColorLossy, code cleanup
DedeHai Aug 31, 2025
fa06732
speed optimization in color_add, PS fast_color_add and blur functions
DedeHai Sep 13, 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
60 changes: 25 additions & 35 deletions wled00/FX_2Dfcn.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ void WS2812FX::setUpMatrix() {
#ifndef WLED_DISABLE_2D
// pixel is clipped if it falls outside clipping range
// if clipping start > stop the clipping range is inverted
bool IRAM_ATTR_YN Segment::isPixelXYClipped(int x, int y) const {
bool Segment::isPixelXYClipped(int x, int y) const {
if (blendingStyle != BLEND_STYLE_FADE && isInTransition() && _clipStart != _clipStop) {
const bool invertX = _clipStart > _clipStop;
const bool invertY = _clipStartY > _clipStopY;
Expand Down Expand Up @@ -186,7 +186,7 @@ bool IRAM_ATTR_YN Segment::isPixelXYClipped(int x, int y) const {
void IRAM_ATTR_YN Segment::setPixelColorXY(int x, int y, uint32_t col) const
{
if (!isActive()) return; // not active
if (x >= (int)vWidth() || y >= (int)vHeight() || x < 0 || y < 0) return; // if pixel would fall out of virtual segment just exit
if ((unsigned)x >= vWidth() || (unsigned)y >= vHeight()) return; // if pixel would fall out of virtual segment just exit
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why don't you go all the way and change every int into unsigned?

It would save you a few manual conversions like this and the code would be cleaner.

However, do check if any of the effects produce negative index/coordinates to avoid other errors.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

"Get/setPixelColorX: instead of casting vWidth/vHeight to int, cast X and Y to unsigned and save the negative check. Makes code size smaller so I assume it is faster too. I also thought about making X/Y coordinated unsigned in general to get more consistency throughout the code but decided I will leave that to a futuer endeavour."

Copy link
Collaborator

Choose a reason for hiding this comment

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

int was introduced to combat issues @softhack007 noticed with a few SR effects (possibly others too) that produced negative indexes.
Originally indexes were uint16_t.

Copy link
Collaborator Author

@DedeHai DedeHai Sep 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

1D uses int for "vitual strips" IIRC, I wanted to convert 2D to unsigned before but struggled with "ambiguous" function calls I was not sure how to resolve so I abandoned it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

1D uses int for "vitual strips"

not int actually. it encodes virtual strip number into upper 16 bits. which means it can produce incorrect results.

setPixelColorXYRaw(x, y, col);
}

Expand Down Expand Up @@ -236,7 +236,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 (x >= (int)vWidth() || y >= (int)vHeight() || x<0 || y<0) return 0; // if pixel would fall out of virtual segment just exit
if ((unsigned)x >= vWidth() || (unsigned)y >= vHeight()) return 0; // if pixel would fall out of virtual segment just exit
return getPixelColorXYRaw(x,y);
}

Expand All @@ -246,52 +246,42 @@ void Segment::blur2D(uint8_t blur_x, uint8_t blur_y, bool smear) const {
const unsigned cols = vWidth();
const unsigned rows = vHeight();
const auto XY = [&](unsigned x, unsigned y){ return x + y*cols; };
uint32_t lastnew; // not necessary to initialize lastnew and last, as both will be initialized by the first loop iteration
uint32_t last;
if (blur_x) {
const uint8_t keepx = smear ? 255 : 255 - blur_x;
const uint8_t seepx = blur_x >> 1;
for (unsigned row = 0; row < rows; row++) { // blur rows (x direction)
uint32_t carryover = BLACK;
uint32_t curnew = BLACK;
for (unsigned x = 0; x < cols; x++) {
uint32_t cur = getPixelColorRaw(XY(x, row));
uint32_t part = color_fade(cur, seepx);
curnew = color_fade(cur, keepx);
if (x > 0) {
if (carryover) curnew = color_add(curnew, carryover);
uint32_t prev = color_add(lastnew, part);
// optimization: only set pixel if color has changed
if (last != prev) setPixelColorRaw(XY(x - 1, row), prev);
} else setPixelColorRaw(XY(x, row), curnew); // first pixel
lastnew = curnew;
last = cur; // save original value for comparison on next iteration
// handle first pixel in row to avoid conditional in loop (faster)
uint32_t cur = getPixelColorRaw(XY(0, row));
uint32_t carryover = fast_color_scale(cur, seepx);
setPixelColorRaw(XY(0, row), fast_color_scale(cur, keepx));
for (unsigned x = 1; x < cols; x++) {
cur = getPixelColorRaw(XY(x, row));
uint32_t part = fast_color_scale(cur, seepx);
cur = fast_color_scale(cur, keepx);
cur = color_add(cur, carryover);
setPixelColorRaw(XY(x - 1, row), color_add(getPixelColorRaw(XY(x-1, row)), part)); // previous pixel
setPixelColorRaw(XY(x, row), cur); // current pixel
carryover = part;
}
setPixelColorRaw(XY(cols-1, row), curnew); // set last pixel
}
}
if (blur_y) {
const uint8_t keepy = smear ? 255 : 255 - blur_y;
const uint8_t seepy = blur_y >> 1;
for (unsigned col = 0; col < cols; col++) {
uint32_t carryover = BLACK;
uint32_t curnew = BLACK;
for (unsigned y = 0; y < rows; y++) {
uint32_t cur = getPixelColorRaw(XY(col, y));
uint32_t part = color_fade(cur, seepy);
curnew = color_fade(cur, keepy);
if (y > 0) {
if (carryover) curnew = color_add(curnew, carryover);
uint32_t prev = color_add(lastnew, part);
// optimization: only set pixel if color has changed
if (last != prev) setPixelColorRaw(XY(col, y - 1), prev);
} else setPixelColorRaw(XY(col, y), curnew); // first pixel
lastnew = curnew;
last = cur; //save original value for comparison on next iteration
// handle first pixel in column
uint32_t cur = getPixelColorRaw(XY(col, 0));
uint32_t carryover = fast_color_scale(cur, seepy);
setPixelColorRaw(XY(col, 0), fast_color_scale(cur, keepy));
for (unsigned y = 1; y < rows; y++) {
cur = getPixelColorRaw(XY(col, y));
uint32_t part = fast_color_scale(cur, seepy);
cur = fast_color_scale(cur, keepy);
cur = color_add(cur, carryover);
setPixelColorRaw(XY(col, y - 1), color_add(getPixelColorRaw(XY(col, y-1)), part)); // previous pixel
setPixelColorRaw(XY(col, y), cur); // current pixel
carryover = part;
}
setPixelColorRaw(XY(col, rows - 1), curnew);
}
}
}
Expand Down
36 changes: 15 additions & 21 deletions wled00/FX_fcn.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,7 @@ uint16_t Segment::maxMappingLength() const {
#endif
// pixel is clipped if it falls outside clipping range
// if clipping start > stop the clipping range is inverted
bool IRAM_ATTR_YN Segment::isPixelClipped(int i) const {
bool Segment::isPixelClipped(int i) const {
if (blendingStyle != BLEND_STYLE_FADE && isInTransition() && _clipStart != _clipStop) {
bool invert = _clipStart > _clipStop; // ineverted start & stop
int start = invert ? _clipStop : _clipStart;
Expand All @@ -691,7 +691,7 @@ bool IRAM_ATTR_YN Segment::isPixelClipped(int i) const {
return false;
}

void IRAM_ATTR_YN Segment::setPixelColor(int i, uint32_t col) const
void WLED_O2_ATTR Segment::setPixelColor(int i, uint32_t col) const
{
if (!isActive() || i < 0) return; // not active or invalid index
#ifndef WLED_DISABLE_2D
Expand Down Expand Up @@ -904,7 +904,7 @@ void Segment::setPixelColor(float i, uint32_t col, bool aa) const
}
#endif

uint32_t IRAM_ATTR_YN Segment::getPixelColor(int i) const
uint32_t WLED_O2_ATTR Segment::getPixelColor(int i) const
{
if (!isActive() || i < 0) return 0; // not active or invalid index

Expand Down Expand Up @@ -1043,7 +1043,7 @@ void Segment::fadeToSecondaryBy(uint8_t fadeBy) const {
void Segment::fadeToBlackBy(uint8_t fadeBy) const {
if (!isActive() || fadeBy == 0) return; // optimization - no scaling to apply
const size_t rlength = rawLength(); // calculate only once
for (unsigned i = 0; i < rlength; i++) setPixelColorRaw(i, color_fade(getPixelColorRaw(i), 255-fadeBy));
for (unsigned i = 0; i < rlength; i++) setPixelColorRaw(i, fast_color_scale(getPixelColorRaw(i), 255-fadeBy));
}

/*
Expand All @@ -1063,25 +1063,19 @@ void Segment::blur(uint8_t blur_amount, bool smear) const {
uint8_t keep = smear ? 255 : 255 - blur_amount;
uint8_t seep = blur_amount >> 1;
unsigned vlength = vLength();
uint32_t carryover = BLACK;
uint32_t lastnew; // not necessary to initialize lastnew and last, as both will be initialized by the first loop iteration
uint32_t last;
uint32_t curnew = BLACK;
for (unsigned i = 0; i < vlength; i++) {
uint32_t cur = getPixelColorRaw(i);
uint32_t part = color_fade(cur, seep);
curnew = color_fade(cur, keep);
if (i > 0) {
if (carryover) curnew = color_add(curnew, carryover);
uint32_t prev = color_add(lastnew, part);
// optimization: only set pixel if color has changed
if (last != prev) setPixelColorRaw(i - 1, prev);
} else setPixelColorRaw(i, curnew); // first pixel
lastnew = curnew;
last = cur; // save original value for comparison on next iteration
// handle first pixel to avoid conditional in loop (faster)
uint32_t cur = getPixelColorRaw(0);
uint32_t carryover = fast_color_scale(cur, seep);
setPixelColorRaw(0, fast_color_scale(cur, keep));
for (unsigned i = 1; i < vlength; i++) {
cur = getPixelColorRaw(i);
uint32_t part = fast_color_scale(cur, seep);
cur = fast_color_scale(cur, keep);
cur = color_add(cur, carryover);
setPixelColorRaw(i - 1, color_add(getPixelColorRaw(i - 1), part)); // previous pixel
setPixelColorRaw(i, cur); // current pixel
carryover = part;
}
setPixelColorRaw(vlength - 1, curnew);
}

/*
Expand Down
Loading