Skip to content

Commit

Permalink
Fix vulkano-rs#1643 (vulkano-rs#2103)
Browse files Browse the repository at this point in the history 
* Fix vulkano-rs#1643

* Remove old code that was incorrect anyway
  • Loading branch information
@Rua @hakolao
Rua authored and hakolao committed Feb 20, 2024
1 parent d03c7b0 commit 978b7e4
Show file tree
Hide file tree
Showing 10 changed files with 877 additions and 481 deletions.
12 changes: 10 additions & 2 deletions examples/src/bin/dynamic-buffers.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use vulkano::{
memory::allocator::StandardMemoryAllocator,
pipeline::{ComputePipeline, Pipeline, PipelineBindPoint},
sync::{self, GpuFuture},
VulkanLibrary,
DeviceSize, VulkanLibrary,
};

fn main() {
Expand Down Expand Up @@ -188,7 +188,15 @@ fn main() {
&descriptor_set_allocator,
layout.clone(),
[
WriteDescriptorSet::buffer(0, input_buffer),
// When writing to the dynamic buffer binding, the range of the buffer that the shader
// will access must also be provided. We specify the size of the `InData` struct here.
// When dynamic offsets are provided later, they get added to the start and end of
// this range.
WriteDescriptorSet::buffer_with_range(
0,
input_buffer,
0..size_of::<shader::ty::InData>() as DeviceSize,
),
WriteDescriptorSet::buffer(1, output_buffer.clone()),
],
)
Expand Down
165 changes: 154 additions & 11 deletions vulkano/src/command_buffer/commands/bind_push.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ use crate::{
AutoCommandBufferBuilder,
},
descriptor_set::{
check_descriptor_write, sys::UnsafeDescriptorSet, DescriptorSetResources,
DescriptorSetUpdateError, DescriptorSetWithOffsets, DescriptorSetsCollection,
DescriptorWriteInfo, WriteDescriptorSet,
check_descriptor_write, layout::DescriptorType, sys::UnsafeDescriptorSet,
DescriptorBindingResources, DescriptorSetResources, DescriptorSetUpdateError,
DescriptorSetWithOffsets, DescriptorSetsCollection, DescriptorWriteInfo,
WriteDescriptorSet,
},
device::{DeviceOwned, QueueFlags},
pipeline::{
Expand All @@ -36,6 +37,7 @@ use crate::{
use parking_lot::Mutex;
use smallvec::SmallVec;
use std::{
cmp::min,
error,
fmt::{Display, Error as FmtError, Formatter},
mem::{size_of, size_of_val},
Expand Down Expand Up @@ -129,24 +131,93 @@ where
});
}

let properties = self.device().physical_device().properties();
let uniform_alignment = properties.min_uniform_buffer_offset_alignment as u32;
let storage_alignment = properties.min_storage_buffer_offset_alignment as u32;

for (i, set) in descriptor_sets.iter().enumerate() {
let set_num = first_set + i as u32;
let (set, dynamic_offsets) = set.as_ref();

// VUID-vkCmdBindDescriptorSets-commonparent
assert_eq!(self.device(), set.as_ref().0.device());
assert_eq!(self.device(), set.device());

let pipeline_layout_set = &pipeline_layout.set_layouts()[set_num as usize];
let set_layout = set.layout();
let pipeline_set_layout = &pipeline_layout.set_layouts()[set_num as usize];

// VUID-vkCmdBindDescriptorSets-pDescriptorSets-00358
if !pipeline_layout_set.is_compatible_with(set.as_ref().0.layout()) {
if !pipeline_set_layout.is_compatible_with(set_layout) {
return Err(BindPushError::DescriptorSetNotCompatible { set_num });
}

// TODO: see https://github.com/vulkano-rs/vulkano/issues/1643
// VUID-vkCmdBindDescriptorSets-pDynamicOffsets-01971
// VUID-vkCmdBindDescriptorSets-pDynamicOffsets-01972
// VUID-vkCmdBindDescriptorSets-pDescriptorSets-01979
// VUID-vkCmdBindDescriptorSets-pDescriptorSets-06715
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 => uniform_alignment,
DescriptorType::StorageBufferDynamic => storage_alignment,
_ => continue,
};

let count = if binding.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 elements = match set.resources().binding(binding_num) {
Some(DescriptorBindingResources::Buffer(elements)) => elements.as_slice(),
_ => unreachable!(),
};

for (index, (&offset, element)) in
dynamic_offsets.iter().zip(elements).enumerate()
{
// VUID-vkCmdBindDescriptorSets-pDynamicOffsets-01971
// VUID-vkCmdBindDescriptorSets-pDynamicOffsets-01972
if offset % required_alignment != 0 {
return Err(BindPushError::DynamicOffsetNotAligned {
set_num,
binding_num,
index: index as u32,
offset,
required_alignment,
});
}

if let Some((buffer, range)) = element {
// VUID-vkCmdBindDescriptorSets-pDescriptorSets-01979
if offset as DeviceSize + range.end > buffer.size() {
return Err(BindPushError::DynamicOffsetOutOfBufferBounds {
set_num,
binding_num,
index: index as u32,
offset,
range_end: range.end,
buffer_size: buffer.size(),
});
}
}
}
}
}

// VUID-vkCmdBindDescriptorSets-dynamicOffsetCount-00359
if dynamic_offsets.len() != required_dynamic_offset_count {
return Err(BindPushError::DynamicOffsetCountMismatch {
set_num,
provided_count: dynamic_offsets.len(),
required_count: required_dynamic_offset_count,
});
}
}

Ok(())
Expand Down Expand Up @@ -1246,6 +1317,39 @@ pub(in super::super) enum BindPushError {
pipeline_layout_set_count: u32,
},

/// In an element of `descriptor_sets`, the number of provided dynamic offsets does not match
/// the number required by the descriptor set.
DynamicOffsetCountMismatch {
set_num: u32,
provided_count: usize,
required_count: usize,
},

/// In an element of `descriptor_sets`, a provided dynamic offset
/// is not a multiple of the value of the [`min_uniform_buffer_offset_alignment`] or
/// [`min_storage_buffer_offset_alignment`] property.
///
/// min_uniform_buffer_offset_alignment: crate::device::Properties::min_uniform_buffer_offset_alignment
/// min_storage_buffer_offset_alignment: crate::device::Properties::min_storage_buffer_offset_alignment
DynamicOffsetNotAligned {
set_num: u32,
binding_num: u32,
index: u32,
offset: u32,
required_alignment: u32,
},

/// In an element of `descriptor_sets`, a provided dynamic offset, when added to the end of the
/// buffer range bound to the descriptor set, is greater than the size of the buffer.
DynamicOffsetOutOfBufferBounds {
set_num: u32,
binding_num: u32,
index: u32,
offset: u32,
range_end: DeviceSize,
buffer_size: DeviceSize,
},

/// An index buffer is missing the `index_buffer` usage.
IndexBufferMissingUsage,

Expand Down Expand Up @@ -1328,6 +1432,45 @@ impl Display for BindPushError {
sets in `pipeline_layout` ({})",
set_num, pipeline_layout_set_count,
),
Self::DynamicOffsetCountMismatch {
set_num,
provided_count,
required_count,
} => write!(
f,
"in the element of `descriptor_sets` being bound to slot {}, the number of \
provided dynamic offsets ({}) does not match the number required by the \
descriptor set ({})",
set_num, provided_count, required_count,
),
Self::DynamicOffsetNotAligned {
set_num,
binding_num,
index,
offset,
required_alignment,
} => write!(
f,
"in the element of `descriptor_sets` being bound to slot {}, the dynamic offset \
provided for binding {} index {} ({}) is not a multiple of the value of the \
`min_uniform_buffer_offset_alignment` or `min_storage_buffer_offset_alignment` \
property ({})",
set_num, binding_num, index, offset, required_alignment,
),
Self::DynamicOffsetOutOfBufferBounds {
set_num,
binding_num,
index,
offset,
range_end,
buffer_size,
} => write!(
f,
"in the element of `descriptor_sets` being bound to slot {}, the dynamic offset \
provided for binding {} index {} ({}), when added to the end of the buffer range \
bound to the descriptor set ({}), is greater than the size of the buffer ({})",
set_num, binding_num, index, offset, range_end, buffer_size,
),
Self::IndexBufferMissingUsage => {
write!(f, "an index buffer is missing the `index_buffer` usage")
}
Expand Down
50 changes: 35 additions & 15 deletions vulkano/src/command_buffer/commands/pipeline.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ where
let layout_binding =
&pipeline.layout().set_layouts()[set_num as usize].bindings()[&binding_num];

let check_buffer = |_index: u32, _buffer: &Arc<dyn BufferAccess>| Ok(());
let check_buffer = |_index: u32, (_buffer, _range): &(Arc<dyn BufferAccess>, Range<DeviceSize>)| Ok(());

let check_buffer_view = |index: u32, buffer_view: &Arc<dyn BufferViewAbstract>| {
for desc_reqs in (binding_reqs.descriptors.get(&Some(index)).into_iter())
Expand Down Expand Up @@ -2144,26 +2144,46 @@ impl SyncCommandBufferBuilder {
})
};

match descriptor_sets_state.descriptor_sets[&set]
.resources()
let descriptor_set_state = &descriptor_sets_state.descriptor_sets[&set];

match descriptor_set_state.resources()
.binding(binding)
.unwrap()
{
DescriptorBindingResources::None(_) => continue,
DescriptorBindingResources::Buffer(elements) => {
resources.extend(
(elements.iter().enumerate())
.filter_map(|(index, element)| {
element.as_ref().map(|buffer| {
(
index as u32,
buffer.clone(),
0..buffer.size(), // TODO:
)
if matches!(descriptor_type, DescriptorType::UniformBufferDynamic | DescriptorType::StorageBufferDynamic) {
let dynamic_offsets = descriptor_set_state.dynamic_offsets();
resources.extend(
(elements.iter().enumerate())
.filter_map(|(index, element)| {
element.as_ref().map(|(buffer, range)| {
let dynamic_offset = dynamic_offsets[index] as DeviceSize;

(
index as u32,
buffer.clone(),
dynamic_offset + range.start..dynamic_offset + range.end,
)
})
})
})
.flat_map(buffer_resource),
);
.flat_map(buffer_resource),
);
} else {
resources.extend(
(elements.iter().enumerate())
.filter_map(|(index, element)| {
element.as_ref().map(|(buffer, range)| {
(
index as u32,
buffer.clone(),
range.clone(),
)
})
})
.flat_map(buffer_resource),
);
}
}
DescriptorBindingResources::BufferView(elements) => {
resources.extend(
Expand Down
Loading

0 comments on commit 978b7e4

Please sign in to comment.