ash
ash copied to clipboard
Inconsistent interface & unnecessary forced memory allocations in handcrafted methods.
One thing I noticed messing with Ash is some of the handcrafted method's return a Vec, but some methods don't. This seems weird, unnecessary and kinda against the spirit of matching the C++ code with no major compromises in release build.
Ex: Forces Vec
/// <https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/vkAllocateDescriptorSets.html>
#[inline]
pub unsafe fn allocate_descriptor_sets(
&self,
allocate_info: &vk::DescriptorSetAllocateInfo<'_>,
) -> VkResult<Vec<vk::DescriptorSet>> {
let mut desc_set = Vec::with_capacity(allocate_info.descriptor_set_count as usize);
(self.device_fn_1_0.allocate_descriptor_sets)(
self.handle(),
allocate_info,
desc_set.as_mut_ptr(),
)
.set_vec_len_on_success(desc_set, allocate_info.descriptor_set_count as usize)
}
Ex: Does not return Vec
/// <https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/vkGetImageSparseMemoryRequirements2.html>
///
/// Call [`get_image_sparse_memory_requirements2_len()`][Self::get_image_sparse_memory_requirements2_len()] to query the number of elements to pass to `out`.
/// Be sure to [`Default::default()`]-initialize these elements and optionally set their `p_next` pointer.
#[inline]
pub unsafe fn get_image_sparse_memory_requirements2(
&self,
info: &vk::ImageSparseMemoryRequirementsInfo2<'_>,
out: &mut [vk::SparseImageMemoryRequirements2<'_>],
) {
let mut count = out.len() as u32;
(self.device_fn_1_1.get_image_sparse_memory_requirements2)(
self.handle(),
info,
&mut count,
out.as_mut_ptr(),
);
assert_eq!(count as usize, out.len());
}
Imo, the library should move all methods to match the C++ version so the end user can determine how they want to allocate the memory or potentially reuse existing memory. Rust already has slice syntax which keeps this ergonomic.
Ex: Update descriptor sets to not use Vec
/// <https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/vkAllocateDescriptorSets.html>
#[inline]
pub unsafe fn allocate_descriptor_sets(
&self,
allocate_info: &vk::DescriptorSetAllocateInfo<'_>,
descriptor_sets: &mut[vk::DescriptorSet]
) -> VkResult<()> {
assert_eq!(allocate_info.descriptor_set_count,descriptor_sets.len() as u32);
(self.device_fn_1_0.allocate_descriptor_sets)(
self.handle(),
allocate_info,
descriptor_sets.as_mut_ptr()
).result()
}
Made a fork with PR if there is any interest here. There were a couple vecs still used for loading data which probably should be removed also but they weren't methods I know a lot about in Vulkan so I just changed the heavily used ones like DescriptorSets, CommandBuffers, etc. https://github.com/ash-rs/ash/pull/883