Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1643 #2103

Merged
merged 2 commits into from
Dec 16, 2022
Merged

Fix #1643 #2103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
17c8f7e
Fix #1643
Rua Dec 13, 2022
abcefb0
Remove old code that was incorrect anyway
Rua Dec 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions examples/src/bin/dynamic-buffers.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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