int s = a >> 31; // cdq, signed shift, -1 if negative, else 0
to
int s = a >> 15; // cdq, signed shift, -1 if negative, else 0 on the arduino an integer is 16 bit not 32
here is the working code
inline int abs_fast(int a) {
int s = a >> 15; // cdq, signed shift, -1 if negative, else 0 on the arduino an integer is 16 bit not 32
a ^= s; // ones' complement if negative
a -= s; // plus one if negative -> two's complement if negative
return a;
}
//function line(x0, y0, x1, y1)
void draw_line(unsigned int x0,unsigned int y0,unsigned int x1,unsigned int y1,unsigned int color)
{
//dx := abs(x1-x0)
int dx=abs_fast(x1-x0);
//dy := abs(y1-y0)
int dy=abs_fast(y1-y0);
char sx,sy;
//if x0 < x1 then sx := 1 else sx := -1
if (x0 < x1)
{
sx=1;
}
else
{
sx=-1;
}
// if y0 < y1 then sy := 1 else sy := -1
if (y0 < y1)
{
sy=1;
}
else
{
sy=-1;
}
//err := dx-dy
int err=dx-dy;
// loop
while (1)
{
// setPixel(x0,y0)
Tft.setXY(x0,y0);
Tft.sendData(color);
//if x0 = x1 and y0 = y1 exit loop
if (x0 == x1 && y0 == y1)
{
break;
}
//e2 := 2*err
int e2=2*err;
// if e2 > -dy then
if (e2 > -dy)
{
// err := err - dy
err-=dy;
// x0 := x0 + sx
x0+=sx;
}
//end if
//if e2 < dx then
if (e2 < dx)
{
//err := err + dx
err+=dx;
//y0 := y0 + sy
y0+=sy;
//end if
}
}
//end loop
}
by replacing abs_fast with abs the function does not work but if I use the new abs function it woks perfect.
Please fix the built in abs function it caused me confusion for a while and I don't want programmers to think there is something wrong with their program even though there is not.
The type that results from subtracting two unsigned integers is ............ an unsigned int, so abs( X1-X0) is not going to work as you expect!
PeterO
You wouldn't, but you might want to use it on the results of an operation between two unsigned types, which when used in a macro like that will remain as an unsigned type (and not work right), but in a function with a signed int parameter will result in a signed int value, which will work right.
It's not the algorithm that's wrong but the fact that it's a typeless macro compared to a typed function.
a = 8000
b = 9000
abs(a-b) = 64536
(int)abs(a-b): -1000
Yes, given the data types, and how abs is implemented, that is perfectly correct. An unsigned int that goes negative "wraps round" to be 65536 minus the negative value.
Cast the abs specifically, and it converts that unsigned negative value into a proper negative value. The data in the variable stays the same, just how it is represented differs.
void draw_line(unsigned int x0,unsigned int y0,unsigned int x1,unsigned int y1,unsigned int color)
{
int dx = abs(x1-x0);
Serial.println (x1-x0);
Serial.println (dx);
}
void setup ()
{
Serial.begin (115200);
Serial.println ("Starting ...");
draw_line (50, 100, 0, 0, 0);
}
void loop () {}
Results:
Starting ...
65486
-50
So the argument to abs is 65486, which is positive, and is thus not changed. That is then stuffed into a signed int which then interprets it as negative.
So, Mr_arduino, your algorithm is using abs wrongly. Taking the abs of unsigned numbers is not valid. Clearly an operation on unsigned numbers must result in an unsigned number, and thus the compiler may as well optimize away any attempts to compare such number to a negative number.
inline int abs_fast(int a) {
int s = a >> 15; // cdq, signed shift, -1 if negative, else 0 on the arduino an integer is 16 bit not 32
a ^= s; // ones' complement if negative
a -= s; // plus one if negative -> two's complement if negative
return a;
}
This is not taking the absolute value, this is fiddling with bits.
This will print 50:
void draw_line(unsigned int x0,unsigned int y0,unsigned int x1,unsigned int y1,unsigned int color)
{
int dx = abs(int (x1-x0));
Serial.println (dx);
}
void setup ()
{
Serial.begin (115200);
draw_line (50, 100, 0, 0, 0);
}
void loop () {}
int myAbs (const int a)
{
return (a < 0) ? - a : a;
} // end of myAbs
void draw_line(unsigned int x0,unsigned int y0,unsigned int x1,unsigned int y1,unsigned int color)
{
int dx = myAbs(x1-x0);
Serial.println (dx);
}
void setup ()
{
Serial.begin (115200);
Serial.println ("Starting ...");
draw_line (50, 100, 0, 0, 0);
}
void loop () {}
This prints 50 because the result of x1-x0 is forced to be a signed int, and then the abs is taken of it. The "define" approach does not have that extra step. Nor does the template approach:
template <typename T>
T myAbs (const T a)
{
return (a < 0) ? - a : a;
} // end of myAbs
This is because the result of x1-x0 is still an unsigned int, and thus the myAbs function is generated for an unsigned int, not an int.
The reason I used an unsigned input for the start and end points is because it is impossible to have a pixel that is in a negative location and the line algorithm is made to handle both directions for example x1 can be bigger than x0 or the other way around.
In the following form, c = a - b whenever a is less than b, the result will be incorrect due to the "wraparound" effect. There is no need for type conversion because the difference between any unsigned types can be stored in the same type as its operands. Your problem has nothing to do with the abs function. The problem is with the subtraction. Change the code from abs(a-b) to delta(a,b).