SDL 2.0.6 and Vulkan

[Rua]

SDL 2.0.6 added a bunch of features to allow SDL to interface with Vulkan, but these are currently missing from sdl2_sys because it's still based on 2.0.5. Will the SDL version used in the crate be bumped up to 2.0.6 anytime soon? I'm interested in using SDL with Vulkan in Rust but I don't know how I can do this with the current state of things.

Furthermore, once this has been done, is anyone working on including Rustified versions of these functions, to be added to the sdl2 crate proper?

[Cobrand]

It is up to date on master, but it has not been pushed on crates.io yet (I am waiting for #781 to pass). You can add rust-sdl2's master branch as a dependency like so:

[dependencies.sdl2]
    git = "https://github.com/rust-sdl2/rust-sdl2"

Nobody is working on functions for Vulkan bindings in sdl2 I'm afraid, but PRs are always welcome!

[Rua]

I could certainly give a shot at Vulkan bindings. However, at the moment I'm having trouble generating an updated sdl_bindings.rs that includes the raw C SDL functions for Vulkan. When I clone the git version and run cargo build --features use-bindgen for sdl2-sys the file isn't actually being generated. There is no error message, just no new file. It's possible the file is being placed somewhere else where I can't find it.

[Rua]

Indeed it was placed somewhere else, but I found it now. I'll let you know if I have anything new.

[Cobrand]

The vulkan bindings aren't amongst the pre-generated bindings because we never include "SDL_vulkan.h".

Perhaps we could add it to wrapper.h to generate the bindings automatically? I'm sure sure if there is a downside to add vulkan directly without feature gate, but my first guess would be that it should be fine.

You could try to add SDL_vulkan.h to https://github.com/Rust-SDL2/rust-sdl2/blob/master/sdl2-sys/wrapper.h and see what you got for there.

[Rua]

I've finished implementing the SDL Vulkan functions in Rust, but I still need to test them. However, I hit a bit of a problem.

The SDL_Vulkan_CreateSurface function has VkInstance and VkSurfaceKHR as parameters, which are Vulkan types that are also defined in SDL_vulkan.h. VkInstance is a pointer to some opaque type, while VkSurfaceKHR is either a pointer to an opaque type on 64-bit platforms, or a 64-bit integer on other platforms. The bindings generated by bindgen are:

#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct VkInstance_T {
    _unused: [u8; 0],
}
pub type VkInstance = *mut VkInstance_T;
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct VkSurfaceKHR_T {
    _unused: [u8; 0],
}
pub type VkSurfaceKHR = *mut VkSurfaceKHR_T;

These definitions do not match Vulkan's own definition, because VkSurfaceKHR should be the size of u64 (always 64 bits) while it is defined here as being the size of a pointer (32 bits on some platforms).

To make it more confusing, different Rust Vulkan libraries have different definitions of these handle types. The popular Vulkano library for Rust defines them directly as usize and u64 respectively. Other Rust Vulkan libraries all have their own definitions of these handles, such as structs containing pointers or integers, etc. I'm not aware of any universal "standard" definition.

This makes it rather ugly and fiddly to interface SDL with these other libraries. Treating them simply as integers of a certain size, like Vulkano does, may be the best option, because these really should be considered completely opaque values that have no internal structure whatsoever. On some platforms, VkSurfaceKHR isn't even a pointer. But no matter what we choose, there will be a mismatch with some libraries, requiring pointer casts and other unsafe ugliness.

[Cobrand]

#if defined(__LP64__) || defined(_WIN64) || defined(__x86_64__) || defined(_M_X64) || defined(__ia64) || defined (_M_IA64) || defined(__aarch64__) || defined(__powerpc64__)
#define VK_DEFINE_NON_DISPATCHABLE_HANDLE(object) typedef struct object##_T *object;
#else
#define VK_DEFINE_NON_DISPATCHABLE_HANDLE(object) typedef uint64_t object;
#endif

From what I understand this macro defines a struct of size 64bits every time: if the arch supports 64bits, use a pointer size, otherwise use a u64 -- so it should be a u64 every time?

#define VK_DEFINE_HANDLE(object) typedef struct object##_T* object;

This is definitely the size of a pointer

VK_DEFINE_HANDLE(VkInstance)
VK_DEFINE_NON_DISPATCHABLE_HANDLE(VkSurfaceKHR)

typedef VkInstance SDL_vulkanInstance;
typedef VkSurfaceKHR SDL_vulkanSurface;

So SDL_vulkanInstance has the size of a pointer and SDL_vulkanSurface has the size of a u64. THe implementation of vulkano looks good to me.

If you need help on how to shadow/replace types directly in bindgen, I'm no specialist and can only redirect you to the official documentation. To be honest I understood your issue but I did not quite understand your question if there was one, sorry if I didn't hit the mark.

[Rua]

I've submitted a pull request for Vulkan support.

There is still a bug with the generated sdl_bindings.rs, which I mentioned above. The generated file differs depending on whether you generate on a 32-bit or 64-bit system. On a 64-bit system, like mine, it defines VkSurfaceKHR as a pointer to some struct. When generating on a 32-bit system, it becomes a u64. This means that if sdl_bindings.rs is generated on a 64-bit system, it will give an invalid type when used on a 32-bit system because pointers are only 32 bits there. Presumably the upper 32 bits of the handle are just discarded after calling the native C SDL_Vulkan_CreateSurface function when passing the values to Rust.

[Cobrand]

This means that if sdl_bindings.rs is generated on a 64-bit system, it will give an invalid type when used on a 32-bit system because pointers are only 32 bits there

@Rua the alternative would be to panic at compile-time if we try to compile to a target that is 32bits when the generated bindings were for 64bits (and reverse). I'm not sure on how to do that, but we can probably find some build.rs shenanigans to help us do exactly that.

[Rua]

I'm not sure how to do it either I'm afraid. I'm still fairly new to Rust.

At the very least, the bindings should be generated on a 32-bit system because then they will be valid for 64-bit systems as well (I think?). The readme should warn that the bindings generated on 64-bit systems don't work for 32-bit systems, but if the bindings that are supplied with the library are right, then this only applies for people who want to regenerate the bindings themselves anyway.

[Cobrand]

Jus to give a hint, you could add a typedef in the wrapper.h that would simply be a void*, and then in the build.rs, if sizeof<wrapper_void_ptr> is not the same as sizeof<const* ()> then panic with an error? I'm pretty sure this would cover almost all cases.

…

On Jul 12, 2018, 16:29, at 16:29, Rua ***@***.***> wrote: I'm not sure how to do it either I'm afraid. I'm still fairly new to Rust. At the very least, the bindings should be generated on a 32-bit system because then they will be valid for 64-bit systems as well (I think?). The readme should warn that the bindings generated on 64-bit systems don't work for 32-bit systems, but if the bindings that are supplied with the library are right, then this only applies for people who want to regenerate the bindings themselves anyway. -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: #784 (comment)

[Rua]

I had a look at the bindgen documentation and managed to make it generate the right types. It still generates useless VkInstance_T and VkSurfaceKHR_T structs, but at least they don't get in the way as long as nobody uses them.