Gfx: Clean up Vulkan bindings usage

[kvark]

Currently, we are using a modified vk-sys from https://github.com/sectopod/vulkano. This is hacky, slows down CI (for the need to fetch vulkano sources), and could be refactored.
One option is to use vk-sys from crates.io and work around the issues we have, maybe submitting a few PRs upstream.
Another option is to use @MaikKlein 's https://crates.io/crates/ash

[MaikKlein]

If you have any questions or concerns about Ash, feel free to ask. I would love to get some real world usage reports.

[MaikKlein]

The only big subjective feature is most likely the loader.

I have only recently implemented a version specific loader and I can't yet say how stable it is. But it pretty much gets rid of any run time initialization errors and it removes the guess work which functions are actually supported.

If you decide not to use Ash you would probably still have to implement your own loader. For example one day Vulkan 1.1 might be released and it comes with some nice new functionality that you want to make us of. But of course you still want to maintain backwards compatibility for every device that doesn't support version 1.1.

You still have to do that in Ash too but Ash gives you some foundation to build upon. The way I imagine it is that you would have two different Vulkan backends. Vulkan 1.0, Vulkan 1.1. And you can easily share code because Ash exposes traits.

fn do_something_with_a_device<Device: DeviceV1_0>(device: &Device){}

For example this function can be used in every Vulkan backend.

Also I didn't expose the whole spec yet, I only wrap the functions that I currently need in my own project. But feel free to open an issue if something is not exposed or directly make a PR as it should be fairly trivial to implement.

In the end you have to decide what is best for you.

Edit:

I had a quick look at your current Vulkan implementation. This is what you would have to do if you would refactor with Ash.

Types shouldn't cause any breakage as they are have the same name, so for example https://github.com/gfx-rs/gfx/blob/master/src/backend/vulkan/src/native.rs shouldn't require any change at all.

The loader can be removed completely https://github.com/gfx-rs/gfx/blob/master/src/backend/vulkan/src/lib.rs#L77 Every instance / device will manage its own function pointers for you.

https://github.com/gfx-rs/gfx/blob/master/src/backend/vulkan/src/lib.rs#L193

    let physical_devices = {
        let mut num = 0;
        assert_eq!(vk::SUCCESS, unsafe {
            inst_pointers.EnumeratePhysicalDevices(instance, &mut num, ptr::null_mut())
        });
        let mut devices = Vec::with_capacity(num as usize);
        assert_eq!(vk::SUCCESS, unsafe {
            inst_pointers.EnumeratePhysicalDevices(instance, &mut num, devices.as_mut_ptr())
        });
        unsafe { devices.set_len(num as usize); }
        devices
    };

This would become

let physical_devices = instance.enumerate_physical_devices().expect("something");

https://github.com/gfx-rs/gfx/blob/master/src/backend/vulkan/src/lib.rs#L255

        let mut out = 0;
        assert_eq!(vk::SUCCESS, unsafe {
            inst_pointers.CreateDevice(dev.device, &dev_info, ptr::null(), &mut out)
        });
        out

Would become

unsafe{ instance.create_device(dev.device, &dev, None).expect("Device failed to create") }

In general all functions will directly be called on instance / device instead of the function pointers. Also every function is snake_case.

https://github.com/gfx-rs/gfx/blob/master/src/backend/vulkan/src/lib.rs#L289 can also be removed because error codes in Ash are an enum https://github.com/MaikKlein/ash/blob/master/src/vk.rs#L2634

https://github.com/gfx-rs/gfx/blob/master/src/backend/vulkan/src/data.rs#L42
(Kind::D2Array(..), None) => Ok(vk::IMAGE_VIEW_TYPE_2D_ARRAY),

would become

(Kind::D2Array(..), None) => Ok(vk::ImageViewType::Type2dArray),

I just randomly picked out a few things, this should give you a better view on how much effort it will require to do the refactoring.

[kvark]

@MaikKlein really appreciate you looking into our (incomplete) Vulkan implementation!

One thing I remember of a concern - the reason we can't use upstream vk-sys is because we need some traits implemented for some of the Vulkan structs, i.e. Hash/Clone for vk::Extent2D (out of my head). Do you have stuff derived on those?

[MaikKlein]

Pretty much everything implements clone, https://github.com/MaikKlein/ash/blob/master/src/vk.rs#L1600 but Hash isn't currently derived but I don't see any reason why it shouldn't be.

I started with https://github.com/Osspial/vk-rs which generated the vk.rs file for me but it is now heavily modified. I just haven't looked at all the derives yet. I'll add Hash to all data structures where it makes sense tomorrow.

I also really need to clean up some stuff that doesn't need to be there at all.

[torkleyy]

Why are you using this fork and not the upstream vulkano?

[minecrawler]

Vulkano is more than just the Vulkan API for Rust; it is a safe wrapper over that low-level API which is easy to miss-use. Why not stay with the safe wrapper instead of trying to re-invent all the security and benefits?

[Bastacyclop]

In my opinion, we don't use vulkano for the Vulkan backend for the same reason that we don't use glium for the OpenGL backend. Those libraries have their own design and using them in GFX would force compromises, and cripple the design freedom of GFX.

[kvark]

@torkleyy and @minecrawler
Basically what @Bastacyclop said, plus less dependencies from external authors. @tomaka is not doing anything with gfx-rs, neither does he have interest in it.

[msiglreith]

We employ now ash for our vulkan and vulkanll backends.