Comment 30 for bug 415357

Revision history for this message
In , Simon Schubert (corecode) wrote :

Ok, I see what is going on there.

The len++ is to make the end coordinates inclusive, which they should be if drawLast is set, or if we clipped the end.

Now, we changed the end coordinates, but we keep the Bresenham error terms, because we want the same angle (I suppose).

However, if we look at fbBresSolid, we see this sequence:

 while (len--)
 {
...
/// (1) ///
  WRITE(dst, FbDoMaskRRop (READ(dst), and, xor, bits));
  bits = 0;
  dst += signdx;
...
     e += e1;
/// (2) ///
     if (e >= 0)
     {
/// (3) ///
  WRITE(dst, FbDoMaskRRop (READ(dst), and, xor, bits));
  bits = 0;
  dst += dstStride;
  e += e3;
     }
 }

Now assume we have arrived at len = 1. We start the last iteration for the last pixel, at (x2,y2). We draw the pixel (location (1)), and we *should* be done. However, because of the previously unmodified Bresenham error terms, it can happen that the error total overflows (location (2)), and we will draw another pixel, now at (x2+signdx,y2), before adjusting the error terms and exiting the loop.

In short, it might happen that (I'm using signdx=-1, just because my case happens to be that way):

- (orig_x2, orig_y2) get clipped
- the algorithm then goes on to draw:

(x1,y1), (x1-1,y1), ..., (x2,y2), (x2-1,y2)

Now, if x2 = 0, y2 = 0, then we overshoot into negative address land (-1,0) and might segfault (actually do).

Solutions
=========

I don't directly see how this could be fixed:

a) Check dst for every Bresenham error pixel, but that seems excessive.
b) Adjust the error terms, but that would change the slope of the line (slightly)
c) Check for this case in advance and reduce len, but then you'd lose one pixel at the end
d) Rewrite fbseg to draw the error pixel in the next iteration, instead of in the same. This touches central code though.