From 5106b6aa0f7750128272d17b9ce10356900a0b56 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Mon, 22 Feb 2016 15:09:55 +0100 Subject: [PATCH] Better descriptor sets design --- vulkano/examples/teapot.rs | 24 +- vulkano/src/command_buffer/inner.rs | 27 ++- vulkano/src/command_buffer/outer.rs | 19 +- vulkano/src/descriptor_set.rs | 256 ++++++++++++++++------ vulkano/src/pipeline/graphics_pipeline.rs | 12 +- 5 files changed, 242 insertions(+), 96 deletions(-) diff --git a/vulkano/examples/teapot.rs b/vulkano/examples/teapot.rs index 98c50cde..663f45ee 100644 --- a/vulkano/examples/teapot.rs +++ b/vulkano/examples/teapot.rs @@ -140,14 +140,26 @@ fn main() { let descriptor_pool = vulkano::descriptor_set::DescriptorPool::new(&device).unwrap(); + let descriptor_set_layout = { + let desc = vulkano::descriptor_set::RuntimeDescriptorSetDesc { + descriptors: vec![ + vulkano::descriptor_set::DescriptorDesc { + binding: 0, + ty: vulkano::descriptor_set::DescriptorType::UniformBuffer, + array_count: 1, + stages: vulkano::descriptor_set::ShaderStages::all_graphics(), + } + ] + }; - let (pipeline_layout, set) = { - let layout1 = vulkano::descriptor_set::DescriptorSetLayout::new(&device, Default::default()).unwrap(); - let pipeline_layout = vulkano::descriptor_set::PipelineLayout::new(&device, Default::default(), (layout1.clone(), ())).unwrap(); - let set1 = vulkano::descriptor_set::DescriptorSet::new(&descriptor_pool, &layout1, uniform_buffer.clone() as std::sync::Arc<_>).unwrap(); - (pipeline_layout, set1) + vulkano::descriptor_set::DescriptorSetLayout::new(&device, desc).unwrap() }; + let pipeline_layout = vulkano::descriptor_set::PipelineLayout::new(&device, vulkano::descriptor_set::RuntimeDesc, vec![descriptor_set_layout.clone()]).unwrap(); + let set = vulkano::descriptor_set::DescriptorSet::new(&descriptor_pool, &descriptor_set_layout, + vec![(0, vulkano::descriptor_set::DescriptorBind::UniformBuffer(uniform_buffer.clone()))]).unwrap(); + + let pipeline = { let ia = vulkano::pipeline::input_assembly::InputAssembly { topology: vulkano::pipeline::input_assembly::PrimitiveTopology::TriangleList, @@ -189,7 +201,7 @@ fn main() { let command_buffers = framebuffers.iter().map(|framebuffer| { vulkano::command_buffer::PrimaryCommandBufferBuilder::new(&cb_pool).unwrap() .draw_inline(&renderpass, &framebuffer, ([0.0, 0.0, 1.0, 1.0], 1.0)) - .draw_indexed(&pipeline, (vertex_buffer.clone(), normals_buffer.clone()), &index_buffer, &vulkano::command_buffer::DynamicState::none(), (set.clone(), ())) + .draw_indexed(&pipeline, (vertex_buffer.clone(), normals_buffer.clone()), &index_buffer, &vulkano::command_buffer::DynamicState::none(), set.clone()) .draw_end() .build().unwrap() }).collect::>(); diff --git a/vulkano/src/command_buffer/inner.rs b/vulkano/src/command_buffer/inner.rs index f773cbde..423de0ae 100644 --- a/vulkano/src/command_buffer/inner.rs +++ b/vulkano/src/command_buffer/inner.rs @@ -8,6 +8,7 @@ use buffer::BufferResource; use command_buffer::CommandBufferPool; use command_buffer::DynamicState; use descriptor_set::PipelineLayoutDesc; +use descriptor_set::DescriptorSetsCollection; use device::Queue; use framebuffer::ClearValue; use framebuffer::Framebuffer; @@ -265,10 +266,11 @@ impl InnerCommandBufferBuilder { /// Calls `vkCmdDraw`. // FIXME: push constants - pub unsafe fn draw(mut self, pipeline: &Arc>, + pub unsafe fn draw(mut self, pipeline: &Arc>, vertices: V, dynamic: &DynamicState, - sets: L::DescriptorSets) -> InnerCommandBufferBuilder - where V: 'static + MultiVertex, L: 'static + PipelineLayoutDesc + sets: L) -> InnerCommandBufferBuilder + where V: 'static + MultiVertex, L: 'static + DescriptorSetsCollection, + Pl: 'static + PipelineLayoutDesc { // FIXME: add buffers to the resources @@ -292,10 +294,11 @@ impl InnerCommandBufferBuilder { /// Calls `vkCmdDrawIndexed`. // FIXME: push constants - pub unsafe fn draw_indexed<'a, V, L, I, Ib, IbM>(mut self, pipeline: &Arc>, + pub unsafe fn draw_indexed<'a, V, Pl, L, I, Ib, IbM>(mut self, pipeline: &Arc>, vertices: V, indices: Ib, dynamic: &DynamicState, - sets: L::DescriptorSets) -> InnerCommandBufferBuilder - where V: 'static + MultiVertex, L: 'static + PipelineLayoutDesc, + sets: L) -> InnerCommandBufferBuilder + where V: 'static + MultiVertex, L: 'static + DescriptorSetsCollection, + Pl: 'static + PipelineLayoutDesc, Ib: Into>, I: 'static + Index, IbM: 'static { @@ -323,13 +326,16 @@ impl InnerCommandBufferBuilder { self } - fn bind_gfx_pipeline_state(&mut self, pipeline: &Arc>, - dynamic: &DynamicState, sets: L::DescriptorSets) - where V: MultiVertex, L: PipelineLayoutDesc + fn bind_gfx_pipeline_state(&mut self, pipeline: &Arc>, + dynamic: &DynamicState, sets: L) + where V: 'static + MultiVertex, L: 'static + DescriptorSetsCollection, + Pl: 'static + PipelineLayoutDesc { unsafe { let vk = self.device.pointers(); + assert!(sets.is_compatible_with(pipeline.layout())); + if self.graphics_pipeline != Some(pipeline.internal_object()) { vk.CmdBindPipeline(self.cmd.unwrap(), vk::PIPELINE_BIND_POINT_GRAPHICS, pipeline.internal_object()); @@ -349,7 +355,8 @@ impl InnerCommandBufferBuilder { } // FIXME: keep these alive - let descriptor_sets = pipeline.layout().description().decode_descriptor_sets(sets); + // TODO: allocate on stack instead (https://github.com/rust-lang/rfcs/issues/618) + let descriptor_sets = sets.list().collect::>(); // TODO: allocate on stack instead (https://github.com/rust-lang/rfcs/issues/618) let descriptor_sets = descriptor_sets.into_iter().map(|set| set.internal_object()).collect::>(); diff --git a/vulkano/src/command_buffer/outer.rs b/vulkano/src/command_buffer/outer.rs index 03cd5f86..7885a0fd 100644 --- a/vulkano/src/command_buffer/outer.rs +++ b/vulkano/src/command_buffer/outer.rs @@ -6,6 +6,7 @@ use command_buffer::CommandBufferPool; use command_buffer::inner::InnerCommandBufferBuilder; use command_buffer::inner::InnerCommandBuffer; use descriptor_set::PipelineLayoutDesc; +use descriptor_set::DescriptorSetsCollection; use device::Queue; use framebuffer::Framebuffer; use framebuffer::RenderPass; @@ -203,10 +204,11 @@ pub struct PrimaryCommandBufferBuilderInlineDraw { impl PrimaryCommandBufferBuilderInlineDraw { /// Calls `vkCmdDraw`. // FIXME: push constants - pub fn draw(self, pipeline: &Arc>, - vertices: V, dynamic: &DynamicState, sets: L::DescriptorSets) - -> PrimaryCommandBufferBuilderInlineDraw - where V: MultiVertex + 'static, L: PipelineLayoutDesc + 'static + pub fn draw(self, pipeline: &Arc>, + vertices: V, dynamic: &DynamicState, sets: L) + -> PrimaryCommandBufferBuilderInlineDraw + where V: MultiVertex + 'static, Pl: PipelineLayoutDesc + 'static, + L: DescriptorSetsCollection + 'static { unsafe { PrimaryCommandBufferBuilderInlineDraw { @@ -218,11 +220,12 @@ impl PrimaryCommandBufferBuilderInlineDraw { } /// Calls `vkCmdDrawIndexed`. - pub fn draw_indexed<'a, V, L, I, Ib, IbM>(mut self, pipeline: &Arc>, + pub fn draw_indexed<'a, V, L, Pl, I, Ib, IbM>(mut self, pipeline: &Arc>, vertices: V, indices: Ib, dynamic: &DynamicState, - sets: L::DescriptorSets) -> PrimaryCommandBufferBuilderInlineDraw - where V: 'static + MultiVertex, L: 'static + PipelineLayoutDesc, - Ib: Into>, I: 'static + Index, IbM: 'static + sets: L) -> PrimaryCommandBufferBuilderInlineDraw + where V: 'static + MultiVertex, Pl: 'static + PipelineLayoutDesc, + Ib: Into>, I: 'static + Index, IbM: 'static, + L: DescriptorSetsCollection + 'static { unsafe { PrimaryCommandBufferBuilderInlineDraw { diff --git a/vulkano/src/descriptor_set.rs b/vulkano/src/descriptor_set.rs index 158d05b2..e58ffee2 100644 --- a/vulkano/src/descriptor_set.rs +++ b/vulkano/src/descriptor_set.rs @@ -19,7 +19,7 @@ //! # Binding resources //! //! In parallel of the pipeline initialization, you have to create a `DescriptorSet`. This -//! struct contains the list of actual resources that will be binded when the pipeline is executed. +//! struct contains the list of actual resources that will be bound when the pipeline is executed. //! To build a `DescriptorSet`, you need to pass a `DescriptorSetLayout`. The `T` parameter //! must implement `DescriptorSetDesc` as if the same for both the descriptor set and its layout. //! @@ -37,6 +37,7 @@ use std::mem; +use std::option::IntoIter as OptionIntoIter; use std::ptr; use std::sync::Arc; @@ -63,43 +64,135 @@ pub unsafe trait PipelineLayoutDesc { type PushConstants; /// Turns the `DescriptorSets` associated type into something vulkano can understand. - fn decode_descriptor_sets(&self, Self::DescriptorSets) -> Vec>; + fn decode_descriptor_sets(&self, Self::DescriptorSets) -> Vec>; // TODO: vec is slow /// Turns the `DescriptorSetLayouts` associated type into something vulkano can understand. fn decode_descriptor_set_layouts(&self, Self::DescriptorSetLayouts) - -> Vec>; + -> Vec>; // TODO: vec is slow // FIXME: implement this correctly fn is_compatible_with

(&self, _: &P) -> bool where P: PipelineLayoutDesc { true } } -// FIXME: should merge the two and check for collisions instead of making them cohabit -unsafe impl PipelineLayoutDesc for (A, B) - where A: PipelineLayoutDesc, B: PipelineLayoutDesc -{ - type DescriptorSets = (A::DescriptorSets, B::DescriptorSets); - type DescriptorSetLayouts = (A::DescriptorSetLayouts, B::DescriptorSetLayouts); - type PushConstants = (A::PushConstants, B::PushConstants); +/// FIXME: it should be unsafe to create this struct +pub struct RuntimeDesc; + +unsafe impl PipelineLayoutDesc for RuntimeDesc { + type DescriptorSets = Vec>; + type DescriptorSetLayouts = Vec>; + type PushConstants = (); #[inline] - fn decode_descriptor_sets(&self, s: Self::DescriptorSets) -> Vec> { - let mut a = self.0.decode_descriptor_sets(s.0); - let b = self.1.decode_descriptor_sets(s.1); - a.extend_from_slice(&b); - a + fn decode_descriptor_sets(&self, sets: Self::DescriptorSets) -> Vec> { + sets } #[inline] - fn decode_descriptor_set_layouts(&self, s: Self::DescriptorSetLayouts) + fn decode_descriptor_set_layouts(&self, layouts: Self::DescriptorSetLayouts) -> Vec> { - let mut a = self.0.decode_descriptor_set_layouts(s.0); - let b = self.1.decode_descriptor_set_layouts(s.1); - a.extend_from_slice(&b); - a + layouts } } +/* +#[macro_export] +macro_rules! pipeline_layout { + (sets: {$($set_name:ident: { $($name:ident : ),* }),*}) => { + mod layout { + use std::sync::Arc; + use $crate::descriptor_set::DescriptorType; + use $crate::descriptor_set::DescriptorDesc; + use $crate::descriptor_set::DescriptorSetDesc; + use $crate::descriptor_set::DescriptorWrite; + use $crate::descriptor_set::DescriptorBind; + use $crate::descriptor_set::PipelineLayout; + use $crate::descriptor_set::PipelineLayoutDesc; + use $crate::descriptor_set::ShaderStages; + use $crate::buffer::BufferResource; + + $( + pub struct $set_name; + unsafe impl DescriptorSetDesc for $set_name { + type Write = ( // FIXME: variable number of elems + Arc // FIXME: strong typing + ); + + type Init = Self::Write; + + #[inline] + fn descriptors(&self) -> Vec { + let mut binding = 0; + let mut result = Vec::new(); // TODO: with_capacity + + //$( + result.push(DescriptorDesc { + binding: binding, + ty: DescriptorType::UniformBuffer, // FIXME: + array_count: 1, // FIXME: + stages: ShaderStages::all_graphics(), // FIXME: + }); + + binding += 1; + //)* // FIXME: variable number of elems + + let _ = binding; // removes a warning + + result + } + + fn decode_write(&self, data: Self::Write) -> Vec { + let mut binding = 0; + let mut result = Vec::new(); // TODO: with_capacity + + let $($name),* = data; + + $( + result.push(DescriptorWrite { + binding: binding, + array_element: 0, // FIXME: + content: DescriptorBind::UniformBuffer($name), + }); + + binding += 1; + )* + + result + } + + #[inline] + fn decode_init(&self, data: Self::Init) -> Vec { + self.decode_write(data) + } + } + )* + + pub struct Layout; + unsafe impl PipelineLayoutDesc for Layout { + type DescriptorSets = ($(Arc>),*); + type DescriptorSetLayouts = ($(Arc>),*); + type PushConstants = (); + + #[inline] + fn decode_descriptor_sets(&self, sets: Self::DescriptorSets) + -> Vec> + { + let $($set_name),* = sets; + vec![$($set_name as Arc<_>),*] + } + + #[inline] + fn decode_descriptor_set_layouts(&self, layouts: Self::DescriptorSetLayouts) + -> Vec> + { + let $($set_name),* = layouts; + vec![$($set_name as Arc<_>),*] + } + } + } + } +}*/ + /// Dummy implementation of `PipelineLayoutDesc` that describes an empty pipeline. /// /// The descriptors, descriptor sets and push constants are all `()`. You have to pass `()` when @@ -119,6 +212,26 @@ unsafe impl PipelineLayoutDesc for EmptyPipelineDesc { -> Vec> { vec![] } } +unsafe impl PipelineLayoutDesc for (Arc>, Arc>) + where A: 'static + DescriptorSetDesc, B: 'static + DescriptorSetDesc +{ + type DescriptorSets = (Arc>, Arc>); + type DescriptorSetLayouts = (Arc>, Arc>); + type PushConstants = (); + + #[inline] + fn decode_descriptor_sets(&self, sets: Self::DescriptorSets) -> Vec> { + vec![sets.0, sets.1] + } + + #[inline] + fn decode_descriptor_set_layouts(&self, layouts: Self::DescriptorSetLayouts) + -> Vec> + { + vec![layouts.0, layouts.1] + } +} + /// Types that describe a single descriptor set. pub unsafe trait DescriptorSetDesc { /// Represents a modification of a descriptor set. A parameter of this type must be passed @@ -129,13 +242,13 @@ pub unsafe trait DescriptorSetDesc { type Init; /// Returns the list of descriptors contained in this set. - fn descriptors(&self) -> Vec; // TODO: Cow for better perfs + fn descriptors(&self) -> Vec; // TODO: better perfs /// Turns the `Write` associated type into something vulkano can understand. - fn decode_write(&self, Self::Write) -> Vec; + fn decode_write(&self, Self::Write) -> Vec; // TODO: better perfs /// Turns the `Init` associated type into something vulkano can understand. - fn decode_init(&self, Self::Init) -> Vec; + fn decode_init(&self, Self::Init) -> Vec; // TODO: better perfs // FIXME: implement this correctly fn is_compatible_with(&self, _: &S) -> bool where S: DescriptorSetDesc { true } @@ -155,6 +268,7 @@ pub enum DescriptorBind { } /// Describes a single descriptor. +#[derive(Debug, Copy, Clone)] pub struct DescriptorDesc { /// Offset of the binding within the descriptor. pub binding: u32, @@ -169,7 +283,7 @@ pub struct DescriptorDesc { pub stages: ShaderStages, } -/// Describes what kind of resource may later be bind to a descriptor. +/// Describes what kind of resource may later be bound to a descriptor. // FIXME: add immutable sampler when relevant #[derive(Debug, Copy, Clone)] #[repr(u32)] @@ -197,6 +311,7 @@ impl DescriptorType { } /// Describes which shader stages have access to a descriptor. +#[derive(Debug, Copy, Clone)] pub struct ShaderStages { pub vertex: bool, pub tessellation_control: bool, @@ -249,55 +364,37 @@ impl Into for ShaderStages { } } -/* -pub unsafe trait CompatiblePipeline { type Out: PipelineLayoutDesc; } -pub unsafe trait CompatibleSet { type Out: DescriptorSetDesc; } - -macro_rules! impl_tuple { - (($in_first:ident $out_first:ident) $(, ($in_rest:ident $out_rest:ident))*) => { - unsafe impl<$in_first, $out_first $(, $in_rest, $out_rest)*> - CompatibleSet<($out_first, $($out_rest,)*)> for ($in_first, $($in_rest,)*) - where $in_first: CompatibleDescriptor<$out_first> $(, $in_rest: CompatibleDescriptor<$out_rest>)* - { - type Out = ( - <$in_first as CompatibleDescriptor<$out_first>>::Out, - $( - <$in_rest as CompatibleDescriptor<$out_rest>>::Out, - )* - ); - } - - unsafe impl<$in_first, $out_first $(, $in_rest, $out_rest)*> - CompatiblePipeline<($out_first, $($out_rest,)*)> for ($in_first, $($in_rest,)*) - where $in_first: CompatibleSet<$out_first> $(, $in_rest: CompatibleSet<$out_rest>)* - { - type Out = ( - <$in_first as CompatibleSet<$out_first>>::Out, - $( - <$in_rest as CompatibleSet<$out_rest>>::Out, - )* - ); - } - - impl_tuple!{$(($in_rest $out_rest)),*} - }; - - () => (); +/// FIXME: should be unsafe to create this struct +pub struct RuntimeDescriptorSetDesc { + pub descriptors: Vec, } -impl_tuple!( (A N), (B O), (C P), (D Q), (E R), (F S), (G T), - (H U), (I V), (J W), (K X), (L Y), (M Z) ); +unsafe impl DescriptorSetDesc for RuntimeDescriptorSetDesc { + type Write = Vec<(u32, DescriptorBind)>; -/// If a type `A` can be interpreted as a `T`, then `A` will implement `CompatibleDescriptor`. -trait CompatibleDescriptor { type Out; } + type Init = Vec<(u32, DescriptorBind)>; -impl CompatibleDescriptor<()> for () { type Out = (); } -impl CompatibleDescriptor<()> for T where T: Descriptor { type Out = T; } -impl CompatibleDescriptor for () where T: Descriptor { type Out = T; } -impl CompatibleDescriptor for T where T: Descriptor { type Out = T; } + fn descriptors(&self) -> Vec { + self.descriptors.clone() + } + fn decode_write(&self, data: Self::Write) -> Vec { + data.into_iter().map(|(binding, bind)| { + // TODO: check correctness? + + DescriptorWrite { + binding: binding, + array_element: 0, // FIXME: + content: bind, + } + }).collect() + } + + fn decode_init(&self, data: Self::Init) -> Vec { + self.decode_write(data) + } +} -pub unsafe trait Descriptor {}*/ /// An actual descriptor set with the resources that are binded to it. pub struct DescriptorSet { @@ -595,3 +692,28 @@ impl DescriptorPool { })) } } + +pub unsafe trait DescriptorSetsCollection { + type Iter: ExactSizeIterator>; + + fn list(&self) -> Self::Iter; + + fn is_compatible_with

(&self, pipeline_layout: &Arc>) -> bool; +} + +unsafe impl DescriptorSetsCollection for Arc> + where T: 'static + DescriptorSetDesc +{ + type Iter = OptionIntoIter>; + + #[inline] + fn list(&self) -> Self::Iter { + Some(self.clone() as Arc<_>).into_iter() + } + + #[inline] + fn is_compatible_with

(&self, pipeline_layout: &Arc>) -> bool { + // FIXME: + true + } +} diff --git a/vulkano/src/pipeline/graphics_pipeline.rs b/vulkano/src/pipeline/graphics_pipeline.rs index 69868fc7..ac905a0f 100644 --- a/vulkano/src/pipeline/graphics_pipeline.rs +++ b/vulkano/src/pipeline/graphics_pipeline.rs @@ -42,8 +42,8 @@ pub struct GraphicsPipeline { marker: PhantomData<(MultiVertex,)> } -impl GraphicsPipeline - where MV: MultiVertex +impl GraphicsPipeline + where MV: MultiVertex, L: PipelineLayoutDesc { /// Builds a new graphics pipeline object. /// @@ -54,16 +54,18 @@ impl GraphicsPipeline /// - Panicks if the `sample_shading` parameter of `multisample` is not between 0.0 and 1.0. /// // TODO: check all the device's limits - pub fn new + pub fn new (device: &Arc, vertex_shader: &VertexShaderEntryPoint, input_assembly: &InputAssembly, viewport: &ViewportsState, raster: &Rasterization, multisample: &Multisample, blend: &Blend, fragment_shader: &FragmentShaderEntryPoint, - layout: &Arc>, render_pass: &Subpass) - -> Result>, OomError> + layout: &Arc>, render_pass: &Subpass) + -> Result>, OomError> { let vk = device.pointers(); + // FIXME: check layout compatibility + let pipeline = unsafe { // TODO: allocate on stack instead (https://github.com/rust-lang/rfcs/issues/618) let mut dynamic_states: Vec = Vec::new();