Re: [AD] rotate.c hang fix

[Eric Botcazou <ebotcazou@xxxxxxxxxx>]

> Compiled and running on Mac OS X 10.2.6, game atanks 0.9.8g running on top
> of Allegro 4.1.11 repeatedly stuck hanging with a missile at it's flight
> zenith.

I can't reproduce on x86 (compiled with GCC), but the x86 has a notoriously
weird FPU (i.e non IEEE-754 compliant) which may hide the bug.

> I traced the bug into Allegro's rotate.c, function _parallelogram_map, and
> although I didn't quite understand what the code did, I came up with the
> accompanied fix to situations when spr_dx equals zero, and thusly a while 
> loop never exits.

I think your patch is valid purely on considerations of symmetry:

	    if ((unsigned)(l_spr_x_rounded >> 16) >= (unsigned)spr->w) {
	       if ((l_spr_x_rounded < 0) == (spr_dx < 0)) {
		  /* This can happen. */
		  goto skip_draw;
	       }

spr->w is not zero so l_spr_x_rounded is not zero in the body of the 'if'. So 
the condition (l_spr_x_rounded < 0) == (spr_dx < 0) can be rewritten as

((l_spr_x_rounded < 0) && (spr_dx < 0))
  || ((l_spr_x_rounded > 0) && (spr_dx >= 0))

which is not invariant by a 180° rotation of the sprite as it ought to be.

So I think the condition should be rewritten as

((l_spr_x_rounded < 0) && (spr_dx <= 0))
  || ((l_spr_x_rounded > 0) && (spr_dx >= 0))

which is equivalent to your patch.


There are three other similar non-symmetrical conditions just below, so I 
think the attached patch is more complete.

Sven, what do think about this?

-- 
Eric Botcazou

--- /home/eric/cvs/allegro/src/rotate.c	Tue Oct  1 20:33:44 2002
+++ allegro/src/rotate.c	Mon Sep 29 13:41:28 2003
@@ -510,7 +510,8 @@ void _parallelogram_map(BITMAP *bmp, BIT
 	       though.
 	    */
 	    if ((unsigned)(l_spr_x_rounded >> 16) >= (unsigned)spr->w) {
-	       if ((l_spr_x_rounded < 0) == (spr_dx < 0)) {
+	       if (((l_spr_x_rounded < 0) && (spr_dx <= 0)) ||
+		   ((l_spr_x_rounded > 0) && (spr_dx >= 0))) {
 		  /* This can happen. */
 		  goto skip_draw;
 	       }
@@ -529,7 +530,8 @@ void _parallelogram_map(BITMAP *bmp, BIT
 			      ((r_bmp_x_rounded - l_bmp_x_rounded) >> 16) *
 			      spr_dx;
 	    if ((unsigned)(right_edge_test >> 16) >= (unsigned)spr->w) {
-	       if ((right_edge_test < 0) == (spr_dx < 0)) {
+	       if (((right_edge_test < 0) && (spr_dx <= 0)) ||
+		   ((right_edge_test > 0) && (spr_dx >= 0))) {
 		  /* This can happen. */
 		  while ((unsigned)(right_edge_test >> 16) >=
 			 (unsigned)spr->w) {
@@ -545,7 +547,8 @@ void _parallelogram_map(BITMAP *bmp, BIT
 	       }
 	    }
 	    if ((unsigned)(l_spr_y_rounded >> 16) >= (unsigned)spr->h) {
-	       if ((l_spr_y_rounded < 0) == (spr_dy < 0)) {
+	       if (((l_spr_y_rounded < 0) && (spr_dy <= 0)) ||
+		   ((l_spr_y_rounded > 0) && (spr_dy >= 0))) {
 		  /* This can happen. */
 		  goto skip_draw;
 	       }
@@ -564,7 +567,8 @@ void _parallelogram_map(BITMAP *bmp, BIT
 			      ((r_bmp_x_rounded - l_bmp_x_rounded) >> 16) *
 			      spr_dy;
 	    if ((unsigned)(right_edge_test >> 16) >= (unsigned)spr->h) {
-	       if ((right_edge_test < 0) == (spr_dy < 0)) {
+	       if (((right_edge_test < 0) && (spr_dy <= 0)) ||
+		   ((right_edge_test > 0) && (spr_dy >= 0))) {
 		  /* This can happen. */
 		  while ((unsigned)(right_edge_test >> 16) >=
 			 (unsigned)spr->h) {