FP precision crushing the player on buttons

Discuss programming topics for the various GPL'd game engine sources.
Post Reply
ericw
Posts: 92
Joined: Sat Jan 18, 2014 2:11 am

FP precision crushing the player on buttons

Post by ericw »

We had a report that, on 64-bit builds of Quakespasm, players were getting crushed when pressing angled buttons in some custom maps/mods: *
- Travail's qte1m2.bsp: noclip to 2788 732 -584, and press the button. The player will get crushed and killed on affected engines.
- sm179_otp.bsp: pressing the left button causes the player to get stuck.

The above happens on 64-bit QS builds (Windows x64, macOS builds) but also Fitzquake 0.85 compiled with VS2017. It'll probably happen on anything but gcc/msvc building for x86 without SSE.

I've posted about 80-bit x87 vs C standard compliant FP precision behaviour differences before (e.g. lightmaps:
http://forums.insideqc.com/viewtopic.ph ... uct#p53977
) so I knew what to expect, it was going to be some FP sensitive code (probably a dot product) that worked fine with 80-bit temporaries but "broke" with 32-bit rounded temporaries.

The culprit - whether the button worked normally or crushed the player - turned out to be this bit of code in SUB_CalcMove (subs.qc):

Code: Select all

// calculate length of vector
        len = vlen (vdestdelta);

// divide by speed to get time to reach dest
        traveltime = len / tspeed;

        if (traveltime < 0.1)
        {
traveltime would be very near 0.1, and the "if" branch would either be taken or not causing the crushing or not.

The source of the FP precision difference based on the compiler settings was PF_vlen:

Code: Select all

void PF_vlen (void)
{
	float	*value1;
	float	new;
	
	value1 = G_VECTOR(OFS_PARM0);

	new = value1[0] * value1[0] + value1[1] * value1[1] + value1[2]*value1[2];
	new = sqrt(new);
	
	G_FLOAT(OFS_RETURN) = new;
}
The fix was:

Code: Select all

-	float	new_temp;
+	double	new_temp;
 
 	value1 = G_VECTOR(OFS_PARM0);
 
-	new_temp = value1[0] * value1[0] + value1[1] * value1[1] + value1[2]*value1[2];
+	new_temp = (double)value1[0] * value1[0] + (double)value1[1] * value1[1] + (double)value1[2]*value1[2];
After tracking down and "fixing" enough instances of the same bug in response to reports from users, I'm thinking all math temporaries where the precision could matter should just have casts added preemptively.

[*] QS issue / commit links:
https://sourceforge.net/p/quakespasm/code/1554/
https://sourceforge.net/p/quakespasm/bugs/26/
Baker
Posts: 3666
Joined: Tue Mar 14, 2006 5:15 am

Re: FP precision crushing the player on buttons

Post by Baker »

That's a great bug fix.
The night is young. How else can I annoy the world before sunsrise? 8) Inquisitive minds want to know ! And if they don't -- well like that ever has stopped me before ..
Post Reply