Skip to content

Commit

Permalink
Bindless Descriptor fixes and optimizations (#2515)
Browse files Browse the repository at this point in the history 
* more SmallVec use in DescriptorPool, to improve variable descriptor count allocation

* descriptor updates with no writes or copies return early

* add DescriptorSet::invalidate()

* make InvalidateDescriptorSet pub

* allow descriptor bindings with update_after_bind or partially_bound to be unbound on recording a draw/dispatch command

---------

Co-authored-by: Firestar99 <4696087-firestar99@users.noreply.gitlab.com>
  • Loading branch information
@Firestar99
Firestar99 and Firestar99 authored Jul 2, 2024
1 parent 252329c commit 7841628
Show file tree
Hide file tree
Showing 5 changed files with 307 additions and 103 deletions.
200 changes: 103 additions & 97 deletions vulkano/src/command_buffer/commands/pipeline.rs
View file
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
View file
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
View file
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

0 comments on commit 7841628

Please sign in to comment.