[Fixed] [script]Pitch broken for REAL now.

Bugs that have been investigated and resolved somehow.

Moderator: GZDoom Developers

[script]Pitch broken for REAL now.

Postby Major Cooke » Sun Feb 07, 2016 9:38 pm

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.
User avatar
Major Cooke
The road to Hell is paved in the carrion she leaves behind.
 
Joined: 28 Jan 2007

Re: [script]Pitch broken for REAL now.

Postby Major Cooke » Sun Feb 07, 2016 9:51 pm

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.
User avatar
Major Cooke
The road to Hell is paved in the carrion she leaves behind.
 
Joined: 28 Jan 2007

Re: [script]Pitch broken for REAL now.

Postby Blzut3 » Sun Feb 07, 2016 11:30 pm

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.
Blzut3
Pronounced: B-l-zut
 
 
 
Joined: 24 Nov 2004

Re: [script]Pitch broken for REAL now.

Postby randi » Sun Feb 07, 2016 11:35 pm

What about double->int64 like it shows in your link?
User avatar
randi
Site Admin
 
Joined: 09 Jul 2003

Re: [script]Pitch broken for REAL now.

Postby Blzut3 » Sun Feb 07, 2016 11:47 pm

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.
Blzut3
Pronounced: B-l-zut
 
 
 
Joined: 24 Nov 2004

Re: [script]Pitch broken for REAL now.

Postby edward850 » Sun Feb 07, 2016 11:51 pm

This basically means the upper range of a valid conversion from Double to int64 is about... 0x7FFFFFFFFFFFF000?
User avatar
edward850
[netcode intensifies]
 
Joined: 19 Jul 2005
Location: New Zealand

Re: [script]Pitch broken for REAL now.

Postby Graf Zahl » Mon Feb 08, 2016 3:38 am

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.
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: [script]Pitch broken for REAL now.

Postby Graf Zahl » Mon Feb 08, 2016 3:44 am

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.
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: [script]Pitch broken for REAL now.

Postby Graf Zahl » Mon Feb 08, 2016 3:59 am

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

Code: Select allExpand view
         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'...
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: [script]Pitch broken for REAL now.

Postby Graf Zahl » Mon Feb 08, 2016 4:53 am

More info on this mess.

I added these:

Code: Select allExpand view
#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 allExpand view
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 allExpand view
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 allExpand view
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...
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: [script]Pitch broken for REAL now.

Postby Edward-san » Mon Feb 08, 2016 5:37 am

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

Code: Select allExpand view
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 allExpand view
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
Edward-san
Mathematics is the language with which God has written the universe. (Galilei)
 
Joined: 17 Oct 2009

Re: [script]Pitch broken for REAL now.

Postby Graf Zahl » Mon Feb 08, 2016 5:57 am

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.
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: [script]Pitch broken for REAL now.

Postby Graf Zahl » Mon Feb 08, 2016 6:14 am

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.
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: [script]Pitch broken for REAL now.

Postby Edward-san » Mon Feb 08, 2016 6:17 am

Do we need the angleconvtest CCMD at all? :mrgreen:
Edward-san
Mathematics is the language with which God has written the universe. (Galilei)
 
Joined: 17 Oct 2009

Re: [script]Pitch broken for REAL now.

Postby edward850 » Mon Feb 08, 2016 6:20 am

Wouldn't hurt to keep it around for platform testing.
User avatar
edward850
[netcode intensifies]
 
Joined: 19 Jul 2005
Location: New Zealand

Next

Return to Closed Bugs

Who is online

Users browsing this forum: No registered users and 1 guest