From 378ea89b069dcb2961d0a61a26136d4dd361257c Mon Sep 17 00:00:00 2001 From: Fletterio Date: Fri, 3 May 2024 17:14:05 -0300 Subject: [PATCH 01/18] Got sampled images working. Samplers are next --- examples_tests | 2 +- include/nbl/asset/ICPUDescriptorSet.h | 1 + include/nbl/asset/IDescriptor.h | 5 ++++- .../asset/metadata/IRenderpassIndependentPipelineMetadata.h | 2 ++ include/nbl/video/CVulkanCommon.h | 4 ++++ include/nbl/video/IDescriptorPool.h | 1 + src/nbl/video/IDescriptorPool.cpp | 2 +- 7 files changed, 14 insertions(+), 3 deletions(-) diff --git a/examples_tests b/examples_tests index 870e1d5cda..f91ccdc92f 160000 --- a/examples_tests +++ b/examples_tests @@ -1 +1 @@ -Subproject commit 870e1d5cdac0b8649dadcf1a5b29cf8f2b61acb4 +Subproject commit f91ccdc92ffe5312280c0e8e1db7987f21f8c676 diff --git a/include/nbl/asset/ICPUDescriptorSet.h b/include/nbl/asset/ICPUDescriptorSet.h index 805f8e1bed..3931099372 100644 --- a/include/nbl/asset/ICPUDescriptorSet.h +++ b/include/nbl/asset/ICPUDescriptorSet.h @@ -105,6 +105,7 @@ class NBL_API2 ICPUDescriptorSet final : public IDescriptorSet*>(m_textureStorage.get()); break; case asset::IDescriptor::E_TYPE::ET_STORAGE_IMAGE: diff --git a/src/nbl/video/IDescriptorPool.cpp b/src/nbl/video/IDescriptorPool.cpp index cfce368542..ab6354b104 100644 --- a/src/nbl/video/IDescriptorPool.cpp +++ b/src/nbl/video/IDescriptorPool.cpp @@ -14,7 +14,7 @@ IDescriptorPool::IDescriptorPool(core::smart_refctd_ptr&& m_descriptorAllocators[static_cast(asset::IDescriptor::E_TYPE::ET_COUNT)] = std::make_unique(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)], m_creationParameters.flags.hasFlags(ECF_FREE_DESCRIPTOR_SET_BIT)); // Initialize the storages. - m_textureStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)]); + m_textureStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_SAMPLED_IMAGE)]); m_mutableSamplerStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)]); m_storageImageStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_STORAGE_IMAGE)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_INPUT_ATTACHMENT)]); m_UBO_SSBOStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_UNIFORM_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_STORAGE_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_UNIFORM_BUFFER_DYNAMIC)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_STORAGE_BUFFER_DYNAMIC)]); From 14a2a88a63c708946d1cabe857b89560beb4bcb0 Mon Sep 17 00:00:00 2001 From: Fletterio Date: Mon, 6 May 2024 16:20:46 -0300 Subject: [PATCH 02/18] Makes ISampler extend from IDescriptor --- include/nbl/asset/ICPUSampler.h | 2 +- include/nbl/asset/IDescriptor.h | 1 + include/nbl/asset/ISampler.h | 4 +++- src/nbl/video/IDescriptorPool.cpp | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/include/nbl/asset/ICPUSampler.h b/include/nbl/asset/ICPUSampler.h index e93538e693..384ddb8846 100644 --- a/include/nbl/asset/ICPUSampler.h +++ b/include/nbl/asset/ICPUSampler.h @@ -77,7 +77,7 @@ class ICPUSampler : public ISampler, public IAsset _NBL_STATIC_INLINE_CONSTEXPR auto AssetType = ET_SAMPLER; - inline E_TYPE getAssetType() const override { return AssetType; } + inline IAsset::E_TYPE getAssetType() const override { return AssetType; } bool canBeRestoredFrom(const IAsset* _other) const override { diff --git a/include/nbl/asset/IDescriptor.h b/include/nbl/asset/IDescriptor.h index 123e19aa96..552c733322 100644 --- a/include/nbl/asset/IDescriptor.h +++ b/include/nbl/asset/IDescriptor.h @@ -41,6 +41,7 @@ class IDescriptor : public virtual core::IReferenceCounted enum E_CATEGORY : uint8_t { EC_BUFFER = 0, + EC_SAMPLER, EC_IMAGE, EC_BUFFER_VIEW, EC_ACCELERATION_STRUCTURE, diff --git a/include/nbl/asset/ISampler.h b/include/nbl/asset/ISampler.h index d5d5b9157a..ed6ee6db4d 100644 --- a/include/nbl/asset/ISampler.h +++ b/include/nbl/asset/ISampler.h @@ -12,7 +12,7 @@ namespace nbl namespace asset { -class ISampler : public virtual core::IReferenceCounted +class ISampler : IDescriptor { public: //! Texture coord clamp mode outside [0.0, 1.0] @@ -120,6 +120,8 @@ class ISampler : public virtual core::IReferenceCounted } PACK_STRUCT; #include "nbl/nblunpack.h" + E_CATEGORY getTypeCategory() const override { return EC_SAMPLER; } + protected: ISampler(const SParams& _params) : m_params(_params) {} virtual ~ISampler() = default; diff --git a/src/nbl/video/IDescriptorPool.cpp b/src/nbl/video/IDescriptorPool.cpp index ab6354b104..335aef69d0 100644 --- a/src/nbl/video/IDescriptorPool.cpp +++ b/src/nbl/video/IDescriptorPool.cpp @@ -18,7 +18,7 @@ IDescriptorPool::IDescriptorPool(core::smart_refctd_ptr&& m_mutableSamplerStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)]); m_storageImageStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_STORAGE_IMAGE)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_INPUT_ATTACHMENT)]); m_UBO_SSBOStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_UNIFORM_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_STORAGE_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_UNIFORM_BUFFER_DYNAMIC)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_STORAGE_BUFFER_DYNAMIC)]); - m_UTB_STBStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_UNIFORM_TEXEL_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_STORAGE_BUFFER_DYNAMIC)]); + m_UTB_STBStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_UNIFORM_TEXEL_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_STORAGE_TEXEL_BUFFER)]); m_accelerationStructureStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_ACCELERATION_STRUCTURE)]); m_allocatedDescriptorSets = std::make_unique(m_creationParameters.maxSets); From c867585148344298a394eecb9caca58d106db157 Mon Sep 17 00:00:00 2001 From: Fletterio Date: Tue, 7 May 2024 17:58:49 -0300 Subject: [PATCH 03/18] Adding support for samplers, few bugfixes, updated IGPUDescriptorSet version increase behaviour, corrected some logger messages --- include/nbl/asset/IDescriptorSetLayout.h | 4 +- include/nbl/video/IGPUDescriptorSetLayout.h | 8 ++-- include/nbl/video/ILogicalDevice.h | 1 + src/nbl/video/CVulkanLogicalDevice.cpp | 12 +++++- src/nbl/video/IGPUCommandBuffer.cpp | 2 +- src/nbl/video/IGPUDescriptorSet.cpp | 47 ++++++++++++++++----- src/nbl/video/ILogicalDevice.cpp | 6 ++- 7 files changed, 60 insertions(+), 20 deletions(-) diff --git a/include/nbl/asset/IDescriptorSetLayout.h b/include/nbl/asset/IDescriptorSetLayout.h index 617717e24e..511f1e4c58 100644 --- a/include/nbl/asset/IDescriptorSetLayout.h +++ b/include/nbl/asset/IDescriptorSetLayout.h @@ -361,7 +361,7 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr { buildInfo_descriptors[static_cast(b.type)].emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); - if (b.type == IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) + if (b.type == IDescriptor::E_TYPE::ET_SAMPLER or b.type == IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) { if (b.samplers) buildInfo_immutableSamplers.emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); @@ -381,7 +381,7 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr for (const auto& b : _bindings) { - if (b.type == IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER && b.samplers) + if ((b.type == IDescriptor::E_TYPE::ET_SAMPLER or b.type == IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) and b.samplers) { const auto localOffset = m_immutableSamplerRedirect.getStorageOffset(typename CBindingRedirect::binding_number_t(b.binding)).data; assert(localOffset != m_immutableSamplerRedirect.Invalid); diff --git a/include/nbl/video/IGPUDescriptorSetLayout.h b/include/nbl/video/IGPUDescriptorSetLayout.h index 289468c046..c688e4181d 100644 --- a/include/nbl/video/IGPUDescriptorSetLayout.h +++ b/include/nbl/video/IGPUDescriptorSetLayout.h @@ -28,21 +28,21 @@ class IGPUDescriptorSetLayout : public asset::IDescriptorSetLayout, { for (const auto& binding : _bindings) { - if (binding.createFlags.hasFlags(SBinding::E_CREATE_FLAGS::ECF_UPDATE_AFTER_BIND_BIT) || binding.createFlags.hasFlags(SBinding::E_CREATE_FLAGS::ECF_UPDATE_UNUSED_WHILE_PENDING_BIT)) + if (not (binding.createFlags.hasFlags(SBinding::E_CREATE_FLAGS::ECF_UPDATE_AFTER_BIND_BIT) or binding.createFlags.hasFlags(SBinding::E_CREATE_FLAGS::ECF_UPDATE_UNUSED_WHILE_PENDING_BIT))) { - m_canUpdateAfterBind = true; + m_versionChangeInvalidatesCommandBuffer = true; break; } } } - inline bool canUpdateAfterBind() const { return m_canUpdateAfterBind; } + inline bool versionChangeInvalidatesCommandBuffer() const { return m_versionChangeInvalidatesCommandBuffer; } protected: virtual ~IGPUDescriptorSetLayout() = default; bool m_isPushDescLayout = false; - bool m_canUpdateAfterBind = false; + bool m_versionChangeInvalidatesCommandBuffer = false; }; } diff --git a/include/nbl/video/ILogicalDevice.h b/include/nbl/video/ILogicalDevice.h index da87b69a1a..1179a38b54 100644 --- a/include/nbl/video/ILogicalDevice.h +++ b/include/nbl/video/ILogicalDevice.h @@ -889,6 +889,7 @@ class NBL_API2 ILogicalDevice : public core::IReferenceCounted, public IDeviceMe std::span writes; std::span copies; const asset::IDescriptor::E_TYPE* pWriteTypes; + uint32_t samplerCount = 0u; uint32_t bufferCount = 0u; uint32_t bufferViewCount = 0u; uint32_t imageCount = 0u; diff --git a/src/nbl/video/CVulkanLogicalDevice.cpp b/src/nbl/video/CVulkanLogicalDevice.cpp index 07ff4ae77c..bec603b92b 100644 --- a/src/nbl/video/CVulkanLogicalDevice.cpp +++ b/src/nbl/video/CVulkanLogicalDevice.cpp @@ -551,7 +551,7 @@ core::smart_refctd_ptr CVulkanLogicalDevice::createDesc vkDescSetLayoutBinding.stageFlags = getVkShaderStageFlagsFromShaderStage(binding.stageFlags); vkDescSetLayoutBinding.pImmutableSamplers = nullptr; - if (binding.type==asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER && binding.samplers && binding.count) + if ((binding.type == asset::IDescriptor::E_TYPE::ET_SAMPLER or binding.type==asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) and binding.samplers and binding.count) { // If descriptorType is VK_DESCRIPTOR_TYPE_SAMPLER or VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, and descriptorCount is not 0 and pImmutableSamplers is not NULL: // pImmutableSamplers must be a valid pointer to an array of descriptorCount valid VkSampler handles. @@ -681,6 +681,16 @@ void CVulkanLogicalDevice::updateDescriptorSets_impl(const SUpdateDescriptorSets outWrite->descriptorCount = write.count; switch (asset::IDescriptor::GetTypeCategory(type)) { + case asset::IDescriptor::EC_SAMPLER: + { + outWrite->pImageInfo = outImageInfo; + for (auto j = 0u; j < write.count; j++, outImageInfo++) + { + const auto& imageInfo = infos[j].info.image; + // Write was validated so we can just get the sampler + outImageInfo->sampler = static_cast(imageInfo.sampler.get())->getInternalObject(); + } + } break; case asset::IDescriptor::EC_BUFFER: { outWrite->pBufferInfo = outBufferInfo; diff --git a/src/nbl/video/IGPUCommandBuffer.cpp b/src/nbl/video/IGPUCommandBuffer.cpp index 23ef9c7f9d..5cf306ede2 100644 --- a/src/nbl/video/IGPUCommandBuffer.cpp +++ b/src/nbl/video/IGPUCommandBuffer.cpp @@ -728,7 +728,7 @@ bool IGPUCommandBuffer::bindDescriptorSets( return false; for (uint32_t i=0u; igetLayout()->canUpdateAfterBind()) + if (pDescriptorSets[i] && pDescriptorSets[i]->getLayout()->versionChangeInvalidatesCommandBuffer()) { const auto currentVersion = pDescriptorSets[i]->getVersion(); diff --git a/src/nbl/video/IGPUDescriptorSet.cpp b/src/nbl/video/IGPUDescriptorSet.cpp index 3313ed3388..f735decf47 100644 --- a/src/nbl/video/IGPUDescriptorSet.cpp +++ b/src/nbl/video/IGPUDescriptorSet.cpp @@ -59,14 +59,16 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor } core::smart_refctd_ptr* mutableSamplers = nullptr; - if (descriptorType==asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER && write.info->info.image.sampler) + if ((descriptorType == asset::IDescriptor::E_TYPE::ET_SAMPLER or descriptorType == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) and write.info->info.image.sampler) { - if (m_layout->getImmutableSamplerRedirect().getCount(IGPUDescriptorSetLayout::CBindingRedirect::binding_number_t{ write.binding }) != 0) + if (m_layout->getImmutableSamplerRedirect().getCount(IGPUDescriptorSetLayout::CBindingRedirect::binding_number_t{ write.binding }) != 0 + // providing samplers to an immutable sampler binding is only a problem if the descriptor is not a combined image sampler + and descriptorType == asset::IDescriptor::E_TYPE::ET_SAMPLER) { if (debugName) - m_pool->m_logger.log("Descriptor set (%s, %p) doesn't allow immutable samplers at binding %u, but immutable samplers found.", system::ILogger::ELL_ERROR, debugName, this, write.binding); + m_pool->m_logger.log("Trying to write samplers at binding %u of Descriptor set (%s, %p), but those are immutable.", system::ILogger::ELL_ERROR, debugName, this, write.binding); else - m_pool->m_logger.log("Descriptor set (%p) doesn't allow immutable samplers at binding %u, but immutable samplers found.", system::ILogger::ELL_ERROR, this, write.binding); + m_pool->m_logger.log("Trying to write samplers at binding %u of Descriptor set (%p), but those are immutable.", system::ILogger::ELL_ERROR, this, write.binding); return asset::IDescriptor::E_TYPE::ET_COUNT; } @@ -82,8 +84,14 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor return asset::IDescriptor::E_TYPE::ET_COUNT; } auto* sampler = write.info[i].info.image.sampler.get(); - if (!sampler || !sampler->isCompatibleDevicewise(write.dstSet)) - { + if (not sampler) { + if (debugName) + m_pool->m_logger.log("Null sampler provided when trying to write descriptor set (%s, %p). All writes should provide a sampler.", system::ILogger::ELL_ERROR, debugName, write.dstSet); + else + m_pool->m_logger.log("Null sampler provided when trying to write descriptor set (%p). All writes should provide a sampler.", system::ILogger::ELL_ERROR, write.dstSet); + return asset::IDescriptor::E_TYPE::ET_COUNT; + } + else if (not sampler->isCompatibleDevicewise(write.dstSet)) { const char* samplerDebugName = sampler->getDebugName(); if (samplerDebugName && debugName) m_pool->m_logger.log("Sampler (%s, %p) does not exist or is not device-compatible with descriptor set (%s, %p).", system::ILogger::ELL_ERROR, samplerDebugName, sampler, debugName, write.dstSet); @@ -116,7 +124,11 @@ void IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSe assert(descriptors); core::smart_refctd_ptr* mutableSamplers = nullptr; - if (type==asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER && write.info->info.image.sampler) + if ((type == asset::IDescriptor::E_TYPE::ET_SAMPLER or type == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) + and write.info->info.image.sampler + // Since the write was validated, immutable samplers will only show up here if we're trying to write to a combined image sampler, in which case + // the samplers will be ignored by the Vulkan call. We avoid lifetime tracking them in this case + and m_layout->getImmutableSamplerRedirect().getCount(IGPUDescriptorSetLayout::CBindingRedirect::binding_number_t{ write.binding }) == 0) { mutableSamplers = getMutableSamplers(write.binding); assert(mutableSamplers); @@ -129,8 +141,10 @@ void IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSe if (mutableSamplers) mutableSamplers[j] = write.info[j].info.image.sampler; } - - incrementVersion(); + auto& bindingRedirect = m_layout->getDescriptorRedirect(type); + auto bindingCreateFlags = bindingRedirect.getCreateFlags(bindingRedirect.findBindingStorageIndex({ write.binding })); + if (not (bindingCreateFlags.hasFlags(IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_UPDATE_AFTER_BIND_BIT) or bindingCreateFlags.hasFlags(IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_UPDATE_UNUSED_WHILE_PENDING_BIT))) + incrementVersion(); } void IGPUDescriptorSet::dropDescriptors(const IGPUDescriptorSet::SDropDescriptorSet& drop) @@ -152,7 +166,10 @@ void IGPUDescriptorSet::dropDescriptors(const IGPUDescriptorSet::SDropDescriptor // we only increment the version to detect UPDATE-AFTER-BIND and automagically invalidate descriptor sets // so, only if we do the path that writes descriptors, do we want to increment version - if (getOriginDevice()->getEnabledFeatures().nullDescriptor) + auto& bindingRedirect = m_layout->getDescriptorRedirect(descriptorType); + auto bindingCreateFlags = bindingRedirect.getCreateFlags(bindingRedirect.findBindingStorageIndex({ drop.binding })); + bool shouldIncrementVersion = not (bindingCreateFlags.hasFlags(IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_UPDATE_AFTER_BIND_BIT) or bindingCreateFlags.hasFlags(IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_UPDATE_UNUSED_WHILE_PENDING_BIT)); + if (getOriginDevice()->getEnabledFeatures().nullDescriptor or shouldIncrementVersion) { incrementVersion(); } @@ -193,6 +210,8 @@ void IGPUDescriptorSet::processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& { assert(copy.dstSet == this); + bool shouldIncrementVersion = false; + for (uint32_t t = 0; t < static_cast(asset::IDescriptor::E_TYPE::ET_COUNT); ++t) { const auto type = static_cast(t); @@ -210,9 +229,15 @@ void IGPUDescriptorSet::processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& if (srcSamplers && dstSamplers) std::copy_n(srcSamplers, copy.count, dstSamplers); + + auto& bindingRedirect = m_layout->getDescriptorRedirect(type); + auto bindingCreateFlags = bindingRedirect.getCreateFlags(bindingRedirect.findBindingStorageIndex({ copy.srcBinding })); + if (not (bindingCreateFlags.hasFlags(IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_UPDATE_AFTER_BIND_BIT) or bindingCreateFlags.hasFlags(IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_UPDATE_UNUSED_WHILE_PENDING_BIT))) + shouldIncrementVersion = true; } - incrementVersion(); + if (shouldIncrementVersion) + incrementVersion(); } } \ No newline at end of file diff --git a/src/nbl/video/ILogicalDevice.cpp b/src/nbl/video/ILogicalDevice.cpp index c902efb63b..1a59d40028 100644 --- a/src/nbl/video/ILogicalDevice.cpp +++ b/src/nbl/video/ILogicalDevice.cpp @@ -392,7 +392,8 @@ core::smart_refctd_ptr ILogicalDevice::createDescriptor dynamicSSBOCount++; else if (binding.type==asset::IDescriptor::E_TYPE::ET_UNIFORM_BUFFER_DYNAMIC) dynamicUBOCount++; - else if (binding.type==asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER && binding.samplers) + // If binding comes with samplers, we're binding an immutable sampler + else if ((binding.type == asset::IDescriptor::E_TYPE::ET_SAMPLER or binding.type==asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) and binding.samplers) { auto* samplers = binding.samplers; for (uint32_t i=0u; ivalidateWrite(write))) { + case asset::IDescriptor::EC_SAMPLER: + params.samplerCount += writeCount; + break; case asset::IDescriptor::EC_BUFFER: params.bufferCount += writeCount; break; From 2f6b88a47b2b2806c545157c4c30fce553443f13 Mon Sep 17 00:00:00 2001 From: Fletterio Date: Thu, 9 May 2024 20:54:41 -0300 Subject: [PATCH 04/18] That should be all the logic needed to support samplers. Testing tomorrow --- include/nbl/asset/ICPUDescriptorSet.h | 31 +----------- include/nbl/asset/ICPUDescriptorSetLayout.h | 42 ++++++++--------- include/nbl/asset/IDescriptor.h | 6 +-- include/nbl/asset/IDescriptorSet.h | 10 ++-- include/nbl/asset/IDescriptorSetLayout.h | 52 ++++++++++++--------- include/nbl/asset/ISampler.h | 2 +- include/nbl/video/IDescriptorPool.h | 12 +++-- include/nbl/video/IGPUDescriptorSet.h | 14 +++--- include/nbl/video/ILogicalDevice.h | 3 +- src/nbl/asset/ICPUDescriptorSet.cpp | 38 +++++++++++++-- src/nbl/video/CVulkanLogicalDevice.cpp | 1 + src/nbl/video/IDescriptorPool.cpp | 14 +++--- src/nbl/video/IGPUDescriptorSet.cpp | 41 ++++++++-------- src/nbl/video/ILogicalDevice.cpp | 9 ++-- 14 files changed, 146 insertions(+), 129 deletions(-) diff --git a/include/nbl/asset/ICPUDescriptorSet.h b/include/nbl/asset/ICPUDescriptorSet.h index 3931099372..d0fbe1bf19 100644 --- a/include/nbl/asset/ICPUDescriptorSet.h +++ b/include/nbl/asset/ICPUDescriptorSet.h @@ -101,36 +101,7 @@ class NBL_API2 ICPUDescriptorSet final : public IDescriptorSet m_descriptorInfos[static_cast(IDescriptor::E_TYPE::ET_COUNT)]; diff --git a/include/nbl/asset/ICPUDescriptorSetLayout.h b/include/nbl/asset/ICPUDescriptorSetLayout.h index c4e7041013..c80ad92167 100644 --- a/include/nbl/asset/ICPUDescriptorSetLayout.h +++ b/include/nbl/asset/ICPUDescriptorSetLayout.h @@ -34,20 +34,20 @@ class ICPUDescriptorSetLayout : public IDescriptorSetLayout, public for (uint32_t t = 0; t < static_cast(asset::IDescriptor::E_TYPE::ET_COUNT); ++t) cp->m_descriptorRedirects[t] = m_descriptorRedirects[t].clone(); cp->m_immutableSamplerRedirect = m_immutableSamplerRedirect.clone(); - cp->m_mutableSamplerRedirect = m_mutableSamplerRedirect.clone(); + cp->m_mutableCombinedSamplerRedirect = m_mutableCombinedSamplerRedirect.clone(); - if (m_samplers) + if (m_immutableSamplers) { - cp->m_samplers = core::make_refctd_dynamic_array(m_samplers->size()); + cp->m_immutableSamplers = core::make_refctd_dynamic_array(m_immutableSamplers->size()); if (_depth > 0u) { - for (size_t i = 0ull; i < m_samplers->size(); ++i) - (*cp->m_samplers)[i] = core::smart_refctd_ptr_static_cast((*m_samplers)[i]->clone(_depth - 1u)); + for (size_t i = 0ull; i < m_immutableSamplers->size(); ++i) + (*cp->m_immutableSamplers)[i] = core::smart_refctd_ptr_static_cast((*m_immutableSamplers)[i]->clone(_depth - 1u)); } else { - std::copy(m_samplers->begin(), m_samplers->end(), cp->m_samplers->begin()); + std::copy(m_immutableSamplers->begin(), m_immutableSamplers->end(), cp->m_immutableSamplers->begin()); } } @@ -60,9 +60,9 @@ class ICPUDescriptorSetLayout : public IDescriptorSetLayout, public for (uint32_t t = 0; t < static_cast(asset::IDescriptor::E_TYPE::ET_COUNT); ++t) result += m_descriptorRedirects[t].conservativeSizeEstimate(); result += m_immutableSamplerRedirect.conservativeSizeEstimate(); - result += m_mutableSamplerRedirect.conservativeSizeEstimate(); + result += m_mutableCombinedSamplerRedirect.conservativeSizeEstimate(); - result += m_samplers->size() * sizeof(void*); + result += m_immutableSamplers->size() * sizeof(void*); return result; } @@ -74,9 +74,9 @@ class ICPUDescriptorSetLayout : public IDescriptorSetLayout, public if (referenceLevelsBelowToConvert) { --referenceLevelsBelowToConvert; - if (m_samplers) + if (m_immutableSamplers) { - for (auto it=m_samplers->begin(); it!=m_samplers->end(); it++) + for (auto it=m_immutableSamplers->begin(); it!=m_immutableSamplers->end(); it++) it->get()->convertToDummyObject(referenceLevelsBelowToConvert); } } @@ -90,15 +90,15 @@ class ICPUDescriptorSetLayout : public IDescriptorSetLayout, public auto* other = static_cast(_other); if (getTotalBindingCount() != other->getTotalBindingCount()) return false; - if ((!m_samplers) != (!other->m_samplers)) + if ((!m_immutableSamplers) != (!other->m_immutableSamplers)) return false; - if (m_samplers && m_samplers->size() != other->m_samplers->size()) + if (m_immutableSamplers && m_immutableSamplers->size() != other->m_immutableSamplers->size()) return false; - if (m_samplers) + if (m_immutableSamplers) { - for (uint32_t i = 0u; i < m_samplers->size(); ++i) + for (uint32_t i = 0u; i < m_immutableSamplers->size(); ++i) { - if (!(*m_samplers)[i]->canBeRestoredFrom((*other->m_samplers)[i].get())) + if (!(*m_immutableSamplers)[i]->canBeRestoredFrom((*other->m_immutableSamplers)[i].get())) return false; } } @@ -115,21 +115,21 @@ class ICPUDescriptorSetLayout : public IDescriptorSetLayout, public return; --_levelsBelow; - if (m_samplers) + if (m_immutableSamplers) { - for (uint32_t i = 0u; i < m_samplers->size(); ++i) - restoreFromDummy_impl_call((*m_samplers)[i].get(), (*other->m_samplers)[i].get(), _levelsBelow); + for (uint32_t i = 0u; i < m_immutableSamplers->size(); ++i) + restoreFromDummy_impl_call((*m_immutableSamplers)[i].get(), (*other->m_immutableSamplers)[i].get(), _levelsBelow); } } bool isAnyDependencyDummy_impl(uint32_t _levelsBelow) const override { --_levelsBelow; - if (m_samplers) + if (m_immutableSamplers) { - for (uint32_t i = 0u; i < m_samplers->size(); ++i) + for (uint32_t i = 0u; i < m_immutableSamplers->size(); ++i) { - if ((*m_samplers)[i]->isAnyDependencyDummy(_levelsBelow)) + if ((*m_immutableSamplers)[i]->isAnyDependencyDummy(_levelsBelow)) return true; } } diff --git a/include/nbl/asset/IDescriptor.h b/include/nbl/asset/IDescriptor.h index 552c733322..c315308582 100644 --- a/include/nbl/asset/IDescriptor.h +++ b/include/nbl/asset/IDescriptor.h @@ -51,25 +51,23 @@ class IDescriptor : public virtual core::IReferenceCounted { switch (type) { + case E_TYPE::ET_SAMPLER: + return EC_SAMPLER; case E_TYPE::ET_COMBINED_IMAGE_SAMPLER: case E_TYPE::ET_SAMPLED_IMAGE: case E_TYPE::ET_STORAGE_IMAGE: case E_TYPE::ET_INPUT_ATTACHMENT: return EC_IMAGE; - break; case E_TYPE::ET_UNIFORM_TEXEL_BUFFER: case E_TYPE::ET_STORAGE_TEXEL_BUFFER: return EC_BUFFER_VIEW; - break; case E_TYPE::ET_UNIFORM_BUFFER: case E_TYPE::ET_STORAGE_BUFFER: case E_TYPE::ET_UNIFORM_BUFFER_DYNAMIC: case E_TYPE::ET_STORAGE_BUFFER_DYNAMIC: return EC_BUFFER; - break; case E_TYPE::ET_ACCELERATION_STRUCTURE: return EC_ACCELERATION_STRUCTURE; - break; default: break; } diff --git a/include/nbl/asset/IDescriptorSet.h b/include/nbl/asset/IDescriptorSet.h index a4aca538b4..2271fb0b9b 100644 --- a/include/nbl/asset/IDescriptorSet.h +++ b/include/nbl/asset/IDescriptorSet.h @@ -92,17 +92,17 @@ class IDescriptorSet : public virtual core::IReferenceCounted // TODO: try to re } ~SDescriptorInfo() { - if (desc && desc->getTypeCategory()==IDescriptor::EC_IMAGE) + if (desc and (desc->getTypeCategory()==IDescriptor::EC_IMAGE or desc->getTypeCategory() == IDescriptor::EC_SAMPLER)) info.image.sampler = nullptr; } inline SDescriptorInfo& operator=(const SDescriptorInfo& other) { - if (desc && desc->getTypeCategory()==IDescriptor::EC_IMAGE) + if (desc and (desc->getTypeCategory()==IDescriptor::EC_IMAGE or desc->getTypeCategory() == IDescriptor::EC_SAMPLER)) info.image.sampler = nullptr; desc = other.desc; const auto type = desc->getTypeCategory(); - if (type!=IDescriptor::EC_IMAGE) + if (type!=IDescriptor::EC_IMAGE and type!=IDescriptor::EC_SAMPLER) info.buffer = other.info.buffer; else info.image = other.info.image; @@ -110,13 +110,13 @@ class IDescriptorSet : public virtual core::IReferenceCounted // TODO: try to re } inline SDescriptorInfo& operator=(SDescriptorInfo&& other) { - if (desc && desc->getTypeCategory()==IDescriptor::EC_IMAGE) + if (desc and (desc->getTypeCategory() == IDescriptor::EC_IMAGE or desc->getTypeCategory() == IDescriptor::EC_SAMPLER)) info.image = {nullptr,IImage::LAYOUT::UNDEFINED}; desc = std::move(other.desc); if (desc) { const auto type = desc->getTypeCategory(); - if (type!=IDescriptor::EC_IMAGE) + if (type!=IDescriptor::EC_IMAGE and type!=IDescriptor::EC_SAMPLER) info.buffer = other.info.buffer; else info.image = other.info.image; diff --git a/include/nbl/asset/IDescriptorSetLayout.h b/include/nbl/asset/IDescriptorSetLayout.h index 511f1e4c58..9ef0a2e721 100644 --- a/include/nbl/asset/IDescriptorSetLayout.h +++ b/include/nbl/asset/IDescriptorSetLayout.h @@ -317,18 +317,20 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr if (!areRedirectsEqual(m_immutableSamplerRedirect, _other->m_immutableSamplerRedirect)) return false; - if (!areRedirectsEqual(m_mutableSamplerRedirect, _other->m_mutableSamplerRedirect)) + if (!areRedirectsEqual(m_mutableCombinedSamplerRedirect, _other->m_mutableCombinedSamplerRedirect)) return false; - if (m_samplers && _other->m_samplers) - return std::equal(m_samplers->begin(), m_samplers->end(), _other->m_samplers->begin(), _other->m_samplers->end()); + if (m_immutableSamplers && _other->m_immutableSamplers) + return std::equal(m_immutableSamplers->begin(), m_immutableSamplers->end(), _other->m_immutableSamplers->begin(), _other->m_immutableSamplers->end()); else - return !m_samplers && !_other->m_samplers; + return !m_immutableSamplers && !_other->m_immutableSamplers; } - inline uint32_t getTotalMutableSamplerCount() const { return m_mutableSamplerRedirect.getTotalCount(); } + inline uint32_t getTotalMutableCombinedSamplerCount() const { return m_mutableCombinedSamplerRedirect.getTotalCount(); } + // Immutable sampler descriptors are not counted here inline uint32_t getTotalDescriptorCount(const IDescriptor::E_TYPE type) const { return m_descriptorRedirects[static_cast(type)].getTotalCount(); } + // Immutable sampler bindings are not counted here inline uint32_t getTotalBindingCount() const { uint32_t result = 0u; @@ -340,14 +342,14 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr inline const CBindingRedirect& getDescriptorRedirect(const IDescriptor::E_TYPE type) const { return m_descriptorRedirects[static_cast(type)]; } inline const CBindingRedirect& getImmutableSamplerRedirect() const { return m_immutableSamplerRedirect; } - inline const CBindingRedirect& getMutableSamplerRedirect() const { return m_mutableSamplerRedirect; } + inline const CBindingRedirect& getMutableCombinedSamplerRedirect() const { return m_mutableCombinedSamplerRedirect; } inline core::SRange> getImmutableSamplers() const { - if (!m_samplers) + if (!m_immutableSamplers) return { nullptr, nullptr }; - return { m_samplers->cbegin(), m_samplers->cend() }; + return { m_immutableSamplers->cbegin(), m_immutableSamplers->cend() }; } protected: @@ -359,14 +361,22 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr for (const auto& b : _bindings) { - buildInfo_descriptors[static_cast(b.type)].emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); - - if (b.type == IDescriptor::E_TYPE::ET_SAMPLER or b.type == IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) - { - if (b.samplers) - buildInfo_immutableSamplers.emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); - else - buildInfo_mutableSamplers.emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); + switch (b.type) { + case IDescriptor::E_TYPE::ET_SAMPLER: + if (b.samplers) + buildInfo_immutableSamplers.emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); + else + buildInfo_descriptors[static_cast(b.type)].emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); + break; + case IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER: + if (b.samplers) + buildInfo_immutableSamplers.emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); + else + buildInfo_mutableSamplers.emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); + [[fallthrough]]; + default: + buildInfo_descriptors[static_cast(b.type)].emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); + break; } } @@ -374,10 +384,10 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr m_descriptorRedirects[type] = CBindingRedirect(std::move(buildInfo_descriptors[type])); m_immutableSamplerRedirect = CBindingRedirect(std::move(buildInfo_immutableSamplers)); - m_mutableSamplerRedirect = CBindingRedirect(std::move(buildInfo_mutableSamplers)); + m_mutableCombinedSamplerRedirect = CBindingRedirect(std::move(buildInfo_mutableSamplers)); const uint32_t immutableSamplerCount = m_immutableSamplerRedirect.getTotalCount(); - m_samplers = immutableSamplerCount ? core::make_refctd_dynamic_array>>(immutableSamplerCount) : nullptr; + m_immutableSamplers = immutableSamplerCount ? core::make_refctd_dynamic_array>>(immutableSamplerCount) : nullptr; for (const auto& b : _bindings) { @@ -386,7 +396,7 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr const auto localOffset = m_immutableSamplerRedirect.getStorageOffset(typename CBindingRedirect::binding_number_t(b.binding)).data; assert(localOffset != m_immutableSamplerRedirect.Invalid); - auto* dst = m_samplers->begin() + localOffset; + auto* dst = m_immutableSamplers->begin() + localOffset; std::copy_n(b.samplers, b.count, dst); } } @@ -396,9 +406,9 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr CBindingRedirect m_descriptorRedirects[static_cast(asset::IDescriptor::E_TYPE::ET_COUNT)]; CBindingRedirect m_immutableSamplerRedirect; - CBindingRedirect m_mutableSamplerRedirect; + CBindingRedirect m_mutableCombinedSamplerRedirect; - core::smart_refctd_dynamic_array> m_samplers = nullptr; + core::smart_refctd_dynamic_array> m_immutableSamplers = nullptr; }; } diff --git a/include/nbl/asset/ISampler.h b/include/nbl/asset/ISampler.h index ed6ee6db4d..c9f7aa9834 100644 --- a/include/nbl/asset/ISampler.h +++ b/include/nbl/asset/ISampler.h @@ -12,7 +12,7 @@ namespace nbl namespace asset { -class ISampler : IDescriptor +class ISampler : public IDescriptor { public: //! Texture coord clamp mode outside [0.0, 1.0] diff --git a/include/nbl/video/IDescriptorPool.h b/include/nbl/video/IDescriptorPool.h index 389fa0d5ac..309aa8608e 100644 --- a/include/nbl/video/IDescriptorPool.h +++ b/include/nbl/video/IDescriptorPool.h @@ -55,7 +55,7 @@ class NBL_API2 IDescriptorPool : public IBackendObject return data[idx]; } - inline uint32_t getMutableSamplerOffset() const { return data[static_cast(asset::IDescriptor::E_TYPE::ET_COUNT)]; } + inline uint32_t getMutableCombinedSamplerOffset() const { return data[static_cast(asset::IDescriptor::E_TYPE::ET_COUNT)]; } inline uint32_t getSetOffset() const { return data[static_cast(asset::IDescriptor::E_TYPE::ET_COUNT) + 1]; } inline uint32_t& getSetOffset() { return data[static_cast(asset::IDescriptor::E_TYPE::ET_COUNT) + 1]; } @@ -101,6 +101,9 @@ class NBL_API2 IDescriptorPool : public IBackendObject core::smart_refctd_ptr* baseAddress; switch (type) { + case asset::IDescriptor::E_TYPE::ET_SAMPLER: + baseAddress = reinterpret_cast*>(m_MutableStandaloneSamplerStorage.get()); + break; case asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER: case asset::IDescriptor::E_TYPE::ET_SAMPLED_IMAGE: baseAddress = reinterpret_cast*>(m_textureStorage.get()); @@ -140,9 +143,9 @@ class NBL_API2 IDescriptorPool : public IBackendObject return baseAddress; } - inline core::smart_refctd_ptr* getMutableSamplerStorage() const + inline core::smart_refctd_ptr* getMutableCombinedSamplerStorage() const { - return reinterpret_cast*>(m_mutableSamplerStorage.get()); + return reinterpret_cast*>(m_mutableCombinedSamplerStorage.get()); } friend class IGPUDescriptorSet; @@ -219,7 +222,8 @@ class NBL_API2 IDescriptorPool : public IBackendObject std::unique_ptr m_allocatedDescriptorSets = nullptr; // This array might be sparse. std::unique_ptr>[]> m_textureStorage; - std::unique_ptr>[]> m_mutableSamplerStorage; + std::unique_ptr>[]> m_mutableCombinedSamplerStorage; + std::unique_ptr>[]> m_MutableStandaloneSamplerStorage; std::unique_ptr>[]> m_storageImageStorage; // storage image | input attachment std::unique_ptr>[]> m_UBO_SSBOStorage; // ubo | ssbo | ubo dynamic | ssbo dynamic std::unique_ptr>[]> m_UTB_STBStorage; // utb | stb diff --git a/include/nbl/video/IGPUDescriptorSet.h b/include/nbl/video/IGPUDescriptorSet.h index c8ce45b097..5ee17a3433 100644 --- a/include/nbl/video/IGPUDescriptorSet.h +++ b/include/nbl/video/IGPUDescriptorSet.h @@ -99,13 +99,13 @@ class IGPUDescriptorSet : public asset::IDescriptorSet* getMutableSamplers(const uint32_t binding) const + inline core::smart_refctd_ptr* getMutableCombinedSamplers(const uint32_t binding) const { - const auto localOffset = getLayout()->getMutableSamplerRedirect().getStorageOffset(redirect_t::binding_number_t{ binding }).data; - if (localOffset == getLayout()->getMutableSamplerRedirect().Invalid) + const auto localOffset = getLayout()->getMutableCombinedSamplerRedirect().getStorageOffset(redirect_t::binding_number_t{ binding }).data; + if (localOffset == getLayout()->getMutableCombinedSamplerRedirect().Invalid) return nullptr; - auto* samplers = getAllMutableSamplers(); + auto* samplers = getAllMutableCombinedSamplers(); if (!samplers) return nullptr; @@ -125,13 +125,13 @@ class IGPUDescriptorSet : public asset::IDescriptorSet* getAllMutableSamplers() const + inline core::smart_refctd_ptr* getAllMutableCombinedSamplers() const { - auto* baseAddress = m_pool->getMutableSamplerStorage(); + auto* baseAddress = m_pool->getMutableCombinedSamplerStorage(); if (baseAddress == nullptr) return nullptr; - const auto offset = m_storageOffsets.getMutableSamplerOffset(); + const auto offset = m_storageOffsets.getMutableCombinedSamplerOffset(); if (offset == ~0u) return nullptr; diff --git a/include/nbl/video/ILogicalDevice.h b/include/nbl/video/ILogicalDevice.h index 1179a38b54..84557c2edb 100644 --- a/include/nbl/video/ILogicalDevice.h +++ b/include/nbl/video/ILogicalDevice.h @@ -889,10 +889,9 @@ class NBL_API2 ILogicalDevice : public core::IReferenceCounted, public IDeviceMe std::span writes; std::span copies; const asset::IDescriptor::E_TYPE* pWriteTypes; - uint32_t samplerCount = 0u; uint32_t bufferCount = 0u; uint32_t bufferViewCount = 0u; - uint32_t imageCount = 0u; + uint32_t imageCount = 0u; // combined image/samplers as well as samplers belong here, since they're written through a VkDescriptorImageInfo uint32_t accelerationStructureCount = 0u; uint32_t accelerationStructureWriteCount = 0u; }; diff --git a/src/nbl/asset/ICPUDescriptorSet.cpp b/src/nbl/asset/ICPUDescriptorSet.cpp index 5cdf430fd9..0f6442b24e 100644 --- a/src/nbl/asset/ICPUDescriptorSet.cpp +++ b/src/nbl/asset/ICPUDescriptorSet.cpp @@ -59,7 +59,7 @@ core::smart_refctd_ptr ICPUDescriptorSet::clone(uint32_t _depth) const auto category = getCategoryFromType(type); - if (category == IDescriptor::E_CATEGORY::EC_IMAGE) + if (category == IDescriptor::E_CATEGORY::EC_IMAGE or category == IDescriptor::E_CATEGORY::EC_SAMPLER) dstDescriptorInfo.info.image = srcDescriptorInfo.info.image; else dstDescriptorInfo.info.buffer = srcDescriptorInfo.info.buffer; @@ -71,7 +71,9 @@ core::smart_refctd_ptr ICPUDescriptorSet::clone(uint32_t _depth) const assert(srcDescriptorInfo.desc); IAsset* descriptor = nullptr; - if (category == IDescriptor::E_CATEGORY::EC_IMAGE) + if (category == IDescriptor::E_CATEGORY::EC_SAMPLER) + descriptor = static_cast(srcDescriptorInfo.desc.get()); + else if (category == IDescriptor::E_CATEGORY::EC_IMAGE) descriptor = static_cast(srcDescriptorInfo.desc.get()); else if (category == IDescriptor::E_CATEGORY::EC_BUFFER_VIEW) descriptor = static_cast(srcDescriptorInfo.desc.get()); @@ -80,7 +82,9 @@ core::smart_refctd_ptr ICPUDescriptorSet::clone(uint32_t _depth) const auto descriptorClone = descriptor->clone(_depth - 1); - if (category == IDescriptor::E_CATEGORY::EC_IMAGE) + if (category == IDescriptor::E_CATEGORY::EC_SAMPLER) + dstDescriptorInfo.desc = core::smart_refctd_ptr_static_cast(std::move(descriptorClone)); + else if (category == IDescriptor::E_CATEGORY::EC_IMAGE) dstDescriptorInfo.desc = core::smart_refctd_ptr_static_cast(std::move(descriptorClone)); else if (category == IDescriptor::E_CATEGORY::EC_BUFFER_VIEW) dstDescriptorInfo.desc = core::smart_refctd_ptr_static_cast(std::move(descriptorClone)); @@ -91,7 +95,7 @@ core::smart_refctd_ptr ICPUDescriptorSet::clone(uint32_t _depth) const // Clone the sampler. { - if ((category == IDescriptor::E_CATEGORY::EC_IMAGE) && srcDescriptorInfo.info.image.sampler) + if ((category == IDescriptor::E_CATEGORY::EC_IMAGE or category == IDescriptor::E_CATEGORY::EC_SAMPLER) and srcDescriptorInfo.info.image.sampler) dstDescriptorInfo.info.image.sampler = core::smart_refctd_ptr_static_cast(srcDescriptorInfo.info.image.sampler->clone(_depth - 1u)); } } @@ -132,6 +136,14 @@ void ICPUDescriptorSet::convertToDummyObject(uint32_t referenceLevelsBelowToConv static_cast(descriptorInfos[i].desc.get())->convertToDummyObject(referenceLevelsBelowToConvert); break; + case IDescriptor::E_CATEGORY::EC_SAMPLER: + { + static_cast(descriptorInfos[i].desc.get())->convertToDummyObject(referenceLevelsBelowToConvert); + // should add asserts here? Not sure + if (descriptorInfos[i].info.image.sampler) + descriptorInfos[i].info.image.sampler->convertToDummyObject(referenceLevelsBelowToConvert); + } break; + case IDescriptor::E_CATEGORY::EC_IMAGE: { static_cast(descriptorInfos[i].desc.get())->convertToDummyObject(referenceLevelsBelowToConvert); @@ -183,6 +195,14 @@ void ICPUDescriptorSet::restoreFromDummy_impl(IAsset* _other, uint32_t _levelsBe restoreFromDummy_impl_call(static_cast(descriptorInfos[i].desc.get()), static_cast(otherDescriptorInfos[i].desc.get()), _levelsBelow); break; + case IDescriptor::EC_SAMPLER: + { + restoreFromDummy_impl_call(static_cast(descriptorInfos[i].desc.get()), static_cast(otherDescriptorInfos[i].desc.get()), _levelsBelow); + // should add asserts here? Not sure + if (descriptorInfos[i].info.image.sampler && otherDescriptorInfos[i].info.image.sampler) + restoreFromDummy_impl_call(descriptorInfos[i].info.image.sampler.get(), otherDescriptorInfos[i].info.image.sampler.get(), _levelsBelow); + } break; + case IDescriptor::EC_IMAGE: { restoreFromDummy_impl_call(static_cast(descriptorInfos[i].desc.get()), static_cast(otherDescriptorInfos[i].desc.get()), _levelsBelow); @@ -230,6 +250,16 @@ bool ICPUDescriptorSet::isAnyDependencyDummy_impl(uint32_t _levelsBelow) const return true; break; + case IDescriptor::EC_SAMPLER: + { + if (static_cast(descriptorInfos[i].desc.get())->isAnyDependencyDummy(_levelsBelow)) + return true; + + // should add asserts here? Not sure + if (descriptorInfos[i].info.image.sampler && descriptorInfos[i].info.image.sampler->isAnyDependencyDummy(_levelsBelow)) + return true; + } break; + case IDescriptor::EC_IMAGE: { if (static_cast(descriptorInfos[i].desc.get())->isAnyDependencyDummy(_levelsBelow)) diff --git a/src/nbl/video/CVulkanLogicalDevice.cpp b/src/nbl/video/CVulkanLogicalDevice.cpp index bec603b92b..6ea4d321b0 100644 --- a/src/nbl/video/CVulkanLogicalDevice.cpp +++ b/src/nbl/video/CVulkanLogicalDevice.cpp @@ -788,6 +788,7 @@ void CVulkanLogicalDevice::nullifyDescriptors_impl(const SDropDescriptorSetsPara case asset::IDescriptor::EC_BUFFER: outWrite->pBufferInfo = reinterpret_cast(nullDescriptors.data()); break; + case asset::IDescriptor::EC_SAMPLER: case asset::IDescriptor::EC_IMAGE: outWrite->pImageInfo = reinterpret_cast(nullDescriptors.data()); break; diff --git a/src/nbl/video/IDescriptorPool.cpp b/src/nbl/video/IDescriptorPool.cpp index 335aef69d0..6c79132140 100644 --- a/src/nbl/video/IDescriptorPool.cpp +++ b/src/nbl/video/IDescriptorPool.cpp @@ -10,12 +10,14 @@ IDescriptorPool::IDescriptorPool(core::smart_refctd_ptr&& for (auto i = 0; i < static_cast(asset::IDescriptor::E_TYPE::ET_COUNT); ++i) m_descriptorAllocators[i] = std::make_unique(m_creationParameters.maxDescriptorCount[i], m_creationParameters.flags.hasFlags(ECF_FREE_DESCRIPTOR_SET_BIT)); - // For mutable samplers. We don't know if there will be mutable samplers in sets allocated by this pool when we create the pool. + // For mutable samplers pertaining to combined image samplers. We don't know if there will be mutable samplers in sets allocated by this pool when we create the pool. m_descriptorAllocators[static_cast(asset::IDescriptor::E_TYPE::ET_COUNT)] = std::make_unique(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)], m_creationParameters.flags.hasFlags(ECF_FREE_DESCRIPTOR_SET_BIT)); // Initialize the storages. m_textureStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_SAMPLED_IMAGE)]); - m_mutableSamplerStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)]); + m_mutableCombinedSamplerStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)]); + // Keep standalone samplers separate from the ones in combined + m_MutableStandaloneSamplerStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_SAMPLER)]); m_storageImageStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_STORAGE_IMAGE)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_INPUT_ATTACHMENT)]); m_UBO_SSBOStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_UNIFORM_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_STORAGE_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_UNIFORM_BUFFER_DYNAMIC)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_STORAGE_BUFFER_DYNAMIC)]); m_UTB_STBStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_UNIFORM_TEXEL_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_STORAGE_TEXEL_BUFFER)]); @@ -112,7 +114,7 @@ bool IDescriptorPool::allocateStorageOffsets(SStorageOffsets& offsets, const IGP uint32_t maxCount = 0u; if (i == static_cast(asset::IDescriptor::E_TYPE::ET_COUNT)) { - count = layout->getTotalMutableSamplerCount(); + count = layout->getTotalMutableCombinedSamplerCount(); maxCount = m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)]; } else @@ -202,13 +204,13 @@ void IDescriptorPool::deleteSetStorage(const uint32_t setIndex) m_descriptorAllocators[i]->free(allocatedOffset, count); } - const auto mutableSamplerCount = set->getLayout()->getTotalMutableSamplerCount(); + const auto mutableSamplerCount = set->getLayout()->getTotalMutableCombinedSamplerCount(); if (mutableSamplerCount > 0) { - const uint32_t allocatedOffset = set->m_storageOffsets.getMutableSamplerOffset(); + const uint32_t allocatedOffset = set->m_storageOffsets.getMutableCombinedSamplerOffset(); assert(allocatedOffset != ~0u); - std::destroy_n(getMutableSamplerStorage() + allocatedOffset, mutableSamplerCount); + std::destroy_n(getMutableCombinedSamplerStorage() + allocatedOffset, mutableSamplerCount); if (allowsFreeing()) m_descriptorAllocators[static_cast(asset::IDescriptor::E_TYPE::ET_COUNT)]->free(allocatedOffset, mutableSamplerCount); diff --git a/src/nbl/video/IGPUDescriptorSet.cpp b/src/nbl/video/IGPUDescriptorSet.cpp index f735decf47..190816f350 100644 --- a/src/nbl/video/IGPUDescriptorSet.cpp +++ b/src/nbl/video/IGPUDescriptorSet.cpp @@ -23,10 +23,10 @@ IGPUDescriptorSet::IGPUDescriptorSet(core::smart_refctd_ptrgetTotalDescriptorCount(type)); } - const auto mutableSamplerCount = m_layout->getTotalMutableSamplerCount(); + const auto mutableSamplerCount = m_layout->getTotalMutableCombinedSamplerCount(); if (mutableSamplerCount > 0) { - auto mutableSamplers = getAllMutableSamplers(); + auto mutableSamplers = getAllMutableCombinedSamplers(); assert(mutableSamplers); std::uninitialized_default_construct_n(mutableSamplers, mutableSamplerCount); } @@ -101,15 +101,18 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor } } - mutableSamplers = getMutableSamplers(write.binding); - if (!mutableSamplers) - { - if (debugName) - m_pool->m_logger.log("Descriptor set (%s, %p) doesn't allow mutable samplers at binding %u.", system::ILogger::ELL_ERROR, debugName, this, write.binding); - else - m_pool->m_logger.log("Descriptor set (%p) doesn't allow mutable samplers at binding %u.", system::ILogger::ELL_ERROR, this, write.binding); + if (descriptorType == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) { + mutableSamplers = getMutableCombinedSamplers(write.binding); + // Should never reach here, the GetTypeCategory check earlier should ensure that + if (!mutableSamplers) + { + if (debugName) + m_pool->m_logger.log("Descriptor set (%s, %p) only allows standalone mutable samplers at binding %u (no combined image samplers).", system::ILogger::ELL_ERROR, debugName, this, write.binding); + else + m_pool->m_logger.log("Descriptor set (%p) only allows standalone mutable samplers at binding %u (no combined image samplers).", system::ILogger::ELL_ERROR, this, write.binding); - return asset::IDescriptor::E_TYPE::ET_COUNT; + return asset::IDescriptor::E_TYPE::ET_COUNT; + } } } @@ -124,13 +127,13 @@ void IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSe assert(descriptors); core::smart_refctd_ptr* mutableSamplers = nullptr; - if ((type == asset::IDescriptor::E_TYPE::ET_SAMPLER or type == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) + if (type == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER and write.info->info.image.sampler - // Since the write was validated, immutable samplers will only show up here if we're trying to write to a combined image sampler, in which case - // the samplers will be ignored by the Vulkan call. We avoid lifetime tracking them in this case + // In the case of COMBINED_IMAGE_SAMPLER, one can provide samplers on a write, but if the descriptor we're trying to write to has those immutable, + // samplers will just be ignored by Vulkan. Therefore we only track the samplers' lifetimes if the binding is has mutable samplers and m_layout->getImmutableSamplerRedirect().getCount(IGPUDescriptorSetLayout::CBindingRedirect::binding_number_t{ write.binding }) == 0) { - mutableSamplers = getMutableSamplers(write.binding); + mutableSamplers = getMutableCombinedSamplers(write.binding); assert(mutableSamplers); } @@ -154,7 +157,7 @@ void IGPUDescriptorSet::dropDescriptors(const IGPUDescriptorSet::SDropDescriptor const auto descriptorType = getBindingType(drop.binding); auto* dstDescriptors = drop.dstSet->getDescriptors(descriptorType, drop.binding); - auto* dstSamplers = drop.dstSet->getMutableSamplers(drop.binding); + auto* dstSamplers = drop.dstSet->getMutableCombinedSamplers(drop.binding); if (dstDescriptors) for (uint32_t i = 0; i < drop.count; i++) @@ -189,8 +192,8 @@ bool IGPUDescriptorSet::validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet auto* srcDescriptors = copy.srcSet->getDescriptors(type, copy.srcBinding); auto* dstDescriptors = copy.dstSet->getDescriptors(type, copy.dstBinding); - auto* srcSamplers = copy.srcSet->getMutableSamplers(copy.srcBinding); - auto* dstSamplers = copy.dstSet->getMutableSamplers(copy.dstBinding); + auto* srcSamplers = copy.srcSet->getMutableCombinedSamplers(copy.srcBinding); + auto* dstSamplers = copy.dstSet->getMutableCombinedSamplers(copy.dstBinding); if ((!srcDescriptors != !dstDescriptors) || (!srcSamplers != !dstSamplers)) { @@ -220,8 +223,8 @@ void IGPUDescriptorSet::processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& auto* dstDescriptors = copy.dstSet->getDescriptors(type, copy.dstBinding); assert(!(!srcDescriptors != !dstDescriptors)); - auto* srcSamplers = copy.srcSet->getMutableSamplers(copy.srcBinding); - auto* dstSamplers = copy.dstSet->getMutableSamplers(copy.dstBinding); + auto* srcSamplers = copy.srcSet->getMutableCombinedSamplers(copy.srcBinding); + auto* dstSamplers = copy.dstSet->getMutableCombinedSamplers(copy.dstBinding); assert(!(!srcSamplers != !dstSamplers)); if (srcDescriptors && dstDescriptors) diff --git a/src/nbl/video/ILogicalDevice.cpp b/src/nbl/video/ILogicalDevice.cpp index 1a59d40028..a86d3f40ea 100644 --- a/src/nbl/video/ILogicalDevice.cpp +++ b/src/nbl/video/ILogicalDevice.cpp @@ -392,12 +392,12 @@ core::smart_refctd_ptr ILogicalDevice::createDescriptor dynamicSSBOCount++; else if (binding.type==asset::IDescriptor::E_TYPE::ET_UNIFORM_BUFFER_DYNAMIC) dynamicUBOCount++; - // If binding comes with samplers, we're binding an immutable sampler + // If binding comes with samplers, we're specifying that this binding corresponds to immutable samplers else if ((binding.type == asset::IDescriptor::E_TYPE::ET_SAMPLER or binding.type==asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) and binding.samplers) { auto* samplers = binding.samplers; for (uint32_t i=0u; iwasCreatedBy(this)) + if ((not samplers[i]) or (not samplers[i]->wasCreatedBy(this))) return nullptr; maxSamplersCount += binding.count; } @@ -443,12 +443,10 @@ bool ILogicalDevice::updateDescriptorSets(const std::spanvalidateWrite(write))) { - case asset::IDescriptor::EC_SAMPLER: - params.samplerCount += writeCount; - break; case asset::IDescriptor::EC_BUFFER: params.bufferCount += writeCount; break; + case asset::IDescriptor::EC_SAMPLER: case asset::IDescriptor::EC_IMAGE: params.imageCount += writeCount; break; @@ -507,6 +505,7 @@ bool ILogicalDevice::nullifyDescriptors(const std::span Date: Fri, 10 May 2024 18:26:34 -0300 Subject: [PATCH 05/18] Separable image / samplers work! --- examples_tests | 2 +- include/nbl/asset/ICPUDescriptorSet.h | 8 ++++---- src/nbl/asset/ICPUDescriptorSet.cpp | 12 ++++++------ 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/examples_tests b/examples_tests index f91ccdc92f..efabef620f 160000 --- a/examples_tests +++ b/examples_tests @@ -1 +1 @@ -Subproject commit f91ccdc92ffe5312280c0e8e1db7987f21f8c676 +Subproject commit efabef620ffb29ed292e2498739ddfacd3456d5b diff --git a/include/nbl/asset/ICPUDescriptorSet.h b/include/nbl/asset/ICPUDescriptorSet.h index d0fbe1bf19..b0d50bc3a2 100644 --- a/include/nbl/asset/ICPUDescriptorSet.h +++ b/include/nbl/asset/ICPUDescriptorSet.h @@ -67,7 +67,7 @@ class NBL_API2 ICPUDescriptorSet final : public IDescriptorSet getDescriptorInfoStorage(const IDescriptor::E_TYPE type) const + inline std::span getDescriptorInfoStorage(const IDescriptor::E_TYPE type) const { // TODO: @Hazardu // Cannot do the mutability check here because it requires the function to be non-const, but the function cannot be non-const because it's called @@ -78,14 +78,14 @@ class NBL_API2 ICPUDescriptorSet final : public IDescriptorSet(type)]) - return { nullptr, nullptr }; + return { }; else return { m_descriptorInfos[static_cast(type)]->begin(), m_descriptorInfos[static_cast(type)]->end() }; } - core::SRange getDescriptorInfos(const ICPUDescriptorSetLayout::CBindingRedirect::binding_number_t binding, IDescriptor::E_TYPE type = IDescriptor::E_TYPE::ET_COUNT); + std::span getDescriptorInfos(const ICPUDescriptorSetLayout::CBindingRedirect::binding_number_t binding, IDescriptor::E_TYPE type = IDescriptor::E_TYPE::ET_COUNT); - core::SRange getDescriptorInfos(const ICPUDescriptorSetLayout::CBindingRedirect::binding_number_t binding, IDescriptor::E_TYPE type = IDescriptor::E_TYPE::ET_COUNT) const; + std::span getDescriptorInfos(const ICPUDescriptorSetLayout::CBindingRedirect::binding_number_t binding, IDescriptor::E_TYPE type = IDescriptor::E_TYPE::ET_COUNT) const; core::smart_refctd_ptr clone(uint32_t _depth = ~0u) const override; diff --git a/src/nbl/asset/ICPUDescriptorSet.cpp b/src/nbl/asset/ICPUDescriptorSet.cpp index 0f6442b24e..d5c16b7098 100644 --- a/src/nbl/asset/ICPUDescriptorSet.cpp +++ b/src/nbl/asset/ICPUDescriptorSet.cpp @@ -3,14 +3,14 @@ namespace nbl::asset { -core::SRange ICPUDescriptorSet::getDescriptorInfos(const ICPUDescriptorSetLayout::CBindingRedirect::binding_number_t binding, IDescriptor::E_TYPE type) +std::span ICPUDescriptorSet::getDescriptorInfos(const ICPUDescriptorSetLayout::CBindingRedirect::binding_number_t binding, IDescriptor::E_TYPE type) { assert(!isImmutable_debug()); auto immutableResult = const_cast(this)->getDescriptorInfos(binding, type); - return {const_cast(immutableResult.begin()), const_cast(immutableResult.end())}; + return {const_cast(immutableResult.data()), immutableResult.size()}; } -core::SRange ICPUDescriptorSet::getDescriptorInfos(const ICPUDescriptorSetLayout::CBindingRedirect::binding_number_t binding, IDescriptor::E_TYPE type) const +std::span ICPUDescriptorSet::getDescriptorInfos(const ICPUDescriptorSetLayout::CBindingRedirect::binding_number_t binding, IDescriptor::E_TYPE type) const { if (type == IDescriptor::E_TYPE::ET_COUNT) { @@ -26,20 +26,20 @@ core::SRange ICPUDescriptorSet::getDes } if (type == IDescriptor::E_TYPE::ET_COUNT) - return { nullptr, nullptr }; + return { }; } const auto& redirect = getLayout()->getDescriptorRedirect(type); const auto bindingNumberIndex = redirect.findBindingStorageIndex(binding); if (bindingNumberIndex.data == redirect.Invalid) - return { nullptr, nullptr }; + return { }; const auto offset = redirect.getStorageOffset(asset::ICPUDescriptorSetLayout::CBindingRedirect::storage_range_index_t{ bindingNumberIndex }).data; const auto count = redirect.getCount(asset::ICPUDescriptorSetLayout::CBindingRedirect::storage_range_index_t{ bindingNumberIndex }); auto infosBegin = m_descriptorInfos[static_cast(type)]->begin() + offset; - return { infosBegin, infosBegin + count }; + return { infosBegin, count }; } core::smart_refctd_ptr ICPUDescriptorSet::clone(uint32_t _depth) const From d881798c7dfe44b683d9ba206ab2be4714339bdb Mon Sep 17 00:00:00 2001 From: Fletterio Date: Mon, 13 May 2024 16:49:25 -0300 Subject: [PATCH 06/18] IGPUDescriptorSetLayout constructor to protected, ILogicalDevice's creation for the former has some new validation --- include/nbl/video/IGPUDescriptorSetLayout.h | 7 ++++--- src/nbl/video/ILogicalDevice.cpp | 8 ++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/include/nbl/video/IGPUDescriptorSetLayout.h b/include/nbl/video/IGPUDescriptorSetLayout.h index c688e4181d..5f60a9c009 100644 --- a/include/nbl/video/IGPUDescriptorSetLayout.h +++ b/include/nbl/video/IGPUDescriptorSetLayout.h @@ -24,6 +24,10 @@ class IGPUDescriptorSetLayout : public asset::IDescriptorSetLayout, using base_t = asset::IDescriptorSetLayout; public: + + inline bool versionChangeInvalidatesCommandBuffer() const { return m_versionChangeInvalidatesCommandBuffer; } + + protected: inline IGPUDescriptorSetLayout(core::smart_refctd_ptr&& dev, const std::span _bindings) : base_t(_bindings), IBackendObject(std::move(dev)) { for (const auto& binding : _bindings) @@ -36,9 +40,6 @@ class IGPUDescriptorSetLayout : public asset::IDescriptorSetLayout, } } - inline bool versionChangeInvalidatesCommandBuffer() const { return m_versionChangeInvalidatesCommandBuffer; } - - protected: virtual ~IGPUDescriptorSetLayout() = default; bool m_isPushDescLayout = false; diff --git a/src/nbl/video/ILogicalDevice.cpp b/src/nbl/video/ILogicalDevice.cpp index a86d3f40ea..3c63699b3e 100644 --- a/src/nbl/video/ILogicalDevice.cpp +++ b/src/nbl/video/ILogicalDevice.cpp @@ -380,6 +380,7 @@ core::smart_refctd_ptr ILogicalDevice::createDescriptor // TODO: MORE VALIDATION, but after descriptor indexing. bool variableLengthArrayDescriptorFound = false; + bool updateableAfterBindBindingFound = false; uint32_t variableLengthArrayDescriptorBindingNr = 0; uint32_t highestBindingNr = 0u; uint32_t maxSamplersCount = 0u; @@ -402,6 +403,9 @@ core::smart_refctd_ptr ILogicalDevice::createDescriptor maxSamplersCount += binding.count; } + if (bindings[i].createFlags & IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_UPDATE_AFTER_BIND_BIT) + updateableAfterBindBindingFound = true; + // validate if only last binding is run-time sized and there is only one run-time sized binding bool isCurrentDescriptorVariableLengthArray = static_cast(bindings[i].createFlags & IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_VARIABLE_DESCRIPTOR_COUNT_BIT); // no 2 run-time sized descriptors allowed @@ -416,6 +420,10 @@ core::smart_refctd_ptr ILogicalDevice::createDescriptor highestBindingNr = std::max(highestBindingNr, bindings[i].binding); } + // https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkDescriptorSetLayoutCreateInfo-descriptorType-03001 + if (updateableAfterBindBindingFound and dynamicSSBOCount + dynamicUBOCount != 0) + return nullptr; + // only last binding can be run-time sized if (variableLengthArrayDescriptorFound && variableLengthArrayDescriptorBindingNr != highestBindingNr) return nullptr; From 3df38a45978510f9823c97adfbc3bea4c90e8aee Mon Sep 17 00:00:00 2001 From: Fletterio Date: Wed, 15 May 2024 00:31:59 -0300 Subject: [PATCH 07/18] Minor IDescriptorSet::SDescriptorInfo QoL update --- include/nbl/asset/IDescriptorSet.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/include/nbl/asset/IDescriptorSet.h b/include/nbl/asset/IDescriptorSet.h index 2271fb0b9b..fa44696f8c 100644 --- a/include/nbl/asset/IDescriptorSet.h +++ b/include/nbl/asset/IDescriptorSet.h @@ -47,9 +47,10 @@ class IDescriptorSet : public virtual core::IReferenceCounted // TODO: try to re }; struct SImageInfo { - // This will be ignored if the DS layout already has an immutable sampler specified for the binding. - core::smart_refctd_ptr sampler; - IImage::LAYOUT imageLayout; + IImage::LAYOUT imageLayout; + // If the binding is COMBINED, this will be ignored if the DS layout already has an immutable sampler specified for the binding + // If the binding is an immutable SAMPLER, whatever's here doesn't matter because the write validation will fail + core::smart_refctd_ptr sampler; }; core::smart_refctd_ptr desc; From e3e5b461671bb6e6456b88c2958fd119aa942403 Mon Sep 17 00:00:00 2001 From: Fletterio Date: Sun, 23 Jun 2024 18:43:07 -0300 Subject: [PATCH 08/18] Update submodule for master merge --- examples_tests | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples_tests b/examples_tests index efabef620f..d478a92baf 160000 --- a/examples_tests +++ b/examples_tests @@ -1 +1 @@ -Subproject commit efabef620ffb29ed292e2498739ddfacd3456d5b +Subproject commit d478a92baf92572434454bf624ad0821b79705f6 From 0b3efea5378d7c324aa1249929a16c195ba2339e Mon Sep 17 00:00:00 2001 From: Fletterio Date: Mon, 24 Jun 2024 15:59:51 -0300 Subject: [PATCH 09/18] Some changes following PR review --- examples_tests | 2 +- include/nbl/asset/IDescriptorSet.h | 4 +++- include/nbl/asset/IDescriptorSetLayout.h | 2 +- include/nbl/video/IDescriptorPool.h | 4 ++-- src/nbl/video/IGPUDescriptorSet.cpp | 14 ++++++++------ 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/examples_tests b/examples_tests index d478a92baf..6087164acf 160000 --- a/examples_tests +++ b/examples_tests @@ -1 +1 @@ -Subproject commit d478a92baf92572434454bf624ad0821b79705f6 +Subproject commit 6087164acf7de85c253cddc49aee9fcb999be2e3 diff --git a/include/nbl/asset/IDescriptorSet.h b/include/nbl/asset/IDescriptorSet.h index fa44696f8c..3338c1511b 100644 --- a/include/nbl/asset/IDescriptorSet.h +++ b/include/nbl/asset/IDescriptorSet.h @@ -47,9 +47,11 @@ class IDescriptorSet : public virtual core::IReferenceCounted // TODO: try to re }; struct SImageInfo { + // If the binding is a SAMPLER, this field is ignored IImage::LAYOUT imageLayout; // If the binding is COMBINED, this will be ignored if the DS layout already has an immutable sampler specified for the binding // If the binding is an immutable SAMPLER, whatever's here doesn't matter because the write validation will fail + // If the binding is a SAMPLER, this field is ignored: the only sampler being used will be desc core::smart_refctd_ptr sampler; }; @@ -127,7 +129,7 @@ class IDescriptorSet : public virtual core::IReferenceCounted // TODO: try to re inline bool operator!=(const SDescriptorInfo& other) const { - if (desc != desc) + if (desc != other.desc) return true; return info.buffer != other.info.buffer; } diff --git a/include/nbl/asset/IDescriptorSetLayout.h b/include/nbl/asset/IDescriptorSetLayout.h index bc16864658..683cc74bee 100644 --- a/include/nbl/asset/IDescriptorSetLayout.h +++ b/include/nbl/asset/IDescriptorSetLayout.h @@ -412,7 +412,7 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr if (b.samplers) buildInfo_immutableSamplers.emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); else - buildInfo_descriptors[static_cast(b.type)].emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); + buildInfo_descriptors[IDescriptor::E_TYPE::ET_SAMPLER].emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); break; case IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER: if (b.samplers) diff --git a/include/nbl/video/IDescriptorPool.h b/include/nbl/video/IDescriptorPool.h index 309aa8608e..e4844bfcd5 100644 --- a/include/nbl/video/IDescriptorPool.h +++ b/include/nbl/video/IDescriptorPool.h @@ -102,7 +102,7 @@ class NBL_API2 IDescriptorPool : public IBackendObject switch (type) { case asset::IDescriptor::E_TYPE::ET_SAMPLER: - baseAddress = reinterpret_cast*>(m_MutableStandaloneSamplerStorage.get()); + baseAddress = reinterpret_cast*>(m_mutableStandaloneSamplerStorage.get()); break; case asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER: case asset::IDescriptor::E_TYPE::ET_SAMPLED_IMAGE: @@ -223,7 +223,7 @@ class NBL_API2 IDescriptorPool : public IBackendObject std::unique_ptr>[]> m_textureStorage; std::unique_ptr>[]> m_mutableCombinedSamplerStorage; - std::unique_ptr>[]> m_MutableStandaloneSamplerStorage; + std::unique_ptr>[]> m_mutableStandaloneSamplerStorage; std::unique_ptr>[]> m_storageImageStorage; // storage image | input attachment std::unique_ptr>[]> m_UBO_SSBOStorage; // ubo | ssbo | ubo dynamic | ssbo dynamic std::unique_ptr>[]> m_UTB_STBStorage; // utb | stb diff --git a/src/nbl/video/IGPUDescriptorSet.cpp b/src/nbl/video/IGPUDescriptorSet.cpp index 190816f350..0a593a2a7a 100644 --- a/src/nbl/video/IGPUDescriptorSet.cpp +++ b/src/nbl/video/IGPUDescriptorSet.cpp @@ -157,16 +157,18 @@ void IGPUDescriptorSet::dropDescriptors(const IGPUDescriptorSet::SDropDescriptor const auto descriptorType = getBindingType(drop.binding); auto* dstDescriptors = drop.dstSet->getDescriptors(descriptorType, drop.binding); - auto* dstSamplers = drop.dstSet->getMutableCombinedSamplers(drop.binding); - if (dstDescriptors) for (uint32_t i = 0; i < drop.count; i++) dstDescriptors[drop.arrayElement + i] = nullptr; - if (dstSamplers) - for (uint32_t i = 0; i < drop.count; i++) - dstSamplers[drop.arrayElement + i] = nullptr; - + if (asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER == descriptorType) + { + auto* dstSamplers = drop.dstSet->getMutableCombinedSamplers(drop.binding); + if (dstSamplers) + for (uint32_t i = 0; i < drop.count; i++) + dstSamplers[drop.arrayElement + i] = nullptr; + } + // we only increment the version to detect UPDATE-AFTER-BIND and automagically invalidate descriptor sets // so, only if we do the path that writes descriptors, do we want to increment version auto& bindingRedirect = m_layout->getDescriptorRedirect(descriptorType); From 8aef486c514ea26f35361afcf74fa153c1320da0 Mon Sep 17 00:00:00 2001 From: Fletterio Date: Fri, 28 Jun 2024 17:52:41 -0300 Subject: [PATCH 10/18] PR review done (excepts perhaps for a suggested name change) --- 3rdparty/dxc/dxc | 2 +- include/nbl/asset/IDescriptorSet.h | 8 +- include/nbl/asset/IDescriptorSetLayout.h | 2 +- include/nbl/video/IGPUDescriptorSet.h | 42 ++++++++++ include/nbl/video/IGPUDescriptorSetLayout.h | 4 + src/nbl/video/IDescriptorPool.cpp | 2 +- src/nbl/video/IGPUDescriptorSet.cpp | 89 ++++++++++----------- 7 files changed, 96 insertions(+), 53 deletions(-) diff --git a/3rdparty/dxc/dxc b/3rdparty/dxc/dxc index a08b6cbeb1..2a0f9968de 160000 --- a/3rdparty/dxc/dxc +++ b/3rdparty/dxc/dxc @@ -1 +1 @@ -Subproject commit a08b6cbeb1038d14d0586d10a8cfa507b2fda8eb +Subproject commit 2a0f9968de8e554b6dc104e4bc0f7f7f7122f0cd diff --git a/include/nbl/asset/IDescriptorSet.h b/include/nbl/asset/IDescriptorSet.h index 3338c1511b..dbe40af1e4 100644 --- a/include/nbl/asset/IDescriptorSet.h +++ b/include/nbl/asset/IDescriptorSet.h @@ -45,15 +45,13 @@ class IDescriptorSet : public virtual core::IReferenceCounted // TODO: try to re auto operator<=>(const SBufferInfo&) const = default; }; - struct SImageInfo - { + struct SImageInfo + { // If the binding is a SAMPLER, this field is ignored IImage::LAYOUT imageLayout; // If the binding is COMBINED, this will be ignored if the DS layout already has an immutable sampler specified for the binding - // If the binding is an immutable SAMPLER, whatever's here doesn't matter because the write validation will fail - // If the binding is a SAMPLER, this field is ignored: the only sampler being used will be desc core::smart_refctd_ptr sampler; - }; + }; core::smart_refctd_ptr desc; union SBufferImageInfo diff --git a/include/nbl/asset/IDescriptorSetLayout.h b/include/nbl/asset/IDescriptorSetLayout.h index 683cc74bee..2d425c84ac 100644 --- a/include/nbl/asset/IDescriptorSetLayout.h +++ b/include/nbl/asset/IDescriptorSetLayout.h @@ -412,7 +412,7 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr if (b.samplers) buildInfo_immutableSamplers.emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); else - buildInfo_descriptors[IDescriptor::E_TYPE::ET_SAMPLER].emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); + buildInfo_descriptors[size_t(IDescriptor::E_TYPE::ET_SAMPLER)].emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); break; case IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER: if (b.samplers) diff --git a/include/nbl/video/IGPUDescriptorSet.h b/include/nbl/video/IGPUDescriptorSet.h index 5ee17a3433..37a4693ce3 100644 --- a/include/nbl/video/IGPUDescriptorSet.h +++ b/include/nbl/video/IGPUDescriptorSet.h @@ -70,6 +70,20 @@ class IGPUDescriptorSet : public asset::IDescriptorSet(asset::IDescriptor::E_TYPE::ET_COUNT); t++) + { + const auto type = static_cast(t); + const auto& bindingRedirect = getLayout()->getDescriptorRedirect(type); + bindingStorageIndex = bindingRedirect.findBindingStorageIndex(binding).data; + if (bindingStorageIndex != redirect_t::Invalid) + return type; + } + return asset::IDescriptor::E_TYPE::ET_COUNT; + } + protected: IGPUDescriptorSet(core::smart_refctd_ptr&& _layout, core::smart_refctd_ptr&& pool, IDescriptorPool::SStorageOffsets&& offsets); virtual ~IGPUDescriptorSet(); @@ -99,6 +113,20 @@ class IGPUDescriptorSet : public asset::IDescriptorSet* getDescriptorsIndexed(const asset::IDescriptor::E_TYPE type, const uint32_t bindingStorageIndex) const + { + const auto localOffset = getLayout()->getDescriptorRedirect(type).getStorageOffset(redirect_t::storage_range_index_t{ bindingStorageIndex }).data; + if (localOffset == ~0) + return nullptr; + + auto* descriptors = getAllDescriptors(type); + if (!descriptors) + return nullptr; + + return descriptors + localOffset; + } + inline core::smart_refctd_ptr* getMutableCombinedSamplers(const uint32_t binding) const { const auto localOffset = getLayout()->getMutableCombinedSamplerRedirect().getStorageOffset(redirect_t::binding_number_t{ binding }).data; @@ -112,6 +140,20 @@ class IGPUDescriptorSet : public asset::IDescriptorSet* getMutableCombinedSamplersIndexed(const uint32_t bindingStorageIndex) const + { + const auto localOffset = getLayout()->getMutableCombinedSamplerRedirect().getStorageOffset(redirect_t::storage_range_index_t{ bindingStorageIndex }).data; + if (localOffset == getLayout()->getMutableCombinedSamplerRedirect().Invalid) + return nullptr; + + auto* samplers = getAllMutableCombinedSamplers(); + if (!samplers) + return nullptr; + + return samplers + localOffset; + } + inline core::smart_refctd_ptr* getAllDescriptors(const asset::IDescriptor::E_TYPE type) const { auto* baseAddress = m_pool->getDescriptorStorage(type); diff --git a/include/nbl/video/IGPUDescriptorSetLayout.h b/include/nbl/video/IGPUDescriptorSetLayout.h index 5f60a9c009..afb1a3216c 100644 --- a/include/nbl/video/IGPUDescriptorSetLayout.h +++ b/include/nbl/video/IGPUDescriptorSetLayout.h @@ -27,6 +27,10 @@ class IGPUDescriptorSetLayout : public asset::IDescriptorSetLayout, inline bool versionChangeInvalidatesCommandBuffer() const { return m_versionChangeInvalidatesCommandBuffer; } + static inline bool writeIncrementsVersion(const core::bitflag bindingCreateFlags) { + return not (bindingCreateFlags.hasFlags(SBinding::E_CREATE_FLAGS::ECF_UPDATE_AFTER_BIND_BIT) or bindingCreateFlags.hasFlags(SBinding::E_CREATE_FLAGS::ECF_UPDATE_UNUSED_WHILE_PENDING_BIT)); + } + protected: inline IGPUDescriptorSetLayout(core::smart_refctd_ptr&& dev, const std::span _bindings) : base_t(_bindings), IBackendObject(std::move(dev)) { diff --git a/src/nbl/video/IDescriptorPool.cpp b/src/nbl/video/IDescriptorPool.cpp index 6c79132140..1d47900efb 100644 --- a/src/nbl/video/IDescriptorPool.cpp +++ b/src/nbl/video/IDescriptorPool.cpp @@ -17,7 +17,7 @@ IDescriptorPool::IDescriptorPool(core::smart_refctd_ptr&& m_textureStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_SAMPLED_IMAGE)]); m_mutableCombinedSamplerStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)]); // Keep standalone samplers separate from the ones in combined - m_MutableStandaloneSamplerStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_SAMPLER)]); + m_mutableStandaloneSamplerStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_SAMPLER)]); m_storageImageStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_STORAGE_IMAGE)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_INPUT_ATTACHMENT)]); m_UBO_SSBOStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_UNIFORM_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_STORAGE_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_UNIFORM_BUFFER_DYNAMIC)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_STORAGE_BUFFER_DYNAMIC)]); m_UTB_STBStorage = std::make_unique>[]>(m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_UNIFORM_TEXEL_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast(asset::IDescriptor::E_TYPE::ET_STORAGE_TEXEL_BUFFER)]); diff --git a/src/nbl/video/IGPUDescriptorSet.cpp b/src/nbl/video/IGPUDescriptorSet.cpp index 0a593a2a7a..9af9b6986d 100644 --- a/src/nbl/video/IGPUDescriptorSet.cpp +++ b/src/nbl/video/IGPUDescriptorSet.cpp @@ -45,9 +45,10 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor const char* debugName = getDebugName(); // screw it, we'll need to replace the descriptor writing with update templates of descriptor buffer soon anyway - const auto descriptorType = getBindingType(write.binding); + uint32_t descriptorRedirectBindingIndex; + const auto descriptorType = getBindingType(write.binding, descriptorRedirectBindingIndex); - auto* descriptors = getDescriptors(descriptorType,write.binding); + auto* descriptors = getDescriptorsIndexed(descriptorType, descriptorRedirectBindingIndex); if (!descriptors) { if (debugName) @@ -59,11 +60,9 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor } core::smart_refctd_ptr* mutableSamplers = nullptr; - if ((descriptorType == asset::IDescriptor::E_TYPE::ET_SAMPLER or descriptorType == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) and write.info->info.image.sampler) + if (descriptorType == asset::IDescriptor::E_TYPE::ET_SAMPLER or (descriptorType == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER and write.info->info.image.sampler)) { - if (m_layout->getImmutableSamplerRedirect().getCount(IGPUDescriptorSetLayout::CBindingRedirect::binding_number_t{ write.binding }) != 0 - // providing samplers to an immutable sampler binding is only a problem if the descriptor is not a combined image sampler - and descriptorType == asset::IDescriptor::E_TYPE::ET_SAMPLER) + if (m_layout->getImmutableSamplerRedirect().getCount(IGPUDescriptorSetLayout::CBindingRedirect::binding_number_t{ write.binding }) != 0) { if (debugName) m_pool->m_logger.log("Trying to write samplers at binding %u of Descriptor set (%s, %p), but those are immutable.", system::ILogger::ELL_ERROR, debugName, this, write.binding); @@ -83,7 +82,7 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor return asset::IDescriptor::E_TYPE::ET_COUNT; } - auto* sampler = write.info[i].info.image.sampler.get(); + auto* sampler = descriptorType == asset::IDescriptor::E_TYPE::ET_SAMPLER ? reinterpret_cast(write.info[i].desc.get()) : write.info[i].info.image.sampler.get(); if (not sampler) { if (debugName) m_pool->m_logger.log("Null sampler provided when trying to write descriptor set (%s, %p). All writes should provide a sampler.", system::ILogger::ELL_ERROR, debugName, write.dstSet); @@ -123,21 +122,19 @@ void IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSe { assert(write.dstSet == this); - auto* descriptors = getDescriptors(type,write.binding); + auto descriptorRedirectBindingIndex = getLayout()->getDescriptorRedirect(type).findBindingStorageIndex(write.binding).data; + auto* descriptors = getDescriptorsIndexed(type, descriptorRedirectBindingIndex); assert(descriptors); core::smart_refctd_ptr* mutableSamplers = nullptr; if (type == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER - and write.info->info.image.sampler - // In the case of COMBINED_IMAGE_SAMPLER, one can provide samplers on a write, but if the descriptor we're trying to write to has those immutable, - // samplers will just be ignored by Vulkan. Therefore we only track the samplers' lifetimes if the binding is has mutable samplers - and m_layout->getImmutableSamplerRedirect().getCount(IGPUDescriptorSetLayout::CBindingRedirect::binding_number_t{ write.binding }) == 0) + and write.info->info.image.sampler) { mutableSamplers = getMutableCombinedSamplers(write.binding); assert(mutableSamplers); } - for (auto j=0; jgetDescriptorRedirect(type); - auto bindingCreateFlags = bindingRedirect.getCreateFlags(bindingRedirect.findBindingStorageIndex({ write.binding })); - if (not (bindingCreateFlags.hasFlags(IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_UPDATE_AFTER_BIND_BIT) or bindingCreateFlags.hasFlags(IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_UPDATE_UNUSED_WHILE_PENDING_BIT))) + auto bindingCreateFlags = bindingRedirect.getCreateFlags(redirect_t::storage_range_index_t{ descriptorRedirectBindingIndex }); + if (IGPUDescriptorSetLayout::writeIncrementsVersion(bindingCreateFlags)) incrementVersion(); } @@ -154,12 +151,13 @@ void IGPUDescriptorSet::dropDescriptors(const IGPUDescriptorSet::SDropDescriptor { assert(drop.dstSet == this); - const auto descriptorType = getBindingType(drop.binding); + uint32_t descriptorRedirectBindingIndex; + const auto descriptorType = getBindingType(drop.binding, descriptorRedirectBindingIndex); - auto* dstDescriptors = drop.dstSet->getDescriptors(descriptorType, drop.binding); - if (dstDescriptors) - for (uint32_t i = 0; i < drop.count; i++) - dstDescriptors[drop.arrayElement + i] = nullptr; + auto* dstDescriptors = drop.dstSet->getDescriptorsIndexed(descriptorType, descriptorRedirectBindingIndex); + if (dstDescriptors) + for (uint32_t i = 0; i < drop.count; i++) + dstDescriptors[drop.arrayElement + i] = nullptr; if (asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER == descriptorType) { @@ -168,16 +166,13 @@ void IGPUDescriptorSet::dropDescriptors(const IGPUDescriptorSet::SDropDescriptor for (uint32_t i = 0; i < drop.count; i++) dstSamplers[drop.arrayElement + i] = nullptr; } - - // we only increment the version to detect UPDATE-AFTER-BIND and automagically invalidate descriptor sets - // so, only if we do the path that writes descriptors, do we want to increment version + + // we only increment the version to detect UPDATE-AFTER-BIND and automagically invalidate descriptor sets + // so, only if we do the path that writes descriptors, do we want to increment version auto& bindingRedirect = m_layout->getDescriptorRedirect(descriptorType); - auto bindingCreateFlags = bindingRedirect.getCreateFlags(bindingRedirect.findBindingStorageIndex({ drop.binding })); - bool shouldIncrementVersion = not (bindingCreateFlags.hasFlags(IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_UPDATE_AFTER_BIND_BIT) or bindingCreateFlags.hasFlags(IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_UPDATE_UNUSED_WHILE_PENDING_BIT)); - if (getOriginDevice()->getEnabledFeatures().nullDescriptor or shouldIncrementVersion) - { + auto bindingCreateFlags = bindingRedirect.getCreateFlags(redirect_t::storage_range_index_t{ descriptorRedirectBindingIndex }); + if (IGPUDescriptorSetLayout::writeIncrementsVersion(bindingCreateFlags)) incrementVersion(); - } } bool IGPUDescriptorSet::validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy) const @@ -194,8 +189,8 @@ bool IGPUDescriptorSet::validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet auto* srcDescriptors = copy.srcSet->getDescriptors(type, copy.srcBinding); auto* dstDescriptors = copy.dstSet->getDescriptors(type, copy.dstBinding); - auto* srcSamplers = copy.srcSet->getMutableCombinedSamplers(copy.srcBinding); - auto* dstSamplers = copy.dstSet->getMutableCombinedSamplers(copy.dstBinding); + auto* srcSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.srcSet->getMutableCombinedSamplers(copy.srcBinding); + auto* dstSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.dstSet->getMutableCombinedSamplers(copy.dstBinding); if ((!srcDescriptors != !dstDescriptors) || (!srcSamplers != !dstSamplers)) { @@ -215,34 +210,38 @@ void IGPUDescriptorSet::processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& { assert(copy.dstSet == this); - bool shouldIncrementVersion = false; - for (uint32_t t = 0; t < static_cast(asset::IDescriptor::E_TYPE::ET_COUNT); ++t) { const auto type = static_cast(t); + auto& bindingRedirect = m_layout->getDescriptorRedirect(type); auto* srcDescriptors = copy.srcSet->getDescriptors(type, copy.srcBinding); - auto* dstDescriptors = copy.dstSet->getDescriptors(type, copy.dstBinding); - assert(!(!srcDescriptors != !dstDescriptors)); + // We can do this because dstSet === this as asserted at the start + auto descriptorRedirectBindingIndex = bindingRedirect.findBindingStorageIndex({ copy.dstBinding }).data; + auto* dstDescriptors = copy.dstSet->getDescriptorsIndexed(type, descriptorRedirectBindingIndex); + if (!srcDescriptors) + { + assert(!dstDescriptors); + continue; + } + assert(dstDescriptors); - auto* srcSamplers = copy.srcSet->getMutableCombinedSamplers(copy.srcBinding); - auto* dstSamplers = copy.dstSet->getMutableCombinedSamplers(copy.dstBinding); + auto* srcSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.srcSet->getMutableCombinedSamplers(copy.srcBinding); + auto* dstSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.dstSet->getMutableCombinedSamplers(copy.dstBinding); assert(!(!srcSamplers != !dstSamplers)); - if (srcDescriptors && dstDescriptors) - std::copy_n(srcDescriptors, copy.count, dstDescriptors); + + auto bindingCreateFlags = bindingRedirect.getCreateFlags(redirect_t::storage_range_index_t{ descriptorRedirectBindingIndex }); + if (IGPUDescriptorSetLayout::writeIncrementsVersion(bindingCreateFlags)) + incrementVersion(); + + std::copy_n(srcDescriptors, copy.count, dstDescriptors); if (srcSamplers && dstSamplers) std::copy_n(srcSamplers, copy.count, dstSamplers); - auto& bindingRedirect = m_layout->getDescriptorRedirect(type); - auto bindingCreateFlags = bindingRedirect.getCreateFlags(bindingRedirect.findBindingStorageIndex({ copy.srcBinding })); - if (not (bindingCreateFlags.hasFlags(IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_UPDATE_AFTER_BIND_BIT) or bindingCreateFlags.hasFlags(IGPUDescriptorSetLayout::SBinding::E_CREATE_FLAGS::ECF_UPDATE_UNUSED_WHILE_PENDING_BIT))) - shouldIncrementVersion = true; + break; } - - if (shouldIncrementVersion) - incrementVersion(); } } \ No newline at end of file From 68d1604258b2dbc36d8f92a92cdd721dcdd3d23d Mon Sep 17 00:00:00 2001 From: Fletterio Date: Sat, 29 Jun 2024 18:45:54 -0300 Subject: [PATCH 11/18] Amortizing the double lookup by saving indices between validation and processing passes --- include/nbl/video/IGPUDescriptorSet.h | 8 +-- src/nbl/video/IGPUDescriptorSet.cpp | 80 ++++++++++++++------------- src/nbl/video/ILogicalDevice.cpp | 15 +++-- 3 files changed, 56 insertions(+), 47 deletions(-) diff --git a/include/nbl/video/IGPUDescriptorSet.h b/include/nbl/video/IGPUDescriptorSet.h index 37a4693ce3..57cf86e072 100644 --- a/include/nbl/video/IGPUDescriptorSet.h +++ b/include/nbl/video/IGPUDescriptorSet.h @@ -92,10 +92,10 @@ class IGPUDescriptorSet : public asset::IDescriptorSetdeleteSetStorage(m_storageOffsets.getSetOffset()); } -asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write) const +asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write, uint32_t& descriptorRedirectBindingIndex, uint32_t& mutableSamplerRedirectBindingIndex) const { assert(write.dstSet == this); const char* debugName = getDebugName(); // screw it, we'll need to replace the descriptor writing with update templates of descriptor buffer soon anyway - uint32_t descriptorRedirectBindingIndex; const auto descriptorType = getBindingType(write.binding, descriptorRedirectBindingIndex); auto* descriptors = getDescriptorsIndexed(descriptorType, descriptorRedirectBindingIndex); @@ -101,7 +100,8 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor } if (descriptorType == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) { - mutableSamplers = getMutableCombinedSamplers(write.binding); + mutableSamplerRedirectBindingIndex = getLayout()->getMutableCombinedSamplerRedirect().findBindingStorageIndex(write.binding).data; + mutableSamplers = getMutableCombinedSamplersIndexed(mutableSamplerRedirectBindingIndex); // Should never reach here, the GetTypeCategory check earlier should ensure that if (!mutableSamplers) { @@ -118,11 +118,10 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor return descriptorType; } -void IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write, const asset::IDescriptor::E_TYPE type) +void IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write, const asset::IDescriptor::E_TYPE type, const uint32_t descriptorRedirectBindingIndex, const uint32_t mutableSamplerRedirectBindingIndex) { assert(write.dstSet == this); - auto descriptorRedirectBindingIndex = getLayout()->getDescriptorRedirect(type).findBindingStorageIndex(write.binding).data; auto* descriptors = getDescriptorsIndexed(type, descriptorRedirectBindingIndex); assert(descriptors); @@ -130,7 +129,7 @@ void IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSe if (type == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER and write.info->info.image.sampler) { - mutableSamplers = getMutableCombinedSamplers(write.binding); + mutableSamplers = getMutableCombinedSamplersIndexed(mutableSamplerRedirectBindingIndex); assert(mutableSamplers); } @@ -175,22 +174,40 @@ void IGPUDescriptorSet::dropDescriptors(const IGPUDescriptorSet::SDropDescriptor incrementVersion(); } -bool IGPUDescriptorSet::validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy) const +bool IGPUDescriptorSet::validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy, asset::IDescriptor::E_TYPE& type, uint32_t& srcDescriptorRedirectBindingIndex, uint32_t& dstDescriptorRedirectBindingIndex, uint32_t& srcMutableSamplerRedirectBindingIndex, uint32_t& dstMutableSamplerRedirectBindingIndex) const { assert(copy.dstSet == this); const char* srcDebugName = copy.srcSet->getDebugName(); const char* dstDebugName = copy.dstSet->getDebugName(); + const auto srcLayout = copy.srcSet->getLayout(); + const auto dstLayout = copy.dstSet->getLayout(); + for (uint32_t t = 0; t < static_cast(asset::IDescriptor::E_TYPE::ET_COUNT); ++t) { - const auto type = static_cast(t); + type = static_cast(t); + + srcDescriptorRedirectBindingIndex = srcLayout->getDescriptorRedirect(type).findBindingStorageIndex(copy.srcBinding).data; + dstDescriptorRedirectBindingIndex = dstLayout->getDescriptorRedirect(type).findBindingStorageIndex(copy.dstBinding).data; + auto* srcDescriptors = copy.srcSet->getDescriptorsIndexed(type, srcDescriptorRedirectBindingIndex); + auto* dstDescriptors = copy.dstSet->getDescriptorsIndexed(type, dstDescriptorRedirectBindingIndex); - auto* srcDescriptors = copy.srcSet->getDescriptors(type, copy.srcBinding); - auto* dstDescriptors = copy.dstSet->getDescriptors(type, copy.dstBinding); + if (!srcDescriptors) + { + assert(!dstDescriptors); + continue; + } + assert(dstDescriptors); - auto* srcSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.srcSet->getMutableCombinedSamplers(copy.srcBinding); - auto* dstSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.dstSet->getMutableCombinedSamplers(copy.dstBinding); + core::smart_refctd_ptr *srcSamplers = nullptr, *dstSamplers = nullptr; + if (asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER == type) + { + srcMutableSamplerRedirectBindingIndex = srcLayout->getMutableCombinedSamplerRedirect().findBindingStorageIndex(copy.srcBinding).data; + dstMutableSamplerRedirectBindingIndex = dstLayout->getMutableCombinedSamplerRedirect().findBindingStorageIndex(copy.dstBinding).data; + srcSamplers = copy.srcSet->getMutableCombinedSamplersIndexed(srcMutableSamplerRedirectBindingIndex); + dstSamplers = copy.dstSet->getMutableCombinedSamplersIndexed(dstMutableSamplerRedirectBindingIndex); + } if ((!srcDescriptors != !dstDescriptors) || (!srcSamplers != !dstSamplers)) { @@ -206,42 +223,27 @@ bool IGPUDescriptorSet::validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet return true; } -void IGPUDescriptorSet::processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy) +void IGPUDescriptorSet::processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy, const asset::IDescriptor::E_TYPE type, const uint32_t srcDescriptorRedirectBindingIndex, const uint32_t dstDescriptorRedirectBindingIndex, const uint32_t srcMutableSamplerRedirectBindingIndex, const uint32_t dstMutableSamplerRedirectBindingIndex) { assert(copy.dstSet == this); - for (uint32_t t = 0; t < static_cast(asset::IDescriptor::E_TYPE::ET_COUNT); ++t) - { - const auto type = static_cast(t); - auto& bindingRedirect = m_layout->getDescriptorRedirect(type); + auto& bindingRedirect = m_layout->getDescriptorRedirect(type); - auto* srcDescriptors = copy.srcSet->getDescriptors(type, copy.srcBinding); - // We can do this because dstSet === this as asserted at the start - auto descriptorRedirectBindingIndex = bindingRedirect.findBindingStorageIndex({ copy.dstBinding }).data; - auto* dstDescriptors = copy.dstSet->getDescriptorsIndexed(type, descriptorRedirectBindingIndex); - if (!srcDescriptors) - { - assert(!dstDescriptors); - continue; - } - assert(dstDescriptors); + auto* srcDescriptors = copy.srcSet->getDescriptorsIndexed(type, srcDescriptorRedirectBindingIndex); + auto* dstDescriptors = copy.dstSet->getDescriptorsIndexed(type, dstDescriptorRedirectBindingIndex); - auto* srcSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.srcSet->getMutableCombinedSamplers(copy.srcBinding); - auto* dstSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.dstSet->getMutableCombinedSamplers(copy.dstBinding); - assert(!(!srcSamplers != !dstSamplers)); + auto* srcSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.srcSet->getMutableCombinedSamplersIndexed(srcMutableSamplerRedirectBindingIndex); + auto* dstSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.dstSet->getMutableCombinedSamplersIndexed(dstMutableSamplerRedirectBindingIndex); - auto bindingCreateFlags = bindingRedirect.getCreateFlags(redirect_t::storage_range_index_t{ descriptorRedirectBindingIndex }); - if (IGPUDescriptorSetLayout::writeIncrementsVersion(bindingCreateFlags)) - incrementVersion(); - - std::copy_n(srcDescriptors, copy.count, dstDescriptors); + auto bindingCreateFlags = bindingRedirect.getCreateFlags(redirect_t::storage_range_index_t{ dstDescriptorRedirectBindingIndex }); + if (IGPUDescriptorSetLayout::writeIncrementsVersion(bindingCreateFlags)) + incrementVersion(); - if (srcSamplers && dstSamplers) - std::copy_n(srcSamplers, copy.count, dstSamplers); + std::copy_n(srcDescriptors, copy.count, dstDescriptors); - break; - } + if (srcSamplers && dstSamplers) + std::copy_n(srcSamplers, copy.count, dstSamplers); } } \ No newline at end of file diff --git a/src/nbl/video/ILogicalDevice.cpp b/src/nbl/video/ILogicalDevice.cpp index 3c63699b3e..9c49ccc5a9 100644 --- a/src/nbl/video/ILogicalDevice.cpp +++ b/src/nbl/video/ILogicalDevice.cpp @@ -442,6 +442,8 @@ bool ILogicalDevice::updateDescriptorSets(const std::span writeTypes(descriptorWrites.size()); auto outCategory = writeTypes.data(); params.pWriteTypes = outCategory; + core::vector writeDescriptorRedirectIndices(descriptorWrites.size()), writeMutableSamplerRedirectIndices(descriptorWrites.size()); + auto i = 0u; for (const auto& write : descriptorWrites) { auto* ds = write.dstSet; @@ -449,7 +451,7 @@ bool ILogicalDevice::updateDescriptorSets(const std::spanvalidateWrite(write))) + switch (asset::IDescriptor::GetTypeCategory(*outCategory=ds->validateWrite(write, writeDescriptorRedirectIndices[i], writeMutableSamplerRedirectIndices[i]))) { case asset::IDescriptor::EC_BUFFER: params.bufferCount += writeCount; @@ -469,8 +471,13 @@ bool ILogicalDevice::updateDescriptorSets(const std::span copySrcDescriptorRedirectIndices(descriptorCopies.size()), copySrcMutableSamplerRedirectIndices(descriptorCopies.size()); + core::vector copyDstDescriptorRedirectIndices(descriptorCopies.size()), copyDstMutableSamplerRedirectIndices(descriptorCopies.size()); + core::vector copyTypes(descriptorCopies.size()); + i = 0; for (const auto& copy : descriptorCopies) { const auto* srcDS = copy.srcSet; @@ -480,17 +487,17 @@ bool ILogicalDevice::updateDescriptorSets(const std::spanisCompatibleDevicewise(srcDS)) return false; - if (!dstDS->validateCopy(copy)) + if (!dstDS->validateCopy(copy, copyTypes[i], copySrcDescriptorRedirectIndices[i], copyDstDescriptorRedirectIndices[i], copySrcMutableSamplerRedirectIndices[i], copyDstMutableSamplerRedirectIndices[i])) return false; } for (auto i=0; iprocessWrite(write,params.pWriteTypes[i]); + write.dstSet->processWrite(write,params.pWriteTypes[i], writeDescriptorRedirectIndices[i], writeMutableSamplerRedirectIndices[i]); } for (const auto& copy : descriptorCopies) - copy.dstSet->processCopy(copy); + copy.dstSet->processCopy(copy, copyTypes[i], copySrcDescriptorRedirectIndices[i], copyDstDescriptorRedirectIndices[i], copySrcMutableSamplerRedirectIndices[i], copyDstMutableSamplerRedirectIndices[i]); updateDescriptorSets_impl(params); From 346f61adcf46fa5d416fa29d71f01b0d435b4715 Mon Sep 17 00:00:00 2001 From: Fletterio Date: Mon, 1 Jul 2024 14:32:03 -0300 Subject: [PATCH 12/18] Refactored CBindingRedirect's types to require explicit construction --- include/nbl/asset/IDescriptorSet.h | 26 ++++---- include/nbl/asset/IDescriptorSetLayout.h | 29 +++++---- include/nbl/asset/utils/ICPUVirtualTexture.h | 9 +-- include/nbl/video/IGPUDescriptorSet.h | 44 ++++++------- src/nbl/asset/IAssetManager.cpp | 2 +- src/nbl/asset/ICPUDescriptorSet.cpp | 28 +++------ .../CGraphicsPipelineLoaderMTL.cpp | 2 +- src/nbl/video/CVulkanLogicalDevice.cpp | 8 +-- src/nbl/video/IGPUDescriptorSet.cpp | 62 ++++++++++++------- src/nbl/video/ILogicalDevice.cpp | 2 +- 10 files changed, 107 insertions(+), 105 deletions(-) diff --git a/include/nbl/asset/IDescriptorSet.h b/include/nbl/asset/IDescriptorSet.h index dbe40af1e4..4eec5542e3 100644 --- a/include/nbl/asset/IDescriptorSet.h +++ b/include/nbl/asset/IDescriptorSet.h @@ -47,9 +47,10 @@ class IDescriptorSet : public virtual core::IReferenceCounted // TODO: try to re }; struct SImageInfo { - // If the binding is a SAMPLER, this field is ignored IImage::LAYOUT imageLayout; - // If the binding is COMBINED, this will be ignored if the DS layout already has an immutable sampler specified for the binding + }; + struct SCombinedImageSamplerInfo : SImageInfo + { core::smart_refctd_ptr sampler; }; @@ -58,13 +59,14 @@ class IDescriptorSet : public virtual core::IReferenceCounted // TODO: try to re { SBufferImageInfo() { - memset(&buffer, 0, core::max(sizeof(buffer), sizeof(image))); + memset(&buffer, 0, core::max(sizeof(buffer), sizeof(combinedImageSampler))); }; ~SBufferImageInfo() {}; SBufferInfo buffer; SImageInfo image; + SCombinedImageSamplerInfo combinedImageSampler; } info; SDescriptorInfo() {} @@ -93,34 +95,32 @@ class IDescriptorSet : public virtual core::IReferenceCounted // TODO: try to re } ~SDescriptorInfo() { - if (desc and (desc->getTypeCategory()==IDescriptor::EC_IMAGE or desc->getTypeCategory() == IDescriptor::EC_SAMPLER)) - info.image.sampler = nullptr; } inline SDescriptorInfo& operator=(const SDescriptorInfo& other) { - if (desc and (desc->getTypeCategory()==IDescriptor::EC_IMAGE or desc->getTypeCategory() == IDescriptor::EC_SAMPLER)) - info.image.sampler = nullptr; + if (desc and desc->getTypeCategory()==IDescriptor::EC_IMAGE) + info.combinedImageSampler.sampler = nullptr; desc = other.desc; const auto type = desc->getTypeCategory(); - if (type!=IDescriptor::EC_IMAGE and type!=IDescriptor::EC_SAMPLER) + if (type!=IDescriptor::EC_IMAGE) info.buffer = other.info.buffer; else - info.image = other.info.image; + info.combinedImageSampler = other.info.combinedImageSampler; return *this; } inline SDescriptorInfo& operator=(SDescriptorInfo&& other) { - if (desc and (desc->getTypeCategory() == IDescriptor::EC_IMAGE or desc->getTypeCategory() == IDescriptor::EC_SAMPLER)) - info.image = {nullptr,IImage::LAYOUT::UNDEFINED}; + if (desc and desc->getTypeCategory() == IDescriptor::EC_IMAGE) + info.combinedImageSampler = {IImage::LAYOUT::UNDEFINED, nullptr}; desc = std::move(other.desc); if (desc) { const auto type = desc->getTypeCategory(); - if (type!=IDescriptor::EC_IMAGE and type!=IDescriptor::EC_SAMPLER) + if (type!=IDescriptor::EC_IMAGE) info.buffer = other.info.buffer; else - info.image = other.info.image; + info.combinedImageSampler = other.info.combinedImageSampler; } return *this; } diff --git a/include/nbl/asset/IDescriptorSetLayout.h b/include/nbl/asset/IDescriptorSetLayout.h index 2d425c84ac..729ba5ccd8 100644 --- a/include/nbl/asset/IDescriptorSetLayout.h +++ b/include/nbl/asset/IDescriptorSetLayout.h @@ -70,19 +70,22 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr struct binding_number_t { - inline binding_number_t(const uint32_t d) : data(d) {} + explicit inline binding_number_t(const uint32_t d) : data(d) {} + inline binding_number_t() : data(0) {} uint32_t data; }; struct storage_offset_t { - inline storage_offset_t(const uint32_t d) : data(d) {} + explicit inline storage_offset_t(const uint32_t d) : data(d) {} + inline storage_offset_t() : data(0) {} uint32_t data; }; struct storage_range_index_t { - inline storage_range_index_t(const uint32_t d) : data(d) {} + explicit inline storage_range_index_t(const uint32_t d) : data(d) {} + inline storage_range_index_t() : data(0) {} uint32_t data; }; @@ -93,18 +96,18 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr inline storage_range_index_t findBindingStorageIndex(const binding_number_t binding) const { if (!m_bindingNumbers) - return { Invalid }; + return storage_range_index_t(Invalid); assert(m_storageOffsets && (m_count != 0u)); auto found = std::lower_bound(m_bindingNumbers, m_bindingNumbers + m_count, binding, [](binding_number_t a, binding_number_t b) -> bool {return a.data < b.data; }); if ((found >= m_bindingNumbers + m_count) || (found->data != binding.data)) - return { Invalid }; + return storage_range_index_t(Invalid); const uint32_t foundIndex = found - m_bindingNumbers; assert(foundIndex < m_count); - return { foundIndex }; + return storage_range_index_t(foundIndex); } inline binding_number_t getBinding(const storage_range_index_t index) const @@ -134,7 +137,7 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr inline storage_offset_t getStorageOffset(const storage_range_index_t index) const { assert(index.data < m_count); - return (index.data == 0u) ? 0u : m_storageOffsets[index.data - 1]; + return storage_offset_t((index.data == 0u) ? 0u : m_storageOffsets[index.data - 1].data); } // The following are merely convenience functions for one off use. @@ -162,7 +165,7 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr { const auto index = findBindingStorageIndex(binding); if (index.data == Invalid) - return { Invalid }; + return storage_offset_t(Invalid); return getStorageOffset(index); } @@ -308,19 +311,19 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr for (uint32_t b = 0u; b < bindingCnt; ++b) { - uint32_t bindingNumber = m_descriptorRedirects[t].m_bindingNumbers[b].data; - uint32_t otherBindingNumber = CBindingRedirect::Invalid; + auto bindingNumber = m_descriptorRedirects[t].m_bindingNumbers[b]; + CBindingRedirect::template binding_number_t otherBindingNumber(CBindingRedirect::Invalid); // TODO: std::find instead? for (uint32_t ob = 0u; ob < otherBindingCnt; ++ob) { - if (bindingNumber == other->m_descriptorRedirects[t].m_bindingNumbers[ob].data) + if (bindingNumber.data == other->m_descriptorRedirects[t].m_bindingNumbers[ob].data) { - otherBindingNumber = ob; + otherBindingNumber.data = ob; break; } } - if (otherBindingNumber == CBindingRedirect::Invalid) + if (otherBindingNumber.data == CBindingRedirect::Invalid) return false; diff --git a/include/nbl/asset/utils/ICPUVirtualTexture.h b/include/nbl/asset/utils/ICPUVirtualTexture.h index d1bcd96339..b20f142160 100644 --- a/include/nbl/asset/utils/ICPUVirtualTexture.h +++ b/include/nbl/asset/utils/ICPUVirtualTexture.h @@ -535,8 +535,9 @@ class ICPUVirtualTexture final : public IVirtualTexturegetDescriptorInfos(_pgtBinding, IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER); + auto pgtInfos = _dstSet->getDescriptorInfos(redirect_t::binding_number_t(_pgtBinding), IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER); if (pgtInfos.empty()) return false; // TODO: Log @@ -545,13 +546,13 @@ class ICPUVirtualTexture final : public IVirtualTexture(getPageTableView()); } auto updateSamplersBinding = [&](const uint32_t binding, const auto& views) -> bool { - auto infos = _dstSet->getDescriptorInfos(binding, IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER); + auto infos = _dstSet->getDescriptorInfos(redirect_t::binding_number_t(binding), IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER); if (infos.size() < views.size()) return false; // TODO: Log @@ -561,7 +562,7 @@ class ICPUVirtualTexture final : public IVirtualTexture(asset::IDescriptor::E_TYPE::ET_COUNT); t++) + for (auto t = 0u; t < static_cast(asset::IDescriptor::E_TYPE::ET_COUNT); t++) { const auto type = static_cast(t); const auto& bindingRedirect = getLayout()->getDescriptorRedirect(type); - if (bindingRedirect.getStorageOffset(redirect_t::binding_number_t{binding}).data!=redirect_t::Invalid) + bindingStorageIndex = bindingRedirect.findBindingStorageIndex(binding); + if (bindingStorageIndex.data != redirect_t::Invalid) return type; } return asset::IDescriptor::E_TYPE::ET_COUNT; } - // Same as above, but also retrieving the binding's storage index for its corresponsing redirect - inline asset::IDescriptor::E_TYPE getBindingType(const uint32_t binding, uint32_t& bindingStorageIndex) const + // If you only need the type + inline asset::IDescriptor::E_TYPE getBindingType(const IGPUDescriptorSetLayout::CBindingRedirect::binding_number_t binding) const { - for (auto t = 0u; t < static_cast(asset::IDescriptor::E_TYPE::ET_COUNT); t++) - { - const auto type = static_cast(t); - const auto& bindingRedirect = getLayout()->getDescriptorRedirect(type); - bindingStorageIndex = bindingRedirect.findBindingStorageIndex(binding).data; - if (bindingStorageIndex != redirect_t::Invalid) - return type; - } - return asset::IDescriptor::E_TYPE::ET_COUNT; + IGPUDescriptorSetLayout::CBindingRedirect::storage_range_index_t dummy(0); + return getBindingType(binding, dummy); } + + protected: IGPUDescriptorSet(core::smart_refctd_ptr&& _layout, core::smart_refctd_ptr&& pool, IDescriptorPool::SStorageOffsets&& offsets); virtual ~IGPUDescriptorSet(); @@ -100,9 +96,9 @@ class IGPUDescriptorSet : public asset::IDescriptorSet* getDescriptors(const asset::IDescriptor::E_TYPE type, const uint32_t binding) const + inline core::smart_refctd_ptr* getDescriptors(const asset::IDescriptor::E_TYPE type, const redirect_t::binding_number_t binding) const { - const auto localOffset = getLayout()->getDescriptorRedirect(type).getStorageOffset(redirect_t::binding_number_t{ binding }).data; + const auto localOffset = getLayout()->getDescriptorRedirect(type).getStorageOffset(binding).data; if (localOffset == ~0) return nullptr; @@ -113,10 +109,10 @@ class IGPUDescriptorSet : public asset::IDescriptorSet* getDescriptorsIndexed(const asset::IDescriptor::E_TYPE type, const uint32_t bindingStorageIndex) const + // Same as above, but amortizes lookup if you already have an index into the type's redirect + inline core::smart_refctd_ptr* getDescriptors(const asset::IDescriptor::E_TYPE type, const redirect_t::storage_range_index_t bindingStorageIndex) const { - const auto localOffset = getLayout()->getDescriptorRedirect(type).getStorageOffset(redirect_t::storage_range_index_t{ bindingStorageIndex }).data; + const auto localOffset = getLayout()->getDescriptorRedirect(type).getStorageOffset(bindingStorageIndex).data; if (localOffset == ~0) return nullptr; @@ -127,9 +123,9 @@ class IGPUDescriptorSet : public asset::IDescriptorSet* getMutableCombinedSamplers(const uint32_t binding) const + inline core::smart_refctd_ptr* getMutableCombinedSamplers(const redirect_t::binding_number_t binding) const { - const auto localOffset = getLayout()->getMutableCombinedSamplerRedirect().getStorageOffset(redirect_t::binding_number_t{ binding }).data; + const auto localOffset = getLayout()->getMutableCombinedSamplerRedirect().getStorageOffset(binding).data; if (localOffset == getLayout()->getMutableCombinedSamplerRedirect().Invalid) return nullptr; @@ -141,9 +137,9 @@ class IGPUDescriptorSet : public asset::IDescriptorSet* getMutableCombinedSamplersIndexed(const uint32_t bindingStorageIndex) const + inline core::smart_refctd_ptr* getMutableCombinedSamplers(const redirect_t::storage_range_index_t bindingStorageIndex) const { - const auto localOffset = getLayout()->getMutableCombinedSamplerRedirect().getStorageOffset(redirect_t::storage_range_index_t{ bindingStorageIndex }).data; + const auto localOffset = getLayout()->getMutableCombinedSamplerRedirect().getStorageOffset(bindingStorageIndex).data; if (localOffset == getLayout()->getMutableCombinedSamplerRedirect().Invalid) return nullptr; diff --git a/src/nbl/asset/IAssetManager.cpp b/src/nbl/asset/IAssetManager.cpp index b6a93b75ca..d2a06e9e5b 100644 --- a/src/nbl/asset/IAssetManager.cpp +++ b/src/nbl/asset/IAssetManager.cpp @@ -360,7 +360,7 @@ void IAssetManager::insertBuiltinAssets() //for filling this UBO with actual data, one can use asset::SBasicViewParameters struct defined in nbl/asset/asset_utils.h asset::fillBufferWithDeadBeef(ubo.get()); - auto descriptorInfos = ds1->getDescriptorInfos(0u, IDescriptor::E_TYPE::ET_UNIFORM_BUFFER); + auto descriptorInfos = ds1->getDescriptorInfos(ICPUDescriptorSetLayout::CBindingRedirect::binding_number_t(0), IDescriptor::E_TYPE::ET_UNIFORM_BUFFER); descriptorInfos.begin()[0].desc = std::move(ubo); descriptorInfos.begin()[0].info.buffer.offset = 0ull; descriptorInfos.begin()[0].info.buffer.size = UBO_SZ; diff --git a/src/nbl/asset/ICPUDescriptorSet.cpp b/src/nbl/asset/ICPUDescriptorSet.cpp index d5c16b7098..40a38cfe29 100644 --- a/src/nbl/asset/ICPUDescriptorSet.cpp +++ b/src/nbl/asset/ICPUDescriptorSet.cpp @@ -59,8 +59,8 @@ core::smart_refctd_ptr ICPUDescriptorSet::clone(uint32_t _depth) const auto category = getCategoryFromType(type); - if (category == IDescriptor::E_CATEGORY::EC_IMAGE or category == IDescriptor::E_CATEGORY::EC_SAMPLER) - dstDescriptorInfo.info.image = srcDescriptorInfo.info.image; + if (category == IDescriptor::E_CATEGORY::EC_IMAGE) + dstDescriptorInfo.info.combinedImageSampler = srcDescriptorInfo.info.combinedImageSampler; else dstDescriptorInfo.info.buffer = srcDescriptorInfo.info.buffer; @@ -95,8 +95,8 @@ core::smart_refctd_ptr ICPUDescriptorSet::clone(uint32_t _depth) const // Clone the sampler. { - if ((category == IDescriptor::E_CATEGORY::EC_IMAGE or category == IDescriptor::E_CATEGORY::EC_SAMPLER) and srcDescriptorInfo.info.image.sampler) - dstDescriptorInfo.info.image.sampler = core::smart_refctd_ptr_static_cast(srcDescriptorInfo.info.image.sampler->clone(_depth - 1u)); + if (category == IDescriptor::E_CATEGORY::EC_IMAGE and srcDescriptorInfo.info.combinedImageSampler.sampler) + dstDescriptorInfo.info.combinedImageSampler.sampler = core::smart_refctd_ptr_static_cast(srcDescriptorInfo.info.combinedImageSampler.sampler->clone(_depth - 1u)); } } else @@ -139,16 +139,13 @@ void ICPUDescriptorSet::convertToDummyObject(uint32_t referenceLevelsBelowToConv case IDescriptor::E_CATEGORY::EC_SAMPLER: { static_cast(descriptorInfos[i].desc.get())->convertToDummyObject(referenceLevelsBelowToConvert); - // should add asserts here? Not sure - if (descriptorInfos[i].info.image.sampler) - descriptorInfos[i].info.image.sampler->convertToDummyObject(referenceLevelsBelowToConvert); } break; case IDescriptor::E_CATEGORY::EC_IMAGE: { static_cast(descriptorInfos[i].desc.get())->convertToDummyObject(referenceLevelsBelowToConvert); - if (descriptorInfos[i].info.image.sampler) - descriptorInfos[i].info.image.sampler->convertToDummyObject(referenceLevelsBelowToConvert); + if (descriptorInfos[i].info.combinedImageSampler.sampler) + descriptorInfos[i].info.combinedImageSampler.sampler->convertToDummyObject(referenceLevelsBelowToConvert); } break; case IDescriptor::EC_BUFFER_VIEW: @@ -198,16 +195,13 @@ void ICPUDescriptorSet::restoreFromDummy_impl(IAsset* _other, uint32_t _levelsBe case IDescriptor::EC_SAMPLER: { restoreFromDummy_impl_call(static_cast(descriptorInfos[i].desc.get()), static_cast(otherDescriptorInfos[i].desc.get()), _levelsBelow); - // should add asserts here? Not sure - if (descriptorInfos[i].info.image.sampler && otherDescriptorInfos[i].info.image.sampler) - restoreFromDummy_impl_call(descriptorInfos[i].info.image.sampler.get(), otherDescriptorInfos[i].info.image.sampler.get(), _levelsBelow); } break; case IDescriptor::EC_IMAGE: { restoreFromDummy_impl_call(static_cast(descriptorInfos[i].desc.get()), static_cast(otherDescriptorInfos[i].desc.get()), _levelsBelow); - if (descriptorInfos[i].info.image.sampler && otherDescriptorInfos[i].info.image.sampler) - restoreFromDummy_impl_call(descriptorInfos[i].info.image.sampler.get(), otherDescriptorInfos[i].info.image.sampler.get(), _levelsBelow); + if (descriptorInfos[i].info.combinedImageSampler.sampler && otherDescriptorInfos[i].info.combinedImageSampler.sampler) + restoreFromDummy_impl_call(descriptorInfos[i].info.combinedImageSampler.sampler.get(), otherDescriptorInfos[i].info.combinedImageSampler.sampler.get(), _levelsBelow); } break; case IDescriptor::EC_BUFFER_VIEW: @@ -254,10 +248,6 @@ bool ICPUDescriptorSet::isAnyDependencyDummy_impl(uint32_t _levelsBelow) const { if (static_cast(descriptorInfos[i].desc.get())->isAnyDependencyDummy(_levelsBelow)) return true; - - // should add asserts here? Not sure - if (descriptorInfos[i].info.image.sampler && descriptorInfos[i].info.image.sampler->isAnyDependencyDummy(_levelsBelow)) - return true; } break; case IDescriptor::EC_IMAGE: @@ -265,7 +255,7 @@ bool ICPUDescriptorSet::isAnyDependencyDummy_impl(uint32_t _levelsBelow) const if (static_cast(descriptorInfos[i].desc.get())->isAnyDependencyDummy(_levelsBelow)) return true; - if (descriptorInfos[i].info.image.sampler && descriptorInfos[i].info.image.sampler->isAnyDependencyDummy(_levelsBelow)) + if (descriptorInfos[i].info.combinedImageSampler.sampler && descriptorInfos[i].info.combinedImageSampler.sampler->isAnyDependencyDummy(_levelsBelow)) return true; } break; diff --git a/src/nbl/asset/interchange/CGraphicsPipelineLoaderMTL.cpp b/src/nbl/asset/interchange/CGraphicsPipelineLoaderMTL.cpp index bebaeda14c..70ac5d2d19 100644 --- a/src/nbl/asset/interchange/CGraphicsPipelineLoaderMTL.cpp +++ b/src/nbl/asset/interchange/CGraphicsPipelineLoaderMTL.cpp @@ -738,7 +738,7 @@ core::smart_refctd_ptr CGraphicsPipelineLoaderMTL::makeDescSe auto dummy2d = _ctx.loaderOverride->findDefaultAsset("nbl/builtin/image_view/dummy2d",_ctx.inner,_ctx.topHierarchyLevel+ICPURenderpassIndependentPipeline::IMAGEVIEW_HIERARCHYLEVELS_BELOW).first; for (uint32_t i = 0u; i <= CMTLMetadata::CRenderpassIndependentPipeline::EMP_REFL_POSX; ++i) { - auto descriptorInfos = ds->getDescriptorInfos(i, IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER); + auto descriptorInfos = ds->getDescriptorInfos(ICPUDescriptorSetLayout::CBindingRedirect::binding_number_t(i), IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER); descriptorInfos.begin()[0].desc = _views[i] ? std::move(_views[i]) : dummy2d; descriptorInfos.begin()[0].info.image.imageLayout = IImage::LAYOUT::READ_ONLY_OPTIMAL; } diff --git a/src/nbl/video/CVulkanLogicalDevice.cpp b/src/nbl/video/CVulkanLogicalDevice.cpp index ae5edff2e2..71e740688d 100644 --- a/src/nbl/video/CVulkanLogicalDevice.cpp +++ b/src/nbl/video/CVulkanLogicalDevice.cpp @@ -686,9 +686,7 @@ void CVulkanLogicalDevice::updateDescriptorSets_impl(const SUpdateDescriptorSets outWrite->pImageInfo = outImageInfo; for (auto j = 0u; j < write.count; j++, outImageInfo++) { - const auto& imageInfo = infos[j].info.image; - // Write was validated so we can just get the sampler - outImageInfo->sampler = static_cast(imageInfo.sampler.get())->getInternalObject(); + outImageInfo->sampler = static_cast(infos[j].desc.get())->getInternalObject(); } } break; case asset::IDescriptor::EC_BUFFER: @@ -707,7 +705,7 @@ void CVulkanLogicalDevice::updateDescriptorSets_impl(const SUpdateDescriptorSets outWrite->pImageInfo = outImageInfo; for (auto j=0u; jsampler = imageInfo.sampler ? static_cast(imageInfo.sampler.get())->getInternalObject():VK_NULL_HANDLE; outImageInfo->imageView = static_cast(infos[j].desc.get())->getInternalObject(); outImageInfo->imageLayout = getVkImageLayoutFromImageLayout(imageInfo.imageLayout); @@ -776,7 +774,7 @@ void CVulkanLogicalDevice::nullifyDescriptors_impl(const SDropDescriptorSetsPara for (auto i=0; igetBindingType(write.binding); + auto descriptorType = write.dstSet->getBindingType(IGPUDescriptorSetLayout::CBindingRedirect::binding_number_t(write.binding)); outWrite->dstSet = static_cast(write.dstSet)->getInternalObject(); outWrite->dstBinding = write.binding; diff --git a/src/nbl/video/IGPUDescriptorSet.cpp b/src/nbl/video/IGPUDescriptorSet.cpp index 9af9b6986d..0c96784957 100644 --- a/src/nbl/video/IGPUDescriptorSet.cpp +++ b/src/nbl/video/IGPUDescriptorSet.cpp @@ -43,12 +43,14 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor assert(write.dstSet == this); const char* debugName = getDebugName(); + using redirect_t = IGPUDescriptorSetLayout::CBindingRedirect; + const redirect_t::binding_number_t bindingNumber(write.binding); // screw it, we'll need to replace the descriptor writing with update templates of descriptor buffer soon anyway - uint32_t descriptorRedirectBindingIndex; - const auto descriptorType = getBindingType(write.binding, descriptorRedirectBindingIndex); + redirect_t::storage_range_index_t descriptorRedirectBindingIndex; + const auto descriptorType = getBindingType(bindingNumber, descriptorRedirectBindingIndex); - auto* descriptors = getDescriptorsIndexed(descriptorType, descriptorRedirectBindingIndex); + auto* descriptors = getDescriptors(descriptorType, descriptorRedirectBindingIndex); if (!descriptors) { if (debugName) @@ -60,7 +62,7 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor } core::smart_refctd_ptr* mutableSamplers = nullptr; - if (descriptorType == asset::IDescriptor::E_TYPE::ET_SAMPLER or (descriptorType == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER and write.info->info.image.sampler)) + if (descriptorType == asset::IDescriptor::E_TYPE::ET_SAMPLER or (descriptorType == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER and write.info->info.combinedImageSampler.sampler)) { if (m_layout->getImmutableSamplerRedirect().getCount(IGPUDescriptorSetLayout::CBindingRedirect::binding_number_t{ write.binding }) != 0) { @@ -82,7 +84,7 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor return asset::IDescriptor::E_TYPE::ET_COUNT; } - auto* sampler = descriptorType == asset::IDescriptor::E_TYPE::ET_SAMPLER ? reinterpret_cast(write.info[i].desc.get()) : write.info[i].info.image.sampler.get(); + auto* sampler = descriptorType == asset::IDescriptor::E_TYPE::ET_SAMPLER ? reinterpret_cast(write.info[i].desc.get()) : write.info[i].info.combinedImageSampler.sampler.get(); if (not sampler) { if (debugName) m_pool->m_logger.log("Null sampler provided when trying to write descriptor set (%s, %p). All writes should provide a sampler.", system::ILogger::ELL_ERROR, debugName, write.dstSet); @@ -101,7 +103,7 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor } if (descriptorType == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) { - mutableSamplers = getMutableCombinedSamplers(write.binding); + mutableSamplers = getMutableCombinedSamplers(bindingNumber); // Should never reach here, the GetTypeCategory check earlier should ensure that if (!mutableSamplers) { @@ -122,15 +124,17 @@ void IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSe { assert(write.dstSet == this); - auto descriptorRedirectBindingIndex = getLayout()->getDescriptorRedirect(type).findBindingStorageIndex(write.binding).data; - auto* descriptors = getDescriptorsIndexed(type, descriptorRedirectBindingIndex); + using redirect_t = IGPUDescriptorSetLayout::CBindingRedirect; + const redirect_t::binding_number_t bindingNumber(write.binding); + auto descriptorRedirectBindingIndex = getLayout()->getDescriptorRedirect(type).findBindingStorageIndex(bindingNumber); + auto* descriptors = getDescriptors(type, descriptorRedirectBindingIndex); assert(descriptors); core::smart_refctd_ptr* mutableSamplers = nullptr; if (type == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER - and write.info->info.image.sampler) + and write.info->info.combinedImageSampler.sampler) { - mutableSamplers = getMutableCombinedSamplers(write.binding); + mutableSamplers = getMutableCombinedSamplers(bindingNumber); assert(mutableSamplers); } @@ -139,7 +143,7 @@ void IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSe descriptors[j] = write.info[j].desc; if (mutableSamplers) - mutableSamplers[j] = write.info[j].info.image.sampler; + mutableSamplers[j] = write.info[j].info.combinedImageSampler.sampler; } auto& bindingRedirect = m_layout->getDescriptorRedirect(type); auto bindingCreateFlags = bindingRedirect.getCreateFlags(redirect_t::storage_range_index_t{ descriptorRedirectBindingIndex }); @@ -151,17 +155,19 @@ void IGPUDescriptorSet::dropDescriptors(const IGPUDescriptorSet::SDropDescriptor { assert(drop.dstSet == this); - uint32_t descriptorRedirectBindingIndex; - const auto descriptorType = getBindingType(drop.binding, descriptorRedirectBindingIndex); + using redirect_t = IGPUDescriptorSetLayout::CBindingRedirect; + const redirect_t::binding_number_t bindingNumber(drop.binding); + redirect_t::storage_range_index_t descriptorRedirectBindingIndex; + const auto descriptorType = getBindingType(bindingNumber, descriptorRedirectBindingIndex); - auto* dstDescriptors = drop.dstSet->getDescriptorsIndexed(descriptorType, descriptorRedirectBindingIndex); + auto* dstDescriptors = drop.dstSet->getDescriptors(descriptorType, descriptorRedirectBindingIndex); if (dstDescriptors) for (uint32_t i = 0; i < drop.count; i++) dstDescriptors[drop.arrayElement + i] = nullptr; if (asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER == descriptorType) { - auto* dstSamplers = drop.dstSet->getMutableCombinedSamplers(drop.binding); + auto* dstSamplers = drop.dstSet->getMutableCombinedSamplers(bindingNumber); if (dstSamplers) for (uint32_t i = 0; i < drop.count; i++) dstSamplers[drop.arrayElement + i] = nullptr; @@ -179,6 +185,10 @@ bool IGPUDescriptorSet::validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet { assert(copy.dstSet == this); + using redirect_t = IGPUDescriptorSetLayout::CBindingRedirect; + redirect_t::binding_number_t srcBindingNumber(copy.srcBinding); + redirect_t::binding_number_t dstBindingNumber(copy.dstBinding); + const char* srcDebugName = copy.srcSet->getDebugName(); const char* dstDebugName = copy.dstSet->getDebugName(); @@ -186,11 +196,11 @@ bool IGPUDescriptorSet::validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet { const auto type = static_cast(t); - auto* srcDescriptors = copy.srcSet->getDescriptors(type, copy.srcBinding); - auto* dstDescriptors = copy.dstSet->getDescriptors(type, copy.dstBinding); + auto* srcDescriptors = copy.srcSet->getDescriptors(type, srcBindingNumber); + auto* dstDescriptors = copy.dstSet->getDescriptors(type, dstBindingNumber); - auto* srcSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.srcSet->getMutableCombinedSamplers(copy.srcBinding); - auto* dstSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.dstSet->getMutableCombinedSamplers(copy.dstBinding); + auto* srcSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.srcSet->getMutableCombinedSamplers(srcBindingNumber); + auto* dstSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.dstSet->getMutableCombinedSamplers(dstBindingNumber); if ((!srcDescriptors != !dstDescriptors) || (!srcSamplers != !dstSamplers)) { @@ -210,15 +220,19 @@ void IGPUDescriptorSet::processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& { assert(copy.dstSet == this); + using redirect_t = IGPUDescriptorSetLayout::CBindingRedirect; + redirect_t::binding_number_t srcBindingNumber(copy.srcBinding); + redirect_t::binding_number_t dstBindingNumber(copy.dstBinding); + for (uint32_t t = 0; t < static_cast(asset::IDescriptor::E_TYPE::ET_COUNT); ++t) { const auto type = static_cast(t); auto& bindingRedirect = m_layout->getDescriptorRedirect(type); - auto* srcDescriptors = copy.srcSet->getDescriptors(type, copy.srcBinding); + auto* srcDescriptors = copy.srcSet->getDescriptors(type, srcBindingNumber); // We can do this because dstSet === this as asserted at the start - auto descriptorRedirectBindingIndex = bindingRedirect.findBindingStorageIndex({ copy.dstBinding }).data; - auto* dstDescriptors = copy.dstSet->getDescriptorsIndexed(type, descriptorRedirectBindingIndex); + auto descriptorRedirectBindingIndex = bindingRedirect.findBindingStorageIndex(dstBindingNumber); + auto* dstDescriptors = copy.dstSet->getDescriptors(type, descriptorRedirectBindingIndex); if (!srcDescriptors) { assert(!dstDescriptors); @@ -226,8 +240,8 @@ void IGPUDescriptorSet::processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& } assert(dstDescriptors); - auto* srcSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.srcSet->getMutableCombinedSamplers(copy.srcBinding); - auto* dstSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.dstSet->getMutableCombinedSamplers(copy.dstBinding); + auto* srcSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.srcSet->getMutableCombinedSamplers(srcBindingNumber); + auto* dstSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.dstSet->getMutableCombinedSamplers(dstBindingNumber); assert(!(!srcSamplers != !dstSamplers)); diff --git a/src/nbl/video/ILogicalDevice.cpp b/src/nbl/video/ILogicalDevice.cpp index 3c63699b3e..092dddba8d 100644 --- a/src/nbl/video/ILogicalDevice.cpp +++ b/src/nbl/video/ILogicalDevice.cpp @@ -506,7 +506,7 @@ bool ILogicalDevice::nullifyDescriptors(const std::spanwasCreatedBy(this)) return false; - auto bindingType = ds->getBindingType(drop.binding); + auto bindingType = ds->getBindingType(IGPUDescriptorSetLayout::CBindingRedirect::binding_number_t(drop.binding)); auto writeCount = drop.count; switch (asset::IDescriptor::GetTypeCategory(bindingType)) { From 00b4311c5f6ec13f63eb7beae35e4841261d3740 Mon Sep 17 00:00:00 2001 From: Fletterio Date: Mon, 1 Jul 2024 15:16:05 -0300 Subject: [PATCH 13/18] Minor refactor --- src/nbl/video/IGPUDescriptorSet.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/nbl/video/IGPUDescriptorSet.cpp b/src/nbl/video/IGPUDescriptorSet.cpp index 0c96784957..5f8b2686ec 100644 --- a/src/nbl/video/IGPUDescriptorSet.cpp +++ b/src/nbl/video/IGPUDescriptorSet.cpp @@ -146,7 +146,7 @@ void IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSe mutableSamplers[j] = write.info[j].info.combinedImageSampler.sampler; } auto& bindingRedirect = m_layout->getDescriptorRedirect(type); - auto bindingCreateFlags = bindingRedirect.getCreateFlags(redirect_t::storage_range_index_t{ descriptorRedirectBindingIndex }); + auto bindingCreateFlags = bindingRedirect.getCreateFlags(descriptorRedirectBindingIndex); if (IGPUDescriptorSetLayout::writeIncrementsVersion(bindingCreateFlags)) incrementVersion(); } @@ -176,7 +176,7 @@ void IGPUDescriptorSet::dropDescriptors(const IGPUDescriptorSet::SDropDescriptor // we only increment the version to detect UPDATE-AFTER-BIND and automagically invalidate descriptor sets // so, only if we do the path that writes descriptors, do we want to increment version auto& bindingRedirect = m_layout->getDescriptorRedirect(descriptorType); - auto bindingCreateFlags = bindingRedirect.getCreateFlags(redirect_t::storage_range_index_t{ descriptorRedirectBindingIndex }); + auto bindingCreateFlags = bindingRedirect.getCreateFlags(descriptorRedirectBindingIndex); if (IGPUDescriptorSetLayout::writeIncrementsVersion(bindingCreateFlags)) incrementVersion(); } @@ -245,7 +245,7 @@ void IGPUDescriptorSet::processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& assert(!(!srcSamplers != !dstSamplers)); - auto bindingCreateFlags = bindingRedirect.getCreateFlags(redirect_t::storage_range_index_t{ descriptorRedirectBindingIndex }); + auto bindingCreateFlags = bindingRedirect.getCreateFlags(descriptorRedirectBindingIndex); if (IGPUDescriptorSetLayout::writeIncrementsVersion(bindingCreateFlags)) incrementVersion(); From 2ed53f0a594d10880cbd6a8a0df12baa8280b121 Mon Sep 17 00:00:00 2001 From: Fletterio Date: Wed, 3 Jul 2024 22:37:03 -0300 Subject: [PATCH 14/18] making dxc point right --- 3rdparty/dxc/dxc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/3rdparty/dxc/dxc b/3rdparty/dxc/dxc index 2a0f9968de..a08b6cbeb1 160000 --- a/3rdparty/dxc/dxc +++ b/3rdparty/dxc/dxc @@ -1 +1 @@ -Subproject commit 2a0f9968de8e554b6dc104e4bc0f7f7f7122f0cd +Subproject commit a08b6cbeb1038d14d0586d10a8cfa507b2fda8eb From 1e7511e0afe42becda8130f3a96032e28a58266d Mon Sep 17 00:00:00 2001 From: Fletterio Date: Thu, 4 Jul 2024 14:12:45 -0300 Subject: [PATCH 15/18] make examples point back to master --- examples_tests | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples_tests b/examples_tests index 6087164acf..d478a92baf 160000 --- a/examples_tests +++ b/examples_tests @@ -1 +1 @@ -Subproject commit 6087164acf7de85c253cddc49aee9fcb999be2e3 +Subproject commit d478a92baf92572434454bf624ad0821b79705f6 From bad473b6976e1261e077afc9ca4b3dda849dce0a Mon Sep 17 00:00:00 2001 From: Fletterio Date: Sat, 6 Jul 2024 15:57:36 -0300 Subject: [PATCH 16/18] Refactor following PR review done! --- include/nbl/asset/ICPUDescriptorSet.h | 4 -- include/nbl/asset/IDescriptorSet.h | 15 ++++-- include/nbl/asset/IDescriptorSetLayout.h | 14 ++--- include/nbl/asset/utils/IVirtualTexture.h | 2 +- include/nbl/scene/ILevelOfDetailLibrary.h | 2 +- src/nbl/asset/IAssetManager.cpp | 2 +- src/nbl/asset/ICPUDescriptorSet.cpp | 8 +-- src/nbl/video/CVulkanLogicalDevice.cpp | 4 +- src/nbl/video/IGPUDescriptorSet.cpp | 65 ++++++++++++++++------- src/nbl/video/ILogicalDevice.cpp | 4 +- 10 files changed, 74 insertions(+), 46 deletions(-) diff --git a/include/nbl/asset/ICPUDescriptorSet.h b/include/nbl/asset/ICPUDescriptorSet.h index b0d50bc3a2..7c49a385ea 100644 --- a/include/nbl/asset/ICPUDescriptorSet.h +++ b/include/nbl/asset/ICPUDescriptorSet.h @@ -99,10 +99,6 @@ class NBL_API2 ICPUDescriptorSet final : public IDescriptorSet m_descriptorInfos[static_cast(IDescriptor::E_TYPE::ET_COUNT)]; }; diff --git a/include/nbl/asset/IDescriptorSet.h b/include/nbl/asset/IDescriptorSet.h index 4eec5542e3..418446be1b 100644 --- a/include/nbl/asset/IDescriptorSet.h +++ b/include/nbl/asset/IDescriptorSet.h @@ -95,6 +95,8 @@ class IDescriptorSet : public virtual core::IReferenceCounted // TODO: try to re } ~SDescriptorInfo() { + if (desc && desc->getTypeCategory() == IDescriptor::EC_IMAGE) + info.combinedImageSampler.sampler = nullptr; } inline SDescriptorInfo& operator=(const SDescriptorInfo& other) @@ -102,11 +104,14 @@ class IDescriptorSet : public virtual core::IReferenceCounted // TODO: try to re if (desc and desc->getTypeCategory()==IDescriptor::EC_IMAGE) info.combinedImageSampler.sampler = nullptr; desc = other.desc; - const auto type = desc->getTypeCategory(); - if (type!=IDescriptor::EC_IMAGE) - info.buffer = other.info.buffer; - else - info.combinedImageSampler = other.info.combinedImageSampler; + if (desc) + { + const auto type = desc->getTypeCategory(); + if (type != IDescriptor::EC_IMAGE) + info.buffer = other.info.buffer; + else + info.combinedImageSampler = other.info.combinedImageSampler; + } return *this; } inline SDescriptorInfo& operator=(SDescriptorInfo&& other) diff --git a/include/nbl/asset/IDescriptorSetLayout.h b/include/nbl/asset/IDescriptorSetLayout.h index 729ba5ccd8..0e980b9bea 100644 --- a/include/nbl/asset/IDescriptorSetLayout.h +++ b/include/nbl/asset/IDescriptorSetLayout.h @@ -57,9 +57,9 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr core::bitflag createFlags; core::bitflag stageFlags; uint32_t count; - // Use this if you want an immutable sampler that is baked into the DS layout itself. - // If its `nullptr` then the sampler used is mutable and can be specified while writing the image descriptor to a binding while updating the DS. - const core::smart_refctd_ptr* samplers; + // Use this if you want immutable samplers that are baked into the DS layout itself. + // If it's `nullptr` then the samplers used are mutable and can be specified while writing the image descriptor to a binding while updating the DS. + const core::smart_refctd_ptr* immutableSamplers; }; // Maps a binding to a local (to descriptor set layout) offset. @@ -412,13 +412,13 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr { switch (b.type) { case IDescriptor::E_TYPE::ET_SAMPLER: - if (b.samplers) + if (b.immutableSamplers) buildInfo_immutableSamplers.emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); else buildInfo_descriptors[size_t(IDescriptor::E_TYPE::ET_SAMPLER)].emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); break; case IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER: - if (b.samplers) + if (b.immutableSamplers) buildInfo_immutableSamplers.emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); else buildInfo_mutableSamplers.emplace_back(b.binding, b.createFlags, b.stageFlags, b.count); @@ -440,13 +440,13 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted // TODO: tr for (const auto& b : _bindings) { - if ((b.type == IDescriptor::E_TYPE::ET_SAMPLER or b.type == IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) and b.samplers) + if ((b.type == IDescriptor::E_TYPE::ET_SAMPLER or b.type == IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) and b.immutableSamplers) { const auto localOffset = m_immutableSamplerRedirect.getStorageOffset(typename CBindingRedirect::binding_number_t(b.binding)).data; assert(localOffset != m_immutableSamplerRedirect.Invalid); auto* dst = m_immutableSamplers->begin() + localOffset; - std::copy_n(b.samplers, b.count, dst); + std::copy_n(b.immutableSamplers, b.count, dst); } } } diff --git a/include/nbl/asset/utils/IVirtualTexture.h b/include/nbl/asset/utils/IVirtualTexture.h index 3bf31057ae..9b335cfe15 100644 --- a/include/nbl/asset/utils/IVirtualTexture.h +++ b/include/nbl/asset/utils/IVirtualTexture.h @@ -1049,7 +1049,7 @@ class IVirtualTexture : public core::IReferenceCounted, public IVirtualTextureBa bnd.count = _count; bnd.stageFlags = asset::IShader::ESS_ALL; bnd.type = asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER; - bnd.samplers = _samplers; + bnd.immutableSamplers = _samplers; }; fillBinding(bindings[0], _pgtBinding, 1u, samplers); diff --git a/include/nbl/scene/ILevelOfDetailLibrary.h b/include/nbl/scene/ILevelOfDetailLibrary.h index 10b22c3143..e81ea4a96e 100644 --- a/include/nbl/scene/ILevelOfDetailLibrary.h +++ b/include/nbl/scene/ILevelOfDetailLibrary.h @@ -213,7 +213,7 @@ class ILevelOfDetailLibrary : public virtual core::IReferenceCounted bindings[i].type = asset::IDescriptor::E_TYPE::ET_STORAGE_BUFFER; bindings[i].count = 1u; bindings[i].stageFlags = asset::IShader::ESS_COMPUTE; - bindings[i].samplers = nullptr; + bindings[i].immutableSamplers = nullptr; } return device->createDescriptorSetLayout(bindings); } diff --git a/src/nbl/asset/IAssetManager.cpp b/src/nbl/asset/IAssetManager.cpp index d2a06e9e5b..fc3d9b0de0 100644 --- a/src/nbl/asset/IAssetManager.cpp +++ b/src/nbl/asset/IAssetManager.cpp @@ -245,7 +245,7 @@ void IAssetManager::insertBuiltinAssets() binding3.type = IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER; binding3.count = 1u; binding3.stageFlags = static_cast(asset::ICPUShader::ESS_FRAGMENT); - binding3.samplers = nullptr; + binding3.immutableSamplers = nullptr; auto ds3Layout = core::make_smart_refctd_ptr(&binding3, &binding3 + 1); addBuiltInToCaches(ds3Layout, "nbl/builtin/material/lambertian/singletexture/descriptor_set_layout/3"); // TODO find everything what has been using it so far diff --git a/src/nbl/asset/ICPUDescriptorSet.cpp b/src/nbl/asset/ICPUDescriptorSet.cpp index 40a38cfe29..9457fb2147 100644 --- a/src/nbl/asset/ICPUDescriptorSet.cpp +++ b/src/nbl/asset/ICPUDescriptorSet.cpp @@ -57,7 +57,7 @@ core::smart_refctd_ptr ICPUDescriptorSet::clone(uint32_t _depth) const const auto& srcDescriptorInfo = m_descriptorInfos[t]->begin()[i]; auto& dstDescriptorInfo = cp->m_descriptorInfos[t]->begin()[i]; - auto category = getCategoryFromType(type); + auto category = IDescriptor::GetTypeCategory(type); if (category == IDescriptor::E_CATEGORY::EC_IMAGE) dstDescriptorInfo.info.combinedImageSampler = srcDescriptorInfo.info.combinedImageSampler; @@ -127,7 +127,7 @@ void ICPUDescriptorSet::convertToDummyObject(uint32_t referenceLevelsBelowToConv auto descriptorInfos = m_descriptorInfos[t]->begin(); assert(descriptorInfos); - const auto category = getCategoryFromType(type); + const auto category = IDescriptor::GetTypeCategory(type); for (uint32_t i = 0u; i < descriptorCount; ++i) { switch (category) @@ -183,7 +183,7 @@ void ICPUDescriptorSet::restoreFromDummy_impl(IAsset* _other, uint32_t _levelsBe auto otherDescriptorInfos = other->m_descriptorInfos[t]->begin(); - const auto category = getCategoryFromType(type); + const auto category = IDescriptor::GetTypeCategory(type); for (uint32_t i = 0u; i < descriptorCount; ++i) { switch (category) @@ -234,7 +234,7 @@ bool ICPUDescriptorSet::isAnyDependencyDummy_impl(uint32_t _levelsBelow) const auto descriptorInfos = m_descriptorInfos[t]->begin(); assert(descriptorInfos); - const auto category = getCategoryFromType(type); + const auto category = IDescriptor::GetTypeCategory(type); for (uint32_t i = 0u; i < descriptorCount; ++i) { switch (category) diff --git a/src/nbl/video/CVulkanLogicalDevice.cpp b/src/nbl/video/CVulkanLogicalDevice.cpp index 71e740688d..332a46b3ec 100644 --- a/src/nbl/video/CVulkanLogicalDevice.cpp +++ b/src/nbl/video/CVulkanLogicalDevice.cpp @@ -551,13 +551,13 @@ core::smart_refctd_ptr CVulkanLogicalDevice::createDesc vkDescSetLayoutBinding.stageFlags = getVkShaderStageFlagsFromShaderStage(binding.stageFlags); vkDescSetLayoutBinding.pImmutableSamplers = nullptr; - if ((binding.type == asset::IDescriptor::E_TYPE::ET_SAMPLER or binding.type==asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) and binding.samplers and binding.count) + if ((binding.type == asset::IDescriptor::E_TYPE::ET_SAMPLER or binding.type==asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) and binding.immutableSamplers and binding.count) { // If descriptorType is VK_DESCRIPTOR_TYPE_SAMPLER or VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, and descriptorCount is not 0 and pImmutableSamplers is not NULL: // pImmutableSamplers must be a valid pointer to an array of descriptorCount valid VkSampler handles. const uint32_t samplerOffset = vk_samplers.size(); for (uint32_t i=0u; i(binding.samplers[i].get())->getInternalObject()); + vk_samplers.push_back(static_cast(binding.immutableSamplers[i].get())->getInternalObject()); vkDescSetLayoutBinding.pImmutableSamplers = vk_samplers.data()+samplerOffset; } } diff --git a/src/nbl/video/IGPUDescriptorSet.cpp b/src/nbl/video/IGPUDescriptorSet.cpp index 5f8b2686ec..547f4fee3c 100644 --- a/src/nbl/video/IGPUDescriptorSet.cpp +++ b/src/nbl/video/IGPUDescriptorSet.cpp @@ -49,8 +49,18 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor // screw it, we'll need to replace the descriptor writing with update templates of descriptor buffer soon anyway redirect_t::storage_range_index_t descriptorRedirectBindingIndex; const auto descriptorType = getBindingType(bindingNumber, descriptorRedirectBindingIndex); + if (asset::IDescriptor::E_TYPE::ET_COUNT == descriptorType) + { + if (debugName) + m_pool->m_logger.log("Descriptor set (%s, %p) has no binding %u.", system::ILogger::ELL_ERROR, debugName, this, write.binding); + else + m_pool->m_logger.log("Descriptor set (%p) has no binding %u.", system::ILogger::ELL_ERROR, this, write.binding); + + return asset::IDescriptor::E_TYPE::ET_COUNT; + } auto* descriptors = getDescriptors(descriptorType, descriptorRedirectBindingIndex); + // Left this check, but if the above passed then this should never be nullptr I believe if (!descriptors) { if (debugName) @@ -61,49 +71,52 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor return asset::IDescriptor::E_TYPE::ET_COUNT; } - core::smart_refctd_ptr* mutableSamplers = nullptr; + // Possible TODO: ensure the types are the same, not just categories! Requires IDescriptor to provide a virtual getType() method + for (uint32_t i = 0; i < write.count; ++i) + { + if (asset::IDescriptor::GetTypeCategory(descriptorType) != write.info[i].desc->getTypeCategory()) + { + if (debugName) + m_pool->m_logger.log("Descriptor set (%s, %p) doesn't allow descriptor of such type category at binding %u.", system::ILogger::ELL_ERROR, debugName, this, write.binding); + else + m_pool->m_logger.log("Descriptor set (%p) doesn't allow descriptor of such type category at binding %u.", system::ILogger::ELL_ERROR, this, write.binding); + + return asset::IDescriptor::E_TYPE::ET_COUNT; + } + } + if (descriptorType == asset::IDescriptor::E_TYPE::ET_SAMPLER or (descriptorType == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER and write.info->info.combinedImageSampler.sampler)) { if (m_layout->getImmutableSamplerRedirect().getCount(IGPUDescriptorSetLayout::CBindingRedirect::binding_number_t{ write.binding }) != 0) { if (debugName) - m_pool->m_logger.log("Trying to write samplers at binding %u of Descriptor set (%s, %p), but those are immutable.", system::ILogger::ELL_ERROR, debugName, this, write.binding); + m_pool->m_logger.log("Trying to write samplers at binding %u of Descriptor set (%s, %p), but those are immutable.", system::ILogger::ELL_ERROR, write.binding, debugName, this); else - m_pool->m_logger.log("Trying to write samplers at binding %u of Descriptor set (%p), but those are immutable.", system::ILogger::ELL_ERROR, this, write.binding); + m_pool->m_logger.log("Trying to write samplers at binding %u of Descriptor set (%p), but those are immutable.", system::ILogger::ELL_ERROR, write.binding, this); return asset::IDescriptor::E_TYPE::ET_COUNT; } for (uint32_t i=0; igetTypeCategory()) - { - if (debugName) - m_pool->m_logger.log("Descriptor set (%s, %p) doesn't allow descriptor of such type category at binding %u.", system::ILogger::ELL_ERROR, debugName, this, write.binding); - else - m_pool->m_logger.log("Descriptor set (%p) doesn't allow descriptor of such type category at binding %u.", system::ILogger::ELL_ERROR, this, write.binding); - - return asset::IDescriptor::E_TYPE::ET_COUNT; - } auto* sampler = descriptorType == asset::IDescriptor::E_TYPE::ET_SAMPLER ? reinterpret_cast(write.info[i].desc.get()) : write.info[i].info.combinedImageSampler.sampler.get(); if (not sampler) { if (debugName) - m_pool->m_logger.log("Null sampler provided when trying to write descriptor set (%s, %p). All writes should provide a sampler.", system::ILogger::ELL_ERROR, debugName, write.dstSet); + m_pool->m_logger.log("Null sampler provided when trying to write descriptor set (%s, %p) at binding %u. All writes should provide a sampler.", system::ILogger::ELL_ERROR, debugName, this, write.binding); else - m_pool->m_logger.log("Null sampler provided when trying to write descriptor set (%p). All writes should provide a sampler.", system::ILogger::ELL_ERROR, write.dstSet); + m_pool->m_logger.log("Null sampler provided when trying to write descriptor set (%p) at binding %u. All writes should provide a sampler.", system::ILogger::ELL_ERROR, this, write.binding); return asset::IDescriptor::E_TYPE::ET_COUNT; } - else if (not sampler->isCompatibleDevicewise(write.dstSet)) { + if (not sampler->isCompatibleDevicewise(write.dstSet)) { const char* samplerDebugName = sampler->getDebugName(); if (samplerDebugName && debugName) - m_pool->m_logger.log("Sampler (%s, %p) does not exist or is not device-compatible with descriptor set (%s, %p).", system::ILogger::ELL_ERROR, samplerDebugName, sampler, debugName, write.dstSet); + m_pool->m_logger.log("Sampler (%s, %p) does not exist or is not device-compatible with descriptor set (%s, %p).", system::ILogger::ELL_ERROR, samplerDebugName, sampler, debugName, this); else - m_pool->m_logger.log("Sampler (%p) does not exist or is not device-compatible with descriptor set (%p).", system::ILogger::ELL_ERROR, sampler, write.dstSet); + m_pool->m_logger.log("Sampler (%p) does not exist or is not device-compatible with descriptor set (%p).", system::ILogger::ELL_ERROR, sampler, this); return asset::IDescriptor::E_TYPE::ET_COUNT; } } - if (descriptorType == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) { - mutableSamplers = getMutableCombinedSamplers(bindingNumber); + core::smart_refctd_ptr* mutableSamplers = getMutableCombinedSamplers(bindingNumber); // Should never reach here, the GetTypeCategory check earlier should ensure that if (!mutableSamplers) { @@ -117,6 +130,20 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor } } + // https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#descriptorsets-combinedimagesampler + if (asset::IDescriptor::GetTypeCategory(descriptorType) == asset::IDescriptor::EC_IMAGE) + for (uint32_t i = 0; i < write.count; ++i) + { + auto layout = write.info[i].info.image.imageLayout; + if (not (asset::IImage::LAYOUT::GENERAL == layout or asset::IImage::LAYOUT::SHARED_PRESENT == layout or asset::IImage::LAYOUT::READ_ONLY_OPTIMAL == layout)) + { + if (debugName) + m_pool->m_logger.log("When writing to descriptor set (%s, %p), an image was provided with a layout that isn't GENERAL, SHARED_PRESENT or READ_ONLY_OPTIMAL", system::ILogger::ELL_ERROR, debugName, this); + else + m_pool->m_logger.log("When writing to descriptor set (%p), an image was provided with a layout that isn't GENERAL, SHARED_PRESENT or READ_ONLY_OPTIMAL", system::ILogger::ELL_ERROR, this); + } + } + return descriptorType; } diff --git a/src/nbl/video/ILogicalDevice.cpp b/src/nbl/video/ILogicalDevice.cpp index 092dddba8d..68659825be 100644 --- a/src/nbl/video/ILogicalDevice.cpp +++ b/src/nbl/video/ILogicalDevice.cpp @@ -394,9 +394,9 @@ core::smart_refctd_ptr ILogicalDevice::createDescriptor else if (binding.type==asset::IDescriptor::E_TYPE::ET_UNIFORM_BUFFER_DYNAMIC) dynamicUBOCount++; // If binding comes with samplers, we're specifying that this binding corresponds to immutable samplers - else if ((binding.type == asset::IDescriptor::E_TYPE::ET_SAMPLER or binding.type==asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) and binding.samplers) + else if ((binding.type == asset::IDescriptor::E_TYPE::ET_SAMPLER or binding.type==asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) and binding.immutableSamplers) { - auto* samplers = binding.samplers; + auto* samplers = binding.immutableSamplers; for (uint32_t i=0u; iwasCreatedBy(this))) return nullptr; From 8eb33e18fae86d464cf027cf7cd691e52bb751ca Mon Sep 17 00:00:00 2001 From: Fletterio Date: Sat, 6 Jul 2024 17:18:33 -0300 Subject: [PATCH 17/18] Removed senseless comment --- src/nbl/video/IGPUDescriptorSet.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/nbl/video/IGPUDescriptorSet.cpp b/src/nbl/video/IGPUDescriptorSet.cpp index 547f4fee3c..4204b0cc2d 100644 --- a/src/nbl/video/IGPUDescriptorSet.cpp +++ b/src/nbl/video/IGPUDescriptorSet.cpp @@ -71,7 +71,6 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor return asset::IDescriptor::E_TYPE::ET_COUNT; } - // Possible TODO: ensure the types are the same, not just categories! Requires IDescriptor to provide a virtual getType() method for (uint32_t i = 0; i < write.count; ++i) { if (asset::IDescriptor::GetTypeCategory(descriptorType) != write.info[i].desc->getTypeCategory()) From 53921b9f4b6edb4a01dfe5ec9a4067dc5d69cdbd Mon Sep 17 00:00:00 2001 From: Fletterio Date: Wed, 10 Jul 2024 15:03:30 -0300 Subject: [PATCH 18/18] Minor bugfix, update examples submodule pointer --- examples_tests | 2 +- src/nbl/video/ILogicalDevice.cpp | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/examples_tests b/examples_tests index daea3f951b..dd466f6db4 160000 --- a/examples_tests +++ b/examples_tests @@ -1 +1 @@ -Subproject commit daea3f951b59a4b53cec81c44786f5934994176e +Subproject commit dd466f6db4bdedd66f60bacdcc62b1409b10baf5 diff --git a/src/nbl/video/ILogicalDevice.cpp b/src/nbl/video/ILogicalDevice.cpp index 899f0dd403..9c15108fc7 100644 --- a/src/nbl/video/ILogicalDevice.cpp +++ b/src/nbl/video/ILogicalDevice.cpp @@ -473,7 +473,6 @@ bool ILogicalDevice::updateDescriptorSets(const std::span copyValidationResults(descriptorCopies.size()); @@ -490,7 +489,6 @@ bool ILogicalDevice::updateDescriptorSets(const std::spanvalidateCopy(copy); if (asset::IDescriptor::E_TYPE::ET_COUNT == copyValidationResults[i].type) return false; - i++; } for (auto i=0; i