gfx-rs / portability

Vulkan Portability Implementation
Mozilla Public License 2.0
383 stars 25 forks source link

Add transfer bit to all queue families #187

Closed aloucks closed 5 years ago

aloucks commented 5 years ago

Per the gfx_hal::queue::QueueType docs, all variants should include transfer operations. Notably, General now contains all three queue types.

kvark commented 5 years ago

This is actually written down in Vulkan spec:

All commands that are allowed on a queue that supports transfer operations are also allowed on a queue that supports either graphics or compute operations. Thus, if the capabilities of a queue family include VK_QUEUE_GRAPHICS_BIT or VK_QUEUE_COMPUTE_BIT, then reporting the VK_QUEUE_TRANSFER_BIT capability separately for that queue family is optional.

So even though I'm not a fan of that wording, making this change isn't going to make us any more correct than we already are. It's not in our power to change the Vulkan spec, but we aspire to be better with gfx-hal at least.

Saying that, I don't feel like we'd get much from this change. Do you feel strongly about it?

aloucks commented 5 years ago

The drivers that I've seen report all three flags on the "unified" queue (and the sparse binding when supported). I would think that at a minimum all three bits should be on the General queue family so that it looks like most other drivers. I currently check for all three bits and bail out if the unified queue isn't found. I could always change this my code, but it matches the driver behavior as far as I can tell.

NVIDIA GeForce GTX 950 https://vulkan.gpuinfo.org/displayreport.php?id=5691#queuefamilies

NVIDIA GeForce RTX 2060 https://vulkan.gpuinfo.org/displayreport.php?id=5891#queuefamilies

NVIDIA GeForce GTX 1080 Ti https://vulkan.gpuinfo.org/displayreport.php?id=5978#queuefamilies

Radeon(TM) HD 8800 Series https://vulkan.gpuinfo.org/displayreport.php?id=5965#queuefamilies

Intel(R) UHD Graphics 630 https://vulkan.gpuinfo.org/displayreport.php?id=5810#queuefamilies

kvark commented 5 years ago

Ok, I suppose the drivers just try to play safe.

Bors r+

On Jun 1, 2019, at 02:06, aloucks notifications@github.com wrote:

Saying that, I don't feel like we'd get much from this change. Do you feel strongly about it?

The drivers that I've seen report all three flags on the "unified" queue (and the sparse binding when supported). I would think that at a minimum all three bits should be on the General queue family so that it looks like most other drivers. I currently check for all three bits and bail out if the unified queue isn't found. I could always change this my code, but it matches the driver behavior as far as I can tell.

NVIDIA GeForce GTX 950 https://vulkan.gpuinfo.org/displayreport.php?id=5691#queuefamilies

NVIDIA GeForce RTX 2060 https://vulkan.gpuinfo.org/displayreport.php?id=5891#queuefamilies

NVIDIA GeForce GTX 1080 Ti https://vulkan.gpuinfo.org/displayreport.php?id=5978#queuefamilies

Radeon(TM) HD 8800 Series https://vulkan.gpuinfo.org/displayreport.php?id=5965#queuefamilies

Intel(R) UHD Graphics 630 https://vulkan.gpuinfo.org/displayreport.php?id=5810#queuefamilies

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

bors[bot] commented 5 years ago

Build succeeded