Skip to content

Commit f132807

Browse files
achalpandeyyachalpandeyy
committed
Move back allocateDescriptors from IGPUDescriptorSet to IDescriptorPool and rename to allocateDescriptorOffsets.
1 parent 290fde1 commit f132807

File tree

9 files changed

+73
-75
lines changed

9 files changed

+73
-75
lines changed

‎include/nbl/asset/ICPUDescriptorSet.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,6 @@ class NBL_API ICPUDescriptorSet final : public IDescriptorSet<ICPUDescriptorSetL
123123
return category;
124124
}
125125

126-
// TODO(achal): Remove.
127-
void allocateDescriptors() override { assert(!"Invalid code path."); }
128-
129126
core::smart_refctd_dynamic_array<ICPUDescriptorSet::SDescriptorInfo> m_descriptorInfos[static_cast<uint32_t>(IDescriptor::E_TYPE::ET_COUNT)];
130127
};
131128

‎include/nbl/asset/IDescriptorSet.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,6 @@ class NBL_API IDescriptorSet : public virtual core::IReferenceCounted
166166
IDescriptorSet(core::smart_refctd_ptr<layout_t>&& _layout) : m_layout(std::move(_layout)) {}
167167
virtual ~IDescriptorSet() {}
168168

169-
virtual void allocateDescriptors() = 0;
170-
171169
core::smart_refctd_ptr<layout_t> m_layout;
172170
};
173171

‎include/nbl/video/IDescriptorPool.h

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
#include "nbl/asset/IDescriptorSetLayout.h"
99

1010
#include "nbl/video/decl/IBackendObject.h"
11-
#include "nbl/video/IGPUDescriptorSet.h"
1211

1312

1413
namespace nbl::video
@@ -17,6 +16,7 @@ namespace nbl::video
1716
class IGPUImageView;
1817
class IGPUSampler;
1918
class IGPUBufferView;
19+
class IGPUDescriptorSet;
2020
class IGPUDescriptorSetLayout;
2121

2222
class NBL_API IDescriptorPool : public core::IReferenceCounted, public IBackendObject
@@ -38,6 +38,18 @@ class NBL_API IDescriptorPool : public core::IReferenceCounted, public IBackendO
3838
uint32_t count;
3939
};
4040

41+
struct SDescriptorOffsets
42+
{
43+
SDescriptorOffsets()
44+
{
45+
// The default constructor should initiailze all the offsets to an invalid value (~0u) because ~IGPUDescriptorSet relies on it to
46+
// know which descriptors are present in the set and hence should be destroyed.
47+
std::fill_n(data, static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT) + 1, ~0u);
48+
}
49+
50+
uint32_t data[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT) + 1];
51+
};
52+
4153
inline void createDescriptorSets(uint32_t count, const IGPUDescriptorSetLayout* const* layouts, core::smart_refctd_ptr<IGPUDescriptorSet>* output)
4254
{
4355
for (uint32_t i = 0u; i < count; ++i)
@@ -46,6 +58,10 @@ class NBL_API IDescriptorPool : public core::IReferenceCounted, public IBackendO
4658
output[i++] = createDescriptorSet(std::move(layout));
4759
}
4860
}
61+
62+
// Returns the offset into the pool's descriptor storage. These offsets will be combined
63+
// later with base memory addresses to get the actual memory address where we put the core::smart_refctd_ptr<const IDescriptor>.
64+
SDescriptorOffsets allocateDescriptorOffsets(const IGPUDescriptorSetLayout* layout);
4965

5066
core::smart_refctd_ptr<IGPUDescriptorSet> createDescriptorSet(core::smart_refctd_ptr<const IGPUDescriptorSetLayout>&& layout);
5167
bool freeDescriptorSets(const uint32_t descriptorSetCount, IGPUDescriptorSet* const* const descriptorSets);
@@ -98,7 +114,7 @@ class NBL_API IDescriptorPool : public core::IReferenceCounted, public IBackendO
98114

99115
virtual ~IDescriptorPool() {}
100116

101-
virtual core::smart_refctd_ptr<IGPUDescriptorSet> createDescriptorSet_impl(core::smart_refctd_ptr<const IGPUDescriptorSetLayout>&& layout) = 0;
117+
virtual core::smart_refctd_ptr<IGPUDescriptorSet> createDescriptorSet_impl(core::smart_refctd_ptr<const IGPUDescriptorSetLayout>&& layout, SDescriptorOffsets&& offsets) = 0;
102118
virtual bool freeDescriptorSets_impl(const uint32_t descriptorSetCount, IGPUDescriptorSet* const* const descriptorSets) = 0;
103119

104120
uint32_t m_maxSets;

‎include/nbl/video/IGPUDescriptorSet.h

Lines changed: 4 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,11 @@
1414
#include "nbl/video/IGPUSampler.h"
1515
#include "nbl/video/IGPUDescriptorSetLayout.h"
1616

17+
#include "nbl/video/IDescriptorPool.h"
1718

1819
namespace nbl::video
1920
{
2021

21-
class IDescriptorPool;
22-
2322
//! GPU Version of Descriptor Set
2423
/*
2524
@see IDescriptorSet
@@ -30,28 +29,7 @@ class NBL_API IGPUDescriptorSet : public asset::IDescriptorSet<const IGPUDescrip
3029
using base_t = asset::IDescriptorSet<const IGPUDescriptorSetLayout>;
3130

3231
public:
33-
IGPUDescriptorSet(core::smart_refctd_ptr<const ILogicalDevice>&& dev, core::smart_refctd_ptr<const IGPUDescriptorSetLayout>&& _layout, core::smart_refctd_ptr<IDescriptorPool>&& pool)
34-
: base_t(std::move(_layout)), IBackendObject(std::move(dev)), m_version(0ull), m_pool(std::move(pool))
35-
{
36-
allocateDescriptors();
37-
38-
for (auto i = 0u; i < static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT); ++i)
39-
{
40-
// There is no descriptor of such type in the set.
41-
if (m_descriptorStorageOffsets.data[i] == ~0u)
42-
continue;
43-
44-
const auto type = static_cast<asset::IDescriptor::E_TYPE>(i);
45-
46-
// Default-construct the core::smart_refctd_ptr<IDescriptor>s because even if the user didn't update the descriptor set with ILogicalDevice::updateDescriptorSet we
47-
// won't have uninitialized memory and destruction wouldn't crash in ~IGPUDescriptorSet.
48-
std::uninitialized_default_construct_n(getDescriptorStorage(type) + m_descriptorStorageOffsets.data[i], m_layout->getTotalDescriptorCount(type));
49-
}
50-
51-
const auto mutableSamplerCount = m_layout->getTotalMutableSamplerCount();
52-
if (mutableSamplerCount > 0)
53-
std::uninitialized_default_construct_n(getMutableSamplerStorage() + m_descriptorStorageOffsets.data[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT)], mutableSamplerCount);
54-
}
32+
IGPUDescriptorSet(core::smart_refctd_ptr<const IGPUDescriptorSetLayout>&& _layout, core::smart_refctd_ptr<IDescriptorPool>&& pool, IDescriptorPool::SDescriptorOffsets&& offsets);
5533

5634
inline uint64_t getVersion() const { return m_version.load(); }
5735

@@ -134,27 +112,12 @@ class NBL_API IGPUDescriptorSet : public asset::IDescriptorSet<const IGPUDescrip
134112
}
135113

136114
private:
137-
struct SDescriptorOffsets
138-
{
139-
SDescriptorOffsets()
140-
{
141-
// The default constructor should initiailze all the offsets to an invalid value (~0u) because ~IGPUDescriptorSet relies on it to
142-
// know which descriptors are present in the set and hence should be destroyed.
143-
std::fill_n(data, static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT) + 1, ~0u);
144-
}
145-
146-
uint32_t data[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT) + 1];
147-
};
148-
149-
// Returns the offset into the pool's descriptor storage. These offsets will be combined
150-
// later with base memory addresses to get the actual memory address where we put the core::smart_refctd_ptr<const IDescriptor>.
151-
void allocateDescriptors() override;
152115
core::smart_refctd_ptr<asset::IDescriptor>* getDescriptorStorage(const asset::IDescriptor::E_TYPE type) const;
153116
core::smart_refctd_ptr<IGPUSampler>* getMutableSamplerStorage() const;
154117

155118
std::atomic_uint64_t m_version;
156-
core::smart_refctd_ptr<IDescriptorPool> m_pool;
157-
SDescriptorOffsets m_descriptorStorageOffsets;
119+
core::smart_refctd_ptr<IDescriptorPool> m_pool;
120+
IDescriptorPool::SDescriptorOffsets m_descriptorStorageOffsets;
158121
};
159122

160123
}

‎src/nbl/video/CVulkanDescriptorPool.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ void CVulkanDescriptorPool::setObjectDebugName(const char* label) const
2626
vkSetDebugUtilsObjectNameEXT(vulkanDevice->getInternalObject(), &nameInfo);
2727
}
2828

29-
core::smart_refctd_ptr<IGPUDescriptorSet> CVulkanDescriptorPool::createDescriptorSet_impl(core::smart_refctd_ptr<const IGPUDescriptorSetLayout>&& layout)
29+
core::smart_refctd_ptr<IGPUDescriptorSet> CVulkanDescriptorPool::createDescriptorSet_impl(core::smart_refctd_ptr<const IGPUDescriptorSetLayout>&& layout, SDescriptorOffsets&& offsets)
3030
{
3131
if (layout->getAPIType() != EAT_VULKAN)
3232
return nullptr;
@@ -45,7 +45,7 @@ core::smart_refctd_ptr<IGPUDescriptorSet> CVulkanDescriptorPool::createDescripto
4545
const auto* vulkanDevice = static_cast<const CVulkanLogicalDevice*>(getOriginDevice());
4646
auto* vk = vulkanDevice->getFunctionTable();
4747
if (vk->vk.vkAllocateDescriptorSets(vulkanDevice->getInternalObject(), &vk_allocateInfo, &vk_descriptorSet) == VK_SUCCESS)
48-
return core::make_smart_refctd_ptr<CVulkanDescriptorSet>(core::smart_refctd_ptr<const CVulkanLogicalDevice>(vulkanDevice), std::move(layout), core::smart_refctd_ptr<IDescriptorPool>(this), vk_descriptorSet);
48+
return core::make_smart_refctd_ptr<CVulkanDescriptorSet>(std::move(layout), core::smart_refctd_ptr<IDescriptorPool>(this), std::move(offsets), vk_descriptorSet);
4949

5050
return nullptr;
5151
}

‎src/nbl/video/CVulkanDescriptorPool.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class CVulkanDescriptorPool : public IDescriptorPool
2323
void setObjectDebugName(const char* label) const override;
2424

2525
private:
26-
core::smart_refctd_ptr<IGPUDescriptorSet> createDescriptorSet_impl(core::smart_refctd_ptr<const IGPUDescriptorSetLayout>&& layout) override;
26+
core::smart_refctd_ptr<IGPUDescriptorSet> createDescriptorSet_impl(core::smart_refctd_ptr<const IGPUDescriptorSetLayout>&& layout, SDescriptorOffsets&& offsets) override;
2727
bool freeDescriptorSets_impl(const uint32_t descriptorSetCount, IGPUDescriptorSet* const* const descriptorSets) final override;
2828

2929
VkDescriptorPool m_descriptorPool;

‎src/nbl/video/CVulkanDescriptorSet.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,8 @@ class CVulkanDescriptorPool;
1313
class CVulkanDescriptorSet : public IGPUDescriptorSet
1414
{
1515
public:
16-
CVulkanDescriptorSet(core::smart_refctd_ptr<const ILogicalDevice>&& dev, core::smart_refctd_ptr<const IGPUDescriptorSetLayout>&& layout,
17-
core::smart_refctd_ptr<IDescriptorPool>&& pool, VkDescriptorSet descriptorSet)
18-
: IGPUDescriptorSet(std::move(dev), std::move(layout), std::move(pool)), m_descriptorSet(descriptorSet)
16+
CVulkanDescriptorSet(core::smart_refctd_ptr<const IGPUDescriptorSetLayout>&& layout, core::smart_refctd_ptr<IDescriptorPool>&& pool, IDescriptorPool::SDescriptorOffsets offsets, VkDescriptorSet descriptorSet)
17+
: IGPUDescriptorSet(std::move(layout), std::move(pool), std::move(offsets)), m_descriptorSet(descriptorSet)
1918
{}
2019

2120
inline VkDescriptorSet getInternalObject() const { return m_descriptorSet; }

‎src/nbl/video/IDescriptorPool.cpp

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,40 @@ core::smart_refctd_ptr<IGPUDescriptorSet> IDescriptorPool::createDescriptorSet(c
99
if (!isCompatibleDevicewise(layout.get()))
1010
return nullptr;
1111

12-
return createDescriptorSet_impl(std::move(layout));
12+
auto offsets = allocateDescriptorOffsets(layout.get());
13+
14+
return createDescriptorSet_impl(std::move(layout), std::move(offsets));
15+
}
16+
17+
IDescriptorPool::SDescriptorOffsets IDescriptorPool::allocateDescriptorOffsets(const IGPUDescriptorSetLayout* layout)
18+
{
19+
SDescriptorOffsets offsets;
20+
21+
for (uint32_t i = 0u; i < static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT); ++i)
22+
{
23+
const auto type = static_cast<asset::IDescriptor::E_TYPE>(i);
24+
const auto count = layout->getTotalDescriptorCount(type);
25+
if (count == 0ull)
26+
continue;
27+
28+
if (m_flags & IDescriptorPool::ECF_FREE_DESCRIPTOR_SET_BIT)
29+
offsets.data[i] = m_generalAllocators[i].alloc_addr(count, 1u);
30+
else
31+
offsets.data[i] = m_linearAllocators[i].alloc_addr(count, 1u);
32+
33+
assert((offsets.data[i] < m_maxDescriptorCount[i]) && "PANIC: Allocation failed. This shoudn't have happened! Check your descriptor pool.");
34+
}
35+
36+
const auto mutableSamplerCount = layout->getTotalMutableSamplerCount();
37+
if (mutableSamplerCount != 0ull)
38+
{
39+
if (m_flags & IDescriptorPool::ECF_FREE_DESCRIPTOR_SET_BIT)
40+
offsets.data[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT)] = m_generalAllocators[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT)].alloc_addr(mutableSamplerCount, 1u);
41+
else
42+
offsets.data[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT)] = m_linearAllocators[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT)].alloc_addr(mutableSamplerCount, 1u);
43+
}
44+
45+
return offsets;
1346
}
1447

1548
bool IDescriptorPool::freeDescriptorSets(const uint32_t descriptorSetCount, IGPUDescriptorSet* const* const descriptorSets)

‎src/nbl/video/IGPUDescriptorSet.cpp

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,33 +5,25 @@
55
namespace nbl::video
66
{
77

8-
void IGPUDescriptorSet::allocateDescriptors()
8+
IGPUDescriptorSet::IGPUDescriptorSet(core::smart_refctd_ptr<const IGPUDescriptorSetLayout>&& layout, core::smart_refctd_ptr<IDescriptorPool>&& pool, IDescriptorPool::SDescriptorOffsets&& offsets)
9+
: base_t(std::move(layout)), IBackendObject(std::move(core::smart_refctd_ptr<const ILogicalDevice>(pool->getOriginDevice()))), m_version(0ull), m_pool(std::move(pool)), m_descriptorStorageOffsets(std::move(offsets))
910
{
10-
auto& offsets = m_descriptorStorageOffsets;
11-
12-
for (uint32_t i = 0u; i < static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT); ++i)
11+
for (auto i = 0u; i < static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT); ++i)
1312
{
14-
const auto type = static_cast<asset::IDescriptor::E_TYPE>(i);
15-
const auto count = getLayout()->getTotalDescriptorCount(type);
16-
if (count == 0ull)
13+
// There is no descriptor of such type in the set.
14+
if (m_descriptorStorageOffsets.data[i] == ~0u)
1715
continue;
1816

19-
if (m_pool->m_flags & IDescriptorPool::ECF_FREE_DESCRIPTOR_SET_BIT)
20-
offsets.data[i] = m_pool->m_generalAllocators[i].alloc_addr(count, 1u);
21-
else
22-
offsets.data[i] = m_pool->m_linearAllocators[i].alloc_addr(count, 1u);
17+
const auto type = static_cast<asset::IDescriptor::E_TYPE>(i);
2318

24-
assert((offsets.data[i] < m_pool->m_maxDescriptorCount[i]) && "PANIC: Allocation failed. This shoudn't have happened! Check your descriptor pool.");
19+
// Default-construct the core::smart_refctd_ptr<IDescriptor>s because even if the user didn't update the descriptor set with ILogicalDevice::updateDescriptorSet we
20+
// won't have uninitialized memory and destruction wouldn't crash in ~IGPUDescriptorSet.
21+
std::uninitialized_default_construct_n(getDescriptorStorage(type) + m_descriptorStorageOffsets.data[i], m_layout->getTotalDescriptorCount(type));
2522
}
2623

27-
const auto mutableSamplerCount = getLayout()->getTotalMutableSamplerCount();
28-
if (mutableSamplerCount != 0ull)
29-
{
30-
if (m_pool->m_flags & IDescriptorPool::ECF_FREE_DESCRIPTOR_SET_BIT)
31-
offsets.data[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT)] = m_pool->m_generalAllocators[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT)].alloc_addr(mutableSamplerCount, 1u);
32-
else
33-
offsets.data[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT)] = m_pool->m_linearAllocators[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT)].alloc_addr(mutableSamplerCount, 1u);
34-
}
24+
const auto mutableSamplerCount = m_layout->getTotalMutableSamplerCount();
25+
if (mutableSamplerCount > 0)
26+
std::uninitialized_default_construct_n(getMutableSamplerStorage() + m_descriptorStorageOffsets.data[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT)], mutableSamplerCount);
3527
}
3628

3729
core::smart_refctd_ptr<asset::IDescriptor>* IGPUDescriptorSet::getDescriptorStorage(const asset::IDescriptor::E_TYPE type) const

0 commit comments

Comments
 (0)