Provide a Vulkan path for create_surface_metal #8558
Conversation
IsaacMarovitz
changed the title
IsaacMarovitz
force-pushed
the
mvk-surface
branch
from
1925901 to
a915331
Compare
Signed-off-by: Isaac Marovitz <[email protected]>
IsaacMarovitz
force-pushed
the
mvk-surface
branch
from
a915331 to
0779e82
Compare
Wumpf
left a comment
Wumpf
left a comment
There was a problem hiding this comment.
Looking good, but feature flags need fixing and I think we should adjust naming to mirror semantics better.
I don't have an easy way to test this PR, so help in this area would be appreciated.
We unfortunately don't have great coverage for a lot of surface things. And afaik moltenvk is particularly poorly tested. I trust you ran it yourself locally?
| /// # Safety | ||
| /// | ||
| /// `layer` must be a valid pointer. | ||
| #[cfg(metal)] |
There was a problem hiding this comment.
this being now for metal & vulkan @ mac means that the feature cfg is now incorrect. this is also true for callsites
There was a problem hiding this comment.
This codepath should still work in cases of only Metal enabled so I wasn't sure what the best combination was for this case.
There was a problem hiding this comment.
well but it should also work now if only vulkan was enabled on mac
| /// | ||
| /// `layer` must be a valid pointer. | ||
| #[cfg(metal)] | ||
| pub unsafe fn create_surface_metal( |
There was a problem hiding this comment.
this method working with both metal & vulkan is a bit strange with the current name. I initially figured that we may want to split this up into three methods - layer @ metal, layer @ vulkan, layer @ either - but I figure the combined one is the only meaningful one practically speaking. So I suggest rather to clean this up and call this create_surface_core_animation_layer instead. The change should be trickled up the chain so that wgpu_core then calls into instance_create_core_animation_layer when encountering a layer. (care need to be taken to do this under the right feature flags)
I built it locally, but I don't have a proper test case for this right now. I'll try and make one. |
|
@Wumpf It seems I'm running into |
|
As of right now my only test case for this is building wgpu-native based on this branch and plugging the final dylib into the application I need this Vulkan path which I realise has quite a few added variables of potential problems. |
2 participants
Connections
Attempts to fix #8555.
Description
Currently, it is impossible to create a MoltenVK-backed surface when using
wgpu-nativebindings as theRawWindowHandleAPI is not available. This PR attempts to extendcreate_surface_metalto allow the creation of Vulkan backends when requested.Testing
I don't have an easy way to test this PR, so help in this area would be appreciated.
Squash or Rebase?
Rebase
Checklist
cargo fmt.taplo format.cargo clippy --tests. If applicable, add:--target wasm32-unknown-unknowncargo xtask testto run tests.CHANGELOG.mdentry.