Skip to content

Commit 3dd82fd

Browse files
achalpandeyyachalpandeyy
committed
More fixes and cleanups.
1 parent 4b5ac34 commit 3dd82fd

File tree

3 files changed

+50
-72
lines changed

3 files changed

+50
-72
lines changed

‎include/nbl/video/IDescriptorPool.h

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -81,33 +81,15 @@ class IDescriptorPool : public core::IReferenceCounted, public IBackendObject
8181
inline bool allowsFreeing() const { return m_creationParameters.flags.hasFlags(ECF_FREE_DESCRIPTOR_SET_BIT); }
8282

8383
protected:
84-
explicit IDescriptorPool(core::smart_refctd_ptr<const ILogicalDevice>&& dev, SCreateInfo&& createInfo)
85-
: IBackendObject(std::move(dev)), m_creationParameters(std::move(createInfo))
86-
{
87-
for (auto i = 0; i < static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT); ++i)
88-
m_descriptorAllocators[i] = std::make_unique<allocator_state_t>(m_creationParameters.maxDescriptorCount[i], m_creationParameters.flags.hasFlags(ECF_FREE_DESCRIPTOR_SET_BIT));
89-
90-
// For mutable samplers. We don't know if there will be mutable samplers in sets allocated by this pool when we create the pool.
91-
m_descriptorAllocators[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT)] = std::make_unique<allocator_state_t>(m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)], m_creationParameters.flags.hasFlags(ECF_FREE_DESCRIPTOR_SET_BIT));
92-
93-
// Initialize the storages.
94-
m_textureStorage = std::make_unique<core::StorageTrivializer<core::smart_refctd_ptr<video::IGPUImageView>>[]>(m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)]);
95-
m_mutableSamplerStorage = std::make_unique<core::StorageTrivializer<core::smart_refctd_ptr<video::IGPUSampler>>[]>(m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)]);
96-
m_storageImageStorage = std::make_unique<core::StorageTrivializer<core::smart_refctd_ptr<IGPUImageView>>[]>(m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_STORAGE_IMAGE)] + m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_INPUT_ATTACHMENT)]);
97-
m_UBO_SSBOStorage = std::make_unique<core::StorageTrivializer<core::smart_refctd_ptr<IGPUBuffer>>[]>(m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_UNIFORM_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_STORAGE_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_UNIFORM_BUFFER_DYNAMIC)] + m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_STORAGE_BUFFER_DYNAMIC)]);
98-
m_UTB_STBStorage = std::make_unique<core::StorageTrivializer<core::smart_refctd_ptr<IGPUBufferView>>[]>(m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_UNIFORM_TEXEL_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_STORAGE_BUFFER_DYNAMIC)]);
99-
m_accelerationStructureStorage = std::make_unique<core::StorageTrivializer<core::smart_refctd_ptr<IGPUAccelerationStructure>>[]>(m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_ACCELERATION_STRUCTURE)]);
100-
101-
m_allocatedDescriptorSets = std::make_unique<IGPUDescriptorSet* []>(m_creationParameters.maxSets);
102-
std::fill_n(m_allocatedDescriptorSets.get(), m_creationParameters.maxSets, nullptr);
103-
104-
m_descriptorSetAllocatorReservedSpace = std::make_unique<uint8_t[]>(core::IteratablePoolAddressAllocator<uint32_t>::reserved_size(1, m_creationParameters.maxSets, 1));
105-
m_descriptorSetAllocator = core::IteratablePoolAddressAllocator<uint32_t>(m_descriptorSetAllocatorReservedSpace.get(), 0, 0, 1, m_creationParameters.maxSets, 1);
106-
}
84+
IDescriptorPool(core::smart_refctd_ptr<const ILogicalDevice>&& dev, SCreateInfo&& createInfo);
10785

10886
virtual ~IDescriptorPool()
10987
{
11088
assert(m_descriptorSetAllocator.get_allocated_size() == 0);
89+
#ifdef _NBL_DEBUG
90+
for (uint32_t i = 0u; i < m_creationParameters.maxSets; ++i)
91+
assert(m_allocatedDescriptorSets[i] == nullptr);
92+
#endif
11193
}
11294

11395
virtual bool createDescriptorSets_impl(uint32_t count, const IGPUDescriptorSetLayout* const* layouts, SStorageOffsets* const offsets, core::smart_refctd_ptr<IGPUDescriptorSet>* output) = 0;
@@ -169,7 +151,7 @@ class IDescriptorPool : public core::IReferenceCounted, public IBackendObject
169151
bool allocateStorageOffsets(SStorageOffsets& offsets, const IGPUDescriptorSetLayout* layout);
170152
void rewindLastStorageAllocations(const uint32_t count, const SStorageOffsets* offsets, const IGPUDescriptorSetLayout *const *const layouts);
171153

172-
void deleteSetStorage(IGPUDescriptorSet*& set);
154+
void deleteSetStorage(const uint32_t setIndex);
173155

174156
struct allocator_state_t
175157
{
@@ -242,6 +224,8 @@ class IDescriptorPool : public core::IReferenceCounted, public IBackendObject
242224
std::unique_ptr<core::StorageTrivializer<core::smart_refctd_ptr<IGPUBuffer>>[]> m_UBO_SSBOStorage; // ubo | ssbo | ubo dynamic | ssbo dynamic
243225
std::unique_ptr<core::StorageTrivializer<core::smart_refctd_ptr<IGPUBufferView>>[]> m_UTB_STBStorage; // utb | stb
244226
std::unique_ptr<core::StorageTrivializer<core::smart_refctd_ptr<IGPUAccelerationStructure>>[]> m_accelerationStructureStorage;
227+
228+
system::logger_opt_ptr m_logger;
245229
};
246230

247231
}

‎src/nbl/video/IDescriptorPool.cpp

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,33 +4,46 @@
44
namespace nbl::video
55
{
66

7+
IDescriptorPool::IDescriptorPool(core::smart_refctd_ptr<const ILogicalDevice>&& dev, SCreateInfo&& createInfo)
8+
: IBackendObject(std::move(dev)), m_creationParameters(std::move(createInfo)), m_logger(getOriginDevice()->getPhysicalDevice()->getDebugCallback() ? getOriginDevice()->getPhysicalDevice()->getDebugCallback()->getLogger() : nullptr)
9+
{
10+
for (auto i = 0; i < static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT); ++i)
11+
m_descriptorAllocators[i] = std::make_unique<allocator_state_t>(m_creationParameters.maxDescriptorCount[i], m_creationParameters.flags.hasFlags(ECF_FREE_DESCRIPTOR_SET_BIT));
12+
13+
// For mutable samplers. We don't know if there will be mutable samplers in sets allocated by this pool when we create the pool.
14+
m_descriptorAllocators[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT)] = std::make_unique<allocator_state_t>(m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)], m_creationParameters.flags.hasFlags(ECF_FREE_DESCRIPTOR_SET_BIT));
15+
16+
// Initialize the storages.
17+
m_textureStorage = std::make_unique<core::StorageTrivializer<core::smart_refctd_ptr<video::IGPUImageView>>[]>(m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)]);
18+
m_mutableSamplerStorage = std::make_unique<core::StorageTrivializer<core::smart_refctd_ptr<video::IGPUSampler>>[]>(m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)]);
19+
m_storageImageStorage = std::make_unique<core::StorageTrivializer<core::smart_refctd_ptr<IGPUImageView>>[]>(m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_STORAGE_IMAGE)] + m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_INPUT_ATTACHMENT)]);
20+
m_UBO_SSBOStorage = std::make_unique<core::StorageTrivializer<core::smart_refctd_ptr<IGPUBuffer>>[]>(m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_UNIFORM_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_STORAGE_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_UNIFORM_BUFFER_DYNAMIC)] + m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_STORAGE_BUFFER_DYNAMIC)]);
21+
m_UTB_STBStorage = std::make_unique<core::StorageTrivializer<core::smart_refctd_ptr<IGPUBufferView>>[]>(m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_UNIFORM_TEXEL_BUFFER)] + m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_STORAGE_BUFFER_DYNAMIC)]);
22+
m_accelerationStructureStorage = std::make_unique<core::StorageTrivializer<core::smart_refctd_ptr<IGPUAccelerationStructure>>[]>(m_creationParameters.maxDescriptorCount[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_ACCELERATION_STRUCTURE)]);
23+
24+
m_allocatedDescriptorSets = std::make_unique<IGPUDescriptorSet * []>(m_creationParameters.maxSets);
25+
std::fill_n(m_allocatedDescriptorSets.get(), m_creationParameters.maxSets, nullptr);
26+
27+
m_descriptorSetAllocatorReservedSpace = std::make_unique<uint8_t[]>(core::IteratablePoolAddressAllocator<uint32_t>::reserved_size(1, m_creationParameters.maxSets, 1));
28+
m_descriptorSetAllocator = core::IteratablePoolAddressAllocator<uint32_t>(m_descriptorSetAllocatorReservedSpace.get(), 0, 0, 1, m_creationParameters.maxSets, 1);
29+
}
30+
731
uint32_t IDescriptorPool::createDescriptorSets(uint32_t count, const IGPUDescriptorSetLayout* const* layouts, core::smart_refctd_ptr<IGPUDescriptorSet>* output)
832
{
933
core::vector<SStorageOffsets> offsets;
1034
offsets.reserve(count);
1135

12-
system::ILogger* logger = nullptr;
13-
{
14-
auto debugCallback = getOriginDevice()->getPhysicalDevice()->getDebugCallback();
15-
if (debugCallback)
16-
logger = debugCallback->getLogger();
17-
}
18-
1936
for (uint32_t i = 0u; i < count; ++i)
2037
{
2138
if (!isCompatibleDevicewise(layouts[i]))
2239
{
23-
if (logger)
24-
logger->log("Device-Incompatible descriptor set layout found at index %u. Sets for the layouts following this index will not be created.", system::ILogger::ELL_ERROR, i);
25-
40+
m_logger.log("Device-Incompatible descriptor set layout found at index %u. Sets for the layouts following this index will not be created.", system::ILogger::ELL_ERROR, i);
2641
break;
2742
}
2843

2944
if (!allocateStorageOffsets(offsets.emplace_back(), layouts[i]))
3045
{
31-
if (logger)
32-
logger->log("Failed to allocate descriptor or descriptor set offsets in the pool's storage for descriptor set layout at index %u. Sets for the layouts following this index will not be created.", system::ILogger::ELL_WARNING, i);
33-
46+
m_logger.log("Failed to allocate descriptor or descriptor set offsets in the pool's storage for descriptor set layout at index %u. Sets for the layouts following this index will not be created.", system::ILogger::ELL_WARNING, i);
3447
offsets.pop_back();
3548
break;
3649
}
@@ -61,7 +74,7 @@ bool IDescriptorPool::reset()
6174
{
6275
const auto& compilerIsRetarded = m_descriptorSetAllocator;
6376
for (const uint32_t setIndex : compilerIsRetarded)
64-
deleteSetStorage(m_allocatedDescriptorSets[setIndex]);
77+
deleteSetStorage(setIndex);
6578

6679
m_descriptorSetAllocator.reset();
6780

@@ -143,8 +156,10 @@ void IDescriptorPool::rewindLastStorageAllocations(const uint32_t count, const S
143156
}
144157
}
145158

146-
void IDescriptorPool::deleteSetStorage(IGPUDescriptorSet*& set)
159+
void IDescriptorPool::deleteSetStorage(const uint32_t setIndex)
147160
{
161+
auto* set = m_allocatedDescriptorSets[setIndex];
162+
148163
assert(set);
149164
assert(!set->isZombie());
150165

@@ -179,11 +194,13 @@ void IDescriptorPool::deleteSetStorage(IGPUDescriptorSet*& set)
179194
}
180195

181196
m_descriptorSetAllocator.free_addr(set->m_storageOffsets.getSetOffset(), 1);
182-
if (m_descriptorSetAllocator.get_allocated_size() == 0)
197+
if ((m_descriptorSetAllocator.get_allocated_size() == 0) && !allowsFreeing())
183198
reset();
184-
199+
200+
// Order is important because we don't want first nullify the pool (which will destroy the pool for the last surviving DS) because ~IDescriptorPool
201+
// checks if all the allocated DS have been set to nullptr.
202+
m_allocatedDescriptorSets[setIndex] = nullptr;
185203
set->m_pool = nullptr;
186-
set = nullptr;
187204
}
188205

189206
}

‎src/nbl/video/IGPUDescriptorSet.cpp

Lines changed: 7 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -29,28 +29,17 @@ IGPUDescriptorSet::IGPUDescriptorSet(core::smart_refctd_ptr<const IGPUDescriptor
2929
IGPUDescriptorSet::~IGPUDescriptorSet()
3030
{
3131
if (!isZombie())
32-
{
33-
auto dummy = this;
34-
m_pool->deleteSetStorage(dummy);
35-
}
32+
m_pool->deleteSetStorage(m_storageOffsets.getSetOffset());
3633
}
3734

3835
bool IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write)
3936
{
4037
assert(write.dstSet == this);
4138

42-
system::ILogger* logger = nullptr;
43-
{
44-
auto debugCallback = getOriginDevice()->getPhysicalDevice()->getDebugCallback();
45-
if (debugCallback)
46-
logger = debugCallback->getLogger();
47-
}
48-
4939
auto* descriptors = getDescriptors(write.descriptorType, write.binding);
5040
if (!descriptors)
5141
{
52-
if (logger)
53-
logger->log("Descriptor set layout doesn't allow descriptor of such type at binding %u.", system::ILogger::ELL_ERROR, write.binding);
42+
m_pool->m_logger.log("Descriptor set layout doesn't allow descriptor of such type at binding %u.", system::ILogger::ELL_ERROR, write.binding);
5443
return false;
5544
}
5645

@@ -61,8 +50,7 @@ bool IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSe
6150
mutableSamplers = getMutableSamplers(write.binding);
6251
if (!mutableSamplers)
6352
{
64-
if (logger)
65-
logger->log("Descriptor set layout doesn't allow mutable samplers at binding %u.", system::ILogger::ELL_ERROR, write.binding);
53+
m_pool->m_logger.log("Descriptor set layout doesn't allow mutable samplers at binding %u.", system::ILogger::ELL_ERROR, write.binding);
6654
return false;
6755
}
6856
}
@@ -84,46 +72,35 @@ bool IGPUDescriptorSet::processCopy(const IGPUDescriptorSet::SCopyDescriptorSet&
8472
{
8573
assert(copy.dstSet == this);
8674

87-
system::ILogger* logger = nullptr;
88-
{
89-
auto debugCallback = getOriginDevice()->getPhysicalDevice()->getDebugCallback();
90-
if (debugCallback)
91-
logger = debugCallback->getLogger();
92-
}
93-
9475
for (uint32_t t = 0; t < static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT); ++t)
9576
{
9677
const auto type = static_cast<asset::IDescriptor::E_TYPE>(t);
9778

9879
auto* srcDescriptors = copy.srcSet->getDescriptors(type, copy.srcBinding);
9980
if (!srcDescriptors)
10081
{
101-
if (logger)
102-
logger->log("Expected descriptors of given type at binding %u for the src descriptor set but none found.", system::ILogger::ELL_ERROR, copy.srcBinding);
82+
m_pool->m_logger.log("Expected descriptors of given type at binding %u for the src descriptor set but none found.", system::ILogger::ELL_ERROR, copy.srcBinding);
10383
return false;
10484
}
10585

10686
auto* srcSamplers = copy.srcSet->getMutableSamplers(copy.srcBinding);
10787
if (!srcSamplers)
10888
{
109-
if (logger)
110-
logger->log("Expected mutable samplers at binding %u for the src descriptor set, but none found", system::ILogger::ELL_ERROR, copy.srcBinding);
89+
m_pool->m_logger.log("Expected mutable samplers at binding %u for the src descriptor set, but none found", system::ILogger::ELL_ERROR, copy.srcBinding);
11190
return false;
11291
}
11392

11493
auto* dstDescriptors = copy.dstSet->getDescriptors(type, copy.dstBinding);
11594
if (!dstDescriptors)
11695
{
117-
if (logger)
118-
logger->log("Expected descriptors of given type at binding %u for the dst descriptor set but none found.", system::ILogger::ELL_ERROR, copy.dstBinding);
96+
m_pool->m_logger.log("Expected descriptors of given type at binding %u for the dst descriptor set but none found.", system::ILogger::ELL_ERROR, copy.dstBinding);
11997
return false;
12098
}
12199

122100
auto* dstSamplers = copy.dstSet->getMutableSamplers(copy.dstBinding);
123101
if (!dstSamplers)
124102
{
125-
if (logger)
126-
logger->log("Expected mutable samplers at binding %u for the dst descriptor set, but none found", system::ILogger::ELL_ERROR, copy.dstBinding);
103+
m_pool->m_logger.log("Expected mutable samplers at binding %u for the dst descriptor set, but none found", system::ILogger::ELL_ERROR, copy.dstBinding);
127104
return false;
128105
}
129106

0 commit comments

Comments
 (0)