renderdoc
renderdoc copied to clipboard
Fix `vkEnumeratePhysicalDeviceGroups` being non-conformant with Vulkan specification
Fix for #3261
Description
RenderDoc's vkEnumeratePhysicalDeviceGroups
implementation was non-compliant with Vulkan specification.
When vkEnumeratePhysicalDeviceGroups
has been called to query less than the total number of available device groups, then pPhysicalDeviceGroupCount
parameter was overridden with the total amount of available device groups. However, Vulkan specification says that the amount of device groups written to pPhysicalDeviceGroupProperties
should be returned instead.
In such cases, this was causing Vulkan loader (vulkan-1.dll
) to crash on preparing vkEnumeratePhysicalDeviceGroups
trampoline.
To fix this issue, I have reworked vkEnumeratePhysicalDeviceGroups
implementation to be Vulkan specification compliant. I have also made some improvements, like early exit and replacement of raw memory management with rdcarray
container.
Unless I'm missing something I think there's a different problem with the new code. From reading your description, it seems like a simpler commit to only change:
if(pPhysicalDeviceGroupCount) *pPhysicalDeviceGroupCount = RDCMIN(outputSpace, numPhys);
Would fix the problem? Since the array handling is fine we just don't clamp the returned output count to the space given by the application.
We cannot apply such a simple fix to the old code, as it don't differentiate cases of possible pPhysicalDeviceGroupCount
return values:
- total number of available device groups; when
pPhysicalDeviceGroupProperties == NULL
- actual number of written physical device groups; when
pPhysicalDeviceGroupProperties != NULL
I insist on merging this fix with my other changes to vkEnumeratePhysicalDeviceGroups
function.
Nevertheless, if you don't like style of my changes, I can try to just emplace fix in original function body.
Ah OK I see what you mean, yes it needs more than the one line fix. I was just trying to clarify which of the changes are necessary or not, as generally speaking a simpler fix is preferable, I wasn't suggesting that I would reject the fix entirely.