nimious / vulkan

Nim bindings for Vulkan, the API for access to graphics and compute on GPUs.
MIT License
61 stars 8 forks source link

Creating swapchain gives SIGSEGV / segfault #5

Closed Clyybber closed 6 years ago

Clyybber commented 6 years ago

EDIT: There never was an issue, I was dumb enough to pass a physical device to vkCreateSwapchainKHR

If trying to create a swapchain with vkCreateSwapchainKHR(device, addr createInfo, nil, addr swapchain) where swapchain is of type VkSwapchainKHR I get SIGSEGV: Illegal Storage access. I suppose this is just me being dumb but I really can't figure out what I am doing wrong here.

Clyybber commented 6 years ago

I tried to allocate my swapchain variable manually as described here: https://nim-lang.org/docs/manual.html#types-reference-and-pointer-types but that didn't prevent the SIGSEGV either. For windows devs: SIGSEGV is a segfault.

Clyybber commented 6 years ago

Vulkan wrappers for other languages also implement it differently, for example this haskell binding: http://hackage.haskell.org/package/vulkan-2.1.0.0/docs/src/Graphics.Vulkan.Extensions.VK_KHR_swapchain.html#VkSwapchainKHR_T implements it as a pointer to VkSwapchainKHR_T, while this rust binding: https://docs.rs/vks/0.20.0/src/vks/khr_swapchain.rs.html#26 implements it as a struct to be defined by VK_DEFINE_NON_DISPATCHABLE_HANDLE.

Clyybber commented 6 years ago

So according to https://stackoverflow.com/questions/43741998/what-does-vk-define-non-dispatchable-handle-mean and https://github.com/KhronosGroup/Vulkan-Docs/blob/1.0/src/vulkan/vulkan.h#L56 , VkSwapchainKHR is a VkNonDispatchableHandle or something that is 64 bits in size. So type VkSwapchainKHR* = pointer and type VkSwapchainKHR* = VkNonDispatchableHandle should work. But they don't. I don't think its my drivers, since all the vulkan demos I've tested work. Maybe the bug is in the compiler?

Clyybber commented 6 years ago

If it helps here is the relevant code: https://gitlab.com/n0tknot/wyven/blob/master/src/engine/window.nim#L221 The last 3 lines of that function are where the relevant things happen.

gmpreussner commented 6 years ago

Hey Clyybber, thanks for looking into this - I didn't have time yet to take a look myself, but will try to do so soon!

Clyybber commented 6 years ago

In the generated C code the type VkSwapchainKHR gets translated to void*. Then we assign the swapchain variable to alloc0(8), that should be 64 bits in size. The address of that void pointer is then passed to vkCreateSwapchainKHR(). I really can't figure this out. Maybe alloc0(), alloc() or my driver is bugged. EDIT: For reference here is the generated C code for the relevant proc: https://gist.github.com/Clyybber/4aa9835769496cebc9ac5ba13568a079

gmpreussner commented 6 years ago

Hey Clybber, sorry, I still haven't had any time to look into this. Too much going on right now :/

Clyybber commented 6 years ago

No worries, thank you anyways. I wish I could fix it myself, but I can't figure it out.

gmpreussner commented 6 years ago

Had a quick look.

First, my definition of VkSwapchainKHR is incorrect. What the Vulkan API specifies is that this type must be 64-bit on all platforms and unique if possible, so on 64-bit platforms they declare it as a pointer, and on all other platforms as a 64-bit unsigned integer.

Now, the swap chain values that the KHR functions take and return are opaque handles, meaning that we don't actually have to know what they mean under the hood.... could be pointers to structs, could be index numbers into a container, could be a hash value, could be a key into a map, etc.

I'm thinking that we should get rid of the pointer representation altogether and just declare the handle as an int64. That way you are only passing around integers and don't have to worry about (platform-specific) casting and other things.

Can you give that a try?

Clyybber commented 6 years ago

Just tried that, still gives a segfault. Maybe this is an upstream bug, but I can't confirm it since I can't rule out buggy drivers(the examples and demos work though).

Clyybber commented 6 years ago

I'm dumb as fuck :D

Because I used my own function wrapping the creation of a swapchain wrongly, a physical device was passed to vkCreateSwapchainKHR instead of a logical one. The verification layers (and my brain) didnt catch that.

Sry that I have bothered you with this so long

Clyybber commented 6 years ago

Maybe we should make some of those alias types for VkHandle distinct so that such dumbness can be prevented, though on second thought prevention of that might also be the responsibility of the validation layers.

awr1 commented 6 years ago

@Clyybber make an issue at https://github.com/KhronosGroup/Vulkan-ValidationLayers ?

Clyybber commented 6 years ago

Done: https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/288