-
Notifications
You must be signed in to change notification settings - Fork 187
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
extensions/khr: Add VK_KHR_external_fence_fd wrapper #413
Conversation
@MaikKlein Given your question about POSIX fd's in #395 (comment) I've been tempting to replace the types with Either way we should probably be nice and provide the Win32 variants of these import/export functions as well. |
I think it's preferable to be able to build on any platform, so an application can rely on e.g. advertised extension support to select code paths without any special effort. |
@Ralith For one it's a nice way to force users to write: let extensions = vec![
...
#[cfg(unix)]
ash::extensions::khr::ExternalFenceFd::name().as_ptr(),
...
]; When creating this list of desired extensions. Same when using this API, these codepaths already have to be made optional to function correctly which might as well be enforced at compile time? For example when porting a Linux/UNIX application using this API to Windows it is immediately clear that this isn't going to work, rather than having to find that out at runtime. EDIT: I personally appreciated Rusts (and the crates around here) handholding when it comes to not exposing features that are not supported on a platform, rather than running into obscure errors at runtime. At the same time I can see that it might be desired to look at supported extensions and go from there, even though these extensions are rather specific and already need a platform-specific abstraction (Fd vs Win32 handle) anyway. |
It's already fairly common in Vulkan to branch between significantly different code paths depending on device capabilities, after all. |
This extension wrapper is identical to the one for VK_KHR_external_semaphore_fd introduced in ash-rs#395 with 'semaphore' substitued for 'fence'.
3a7209f
to
3645a89
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good as is it with i32
as fd. We don't have platform specific types in ash currently as well.
@MaikKlein Thanks! Allright, let's leave it like this, the underlying type is the same anyway. Maybe someone in the future has reasons to constrain these extensions to be available on the desired ( |
This extension wrapper is identical to the one for
VK_KHR_external_semaphore_fd
introduced in #395 with 'semaphore' substitued for 'fence'.