Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extensions/khr: Add VK_KHR_pipeline_binary extension #944

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Added `VK_KHR_get_display_properties2` instance extension (#932)
- Added `VK_EXT_metal_objects` device extension (#942)
- Added `VK_AMD_anti_lag` device extension (#943)
- Added `VK_KHR_pipeline_binary` device extension (#944)

### Changed

Expand Down
1 change: 1 addition & 0 deletions ash/src/extensions/khr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub mod maintenance4;
pub mod maintenance5;
pub mod maintenance6;
pub mod performance_query;
pub mod pipeline_binary;
pub mod pipeline_executable_properties;
pub mod present_wait;
pub mod push_descriptor;
Expand Down
93 changes: 93 additions & 0 deletions ash/src/extensions/khr/pipeline_binary.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
//! <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_KHR_pipeline_binary.html>

use crate::prelude::*;
use crate::vk;
use crate::RawPtr as _;
use alloc::vec::Vec;
use core::mem;

impl crate::khr::pipeline_binary::Device {
/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkCreatePipelineBinariesKHR.html>
#[inline]
#[doc(alias = "vkCreatePipelineBinariesKHR")]
pub unsafe fn create_pipeline_binaries(
&self,
create_info: &vk::PipelineBinaryCreateInfoKHR<'_>,
allocation_callbacks: Option<&vk::AllocationCallbacks<'_>>,
binaries: &mut vk::PipelineBinaryHandlesInfoKHR<'_>,
) -> VkResult<()> {
(self.fp.create_pipeline_binaries_khr)(
self.handle,
create_info,
allocation_callbacks.as_raw_ptr(),
binaries,
)
.result()
}

/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkDestroyPipelineBinaryKHR.html>
#[inline]
#[doc(alias = "vkDestroyPipelineBinaryKHR")]
pub unsafe fn destroy_pipeline_binary(
&self,
pipeline_binary: vk::PipelineBinaryKHR,
allocation_callbacks: Option<&vk::AllocationCallbacks<'_>>,
) {
(self.fp.destroy_pipeline_binary_khr)(
self.handle,
pipeline_binary,
allocation_callbacks.as_raw_ptr(),
)
}

/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkGetPipelineKeyKHR.html>
#[inline]
#[doc(alias = "vkGetPipelineKeyKHR")]
pub unsafe fn get_pipeline_key(
&self,
pipeline_create_info: Option<&vk::PipelineCreateInfoKHR<'_>>,
) -> VkResult<vk::PipelineBinaryKeyKHR<'_>> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't vk::PipelineBinaryKeyKHR be an out parameter, to allow for extension? If this is for some reason undesired, then the lifetime should be 'static.

let mut pipeline_key = mem::MaybeUninit::uninit();
(self.fp.get_pipeline_key_khr)(
self.handle,
pipeline_create_info.as_raw_ptr(),
pipeline_key.as_mut_ptr(),
)
.assume_init_on_success(pipeline_key)
}

/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkGetPipelineBinaryDataKHR.html>
#[inline]
#[doc(alias = "vkGetPipelineBinaryDataKHR")]
pub unsafe fn get_pipeline_binary_data(
&self,
info: &vk::PipelineBinaryDataInfoKHR<'_>,
pipeline_binary_key: &mut vk::PipelineBinaryKeyKHR<'_>,
) -> VkResult<Vec<u8>> {
read_into_uninitialized_binary_vector(|count, data| {
(self.fp.get_pipeline_binary_data_khr)(
self.handle,
info,
pipeline_binary_key,
count,
data,
)
})
}

/// <https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkReleaseCapturedPipelineDataKHR.html>
#[inline]
#[doc(alias = "vkReleaseCapturedPipelineDataKHR")]
pub unsafe fn release_captured_pipeline_data(
&self,
info: &vk::ReleaseCapturedPipelineDataInfoKHR<'_>,
allocation_callbacks: Option<&vk::AllocationCallbacks<'_>>,
) -> VkResult<()> {
(self.fp.release_captured_pipeline_data_khr)(
self.handle,
info,
allocation_callbacks.as_raw_ptr(),
)
.result()
}
}
41 changes: 38 additions & 3 deletions ash/src/prelude.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use alloc::vec::Vec;
use core::convert::TryInto;
use core::ffi;
use core::mem;
use core::ptr;

Expand Down Expand Up @@ -37,13 +38,13 @@ impl vk::Result {
/// Repeatedly calls `f` until it does not return [`vk::Result::INCOMPLETE`] anymore, ensuring all
/// available data has been read into the vector.
///
/// See for example [`vkEnumerateInstanceExtensionProperties`]: the number of available items may
/// See for example [`vkEnumerateInstanceExtensionProperties()`]: the number of available items may
/// change between calls; [`vk::Result::INCOMPLETE`] is returned when the count increased (and the
/// vector is not large enough after querying the initial size), requiring Ash to try again.
///
/// [`vkEnumerateInstanceExtensionProperties`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkEnumerateInstanceExtensionProperties.html
/// [`vkEnumerateInstanceExtensionProperties()`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkEnumerateInstanceExtensionProperties.html
pub(crate) unsafe fn read_into_uninitialized_vector<N: Copy + Default + TryInto<usize>, T>(
f: impl Fn(&mut N, *mut T) -> vk::Result,
mut f: impl FnMut(&mut N, *mut T) -> vk::Result,
) -> VkResult<Vec<T>>
where
<N as TryInto<usize>>::Error: core::fmt::Debug,
Expand All @@ -64,6 +65,40 @@ where
}
}

/// Calls `f` twice until it does not return [`vk::Result::ERROR_NOT_ENOUGH_SPACE_KHR`], ensuring all
/// available binary data has been read into the vector.
Comment on lines +68 to +69
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"twice until" is weird word choice. Maybe "at most twice"?

///
/// The first call happens with a [`Vec`] of size `4096`. If this is not adequate, `f` is supposed
/// to return [`vk::Result::ERROR_NOT_ENOUGH_SPACE_KHR`] while also updating `count` to the desired
/// number of elements, allowing us to try again.
///
/// This function is _not_ designed to be used with [`vk::Result::INCOMPLETE`], see
/// [`read_into_uninitialized_vector()`] instead.
///
/// See for example [`vkGetPipelineBinaryDataKHR()`], where the new return code was first introduced.
///
/// [`vkGetPipelineBinaryDataKHR()`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkGetPipelineBinaryDataKHR.html
pub(crate) unsafe fn read_into_uninitialized_binary_vector(
mut f: impl FnMut(&mut usize, *mut ffi::c_void) -> vk::Result,
) -> VkResult<Vec<u8>> {
let mut count = 4096;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know that binaries are, in practice, usually smaller than this?

let mut data = Vec::<u8>::with_capacity(count);
Comment on lines +84 to +85
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: with_capacity may allocate extra space, and we might as well use it

Suggested change
let mut count = 4096;
let mut data = Vec::<u8>::with_capacity(count);
let mut data = Vec::<u8>::with_capacity(4096);
let mut count = data.capacity();

(and adjust the debug_assert accordingly)

let mut err_code = f(&mut count, data.as_mut_ptr().cast());
if err_code == vk::Result::ERROR_NOT_ENOUGH_SPACE_KHR {
debug_assert!(
count > 4096,
"Implementation should have updated the value to be higher than the initial request"
);
err_code = f(&mut count, data.as_mut_ptr().cast());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be missing the code to actually expand the Vec.

Suggested change
err_code = f(&mut count, data.as_mut_ptr().cast());
data.reserve(count);
err_code = f(&mut count, data.as_mut_ptr().cast());

debug_assert_ne!(
err_code,
vk::Result::ERROR_NOT_ENOUGH_SPACE_KHR,
"Updated count was still not adequate"
);
}
err_code.set_vec_len_on_success(data, count)
}

#[cfg(feature = "debug")]
pub(crate) fn debug_flags<Value: Into<u64> + Copy>(
f: &mut core::fmt::Formatter<'_>,
Expand Down
Loading