ClearValue is described as "Temporary Hard-Coded union hack; will be automatically generated when actual unions become stable" here: https://docs.rs/ash/0.21.0/ash/vk/types/struct.ClearValue.html

Unions are stable now, I believe, so can this be fixed?

Thanks a lot for ash btw, it's pretty darn great.

Blocks gfx-rs/gfx#1737

These submodules prevents cargo to download the crate (for testing):

error: failed to load source for a dependency on `ash`

Caused by:
  Unable to update https://github.com/MaikKlein/ash/?branch=generator

Caused by:
  failed to update submodule `generator/New-Vulkan-XML-Format`

Caused by:
  no URL configured for submodule 'generator/New-Vulkan-XML-Format'; class=Submodule (17)

e.g. replace ash::extensions::Swapchain::name with ...::NAME being a constant of type &'static str. Simple consistency/clarity matter. I'd happily do this myself but, as always, I don't want to step on toes regarding/make changes mooted by the pending rewrite.

This value is represented in Rust as an enum, but vulkan can provide values that ash doesn't expect. Worse, ash does not sanity-check them, leading to UB when matching (for example, in the derived Debug implementation).

Current ash defines a continuous range of values from 0 to 28 inclusive. By contrast, my vulkan 1.0.61 header defines these additional values:

typedef enum VkDebugReportObjectTypeEXT {
    // ...
    VK_DEBUG_REPORT_OBJECT_TYPE_DISPLAY_KHR_EXT = 29,
    VK_DEBUG_REPORT_OBJECT_TYPE_DISPLAY_MODE_KHR_EXT = 30,
    VK_DEBUG_REPORT_OBJECT_TYPE_OBJECT_TABLE_NVX_EXT = 31,
    VK_DEBUG_REPORT_OBJECT_TYPE_INDIRECT_COMMANDS_LAYOUT_NVX_EXT = 32,
    VK_DEBUG_REPORT_OBJECT_TYPE_VALIDATION_CACHE_EXT = 33,
    VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_UPDATE_TEMPLATE_KHR_EXT = 1000085000,
    VK_DEBUG_REPORT_OBJECT_TYPE_SAMPLER_YCBCR_CONVERSION_KHR_EXT = 1000156000,
    VK_DEBUG_REPORT_OBJECT_TYPE_BEGIN_RANGE_EXT = VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT,
    VK_DEBUG_REPORT_OBJECT_TYPE_END_RANGE_EXT = VK_DEBUG_REPORT_OBJECT_TYPE_VALIDATION_CACHE_EXT,
    VK_DEBUG_REPORT_OBJECT_TYPE_RANGE_SIZE_EXT = (VK_DEBUG_REPORT_OBJECT_TYPE_VALIDATION_CACHE_EXT - VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT + 1),
    VK_DEBUG_REPORT_OBJECT_TYPE_MAX_ENUM_EXT = 0x7FFFFFFF
} VkDebugReportObjectTypeEXT;

Given that ash does not constrain the set of possible extensions enabled at runtime (and indeed doing so would be a severe limitation) I believe an enum is fundamentally inappropriate here, as even the application may not be statically aware of all possible values.

It's probably worth revisiting all other uses of rust enums where the vulkan API instead uses an integer constant to check for similar issues.

There are two types of enumsVk -> Ash and Ash -> Vk. Only the enums that come out of Vulkan are problematic, like Result.

The reason for this is that the Vulkan spec is not completely strict and sends out new values, even if the created vulkan version doesn't support them.

So if you create a 1.0 context, you might get values for 1.1. Because those values are represented as enums, they will just crash.

First problematic enum

    #[repr(C)]
    #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
    pub enum DebugReportObjectTypeEXT {
        Unknown = 0,
        Instance = 1,
        PhysicalDevice = 2,
        Device = 3,
        Queue = 4,
        Semaphore = 5,
        CommandBuffer = 6,
        Fence = 7,
        DeviceMemory = 8,
        Buffer = 9,
        Image = 10,
        Ent = 11,
        QueryPool = 12,
        BufferView = 13,
        ImageView = 14,
        ShaderModule = 15,
        PipelineCache = 16,
        PipelineLayout = 17,
        RenderPass = 18,
        Pipeline = 19,
        DescriptorSetLayout = 20,
        Sampler = 21,
        DescriptorPool = 22,
        DescriptorSet = 23,
        Framebuffer = 24,
        CommandPool = 25,
        SurfaceKhr = 26,
        SwapchainKhr = 27,
        DebugReport = 28,
    } 

Which for example appears directly in a callback

    pub type PFN_vkDebugReportCallbackEXT = unsafe extern "system" fn(
        DebugReportFlagsEXT,
        DebugReportObjectTypeEXT,
        uint64_t,
        size_t,
        int32_t,
        *const c_char,
        *const c_char,
        *mut c_void,
    ) -> Bool32;

This will just straight up crash, if an unknown value comes in.

This is how it looked in 1.0. https://vulkan.lunarg.com/doc/view/1.0.30.0/linux/vkspec.chunked/ch32s01.html#VkDebugReportObjectTypeEXT

and now

https://github.com/KhronosGroup/Vulkan-Headers/blob/master/include/vulkan/vulkan_core.h#L5855

A quick search indicates that enum constants in C are of type i32, so casting a value that is outside of the range of the enum seems to be allowed in C.

How should this be handled in Ash? Reprenting them as enum will cause a crash.

We could simply typedef to a primitive type.

pub type Result = i32`

We could technically generate custom error types for every function because all possible error codes are included in the vk.xml file

Command {
    name: "vkAllocateDescriptorSets",
    ....
    errorcodes: Some(
            "VK_ERROR_OUT_OF_HOST_MEMORY,VK_ERROR_OUT_OF_DEVICE_MEMORY,VK_ERROR_FRAGMENTED_POOL,VK_ERROR_OUT_OF_POOL_MEMORY"
        )
    },
    ....
}

So instead of

    unsafe fn allocate_descriptor_sets(
        &self,
        create_info: &vk::DescriptorSetAllocateInfo,
    ) -> VkResult<Vec<vk::DescriptorSet>> {

We could have

    unsafe fn allocate_descriptor_sets(
        &self,
        create_info: &vk::DescriptorSetAllocateInfo,
    ) -> Result<Vec<vk::DescriptorSet>, AllocateDescriptorSetsError> {..}

    pub enum AllocateDescriptorSetsError {
            OutOfHostMemory,
            OutOfDeviceMemory,
            ...
     }

That would be super ergonomic, but I just know that the Vulkan spec will break this in the future. For example a future version might add more error codes, and then Ash would crash again, so the semantics would need to change to something like

    unsafe fn allocate_descriptor_sets(
        &self,
        create_info: &vk::DescriptorSetAllocateInfo,
    ) -> Result<Vec<vk::DescriptorSet>, Option<AllocateDescriptorSetsError>> {..}

Instead of using enums we could simply do

pub struct AllocateDescriptorSetsError(pub i32);
impl AllocateDescriptorSetsError {
    pub fn out_of_host_memory(self) -> bool {..}
    ...
}

And the same thing can be done to DebugReportObjectTypeEXT and every other enum that comes from Vulkan to Ash.

Of course here custom errors per function here is optional. We can still do

pub struct Result(pub i32);
impl Result {
    pub fn out_of_device_memory(self) -> bool {..}
    // other cases
}
unsafe fn allocate_descriptor_sets(
    &self,
    create_info: &vk::DescriptorSetAllocateInfo,
) -> Result<Vec<vk::DescriptorSet>, vk::Result> {..}

Hello!

First of all, thank you very much for undertaking this project. C++ kinda pisses me off these day, so I just decided to give rust a chance for my vulkan engine :p

Now, on to the matter:
Running both samples gave me a black screen, so I did a renderdoc capture. I found out that all 3 vertices for the triangle had the same position/color.

The culprit is here:

        let mut vert_align = Align::new(vert_ptr,
                                        vertex_input_buffer_memory_req.alignment,
                                        vertex_input_buffer_memory_req.size);

    pub unsafe fn new(ptr: *mut (), alignment: vk::DeviceSize, size: vk::DeviceSize) -> Self {
        let padding = calc_padding(size_of::<T>() as vk::DeviceSize, alignment);
        let elem_size = size_of::<T>() as vk::DeviceSize + padding;

On my system, vertex_input_buffer_memory_req.alignment == 256 as well as vertex_input_buffer_memory_req.size. Hence, it would copy only 1 element.

Alignment is supposed to be for the buffer offset (in case the buffer is not bound to device memory with an offset of 0. With my driver, buffer has to be aligned on 256 bytes boundary). Alignment of individual elements inside the buffer is the stride (which is, in this case, specified by the vertex binding description as mem::size_of::<Vertex>() == 32).

I fixed it on my side by removing + padding, and voila! Triangle (and texture) appeared.

I'm not making a pull request for this as I'm not sure if the issue is the implementation of Align itself, or using Align to copy the vertices; that depends on what else you use Align for, and the intended behavior/design.

On a side note, this is just my personal taste here, but I don't like much the vk::Format::R8g8b8a8Unorm for formats (as the first component is UP, but others are low). I prefer the way they are defined in vulkan.hpp (vk::Format::eR8G8B8A8Unorm). I think it would make more sense to keep the same notation here (including the e) for copy pasting.

Cheers!

enumflags

    #[repr(C)]
    pub struct DebugReportCallbackCreateInfoEXT {
        pub s_type: StructureType,
        pub p_next: *const c_void,
        pub flags: DebugReportFlagsEXT,
        pub pfn_callback: PFN_vkDebugReportCallbackEXT,
        pub p_user_data: *mut c_void,
    }
    
    // would become

    #[repr(C)]
    pub struct DebugReportCallbackCreateInfoEXT {
        pub s_type: StructureType,
        pub p_next: *const c_void,
        pub flags: BitFlags<DebugReport>,
        pub pfn_callback: PFN_vkDebugReportCallbackEXT,
        pub p_user_data: *mut c_void,
    }

    pub const DEBUG_REPORT_INFORMATION_BIT_EXT: DebugReportFlagsEXT =
        DebugReportFlagsEXT { flags: 0b1 };
    pub const DEBUG_REPORT_WARNING_BIT_EXT: DebugReportFlagsEXT =
        DebugReportFlagsEXT { flags: 0b10 };
    pub const DEBUG_REPORT_PERFORMANCE_WARNING_BIT_EXT: DebugReportFlagsEXT =
        DebugReportFlagsEXT { flags: 0b100 };
    pub const DEBUG_REPORT_ERROR_BIT_EXT: DebugReportFlagsEXT =
        DebugReportFlagsEXT { flags: 0b1000 };
    pub const DEBUG_REPORT_DEBUG_BIT_EXT: DebugReportFlagsEXT =
        DebugReportFlagsEXT { flags: 0b10000 };
    vk_bitflags_wrapped!(DebugReportFlagsEXT, 0b11111, Flags);

    // would become 

    #[derive(EnumFlags)]
    pub enum DebugReport{
        InformationBitExt = 0b1,
        WarningBitExt = 0b10,
        ....
     }

This allows you to write

DebugReportCallbackCreateInfoEXT{
    flags: DebugReport::WarningBitExt | DebugReport::PerformanceBitExt
    ....

Which yields better auto completion support, as you can just write DebugReport::. And it would be more consistent because every constant in Ash is currently an enum like vk::StructureType for example.

What do you think?

The recent introduction of NonZeroU64 and NonZeroUsize represents an opportunity to make null handles statically impossible and accurately represent optional handles in the API.

I'll make an initial attempt to see how this works.

PFN_vkDebugReportCallbackEXT expects the last parameter to be a mutable pointer. Currently, it's expecting p_user_data: *const c_void

error[E0308]: mismatched types
   --> /home/thomas/.cargo/registry/src/github.com-1ecc6299db9ec823/ash-0.15.8/src/vk.rs:516:49
    |
516 |                        &unsafe { CStr::from_ptr(&self.device_name[0]) })
    |                                                 ^^^^^^^^^^^^^^^^^^^^ expected u8, found i8
    |
    = note: expected type `*const u8`
               found type `&i8`

error[E0308]: mismatched types
    --> /home/thomas/.cargo/registry/src/github.com-1ecc6299db9ec823/ash-0.15.8/src/vk.rs:1126:49
     |
1126 |                        &unsafe { CStr::from_ptr(&self.extension_name[0]) })
     |                                                 ^^^^^^^^^^^^^^^^^^^^^^^ expected u8, found i8
     |
     = note: expected type `*const u8`
                found type `&i8`

error[E0308]: mismatched types
    --> /home/thomas/.cargo/registry/src/github.com-1ecc6299db9ec823/ash-0.15.8/src/vk.rs:1172:49
     |
1172 |                        &unsafe { CStr::from_ptr(&self.layer_name[0]) })
     |                                                 ^^^^^^^^^^^^^^^^^^^ expected u8, found i8
     |
     = note: expected type `*const u8`
                found type `&i8`

error[E0308]: mismatched types
    --> /home/thomas/.cargo/registry/src/github.com-1ecc6299db9ec823/ash-0.15.8/src/vk.rs:1176:49
     |
1176 |                        &unsafe { CStr::from_ptr(&self.description[0]) })
     |                                                 ^^^^^^^^^^^^^^^^^^^^ expected u8, found i8
     |
     = note: expected type `*const u8`
                found type `&i8`

error[E0308]: mismatched types
  --> /home/thomas/.cargo/registry/src/github.com-1ecc6299db9ec823/ash-0.15.8/src/extensions/swapchain.rs:21:84
   |
21 |             unsafe { mem::transmute(instance.get_device_proc_addr(device.handle(), name.as_ptr())) }
   |                                                                                    ^^^^^^^^^^^^^ expected i8, found u8
   |
   = note: expected type `*const i8`
              found type `*const u8`
   = help: here are some functions which might fulfill your needs:
           - .offset(...)
           - .wrapping_offset(...)

error[E0308]: mismatched types
  --> /home/thomas/.cargo/registry/src/github.com-1ecc6299db9ec823/ash-0.15.8/src/extensions/display_swapchain.rs:20:84
   |
20 |             unsafe { mem::transmute(instance.get_device_proc_addr(device.handle(), name.as_ptr())) }
   |                                                                                    ^^^^^^^^^^^^^ expected i8, found u8
   |
   = note: expected type `*const i8`
              found type `*const u8`
   = help: here are some functions which might fulfill your needs:
           - .offset(...)
           - .wrapping_offset(...)

error[E0308]: mismatched types
  --> /home/thomas/.cargo/registry/src/github.com-1ecc6299db9ec823/ash-0.15.8/src/extensions/surface.rs:22:80
   |
22 |                 mem::transmute(entry.get_instance_proc_addr(instance.handle(), name.as_ptr()))
   |                                                                                ^^^^^^^^^^^^^ expected i8, found u8
   |
   = note: expected type `*const i8`
              found type `*const u8`
   = help: here are some functions which might fulfill your needs:
           - .offset(...)
           - .wrapping_offset(...)

error[E0308]: mismatched types
  --> /home/thomas/.cargo/registry/src/github.com-1ecc6299db9ec823/ash-0.15.8/src/extensions/xlib_surface.rs:21:80
   |
21 |                 mem::transmute(entry.get_instance_proc_addr(instance.handle(), name.as_ptr()))
   |                                                                                ^^^^^^^^^^^^^ expected i8, found u8
   |
   = note: expected type `*const i8`
              found type `*const u8`
   = help: here are some functions which might fulfill your needs:
           - .offset(...)
           - .wrapping_offset(...)

error[E0308]: mismatched types
  --> /home/thomas/.cargo/registry/src/github.com-1ecc6299db9ec823/ash-0.15.8/src/extensions/win32_surface.rs:21:80
   |
21 |                 mem::transmute(entry.get_instance_proc_addr(instance.handle(), name.as_ptr()))
   |                                                                                ^^^^^^^^^^^^^ expected i8, found u8
   |
   = note: expected type `*const i8`
              found type `*const u8`
   = help: here are some functions which might fulfill your needs:
           - .offset(...)
           - .wrapping_offset(...)

error[E0308]: mismatched types
  --> /home/thomas/.cargo/registry/src/github.com-1ecc6299db9ec823/ash-0.15.8/src/extensions/debug_report.rs:21:80
   |
21 |                 mem::transmute(entry.get_instance_proc_addr(instance.handle(), name.as_ptr()))
   |                                                                                ^^^^^^^^^^^^^ expected i8, found u8
   |
   = note: expected type `*const i8`
              found type `*const u8`
   = help: here are some functions which might fulfill your needs:
           - .offset(...)
           - .wrapping_offset(...)

error[E0308]: mismatched types
  --> /home/thomas/.cargo/registry/src/github.com-1ecc6299db9ec823/ash-0.15.8/src/extensions/mir_surface.rs:21:80
   |
21 |                 mem::transmute(entry.get_instance_proc_addr(instance.handle(), name.as_ptr()))
   |                                                                                ^^^^^^^^^^^^^ expected i8, found u8
   |
   = note: expected type `*const i8`
              found type `*const u8`
   = help: here are some functions which might fulfill your needs:
           - .offset(...)
           - .wrapping_offset(...)

error[E0308]: mismatched types
  --> /home/thomas/.cargo/registry/src/github.com-1ecc6299db9ec823/ash-0.15.8/src/extensions/android_surface.rs:21:80
   |
21 |                 mem::transmute(entry.get_instance_proc_addr(instance.handle(), name.as_ptr()))
   |                                                                                ^^^^^^^^^^^^^ expected i8, found u8
   |
   = note: expected type `*const i8`
              found type `*const u8`
   = help: here are some functions which might fulfill your needs:
           - .offset(...)
           - .wrapping_offset(...)

error[E0308]: mismatched types
  --> /home/thomas/.cargo/registry/src/github.com-1ecc6299db9ec823/ash-0.15.8/src/extensions/wayland_surface.rs:21:80
   |
21 |                 mem::transmute(entry.get_instance_proc_addr(instance.handle(), name.as_ptr()))
   |                                                                                ^^^^^^^^^^^^^ expected i8, found u8
   |
   = note: expected type `*const i8`
              found type `*const u8`
   = help: here are some functions which might fulfill your needs:
           - .offset(...)
           - .wrapping_offset(...)

error[E0308]: mismatched types
  --> /home/thomas/.cargo/registry/src/github.com-1ecc6299db9ec823/ash-0.15.8/src/extensions/xcb_surface.rs:21:80
   |
21 |                 mem::transmute(entry.get_instance_proc_addr(instance.handle(), name.as_ptr()))
   |                                                                                ^^^^^^^^^^^^^ expected i8, found u8
   |
   = note: expected type `*const i8`
              found type `*const u8`
   = help: here are some functions which might fulfill your needs:
           - .offset(...)
           - .wrapping_offset(...)

error[E0308]: mismatched types
  --> /home/thomas/.cargo/registry/src/github.com-1ecc6299db9ec823/ash-0.15.8/src/version.rs:39:83
   |
39 |             mem::transmute(static_fn.get_instance_proc_addr(vk::Instance::null(), name.as_ptr()))
   |                                                                                   ^^^^^^^^^^^^^ expected i8, found u8
   |
   = note: expected type `*const i8`
              found type `*const u8`
   = help: here are some functions which might fulfill your needs:
           - .offset(...)
           - .wrapping_offset(...)

error[E0308]: mismatched types
  --> /home/thomas/.cargo/registry/src/github.com-1ecc6299db9ec823/ash-0.15.8/src/version.rs:68:69
   |
68 |             mem::transmute(instance_fn.get_device_proc_addr(device, name.as_ptr()))
   |                                                                     ^^^^^^^^^^^^^ expected i8, found u8
   |
   = note: expected type `*const i8`
              found type `*const u8`
   = help: here are some functions which might fulfill your needs:
           - .offset(...)
           - .wrapping_offset(...)

error[E0308]: mismatched types
  --> /home/thomas/.cargo/registry/src/github.com-1ecc6299db9ec823/ash-0.15.8/src/version.rs:82:71
   |
82 |             mem::transmute(static_fn.get_instance_proc_addr(instance, name.as_ptr()))
   |                                                                       ^^^^^^^^^^^^^ expected i8, found u8
   |
   = note: expected type `*const i8`
              found type `*const u8`
   = help: here are some functions which might fulfill your needs:
           - .offset(...)
           - .wrapping_offset(...)

It seems Android works with different signs (i8 instead of u8).

There are function which convert a successful return code into an error.
For example acquire_next_image may return VK_SUBOPTIMAL_KHR among other codes, indicating that the return index could still be used but the swapchain doesn't match the surface anymore. In the current API, this would return an error.

My suggestion would be to return a tuple of (ReturnType, SuccessCode) in these cases.

https://github.com/MaikKlein/ash/blob/master/ash/src/device.rs#L572

I implemented it with std::slice::from_raw_parts_mut but this will not use the correct alignment. It will most likely just use the default alignment of 4bytes, but the alignment changes at runtime see minUniformBufferOffsetAlignment and minMemoryMapAlignment for example.

Currently a device looks like this

#[derive(Clone)]
pub struct Device<V: FunctionPointers> {
    handle: vk::Device,
    device_fn: V::DeviceFp,
}

The reason behind including the device function pointers in the device itself was that they logically belong together. You are no allowed to call a device level function with a different device afaik.

Currently the function pointers are stored with out any indirection and this makes cloning rather expensive. It should definitely be placed behind a pointer, maybe an Arc or an unsafe pointer. Although in practice this shouldn't be a problem at all. You will most likely end up with something like this

struct Context{
    device: Device,
    instance: Instace,
    swapchain: Swapchain
}

which you also probably put behind an Arc.

This also raises a question if the same pattern should also be applied to other types as well. For example extensions like Swapchain, or handles like PhyiscalDevice. Or if we should decouple the function pointers from the device, like we do with the extensions. I think coupling would make it more weird to do proper versioning.

So for example instead of

let swapchain_loader = SwapchainLoader::new();
let swapchain = swapchain_loader.create_swapchain();
let images = swapchain_loader.get_swapchain_images_khr(swapchain);

// to 

let swapchain = Swapchain::create_swapchain(&device,...);
let images = swapchain.get_swapchain_images_khr();

Applying this to other types like PhysicalDevice would also require versioning. PhysicalDevice<V1_0>

We should also think about future versions of Vulkan, especially about the new multi gpu extensions, like a DeviceGroup

Global constants have a VK_ prefix. This should be removed:

• VK_MAX_PHYSICAL_DEVICE_NAME_SIZE
• VK_UUID_SIZE
• VK_LUID_SIZE
• VK_MAX_EXTENSION_NAME_SIZE
• VK_MAX_DESCRIPTION_SIZE
• VK_MAX_MEMORY_TYPES
• VK_MAX_MEMORY_HEAPS
• VK_LOD_CLAMP_NONE
• VK_REMAINING_MIP_LEVELS
• VK_REMAINING_ARRAY_LAYERS
• VK_WHOLE_SIZE
• VK_ATTACHMENT_UNUSED
• VK_TRUE
• VK_FALSE
• VK_QUEUE_FAMILY_IGNORED
• VK_QUEUE_FAMILY_EXTERNAL
• VK_QUEUE_FAMILY_FOREIGN_EXT
• VK_SUBPASS_EXTERNAL
• VK_MAX_DEVICE_GROUP_SIZE

See PresentModeKHR:

impl PresentModeKHR {
    pub const PRESENT_MODE_IMMEDIATE_KHR: Self = PresentModeKHR(0);
    pub const PRESENT_MODE_MAILBOX_KHR: Self = PresentModeKHR(1);
    pub const PRESENT_MODE_FIFO_KHR: Self = PresentModeKHR(2);
    pub const PRESENT_MODE_FIFO_RELAXED_KHR: Self = PresentModeKHR(3);
}

This should be

impl PresentModeKHR {
    pub const IMMEDIATE Self = PresentModeKHR(0);
    pub const MAILBOX Self = PresentModeKHR(1);
    pub const FIFO Self = PresentModeKHR(2);
    pub const FIFO_RELAXED Self = PresentModeKHR(3);
}

vk::DebugMarkerObjectNameInfoEXT requires an object handle as a uint64_t, but ash's handle wrappers appear to all be opaque, making a transmute necessary to get the value out. To address this, I'd like to implement something like:

trait Handle {
  const TYPE: DebugReportObjectTypeEXT;
  fn into_raw(self) -> u64;
}

However, I hesitate to dive into that with the generator rewrite looming, as I'm not sure what all will be affected.

The examples in the generator branch now segfault on my machine if they are built with the release flag.

➜  examples git:(racer) cargo run --bin triangle --release
    Finished release [optimized] target(s) in 0.07s
     Running `/home/maik/projects/ash/target/release/triangle`
fish: “cargo run --bin triangle --rele…” terminated by signal SIGILL (Illegal instruction)

Can anyone reproduce this?

• cmd_set_scissor: https://github.com/MaikKlein/ash/blob/c7fecbf25e79295a30f048983ffe6c02cbaed69d/ash/src/device.rs#L688

This is a binding limitation that prevents callers from setting a sub range of scissors starting from an index other than 0, which is normally allowed in the Vulkan spec. The current interface does allow partial updates only starting from 0. The comparable cmd_set_viewport does allow updating viewports in the expected way.

Blocks gfx-rs/gfx#1824

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?

In the ApplicationInfo struct, I can pass in a bogus API version, like vk_make_version!(1,0,100) and it still works, but vk_make_version!(1,1,100) will fail with ErrorIncompatibleDriver error. Both should fail in the same way.

For example every handle can be a null handle, and therefore all functions that use those handles should be marked unsafe.

Should those handles be allowed to be created by the user, or only the API itself? If they are not allowed to be created by the user, then more functions should be allowed to be safe. But the default trait can not be implemented on handles.

I honestly think we should just mark everything as unsafe, so that we don't accidentally break the safety guarantees of Rust.

Any thoughts?

Hi, I'm not sure if this makes sense, but we could reduce verboseness by implementing Default trait for structs like DeviceCreateInfo and possibly many others. Filling in s_type and p_next is a hassle.

LunarG Vulkan SDK supports macOS starting from 1.0.69. This SDK includes libvulkan.1.dylib, the fully-functioning official loader library, as well as MoltenVK ICD, standard validation layers, and their JSON definition files.

Since libvulkan.1.dylib provides more functionality and flexibility than loading MoltenVK directly does, maybe it’s a good idea to load it by default in place of libMoltenVK.dylib?

I can't do:

let entry = Entry::new()?;

because I can't wrap your error type.

Also, ash::vk::Result neither implements std::error::Error nor std::fmt::Debug, both of which would be useful.

All of this is minor, but also minor to fix I think.

xcb_window_t is defined as *mut c_void, but it should be u32 according to xcb/xproto.h

There is cmd_copy_buffer_to_image but not the opposite. This is sort of a blocker for me ATM.
Interestingly, documentation has it but the code (and published version) does not.

Currently functions do not expose any custom allocators.

pub fn create_instance(&self,
                       create_info: &vk::InstanceCreateInfo)
                       -> Result<Instance, InstanceError>;

One idea would be to add allocators with default types

pub fn create_instance<A: VkAllocator = DefaultAllocator>(&self,
                                                          create_info: &vk::InstanceCreateInfo)
                                                          -> Result<Instance, InstanceError>;

This has the advantage of not being a breaking change, this means that I could add custom allocators later and that we don't have to pass in an allocator explicitly.

pub fn create_instance(&self,
                       create_info: &vk::InstanceCreateInfo
                       allocator: Option<vk::AllocationCallbacks>)
                       -> Result<Instance, InstanceError>;

// which we would call
let instance = create_instance(&info, None);

Hi,

I am just starting to use your crate, I just encountered an error. When creating a Device with the "VK_EXT_debug_report" extension, I get the ErrorExtensionNotPresent result.
Not sure why it doesnt work. I have a running C++ programm on the same machine with this extension.

It would be very much more clean to split up ash into ash and ash-sys, where ash-sys only contains the generated files from the generator: #52 (comment)

I'd do a PR when other PRs which affects vk.rs has been merged.

Breaking API changes sneaked in across SemVer! This version needs to be yanked.

pp_-fields like InstanceCreateInfo::pp_enabled_layer_names expects *const c_char but should expect *const *const c_char.

See the specs:

typedef struct VkInstanceCreateInfo {
    VkStructureType             sType;
    const void*                 pNext;
    VkInstanceCreateFlags       flags;
    const VkApplicationInfo*    pApplicationInfo;
    uint32_t                    enabledLayerCount;
    const char* const*          ppEnabledLayerNames;
    uint32_t                    enabledExtensionCount;
    const char* const*          ppEnabledExtensionNames;
} VkInstanceCreateInfo;

Blocks gfx-rs/gfx#1859

So we have a Swapchain type that acts as a function loader, then there is an actual Vulkan type called SwapchainKHR. I think this creates unnecessary confusion, the type can be named SwapchainLoader or something instead. This applies to other loader types as well. Example of the problem:
let swapchain_loader = Swapchain::new(&instance, &device).unwrap();
let swapchain = swapchain_loader.create_swapchain_khr(&swapchain_create_info, None).unwrap();

By the way, is it okay to create this loader object multiple times during the application lifetime?

it appears that get_physical_device_features is not safely exposed anywhere, and in order to use it, caller must call instance.fp_v1_0().get_physical_device_features(dev, &mut features); in an unsafe block.
But this is not really right anyway, as this function is a "free" function, it does not require the instance to be passed in.

Currently vk.rs was previously generated from https://github.com/Osspial/vk-rs, but it has been heavily modified since then.

It would be nice to generate it again to cut down on maintaining costs.

At the moment Ash roughly works like this:

Function pointers are loaded in groups. Instance, device and all the extensions. The idea is that if you don't need certain extensions that you don't need to load them.

Instance and device are grouped again in specific Vulkan versions. If you only need 1.0 function pointers, then you don't need to additionally load 1.1, 1.2, 1.3 etc. Further more they are implemented in traits which means that you can now statically know which function pointers you can use.

// requires at least a vk 1.1 context
fn allocate_something<Device: DeviceV1_1>(d: Device) {...} 

This might make it nicer for libraries to be written on top of Ash.

At the moment instance/device or extension creation fails if at least one function pointer fails to load. This sort of makes sense for instance and device because the core spec really should load, but it doesn't make sense for extensions as some function pointers only will load if another extension is also loaded.

I think the best approach would be to be to allow function pointers to fail and initialize them to a static function that always panics. Conditional loading of specific function pointers in extensions is most likely too complicated for traits.

Another alternative would be to remove versioning and put all the function pointers in instance / device. The versioned traits could still be retained if this is wanted. The side effect is that we need to load everything, which means slightly longer initialization time, and more wasted space. Although it might not matter in practice and you are most likely boxing everything anyway.

Personally I like the way Ash works now, but I haven't heard any positive or negative arguments about the trait system and how Ash handles function pointers. I am sure this is the most subjective feature.

I don't personally think that anything, except that function pointers should be allowed to fail, needs to be changed, but I am always open to feedback.

The API constants have the wrong type: usize, while it should be u32.

Wrong constants:

pub const VK_MAX_PHYSICAL_DEVICE_NAME_SIZE: usize = 256;
pub const VK_UUID_SIZE: usize = 16;
pub const VK_LUID_SIZE: usize = 8;
pub const VK_MAX_EXTENSION_NAME_SIZE: usize = 256;
pub const VK_MAX_DESCRIPTION_SIZE: usize = 256;
pub const VK_MAX_MEMORY_TYPES: usize = 32;
pub const VK_MAX_MEMORY_HEAPS: usize = 16;
pub const VK_TRUE: usize = 1;
pub const VK_FALSE: usize = 0;
pub const VK_MAX_DEVICE_GROUP_SIZE: usize = 32;

While it would be handy to leave them usize (e.g. for arrays), it may use the wrong registers when returning them from functions in the API (like debug report callback)

Today I have come back to working on graphics applications (after 3 weeks away from it) and I find that I cannot get my program to work with the vulkan debug layer turned on (it works with it off). When on, I get:

"vkAcquireNextImageKHR: Application has already acquired the maximum number of images (0x2)"

(I requested 3 images, so the 0x2 is a bit odd). This happens right after the swapchain rebuild.

I found your post about this in January here:
http://stackoverflow.com/questions/41502123/vkacquirenextimagekhr-application-has-already-acquired-the-maximum-number-of-im

However, I'm not using Intel / Mesa, I'm using Nvidia proprietary driver 375.39 on Ubuntu 16.04, and I've tried VulkanSDKs 1.0.37.0, 1.0.39.1, and 1.0.46.0. The first 2 render blank (black) screens with validation on, the newest one gives the error. (With validation off, it works fine on all of them).

If I force vk::PresentInfoKHR.p_results to ptr::null_mut() it no longer gives the error, but continues to render only a blank (black) screen.

Oddly, both ash examples work fine. So does SaschaWillems stuff. It's not your bug, I'm just posting here because maybe you know something that can help me out (I've spent about 5 hours today pulling out my remaining hair).

Since [repr(transparent)] is now stabilized you may consider to use it instead of [repr(C)] for the handles (here and here).

I'd do a PR if you want. This could also be enabled with a feature.

I'm trying to make use of Vulkan Memory Allocator, which asks me to give it the inner pointers of the Device and PhysicalDevice handles. However with ash's current API I can't seem to access these.

https://github.com/MaikKlein/ash/blob/master/src/device.rs#L347

Offsets should be a slice not a reference

It's sometimes useful to propagate errors up to the call site and using ? operator makes this cleaner.

In the README, I noticed that this was in progress:

• Wrapping the complete spec

How is the wrapper generated? What parts of the spec are missing that still need to be wrapped, and should we automate generation of those parts?

As a side question, what patch number is Ash correct up to, and should that be more publicly exposed?

We currently have a problem with gfx-rs portability library: if running a Vulkan application linked to our portability so, the gfx-backend-vulkan ends up calling the function queries of the very same portability so, dying from stack overflow in recursion...
I wonder if this can be solved by Ash looking directly for vkGetInstanceProcAddress and friends in the native libvulkan.so.1:

[kvark@carbon build]$ objdump /usr/lib64/libvulkan.so -T | grep vkGetInstance
0000000000025020 g DF .text 00000000000013dc Base vkGetInstanceProcAddr

The standard way to use Vulkan is to link libvulkan directly, and then use the directly-exported vkGet{Instance|Device}ProcAddr functions to load function pointers as needed. Using dlopen instead of linking complicates ash's public API by requiring the Entry type for functions that are statically guaranteed to be available, and makes it more difficult to use ash-based programs in environments where dynamic libraries are not guaranteed to be in LD_LIBRARY_PATH. It's unclear what benefit is gained in exchange. If there is indeed a sufficiently large benefit to justify the increased friction, it should be documented to prevent this question from recurring.

Currently, we have DebugReportFlagsEXT::ERROR_EXT, _EXT is redundant here.

I don't see DeviceV1_0::free_command_buffers()
Is that just an oversight?

I found it to be highly useful for sharing surfaces between different logical devices (in WebGPU). Would be great to expose this extension with basic type safety. Also see vulkano-rs/vulkano#701