Skip to content

Commit

Permalink
Merge pull request #667 from Devsh-Graphics-Programming/suballocdescr…
Browse files Browse the repository at this point in the history 
…iptors-pr-reviews

Suballocated Descriptor Set PR Review Fixes
  • Loading branch information
@devshgraphicsprogramming
devshgraphicsprogramming authored Mar 18, 2024
2 parents 77dd4d7 + b0b2e30 commit 70d77c1
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 33 deletions.
1 change: 0 additions & 1 deletion include/nbl/video/IGPUDescriptorSet.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ class IGPUDescriptorSet : public asset::IDescriptorSet<const IGPUDescriptorSetLa
inline IDescriptorPool* getPool() const { return m_pool.get(); }
inline bool isZombie() const { return (m_pool.get() == nullptr); }

// why is this private? nothing it uses is private
// small utility
inline asset::IDescriptor::E_TYPE getBindingType(const uint32_t binding) const
{
Expand Down
14 changes: 11 additions & 3 deletions include/nbl/video/ILogicalDevice.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -865,12 +865,20 @@ class NBL_API2 ILogicalDevice : public core::IReferenceCounted, public IDeviceMe
uint32_t bufferViewCount = 0u;
uint32_t imageCount = 0u;
uint32_t accelerationStructureCount = 0u;
uint32_t accelerationStructureWriteCount = 0u;
};
virtual void updateDescriptorSets_impl(const SUpdateDescriptorSetsParams& params) = 0;

// Drops refcounted references of the descriptors in these indices for the descriptor lifetime tracking
// If the nullDescriptor device feature is enabled, this would also write a null descriptor to the descriptor set
virtual void nullifyDescriptors_impl(const std::span<const IGPUDescriptorSet::SDropDescriptorSet> dropDescriptors) = 0;
struct SDropDescriptorSetsParams
{
std::span<const IGPUDescriptorSet::SDropDescriptorSet> drops;
uint32_t bufferCount = 0u;
uint32_t bufferViewCount = 0u;
uint32_t imageCount = 0u;
uint32_t accelerationStructureCount = 0u;
uint32_t accelerationStructureWriteCount = 0u;
};
virtual void nullifyDescriptors_impl(const SDropDescriptorSetsParams& params) = 0;

virtual core::smart_refctd_ptr<IGPURenderpass> createRenderpass_impl(const IGPURenderpass::SCreationParams& params, IGPURenderpass::SCreationParamValidationResult&& validation) = 0;
virtual core::smart_refctd_ptr<IGPUFramebuffer> createFramebuffer_impl(IGPUFramebuffer::SCreationParams&& params) = 0;
Expand Down
8 changes: 8 additions & 0 deletions include/nbl/video/alloc/SubAllocatedDescriptorSet.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ class SubAllocatedDescriptorSet : public core::IReferenceCounted
m_composed = other.m_composed;
m_addresses = other.m_addresses;
m_binding = other.m_binding;

// Nullifying other
other.m_composed = nullptr;
other.m_addresses = nullptr;
other.m_binding = 0;
return *this;
}

Expand Down Expand Up @@ -117,6 +122,9 @@ class SubAllocatedDescriptorSet : public core::IReferenceCounted
descriptorType(inDescriptorType) {}
SubAllocDescriptorSetRange() {}

SubAllocDescriptorSetRange(const SubAllocDescriptorSetRange& other) = delete;
SubAllocDescriptorSetRange& operator=(const SubAllocDescriptorSetRange& other) = delete;

SubAllocDescriptorSetRange& operator=(SubAllocDescriptorSetRange&& other)
{
eventHandler = std::move(other.eventHandler);
Expand Down
34 changes: 8 additions & 26 deletions src/nbl/video/CVulkanLogicalDevice.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ void CVulkanLogicalDevice::updateDescriptorSets_impl(const SUpdateDescriptorSets
// Each pNext member of any structure (including this one) in the pNext chain must be either NULL or a pointer to a valid instance of
// VkWriteDescriptorSetAccelerationStructureKHR, VkWriteDescriptorSetAccelerationStructureNV, or VkWriteDescriptorSetInlineUniformBlockEXT
core::vector<VkWriteDescriptorSet> vk_writeDescriptorSets(params.writes.size(),{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET,nullptr});
core::vector<VkWriteDescriptorSetAccelerationStructureKHR> vk_writeDescriptorSetAS(MaxDescriptorSetAsWrites,{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET_ACCELERATION_STRUCTURE_KHR,nullptr});
core::vector<VkWriteDescriptorSetAccelerationStructureKHR> vk_writeDescriptorSetAS(params.accelerationStructureWriteCount,{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET_ACCELERATION_STRUCTURE_KHR,nullptr});

core::vector<VkDescriptorBufferInfo> vk_bufferInfos(params.bufferCount);
core::vector<VkDescriptorImageInfo> vk_imageInfos(params.imageCount);
Expand Down Expand Up @@ -731,39 +731,21 @@ void CVulkanLogicalDevice::updateDescriptorSets_impl(const SUpdateDescriptorSets
m_devf.vk.vkUpdateDescriptorSets(m_vkdev,vk_writeDescriptorSets.size(),vk_writeDescriptorSets.data(),vk_copyDescriptorSets.size(),vk_copyDescriptorSets.data());
}

void CVulkanLogicalDevice::nullifyDescriptors_impl(const std::span<const IGPUDescriptorSet::SDropDescriptorSet> drops)
void CVulkanLogicalDevice::nullifyDescriptors_impl(const SDropDescriptorSetsParams& params)
{
const auto& drops = params.drops;
if (getEnabledFeatures().nullDescriptor)
{
return;
}

core::vector<VkWriteDescriptorSet> vk_writeDescriptorSets(drops.size(),{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET,nullptr});
core::vector<VkWriteDescriptorSetAccelerationStructureKHR> vk_writeDescriptorSetAS(MaxDescriptorSetAsWrites,{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET_ACCELERATION_STRUCTURE_KHR,nullptr});
core::vector<VkWriteDescriptorSetAccelerationStructureKHR> vk_writeDescriptorSetAS(params.accelerationStructureWriteCount,{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET_ACCELERATION_STRUCTURE_KHR,nullptr});

size_t maxSize = 0;
for (auto i = 0; i < drops.size(); i++)
{
const auto& write = drops[i];
auto descriptorType = write.dstSet->getBindingType(write.binding);
size_t descriptorSize;
switch (asset::IDescriptor::GetTypeCategory(descriptorType))
{
case asset::IDescriptor::EC_BUFFER:
descriptorSize = sizeof(VkDescriptorBufferInfo);
break;
case asset::IDescriptor::EC_IMAGE:
descriptorSize = sizeof(VkDescriptorImageInfo);
break;
case asset::IDescriptor::EC_BUFFER_VIEW:
descriptorSize = sizeof(VkBufferView);
break;
case asset::IDescriptor::EC_ACCELERATION_STRUCTURE:
descriptorSize = sizeof(VkAccelerationStructureKHR);
break;
}
maxSize = core::max(maxSize, write.count * descriptorSize);
}
size_t maxSize = core::max(
core::max(params.bufferCount * sizeof(VkDescriptorBufferInfo), params.imageCount * sizeof(VkDescriptorImageInfo)),
core::max(params.bufferViewCount * sizeof(VkBufferView), params.accelerationStructureCount * sizeof(VkAccelerationStructureKHR))
);

core::vector<uint8_t> nullDescriptors(maxSize, 0u);

Expand Down
2 changes: 1 addition & 1 deletion src/nbl/video/CVulkanLogicalDevice.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ class CVulkanLogicalDevice final : public ILogicalDevice
// descriptor sets
core::smart_refctd_ptr<IDescriptorPool> createDescriptorPool_impl(const IDescriptorPool::SCreateInfo& createInfo) override;
void updateDescriptorSets_impl(const SUpdateDescriptorSetsParams& params) override;
void nullifyDescriptors_impl(const std::span<const IGPUDescriptorSet::SDropDescriptorSet> dropDescriptors) override;
void nullifyDescriptors_impl(const SDropDescriptorSetsParams& params) override;

// renderpasses and framebuffers
core::smart_refctd_ptr<IGPURenderpass> createRenderpass_impl(const IGPURenderpass::SCreationParams& params, IGPURenderpass::SCreationParamValidationResult&& validation) override;
Expand Down
28 changes: 26 additions & 2 deletions src/nbl/video/ILogicalDevice.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ bool ILogicalDevice::updateDescriptorSets(const std::span<const IGPUDescriptorSe
break;
case asset::IDescriptor::EC_ACCELERATION_STRUCTURE:
params.accelerationStructureCount += writeCount;
params.accelerationStructureWriteCount++;
break;
default: // validation failed
return false;
Expand Down Expand Up @@ -457,13 +458,36 @@ bool ILogicalDevice::updateDescriptorSets(const std::span<const IGPUDescriptorSe

bool ILogicalDevice::nullifyDescriptors(const std::span<const IGPUDescriptorSet::SDropDescriptorSet> dropDescriptors)
{
SDropDescriptorSetsParams params = {.drops=dropDescriptors};
for (const auto& drop : dropDescriptors)
{
auto ds = drop.dstSet;
if (!ds || !ds->wasCreatedBy(this))
return false;

auto bindingType = ds->getBindingType(drop.binding);
auto writeCount = drop.count;
switch (asset::IDescriptor::GetTypeCategory(bindingType))
{
case asset::IDescriptor::EC_BUFFER:
params.bufferCount += writeCount;
break;
case asset::IDescriptor::EC_IMAGE:
params.imageCount += writeCount;
break;
case asset::IDescriptor::EC_BUFFER_VIEW:
params.bufferViewCount += writeCount;
break;
case asset::IDescriptor::EC_ACCELERATION_STRUCTURE:
params.accelerationStructureCount += writeCount;
params.accelerationStructureWriteCount++;
break;
default: // validation failed
return false;
}

// (no binding)
if (ds->getBindingType(drop.binding) == asset::IDescriptor::E_TYPE::ET_COUNT)
if (bindingType == asset::IDescriptor::E_TYPE::ET_COUNT)
return false;
}

Expand All @@ -473,7 +497,7 @@ bool ILogicalDevice::nullifyDescriptors(const std::span<const IGPUDescriptorSet:
ds->dropDescriptors(drop);
}

nullifyDescriptors_impl(dropDescriptors);
nullifyDescriptors_impl(params);
return true;
}

Expand Down

0 comments on commit 70d77c1

Please sign in to comment.