WIP: OpenXR integration #3761

blaind · 2021-05-14T16:12:24Z

One take at issue #3219

PR checklist:

make succeeds (on *nix)
make reftests succeeds
tested examples with the following backends:

I've made a proof-of-concept for OpenXR rendering on Bevy engine, using wgpu & underlying stack. See https://github.com/blaind/xrbevy

The code in this pull request is not even close to merge-able, but I thought its good idea to open it for further discussion.

For architecture of the pull request, see https://github.com/blaind/xrbevy/blob/main/docs/architecture.md (evolving document)

See also gfx-rs/wgpu-rs#910 and gfx-rs/wgpu#1387

Fixes linking issues when QuartzCore is not linked by some other means

kvark

Thank you for prototyping this! Excited to see XR integration forming 🚀

kvark · 2021-05-16T04:52:18Z

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Change Log

+### backend-metal-0.8.2 (08-05-2021)


something is off here, these commits shouldn't be showing up in this PR

Seems not come after blaind/wgpu@d90b30e, not entirely sure what caused it... will try to fix.

Looks like the PR is still off the base

could you try rebasing properly on master?

I branched off from hal-0.8 (specifically commit e625940, 0.8.2). Had to do that because otherwise I had hard-to-debug problems with syncing gfx/wgpu/wgpu-rs to compatible versions.

Seems like if I change the gfx base to hal-0.8, those commits aren't visible here.

However, the underlying code is used by https://github.com/blaind/xrbevy and people are actively testing, I may not be able to base the gfx/wgpu/wgpu-rs all to latest commits...

I guess one option would be to branch the pull request from the bevy_openxr branch to a separate pull request branch, and update the latest commits when the bevy_openxr branch develops further.

Especially now that people are testing, some changes are to be expected.

Furthermore, I do not know if this pull request is eventually the best approach to land the code. One option is to use the raw handles (#3762) from gfx and expose them through wgpu-layers, I did some experimenting last week but that's quite complex. That, and other alternatives probably good to investigate still.

I see. This code should definitely target gfx master, not 0.8 branch.

Had to do that because otherwise I had hard-to-debug problems with syncing gfx/wgpu/wgpu-rs to compatible versions.

Let us help with this. We override gfx stuff locally all the time. There are even commented out sections of this override in Cargo toml for both wgpu and wgpu-rs.

kvark · 2021-05-16T04:52:52Z

src/backend/vulkan/Cargo.toml

@@ -15,6 +15,7 @@ edition = "2018"
 [features]
 default = []
 use-rtld-next = ["libc"]
+use-openxr = ["openxr", "once_cell"]


maybe just xr instead of use-openxr?

Initially went with "use-openxr" because "openxr" clashed with the crate with same name, but I guess that's doable as a name as well.

I like the 'xr', can make the change. Although, that might make the name ambiguous with other possible xr implementations in the future.

kvark · 2021-05-16T04:54:11Z

src/backend/vulkan/src/device.rs

-                    .attachments(&attachments)
-                    .subpasses(&subpasses)
-                    .dependencies(&dependencies);
+                const VIEW_COUNT: u32 = 2; // FIXME get from xr


it says "get from xr", but shouldn't this be given by the user? I.e. the user would need to check if multi-view is available, enable it as a feature, and then request the view count explicitly here (orthogonal to XR).

I'm not entirely sure about how and at what level this should be handled.

From OpenXR spec: A typical head-mounted VR system has a view configuration with two views, while a typical phone-based AR system has a view configuration with a single view. A simple multi-wall projection-based (CAVE-like) VR system may have a view configuration with at least one view for each display surface (wall, floor, ceiling) in the room. (link). Also Varjo headset seems to have 4 views in total, two screens per eye.

Probably the screens might have different resolutions, so the selection of multiview count and different textures should be left for the application?

I tried to pull the configuration info through the wgpu <--> gfx barrier, but did not manage yet (link to code section)

gfx-hal can't abstract away multi-viewport rendering entirely, it's too low level for this. The most it can offer is exposing the viewport geometry and providing the way to control the multi-viewport swapchain. The user of gfx-hal, which is wgpu in our case, would then be responsible for using this API and actually do the rendering.

kvark · 2021-05-16T04:55:41Z

src/backend/vulkan/src/device.rs

@@ -1156,6 +1172,36 @@ impl d::Device<B> for super::Device {
        }
    }

+    #[cfg(feature = "use-openxr")]
+    unsafe fn create_image_from_openxr_raw_image(


should this be more general, i.e. wrap_raw_image or something like this?

Sounds good. I guess the feature gate can also be removed?
This is part of pub trait Device<B>

yeah, I think adding trait methods behind a cfg flag is an anti-pattern. Since features are additive, if one crate uses X and implements its trait T, but another crate uses X with feature F, then this first implementation will fail at compile time, since an extra method would be unimplemented.

kvark · 2021-05-16T04:56:30Z

src/backend/vulkan/src/lib.rs

+                .api_version(vk::make_version(1, 1, 0))
+        } else {*/
+
+            vk::ApplicationInfo::builder()


what's the difference here? the fact that we are requesting 1.0 only?

For openxr, there's some initialization code to be done with vk::Entry. See example on android. E.g. platform specific initialization and / or querying and enabling/disabling extensions or extra layers.

I guess at least three options could be investigated here:

Can the try_enumerate_instance_version above use entry provided by an app?

Could the incoming xr configuration data have the driver api version?

Should the entry-creation be moved from app to gfx, and raw handle moved back and forth for extra configuration

Sorry, not sure I see anything about vk::Entry in this link. I only see openxr::Entry there.

kvark · 2021-05-16T04:58:03Z

src/backend/vulkan/src/lib.rs

+                let mut instance_guard = xr::INSTANCE.lock().unwrap();
+                let openxr_instance = instance_guard.as_mut().unwrap();
+                let vk_physical_device = openxr_instance.get_device(self.raw.inner.handle());
+                devices = vec![vk_physical_device];


it seems to me that we should just enumerate this together with the other devices, but have some way to check if a particular physical device is XR or not

kvark · 2021-05-16T05:38:42Z

src/backend/vulkan/src/physical_device.rs

+        if xr::in_use() {
+            queue_families
+                .filter(|(_, info)| {
+                    if info.queue_flags.contains(vk::QueueFlags::GRAPHICS) {


similarly, this should be a decision of the user of gfx-hal, not the library itself

Would that mean e.g. wgpu?

correct, that would be wgpu

kvark · 2021-05-16T05:40:30Z

src/backend/vulkan/src/xr.rs

+    frame_waiter: Option<openxr::FrameWaiter>,
+    frame_stream: Option<openxr::FrameStream<openxr::Vulkan>>,
+    space: Option<openxr::Space>,
+    // FIXME add field for all -validations state, that'd track separate parts


what kind of validation is supposed to be done here?

It's a while since I wrote this part of code, but I'm somewhat sure I meant an enum that'd track the initialization state (DeviceCreated, ReadyForRendering, or something similar), that would reflect in the result of xr-related calls. For example, return an error from handles getter (get_session_handles) if the creation process hasn't been finalized yet.

kvark · 2021-05-16T05:41:32Z

src/backend/vulkan/src/xr.rs

+}
+
+#[cfg(feature = "use-openxr")]
+pub(crate) static INSTANCE: Lazy<Mutex<Option<Instance>>> = Lazy::new(|| Mutex::new(None));


We shouldn't need this. Instead, one of the two approaches should be taken:

this data is a part of the main Instance

it's managed by the user, and gfx-backend-vulkan just understands it and knows how to cooperate

This was the biggest hurdle I got into. Been a while since I made this decision, but:

avoid changing the hal structs (which would require changes to all dependents)

could not access instance from device-specific code (impl hal::Backend for Backend), but now it seems that Arc<RawDevice> contains Arc<RawInstance>, so in theory the required openxr data could be stored in those? Maybe some changes have occured between 0.7 and 0.8?

yes, we should store the openxr data in RawDevice or RawInstance if it's needed in multiple places.

blaind · 2021-05-16T07:49:47Z

Even though this renders, there might be some parts from OpenXR api that are missing currently:

https://www.khronos.org/registry/OpenXR/specs/1.0/html/xrspec.html#xrCreateVulkanInstanceKHR creation of vulkan device through openxr API
https://www.khronos.org/registry/OpenXR/specs/1.0/html/xrspec.html#_swapchain_interactions specific swapchain image type?

blaind · 2021-05-16T07:55:59Z

I also drafted a diagram about openxr entities:

I wonder if the swapchain handling could be integrated into gfx (or wgpu?)

blaind · 2021-05-16T13:28:33Z

src/backend/vulkan/src/xr.rs

+
+#[cfg(feature = "use-openxr")]
+impl OpenXR {
+    pub fn configure(instance: openxr::Instance) -> Result<OpenXR, Error> {


This is the definition of openxr::Instance:

pub struct Instance { inner: Arc<InstanceInner>, }

its cloneable, and transferred from app to the gfx through wgpu. See:

However, this brings a problem when dropping the instance. openxr::Instance has a impl Drop that goes through FFI to C code.

Firstly, I wonder what happens on the app code, is the Arc-reference kept even though the instance is on GFX side? Where should it be dropped?

Currently getting a segfault in integration test, since wgpu is trying to drop resources even though the openxr runtime still thinks its running. I guess this also brings out the question about desired safety level of openxr in gfx- and what steps can be documented for end-users to ensure the safeness even if the api is unsafe?

Same applies to all dependent openxr-structs that are instantiated. Although, currently all except session are .take():n away in the get_session_handles() below. Session can also be taken away in current implementation.

I guess this also brings out the question about desired safety level of openxr in gfx- and what steps can be documented for end-users to ensure the safeness even if the api is unsafe?

gfx-hal is totally unsafe, no need to bother with trying to make things safe at this level. But at least it should have a clear way for users to write safe abstractions. I.e. like in Vulkan, you have to destroy device resources before destroying the device itself.

@blaind

3762: Low level interop with Vulkan backend r=kvark a=zarik5 Related issues: #3698 #3761 blaind/xrbevy#1 This PR exposes some Vulkan-backend-specific details needed for interop with other low level APIs. In particular this PR aims to implement a minimum viable API to allow OpenXR integration as a separate crate. This PR is nowhere complete. I opened it to get some feedback; this is my first PR on a project the size of gfx-hal. This is the first of multiple PRs needed for OpenXR support in wgpu. PR checklist: - [ ] `make` succeeds (on *nix) - [ ] `make reftests` succeeds - [ ] tested examples with the following backends: Vulkan EDIT: To make things clear, this is another take on #3761. Me and @blaind are working together on this route. Co-authored-by: zarik5 <[email protected]>

… device

blaind · 2021-07-23T07:26:55Z

I'll close this - there's a better encapsulation approach with raw handles that's being continued at gfx-rs/wgpu#1415 && gfx-rs/wgpu#1609

Uniformbuffer3 and others added 4 commits May 8, 2021 21:16

Fixed missing or wrong names link on documentation.

76a914e

metal: Link to QuartzCore (gfx-rs#3742)

b15e2f0

Fixes linking issues when QuartzCore is not linked by some other means

[metal] bump version to 0.8.2

e625940

Initial bevy_openxr code

fd8bf37

This was referenced May 14, 2021

WIP: OpenXR integration gfx-rs/wgpu-rs#910

Closed

WIP: OpenXR integration gfx-rs/wgpu#1387

Closed

blaind marked this pull request as draft May 14, 2021 16:36

blaind mentioned this pull request May 14, 2021

WIP: OpenXR integration bevyengine/bevy#2166

Closed

Merge remote-tracking branch 'gfx/hal-0.8' into bevy_openxr

c506945

kvark requested changes May 16, 2021

View reviewed changes

blaind mentioned this pull request May 16, 2021

Better integration with gfx-hal blaind/xrbevy#1

Open

blaind commented May 16, 2021

View reviewed changes

zarik5 mentioned this pull request May 16, 2021

Low level interop with Vulkan backend #3762

Merged

3 tasks

blaind added 5 commits May 23, 2021 22:12

Raw images

5225875

Added windows vulkan plugins

3b935a9

Add VK_NV_external_memory_capabilities

ea28f98

On missing extensions, panic only after all extensions have been checked

2538860

Add two new extensions

691d91a

kvark mentioned this pull request May 30, 2021

Add support for raw vulkan types gfx-rs/wgpu#1415

Closed

4 tasks

A bit of debugging information, and call enumerate_* after setting up…

e0f4947

… device

blaind closed this Jul 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: OpenXR integration #3761

WIP: OpenXR integration #3761

blaind commented May 14, 2021 •

edited

Loading

kvark left a comment

kvark May 16, 2021

blaind May 16, 2021

kvark May 25, 2021

kvark May 29, 2021

blaind May 30, 2021 •

edited

Loading

kvark May 30, 2021

kvark May 16, 2021

blaind May 16, 2021

kvark May 16, 2021

blaind May 16, 2021 •

edited

Loading

kvark May 25, 2021

kvark May 16, 2021

blaind May 16, 2021

kvark May 25, 2021

kvark May 16, 2021

blaind May 16, 2021

kvark May 25, 2021

kvark May 16, 2021

kvark May 16, 2021

blaind May 16, 2021

kvark May 25, 2021

kvark May 16, 2021

blaind May 16, 2021 •

edited

Loading

kvark May 16, 2021

blaind May 16, 2021 •

edited

Loading

kvark May 25, 2021

blaind commented May 16, 2021

blaind commented May 16, 2021

blaind May 16, 2021 •

edited

Loading

kvark May 25, 2021

blaind commented Jul 23, 2021

		@@ -1,5 +1,8 @@
		# Change Log

		### backend-metal-0.8.2 (08-05-2021)

WIP: OpenXR integration #3761

WIP: OpenXR integration #3761

Conversation

blaind commented May 14, 2021 • edited Loading

kvark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blaind May 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blaind May 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blaind May 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blaind May 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blaind commented May 16, 2021

blaind commented May 16, 2021

blaind May 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blaind commented Jul 23, 2021

blaind commented May 14, 2021 •

edited

Loading

blaind May 30, 2021 •

edited

Loading

blaind May 16, 2021 •

edited

Loading

blaind May 16, 2021 •

edited

Loading

blaind May 16, 2021 •

edited

Loading

blaind May 16, 2021 •

edited

Loading