Skip to content
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

document calling convention to load() better, PfnGetInstanceProcAddr may not be the right calling convention for most #30

Closed
slimsag opened this issue Nov 26, 2021 · 3 comments

Comments

@slimsag
Copy link
Contributor

slimsag commented Nov 26, 2021

vulkan-zig currently declares vk.PfnGetInstanceProcAddr as using the vulkan calling convention:

pub const PfnGetInstanceProcAddr = fn (
    instance: Instance,
    p_name: [*:0]const u8,
) callconv(vulkan_call_conv) PfnVoidFunction;

But this is not a Vulkan proc, it's a C proc which returns Vulkan procs and should use callconv(.C). For example, GLFW's C API does not define glfwGetInstanceProcAddress as using the Vulkan calling convention, but rather the standard C calling convention

For now, this just happens to work because vulkan_call_conv is the same as callconv(.C) on all platforms except windows-i386 (.Stdcall) and android-arm (.AAPCSVFP) which have presumably not been tested with zig-vulkan.

@Snektron
Copy link
Owner

It's valid for vkGetInstanceProcAddr to resolve itself, in which case i would think that the return value is a PfnGetInstanceProcAddr with the vulkan calling convention. In this sense, PfnGetInstanceProcAddr is a pointer to vkGetInstanceProcAddr as provided by Vulkan itself, and not one as provided by the loader platform. Note that this is also the reason why load on Wrappers accept anytype for the loader function, instead of a PfnGetInstanceProcAddr.

@slimsag
Copy link
Contributor Author

slimsag commented Nov 27, 2021

Ahhh, I see, that's very interesting. I didn't realize vkGetInstanceProcAddr could resolve itself, that makes sense.

Given that, I would suggest maybe some documentation around this as it seems highly likely others will fall into the same trap of casting to PfnGetInstanceProcAddr and thinking it's the right calling convention. I was suspicious enough to find the load code was compatible with any calling convention, as you already mentioned, but I'm not sure others will pay such careful attention.

@slimsag slimsag changed the title vk.PfnGetInstanceProcAddr uses incorrect calling convention document calling convention to load() better, PfnGetInstanceProcAddr may not be the right calling convention for most Nov 27, 2021
@Snektron
Copy link
Owner

Snektron commented Dec 8, 2021

Implemented in c169871

@Snektron Snektron closed this as completed Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants