Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for OpenXR #1129

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

Add support for OpenXR #1129

wants to merge 25 commits into from

Conversation

lubosz
Copy link
Collaborator

@lubosz lubosz commented Sep 4, 2024

This series of patches provides an initial implementation of OpenXrDisplayPlugin and OpenXrInputPlugin to provide VR support using OpenXR runtimes.

In addition, it extends Overte's eyeOffsets to account for orientation, which is required for HMDs with screens that are not parallel like the Valve Index. There are also some minor improvements to the OpenGLDisplayPlugin and Application and adds the openxr-loader to the vcpkg build.

Tested using the Valve Index with Monado and SteamVR on Linux.

Now building on MSVC with C++20!

Potential follow up work

  • Add mappings for more controller profiles, currently only the Khronos Simple and Valve Index profiles are implemented.
  • Instead of rendering a side-by-side buffer in Overte's single-pass-stereo render path, rendering should happen into a texture array, to avoid 2 GPU copies that are currently happening every frame. This will require some renderer restructuring and might require extension use when done in OpenGL, which availabilty may vary across platforms.
  • Make Overte's input system be more action aware, instead of mapping to button events. Currently VR controllers are mostly mapped to SDL style button events (based on the XBox controller, or should I say Dreamcast controller layout) and the raw button input of that is processed in JavaScript without clear action definitions. There are indeed already actions in overte (like Actions.TranslateX), but the consistency in their usage needs to be improved.
  • Implement isHmdMounted, which has no explicit API, could differ between runtimes and might need to be implemented using other system APIs (e.g. on Android)
  • Implement getPlayAreaRect, which will probably require the use of OpenXR extensions.
  • Port to Vulkan

The build demands it but it's missing.
Copy from upstream vcpkg repo.

Fixes the following error:
```
Computing installation plan...
error: while looking for jsoncpp:x64-linux:
overte-files/vcpkg/cfe0a2a0/ports/jsoncpp: error: jsoncpp does not exist
```
@Armored-Dragon Armored-Dragon added needs CR This pull request needs to be code reviewed needs QA This pull request needs to be tested labels Sep 4, 2024
@JulianGro JulianGro self-requested a review as a code owner September 5, 2024 12:10
@JulianGro

This comment was marked as resolved.

@lubosz

This comment was marked as resolved.

@JulianGro

This comment was marked as resolved.

@lubosz

This comment was marked as resolved.

@HifiExperiments
Copy link
Member

super cool! thank you for doing all of this awesome work! I have to take a more complete look, but just a few comments/thoughts on follow up work (not blocking):

@JulianGro
Copy link
Member

I wouldn't get too caught up on the play area. This is probably from the times when VR runtimes didn't include this feature already. While it would be nice to have, since it tries to show the play area when teleporting, I don't think we should get caught up on it. I don't think anyone uses teleport movement on here, and the VR runtime most likely already does enough to show the bounds of you play space.

@HifiExperiments
Copy link
Member

I do think teleporting is an important feature! but I also spent a bunch of time working on the parabola rendering and intersection logic so maybe I'm biased 😄

maybe we could use the play area for more stuff in the future though. but yeah it was more relevant back when everything used outside-in tracking and it also showed you where the base stations were so you could make sure you were facing them. definitely not a blocker, but seems like it would be cool to have eventually since it seems easy to support.

@ksuprynowicz
Copy link
Member

ksuprynowicz commented Sep 7, 2024

It looks like OpenXR doesn't work on Windows currently. I can get it to run only in OpenVR mode and in OpenVR mode shadows are different for each eye on High graphics setting:
shadows_1

@lubosz
Copy link
Collaborator Author

lubosz commented Sep 7, 2024

It looks like OpenXR doesn't work on Windows currently. I can get it to run only in OpenVR mode and in OpenVR mode shadows are different for each eye on High graphics setting:

Do you have any errors in the log output? Did you set the XR_RUNTIME_JSON?

@ksuprynowicz
Copy link
Member

I think I know what happened:

[09/07 13:46:54] [CRITICAL] [openxr.context] Runtime does not support OpenGL!
[09/07 13:46:54] [WARNING] [openxr.context] OpenXR is not supported.

I think that WMR might have set itself as default OpenXR runtime, so I will make sure that SteamVR is the one instead.

@ksuprynowicz

This comment was marked as resolved.

@lubosz
Copy link
Collaborator Author

lubosz commented Sep 7, 2024

I would move any extension implementations to follow up PRs, also since the extension availability is runtime specific.

* for `isHmdMounted`, I wonder if the headset fit events could be used?
  there's a XR_HEADSET_FIT_STATUS_NOT_WORN_ML (https://registry.khronos.org/OpenXR/specs/1.1/html/xrspec.html#_headset_fit_events).  or could we use the XRSessionState?  and then there's https://registry.khronos.org/OpenXR/specs/1.1/html/xrspec.html#XR_EXT_user_presence

XR_ML_user_calibration and XR_EXT_user_presence.

* `getPlayAreaRect`: https://registry.khronos.org/OpenXR/specs/1.1/html/xrspec.html#xrGetReferenceSpaceBoundsRect, maybe some extensions (facebook specific?) for getting non-rectangular bounds.  also maybe something for `getSensorPositions`?

xrGetReferenceSpaceBoundsRect is core, so this would be a first attempt candidate for getPlayAreaRect. I would not consider this essential for initial OpenXR support, so it would fit into a follow up PR.

getSensorPositions is only implemented in the OculusBaseDisplayPlugin, not in the OpenVR plugin.
It calls ovr_GetTrackerPose.
This is very specific to external tracking PCVR setups and not relevant for stand alone VR. This is also usually implemented in the runtime, e.g. when you press the menu button in SteamVR you can see the base stations and not exposed to the application. I also don't think that there is an OpenXR API that exposes that.

* the visibility masks for `getStencilMaskMeshOperator` should be pretty simple to add (see how the other display plugins handle creating the meshes/draw calls): https://registry.khronos.org/OpenXR/specs/1.1/html/xrspec.html#XR_KHR_visibility_mask

Right, this can be probably easily implemented using the XR_KHR_visibility_mask extension.

* can/should we do anything with getPreferredAudioInDevice, getPreferredAudioOutDevice, suppressKeyboard, unsuppressKeyboard, isKeyboardVisible?

I actually see this as runtime functionality as well, since it's only relevant for PC VR. Obviously overte can also implement system sound and input APIs, e.g. PulseAudio, PipeWire, libinput, X Input depending on the system setup. But I would consider this out of scope for the OpenXR plugin.

* `getCullingProjection`: see [models in the very far right corner of eyes sometimes get culled #666](https://github.com/overte-org/overte/issues/666), and in the linked vircadia issue I mentioned https://computergraphics.stackexchange.com/questions/1736/vr-and-frustum-culling but that only works when FOV < 180, so maybe we need to support multiple culling projection matrices (one for each eye?)

I'm actually doing the same as the OpenVR plugin does, but it seems that it was broken there. This would be something worth fixing in connection to the getStencilMaskMeshOperator issue.

@lubosz
Copy link
Collaborator Author

lubosz commented Sep 7, 2024

This is probably from the times when VR runtimes didn't include this feature already.

This was the case for serveral things in the other plugins which were from a PCVR centric time where the runtimes didn't do as much as they do today. E.g. mirror window, displaying external sensors, selecting the audio device etc.

@lubosz
Copy link
Collaborator Author

lubosz commented Sep 7, 2024

I think I know what happened:

[09/07 13:46:54] [CRITICAL] [openxr.context] Runtime does not support OpenGL!
[09/07 13:46:54] [WARNING] [openxr.context] OpenXR is not supported.

I think that WMR might have set itself as default OpenXR runtime, so I will make sure that SteamVR is the one instead.

Right, this implementation is currently OpenGL only. Hopefully a Vulkan version will be available at some point in the future.

@lubosz

This comment was marked as resolved.

@JulianGro

This comment was marked as resolved.

@JulianGro

This comment was marked as resolved.

@ksuprynowicz

This comment was marked as resolved.

@ksuprynowicz

This comment was marked as resolved.

@ksuprynowicz

This comment was marked as resolved.

lubosz and others added 2 commits September 7, 2024 15:52
When enabling C++20 the lerp function seems to be redefined on GCC (not
on MSVC), don't redefine it using a CMake definition.
Add missing includes on Windows.
Include GLX only on Linux.

Move openxr_platform.h include to OpenXrContext.h. To make this
possible, certain names from GLX/X11 need to be undefined in order to be
now includable in OpenXrInput.h.

Add Overte e.V. copyright.

Co-authored-by: Lubosz Sarnecki <[email protected]>
@lubosz

This comment was marked as resolved.

@ksuprynowicz
Copy link
Member

I got the OpenXR run once, and movement using conrtollers worked, but tablet wouldn't show. Ond second run I got a crash immediately. Here's the backtrace:

<unknown> 0x000002348befddf0
[Inlined] gpu::Batch::Cache<std::shared_ptr<gpu::Texture> >::Vector::{dtor}() Batch.h:418
gpu::Batch::~Batch() Batch.cpp:65
OpenGLDisplayPlugin::render(function<…>) OpenGLDisplayPlugin.cpp:905
HmdDisplayPlugin::internalPresent() HmdDisplayPlugin.cpp:231
OpenGLDisplayPlugin::present(const std::shared_ptr<…> &) OpenGLDisplayPlugin.cpp:745
PresentThread::run() OpenGLDisplayPlugin.cpp:201

@ksuprynowicz
Copy link
Member

It works now! I'm not sure what pervious crash was about Batch.h but it doesn't occur on every start.

@ksuprynowicz
Copy link
Member

OpenXR also has issue with different shadows for each eye, which is visible here:
obraz

@ksuprynowicz
Copy link
Member

The hands seem to be offset from controllers, and head is in unusual position too - it seems that it's too low and too far back:
IMG_20240907_170950

@lubosz
Copy link
Collaborator Author

lubosz commented Sep 7, 2024

but tablet wouldn't show.

To properly implement input for that controller on would need to implement the XR_EXT_samsung_odyssey_controller extension and it's profile and create bindings for that. It might fall back to the khr/simple_controller profile now, but that's also worth debugging / confirming.

The hands seem to be offset from controllers, and head is in unusual position too - it seems that it's too low and too far back

I think I got that one right with the Index controllers, but I need to recheck that. For this controller there might need to be XR_EXT_palm_pose implemented, since it interacts with the extension for that controller. The hand position is currently calculated from /input/grip/pose and the transformation is inspired by the OpenVR plugin.
You could maybe also tweak the SteamVR input settings to map it to an Index controller.

@ksuprynowicz
Copy link
Member

but tablet wouldn't show.

To properly implement input for that controller on would need to implement the XR_EXT_samsung_odyssey_controller extension and it's profile and create bindings for that. It might fall back to the khr/simple_controller profile now, but that's also worth debugging / confirming.

I just checked again and tablet works. It's just that it shows on other button than on SteamVR, but works perfectly fine.


if(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
# Silence GCC warnings
target_compile_options(openxr PRIVATE -Wno-missing-field-initializers)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are we silencing here?

Currently, we hide unsolvable warnings through the OVERTE_WARNINGS_WHITELIST mechanism. See this PR for examples: https://github.com/overte-org/overte/pull/76/files#diff-3f39502437ebf928710ee1af1c0d2c0d26918dba1e067b6b08cd6b15cf4fb7bb

@JulianGro
Copy link
Member

Closes #181

@JulianGro JulianGro linked an issue Sep 15, 2024 that may be closed by this pull request
@ksuprynowicz
Copy link
Member

We should test it XRT_PARALLEL_VIEWS=1 Monado environment variable to see if that changes shadows behavior

@ksuprynowicz
Copy link
Member

ksuprynowicz commented Oct 11, 2024

I found another problem, mouse cursor in VR doesn't work correctly in this build. It's a very important feature for headsets without controllers. To enable it go to settings and enable reticle. On master branch it works correctly, and on OpenXR branch cursor position is different for each eye, and it's incorrect for both. Clicking on tablet works, but in incorrect position.

@ksuprynowicz
Copy link
Member

There also seems to be some sort of performance problem - it's extremely stuttery compared to OpenVR with Opencomposite on master branch.

@ksuprynowicz
Copy link
Member

Another performance problem is that game loop rate doesn't seem to be limited at all, game loop was running at 360 FPS for me, which maybe caused performance issues for other parts of the game?

@ksuprynowicz
Copy link
Member

The stutter happens even when game runs at constant 90 FPS. I think that it may be caused by camera positions not being updated correctly on the frame before it is rendered - our system generates frame in platform-independent format and then executes it on present thread. Before executing it camera positions are updated.

@ksuprynowicz
Copy link
Member

ksuprynowicz commented Oct 11, 2024

It looks like OpenXrDisplayPlugin::updatePresentPose() is empty. Maybe this is the cause?

@ksuprynowicz
Copy link
Member

ksuprynowicz commented Oct 12, 2024

I also noticed that head position seems to be too far back compared to master branch. Maybe this also affects controller positions?

@ksuprynowicz
Copy link
Member

It turns out performance issue was due to local build. I built Appimage in an Ubuntu 24.04 VM and it works pretty nice. It's still not as smooth as it's expected to be at 90 FPS, so it would be worth checking if camera corrections are passed correctly.

@ksuprynowicz
Copy link
Member

I just tested with WMR hand tracking on Monado, and hand positions are correct in OpenVR, but are incorrectly translated and rotated in OpenXR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs CR This pull request needs to be code reviewed needs QA This pull request needs to be tested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add OpenXR support
5 participants