extensions: Use Rust IO-safety types for fd/handle arguments/returns #791
Conversation
I like these types in principle, but I'm not sure about the cfg gates. Other ecosystem components (e.g. raw-window-handle) have moved away from |
|
Yup, that - together with being inconsistent with the import calls - is my only reason to not push this through. Coming up with a polyfill isn't desired either, and the caller will anyway have to |
|
Alternatively we have a (default enabled?) |
|
I don't think that's a reasonable use of features, since code built with it disabled will break when it is enabled. In another project, I worked around this problem by rolling my own |
|
Fair enough, Indeed unlikely that rolling our own polyfill warrants the obscurity in Ash. As already said above the caller would have to Vulkan handles share the same problems as the struct-based APIs discussed in the PR description, requiring and enforcing move semantics would make this almost impossible afaik. |
That's the trick I illustrated in sd-listen-fds: because the code will never dynamically run outside of Unixy platforms, you can just stub everything. Still, a lot of ceremony for ash. |
Use Rust's IO-safety types like `OwnedFd`/`BorrowedFd` (and their Windows `*Handle` equivalent) to communicate ownership transfer (or lack thereof) to the user and provide automatic cleanup when an owned handle is transferred to the caller. For Windows however the returned handle is only owned when it is a "handle type defined as NT handle" [1], being unclear what classifies as an NT handle (anything built/used after 1993??). We already contemplated this before, and the change puts `cfg(windows)` and `cfg(unix)` guards in place for the corresponding OS-specific extension wrappers, as these safety types are likewise only available on the aforementioned platforms. Unfortunately the import APIs pass file descriptors and handles via structs which have non-trivial ownership transfer into Vulkan functions: we could switch them to move semantics but there's no guarantee that these structs are only used with these functions. And it's even more complicated for external memory import: there the handle / file descriptor is passed in an `Info` struct chained in `pNext` for calls to `vkAllocateMemory()`, making it even more complicated (if at all possible) for us to intercept and "consume" the owned handle in a wrapper layer to prevent Rust from closing it. Furthermore note that all import APIs only transfer ownership _on Unix_; on Windows the caller remains responsible for calling `CloseHandle()` after (not sooner) the import is no longer used in Vulkan. This bumps MSRV to 1.63, and we could switch from `std::os::unix::io` to `std::os::fd` in MSRV 1.66. [1]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkGetMemoryWin32HandleKHR.html#_description
| /// May return [`None`] if the fence has already been signaled at the time this function is | ||
| /// called. | ||
| #[inline] | ||
| pub unsafe fn get_fence_fd(&self, get_info: &vk::FenceGetFdInfoKHR) -> VkResult<i32> { | ||
| pub unsafe fn get_fence_fd( | ||
| &self, | ||
| get_info: &vk::FenceGetFdInfoKHR, | ||
| ) -> VkResult<Option<OwnedFd>> { |
There was a problem hiding this comment.
Pushed a little update to denote that this can be None (when Vulkan sets it to -1).
The docs for the equivalent Win32 function don't state this though, perhaps there will always be a fence on Windows.
I'll take a look at the conflicts and remaining comments about polyfilling after getting some other PRs in.
Use Rust's IO-safety types like
OwnedFd/BorrowedFd(and their Windows*Handleequivalent) to communicate ownership transfer (or lack thereof) to the user and provide automatic cleanup when an owned handle is transferred to the caller.For Windows however the returned handle is only owned when it is a "handle type defined as NT handle" 1, being unclear what classifies as an NT handle (anything built/used after 1993??).
We already contemplated this before, and the change puts
cfg(windows)andcfg(unix)guards in place for the corresponding OS-specific extension wrappers, as these safety types are likewise only available on the aforementioned platforms.Unfortunately the import APIs pass file descriptors and handles via structs which have non-trivial ownership transfer into Vulkan functions: we could switch them to move semantics but there's no guarantee that these structs are only used with these functions. And it's even more complicated for external memory import: there the handle / file descriptor is passed in an
Infostruct chained inpNextfor calls tovkAllocateMemory(), making it even more complicated (if at all possible) for us to intercept and "consume" the owned handle in a wrapper layer to prevent Rust from closing it.Furthermore note that all import APIs only transfer ownership on Unix; on Windows the caller remains responsible for calling
CloseHandle()after (not sooner) the import is no longer used in Vulkan.This bumps MSRV to 1.63, and we could switch from
std::os::unix::iotostd::os::fdin MSRV 1.66.