ash-rs / ash Goto Github PK
View Code? Open in Web Editor NEWVulkan bindings for Rust
License: Apache License 2.0
Vulkan bindings for Rust
License: Apache License 2.0
Breaking API changes sneaked in across SemVer! This version needs to be yanked.
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.
PFN_vkDebugReportCallbackEXT
expects the last parameter to be a mutable pointer. Currently, it's expecting p_user_data: *const c_void
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);
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, we have DebugReportFlagsEXT::ERROR_EXT
, _EXT
is redundant here.
I don't see DeviceV1_0::free_command_buffers()
Is that just an oversight?
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.
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?
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.
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.
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!
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.
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
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)
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.
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
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.
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
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)
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.
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);
}
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
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
?
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
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.
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
).
xcb_window_t
is defined as *mut c_void
, but it should be u32
according to xcb/xproto.h
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.
pp_
-fields like InstanceCreateInfo::pp_enabled_layer_names
expects *const c_char
but should expect *const *const c_char
.
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;
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.
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.
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.
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?
#[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?
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.
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).
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> {..}
It's sometimes useful to propagate errors up to the call site and using ? operator makes this cleaner.
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.
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.
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?
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.
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.
Blocks gfx-rs/gfx#1737
https://github.com/MaikKlein/ash/blob/master/src/device.rs#L347
Offsets should be a slice not a reference
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?
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?
Blocks gfx-rs/gfx#1859
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.