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 raw vulkan types #1415

Closed
wants to merge 3 commits into from
Closed

Add support for raw vulkan types #1415

wants to merge 3 commits into from

Conversation

LaylBongers
Copy link
Contributor

@LaylBongers LaylBongers commented May 29, 2021

Connections
This PR depends on: gfx-rs/gfx#3767
This PR is necessary for: #602

Description
This adds support for using raw Vulkan instances, devices, and images with WGPU. This allows applications to use OpenXR with WGPU.

Testing
Bindings and example in WGPU-RS PR: gfx-rs/wgpu-rs#919

To-Do

  • Review public API, is this API fine for inclusion in WGPU?
  • Review the way external image views affect framebuffer attachments lifetime and texture transitions (I'm not familiar enough with this system, help needed! if you happen to know the specific rules surrounding the unsafe lifetime of image views for this too, please add that to the docs)
  • Document unsafe lifetime rules for external data (instances, devices, image views)
  • Improve error handling in the new functions

@@ -582,6 +582,9 @@ impl<'a, B: GfxBackend> RenderPassInfo<'a, B> {
TextureViewInner::SwapChain { .. } => {
return Err(RenderPassErrorInner::SwapChainImageAsDepthStencil);
}
TextureViewInner::Raw { .. } => {
return Err(RenderPassErrorInner::SwapChainImageAsDepthStencil);
Copy link
Member

Choose a reason for hiding this comment

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

wrong error

@@ -496,7 +496,7 @@ fn check_device_features(
}

struct RenderAttachment<'a> {
texture_id: &'a Stored<id::TextureId>,
texture_id: Option<&'a Stored<id::TextureId>>,
Copy link
Member

Choose a reason for hiding this comment

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

why is it optional now?

@@ -2626,6 +2632,66 @@ impl<B: GfxBackend> Device<B> {
}
}

impl Device<crate::backend::Vulkan> {
unsafe fn create_raw_vulkan_texture_view(
Copy link
Member

Choose a reason for hiding this comment

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

This is quite tricky. How do you feel about the approach in gfx-rs/gfx#3761 as an alternative?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Recap from discussion we've had on this on Matrix:
The plan right now is to introduce external textures to gfx-rs and use that instead of the raw vulkan views, and see if that's possible with OpenXR.

@kvark
Copy link
Member

kvark commented Jun 3, 2021

Heads up - we have major re-organization happening, so it would be better to land this after https://github.com/gfx-rs/wgpu/milestone/9

@LaylBongers
Copy link
Contributor Author

Sounds good, I'll likely re-write this using some more context when that's all landed.

@LaylBongers
Copy link
Contributor Author

This seems to have been superseded by #1609 so I'll close this

@blaind blaind mentioned this pull request Jul 23, 2021
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants