ValveSoftware / unity-xr-plugin

OpenVR plugin for Unity's XR API
BSD 3-Clause "New" or "Revised" License
309 stars 64 forks source link

GetGraphicsAdapterId never actually returns the ID, possibly working around Unity bug? #49

Closed rpavlik closed 3 years ago

rpavlik commented 4 years ago

https://github.com/ValveSoftware/unity-xr-plugin/blob/39ad95e54633c553740b972911f35f7aefbdaf0e/Providers/OpenVRSystem.cpp#L114

*adapterId is never being set - the local uint64_t luid is being set to whatever is in adapterId on call (which may be undefined behavior, the API doesn't explicitly state that it's been initialized, I don't think), then luid is being reassigned by steamvr. However, the contents of luid never make it back to the caller.

On one hand, this means multi-gpu systems probably don't work, and this code could be replaced with return false;. On the other hand, adding the missing *adapterId = luid; line actually causes Unity to crash reliably in the caller of this interface function, so there's probably a Unity bug: call stack follows. (SelectAdapterByLuidIfAvailableOrByIndex is the function that calls the XRPreInit GetGraphicsAdapterId function in the first place, and it crashes after that call returns.)

UnityPlayer.dll!SelectAdapterByLuidIfAvailableOrByIndex() (Unknown Source:0)
UnityPlayer.dll!InitializeD3D11(bool) (Unknown Source:0)
UnityPlayer.dll!CreateD3D11GfxDevice(void) (Unknown Source:0)
UnityPlayer.dll!CreateRealGfxDevice(enum GfxDeviceRenderer) (Unknown Source:0)
UnityPlayer.dll!CreateClientGfxDevice(enum GfxDeviceRenderer,enum GfxCreateDeviceFlags) (Unknown Source:0)
UnityPlayer.dll!CreateGfxDevice(enum GfxDeviceRenderer,enum GfxCreateDeviceFlags) (Unknown Source:0)
UnityPlayer.dll!InitializeGfxDevice(void) (Unknown Source:0)
UnityPlayer.dll!InitializeEngineGraphics(bool) (Unknown Source:0)
UnityPlayer.dll!PlayerInitEngineGraphics(bool) (Unknown Source:0)
UnityPlayer.dll!UnityMainImpl(struct HINSTANCE__ *,struct HINSTANCE__ *,wchar_t *,int) (Unknown Source:0)
UnityPlayer.dll!UnityMain() (Unknown Source:0)
1runeberg commented 4 years ago

Hi @rpavlik - thanks for reporting! does seem like this was short-circuited from the engine side, not sure if that's a regression on Unity's end, will clarify as this is one of the test parameters we had prior to public release a couple of months ago.

rpavlik commented 4 years ago

I have tested this now on 2019.3.15 as well as 2020.1.6 and reproduced in both places.

There is that symbol in the exe being generated that should encourage hybrid laptops to pick the right (discrete) GPU, so perhaps that's how it was missed. A desktop with two GPUs is probably the easiest way to test this reliably

zite commented 4 years ago

Looks like we need to pass a pointer to a variable that's going to stay around a while. Fixed in this commit: https://github.com/ValveSoftware/unity-xr-plugin/commit/c415712deabd6474d27302e1f63588c5ccf6b2b0

Should come in a new version soon.

rpavlik commented 4 years ago

ah, go figure. Weird API on Unity's part, thanks for digging in to it.

zite commented 3 years ago

This finally made it into a release. You're welcome to give it a try and let us know if it solves the problem. https://github.com/ValveSoftware/unity-xr-plugin/releases/tag/v1.1.1b

kisak-valve commented 3 years ago

Closing per the last comment.