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 more hal methods #5452

Merged
merged 8 commits into from
Apr 2, 2024
Merged

Add more hal methods #5452

merged 8 commits into from
Apr 2, 2024

Conversation

JMS55
Copy link
Contributor

@JMS55 JMS55 commented Mar 29, 2024

Connections
Revival of #3966

Description

  • Adds CommandEncoder::as_hal_mut()
  • Adds TextureView::as_hal()
  • Adds a return value to Texture::has_hal()

Enable some interop with native graphics APIs. I'm not convinced that this is in any way a good idea. Users have to be extremely careful about not accidentally taking locks on or destroying resources while using them from native dispatches. They also have to consider pipeline barriers and image transitions themselves, with no help from wgpu in tracking them. To work around this in Bevy, I plan to bind the resources to a dummy compute pass that does nothing, but that's run before+after my native graphics API to try and coerce wgpu into tracking the lifetimes and barriers appropriately. This is very much a band-aid patch while I wait for a proper fix in #4067.

Testing
Will be tested as part of Bevy's FSR integration bevyengine/bevy#12768.

Checklist

  • [ x ] Run cargo fmt.
  • [ x ] Run cargo clippy. If applicable, add:
    • [ x ] --target wasm32-unknown-unknown
    • [ x ] --target wasm32-unknown-emscripten
  • [ x ] Run cargo xtask test to run tests.
  • [ x ] Add change to CHANGELOG.md. See simple instructions inside file.

@JMS55 JMS55 requested a review from a team as a code owner March 29, 2024 03:24
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

I'm not convinced that this is in any way a good idea.

neither 😄, as you point out every single one of these apis is a massive footgun and requires intricate knowledge of wgpu almost down to the specific version. But these fit into what's already there and your usecase illustrates the need for workarounds like this.

Just a note on missing docs - I find it particularly scary that this bottoms to self.raw.begin_encoding and would like to have this visible from the top without going down into the code.

wgpu-core/src/resource.rs Show resolved Hide resolved
@JMS55 JMS55 requested a review from Wumpf April 2, 2024 01:43
@Wumpf Wumpf merged commit ed843f8 into gfx-rs:trunk Apr 2, 2024
27 checks passed
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