From fb472208e03b1cf562b4ad67254834b83c9658c9 Mon Sep 17 00:00:00 2001 From: marc0246 <40955683+marc0246@users.noreply.github.com> Date: Sun, 10 Dec 2023 00:52:27 +0100 Subject: [PATCH] Allow binding raw descriptor sets --- examples/async-update/main.rs | 8 +- examples/bloom/bloom.rs | 7 +- examples/bloom/main.rs | 54 +-- examples/bloom/tonemap.rs | 3 +- .../src/command_buffer/commands/bind_push.rs | 375 ++++++++++++------ vulkano/src/descriptor_set/mod.rs | 6 + 6 files changed, 301 insertions(+), 152 deletions(-) diff --git a/examples/async-update/main.rs b/examples/async-update/main.rs index 7669192c29..bf69bca46f 100644 --- a/examples/async-update/main.rs +++ b/examples/async-update/main.rs @@ -843,14 +843,12 @@ impl Task for RenderTask { 0, &[ // Bind the uniform buffer designated for this frame. - self.uniform_buffer_sets[frame_index as usize] - .clone() - .into(), + self.uniform_buffer_sets[frame_index as usize].as_raw(), // Bind the currently most up-to-date texture. self.sampler_sets[self.current_texture_index.load(Ordering::Relaxed) as usize] - .clone() - .into(), + .as_raw(), ], + &[], )?; cbf.bind_vertex_buffers(0, &[self.vertex_buffer_id], &[0], &[], &[])?; diff --git a/examples/bloom/bloom.rs b/examples/bloom/bloom.rs index c339e5c2fd..10809ee48b 100644 --- a/examples/bloom/bloom.rs +++ b/examples/bloom/bloom.rs @@ -1,5 +1,5 @@ use crate::{App, RenderContext}; -use std::{slice, sync::Arc}; +use std::sync::Arc; use vulkano::{ image::{mip_level_extent, Image}, pipeline::{ @@ -80,7 +80,8 @@ impl Task for BloomTask { PipelineBindPoint::Compute, &rcx.pipeline_layout, 0, - slice::from_ref(&rcx.descriptor_set), + &[&rcx.descriptor_set], + &[], )?; let bloom_image = tcx.image(self.bloom_image_id)?.image(); @@ -141,7 +142,7 @@ impl Task for BloomTask { } cbf.destroy_object(bloom_image.clone()); - cbf.destroy_object(rcx.descriptor_set.as_ref().0.clone()); + cbf.destroy_object(rcx.descriptor_set.clone()); Ok(()) } diff --git a/examples/bloom/main.rs b/examples/bloom/main.rs index 6425be9ca2..1b6870c509 100644 --- a/examples/bloom/main.rs +++ b/examples/bloom/main.rs @@ -11,7 +11,8 @@ use vulkano::{ DescriptorSetLayout, DescriptorSetLayoutBinding, DescriptorSetLayoutCreateInfo, DescriptorType, }, - DescriptorImageViewInfo, DescriptorSet, DescriptorSetWithOffsets, WriteDescriptorSet, + sys::RawDescriptorSet, + DescriptorImageViewInfo, WriteDescriptorSet, }, device::{ physical::PhysicalDeviceType, Device, DeviceCreateInfo, DeviceExtensions, DeviceOwned, @@ -80,7 +81,7 @@ pub struct RenderContext { recreate_swapchain: bool, descriptor_set_allocator: Arc, sampler: Arc, - descriptor_set: DescriptorSetWithOffsets, + descriptor_set: Arc, task_graph: ExecutableTaskGraph, scene_node_id: NodeId, tonemap_node_id: NodeId, @@ -500,7 +501,7 @@ fn window_size_dependent_setup( pipeline_layout: &Arc, sampler: &Arc, descriptor_set_allocator: &Arc, -) -> (Id, DescriptorSetWithOffsets) { +) -> (Id, Arc) { let device = resources.device(); let swapchain_state = resources.swapchain(swapchain_id).unwrap(); let images = swapchain_state.images(); @@ -570,30 +571,31 @@ fn window_size_dependent_setup( .unwrap() }); - let descriptor_set = DescriptorSet::new( + let descriptor_set = RawDescriptorSet::new( descriptor_set_allocator.clone(), - pipeline_layout.set_layouts()[0].clone(), - [ - WriteDescriptorSet::sampler(0, sampler.clone()), - WriteDescriptorSet::image_view_with_layout( - 1, - DescriptorImageViewInfo { - image_view: bloom_texture_view, - image_layout: ImageLayout::General, - }, - ), - WriteDescriptorSet::image_view_with_layout_array( - 2, - 0, - bloom_mip_chain_views.map(|image_view| DescriptorImageViewInfo { - image_view, - image_layout: ImageLayout::General, - }), - ), - ], - [], + &pipeline_layout.set_layouts()[0], + 0, ) .unwrap(); - - (bloom_image_id, descriptor_set.into()) + let writes = &[ + WriteDescriptorSet::sampler(0, sampler.clone()), + WriteDescriptorSet::image_view_with_layout( + 1, + DescriptorImageViewInfo { + image_view: bloom_texture_view, + image_layout: ImageLayout::General, + }, + ), + WriteDescriptorSet::image_view_with_layout_array( + 2, + 0, + bloom_mip_chain_views.map(|image_view| DescriptorImageViewInfo { + image_view, + image_layout: ImageLayout::General, + }), + ), + ]; + unsafe { descriptor_set.update(writes, &[]) }.unwrap(); + + (bloom_image_id, Arc::new(descriptor_set)) } diff --git a/examples/bloom/tonemap.rs b/examples/bloom/tonemap.rs index 23f49e8616..12c146e585 100644 --- a/examples/bloom/tonemap.rs +++ b/examples/bloom/tonemap.rs @@ -124,7 +124,8 @@ impl Task for TonemapTask { PipelineBindPoint::Graphics, &rcx.pipeline_layout, 0, - slice::from_ref(&rcx.descriptor_set), + &[&rcx.descriptor_set], + &[], )?; let swapchain_state = tcx.swapchain(self.swapchain_id)?; diff --git a/vulkano/src/command_buffer/commands/bind_push.rs b/vulkano/src/command_buffer/commands/bind_push.rs index 561f8f1a5e..7ecbbdfc8f 100644 --- a/vulkano/src/command_buffer/commands/bind_push.rs +++ b/vulkano/src/command_buffer/commands/bind_push.rs @@ -3,6 +3,7 @@ use crate::{ command_buffer::{auto::SetOrPush, sys::RecordingCommandBuffer, AutoCommandBufferBuilder}, descriptor_set::{ layout::{DescriptorBindingFlags, DescriptorSetLayoutCreateFlags, DescriptorType}, + sys::RawDescriptorSet, DescriptorBindingResources, DescriptorBufferInfo, DescriptorSetResources, DescriptorSetWithOffsets, DescriptorSetsCollection, WriteDescriptorSet, }, @@ -47,6 +48,8 @@ impl AutoCommandBufferBuilder { } } + // TODO: The validation here is somewhat duplicated because of how different the parameters are + // here compared to the raw command buffer. fn validate_bind_descriptor_sets( &self, pipeline_bind_point: PipelineBindPoint, @@ -54,13 +57,155 @@ impl AutoCommandBufferBuilder { first_set: u32, descriptor_sets: &[DescriptorSetWithOffsets], ) -> Result<(), Box> { - self.inner.validate_bind_descriptor_sets( + self.inner.validate_bind_descriptor_sets_inner( pipeline_bind_point, pipeline_layout, first_set, - descriptor_sets, + descriptor_sets.len(), )?; + let properties = self.device().physical_device().properties(); + + for (descriptor_sets_index, set) in descriptor_sets.iter().enumerate() { + let set_num = first_set + descriptor_sets_index as u32; + let (set, dynamic_offsets) = set.as_ref(); + + // VUID-vkCmdBindDescriptorSets-commonparent + assert_eq!(self.device(), set.device()); + + let set_layout = set.layout(); + let pipeline_set_layout = &pipeline_layout.set_layouts()[set_num as usize]; + + if !pipeline_set_layout.is_compatible_with(set_layout) { + return Err(Box::new(ValidationError { + problem: format!( + "`descriptor_sets[{0}]` (for set number {1}) is not compatible with \ + `pipeline_layout.set_layouts()[{1}]`", + descriptor_sets_index, set_num + ) + .into(), + vuids: &["VUID-vkCmdBindDescriptorSets-pDescriptorSets-00358"], + ..Default::default() + })); + } + + let mut dynamic_offsets_remaining = dynamic_offsets; + let mut required_dynamic_offset_count = 0; + + for (&binding_num, binding) in set_layout.bindings() { + let required_alignment = match binding.descriptor_type { + DescriptorType::UniformBufferDynamic => { + properties.min_uniform_buffer_offset_alignment + } + DescriptorType::StorageBufferDynamic => { + properties.min_storage_buffer_offset_alignment + } + _ => continue, + }; + + let count = if binding + .binding_flags + .intersects(DescriptorBindingFlags::VARIABLE_DESCRIPTOR_COUNT) + { + set.variable_descriptor_count() + } else { + binding.descriptor_count + } as usize; + + required_dynamic_offset_count += count; + + if !dynamic_offsets_remaining.is_empty() { + let split_index = min(count, dynamic_offsets_remaining.len()); + let dynamic_offsets = &dynamic_offsets_remaining[..split_index]; + dynamic_offsets_remaining = &dynamic_offsets_remaining[split_index..]; + + let resources = set.resources(); + let elements = match resources.binding(binding_num) { + Some(DescriptorBindingResources::Buffer(elements)) => elements.as_slice(), + _ => unreachable!(), + }; + + for (index, (&offset, element)) in + dynamic_offsets.iter().zip(elements).enumerate() + { + if !is_aligned(offset as DeviceSize, required_alignment) { + match binding.descriptor_type { + DescriptorType::UniformBufferDynamic => { + return Err(Box::new(ValidationError { + problem: format!( + "the descriptor type of `descriptor_sets[{}]` \ + (for set number {}) is \ + `DescriptorType::UniformBufferDynamic`, but the \ + dynamic offset provided for binding {} index {} is \ + not aligned to the \ + `min_uniform_buffer_offset_alignment` device property", + descriptor_sets_index, set_num, binding_num, index, + ) + .into(), + vuids: &[ + "VUID-vkCmdBindDescriptorSets-pDynamicOffsets-01971", + ], + ..Default::default() + })); + } + DescriptorType::StorageBufferDynamic => { + return Err(Box::new(ValidationError { + problem: format!( + "the descriptor type of `descriptor_sets[{}]` \ + (for set number {}) is \ + `DescriptorType::StorageBufferDynamic`, but the \ + dynamic offset provided for binding {} index {} is \ + not aligned to the \ + `min_storage_buffer_offset_alignment` device property", + descriptor_sets_index, set_num, binding_num, index, + ) + .into(), + vuids: &[ + "VUID-vkCmdBindDescriptorSets-pDynamicOffsets-01972", + ], + ..Default::default() + })); + } + _ => unreachable!(), + } + } + + if let Some(buffer_info) = element { + let DescriptorBufferInfo { buffer, range } = buffer_info; + + if offset as DeviceSize + range.end > buffer.size() { + return Err(Box::new(ValidationError { + problem: format!( + "the dynamic offset of `descriptor_sets[{}]` \ + (for set number {}) for binding {} index {}, when \ + added to `range.end` of the descriptor write, is \ + greater than the size of the bound buffer", + descriptor_sets_index, set_num, binding_num, index, + ) + .into(), + vuids: &["VUID-vkCmdBindDescriptorSets-pDescriptorSets-01979"], + ..Default::default() + })); + } + } + } + } + } + + if dynamic_offsets.len() != required_dynamic_offset_count { + return Err(Box::new(ValidationError { + problem: format!( + "the number of dynamic offsets provided for `descriptor_sets[{}]` \ + (for set number {}) does not equal the number required ({})", + descriptor_sets_index, set_num, required_dynamic_offset_count, + ) + .into(), + vuids: &["VUID-vkCmdBindDescriptorSets-dynamicOffsetCount-00359"], + ..Default::default() + })); + } + } + Ok(()) } @@ -73,7 +218,6 @@ impl AutoCommandBufferBuilder { descriptor_sets: impl DescriptorSetsCollection, ) -> &mut Self { let descriptor_sets = descriptor_sets.into_vec(); - if descriptor_sets.is_empty() { return self; } @@ -95,11 +239,21 @@ impl AutoCommandBufferBuilder { "bind_descriptor_sets", Default::default(), move |out: &mut RecordingCommandBuffer| { + let dynamic_offsets: SmallVec<[_; 32]> = descriptor_sets + .iter() + .flat_map(|x| x.as_ref().1.iter().copied()) + .collect(); + let descriptor_sets: SmallVec<[_; 12]> = descriptor_sets + .iter() + .map(|x| x.as_ref().0.as_raw()) + .collect(); + out.bind_descriptor_sets_unchecked( pipeline_bind_point, &pipeline_layout, first_set, &descriptor_sets, + &dynamic_offsets, ); }, ); @@ -434,13 +588,15 @@ impl RecordingCommandBuffer { pipeline_bind_point: PipelineBindPoint, pipeline_layout: &PipelineLayout, first_set: u32, - descriptor_sets: &[DescriptorSetWithOffsets], + descriptor_sets: &[&RawDescriptorSet], + dynamic_offsets: &[u32], ) -> Result<&mut Self, Box> { self.validate_bind_descriptor_sets( pipeline_bind_point, pipeline_layout, first_set, descriptor_sets, + dynamic_offsets, )?; Ok(self.bind_descriptor_sets_unchecked( @@ -448,6 +604,7 @@ impl RecordingCommandBuffer { pipeline_layout, first_set, descriptor_sets, + dynamic_offsets, )) } @@ -456,73 +613,22 @@ impl RecordingCommandBuffer { pipeline_bind_point: PipelineBindPoint, pipeline_layout: &PipelineLayout, first_set: u32, - descriptor_sets: &[DescriptorSetWithOffsets], + descriptor_sets: &[&RawDescriptorSet], + dynamic_offsets: &[u32], ) -> Result<(), Box> { - pipeline_bind_point - .validate_device(self.device()) - .map_err(|err| { - err.add_context("pipeline_bind_point") - .set_vuids(&["VUID-vkCmdBindDescriptorSets-pipelineBindPoint-parameter"]) - })?; - - let queue_family_properties = self.queue_family_properties(); - - match pipeline_bind_point { - PipelineBindPoint::Compute => { - if !queue_family_properties - .queue_flags - .intersects(QueueFlags::COMPUTE) - { - return Err(Box::new(ValidationError { - context: "pipeline_bind_point".into(), - problem: "is `PipelineBindPoint::Compute`, but \ - the queue family of the command buffer does not support \ - compute operations" - .into(), - vuids: &[ - "VUID-vkCmdBindDescriptorSets-pipelineBindPoint-00361", - "VUID-vkCmdBindDescriptorSets-commandBuffer-cmdpool", - ], - ..Default::default() - })); - } - } - PipelineBindPoint::Graphics => { - if !queue_family_properties - .queue_flags - .intersects(QueueFlags::GRAPHICS) - { - return Err(Box::new(ValidationError { - context: "pipeline_bind_point".into(), - problem: "is `PipelineBindPoint::Graphics`, but \ - the queue family of the command buffer does not support \ - graphics operations" - .into(), - vuids: &[ - "VUID-vkCmdBindDescriptorSets-pipelineBindPoint-00361", - "VUID-vkCmdBindDescriptorSets-commandBuffer-cmdpool", - ], - ..Default::default() - })); - } - } - } - - if first_set + descriptor_sets.len() as u32 > pipeline_layout.set_layouts().len() as u32 { - return Err(Box::new(ValidationError { - problem: "`first_set + descriptor_sets.len()` is greater than \ - `pipeline_layout.set_layouts().len()`" - .into(), - vuids: &["VUID-vkCmdBindDescriptorSets-firstSet-00360"], - ..Default::default() - })); - } + self.validate_bind_descriptor_sets_inner( + pipeline_bind_point, + pipeline_layout, + first_set, + descriptor_sets.len(), + )?; let properties = self.device().physical_device().properties(); + let mut dynamic_offsets_remaining = dynamic_offsets; + let mut required_dynamic_offset_count = 0; for (descriptor_sets_index, set) in descriptor_sets.iter().enumerate() { let set_num = first_set + descriptor_sets_index as u32; - let (set, dynamic_offsets) = set.as_ref(); // VUID-vkCmdBindDescriptorSets-commonparent assert_eq!(self.device(), set.device()); @@ -543,9 +649,6 @@ impl RecordingCommandBuffer { })); } - let mut dynamic_offsets_remaining = dynamic_offsets; - let mut required_dynamic_offset_count = 0; - for (&binding_num, binding) in set_layout.bindings() { let required_alignment = match binding.descriptor_type { DescriptorType::UniformBufferDynamic => { @@ -573,15 +676,7 @@ impl RecordingCommandBuffer { let dynamic_offsets = &dynamic_offsets_remaining[..split_index]; dynamic_offsets_remaining = &dynamic_offsets_remaining[split_index..]; - let resources = set.resources(); - let elements = match resources.binding(binding_num) { - Some(DescriptorBindingResources::Buffer(elements)) => elements.as_slice(), - _ => unreachable!(), - }; - - for (index, (&offset, element)) in - dynamic_offsets.iter().zip(elements).enumerate() - { + for (index, &offset) in dynamic_offsets.iter().enumerate() { if !is_aligned(offset as DeviceSize, required_alignment) { match binding.descriptor_type { DescriptorType::UniformBufferDynamic => { @@ -623,41 +718,92 @@ impl RecordingCommandBuffer { _ => unreachable!(), } } + } + } + } + } - if let Some(buffer_info) = element { - let DescriptorBufferInfo { buffer, range } = buffer_info; + if dynamic_offsets.len() != required_dynamic_offset_count { + return Err(Box::new(ValidationError { + problem: format!( + "the number of dynamic offsets provided does not equal the number required \ + ({})", + required_dynamic_offset_count, + ) + .into(), + vuids: &["VUID-vkCmdBindDescriptorSets-dynamicOffsetCount-00359"], + ..Default::default() + })); + } - if offset as DeviceSize + range.end > buffer.size() { - return Err(Box::new(ValidationError { - problem: format!( - "the dynamic offset of `descriptor_sets[{}]` \ - (for set number {}) for binding {} index {}, when \ - added to `range.end` of the descriptor write, is \ - greater than the size of the bound buffer", - descriptor_sets_index, set_num, binding_num, index, - ) - .into(), - vuids: &["VUID-vkCmdBindDescriptorSets-pDescriptorSets-01979"], - ..Default::default() - })); - } - } - } + Ok(()) + } + + fn validate_bind_descriptor_sets_inner( + &self, + pipeline_bind_point: PipelineBindPoint, + pipeline_layout: &PipelineLayout, + first_set: u32, + descriptor_sets: usize, + ) -> Result<(), Box> { + pipeline_bind_point + .validate_device(self.device()) + .map_err(|err| { + err.add_context("pipeline_bind_point") + .set_vuids(&["VUID-vkCmdBindDescriptorSets-pipelineBindPoint-parameter"]) + })?; + + let queue_family_properties = self.queue_family_properties(); + + match pipeline_bind_point { + PipelineBindPoint::Compute => { + if !queue_family_properties + .queue_flags + .intersects(QueueFlags::COMPUTE) + { + return Err(Box::new(ValidationError { + context: "pipeline_bind_point".into(), + problem: "is `PipelineBindPoint::Compute`, but \ + the queue family of the command buffer does not support \ + compute operations" + .into(), + vuids: &[ + "VUID-vkCmdBindDescriptorSets-pipelineBindPoint-00361", + "VUID-vkCmdBindDescriptorSets-commandBuffer-cmdpool", + ], + ..Default::default() + })); } } + PipelineBindPoint::Graphics => { + if !queue_family_properties + .queue_flags + .intersects(QueueFlags::GRAPHICS) + { + return Err(Box::new(ValidationError { + context: "pipeline_bind_point".into(), + problem: "is `PipelineBindPoint::Graphics`, but \ + the queue family of the command buffer does not support \ + graphics operations" + .into(), + vuids: &[ + "VUID-vkCmdBindDescriptorSets-pipelineBindPoint-00361", + "VUID-vkCmdBindDescriptorSets-commandBuffer-cmdpool", + ], + ..Default::default() + })); + } + } + } - if dynamic_offsets.len() != required_dynamic_offset_count { - return Err(Box::new(ValidationError { - problem: format!( - "the number of dynamic offsets provided for `descriptor_sets[{}]` \ - (for set number {}) does not equal the number required ({})", - descriptor_sets_index, set_num, required_dynamic_offset_count, - ) + if first_set + descriptor_sets as u32 > pipeline_layout.set_layouts().len() as u32 { + return Err(Box::new(ValidationError { + problem: "`first_set + descriptor_sets.len()` is greater than \ + `pipeline_layout.set_layouts().len()`" .into(), - vuids: &["VUID-vkCmdBindDescriptorSets-dynamicOffsetCount-00359"], - ..Default::default() - })); - } + vuids: &["VUID-vkCmdBindDescriptorSets-firstSet-00360"], + ..Default::default() + })); } Ok(()) @@ -669,20 +815,15 @@ impl RecordingCommandBuffer { pipeline_bind_point: PipelineBindPoint, pipeline_layout: &PipelineLayout, first_set: u32, - descriptor_sets: &[DescriptorSetWithOffsets], + descriptor_sets: &[&RawDescriptorSet], + dynamic_offsets: &[u32], ) -> &mut Self { if descriptor_sets.is_empty() { return self; } - let descriptor_sets_vk: SmallVec<[_; 12]> = descriptor_sets - .iter() - .map(|x| x.as_ref().0.handle()) - .collect(); - let dynamic_offsets_vk: SmallVec<[_; 32]> = descriptor_sets - .iter() - .flat_map(|x| x.as_ref().1.iter().copied()) - .collect(); + let descriptor_sets_vk: SmallVec<[_; 12]> = + descriptor_sets.iter().map(|x| x.handle()).collect(); let fns = self.device().fns(); (fns.v1_0.cmd_bind_descriptor_sets)( @@ -692,8 +833,8 @@ impl RecordingCommandBuffer { first_set, descriptor_sets_vk.len() as u32, descriptor_sets_vk.as_ptr(), - dynamic_offsets_vk.len() as u32, - dynamic_offsets_vk.as_ptr(), + dynamic_offsets.len() as u32, + dynamic_offsets.as_ptr(), ); self diff --git a/vulkano/src/descriptor_set/mod.rs b/vulkano/src/descriptor_set/mod.rs index c858c02cd4..f220fc7906 100644 --- a/vulkano/src/descriptor_set/mod.rs +++ b/vulkano/src/descriptor_set/mod.rs @@ -144,6 +144,12 @@ impl DescriptorSet { Ok(Arc::new(set)) } + /// Returns the inner raw descriptor set. + #[inline] + pub fn as_raw(&self) -> &RawDescriptorSet { + &self.inner + } + /// Returns the allocation of the descriptor set. #[inline] pub fn alloc(&self) -> &DescriptorPoolAlloc {