search

Prof-Butts / xwa_ddraw_d3d11

Direct3D 11 implementation of DDraw.dll for XWA with VR support and New Shaders
MIT License
6 stars 4 forks source link

Unnecessary rotation matrix re-calculation #44

Closed morallo closed 3 years ago

morallo commented 3 years ago

hi @Prof-Butts, while trying to fully understand how the 3D transformations work, I realized that in the SteamVR path the rotation matrix is calculated again from yaw,pitch,roll, instead of using the value provided by GetSteamVRPositionalData()

See here that rotMatrix is never used after inverting it. Instead, the calculated rotMatrixFull is passed to the shader as g_VSMatrixCB.fullViewMat.fullViewMat.

https://github.com/Prof-Butts/xwa_ddraw_d3d11/blob/c57c9053ab82e6dd7055931489b6ff8c0d349d61/impl11/ddraw/PrimarySurface.cpp#L7416

So, this has no real effect on performance since it is only done once per frame, but I wanted to make sure I'm not missing anything.

My only remaining question is: why rotMatrix is inverted, but not rotMatrixFull? And why would you invert it in the first place? I guess this is left over from a previous version of the code?

Prof-Butts commented 3 years ago

There are a number of things going on here. Bottom line is: I think rotMatrix is no longer used and it's irrelevant whether we invert it or not (but I need to test it).

The reason we create a new matrix from yaw, pitch, roll is because XWA's coordinate system does not match SteamVR's coordinate system. For instance, the block that starts here:

https://github.com/Prof-Butts/xwa_ddraw_d3d11/blob/c57c9053ab82e6dd7055931489b6ff8c0d349d61/impl11/ddraw/PrimarySurface.cpp#L7419

shows how the yaw and pitch are inverted. Also, the block here:

https://github.com/Prof-Butts/xwa_ddraw_d3d11/blob/c57c9053ab82e6dd7055931489b6ff8c0d349d61/impl11/ddraw/PrimarySurface.cpp#L7413

adds an optional offset to the yaw, pitch. Even if we got rid of this offset we still need to invert the yaw and pitch, though.

The explanation for why this matrix was inverted starts here:

https://github.com/Prof-Butts/xwa_ddraw_d3d11/blob/c57c9053ab82e6dd7055931489b6ff8c0d349d61/impl11/ddraw/PrimarySurface.cpp#L7424

However, I think the matrix inversion is no longer necessary because the invidual yaw, pitch angles are inverted when building the matrix. This is probably dead code. I'm going to see if I can remove rotMatrix altogether. There's a similar matrix in the DirectSBS path -- we can probably remove that as well.

Prof-Butts commented 3 years ago

@morallo This was indeed dead code. I just removed it and things work fine. Good catch!