-
Notifications
You must be signed in to change notification settings - Fork 81
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
Allow vkb::Device::get_queue to return a graphics queue as a fallback when no separate compute / transfer queue found. #175
Conversation
…t available in Device::get_queue_index.
Having get_queue return nothing when requesting a transfer queue or compute queue that isn't separate from graphics is by design. What happens is that applications must know when the have a 'distinct' compute/transfer queue, since it affects the way in which semaphores are used. Not to mention, queues are externally synchronized, so its important they not think they have two separate queues when in reality its the same VkQueue handle. I believe the intent of the mentioned issue is that a queue can support transfer operations even if it doesn't say it does. This only applies to transfer queues, not compute. And I hadn't resolved it because it doesn't mesh well with the existing queue logic. Not to mention, its a 'corner case' in the spec that to my knowledge doesn't exist in real drivers. |
I do agree that applications must know if there is a 'distinct' compute/transfer queue. However, this can be left for the users to perform a simple additional check on the returned queue handle, but not preventing them to get one. Having only one queue supporting both graphics and compute functionality is not rare in low-end devices, especially integrated graphics card or older laptop graphics card like GTX960M. |
So I am not going to budge on the current behavior. Trying to get a transfer queue or compute queue should not return the graphics queue if there is only 1 queue, this prevents users from misusing the queue by thinking its a separate queue. However, what your PR indicates is that the stated design is not communicated well through the API or the documentation. This absolutely can be improved. |
This might be improved through an extra There's another problem that current implementation may return duplicated compute / transfer queue (index) through |
I think a solution to this problem would be returning 'all' the queues in a single struct, rather than allowing individual querying of queues. struct Queue {
VkQueue queue;
VkQueueFlags capabilities; //specifies all the 'capabilities' of this queue.
bool supports_present;
};
struct Queues {
Queue graphics;
Queue compute; // non-null queue member if distinct from the graphics queue
Queue transfer; // non-null queue member if distinct from the graphics queue & compute
};
Result<Queues> vkb::Device::get_queues() {...} |
So after coming back to this repo and trying to clean things up, this is not the only instance of problems with the current design for getting VkQueues. It is probably the part of the API that was reworked the most, and did not have a satisfying design when I first released vk-bootstrap. That said, I do not think I can take this PR mostly because it is doesn't do enough to clean things up and make things better. I am not closing the issue yet, rather I would like to keep it open until I create my own PR to implement what I think is a better solution. As a reminder, this will stay open. I'd love to have your review & feedback when that happens, which hopefully should be soon (feel free to @ me if you think its been too long). |
I have begun work on a solution to this here: Needs tests & documentation, but the API is solidified (check VkBootstrap.h) |
Closing in favor of #214 |
Plz forgive my stupid mistake in PR #174 🤣
===========================================
As mentioned in #109 , vkb currently throws an error when trying to get a compute / transfer queue in low-end devices that has no separate queues.
Following changes are made to allow
vkb::Device::get_queue(_index)
to return a graphics queue (index) as a fallback:undesired_flags
parameter tovkb::detail::get_first_queue_index
, it defaults to 0 to avoid changing existing callsget_seperate_queue_index
which removes the graphics queue limitation to rewrite the functionget_seperate_queue_index
invkb::Device::get_queue_index
withget_first_queue_index
and specify graphics & compute/transfer queue as undesired flagsHope this can help.