From b2c3a3347b58db126a825b189b4ab97784dbd661 Mon Sep 17 00:00:00 2001 From: deprilula28 Date: Sun, 17 Mar 2024 00:29:24 -0300 Subject: [PATCH 1/3] PR reviews --- include/nbl/video/IGPUDescriptorSet.h | 1 - include/nbl/video/ILogicalDevice.h | 12 +++++-- .../video/alloc/SubAllocatedDescriptorSet.h | 3 ++ src/nbl/video/CVulkanLogicalDevice.cpp | 34 +++++-------------- src/nbl/video/CVulkanLogicalDevice.h | 2 +- src/nbl/video/ILogicalDevice.cpp | 26 ++++++++++++-- 6 files changed, 45 insertions(+), 33 deletions(-) diff --git a/include/nbl/video/IGPUDescriptorSet.h b/include/nbl/video/IGPUDescriptorSet.h index f81f8190bf..e27b916b85 100644 --- a/include/nbl/video/IGPUDescriptorSet.h +++ b/include/nbl/video/IGPUDescriptorSet.h @@ -57,7 +57,6 @@ class IGPUDescriptorSet : public asset::IDescriptorSet dropDescriptors) = 0; + struct SDropDescriptorSetsParams + { + std::span drops; + uint32_t bufferCount = 0u; + uint32_t bufferViewCount = 0u; + uint32_t imageCount = 0u; + uint32_t accelerationStructureCount = 0u; + }; + virtual void nullifyDescriptors_impl(const SDropDescriptorSetsParams& params) = 0; virtual core::smart_refctd_ptr createRenderpass_impl(const IGPURenderpass::SCreationParams& params, IGPURenderpass::SCreationParamValidationResult&& validation) = 0; virtual core::smart_refctd_ptr createFramebuffer_impl(IGPUFramebuffer::SCreationParams&& params) = 0; diff --git a/include/nbl/video/alloc/SubAllocatedDescriptorSet.h b/include/nbl/video/alloc/SubAllocatedDescriptorSet.h index 601f0ee12a..45bf5f3d79 100644 --- a/include/nbl/video/alloc/SubAllocatedDescriptorSet.h +++ b/include/nbl/video/alloc/SubAllocatedDescriptorSet.h @@ -117,6 +117,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); diff --git a/src/nbl/video/CVulkanLogicalDevice.cpp b/src/nbl/video/CVulkanLogicalDevice.cpp index 6184e3f96b..577edd8855 100644 --- a/src/nbl/video/CVulkanLogicalDevice.cpp +++ b/src/nbl/video/CVulkanLogicalDevice.cpp @@ -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 vk_writeDescriptorSets(params.writes.size(),{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET,nullptr}); - core::vector vk_writeDescriptorSetAS(MaxDescriptorSetAsWrites,{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET_ACCELERATION_STRUCTURE_KHR,nullptr}); + core::vector vk_writeDescriptorSetAS(params.accelerationStructureCount,{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET_ACCELERATION_STRUCTURE_KHR,nullptr}); core::vector vk_bufferInfos(params.bufferCount); core::vector vk_imageInfos(params.imageCount); @@ -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 drops) +void CVulkanLogicalDevice::nullifyDescriptors_impl(const SDropDescriptorSetsParams& params) { + const auto& drops = params.drops; if (getEnabledFeatures().nullDescriptor) { return; } core::vector vk_writeDescriptorSets(drops.size(),{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET,nullptr}); - core::vector vk_writeDescriptorSetAS(MaxDescriptorSetAsWrites,{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET_ACCELERATION_STRUCTURE_KHR,nullptr}); + core::vector vk_writeDescriptorSetAS(params.accelerationStructureCount,{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 nullDescriptors(maxSize, 0u); diff --git a/src/nbl/video/CVulkanLogicalDevice.h b/src/nbl/video/CVulkanLogicalDevice.h index 3521a5d661..0d057a8c6c 100644 --- a/src/nbl/video/CVulkanLogicalDevice.h +++ b/src/nbl/video/CVulkanLogicalDevice.h @@ -283,7 +283,7 @@ class CVulkanLogicalDevice final : public ILogicalDevice // descriptor sets core::smart_refctd_ptr createDescriptorPool_impl(const IDescriptorPool::SCreateInfo& createInfo) override; void updateDescriptorSets_impl(const SUpdateDescriptorSetsParams& params) override; - void nullifyDescriptors_impl(const std::span dropDescriptors) override; + void nullifyDescriptors_impl(const SDropDescriptorSetsParams& params) override; // renderpasses and framebuffers core::smart_refctd_ptr createRenderpass_impl(const IGPURenderpass::SCreationParams& params, IGPURenderpass::SCreationParamValidationResult&& validation) override; diff --git a/src/nbl/video/ILogicalDevice.cpp b/src/nbl/video/ILogicalDevice.cpp index 2ad33545db..f18456dc5b 100644 --- a/src/nbl/video/ILogicalDevice.cpp +++ b/src/nbl/video/ILogicalDevice.cpp @@ -457,13 +457,35 @@ bool ILogicalDevice::updateDescriptorSets(const std::span 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; + 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; } @@ -473,7 +495,7 @@ bool ILogicalDevice::nullifyDescriptors(const std::spandropDescriptors(drop); } - nullifyDescriptors_impl(dropDescriptors); + nullifyDescriptors_impl(params); return true; } From bfd3be36ac97747829f9698503d59522203e42b1 Mon Sep 17 00:00:00 2001 From: deprilula28 Date: Sun, 17 Mar 2024 20:40:06 -0300 Subject: [PATCH 2/3] PR reviews --- include/nbl/video/ILogicalDevice.h | 2 ++ include/nbl/video/alloc/SubAllocatedDescriptorSet.h | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/include/nbl/video/ILogicalDevice.h b/include/nbl/video/ILogicalDevice.h index a732431fcc..1cbd7bcbbc 100644 --- a/include/nbl/video/ILogicalDevice.h +++ b/include/nbl/video/ILogicalDevice.h @@ -865,6 +865,7 @@ 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; @@ -875,6 +876,7 @@ 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 nullifyDescriptors_impl(const SDropDescriptorSetsParams& params) = 0; diff --git a/include/nbl/video/alloc/SubAllocatedDescriptorSet.h b/include/nbl/video/alloc/SubAllocatedDescriptorSet.h index 45bf5f3d79..e65d82ef6a 100644 --- a/include/nbl/video/alloc/SubAllocatedDescriptorSet.h +++ b/include/nbl/video/alloc/SubAllocatedDescriptorSet.h @@ -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; } From b0b2e30cdbcbd5a93219e8398fe4073c833b5016 Mon Sep 17 00:00:00 2001 From: deprilula28 Date: Sun, 17 Mar 2024 20:43:19 -0300 Subject: [PATCH 3/3] Didn't go in the last commit for some reason --- src/nbl/video/CVulkanLogicalDevice.cpp | 4 ++-- src/nbl/video/ILogicalDevice.cpp | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/nbl/video/CVulkanLogicalDevice.cpp b/src/nbl/video/CVulkanLogicalDevice.cpp index 577edd8855..d462881b99 100644 --- a/src/nbl/video/CVulkanLogicalDevice.cpp +++ b/src/nbl/video/CVulkanLogicalDevice.cpp @@ -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 vk_writeDescriptorSets(params.writes.size(),{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET,nullptr}); - core::vector vk_writeDescriptorSetAS(params.accelerationStructureCount,{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET_ACCELERATION_STRUCTURE_KHR,nullptr}); + core::vector vk_writeDescriptorSetAS(params.accelerationStructureWriteCount,{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET_ACCELERATION_STRUCTURE_KHR,nullptr}); core::vector vk_bufferInfos(params.bufferCount); core::vector vk_imageInfos(params.imageCount); @@ -740,7 +740,7 @@ void CVulkanLogicalDevice::nullifyDescriptors_impl(const SDropDescriptorSetsPara } core::vector vk_writeDescriptorSets(drops.size(),{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET,nullptr}); - core::vector vk_writeDescriptorSetAS(params.accelerationStructureCount,{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET_ACCELERATION_STRUCTURE_KHR,nullptr}); + core::vector vk_writeDescriptorSetAS(params.accelerationStructureWriteCount,{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET_ACCELERATION_STRUCTURE_KHR,nullptr}); size_t maxSize = core::max( core::max(params.bufferCount * sizeof(VkDescriptorBufferInfo), params.imageCount * sizeof(VkDescriptorImageInfo)), diff --git a/src/nbl/video/ILogicalDevice.cpp b/src/nbl/video/ILogicalDevice.cpp index f18456dc5b..72c048a0c8 100644 --- a/src/nbl/video/ILogicalDevice.cpp +++ b/src/nbl/video/ILogicalDevice.cpp @@ -422,6 +422,7 @@ bool ILogicalDevice::updateDescriptorSets(const std::span