Skip to content

Commit eed4c8f

Browse files
authored
Deduplicate push()/extend() helpers as a default impl on TaggedStructure (#994)
Having these functions generated for every root struct in our definitions file contributes to significant bloat, in part because of their massive documentation comments and not containing any generator-dynamic code or identifiers. Now that `trait Extends{Root}` was generalized into a single `trait Extends<Root>` with the root structure as generic argument, it becomes possible to build on top of that and define a default trait function on every Vulkan structure. These functions require the "pushed" type to derive `Extends<Self>`, hence limiting the function to be called without ever having to track if the root structure is being extended.
1 parent 3eb2ebc commit eed4c8f

File tree

5 files changed

+1038
-6006
lines changed

5 files changed

+1038
-6006
lines changed

Changelog.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2929
- `push_next()` has been renamed to `extend()` and marked as `unsafe`. Users are encouraged to call `push()` for singular structs instead. (#909)
3030
- Changed and renamed `RawPtr::as_raw_ptr(&self)` trait function to a by-value `RawPtr::to_raw_ptr(self)` function. (#965)
3131
- All `Extends{Root}` traits have been replaced with a single `Extends<Root>` trait using generics. (#971)
32+
- Moved `push()` and `extend()` methods from individual Vulkan structs (builders) into the `TaggedStructure` trait. (#994)
33+
This trait must be imported in scope (`use ash::vk::TaggedStructure as _;`) to use the `push()` and `extend()` methods.
3234

3335
### Removed
3436

ash/src/vk.rs

Lines changed: 74 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,83 @@ pub trait Handle: Sized {
4949
}
5050
}
5151

52+
/// Iterates through the pointer chain. Includes the item that is passed into the function. Stops at
53+
/// the last [`BaseOutStructure`] that has a null [`BaseOutStructure::p_next`] field.
54+
pub(crate) unsafe fn ptr_chain_iter<'a, T: TaggedStructure<'a>>(
55+
ptr: &mut T,
56+
) -> impl Iterator<Item = *mut BaseOutStructure<'_>> {
57+
let ptr = <*mut T>::cast::<BaseOutStructure<'_>>(ptr);
58+
(0..).scan(ptr, |p_ptr, _| {
59+
if p_ptr.is_null() {
60+
return None;
61+
}
62+
let n_ptr = (**p_ptr).p_next;
63+
let old = *p_ptr;
64+
*p_ptr = n_ptr;
65+
Some(old)
66+
})
67+
}
68+
5269
/// Structures implementing this trait are layout-compatible with [`BaseInStructure`] and
5370
/// [`BaseOutStructure`]. Such structures have an `s_type` field indicating its type, which must
5471
/// always match the value of [`TaggedStructure::STRUCTURE_TYPE`].
55-
pub unsafe trait TaggedStructure {
72+
pub unsafe trait TaggedStructure<'a>: Sized {
5673
const STRUCTURE_TYPE: StructureType;
74+
75+
/// Prepends the given extension struct between the root and the first pointer. This method is
76+
/// only available on structs that can be passed to a function directly. Only valid extension
77+
/// structs can be pushed into the chain.
78+
/// If the chain looks like `A -> B -> C`, and you call `A.push(&mut D)`, then the
79+
/// chain will look like `A -> D -> B -> C`.
80+
///
81+
/// # Panics
82+
/// If `next` contains a pointer chain of its own, this function will panic. Call `unsafe`
83+
/// [`Self::extend()`] to insert this chain instead.
84+
fn push<T: Extends<Self> + TaggedStructure<'a>>(mut self, next: &'a mut T) -> Self {
85+
// SAFETY: All implementers of `TaggedStructure` are required to have the `BaseOutStructure` layout
86+
let slf_base = unsafe { &mut *<*mut _>::cast::<BaseOutStructure<'_>>(&mut self) };
87+
// SAFETY: All implementers of `T: TaggedStructure` are required to have the `BaseOutStructure` layout
88+
let next_base = unsafe { &mut *<*mut T>::cast::<BaseOutStructure<'_>>(next) };
89+
// `next` here can contain a pointer chain. This function refuses to insert the struct,
90+
// in favour of calling unsafe extend().
91+
assert!(
92+
next_base.p_next.is_null(),
93+
"push() expects a struct without an existing p_next pointer chain (equal to NULL)"
94+
);
95+
next_base.p_next = slf_base.p_next;
96+
slf_base.p_next = next_base;
97+
self
98+
}
99+
100+
/// Prepends the given extension struct between the root and the first pointer. This method is
101+
/// only available on structs that can be passed to a function directly. Only valid extension
102+
/// structs can be pushed into the chain.
103+
/// If the chain looks like `A -> B -> C` and `D -> E`, and you call `A.extend(&mut D)`,
104+
/// then the chain will look like `A -> D -> E -> B -> C`.
105+
///
106+
/// # Safety
107+
/// This function will walk the [`BaseOutStructure::p_next`] chain of `next`, requiring
108+
/// all non-`NULL` pointers to point to a valid Vulkan structure starting with the
109+
/// [`BaseOutStructure`] layout.
110+
///
111+
/// The last struct in this chain (i.e. the one where `p_next` is `NULL`) must be writable
112+
/// memory, as its `p_next` field will be updated with the value of `self.p_next`.
113+
unsafe fn extend<T: Extends<Self> + TaggedStructure<'a>>(mut self, next: &'a mut T) -> Self {
114+
// `next` here can contain a pointer chain. This means that we must correctly attach he head
115+
// to the root and the tail to the rest of the chain For example:
116+
//
117+
// next = A -> B
118+
// Before: `Root -> C -> D -> E`
119+
// After: `Root -> A -> B -> C -> D -> E`
120+
// ^^^^^^
121+
// next chain
122+
let slf_base = unsafe { &mut *<*mut _>::cast::<BaseOutStructure<'_>>(&mut self) };
123+
let next_base = <*mut T>::cast::<BaseOutStructure<'_>>(next);
124+
let last_next = ptr_chain_iter(next).last().unwrap();
125+
(*last_next).p_next = slf_base.p_next;
126+
slf_base.p_next = next_base;
127+
self
128+
}
57129
}
58130

59131
/// Implemented for every structure that extends base structure `B`. Concretely that means struct
@@ -65,23 +137,6 @@ pub unsafe trait TaggedStructure {
65137
/// [1]: https://registry.khronos.org/vulkan/specs/latest/styleguide.html#extensions-interactions
66138
pub unsafe trait Extends<B> {}
67139

68-
/// Iterates through the pointer chain. Includes the item that is passed into the function.
69-
/// Stops at the last [`BaseOutStructure`] that has a null [`BaseOutStructure::p_next`] field.
70-
pub(crate) unsafe fn ptr_chain_iter<T: ?Sized>(
71-
ptr: &mut T,
72-
) -> impl Iterator<Item = *mut BaseOutStructure<'_>> {
73-
let ptr = <*mut T>::cast::<BaseOutStructure<'_>>(ptr);
74-
(0..).scan(ptr, |p_ptr, _| {
75-
if p_ptr.is_null() {
76-
return None;
77-
}
78-
let n_ptr = (**p_ptr).p_next;
79-
let old = *p_ptr;
80-
*p_ptr = n_ptr;
81-
Some(old)
82-
})
83-
}
84-
85140
/// Holds 24 bits in the least significant bits of memory,
86141
/// and 8 bytes in the most significant bits of that memory,
87142
/// occupying a single [`u32`] in total. This is commonly used in
@@ -211,6 +266,7 @@ pub(crate) fn debug_flags<Value: Into<u64> + Copy>(
211266
#[cfg(test)]
212267
mod tests {
213268
use crate::vk;
269+
use crate::vk::TaggedStructure as _;
214270
use alloc::vec::Vec;
215271
#[test]
216272
fn test_ptr_chains() {
@@ -272,21 +328,6 @@ mod tests {
272328
assert_eq!(chain, chain2);
273329
}
274330

275-
#[test]
276-
fn test_dynamic_add_to_ptr_chain() {
277-
let mut variable_pointers = vk::PhysicalDeviceVariablePointerFeatures::default();
278-
let variable_pointers: &mut dyn vk::Extends<vk::DeviceCreateInfo<'_>> =
279-
&mut variable_pointers;
280-
let chain = alloc::vec![<*mut _>::cast(variable_pointers)];
281-
let mut device_create_info = vk::DeviceCreateInfo::default().push(variable_pointers);
282-
let chain2: Vec<*mut vk::BaseOutStructure<'_>> = unsafe {
283-
vk::ptr_chain_iter(&mut device_create_info)
284-
.skip(1)
285-
.collect()
286-
};
287-
assert_eq!(chain, chain2);
288-
}
289-
290331
#[test]
291332
fn test_debug_flags() {
292333
assert_eq!(

0 commit comments

Comments
 (0)