Add support for 1.1

[MaikKlein]

Vulkan 1.1 has officially been released.

It seems they didn't actually add any new functionality to the core spec, but only moved some KHX extensions into core as KHR.

I think this means that the version specific loader in ash is almost useless, if Khronos continues to only add functionality with extensions.

I think I will still add an V1_1 struct, it will just not add any new functionality.

The update should be relatively painless, although it would be really nice if vk.rs could be generated which means I am going to prioritize #7 now.

The KHX versions of these extensions are no longer supported

I think this is interesting, and I am not sure how I should handle it. I don't think ash currently exposes any KHX extensions yet, so it is not a problem currently but the question is how should ash handle it in the future?

[farnoy]

I don't think ash currently exposes any KHX extensions yet, so it is not a problem currently but the question is how should ash handle it in the future?

Maybe parameterize the implementation by revision number of the extension? So that when we go from a hypothetical ash::extensions::DeviceGroup<Revision_2> to Revision_3 (like they have with vulkan 1.1), we can adjust function names, signatures and also the static CStr name of the extension? Would it be possible for Device<V1_1> to also impl extension traits like impl DeviceGroup<Revision_3> for Device<V1_1>? Maybe that is too much hassle with a generated vk.rs, I'm probably not making any sense.

[MaikKlein]

Maybe parameterize the implementation by revision number of the extension? So that when we go from a hypothetical ash::extensions::DeviceGroup<Revision_2> to Revision_3 (like they have with vulkan 1.1), we can adjust function names, signatures and also the static CStr name of the extension? Would it be possible for Device<V1_1> to also impl extension traits like impl DeviceGroup<Revision_3> for Device<V1_1>? Maybe that is too much hassle with a generated vk.rs, I'm probably not making any sense

I just add V1_1 then V1_0 can not use those new extensions. And the other way around, I guess the instance / device creation will just fail. So I don't think it will be a problem.

I think the bigger problem is that the KHX extensions won't be in the in the vk.xml file anymore, so I can't generate them.

I guess KHX will be just for experimentation and they can be removed in ash with only a minor version update.

[MaikKlein]

Also I was wrong about the extension part, I should have looked at the spec directly. https://www.khronos.org/registry/vulkan/specs/1.1-khr-extensions/html/vkspec.html#versions-1.1

Promoted to core just means that the functions from the extensions are now normal core functions, and they will be added to the V1_1 struct. The KHR part is just an alias.

Perfect :)

[TimDiekmann]

Any news on this?

[MaikKlein]

@cyres Currently blocked on NicolBolas/New-Vulkan-XML-Format#17

[MaikKlein]

Quick update:

I started to write my own xml parser because I can't wait any longer.

After reading the xml file I discovered that there are conditional extensions, which is problematic.

For example vkGetPhysicalDevicePresentRectanglesKHR is a function in the device group but only if the surface extensions is present. This is problematic because in ash we either load everything or we fail. A possible solution would be sub-extensions but that starts to feel a bit hacky.

And now in 1.1 it even moved into the swapchain extension.

[farnoy]

@MaikKlein How does the official SDK handle this? Maybe we could expose everything in ash, but load the conditional functions lazily/on demand? To let the application developer resolve dependencies between extensions. I'm not sure if this makes sense, but if you need any help, let us know!

[MaikKlein]

@farnoy Thanks, I'll try to finish up the new generator until tomorrow. I think every other loader just silently fails to load the fp if it is not available and then crashes at runtime if it is used. I still kind of like the idea that in ash you can be sure that if you create a 1.0 context, you can't accidentally use 1.1 functions. But I'll worry about this after I finish up the generator.

[farnoy]

Hey @MaikKlein, any updates?

[MaikKlein]

Sorry, I am almost done but I couldn't finish it yet. I have a live presentation for rlsl tomorrow and I needed to implement a few things for it. I think I should be able to finish the generator branch this weekend.

[farnoy]

Of course, no worries! Thanks for the update and good luck tomorrow. Hope the presentation gets linked on /r/rust 😄

[MaikKlein]

@farnoy It wasn't recorded and I only presented it 1 on 1.

So the generator branch is mostly done, I think. I need to fix a few extensions names. I'll probably merge EntryFn and StaticFn together.

There are also a few breaking changes for some static arrays where vk-rs previously did the wrong thing.

There is the issue for platform dependent extensions. Ash currently just exposed every extension unconditionally but some extensions use types that are not simple typedefs, like the winapi and Android like AHardwareBuffer. The xml file declares it as an empty struct but maybe it would be better to use a real ffi library for that. Is there even an android ffi library that has an AHardwareBuffer? I can't seem to find one. The same goes for SECURITY_ATTRIBUTES from the winapi. It might be beneficial to feature gate those platform dependent apis. Of course the xml doesn't say which extensions are platform dependent, we would have to rely on a consistent naming scheme.

I also want to make it easier for extensions to be used even if ash doesn't expose them directly. Currently it is awkward to load the raw version manually.

The general idea is that even if ash doesn't expose a nicer interface for a function/extension you can always fall back to the raw Vulkan version.

[TimDiekmann]

Do you plan to make the raw interface available in a seperate crate?

Will it be possible to call functions, which do not require the device as parameter, without the device object? This would make it possible to use e.g. VkCommandBuffer without device and with repr[(transparent)] it adds the ability pass an array of it to a Device::queue_submit directly without the need to create a new slice from it.

[MaikKlein]

@cyres

Will it be possible to call functions, which do not require the device as parameter, without the device object?

Ah I am not sure if that would be so great. Of course can do it, and I maybe should and will, but even if the device is not an explicit parameter, it is an implicit one, so you need to be careful on which device you record the commands on and don't accidentally send it to the wrong device.

The function pointers here belong to one device, that is why they are in the device object. Of course as long as you only create one device, you should be fine.

Good idea I'll probably separate ash and ash-sys maybe. Although ash-sys would just be the generated vk.rs.

[MaikKlein]

Another small update.

Ash now successfully compiles with the new generator branch. A few extension constants are still missing. I refactored the way Ash handled enums and bitflags. Everything is now a struct with constants. Sadly this is a breaking change, but the best possible way forward.

A side effect of it is that racer can now complete bitflag variants for you.

flags: vk::COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER_BIT,
//to
flags: vk::CommandPoolCreateFlags::RESET_COMMAND_BUFFER_BIT,

See #71 for more information about why this was necessary.

The 1.1 function pointers are also working but still have to be exposed on a higher level. The last missing part is now to make the examples work with the new generator branch.

[GabrielMajeri]

Just a minor nitpick: considering all of the bit flags are within a structure namespace, SomeStructureFlags::SOME_CONSTANT, is it necessary to have the _BIT at the end? In raw C it's useful to disambiguate between enum constants and bitflags, but I feel it's now unnecessary.

[Ralith]

Another upside to the new approach is that the rustdocs will now include the constants on the type's page.

Speaking of docs, how difficult would it be to include them, too, at least for stuff like constants and types, which don't need further wrapping?

[MaikKlein]

SomeStructureFlags::SOME_CONSTANT, is it necessary to have the _BIT at the end? I

@GabrielMajeri Good catch, no it is not necessary, I just missed it.

Another upside to the new approach is that the rustdocs will now include the constants on the type's page.

Yep

Speaking of docs, how difficult would it be to include them, too, at least for stuff like constants and types, which don't need further wrapping?

I am not sure what you mean.

[Ralith]

I am not sure what you mean.

vk.xml often contains comment fields, e.g. <enum bitpos="0" name="VK_ACCESS_INDIRECT_COMMAND_READ_BIT" comment="Controls coherency of indirect command reads"/>. When available, perhaps these could be automatically transcribed into rust documentation.

Going further, the spec itself is machine readable and it might be possible to generate long-form documentation from it similar to the official manpages, but on review it looks like that would be a large project.

[MaikKlein]

@Ralith Yeah that should be possible. I am using quote / syn for generation and the whole output is on a single line so any kind of comment will comment out everything. Also there is no real support for comments at all, I am on a dated version where I could technically "hack" it with Term::intern and output /** */ doc comments.

I probably should open an issue for this one.

[Ralith]

Could you use the attribute form instead?

[MaikKlein]

@Ralith Done

Also I discovered that the comments are not as extensive as I thought. A lot of documentation is maintained in a separate json file. It should be possible to integrate, but it is not a priority at the moment.

But I will use all the available documentation that is inside the vk.xml file

[TimDiekmann]

Do you need help converting the examples? I'd do it so you can focus on implementing the extension related things.

[MaikKlein]

@cyres Yes that would be great, someone else also wanted to work on it too. I just need to fix a few naming regressions and properly expose all the extension constants. I'll open an issue when the examples can be ported.

[MaikKlein]

Currently blocked on krolli/vk-parse#3 as I can't generate all the constants for the extensions.

I could temporarily ignore these constants and hope that they are not needed right now.

[MaikKlein]

Update: Extension constants now seem to be properly generated, which means we should be able to update the examples. I'll push the changes shortly. The generator branch is a bit messy right now, there were a couple of edge cases that I didn't think about, and I definitely need to clean everything up, but the generated vk.rs file looks pretty good right now.

What is currently missing are extension constants that are not bitflags and aliases of those constants, because the vk.xml has some weird edge cases that might get fixed in KhronosGroup/Vulkan-Docs#747 but they shouldn't be required right now.

If anyone wants to port examples, just let me know, otherwise I port them myself. It should be fairly straight forward to port them, just let the compiler guide you. Exension structs now have KHR, NV etc in their name, Bitflags and constants that were previously enums are now just constants inside a struct.

As I posted above it will look like

flags: vk::COMMAND_POOL_CREATE_RESET_COMMAND_BUFFER
//to
flags: vk::CommandPoolCreateFlags::RESET_COMMAND_BUFFER

Also racer can now complete those constants, which should make it easier to refactor. If anything is missing or you encounter some weird naming issue, just let me know I am sure there are a few edge cases that I missed.

Bitflags also now contain their vendor name like ERROR_NOT_PERMITTED_EXT. This was necessary because there are some cases where the name will become ambiguous if you remove the vendor name.

After the examples have been ported, I'll start to expose 1.1 functionality.

[TimDiekmann]

As mentioned before, I'd port the samples. You may open an issue, as soon as you pushed the commits.

[MaikKlein]

@cyres Thanks, I pushed the changes into the generator branch. I ran into some git index issues with submodules but generating the vk.rs isn't necessary. Feel free to contact me if you run into an issue.

[Ralith]

Weren't we getting rid of the _BIT suffixes as redundant?

[MaikKlein]

@Ralith Where do you see a _BIT suffix ? Are you looking in the generator branch?

[Ralith]

I'm looking at the comment you posted 11 hours ago, but I'm guessing that was just copypasted from prior discussion.

[MaikKlein]

@Ralith Yes sorry I just copy pasted it from above.

I also now implemented automatic detection and codegen for Debug and Default, where it can't be automatically derived. More derives are planed.

#[repr(C)]
#[derive(Copy, Clone)]
pub struct RenderPassBeginInfo {
    pub s_type: StructureType,
    pub p_next: *const c_void,
    pub render_pass: RenderPass,
    pub framebuffer: Framebuffer,
    pub render_area: Rect2D,
    pub clear_value_count: uint32_t,
    pub p_clear_values: *const ClearValue,
}
impl ::std::fmt::Debug for RenderPassBeginInfo {
    fn fmt(&self, fmt: &mut ::std::fmt::Formatter) -> ::std::result::Result<(), ::std::fmt::Error> {
        fmt.debug_struct("RenderPassBeginInfo")
            .field("s_type", &self.s_type)
            .field("p_next", &self.p_next)
            .field("render_pass", &self.render_pass)
            .field("framebuffer", &self.framebuffer)
            .field("render_area", &self.render_area)
            .field("clear_value_count", &self.clear_value_count)
            .field("p_clear_values", &"union")
            .finish()
    }
}
impl ::std::default::Default for RenderPassBeginInfo {
    fn default() -> RenderPassBeginInfo {
        RenderPassBeginInfo {
            s_type: StructureType::RENDER_PASS_BEGIN_INFO,
            p_next: unsafe { ::std::mem::zeroed() },
            render_pass: RenderPass::default(),
            framebuffer: Framebuffer::default(),
            render_area: Rect2D::default(),
            clear_value_count: uint32_t::default(),
            p_clear_values: unsafe { ::std::mem::zeroed() },
        }
    }
}

For default pointers are currently initialized with zeroed but it should be possible to initialize them with the correct ptr::null fns, I just haven't bothered to implement it. Unions are also zeroed.

Also I wasn't sure what meaningful debug message I could output for unions.

I also now generate disabled extensions because the spec is kinda weird and some types require types from disabled extensions. I also now generate all the bitflags and enum constants, previously I missed a few because they are defined at multiple locations, don't ask me why.

[Ralith]

VK_SAMPLE_COUNT_1_BIT becoming SampleCountFlags::TYPE_1 is a bit unintuitive, though it's not obvious to me that it can be substantially improved on.

[farnoy]

We could always spell out 1 as ONE, but that may be too much 🤷‍♂️

[Ralith]

Prefixing otherwise-illegal identifiers with _ has some precedent and is more obvious/predictable, but is admittedly uglier.

[MaikKlein]

@farnoy I think it would be nice for small numbers, but maybe a bit weird for bigger ones like Thirtytwo, but it shouldn't be too hard to implement.

I feel like _ is the most consistent one. We could also prefix it with N for number.

SampleCountFlags::N1
SampleCountFlags::N2
...

[TimDiekmann]

Since the examples are ported now and I have some free time, I'd like to help you out more. Is there something you need help with?

[MaikKlein]

@cyres Thanks, help is always appreciated. I paved the way for 1.1 a few commits ago
You could add all the new functions from 1.1 for Instance and Device. You can find all the function pointers inside InstanceFnV1_1 etc.

#[allow(non_camel_case_types)]
pub trait InstanceV1_1: InstanceV1_0 {
    fn fp_v1_1(&self) -> &vk::InstanceFnV1_1;

    unsafe fn enumerate_instance_version(&self) -> VkResult<vk::uint32_t> {
        let mut api_version = 0;
        let err_code = self.fp_v1_1()
            .enumerate_instance_version(&mut api_version as *mut _);
        match err_code {
            vk::Result::SUCCESS => Ok(api_version),
            _ => Err(err_code)
        }
    }
    // Expose more stuff
}

Alternatively, I think https://github.com/krolli/vk-parse/issues could probably use a few maintainers as well. vk-parse is essential for ash, and the newest version of the vk.xml doesn't parse. Also vk-parse isn't published yet on crates.io, which I want to have before I merge the generator branch into master and publish a new version.

[TimDiekmann]

Sure, I'll start with wrapping the raw functions for Instance and Device. When exactly do you use an unsafe function?

[MaikKlein]

@cyres Almost everything is unsafe. Just mark everything unsafe, there are a few cases were it possibly can be removed.

If you want a general guideline. I think if the function is stateless like enumerate_instance_version, it shouldn't need to be an unsafe function, but if it interacts with any Vulkan objects like Image CommandBuffer etc it has to be unsafe.

Also any function that uses an AllocationCallback should also be unsafe.

Most instance level functions are probably safe, while almost every device level function is most likely unsafe.

[Ralith]

We could also prefix it with N for number.

I like this better than _, and certainly better than TYPE_. We'll need something else for keyword collisions, but maybe we'll be lucky and dodge those.

[krolli]

Alternatively, I think https://github.com/krolli/vk-parse/issues could probably use a few maintainers as well. vk-parse is essential for ash, and the newest version of the vk.xml doesn't parse. Also vk-parse isn't published yet on crates.io, which I want to have before I merge the generator branch into master and publish a new version.

Hi, vk-parse should now be available on crates.io (huge thanks to @GabrielMajeri for helping out with it). I also set-up nightly parsing of various spec versions including master to get heads-up when spec changes in unexpected ways. It's not as good as I'd like it to be, but still better than nothing.

[farnoy]

@MaikKlein I think we are missing EntryV1_1 along the new structures. There is no way to obtain an Instance<V1_1> without Entry<V1_1>, because the signature for creating an instance is

unsafe fn create_instance(
    &self, 
    create_info: &InstanceCreateInfo, 
    allocation_callbacks: Option<&AllocationCallbacks>
) -> Result<Instance<Self::Fp>, InstanceError>

EDIT: Workaround:

let instance = unsafe { entry.create_instance(&create_info, None)? };

let fp_1_1 = unsafe {
    InstanceFpV1_1::load(entry.static_fn(), instance.handle())
        .expect("failed to load 1.1 instance functions")
};

let instance_1_1 = ash::Instance::<V1_1>::from_raw(instance.handle(), fp_1_1);

[MaikKlein]

Some issues that need to be addressed before we merge into master and release a new version:

• Vulkan 1.1 getter functions don't work #109 pnext for output data is broken at the moment.

• Debug printing tries to convert all arrays to CStr #116 Some debug output is broken right now. This should be easy to fix

• Initializing extern fn with 'zeroed' results in a SIGIL #112 SIGIL for some default implementation because some types are not allowed to be zeroed.

• All functions that use any kind of handle should be marked unsafe #94 We should mark more functions as unsafe. Some invariants have been broken and more functions should be marked unsafe. It is okay to accidentally mark more functions unsafe then necessary.

• [RFC] Error handling #41 I would also like to rework the error handling, at least for the functions that are affected. For example we return an Err for Swapchain::SUBOPTIMAL_KHR, which is just wrong.

• Move vk.rs to new vulkan-sys crate #93 and finally split ash into ash / ash-sys.

Some things that are missing:

IIRC some constants like extension names are still missing, I'll open an issue later.

There are probably a lot of missing derived traits. It is just really annoying to detect which traits should be derived for which structs without any type information. I think we only derive #[derive(Copy, Clone, Default, Debug)], and we can manually override it for certain structs. For example Extent3D also implements #[derive(Copy, Clone, Default, Debug, PartialEq, Eq, Hash)]. I probably should also open an issue for this.

I think I should open a milestone.

I just want to thank all of you for supporting ash. Every contribution is greatly appreciated.

[MaikKlein]

I decided to release a new version on crates.io. As most of you probably know, there were some breaking changes, I documented hopefully most of them in the changelog.

If you encounter any problems, please open an issue.