feat(vulkan): allow specifying queue family ownership transfer in texture barriers#9668
Conversation
…ture barriers
Images imported from external memory must have their backing image's queue
family ownership transferred from a sentinel queue family (FOREIGN/EXTERNAL)
to wgpu's queue family on acquire, and transferred back on release. The
Vulkan HAL previously built image memory barriers without ever setting
src/dst queue family indices, so this was impossible to express.
Add an optional `queue_family_ownership_transfer` field to the
backend-agnostic `hal::TextureBarrier`. It is honored only by the Vulkan
backend (which now sets `src/dstQueueFamilyIndex` on the
`VkImageMemoryBarrier`); all other backends ignore it. When unset, both
indices default to `VK_QUEUE_FAMILY_IGNORED`, preserving existing behavior.
Expose `hal::QUEUE_FAMILY_{IGNORED,EXTERNAL,FOREIGN}` sentinels (with a
compile-time assertion that they match ash's values) so callers using
`vulkan::Device::queue_family_index()` can build acquire/release transfers
without depending on ash directly.
Resolves gfx-rs#2948.
alexander-bruun
force-pushed
the
feat/2948-vulkan-queue-family-ownership-transfer
branch
from
8169962 to
984d3f0
Compare
|
CC @atlv24 :))) As for this PR, we actually plan to implement multiqueue ourselves so QFOTs would be part of that. As for now this is only an unsafe hal interop thing for one API, I would be agaInst putting it in a normal wgpu api like barriers. Finally, even if this were correct, it would probably have to be implemented for all barriers. Happy to discuss this further, I think there is some way to make this work before multiqueue lands. |
cwfitzgerald
left a comment
cwfitzgerald
left a comment
There was a problem hiding this comment.
Thanks for working on this! Some comments
| // The backend-agnostic queue family sentinels in `crate` must match the Vulkan | ||
| // values they stand for, since they are passed straight through to ash. | ||
| const _: () = { | ||
| assert!(crate::QUEUE_FAMILY_IGNORED == vk::QUEUE_FAMILY_IGNORED); | ||
| assert!(crate::QUEUE_FAMILY_EXTERNAL == vk::QUEUE_FAMILY_EXTERNAL); | ||
| assert!(crate::QUEUE_FAMILY_FOREIGN == vk::QUEUE_FAMILY_FOREIGN_EXT); | ||
| }; | ||
|
|
There was a problem hiding this comment.
This seems like unnecessary coupling, we can have a mapping function from an enum in the transition struct to the vulkan ones.
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub struct QueueFamilyOwnershipTransfer { | ||
| /// The queue family that currently owns the image (`srcQueueFamilyIndex`). | ||
| pub src: u32, |
There was a problem hiding this comment.
I think we can have an enum with variants for ignored/external/foreign, then a variant with like Explicit(u8) or something
| /// Special queue family value indicating that no ownership transfer should be | ||
| /// performed (`VK_QUEUE_FAMILY_IGNORED`). | ||
| pub const QUEUE_FAMILY_IGNORED: u32 = u32::MAX; |
There was a problem hiding this comment.
When we have an optional queue family transition, does this variant actually make sense to have, wouldn't you use None
3 participants
Connections
Resolves #2948.
Description
Adds an optional
queue_family_ownership_transferfield tohal::TextureBarrierso textures imported from external memory can be acquired from / released back to a sentinel queue family. Honored only by the Vulkan backend (setssrc/dstQueueFamilyIndexon the image barrier); all other backends ignore it. WhenNone(every existing caller) the indices default toVK_QUEUE_FAMILY_IGNORED, so behavior is unchanged.Also exposes
hal::QUEUE_FAMILY_{IGNORED,EXTERNAL,FOREIGN}sentinels so callers don't need a directashdependency.Testing
cargo check -p wgpu-hal --features vulkan --all-targetsandcargo check -p wgpu-core --features vulkan.ash.Squash or Rebase?
Single commit, ready to rebase onto
trunkas-is.