[script]Pitch broken for REAL now.

[Major Cooke]

http://zdoom.org/Changelog/e3bf1dd/files

Turns out this commit was actually doing the inverse of what it's supposed to do. Weapons above aiming flat out straight ahead on up will kick the pitch all the way to the top.

[Major Cooke]

https://github.com/rheit/zdoom/pull/549

Taking a look back, angle_t parameter turns out to have been the same the entire time even before the scripting merge.

[Blzut3]

So apparently x86_64 has quirky double->int behavior as well: https://social.msdn.microsoft.com/Forum ... lcppnative

Looks like code is needed to make sure that the value is in range [0-360) before casting.

[randi]

What about double->int64 like it shows in your link?

[Blzut3]

If you're not concerned about the performance of always doing that then go for it. As long as the input value is within range of the destination type then it should be a defined conversion.

[edward850]

This basically means the upper range of a valid conversion from Double to int64 is about... 0x7FFFFFFFFFFFF000?

[Graf Zahl]

Crap. I think this all should be refactored into a FLOAT2ANGLE macro that can be implemented differently for the respective platforms. If each one is different, it makes no sense to write out a one-size-fits-all conversion each time.

[Graf Zahl]

Ok, one question before all go mad here:

Is this only for compiler provided float to int conversions or are the xs_RoundToInt functions also affected? My guess would be no, because they store the float as an actual float and then read out parts of that with an integer instruction - without any hardware weirdness.

[Graf Zahl]

BTW, here's something interesting I just found. Look what the moving camera does with the pitch:

Code: Select all

			if (args[2] & 1)
			{ // linear
				if (args[2] & 4)
				{ // interpolate pitch
					pitch = FLOAT2FIXED(Lerp (FIXED2FLOAT(CurrNode->pitch), FIXED2FLOAT(CurrNode->Next->pitch)));
				}
			}
			else
			{ // spline
				if (args[2] & 4)
				{ // interpolate pitch
					pitch = FLOAT2FIXED(Splerp (FIXED2FLOAT(PrevNode->pitch), FIXED2FLOAT(CurrNode->pitch),
						FIXED2FLOAT(CurrNode->Next->pitch), FIXED2FLOAT(CurrNode->Next->Next->pitch)));
				}
			}

I'd guess this can be filed under 'worked by accident'...

[Graf Zahl]

More info on this mess.

I added these:

Code: Select all

#define ANGLE2DBL(f)		((f) * (180./ANGLE_180))
#define ANGLE2FLOAT(f)		(float((f) * (180./ANGLE_180)))
#define FLOAT2ANGLE(f)		((angle_t)xs_CRoundToInt((f) * (ANGLE_180/180.)))

#define ANGLE2RAD(f)		((f) * (M_PI/ANGLE_180))
#define ANGLE2RADF(f)		((f) * float(M_PI/ANGLE_180))
#define RAD2ANGLE(f)		((angle_t)xs_CRoundToInt((f) * (ANGLE_180/M_PI)))

and this testing CCMD

Code: Select all

CCMD(angleconvtest)
{
	Printf("Testing degrees to angle conversion:\n");
	for (double ang = -5 * 180.; ang < 5 * 180.; ang += 45.)
	{
		angle_t ang1 = FLOAT2ANGLE(ang);
		angle_t ang2 = (angle_t)(ang * (ANGLE_180 / 180.));
		angle_t ang3 = (angle_t)(int)(ang * (ANGLE_180 / 180.));
		Printf("Angle = %.5f: xs_RoundToInt = %08x, unsigned cast = %08x, signed cast = %08x\n",
			ang, ang1, ang2, ang3);
	}
}

And here's the output:

Code: Select all

Testing degrees to angle conversion:
Angle = -900.00000: xs_RoundToInt = 80000000, unsigned cast = 80000000, signed cast = 80000000
Angle = -855.00000: xs_RoundToInt = a0000000, unsigned cast = a0000000, signed cast = 80000000
Angle = -810.00000: xs_RoundToInt = c0000000, unsigned cast = c0000000, signed cast = 80000000
Angle = -765.00000: xs_RoundToInt = e0000000, unsigned cast = e0000000, signed cast = 80000000
Angle = -720.00000: xs_RoundToInt = 00000000, unsigned cast = 00000000, signed cast = 80000000
Angle = -675.00000: xs_RoundToInt = 20000000, unsigned cast = 20000000, signed cast = 80000000
Angle = -630.00000: xs_RoundToInt = 40000000, unsigned cast = 40000000, signed cast = 80000000
Angle = -585.00000: xs_RoundToInt = 60000000, unsigned cast = 60000000, signed cast = 80000000
Angle = -540.00000: xs_RoundToInt = 80000000, unsigned cast = 80000000, signed cast = 80000000
Angle = -495.00000: xs_RoundToInt = a0000000, unsigned cast = a0000000, signed cast = 80000000
Angle = -450.00000: xs_RoundToInt = c0000000, unsigned cast = c0000000, signed cast = 80000000
Angle = -405.00000: xs_RoundToInt = e0000000, unsigned cast = e0000000, signed cast = 80000000
Angle = -360.00000: xs_RoundToInt = 00000000, unsigned cast = 00000000, signed cast = 80000000
Angle = -315.00000: xs_RoundToInt = 20000000, unsigned cast = 20000000, signed cast = 80000000
Angle = -270.00000: xs_RoundToInt = 40000000, unsigned cast = 40000000, signed cast = 80000000
Angle = -225.00000: xs_RoundToInt = 60000000, unsigned cast = 60000000, signed cast = 80000000
Angle = -180.00000: xs_RoundToInt = 80000000, unsigned cast = 80000000, signed cast = 80000000
Angle = -135.00000: xs_RoundToInt = a0000000, unsigned cast = a0000000, signed cast = a0000000
Angle = -90.00000: xs_RoundToInt = c0000000, unsigned cast = c0000000, signed cast = c0000000
Angle = -45.00000: xs_RoundToInt = e0000000, unsigned cast = e0000000, signed cast = e0000000
Angle = 0.00000: xs_RoundToInt = 00000000, unsigned cast = 00000000, signed cast = 00000000
Angle = 45.00000: xs_RoundToInt = 20000000, unsigned cast = 20000000, signed cast = 20000000
Angle = 90.00000: xs_RoundToInt = 40000000, unsigned cast = 40000000, signed cast = 40000000
Angle = 135.00000: xs_RoundToInt = 60000000, unsigned cast = 60000000, signed cast = 60000000
Angle = 180.00000: xs_RoundToInt = 80000000, unsigned cast = 80000000, signed cast = 80000000
Angle = 225.00000: xs_RoundToInt = a0000000, unsigned cast = a0000000, signed cast = 80000000
Angle = 270.00000: xs_RoundToInt = c0000000, unsigned cast = c0000000, signed cast = 80000000
Angle = 315.00000: xs_RoundToInt = e0000000, unsigned cast = e0000000, signed cast = 80000000
Angle = 360.00000: xs_RoundToInt = 00000000, unsigned cast = 00000000, signed cast = 80000000
Angle = 405.00000: xs_RoundToInt = 20000000, unsigned cast = 20000000, signed cast = 80000000
Angle = 450.00000: xs_RoundToInt = 40000000, unsigned cast = 40000000, signed cast = 80000000
Angle = 495.00000: xs_RoundToInt = 60000000, unsigned cast = 60000000, signed cast = 80000000
Angle = 540.00000: xs_RoundToInt = 80000000, unsigned cast = 80000000, signed cast = 80000000
Angle = 585.00000: xs_RoundToInt = a0000000, unsigned cast = a0000000, signed cast = 80000000
Angle = 630.00000: xs_RoundToInt = c0000000, unsigned cast = c0000000, signed cast = 80000000
Angle = 675.00000: xs_RoundToInt = e0000000, unsigned cast = e0000000, signed cast = 80000000
Angle = 720.00000: xs_RoundToInt = 00000000, unsigned cast = 00000000, signed cast = 80000000
Angle = 765.00000: xs_RoundToInt = 20000000, unsigned cast = 20000000, signed cast = 80000000
Angle = 810.00000: xs_RoundToInt = 40000000, unsigned cast = 40000000, signed cast = 80000000
Angle = 855.00000: xs_RoundToInt = 60000000, unsigned cast = 60000000, signed cast = 80000000

That was on 32bit Windows with a debug compile. (Release is identical with Visual Studio 2013)

So, xs_CRoundToInt works correctly, so does the unsigned cast, but the signed int cast doesn't. Why?
Actually quite simple:
The unsigned cast uses an inlined intrinsic that performs x87 math specially tailored to that case.
The signed cast calls a function 'ftol_sse'. Which performs

Code: Select all

0159EC82  fstp        qword ptr [esp]  
0159EC85  cvttsd2si   eax,mmword ptr [esp]  

Yes, SSE math!
So, to summarize:

- we cannot use unsigned casts, because ARM does not like them.
- we cannot use signed casts because x86-SSE and x64 do not like them.
- xs_CRoundToInt completely circumvents the floating point math units and only depends on the bit ordering in a double. And that's identical everywhere.

I'm going to test this on ARM, too, but I have used xs_Float on iOS and Android before and it worked fine there, so this seems indeed to be the solution.
I think ZDBSP needs to have this addressed as well...

[Edward-san]

Just for information, I get the same output in linux x64 compiled with debug and gcc:

Code: Select all

Testing degrees to angle conversion:
Angle = -900.00000: xs_RoundToInt = 80000000, unsigned cast = 80000000, signed cast = 80000000
Angle = -855.00000: xs_RoundToInt = a0000000, unsigned cast = a0000000, signed cast = 80000000
Angle = -810.00000: xs_RoundToInt = c0000000, unsigned cast = c0000000, signed cast = 80000000
Angle = -765.00000: xs_RoundToInt = e0000000, unsigned cast = e0000000, signed cast = 80000000
Angle = -720.00000: xs_RoundToInt = 00000000, unsigned cast = 00000000, signed cast = 80000000
Angle = -675.00000: xs_RoundToInt = 20000000, unsigned cast = 20000000, signed cast = 80000000
Angle = -630.00000: xs_RoundToInt = 40000000, unsigned cast = 40000000, signed cast = 80000000
Angle = -585.00000: xs_RoundToInt = 60000000, unsigned cast = 60000000, signed cast = 80000000
Angle = -540.00000: xs_RoundToInt = 80000000, unsigned cast = 80000000, signed cast = 80000000
Angle = -495.00000: xs_RoundToInt = a0000000, unsigned cast = a0000000, signed cast = 80000000
Angle = -450.00000: xs_RoundToInt = c0000000, unsigned cast = c0000000, signed cast = 80000000
Angle = -405.00000: xs_RoundToInt = e0000000, unsigned cast = e0000000, signed cast = 80000000
Angle = -360.00000: xs_RoundToInt = 00000000, unsigned cast = 00000000, signed cast = 80000000
Angle = -315.00000: xs_RoundToInt = 20000000, unsigned cast = 20000000, signed cast = 80000000
Angle = -270.00000: xs_RoundToInt = 40000000, unsigned cast = 40000000, signed cast = 80000000
Angle = -225.00000: xs_RoundToInt = 60000000, unsigned cast = 60000000, signed cast = 80000000
Angle = -180.00000: xs_RoundToInt = 80000000, unsigned cast = 80000000, signed cast = 80000000
Angle = -135.00000: xs_RoundToInt = a0000000, unsigned cast = a0000000, signed cast = a0000000
Angle = -90.00000: xs_RoundToInt = c0000000, unsigned cast = c0000000, signed cast = c0000000
Angle = -45.00000: xs_RoundToInt = e0000000, unsigned cast = e0000000, signed cast = e0000000
Angle = 0.00000: xs_RoundToInt = 00000000, unsigned cast = 00000000, signed cast = 00000000
Angle = 45.00000: xs_RoundToInt = 20000000, unsigned cast = 20000000, signed cast = 20000000
Angle = 90.00000: xs_RoundToInt = 40000000, unsigned cast = 40000000, signed cast = 40000000
Angle = 135.00000: xs_RoundToInt = 60000000, unsigned cast = 60000000, signed cast = 60000000
Angle = 180.00000: xs_RoundToInt = 80000000, unsigned cast = 80000000, signed cast = 80000000
Angle = 225.00000: xs_RoundToInt = a0000000, unsigned cast = a0000000, signed cast = 80000000
Angle = 270.00000: xs_RoundToInt = c0000000, unsigned cast = c0000000, signed cast = 80000000
Angle = 315.00000: xs_RoundToInt = e0000000, unsigned cast = e0000000, signed cast = 80000000
Angle = 360.00000: xs_RoundToInt = 00000000, unsigned cast = 00000000, signed cast = 80000000
Angle = 405.00000: xs_RoundToInt = 20000000, unsigned cast = 20000000, signed cast = 80000000
Angle = 450.00000: xs_RoundToInt = 40000000, unsigned cast = 40000000, signed cast = 80000000
Angle = 495.00000: xs_RoundToInt = 60000000, unsigned cast = 60000000, signed cast = 80000000
Angle = 540.00000: xs_RoundToInt = 80000000, unsigned cast = 80000000, signed cast = 80000000
Angle = 585.00000: xs_RoundToInt = a0000000, unsigned cast = a0000000, signed cast = 80000000
Angle = 630.00000: xs_RoundToInt = c0000000, unsigned cast = c0000000, signed cast = 80000000
Angle = 675.00000: xs_RoundToInt = e0000000, unsigned cast = e0000000, signed cast = 80000000
Angle = 720.00000: xs_RoundToInt = 00000000, unsigned cast = 00000000, signed cast = 80000000
Angle = 765.00000: xs_RoundToInt = 20000000, unsigned cast = 20000000, signed cast = 80000000
Angle = 810.00000: xs_RoundToInt = 40000000, unsigned cast = 40000000, signed cast = 80000000
Angle = 855.00000: xs_RoundToInt = 60000000, unsigned cast = 60000000, signed cast = 80000000

[edit]and if I change the 'int' cast in 'ang3' assignment to 'int64_t', it works fine:

Code: Select all

Testing degrees to angle conversion:
Angle = -900.00000: xs_RoundToInt = 80000000, unsigned cast = 80000000, signed cast = 80000000
Angle = -855.00000: xs_RoundToInt = a0000000, unsigned cast = a0000000, signed cast = a0000000
Angle = -810.00000: xs_RoundToInt = c0000000, unsigned cast = c0000000, signed cast = c0000000
Angle = -765.00000: xs_RoundToInt = e0000000, unsigned cast = e0000000, signed cast = e0000000
Angle = -720.00000: xs_RoundToInt = 00000000, unsigned cast = 00000000, signed cast = 00000000
Angle = -675.00000: xs_RoundToInt = 20000000, unsigned cast = 20000000, signed cast = 20000000
Angle = -630.00000: xs_RoundToInt = 40000000, unsigned cast = 40000000, signed cast = 40000000
Angle = -585.00000: xs_RoundToInt = 60000000, unsigned cast = 60000000, signed cast = 60000000
Angle = -540.00000: xs_RoundToInt = 80000000, unsigned cast = 80000000, signed cast = 80000000
Angle = -495.00000: xs_RoundToInt = a0000000, unsigned cast = a0000000, signed cast = a0000000
Angle = -450.00000: xs_RoundToInt = c0000000, unsigned cast = c0000000, signed cast = c0000000
Angle = -405.00000: xs_RoundToInt = e0000000, unsigned cast = e0000000, signed cast = e0000000
Angle = -360.00000: xs_RoundToInt = 00000000, unsigned cast = 00000000, signed cast = 00000000
Angle = -315.00000: xs_RoundToInt = 20000000, unsigned cast = 20000000, signed cast = 20000000
Angle = -270.00000: xs_RoundToInt = 40000000, unsigned cast = 40000000, signed cast = 40000000
Angle = -225.00000: xs_RoundToInt = 60000000, unsigned cast = 60000000, signed cast = 60000000
Angle = -180.00000: xs_RoundToInt = 80000000, unsigned cast = 80000000, signed cast = 80000000
Angle = -135.00000: xs_RoundToInt = a0000000, unsigned cast = a0000000, signed cast = a0000000
Angle = -90.00000: xs_RoundToInt = c0000000, unsigned cast = c0000000, signed cast = c0000000
Angle = -45.00000: xs_RoundToInt = e0000000, unsigned cast = e0000000, signed cast = e0000000
Angle = 0.00000: xs_RoundToInt = 00000000, unsigned cast = 00000000, signed cast = 00000000
Angle = 45.00000: xs_RoundToInt = 20000000, unsigned cast = 20000000, signed cast = 20000000
Angle = 90.00000: xs_RoundToInt = 40000000, unsigned cast = 40000000, signed cast = 40000000
Angle = 135.00000: xs_RoundToInt = 60000000, unsigned cast = 60000000, signed cast = 60000000
Angle = 180.00000: xs_RoundToInt = 80000000, unsigned cast = 80000000, signed cast = 80000000
Angle = 225.00000: xs_RoundToInt = a0000000, unsigned cast = a0000000, signed cast = a0000000
Angle = 270.00000: xs_RoundToInt = c0000000, unsigned cast = c0000000, signed cast = c0000000
Angle = 315.00000: xs_RoundToInt = e0000000, unsigned cast = e0000000, signed cast = e0000000
Angle = 360.00000: xs_RoundToInt = 00000000, unsigned cast = 00000000, signed cast = 00000000
Angle = 405.00000: xs_RoundToInt = 20000000, unsigned cast = 20000000, signed cast = 20000000
Angle = 450.00000: xs_RoundToInt = 40000000, unsigned cast = 40000000, signed cast = 40000000
Angle = 495.00000: xs_RoundToInt = 60000000, unsigned cast = 60000000, signed cast = 60000000
Angle = 540.00000: xs_RoundToInt = 80000000, unsigned cast = 80000000, signed cast = 80000000
Angle = 585.00000: xs_RoundToInt = a0000000, unsigned cast = a0000000, signed cast = a0000000
Angle = 630.00000: xs_RoundToInt = c0000000, unsigned cast = c0000000, signed cast = c0000000
Angle = 675.00000: xs_RoundToInt = e0000000, unsigned cast = e0000000, signed cast = e0000000
Angle = 720.00000: xs_RoundToInt = 00000000, unsigned cast = 00000000, signed cast = 00000000
Angle = 765.00000: xs_RoundToInt = 20000000, unsigned cast = 20000000, signed cast = 20000000
Angle = 810.00000: xs_RoundToInt = 40000000, unsigned cast = 40000000, signed cast = 40000000
Angle = 855.00000: xs_RoundToInt = 60000000, unsigned cast = 60000000, signed cast = 60000000

[Graf Zahl]

Yeah, not that this surprises me the least.
But since the xs_RoundToInt method is the only one that's hardware independent I think it's the safest bet here.
All it does is multiply the float with a magic constant, stores the value in a union that maps to two ints and then reads out the lower part. There's no dependency on some weird conversion instruction that gets implemented differently by different hardware.
It only depends on how doubles are stored in memory, and all common architectures are still using the same IEEE format. (And to calm the paranoid ones - it can be switched off and revert to the compiler's implementation, should something not work on some odd system.

[Graf Zahl]

I hope, that this is plugged for good now.
I had a look over the entire code and replaced all angle conversions I found with the abovelisted macros.

But seeing that nearly every place did it differently, there may still be hiding some deep in the bowels of the code. The ones in vmexec.h I only found by random chance because they didn't match any pattern I was looking for.

And it should be clear for the future: Use those macros! If they create a problem on some odd system, it's only one place that needs to be addressed, not fifty different variations on the same theme.

[Edward-san]

Do we need the angleconvtest CCMD at all?

[edward850]

Wouldn't hurt to keep it around for platform testing.