godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.41k stars 21.26k forks source link

Godot OpenXR, improved handling of state #83471

Closed BastiaanOlij closed 6 months ago

BastiaanOlij commented 1 year ago

Godot version

4.2.dev

System information

Any

Issue description

This issue is based on feedback after discussing various timing issues around OpenXR and clarification of when certain calls into the OpenXR runtime should take place.

Current Godot OpenXR loop

While I've color coded this with threading in mind, currently there are a number of issues to fix around this. The one that stands out is that xrWaitFrame provides us with the timing information on which we update our tracking data. In our Update XR Trackers step we're thus using predictedDisplayTime + predictedDisplayPeriod, however in a multi threading environment there is no guarantee that wait frame has already been processed before we start handling our next frame. This is part of what we need to fix.

The actual flow should be akin to:

New Godot OpenXR loop

Seems a simple enough change, xrWaitFrame is now called at the start from our main thread (this will need to be moved into our XR process, which is where our tracker update also happens), but we need to do a little more as some of our logic that runs in the render thread shares information with our main thread. That will need to be duplicated so we don't overwrite this data before hand. After this change we can purely rely on predictedDisplayTime. Obtaining new camera data will use updated tracking data to minimize latency, which was one of the misunderstandings in how this was designed to work.

Similarly, we keep track of the location of our XROrigin3D node in order for us to update the XR camera position correctly. This currently isn't implemented thread safe either.

The final change that is needed specifically effects Android. As part of our submit logic in the default rendering pipeline, we manage an internal swapchain for our normal game presentation. The contents of this swapchain is then output to screen. On Android presentation is fully handled by OpenXR and this introduces unnecessary overhead. We can skip this part if there is nothing to present (our internal blit list will be empty).

This last one also kind of applies to PCVR, but we currently don't have a way to prevent the default window from opening as this is a core part of the display server. That is a separate issue for another day.

Steps to reproduce

n/a

Minimal reproduction project

n/a

BastiaanOlij commented 1 year ago

cc @lyuma @m4gr3d

BastiaanOlij commented 9 months ago

OK, getting this done and making sure XR works properly with rendering happening on a separate thread has now become a higher priority. Turns out issues on Quest 3 were related to Meta assuming rendering happens on a separate thread and there are timing issues when this is not the case...

Kimau commented 8 months ago

Excited for this change 👍