Skip to content

Commit

Permalink
BUGFIX in oscillate FX (#4494)
Browse files Browse the repository at this point in the history
effect was changed from int to uint but it relied on negative numbers. fixed by checking overflow and a cast.
  • Loading branch information
DedeHai authored Jan 15, 2025
1 parent 9e37d70 commit 39b3e7e
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions wled00/FX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1827,7 +1827,7 @@ uint16_t mode_oscillate(void) {
// if the counter has increased, move the oscillator by the random step
if (it != SEGENV.step) oscillators[i].pos += oscillators[i].dir * oscillators[i].speed;
oscillators[i].size = SEGLEN/(3+SEGMENT.intensity/8);
if((oscillators[i].dir == -1) && (oscillators[i].pos <= 0)) {
if((oscillators[i].dir == -1) && (oscillators[i].pos > SEGLEN << 1)) { // use integer overflow

This comment has been minimized.

Copy link
@blazoncek

blazoncek Jan 16, 2025

Collaborator

Instead I would rather choose to modify pos to be int16_t and use int for i, j and numOscillators.
Now the logic assumes overflow of 16 bit number which is also prone to future errors.

This comment has been minimized.

Copy link
@DedeHai

DedeHai Jan 16, 2025

Author Collaborator

yea, that dawned on me after I made the commit. will adjust.

oscillators[i].pos = 0;
oscillators[i].dir = 1;
// make bigger steps for faster speeds
Expand All @@ -1843,7 +1843,7 @@ uint16_t mode_oscillate(void) {
for (unsigned i = 0; i < SEGLEN; i++) {
uint32_t color = BLACK;
for (unsigned j = 0; j < numOscillators; j++) {
if(i >= (unsigned)oscillators[j].pos - oscillators[j].size && i <= oscillators[j].pos + oscillators[j].size) {
if((int)i >= (int)oscillators[j].pos - oscillators[j].size && i <= oscillators[j].pos + oscillators[j].size) {
color = (color == BLACK) ? SEGCOLOR(j) : color_blend(color, SEGCOLOR(j), uint8_t(128));
}
}
Expand Down

0 comments on commit 39b3e7e

Please sign in to comment.