From eb5fe5bce9be17f89bf2a394aee9f2b4ce3c72c8 Mon Sep 17 00:00:00 2001
From: Thomas Debesse <dev@illwieckz.net>
Date: Wed, 1 Jul 2020 18:20:25 +0200
Subject: [PATCH] q3map2/light_bounce: prevent infinite loop on obscure bias
 compute

New code produces same result without loop at all, so
it cannot fall in infinite loop, and it is faster in
use cases requiring more than one loop in previous code.

The Unvanquished vega map is known to trigger the bug:
https://github.com/UnvanquishedAssets/map-vega_src.dpkdir
I reproduced it multiple time on various hardware (8 core FX-9590,
12 core/24 thread Ryzen 9 3900X) with commit af40508 and using
final compilation profile edited to use -fastbounce instead
of -fast option.

The symptom is simple, q3map2 stucks there:

--- Radiosity (bounce 1 of 8) ---
--- RadCreateDiffuseLights ---
0...1...2...3..

Or somewhere else in that progression bar given your hardware
and the amount of core your CPU has.

When stuck, all the CPU cores are running 100% but the thread
never returns (a strace can reveals it, a gdb backtrace too).

Thanks to @slipher for the precious advices and improving my first
attempt to fix it.

For more information on the issue, I asked:

> which negative value never can become positive
> when incremented infinitely?

slipher said:

> for a double, any value less than -2^53 would have this property
> don't know for float off the top of my head

But then, it means that's theorically verified this loop was able
to run forever in some case.

I don't know what this code is doing anyway, but at least we can
keep the behaviour without requiring to understand it.
---
 tools/quake3/q3map2/light_bounce.c | 31 ++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/tools/quake3/q3map2/light_bounce.c b/tools/quake3/q3map2/light_bounce.c
index 7b428e11..97da22f5 100644
--- a/tools/quake3/q3map2/light_bounce.c
+++ b/tools/quake3/q3map2/light_bounce.c
@@ -195,6 +195,23 @@ static void RadClipWindingEpsilon( radWinding_t *in, vec3_t normal, vec_t dist,
 
 
 
+/*
+   Modulo1IfNegative()
+   Previously the bias computation was doing:
+
+      while ( f < 0.0f ) {
+         f += 1.0f;
+      }
+
+   That may end in infinite loop in some case.
+   It may also be slower because of useless loops.
+   I don't know what that computation is for.
+   -- illwieckz
+*/
+float Modulo1IfNegative( float f ){
+	return f < 0.0f ? f - floor( f ) : f;
+}
+
 
 
 /*
@@ -204,10 +221,8 @@ static void RadClipWindingEpsilon( radWinding_t *in, vec3_t normal, vec_t dist,
  */
 
 qboolean RadSampleImage( byte *pixels, int width, int height, float st[ 2 ], float color[ 4 ] ){
-	float sto[ 2 ];
 	int x, y;
 
-
 	/* clear color first */
 	color[ 0 ] = color[ 1 ] = color[ 2 ] = color[ 3 ] = 255;
 
@@ -216,18 +231,10 @@ qboolean RadSampleImage( byte *pixels, int width, int height, float st[ 2 ], flo
 		return qfalse;
 	}
 
-	/* bias st */
-	sto[ 0 ] = st[ 0 ];
-	while ( sto[ 0 ] < 0.0f )
-		sto[ 0 ] += 1.0f;
-	sto[ 1 ] = st[ 1 ];
-	while ( sto[ 1 ] < 0.0f )
-		sto[ 1 ] += 1.0f;
-
 	/* get offsets */
-	x = ( (float) width * sto[ 0 ] ) + 0.5f;
+	x = ( (float) width * Modulo1IfNegative( st[ 0 ] ) ) + 0.5f;
 	x %= width;
-	y = ( (float) height * sto[ 1 ] )  + 0.5f;
+	y = ( (float) height * Modulo1IfNegative( st[ 1 ] ) ) + 0.5f;
 	y %= height;
 
 	/* get pixel */