Merge #183

183: [WIP] Implement extension chaining r=MaikKlein a=MaikKlein

This is only a proof on concept right now. I'll finish it up next week.

What are your thoughts? Right now this requires a cast from `*const` to `*mut`.

Co-authored-by: Maik Klein <maikklein@googlemail.com>

README.md

@@ -82,6 +82,23 @@ Builders implement `Deref` targeting their corresponding Vulkan struct, so refer
 to Vulkan functions. This is encouraged as doing so allows Rust to check the lifetimes of captured objects are valid,
 whereas calling `build` discards lifetime information, making inadvertent use-after-free errors more likely.
 
+### Pointer chains
+
+```Rust
+let mut variable_pointers = vk::PhysicalDeviceVariablePointerFeatures::builder();
+let mut corner =
+    vk::PhysicalDeviceCornerSampledImageFeaturesNV::builder();
+;
+let mut device_create_info = vk::DeviceCreateInfo::builder()
+    .push_next(&mut corner)
+    .push_next(&mut variable_pointers);
+```
+
+Pointer chains in builders differ from raw Vulkan. Instead of chaining every struct manually, you instead use `.push_next` on the struct that you are going to pass into the function. Those structs then get *pepended* into the chain.
+
+`push_next` is also type checked, you can only add valid structs to the chain. Both the structs and the builders can be passed into `push_next`. Only builders for structs that can be passed into functions will implement a `push_next`.
+
+
 ### Flags and constants as associated constants
 
 ```Rust

ash/src/extensions/experimental/amd.rs

@@ -657,11 +657,19 @@ impl<'a> ::std::ops::Deref for PhysicalDeviceWaveLimitPropertiesAmdBuilder<'a> {
     }
 }
 impl<'a> PhysicalDeviceWaveLimitPropertiesAmdBuilder<'a> {
-    pub fn next<T>(mut self, next: &'a mut T) -> PhysicalDeviceWaveLimitPropertiesAmdBuilder<'a>
+    pub fn push_next<T>(
+        mut self,
+        next: &'a mut T,
+    ) -> PhysicalDeviceWaveLimitPropertiesAmdBuilder<'a>
     where
         T: ExtendsPhysicalDeviceWaveLimitPropertiesAmd,
     {
-        self.inner.p_next = next as *mut T as *mut c_void;
+        unsafe {
+            let next_ptr = next as *mut T as *mut BaseOutStructure;
+            let last_next = ptr_chain_iter(next).last().unwrap();
+            (*last_next).p_next = self.inner.p_next as _;
+            self.inner.p_next = next_ptr as _;
+        }
         self
     }
     pub fn build(self) -> PhysicalDeviceWaveLimitPropertiesAmd {

ash/src/extensions/nv/ray_tracing.rs

@@ -28,11 +28,10 @@ impl RayTracing {
         pdevice: vk::PhysicalDevice,
     ) -> vk::PhysicalDeviceRayTracingPropertiesNV {
         let mut props_rt = vk::PhysicalDeviceRayTracingPropertiesNV::default();
-        let mut props = vk::PhysicalDeviceProperties2::builder()
+        {
-            .next(&mut props_rt)
+            let mut props = vk::PhysicalDeviceProperties2::builder().push_next(&mut props_rt);
-            .build();
+            instance.get_physical_device_properties2(pdevice, &mut props);
-
+        }
-        instance.get_physical_device_properties2(pdevice, &mut props);
         props_rt
     }
 

ash/src/lib.rs

@@ -31,3 +31,27 @@ impl<'r, T> RawPtr<T> for Option<&'r T> {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::vk;
+    #[test]
+    fn test_ptr_chains() {
+        let mut variable_pointers = vk::PhysicalDeviceVariablePointerFeatures::builder();
+        let mut corner = vk::PhysicalDeviceCornerSampledImageFeaturesNV::builder();
+        let chain = vec![
+            &variable_pointers as *const _ as usize,
+            &corner as *const _ as usize,
+        ];
+        let mut device_create_info = vk::DeviceCreateInfo::builder()
+            .push_next(&mut corner)
+            .push_next(&mut variable_pointers);
+        let chain2: Vec<usize> = unsafe {
+            vk::ptr_chain_iter(&mut device_create_info)
+                .skip(1)
+                .map(|ptr| ptr as usize)
+                .collect()
+        };
+        assert_eq!(chain, chain2);
+    }
+}

examples/src/lib.rs

@@ -3,14 +3,13 @@ extern crate ash;
 #[cfg(target_os = "windows")]
 extern crate winapi;
 
-extern crate winit;
-
 #[cfg(target_os = "macos")]
 extern crate cocoa;
 #[cfg(target_os = "macos")]
 extern crate metal_rs as metal;
 #[cfg(target_os = "macos")]
 extern crate objc;
+extern crate winit;
 #[cfg(target_os = "macos")]
 use cocoa::appkit::{NSView, NSWindow};
 #[cfg(target_os = "macos")]
@@ -341,7 +340,7 @@ impl ExampleBase {
                 .application_version(0)
                 .engine_name(&app_name)
                 .engine_version(0)
-                .api_version(vk_make_version!(1, 0, 36));
+                .api_version(vk_make_version!(1, 0, 0));
 
             let create_info = vk::InstanceCreateInfo::builder()
                 .application_info(&appinfo)
@@ -415,6 +414,7 @@ impl ExampleBase {
             let device: Device = instance
                 .create_device(pdevice, &device_create_info, None)
                 .unwrap();
+
             let present_queue = device.get_device_queue(queue_family_index as u32, 0);
 
             let surface_formats = surface_loader

generator/src/lib.rs

@@ -1378,7 +1378,10 @@ pub fn derive_debug(_struct: &vkxml::Struct, union_types: &HashSet<&str>) -> Opt
     Some(q)
 }
 
-pub fn derive_setters(_struct: &vkxml::Struct) -> Option<Tokens> {
+pub fn derive_setters(
+    _struct: &vkxml::Struct,
+    root_struct_names: &HashSet<String>,
+) -> Option<Tokens> {
     if &_struct.name == "VkBaseInStructure" || &_struct.name == "VkBaseOutStructure" {
         return None;
     }
@@ -1578,50 +1581,72 @@ pub fn derive_setters(_struct: &vkxml::Struct) -> Option<Tokens> {
         })
     });
 
-    let mut nexts = Vec::new();
     let extends_name = name_to_tokens(&format!("Extends{}", name));
-    if let Some(extends) = &_struct.extends {
-        for target in extends.split(',') {
-            let target = match target {
-                // https://github.com/KhronosGroup/Vulkan-Docs/pull/870
-                "VkPhysicalDeviceProperties" => "VkPhysicalDeviceProperties2",
-                x => x,
-            };
-            let target_ident = name_to_tokens(&format!("Extends{}", name_to_tokens(target)));
-            nexts.push(quote! {
-                unsafe impl #target_ident for #name {}
-            });
-        }
-    }
 
-    let next_function = if has_next {
+    let root_structs: Vec<Ident> = _struct
-        if is_next_const {
+        .extends
-            quote! {
+        .as_ref()
-                pub fn next<T>(mut self, next: &'a T) -> #name_builder<'a> where T: #extends_name {
+        .map(|extends| {
-                    self.inner.p_next = next as *const T as *const c_void;
+            extends
-                    self
+                .split(',')
-                }
+                .filter(|extend| root_struct_names.contains(&extend.to_string()))
-            }
+                .map(|extends| name_to_tokens(&format!("Extends{}", name_to_tokens(&extends))))
-        } else {
+                .collect()
-            quote! {
+        })
-                pub fn next<T>(mut self, next: &'a mut T) -> #name_builder<'a> where T: #extends_name {
+        .unwrap_or(vec![]);
-                    self.inner.p_next = next as *mut T as *mut c_void;
-                    self
-                }
-            }
-        }
-    } else {
-        quote! {}
-    };
 
-    let next_trait = if has_next {
+    // We only implement a next methods for root structs with a `pnext` field.
+    let next_function = if has_next && root_structs.is_empty() {
         quote! {
-            pub unsafe trait #extends_name {}
+            /// Prepends the given extension struct between the root and the first pointer. This
+            /// method only exists on structs that can be passed to a function directly. Only
+            /// valid extension structs can be pushed into the chain.
+            /// If the chain looks like `A -> B -> C`, and you call `builder.push_next(&mut D)`, then the
+            /// chain will look like `A -> D -> B -> C`.
+            pub fn push_next<T: #extends_name>(mut self, next: &'a mut T) -> #name_builder<'a> {
+                unsafe{
+                    let next_ptr = next as *mut T as *mut BaseOutStructure;
+                    // `next` here can contain a pointer chain. This means that we must correctly
+                    // attach he head to the root and the tail to the rest of the chain
+                    // For example:
+                    //
+                    // next = A -> B
+                    // Before: `Root -> C -> D -> E`
+                    // After: `Root -> A -> B -> C -> D -> E`
+                    //                 ^^^^^^
+                    //                 next chain
+                    let last_next = ptr_chain_iter(next).last().unwrap();
+                    (*last_next).p_next = self.inner.p_next as _;
+                    self.inner.p_next = next_ptr as _;
+                }
+                self
+            }
         }
     } else {
         quote! {}
     };
 
+    // Root structs come with their own trait that structs that extends this struct will
+    // implement
+    let next_trait = if has_next && _struct.extends.is_none() {
+        quote! {
+            pub unsafe trait #extends_name {
+            }
+        }
+    } else {
+        quote! {}
+    };
+
+    // If the struct extends something we need to implement the trait.
+    let impl_extend_trait = root_structs.iter().map(|extends| {
+        quote! {
+            unsafe impl #extends for #name_builder<'_> {
+            }
+            unsafe impl #extends for #name {
+            }
+        }
+    });
+
     let q = quote! {
         impl #name {
             pub fn builder<'a>() -> #name_builder<'a> {
@@ -1632,14 +1657,15 @@ pub fn derive_setters(_struct: &vkxml::Struct) -> Option<Tokens> {
             }
         }
 
+        #[repr(transparent)]
         pub struct #name_builder<'a> {
             inner: #name,
             marker: ::std::marker::PhantomData<&'a ()>,
         }
 
+        #(#impl_extend_trait)*
         #next_trait
 
-        #(#nexts)*
 
         impl<'a> ::std::ops::Deref for #name_builder<'a> {
             type Target = #name;
@@ -1649,6 +1675,12 @@ pub fn derive_setters(_struct: &vkxml::Struct) -> Option<Tokens> {
             }
         }
 
+        impl<'a> ::std::ops::DerefMut for #name_builder<'a> {
+            fn deref_mut(&mut self) -> &mut Self::Target {
+                &mut self.inner
+            }
+        }
+
         impl<'a> #name_builder<'a> {
             #(#setters)*
 
@@ -1673,7 +1705,11 @@ pub fn manual_derives(_struct: &vkxml::Struct) -> Tokens {
         _ => quote! {},
     }
 }
-pub fn generate_struct(_struct: &vkxml::Struct, union_types: &HashSet<&str>) -> Tokens {
+pub fn generate_struct(
+    _struct: &vkxml::Struct,
+    root_struct_names: &HashSet<String>,
+    union_types: &HashSet<&str>,
+) -> Tokens {
     let name = name_to_tokens(&_struct.name);
     let members = _struct.elements.iter().filter_map(|elem| match *elem {
         vkxml::StructElement::Member(ref field) => Some(field),
@@ -1688,7 +1724,7 @@ pub fn generate_struct(_struct: &vkxml::Struct, union_types: &HashSet<&str>) ->
 
     let debug_tokens = derive_debug(_struct, union_types);
     let default_tokens = derive_default(_struct);
-    let setter_tokens = derive_setters(_struct);
+    let setter_tokens = derive_setters(_struct, root_struct_names);
     let manual_derive_tokens = manual_derives(_struct);
     let dbg_str = if debug_tokens.is_none() {
         quote!(Debug,)
@@ -1774,15 +1810,32 @@ fn generate_union(union: &vkxml::Union) -> Tokens {
         }
     }
 }
+pub fn root_struct_names(definitions: &[&vkxml::DefinitionsElement]) -> HashSet<String> {
+    definitions
+        .iter()
+        .filter_map(|definition| match *definition {
+            vkxml::DefinitionsElement::Struct(ref _struct) => {
+                let is_root_struct = _struct.extends.is_none();
+                if is_root_struct {
+                    Some(_struct.name.clone())
+                } else {
+                    None
+                }
+            }
+            _ => None,
+        })
+        .collect()
+}
 pub fn generate_definition(
     definition: &vkxml::DefinitionsElement,
     union_types: &HashSet<&str>,
+    root_structs: &HashSet<String>,
     bitflags_cache: &mut HashSet<Ident>,
 ) -> Option<Tokens> {
     match *definition {
         vkxml::DefinitionsElement::Typedef(ref typedef) => Some(generate_typedef(typedef)),
         vkxml::DefinitionsElement::Struct(ref _struct) => {
-            Some(generate_struct(_struct, union_types))
+            Some(generate_struct(_struct, root_structs, union_types))
         }
         vkxml::DefinitionsElement::Bitmask(ref mask) => generate_bitmask(mask, bitflags_cache),
         vkxml::DefinitionsElement::Handle(ref handle) => generate_handle(handle),
@@ -2121,9 +2174,10 @@ pub fn write_source_code(path: &Path) {
         })
         .collect::<HashSet<&str>>();
 
+    let root_names = root_struct_names(&definitions);
     let definition_code: Vec<_> = definitions
         .into_iter()
-        .filter_map(|def| generate_definition(def, &union_types, &mut bitflags_cache))
+        .filter_map(|def| generate_definition(def, &union_types, &root_names, &mut bitflags_cache))
         .collect();
 
     let feature_code: Vec<_> = features
@@ -2144,6 +2198,23 @@ pub fn write_source_code(path: &Path) {
     let source_code = quote! {
         use std::fmt;
         use std::os::raw::*;
+        /// Iterates through the pointer chain. Includes the item that is passed into the function.
+        /// Stops at the last `BaseOutStructure` that has a null `p_next` field.
+        pub(crate) unsafe fn ptr_chain_iter<T>(
+            ptr: &mut T,
+        ) -> impl Iterator<Item = *mut BaseOutStructure> {
+            use std::ptr::null_mut;
+            let ptr: *mut BaseOutStructure = ptr as *mut T as _;
+            (0..).scan(ptr, |p_ptr, _| {
+                if *p_ptr == null_mut() {
+                    return None;
+                }
+                let n_ptr = (**p_ptr).p_next as *mut BaseOutStructure;
+                let old = *p_ptr;
+                *p_ptr = n_ptr;
+                Some(old)
+            })
+        }
 
         pub trait Handle {
             const TYPE: ObjectType;