fix the 2d drift animation

[BaptisteHudyma]

Fix a wrong cast that broke the 2D drift animation, add a small optimization to the loop.

close #3814

[blazoncek]

I do not see anything wrong with Drift.
The optimisation is ok, though.

[BaptisteHudyma]

Well, the cast to uint16 prevent the value returned by the sin/cos functions from being negative, so the projection from the screen center can not be symetrical, so the animation wont look like a wheel as advertized (https://raw.githubusercontent.com/scottrbailey/WLED-Utils/master/gifs/FX_164.gif), but as a quarter of a wheel.

The tests I did on my screen (0,0 at the top left) displays something wrong, fixed by the cast removal :)

[blazoncek]

My own testing on 20x20 and 32x32 (physical) matrices showed no errors and exactly as the advertised image.

[blazoncek]

[BaptisteHudyma]

Sorry to take your time but I need to be sure, how are the coordinates mapped on your screens ?
I tested it on a physical 25x25 screen.
The math just dont add up with the current implementation: in my case, the coordinates with the cast will go from :
colsCenter = rowsCenter = 25/2 + 1 = 13.

const uint16_t maxDim = MAX(cols, rows)/2; => 12, so i from 1 to 12.
(uint16_t)(sin_t/cost (angle) * i) -> always positive, from [0, 12]

so the animation will be displayed between [13, 13] and [25, 25], with the center of the wheel at [13, 13].
The only way to be in [1, 1] [25, 25] is to have a negative value returned by the sin and cos functions.

Am I missing something ? or maybe you are testing the wrong version ?

[blazoncek]

Have you tried running it on an actual device? If not, please do.

Still, your modification is technically correct and the old one is also correct. While not strictly mathematically it is within the scope of digital math on a CPU (underflows and overflows).
Do not get me wrong, I do approve of the change and will merge it. I am just trying to point out the intricacies of implementation behaviour which can be observed in physical output.

[BaptisteHudyma]

Sure did, and the fact that it did not work on my hardware pointed me to this correction :)

I'm used to microcontroller programming, that's why I want to understand why it can work at all with the current code with your device and not mine.
overflow do not apply here with uint16_t, underflow neither as we have no case where it can happen.

thanks for your time

(BTW I work on NRF52)

[blazoncek]

Just for consideration: what's the difference between 65535 and -1 in 16 bit integers? None.
It is just an agreement that 65535 is considered unsigned and -1 is signed. Behind the scenes they are both 0xFFFF.

So if I add 0xFFFE (-2) to 0x0002 I will get exactly the same result if using unsigned or signed integers which is 0. With unsigned the overflow flag will be set, but ignored.

If for some reasons your compiler treats uint16_t and int16_t differently (optimizes for 32 bit which is architecture native) then your result will not be the same of course.

[BaptisteHudyma]

Thank you.

More infos, if you are interested:

I see weird results from the casts:

float t = -1.0;  // sin_t and cos_t function return a float
uint16_t res1 = t * 2;  // expected 0xFFFE, got 0
// ....
uint16_t res2 = -2;  // got 0xFFFE

Same with other sizes (8, 32 bits). I dont know if it's normal behavior for this platform.
Thanks for the help, I'll investigate it.

[blazoncek]

That very much depends on the compile implementation.
Thanks for pointing it out.