* Fix #1643

* Remove old code that was incorrect anyway
This commit is contained in:
Rua 2022-12-16 11:40:46 +01:00 committed by GitHub
parent 79c39ecb06
commit 3aca5dc190
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 914 additions and 518 deletions

View File

@ -32,7 +32,7 @@ use vulkano::{
memory::allocator::StandardMemoryAllocator,
pipeline::{ComputePipeline, Pipeline, PipelineBindPoint},
sync::{self, GpuFuture},
VulkanLibrary,
DeviceSize, VulkanLibrary,
};
fn main() {
@ -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()),
],
)

View File

@ -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::{
@ -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},
@ -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(())
@ -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,
@ -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")
}

View File

@ -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())
@ -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(

View File

@ -12,8 +12,9 @@ use crate::{
buffer::{BufferAccess, BufferContents, BufferUsage, TypedBufferAccess},
command_buffer::{allocator::CommandBufferAllocator, commands::bind_push::BindPushError},
descriptor_set::{
check_descriptor_write, DescriptorSetResources, DescriptorSetWithOffsets,
DescriptorSetsCollection, DescriptorWriteInfo, WriteDescriptorSet,
check_descriptor_write, layout::DescriptorType, DescriptorBindingResources,
DescriptorSetResources, DescriptorSetWithOffsets, DescriptorSetsCollection,
DescriptorWriteInfo, WriteDescriptorSet,
},
device::{DeviceOwned, QueueFlags},
pipeline::{
@ -24,7 +25,7 @@ use crate::{
},
ComputePipeline, GraphicsPipeline, PipelineBindPoint, PipelineLayout,
},
RequiresOneOf, VulkanObject,
DeviceSize, RequiresOneOf, VulkanObject,
};
use smallvec::SmallVec;
use std::{cmp::min, sync::Arc};
@ -108,24 +109,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(())

View File

@ -1124,6 +1124,14 @@ impl SetOrPush {
Self::Push(resources) => resources,
}
}
#[inline]
pub fn dynamic_offsets(&self) -> &[u32] {
match self {
Self::Set(set) => set.as_ref().1,
Self::Push(_) => &[],
}
}
}
#[derive(Clone, Copy, Debug, Default)]

View File

@ -37,10 +37,10 @@ use crate::{
ShaderStages,
},
sync::PipelineStageAccess,
RequiresOneOf, VulkanObject,
RequiresOneOf, VulkanObject, DeviceSize,
};
use ahash::HashMap;
use std::{cmp::min, mem::size_of, sync::Arc};
use std::{cmp::min, mem::size_of, sync::Arc, ops::Range};
impl<L, A> CommandBufferBuilder<L, A>
where
@ -995,7 +995,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())
@ -2058,29 +2058,56 @@ fn record_descriptor_sets_access(
(use_ref, stage_access_iter)
};
match descriptor_sets_state.descriptor_sets[&set]
let descriptor_set_state = &descriptor_sets_state.descriptor_sets[&set];
match descriptor_set_state
.resources()
.binding(binding)
.unwrap()
{
DescriptorBindingResources::None(_) => continue,
DescriptorBindingResources::Buffer(elements) => {
for (index, element) in elements.iter().enumerate() {
if let Some(buffer) = element {
let buffer_inner = buffer.inner();
let (use_ref, stage_access_iter) = use_iter(index as u32);
if matches!(descriptor_type, DescriptorType::UniformBufferDynamic | DescriptorType::StorageBufferDynamic) {
let dynamic_offsets = descriptor_set_state.dynamic_offsets();
let mut range = 0..buffer.size(); // TODO:
range.start += buffer_inner.offset;
range.end += buffer_inner.offset;
for (index, element) in elements.iter().enumerate() {
if let Some((buffer, range)) = element {
let buffer_inner = buffer.inner();
let dynamic_offset = dynamic_offsets[index] as DeviceSize;
let (use_ref, stage_access_iter) = use_iter(index as u32);
for stage_access in stage_access_iter {
resources_usage_state.record_buffer_access(
&use_ref,
buffer_inner.buffer,
range.clone(),
stage_access,
);
let mut range = range.clone();
range.start += buffer_inner.offset + dynamic_offset;
range.end += buffer_inner.offset + dynamic_offset;
for stage_access in stage_access_iter {
resources_usage_state.record_buffer_access(
&use_ref,
buffer_inner.buffer,
range.clone(),
stage_access,
);
}
}
}
} else {
for (index, element) in elements.iter().enumerate() {
if let Some((buffer, range)) = element {
let buffer_inner = buffer.inner();
let (use_ref, stage_access_iter) = use_iter(index as u32);
let mut range = range.clone();
range.start += buffer_inner.offset;
range.end += buffer_inner.offset;
for stage_access in stage_access_iter {
resources_usage_state.record_buffer_access(
&use_ref,
buffer_inner.buffer,
range.clone(),
stage_access,
);
}
}
}
}

View File

@ -1137,6 +1137,14 @@ impl SetOrPush {
Self::Push(resources) => resources,
}
}
#[inline]
pub fn dynamic_offsets(&self) -> &[u32] {
match self {
Self::Set(set) => set.as_ref().1,
Self::Push(_) => &[],
}
}
}
/// Allows you to retrieve the current state of a command buffer builder.

View File

@ -91,7 +91,7 @@ use crate::{
device::DeviceOwned,
image::view::ImageViewAbstract,
sampler::Sampler,
OomError, VulkanObject,
DeviceSize, OomError, VulkanObject,
};
use ahash::HashMap;
use smallvec::{smallvec, SmallVec};
@ -99,6 +99,7 @@ use std::{
error::Error,
fmt::{Display, Error as FmtError, Formatter},
hash::{Hash, Hasher},
ops::Range,
ptr,
sync::Arc,
};
@ -121,6 +122,9 @@ pub unsafe trait DescriptorSet: DeviceOwned + Send + Sync {
/// Returns the layout of this descriptor set.
fn layout(&self) -> &Arc<DescriptorSetLayout>;
/// Returns the variable descriptor count that this descriptor set was allocated with.
fn variable_descriptor_count(&self) -> u32;
/// Creates a [`DescriptorSetWithOffsets`] with the given dynamic offsets.
fn offsets(
self: Arc<Self>,
@ -153,6 +157,7 @@ impl Hash for dyn DescriptorSet {
pub(crate) struct DescriptorSetInner {
layout: Arc<DescriptorSetLayout>,
variable_descriptor_count: u32,
resources: DescriptorSetResources,
}
@ -229,7 +234,11 @@ impl DescriptorSetInner {
);
}
Ok(DescriptorSetInner { layout, resources })
Ok(DescriptorSetInner {
layout,
variable_descriptor_count,
resources,
})
}
pub(crate) fn layout(&self) -> &Arc<DescriptorSetLayout> {
@ -333,7 +342,7 @@ impl DescriptorSetResources {
#[derive(Clone)]
pub enum DescriptorBindingResources {
None(Elements<()>),
Buffer(Elements<Arc<dyn BufferAccess>>),
Buffer(Elements<(Arc<dyn BufferAccess>, Range<DeviceSize>)>),
BufferView(Elements<Arc<dyn BufferViewAbstract>>),
ImageView(Elements<Arc<dyn ImageViewAbstract>>),
ImageViewSampler(Elements<(Arc<dyn ImageViewAbstract>, Arc<Sampler>)>),
@ -416,64 +425,9 @@ impl DescriptorSetWithOffsets {
descriptor_set: Arc<dyn DescriptorSet>,
dynamic_offsets: impl IntoIterator<Item = u32>,
) -> Self {
let dynamic_offsets: SmallVec<_> = dynamic_offsets.into_iter().collect();
let layout = descriptor_set.layout();
let properties = layout.device().physical_device().properties();
let min_uniform_off_align = properties.min_uniform_buffer_offset_alignment as u32;
let min_storage_off_align = properties.min_storage_buffer_offset_alignment as u32;
let mut dynamic_offset_index = 0;
// Ensure that the number of dynamic_offsets is correct and that each
// dynamic offset is a multiple of the minimum offset alignment specified
// by the physical device.
for binding in layout.bindings().values() {
match binding.descriptor_type {
DescriptorType::StorageBufferDynamic => {
// Don't check alignment if there are not enough offsets anyway
if dynamic_offsets.len() > dynamic_offset_index {
assert!(
dynamic_offsets[dynamic_offset_index] % min_storage_off_align == 0,
"Dynamic storage buffer offset must be a multiple of \
min_storage_buffer_offset_alignment: got {}, expected a multiple of {}",
dynamic_offsets[dynamic_offset_index],
min_storage_off_align,
);
}
dynamic_offset_index += 1;
}
DescriptorType::UniformBufferDynamic => {
// Don't check alignment if there are not enough offsets anyway
if dynamic_offsets.len() > dynamic_offset_index {
assert!(
dynamic_offsets[dynamic_offset_index] % min_uniform_off_align == 0,
"Dynamic uniform buffer offset must be a multiple of \
min_uniform_buffer_offset_alignment: got {}, expected a multiple of {}",
dynamic_offsets[dynamic_offset_index],
min_uniform_off_align,
);
}
dynamic_offset_index += 1;
}
_ => (),
}
}
assert!(
dynamic_offsets.len() >= dynamic_offset_index,
"Too few dynamic offsets: got {}, expected {}",
dynamic_offsets.len(),
dynamic_offset_index,
);
assert!(
dynamic_offsets.len() <= dynamic_offset_index,
"Too many dynamic offsets: got {}, expected {}",
dynamic_offsets.len(),
dynamic_offset_index,
);
DescriptorSetWithOffsets {
Self {
descriptor_set,
dynamic_offsets,
dynamic_offsets: dynamic_offsets.into_iter().collect(),
}
}

View File

@ -114,6 +114,10 @@ where
self.inner.layout()
}
fn variable_descriptor_count(&self) -> u32 {
self.inner.variable_descriptor_count
}
fn resources(&self) -> &DescriptorSetResources {
self.inner.resources()
}

File diff suppressed because it is too large Load Diff