Update Vulkan-Headers to 1.2.162 with stable ray-tracing spec #341
Conversation
generator/src/lib.rs
Outdated
| let (slice_param_ty_tokens, set_size_stmt) = if array_size.contains("ename:") { | ||
| // Should contain the same minus `ename:`-prefixed identifiers | ||
| let array_size = field.c_size.as_ref().unwrap(); | ||
| let c_size = convert_c_expression(array_size); | ||
|
|
||
| ( | ||
| quote!(&'a #mutable_ref [#slice_type; #c_size]), | ||
| quote!(), | ||
| ) | ||
| } else { | ||
| let array_size_ident = Ident::from(array_size.to_snake_case().as_str()); | ||
| ( | ||
| quote!(&'a #mutable_ref [#slice_type]), | ||
| quote!(self.inner.#array_size_ident = #param_ident_short.len() as _;), | ||
| ) | ||
| }; |
There was a problem hiding this comment.
I don't really like hacking in this static size in vkxml::ArrayType::Dynamic, but this field is weird: it's definitely a pointer according to the *, p prefix and the header:
KhronosGroup/Vulkan-Headers@d230818#diff-e222ae95c2b0d5082b94d6086fb1c24da18ee31384c1a39840df3b9152023ee6R11550, but the size of the data it points to is statically known. Perhaps we need a third state for this in vkxml::Field?
Otherwise I'd have implemented it something like this: a9b79a3
EDIT: Turns out the field was always there, but the problem never showed up because the setter code for dynamic arrays only runs if the name starts with p or pp. This was fixed in these headers, but I've also fixed in #339 by running this code on every reference (field.reference.is_none()). Perhaps the spec should turn this into a pointer to a fixed-size array of uint8_t[16] though.
There was a problem hiding this comment.
Spent some more time on this over the weekend (have an itch for perfecting "irrelevant" bits of code), and with there being no proper way to represent this const uint8_t (* versionData)[2 * VK_UUID_SIZE] in vk.xml it seems easiest to override the array type to static straight after parsing the file, or adding the "static len expression" heuristic to vk-parse instead.
That'd look about like this: master...Traverse-Research:simplify
vkxmlreally needs to addEq,Clone(andCopy) to their types, but the repo hasn't been touched since 2017...- The heuristic is wrong, and should be moved to
vk-parseas this nestedforis beyond unacceptable; - Finally,
data_voidis now a proper*const [u8; 2 * UUID_SIZE]instead of*const u8in the struct 😬
MarijnS95
force-pushed
the
v11
branch
4 times, most recently
from
b216931
to
acbe28a
Compare
Co-authored-by: Marijn Suijten <marijn@traverseresearch.nl>
| let max_primitive_counts = max_primitive_counts | ||
| .iter() | ||
| .map(|cnt| *cnt as *const _) | ||
| .collect::<Vec<_>>(); | ||
|
|
There was a problem hiding this comment.
This pattern is slightly unfortunate. I think this could actually be safe to transmute, but safe code for now is totally fine.
There was a problem hiding this comment.
Yeah that'd be better, one now has to be aware that cnt is &&u32 to make the pointer cast work.
This handle is already held onto when creating the safe wrapper object, but is inconsistently required again in some but not all functions. Remove it from every function and use the internal handle instead. Closes: ash-rs#377 Fixes: 05747b2 ("Update Vulkan-Headers to 1.2.162 with stable ray-tracing spec (ash-rs#341)")
#378) This handle is already held onto when creating the safe wrapper object, but is inconsistently required again in some but not all functions. Remove it from every function and use the internal handle instead. Closes: #377 Fixes: 05747b2 ("Update Vulkan-Headers to 1.2.162 with stable ray-tracing spec (#341)")
Depends on #339
Fixes #277
Update to the first stable ray-tracing spec released this morning in KhronosGroup/Vulkan-Headers@d230818.
VK_KHR_ray_tracingis now separated inVK_KHR_ray_tracing_pipeline,VK_KHR_ray_queryandVK_KHR_acceleration_structure. Safe extensions wrappers have been updated to account for this.The only interesting change in the generator is dealing with constant-sized arrays, used in
VkAccelerationStructureVersionInfoKHR: