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.

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

Bindless Descriptor fixes and optimizations #2515

Merged
merged 5 commits into from
Jul 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
200 changes: 103 additions & 97 deletions vulkano/src/command_buffer/commands/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use crate::{
DrawMeshTasksIndirectCommand, RecordingCommandBuffer, ResourceInCommand, SubpassContents,
},
descriptor_set::{
layout::DescriptorType, DescriptorBindingResources, DescriptorBufferInfo,
DescriptorImageViewInfo,
layout::{DescriptorBindingFlags, DescriptorType},
DescriptorBindingResources, DescriptorBufferInfo, DescriptorImageViewInfo,
},
device::{DeviceOwned, QueueFlags},
format::{FormatFeatures, NumericType},
Expand Down Expand Up @@ -2146,101 +2146,107 @@ impl RecordingCommandBuffer {
Ok(())
};

let set_resources = descriptor_set_state
.descriptor_sets
.get(&set_num)
.ok_or_else(|| {
Box::new(ValidationError {
problem: format!(
"the currently bound pipeline accesses descriptor set {set_num}, but \
no descriptor set was previously bound"
)
.into(),
// vuids?
..Default::default()
})
})?
.resources();

let binding_resources = set_resources.binding(binding_num).unwrap();

match binding_resources {
DescriptorBindingResources::None(elements) => {
validate_resources(
vuid_type,
set_num,
binding_num,
binding_reqs,
elements,
check_none,
)?;
}
DescriptorBindingResources::Buffer(elements) => {
validate_resources(
vuid_type,
set_num,
binding_num,
binding_reqs,
elements,
check_buffer,
)?;
}
DescriptorBindingResources::BufferView(elements) => {
validate_resources(
vuid_type,
set_num,
binding_num,
binding_reqs,
elements,
check_buffer_view,
)?;
}
DescriptorBindingResources::ImageView(elements) => {
validate_resources(
vuid_type,
set_num,
binding_num,
binding_reqs,
elements,
check_image_view,
)?;
}
DescriptorBindingResources::ImageViewSampler(elements) => {
validate_resources(
vuid_type,
set_num,
binding_num,
binding_reqs,
elements,
check_image_view_sampler,
)?;
}
DescriptorBindingResources::Sampler(elements) => {
validate_resources(
vuid_type,
set_num,
binding_num,
binding_reqs,
elements,
check_sampler,
)?;
}
// Spec:
// Descriptor bindings with descriptor type of
// VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK can be undefined when
// the descriptor set is consumed; though values in that block will be undefined.
//
// TODO: We *may* still want to validate this?
DescriptorBindingResources::InlineUniformBlock => (),
DescriptorBindingResources::AccelerationStructure(elements) => {
validate_resources(
vuid_type,
set_num,
binding_num,
binding_reqs,
elements,
check_acceleration_structure,
)?;
let flags_skip_binding_validation =
DescriptorBindingFlags::UPDATE_AFTER_BIND | DescriptorBindingFlags::PARTIALLY_BOUND;
let requires_binding_validation =
(layout_binding.binding_flags & flags_skip_binding_validation).is_empty();
if requires_binding_validation {
let set_resources = descriptor_set_state
.descriptor_sets
.get(&set_num)
.ok_or_else(|| {
Box::new(ValidationError {
problem: format!(
"the currently bound pipeline accesses descriptor set {set_num}, \
but no descriptor set was previously bound"
)
.into(),
// vuids?
..Default::default()
})
})?
.resources();

let binding_resources = set_resources.binding(binding_num).unwrap();

match binding_resources {
DescriptorBindingResources::None(elements) => {
validate_resources(
vuid_type,
set_num,
binding_num,
binding_reqs,
elements,
check_none,
)?;
}
DescriptorBindingResources::Buffer(elements) => {
validate_resources(
vuid_type,
set_num,
binding_num,
binding_reqs,
elements,
check_buffer,
)?;
}
DescriptorBindingResources::BufferView(elements) => {
validate_resources(
vuid_type,
set_num,
binding_num,
binding_reqs,
elements,
check_buffer_view,
)?;
}
DescriptorBindingResources::ImageView(elements) => {
validate_resources(
vuid_type,
set_num,
binding_num,
binding_reqs,
elements,
check_image_view,
)?;
}
DescriptorBindingResources::ImageViewSampler(elements) => {
validate_resources(
vuid_type,
set_num,
binding_num,
binding_reqs,
elements,
check_image_view_sampler,
)?;
}
DescriptorBindingResources::Sampler(elements) => {
validate_resources(
vuid_type,
set_num,
binding_num,
binding_reqs,
elements,
check_sampler,
)?;
}
// Spec:
// Descriptor bindings with descriptor type of
// VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK can be undefined when the descriptor
// set is consumed; though values in that block will be undefined.
//
// TODO: We *may* still want to validate this?
DescriptorBindingResources::InlineUniformBlock => (),
DescriptorBindingResources::AccelerationStructure(elements) => {
validate_resources(
vuid_type,
set_num,
binding_num,
binding_reqs,
elements,
check_acceleration_structure,
)?;
}
}
}
}
Expand Down
101 changes: 99 additions & 2 deletions vulkano/src/descriptor_set/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ use self::{
pub use self::{
collection::DescriptorSetsCollection,
update::{
CopyDescriptorSet, DescriptorBufferInfo, DescriptorImageViewInfo, WriteDescriptorSet,
WriteDescriptorSetElements,
CopyDescriptorSet, DescriptorBufferInfo, DescriptorImageViewInfo, InvalidateDescriptorSet,
WriteDescriptorSet, WriteDescriptorSetElements,
},
};
use crate::{
Expand Down Expand Up @@ -191,6 +191,9 @@ impl DescriptorSet {
) -> Result<(), Box<ValidationError>> {
let descriptor_writes: SmallVec<[_; 8]> = descriptor_writes.into_iter().collect();
let descriptor_copies: SmallVec<[_; 8]> = descriptor_copies.into_iter().collect();
if descriptor_writes.is_empty() && descriptor_copies.is_empty() {
return Ok(());
}

self.inner
.validate_update(&descriptor_writes, &descriptor_copies)?;
Expand All @@ -215,6 +218,9 @@ impl DescriptorSet {
) {
let descriptor_writes: SmallVec<[_; 8]> = descriptor_writes.into_iter().collect();
let descriptor_copies: SmallVec<[_; 8]> = descriptor_copies.into_iter().collect();
if descriptor_writes.is_empty() && descriptor_copies.is_empty() {
return;
}

Self::update_inner(
&self.inner,
Expand All @@ -236,6 +242,9 @@ impl DescriptorSet {
) -> Result<(), Box<ValidationError>> {
let descriptor_writes: SmallVec<[_; 8]> = descriptor_writes.into_iter().collect();
let descriptor_copies: SmallVec<[_; 8]> = descriptor_copies.into_iter().collect();
if descriptor_writes.is_empty() && descriptor_copies.is_empty() {
return Ok(());
}

self.inner
.validate_update(&descriptor_writes, &descriptor_copies)?;
Expand All @@ -258,6 +267,9 @@ impl DescriptorSet {
) {
let descriptor_writes: SmallVec<[_; 8]> = descriptor_writes.into_iter().collect();
let descriptor_copies: SmallVec<[_; 8]> = descriptor_copies.into_iter().collect();
if descriptor_writes.is_empty() && descriptor_copies.is_empty() {
return;
}

Self::update_inner(
&self.inner,
Expand All @@ -283,6 +295,42 @@ impl DescriptorSet {
resources.copy(copy);
}
}

/// Invalidates descriptors within a descriptor set. Doesn't actually call into vulkan and only
/// invalidates the descriptors inside vulkano's resource tracking. Invalidated descriptors are
/// equivalent to uninitialized descriptors, in that binding a descriptor set to a particular
/// pipeline requires all shader-accessible descriptors to be valid.
///
/// The intended use-case is an update-after-bind or bindless system, where entries in an
/// arrayed binding have to be invalidated so that the backing resource will be freed, and
/// not stay forever referenced until overridden by some update.
pub fn invalidate(
&self,
descriptor_invalidates: &[InvalidateDescriptorSet],
) -> Result<(), Box<ValidationError>> {
self.validate_invalidate(descriptor_invalidates)?;
self.invalidate_unchecked(descriptor_invalidates);
Ok(())
}

pub fn invalidate_unchecked(&self, descriptor_invalidates: &[InvalidateDescriptorSet]) {
let mut resources = self.resources.write();
for invalidate in descriptor_invalidates {
resources.invalidate(invalidate);
}
}

pub(super) fn validate_invalidate(
&self,
descriptor_invalidates: &[InvalidateDescriptorSet],
) -> Result<(), Box<ValidationError>> {
for (index, write) in descriptor_invalidates.iter().enumerate() {
write
.validate(self.layout(), self.variable_descriptor_count())
.map_err(|err| err.add_context(format!("descriptor_writes[{}]", index)))?;
}
Ok(())
}
}

unsafe impl VulkanObject for DescriptorSet {
Expand Down Expand Up @@ -432,6 +480,14 @@ impl DescriptorSetResources {
copy.descriptor_count,
);
}

#[inline]
pub(crate) fn invalidate(&mut self, invalidate: &InvalidateDescriptorSet) {
self.binding_resources
.get_mut(&invalidate.binding)
.expect("descriptor write has invalid binding number")
.invalidate(invalidate)
}
}

/// The resources that are bound to a single descriptor set binding.
Expand Down Expand Up @@ -621,6 +677,47 @@ impl DescriptorBindingResources {
},
}
}

pub(crate) fn invalidate(&mut self, invalidate: &InvalidateDescriptorSet) {
fn invalidate_resources<T: Clone>(
resources: &mut [Option<T>],
invalidate: &InvalidateDescriptorSet,
) {
let first = invalidate.first_array_element as usize;
resources
.get_mut(first..first + invalidate.descriptor_count as usize)
.expect("descriptor write for binding out of bounds")
.iter_mut()
.for_each(|resource| {
*resource = None;
});
}

match self {
DescriptorBindingResources::None(resources) => {
invalidate_resources(resources, invalidate)
}
DescriptorBindingResources::Buffer(resources) => {
invalidate_resources(resources, invalidate)
}
DescriptorBindingResources::BufferView(resources) => {
invalidate_resources(resources, invalidate)
}
DescriptorBindingResources::ImageView(resources) => {
invalidate_resources(resources, invalidate)
}
DescriptorBindingResources::ImageViewSampler(resources) => {
invalidate_resources(resources, invalidate)
}
DescriptorBindingResources::Sampler(resources) => {
invalidate_resources(resources, invalidate)
}
DescriptorBindingResources::InlineUniformBlock => (),
DescriptorBindingResources::AccelerationStructure(resources) => {
invalidate_resources(resources, invalidate)
}
}
}
}

#[derive(Clone)]
Expand Down
9 changes: 5 additions & 4 deletions vulkano/src/descriptor_set/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,10 @@ impl DescriptorPool {
let allocate_infos = allocate_infos.into_iter();

let (lower_size_bound, _) = allocate_infos.size_hint();
let mut layouts_vk = Vec::with_capacity(lower_size_bound);
let mut variable_descriptor_counts = Vec::with_capacity(lower_size_bound);
let mut layouts = Vec::with_capacity(lower_size_bound);
let mut layouts_vk: SmallVec<[_; 1]> = SmallVec::with_capacity(lower_size_bound);
let mut variable_descriptor_counts: SmallVec<[_; 1]> =
SmallVec::with_capacity(lower_size_bound);
let mut layouts: SmallVec<[_; 1]> = SmallVec::with_capacity(lower_size_bound);

for info in allocate_infos {
let DescriptorSetAllocateInfo {
Expand All @@ -266,7 +267,7 @@ impl DescriptorPool {
layouts.push(layout);
}

let mut output = vec![];
let mut output: SmallVec<[_; 1]> = SmallVec::new();

if !layouts_vk.is_empty() {
let variable_desc_count_alloc_info = if (self.device.api_version() >= Version::V1_2
Expand Down
Loading
Loading