Jonas Smedegaard wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Paul wrote:

In addition to my previous fixes for anti-aliased lines (bug #364024),
this corrects a segfault when you try and draw a short line outside
image bounds (so short it is really a pixel).

Attached is a test case and a patch that corrects the problem.

thanks, sorry I didn't get this out sooner,
Paul


Thanks a lot!

A new package is in the works - should be out tonight (european time).


- -  Jonas


Hi Jonas,

Apologies, I prepared a patch for upstream and found something else to improve 
on.

The attached patch REPLACES 1003_fix_aa_segfault.patch, 1004_improve_aa_lines.patch and the just-sent 1009_fix_aa_segfault_2.patch.

There is also attached a little script that builds libgd2-2.0.33-5.2 the way I do, removing my old 1003,1004 patches and adding the new one.

There is also a test file and script that will build the AA test and run it. Look at test_aa_?.png files to check for joins (you can do this before and after applying the new patch.

A main/only difference between the 1003,1004,1009 patch set and the new patch is the clipping algorithm for AA lines has been improved for higher accuracy. This should probably be used for the gdImageLine() as well, I'll talk to upstream about it first.

thanks!
Paul
diff -ruN libgd2-2.0.33/gd.c libgd2-2.0.33.nmu/gd.c
--- libgd2-2.0.33/gd.c	2006-11-08 12:23:24.000000000 +0000
+++ libgd2-2.0.33.nmu/gd.c	2006-12-29 06:40:28.000000000 +0000
@@ -697,6 +697,67 @@
   return 1;
 }
 
+
+// for long variables, for AA
+
+static long
+clip_1d_long (long *x0, long *y0, long *x1, long *y1, long mindim, long maxdim)
+{
+  double m;			/* gradient of line */
+  if (*x0 < mindim)
+    {				/* start of line is left of window */
+      if (*x1 < mindim)		/* as is the end, so the line never cuts the window */
+	return 0;
+      m = (*y1 - *y0) / (double) (*x1 - *x0);	/* calculate the slope of the line */
+      /* adjust x0 to be on the left boundary (ie to be zero), and y0 to match */
+      *y0 -= m * (*x0 - mindim);
+      *x0 = mindim;
+      /* now, perhaps, adjust the far end of the line as well */
+      if (*x1 > maxdim)
+	{
+	  *y1 += m * (maxdim - *x1);
+	  *x1 = maxdim;
+	}
+      return 1;
+    }
+  if (*x0 > maxdim)
+    {				/* start of line is right of window -
+				   complement of above */
+      if (*x1 > maxdim)		/* as is the end, so the line misses the window */
+	return 0;
+      m = (*y1 - *y0) / (double) (*x1 - *x0);	/* calculate the slope of the line */
+      *y0 += m * (maxdim - *x0);	/* adjust so point is on the right
+					   boundary */
+      *x0 = maxdim;
+      /* now, perhaps, adjust the end of the line */
+      if (*x1 < mindim)
+	{
+	  *y1 -= m * (*x1 - mindim);
+	  *x1 = mindim;
+	}
+      return 1;
+    }
+  /* the final case - the start of the line is inside the window */
+  if (*x1 > maxdim)
+    {				/* other end is outside to the right */
+      m = (*y1 - *y0) / (double) (*x1 - *x0);	/* calculate the slope of the line */
+      *y1 += m * (maxdim - *x1);
+      *x1 = maxdim;
+      return 1;
+    }
+  if (*x1 < mindim)
+    {				/* other end is outside to the left */
+      m = (*y1 - *y0) / (double) (*x1 - *x0);	/* calculate the slope of the line */
+      *y1 -= m * (*x1 - mindim);
+      *x1 = mindim;
+      return 1;
+    }
+  /* only get here if both points are inside the window */
+  return 1;
+}
+
+
+
 /* end of line clipping code */
 
 BGD_DECLARE(void) gdImageSetPixel (gdImagePtr im, int x, int y, int color)
@@ -3239,70 +3300,119 @@
 	im->tpixels[y][x]=gdTrueColorAlpha(dr, dg, db,  gdAlphaOpaque);
 }  
 
+/* simple helper */
+inline int min_int(int a, int b) { return (a < b ? a : b); }
+inline long min_long(long a, long b) { return (a < b ? a : b); }
+
 static void gdImageAALine (gdImagePtr im, int x1, int y1, int x2, int y2, int col)
 {
 	/* keep them as 32bits */
 	long x, y, inc;
-	long dx, dy,tmp;
+	long dxL, dyL,tmp;
 	if (!im->trueColor) {
 		/* TBB: don't crash when the image is of the wrong type */
 		gdImageLine(im, x1, y1, x2, y2, col);
 		return;
 	}
-        /* TBB: use the clipping rectangle */
-        if (clip_1d (&x1, &y1, &x2, &y2, im->cx1, im->cx2) == 0)
-          return;
-        if (clip_1d (&y1, &x1, &y2, &x2, im->cy1, im->cy2) == 0)
-          return;
-	dx = x2 - x1;
-	dy = y2 - y1;
 
-	if (dx == 0 && dy == 0) {
+  /* TBB: use the clipping rectangle */
+   /* use expanded image bounds to ensure we get all the AA pixels drawn on the edges */
+   /* Note the +1 and -1 */
+   /* Note also: clip using 32 bit numbers.  To avoid rounding errors. 
+    * I put 'L' after the variable name to be explicit of what changes what. */
+   long x1L = x1 << 16;
+   long y1L = y1 << 16;
+   long x2L = x2 << 16;
+   long y2L = y2 << 16;
+
+   long cx1L = im->cx1 << 16;
+   long cy1L = im->cy1 << 16;
+   long cx2L = im->cx2 << 16;
+   long cy2L = im->cy2 << 16;
+
+   long one_L = 1L << 16;
+
+  if (clip_1d_long (&x1L, &y1L, &x2L, &y2L, cx1L-one_L, cx2L+one_L) == 0)
+     return;
+  if (clip_1d_long (&y1L, &x1L, &y2L, &x2L, cy1L-one_L, cy2L+one_L) == 0) 
+     return;
+
+	dxL = x2L - x1L;
+	dyL = y2L - y1L;
+
+	if (dxL == 0 && dyL == 0) {
 		/* TBB: allow setting points */
-		gdImageSetAAPixelColor(im, x1, y1, col, 0xFF);
+      /* Watch out for -1's etc - possible with above expanded
+       * clipping bounds */
+      if (x1L >= 0 && y1L >= 0 && x1L < cx1L && y1L < cy1L)
+         gdImageSetAAPixelColor(im, x1L >> 16, y1L >> 16, col, 0xFF);
 		return;
 	}
-	if (abs(dx) > abs(dy)) {
-		if (dx < 0) {
-			tmp = x1;
-			x1 = x2;
-			x2 = tmp;
-			tmp = y1;
-			y1 = y2;
-			y2 = tmp;
-			dx = x2 - x1;
-			dy = y2 - y1;
-		}
-		x = x1 << 16;
-		y = y1 << 16;
-		inc = (dy * 65536) / dx;
+	if (abs(dxL) > abs(dyL)) {
+		if (dxL < 0) {
+			tmp = x1L;
+			x1L = x2L;
+			x2L = tmp;
+			tmp = y1L;
+			y1L = y2L;
+			y2L = tmp;
+			dxL = x2L - x1L;
+			dyL = y2L - y1L;
+		}
+		x = x1L;
+		y = y1L;
+		inc = dyL / (dxL >> 16);
+
 		/* TBB: set the last pixel for consistency (<=) */
-		while ((x >> 16) <= x2) {
-			gdImageSetAAPixelColor(im, x >> 16, y >> 16, col, (y >> 8) & 0xFF);
-			gdImageSetAAPixelColor(im, x >> 16, (y >> 16) + 1,col, (~y >> 8) & 0xFF);
-			x += (1 << 16);
+      /* Constrain the loop to the image bounds */
+      if (x < 0)
+      {
+         y -= ((inc * x) >> 16);
+         x = 0;
+      }
+      x2L = min_long(x2L,cx2L);
+		while (x <= x2L) {
+         int px = x >> 16;
+         int py = y >> 16;
+         if (py >= im->cy1 && py <= im->cy2)
+            gdImageSetAAPixelColor(im, px, py, col, (y >> 8) & 0xFF);
+         if (py + 1 <= im->cy2)
+            gdImageSetAAPixelColor(im, px, py + 1,col, (~y >> 8) & 0xFF);
+			x += one_L;
 			y += inc;
 		}
 	} else {
-		if (dy < 0) {
-			tmp = x1;
-			x1 = x2;
-			x2 = tmp;
-			tmp = y1;
-			y1 = y2;
-			y2 = tmp;
-			dx = x2 - x1;
-			dy = y2 - y1;
-		}
-		x = x1 << 16;
-		y = y1 << 16;
-		inc = (dx * 65536) / dy;
+		if (dyL < 0) {
+			tmp = x1L;
+			x1L = x2L;
+			x2L = tmp;
+			tmp = y1L;
+			y1L = y2L;
+			y2L = tmp;
+			dxL = x2L - x1L;
+			dyL = y2L - y1L;
+		}
+		x = x1L;
+      y = y1L;
+		inc = dxL / (dyL >> 16);
+
 		/* TBB: set the last pixel for consistency (<=) */
-		while ((y>>16) <= y2) {
-			gdImageSetAAPixelColor(im, x >> 16, y >> 16, col, (x >> 8) & 0xFF);
-			gdImageSetAAPixelColor(im, (x >> 16) + 1, (y >> 16),col, (~x >> 8) & 0xFF);
+      /* Constrain the loop to the image bounds */
+      if (y < 0)
+      {
+         x -= ((inc * y) >> 16);
+         y = 0;
+      }
+      y2L = min_long(y2L,cy2L);
+		while (y <= y2L) {
+         int px = x >> 16;
+         int py = y >> 16;
+         if (px >= im->cx1 && px <= im->cx2)
+            gdImageSetAAPixelColor(im, px, py, col, (x >> 8) & 0xFF);
+         if (px + 1 <= im->cx2)
+            gdImageSetAAPixelColor(im, px + 1, py, col, (~x >> 8) & 0xFF);
 			x += inc;
-			y += (1<<16);
+			y += one_L;
 		}
 	}
 }
#include <gd.h>
#include <stdio.h>

void gen_image(const char* filename, int idx, int reverse_x, int width, int height)
{
   double gradient = height / (width*2.0);
   int offset = idx*width;

   gdImagePtr im = gdImageCreateTrueColor(width,height);
   gdImageFilledRectangle(im,0,0,width-1,height-1, gdTrueColorAlpha(255, 255, 255, 0));

   gdImageSetAntiAliased(im, gdTrueColorAlpha(0,0,0,0));

   // test for potential segfault (introduced with AA improvements, fixed
   // with the same patch - but I didn't notice it until later).
   gdImageLine(im,-1,-1,-1,-1,gdAntiAliased);

   // draw an AA line
   gdImageLine(im,
          reverse_x * -width,   (offset-width) * gradient,
          reverse_x *  width*2, (offset+width*2) * gradient,
          gdAntiAliased);

   FILE * pngout = fopen(filename,"wb");
   gdImagePng(im,pngout);
   fclose(pngout);

   gdImageDestroy(im);
}

int main()
{
   gen_image("test_aa_a_0.png",0,1,10,100);
   gen_image("test_aa_a_1.png",1,1,10,100);
   gen_image("test_aa_b_0.png",2,-1,10,100);
   gen_image("test_aa_b_1.png",1,-1,10,100);

   gen_image("test_aa_c_0.png",0,1,100,10);
   gen_image("test_aa_c_1.png",1,1,100,10);
   gen_image("test_aa_d_0.png",2,-1,100,10);
   gen_image("test_aa_d_1.png",1,-1,100,10);
   return 0;
}
all: test_aa_native test_aa

test_aa_native: test_aa.c test_aa.mk
        gcc -Wall -g -o test_aa_native test_aa.c -lgd -lm -lpng

test_aa: test_aa.c test_aa.mk libgd.a gd_security.c
        gcc -Wall -g -o test_aa gd_security.c test_aa.c libgd.a -lm -lpng

clean:
        rm test_aa

Attachment: test_aa_native.sh
Description: Bourne shell script

Reply via email to