# feedback on this line function?

## Recommended Posts

This is as clean a function as I've written, I'd like to get some pointers on coding smarter in general if anyone has the inclination. Function draws a line between two points in array unsigned buf[ROW*ROW*RGB]. If slope is outside the range of 1 to -1 then swap x and y values. If x is negative then swap points.
double round(double x) { //thanks kccarter
return ((x - floor(x)) >= 0.5) ? ceil(x) : floor(x);
}
void drawLine( int x1, int y1, int x2, int y2, unsigned char buf[], unsigned char color[] ) {
int xdif = x2-x1;
int ydif = y2-y1;
if( abs(xdif) > abs(ydif) ) {
if( xdif  > 0 ) {
float slope = ydif / float(xdif);
for(int x = 0; x < xdif; x++) {
int y = int( round(slope*x) );
buf[ ( x+x1 + (y+y1)*ROW ) * RGB + RED]   = color[RED];
buf[ ( x+x1 + (y+y1)*ROW ) * RGB + GREEN] = color[GREEN];
buf[ ( x+x1 + (y+y1)*ROW ) * RGB + BLUE]  = color[BLUE];
}
} else drawLine( x2, y2, x1, y1, buf ); //swap points 1 & 2
} else { //same code but swap x and y values
if( ydif  > 0 ) {
float slope = xdif / float(ydif);
for(int y = 0; y < ydif; y++) {
int x = int( round(slope*y) );
buf[ ( x+x1 + (y+y1)*ROW ) * RGB + RED]   = color[RED];
buf[ ( x+x1 + (y+y1)*ROW ) * RGB + GREEN] = color[GREEN];
buf[ ( x+x1 + (y+y1)*ROW ) * RGB + BLUE]  = color[BLUE];
}
} else if( xdif != ydif ) drawLine( x2, y2, x1, y1, buf ); //swap points 1 & 2
}
}


[Edited by - avocados on March 5, 2010 9:12:19 AM]

##### Share on other sites
There's one major bug: It will loop forever (possibly stack overflowing) if the start and end coordinates are equal.

I'd also suggest letting the caller of the function pick the colour of the line they want drawing.

Also adding 0.5 then casting to int doesn't round negative numbers correctly. Consider -0.75 for example.

##### Share on other sites
Bresenham's algorithm is typically implemented without floating point computations.

See the article for a thorough discussion on trade-offs and implementation details.

##### Share on other sites
I defeintely second Bresenham's algorithm for a start. In your case we're easily looking at a 5 times speedup. Float to int conversion is slow! There even are improvement over Bresenham's, but it's a good starting point.

Also, don't put too much faith in your compiler's common subexpression elimination optimisation. For one, your code will run far slower in debug when it isn't performing that optimisation and it might not as fast as it could in a release build if it doesn't see the optimisation there either.

##### Share on other sites
Quote:
 Original post by iMalcAlso, don't put too much faith in your compiler's common subexpression elimination optimisation. For one, your code will run far slower in debug when it isn't performing that optimisation and it might not as fast as it could in a release build if it doesn't see the optimisation there either.

For another, it's messy. If you eliminate a common subexpression, and then there turns out to be something wrong with the subexpression, now there's only one place where you have to fix it instead of many.

## Create an account

Register a new account

• ### Forum Statistics

• Total Topics
628281
• Total Posts
2981801

• 10
• 11
• 17
• 14
• 9