steam_helper: get OpenXR extensions before initializing OpenVR #7906
Conversation
When using OpenComposite, both OpenVR and OpenXR functions will call the same underlying OpenXR loader. Because the OpenXR loader only supports a single active instance, `initialize_vr_data` currently fails because an OpenXR instance has already been initialized when XR extensions are queried. For SteamVR, the runtime already needs to be running for `xrCreateInstance` to succeed. To support both setups, this commit tries to query XR extensions before OpenVR initialization, but still defaults to the previous behavior if the first try was unsuccessful. Fix for: ValveSoftware#7905
|
Thanks for the patch!
Did you check that? It would be very weird, I am not aware there ever was such a bug. As I understood the description here, the problem with OpenComposite is only when VR and XR instances are open at the same time and the order of opening doesn't matter. Is it correct? If it is, AFAIU the only problem is that VR instance is closed in the end of the function, does just closing it after VR queries are done before dealing with XR help? If yes, we may probably try to go this way after some smoke test with SteamVR. If it doesn't for some reason, would you please elaborate more on the actual issue with OpenComposite. Broadly, I think it should be fixed in OpenComposite. There are games which may be doing whatever with initialization. If workint that around that is a matter of a small and sensible change (like release VR instance earlier) than its fine. While we probably don't want to introduce complications like in the current form of the patch for this purpose. |
That's what I observed on my machine with my original proof of concept (didn't keep any logs though). But you are right, that would be weird, I haven't really thought about it thoroughly, just assumed from this comment that (Linux) SteamVR was not intended to be started by the OpenXR loader. Unfortunately, (my) SteamVR feels very unstable in general for a variety of reasons, and I am not sure how much of that is my / NixOS' fault, Linux compatibility in general, Proton, or actual bugs within SteamVR. I don't have a Windows machine to test what SteamVR is supposed to do, but I could install Ubuntu for testing, it seems to be the most supported distro most of the time.
Yes, AFAIK that is correct. I changed the order because I was saw that the XR part could be moved and I didn't have to change anything related to OpenVR that way..
Agreed. AFAIK the VR/XR double-initialization is not a common problem outside of Proton, but i see that every game could try that and OpenComposite should generally expect it.
Also agreed. It's a niche use case and I'm not confident in my ability to test the main use case, so: the less changes the better :)
Thank you for your input. I may be able to write a new patch next week. |
It says so right here (at least if I understand the comment correctly) Line 322 in 8640a5c
AFAICT this is correct, sadly never was able to test it myself due to unable to get proton compiled on my system (complained about fonts or something related to that IIRC)
If this could be easily fixed on OpenComposite side I agree since then it would also be fixed for all proton versions, though I don't know of an easy fix since OpenComposite is since it is just a library that gets loaded into wine process space and then calls OpenXR, if somehow OpenComposite could detect that it was no longer used it could release the OpenXR instance it could be made to work. (that said I just know enough about how this all works to shoot me in the foot so I might be missing something here) Another option might to ask the folks at Krhonos group to reintroduce the ability to make multiple xrCreateInstance calls from the same process, though I think they removed that for good reasons. |
|
I wouldn't be opposed to a patch which would release VR instance earlier before XR initialization, but someone has to check that it actually helps anything first (we don't test against OpenComposite in Proton development now), ideally tested with SteamVR at once to check that nothing is immediately broken. I am afraid it is not possible to make a reasonably verified patch without even compiling Proton. |
Agreed it is the reason I didn't provide a patch myself since I was never able to test it, and though it would be awesome if proton could be more regularly tested against alternative implementations I completely understand that has no priority. I'm pretty sure @emily-is-my-username is able to compile and test (if there is a compiled version I can test that, both vs steamvr and opencomposite/monado, if time permits I might see about digging in why it didn't compile here) |
|
A side note, once there are no concerns for the patch itself it should have real author name, surname and email on it to be pulled as is. If the author wants to stay incognito the patch might still be used but I'll have to commit it under my name referencing the auhtor's nickname and this PR in the commit message. UPDATE By real name I don't mean exactly legal name or something formal, just what the author considers their real name, similar to, e. g., what is assumed Wine upstream. |
|
closing this on in favor of #8126 |
When using OpenComposite, both OpenVR and OpenXR functions will call the same underlying OpenXR loader.
Because the OpenXR loader only supports a single active instance,
initialize_vr_datacurrently fails because an OpenXR instance has already been initialized when XR extensions are queried.For SteamVR, the runtime already needs to be running for
xrCreateInstanceto succeed.To support both setups, this commit tries to query XR extensions before OpenVR initialization, but still defaults to the previous behavior if the first try was unsuccessful.
Fix for:
#7905