2
\$\begingroup\$

I have tested all the cases of how a line could be

  1. vertically
  2. horizontally
  3. has a slope that's positive or less than 1

The function works, but I would like it reviewed, such as if there are overflows, etc.

// Draw line using DDA Algorithm
void Graphics::DrawLine( int x1, int y1, int x2, int y2, Color&color )
{

    float xdiff = x1-x2; 
    float ydiff = y1-y2; 
    int slope  = 1; 
    if ( y1  == y2  )
    {
        slope = 0;
    }
    else if (  x1 == x2 )
    {
        slope = 2; // vertical lines have no slopes...
    }
    else
    {
        slope = (int)xdiff/ydiff; 
    }

    if ( slope <= 1 )
    {    
        int startx = 0;
        int endx   = 0;
        if ( x1 > x2 )
        {
            startx = x2;
            endx   = x1;
        }
        else 
        {
            startx = x1;
            endx   = x2;
        }

        float y = y1; // initial value
        for(int x = startx; x <= endx; x++)
        {
            y += slope;
            DrawPixel(x, (int)abs(y), color);
        }
    }

    else if ( slope > 1 )
    {
        float x = x1; // initial value
        for(int y = y1;y <= y2; y++)
        {
            x += 1/slope;
            DrawPixel((int)x, y, color);
        }

    }

}
\$\endgroup\$
2
  • \$\begingroup\$ en.wikipedia.org/wiki/Bresenham's_line_algorithm \$\endgroup\$
    – Snowbody
    Commented Jul 16, 2014 at 15:15
  • 2
    \$\begingroup\$ Question is almost year old. Working code for him (with flaws). I do not see any reason why we should close this question now as answer is also provided and accepted. \$\endgroup\$
    – chillworld
    Commented Jul 16, 2014 at 20:50

2 Answers 2

5
\$\begingroup\$

Here are some flaws in the current algorithm.

  • xdiff will have a roundoff error if x1-x2 is large in magnitude, likewise for the y variables.
  • slope is set to 1 for no reason, and then immediately reinitialized to something else.
  • slope is restricted to an integer, when most slopes will not be integral.
  • It's impossible to justify setting the slope of a vertical line to 2. There are slopes greater than 2 that are not vertical lines.
  • In the line slope = (int)xdiff/ydiff;, casting is higher precedence than division, so this will first cast xdiff to an int, overflowing if xdiff > MAXINT, throwing away the fractional part if there is one, then dividing this int by ydiff. The odds of this being the actual slope are very small.
  • startx and endx are initialized to 0 for no reason, then immediately reinitialized to somethiing else.
  • Roundoff errors will accumulate as you repeatedly add slope to y (or 1/slope to x).
  • What's the justification for the abs(y)? If y changes sign in the range, this will cause a kink in the line. Similarly for x.
\$\endgroup\$
1
\$\begingroup\$

This makes no sense. The slope must be an int such as 0, 1, 2, 3, 4, … but a vertical line is treated as a line of slope 2? How is that accurate at all?

\$\endgroup\$
4
  • \$\begingroup\$ How would I solve it ? Vertical lines have no slope at all. \$\endgroup\$ Commented Oct 13, 2013 at 10:04
  • \$\begingroup\$ DDA. Make two cases. If abs(ydiff) <= abs(xdiff), then proceed as usual with slope = ydiff / xdiff and iterate along the x-axis pixels, plotting y. Otherwise, reverse the roles of the x and y axes — compute an inverseSlope = xdiff / ydiff and iterate along the y-axis pixels, plotting x. \$\endgroup\$ Commented Oct 13, 2013 at 10:22
  • \$\begingroup\$ I have the book Fundamental of computer graphics, by peter shirely, but it doesn't really explain the algorithms really well, can you suggest a better reference for software rasterizations ? \$\endgroup\$ Commented Oct 13, 2013 at 10:49
  • \$\begingroup\$ Go from a slope to a dx and a dy, with either the dx or dy being 1. \$\endgroup\$
    – Snowbody
    Commented Jul 17, 2014 at 15:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.