Skip to content

Commit d65ffd5

Browse files
achalpandeyyachalpandeyy
committed
Some fixes and cleanups.
1 parent b704f27 commit d65ffd5

File tree

9 files changed

+39
-41
lines changed

9 files changed

+39
-41
lines changed

‎include/nbl/asset/IDescriptorSetLayout.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,25 +107,25 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted
107107
return { foundIndex };
108108
}
109109

110-
inline binding_number_t getBindingFromStorageIndex(const storage_range_index_t index) const
110+
inline binding_number_t getBinding(const storage_range_index_t index) const
111111
{
112112
assert(index.data < m_count);
113113
return m_bindingNumbers[index.data];
114114
}
115115

116-
inline core::bitflag<IShader::E_SHADER_STAGE> getStageFlagsFromStorageIndex(const storage_range_index_t index) const
116+
inline core::bitflag<IShader::E_SHADER_STAGE> getStageFlags(const storage_range_index_t index) const
117117
{
118118
assert(index.data < m_count);
119119
return m_stageFlags[index.data];
120120
}
121121

122-
inline uint32_t getCountFromStorageIndex(const storage_range_index_t index) const
122+
inline uint32_t getCount(const storage_range_index_t index) const
123123
{
124124
assert(index.data < m_count);
125125
return (index.data == 0u) ? m_storageOffsets[index.data].data : m_storageOffsets[index.data].data - m_storageOffsets[index.data - 1].data;
126126
}
127127

128-
inline storage_offset_t getStorageOffsetFromStorageIndex(const storage_range_index_t index) const
128+
inline storage_offset_t getStorageOffset(const storage_range_index_t index) const
129129
{
130130
assert(index.data < m_count);
131131
return (index.data == 0u) ? 0u : m_storageOffsets[index.data - 1];
@@ -158,7 +158,7 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted
158158
if (index.data == Invalid)
159159
return { Invalid };
160160

161-
return getStorageOffsetFromStorageIndex(index);
161+
return getStorageOffset(index);
162162
}
163163

164164
inline uint32_t getTotalCount() const { return (m_count == 0ull) ? 0u : m_storageOffsets[m_count - 1].data; }

‎include/nbl/video/IGPUDescriptorSet.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,9 @@ class IGPUDescriptorSet : public asset::IDescriptorSet<const IGPUDescriptorSetLa
6464
inline void incrementVersion() { m_version.fetch_add(1ull); }
6565

6666
friend class ILogicalDevice;
67-
bool validateWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write);
67+
bool validateWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write) const;
6868
void processWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write);
69-
bool validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy);
69+
bool validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy) const;
7070
void processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy);
7171

7272
// This assumes that descriptors of a particular type in the set will always be contiguous in pool's storage memory, regardless of which binding in the set they belong to.

‎include/nbl/video/utilities/CComputeBlit.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -484,9 +484,9 @@ class CComputeBlit : public core::IReferenceCounted
484484
{
485485
auto& write = writes[writeCount++];
486486
write.dstSet = ds;
487-
write.binding = redirect.getBindingFromStorageIndex(i).data;
487+
write.binding = redirect.getBinding(IGPUDescriptorSetLayout::CBindingRedirect::storage_range_index_t{ i }).data;
488488
write.arrayElement = 0u;
489-
write.count = redirect.getCountFromStorageIndex(i);
489+
write.count = redirect.getCount(IGPUDescriptorSetLayout::CBindingRedirect::storage_range_index_t{ i });
490490
write.info = &infos[i];
491491
write.descriptorType = type;
492492
}

‎include/nbl/video/utilities/IGPUObjectFromAssetConverter.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1382,10 +1382,10 @@ auto IGPUObjectFromAssetConverter::create(const asset::ICPUDescriptorSetLayout**
13821382
for (uint32_t b = 0; b < declaredBindingCount; ++b)
13831383
{
13841384
auto& gpuBinding = tmpBindings->begin()[gpuBindingCount++];
1385-
gpuBinding.binding = descriptorBindingRedirect.getBindingFromStorageIndex(b).data;
1385+
gpuBinding.binding = descriptorBindingRedirect.getBinding(b).data;
13861386
gpuBinding.type = type;
1387-
gpuBinding.count = descriptorBindingRedirect.getCountFromStorageIndex(b);
1388-
gpuBinding.stageFlags = descriptorBindingRedirect.getStageFlagsFromStorageIndex(b);
1387+
gpuBinding.count = descriptorBindingRedirect.getCount(asset::ICPUDescriptorSetLayout::CBindingRedirect::storage_range_index_t{ b });
1388+
gpuBinding.stageFlags = descriptorBindingRedirect.getStageFlags(asset::ICPUDescriptorSetLayout::CBindingRedirect::storage_range_index_t{ b });
13891389
gpuBinding.samplers = nullptr;
13901390

13911391
// If this DS layout has any immutable samplers..
@@ -1721,15 +1721,15 @@ inline created_gpu_object_array<asset::ICPUDescriptorSet> IGPUObjectFromAssetCon
17211721
for (uint32_t b = 0u; b < descriptorBindingRedirect.getBindingCount(); ++b)
17221722
{
17231723
write_it->dstSet = gpuds;
1724-
write_it->binding = descriptorBindingRedirect.getBindingFromStorageIndex(b).data;
1724+
write_it->binding = descriptorBindingRedirect.getBinding(asset::ICPUDescriptorSetLayout::CBindingRedirect::storage_range_index_t{ b }).data;
17251725
write_it->arrayElement = 0u;
17261726

1727-
const uint32_t descriptorCount = descriptorBindingRedirect.getCountFromStorageIndex(b);
1727+
const uint32_t descriptorCount = descriptorBindingRedirect.getCount(asset::ICPUDescriptorSetLayout::CBindingRedirect::storage_range_index_t{ b });
17281728
write_it->count = descriptorCount;
17291729
write_it->descriptorType = type;
17301730
write_it->info = &(*info);
17311731

1732-
const uint32_t offset = descriptorBindingRedirect.getStorageOffsetFromStorageIndex(b).data;
1732+
const uint32_t offset = descriptorBindingRedirect.getStorageOffset(asset::ICPUDescriptorSetLayout::CBindingRedirect::storage_range_index_t{ b }).data;
17331733

17341734
// It is better to use getDescriptorInfoStorage over getDescriptorInfos, because the latter does a binary search
17351735
// over the bindings, which is not really required given we have the index of binding number (since we're iterating

‎src/nbl/asset/ICPUDescriptorSet.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ core::SRange<const ICPUDescriptorSet::SDescriptorInfo> ICPUDescriptorSet::getDes
3434
if (bindingNumberIndex.data == redirect.Invalid)
3535
return { nullptr, nullptr };
3636

37-
const auto offset = redirect.getStorageOffsetFromStorageIndex(bindingNumberIndex).data;
38-
const auto count = redirect.getCountFromStorageIndex(bindingNumberIndex);
37+
const auto offset = redirect.getStorageOffset(asset::ICPUDescriptorSetLayout::CBindingRedirect::storage_range_index_t{ bindingNumberIndex }).data;
38+
const auto count = redirect.getCount(asset::ICPUDescriptorSetLayout::CBindingRedirect::storage_range_index_t{ bindingNumberIndex });
3939

4040
auto infosBegin = m_descriptorInfos[static_cast<uint32_t>(type)]->begin() + offset;
4141

‎src/nbl/asset/interchange/IRenderpassIndependentPipelineLoader.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ void IRenderpassIndependentPipelineLoader::initialize()
4848
auto& semantic = (m_basicViewParamsSemantics->end()-i-1u)[0];
4949
semantic.type = types[i];
5050
semantic.descriptorSection.type = IRenderpassIndependentPipelineMetadata::ShaderInput::ET_UNIFORM_BUFFER;
51-
semantic.descriptorSection.uniformBufferObject.binding = ds1layout->getDescriptorRedirect(IDescriptor::E_TYPE::ET_UNIFORM_BUFFER).getBindingFromStorageIndex(0).data;
51+
semantic.descriptorSection.uniformBufferObject.binding = ds1layout->getDescriptorRedirect(IDescriptor::E_TYPE::ET_UNIFORM_BUFFER).getBinding(asset::ICPUDescriptorSetLayout::CBindingRedirect::storage_range_index_t{ 0 }).data;
5252
semantic.descriptorSection.uniformBufferObject.set = 1u;
5353
semantic.descriptorSection.uniformBufferObject.relByteoffset = relOffsets[i];
5454
semantic.descriptorSection.uniformBufferObject.bytesize = sizes[i];

‎src/nbl/video/CVulkanLogicalDevice.h

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ class CVulkanLogicalDevice final : public ILogicalDevice
469469
core::vector<VkWriteDescriptorSet> vk_writeDescriptorSets(descriptorWriteCount);
470470
core::vector<VkWriteDescriptorSetAccelerationStructureKHR> vk_writeDescriptorSetAS(descriptorWriteCount);
471471

472-
core::vector<VkDescriptorBufferInfo>vk_bufferInfos;
472+
core::vector<VkDescriptorBufferInfo> vk_bufferInfos;
473473
core::vector<VkDescriptorImageInfo> vk_imageInfos;
474474
core::vector<VkBufferView> vk_bufferViews;
475475
core::vector<VkAccelerationStructureKHR> vk_accelerationStructures;
@@ -479,23 +479,6 @@ class CVulkanLogicalDevice final : public ILogicalDevice
479479
vk_writeDescriptorSets[i].sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET;
480480
vk_writeDescriptorSets[i].pNext = nullptr; // Each pNext member of any structure (including this one) in the pNext chain must be either NULL or a pointer to a valid instance of VkWriteDescriptorSetAccelerationStructureKHR, VkWriteDescriptorSetAccelerationStructureNV, or VkWriteDescriptorSetInlineUniformBlockEXT
481481

482-
const IGPUDescriptorSetLayout* layout = pDescriptorWrites[i].dstSet->getLayout();
483-
if (layout->getAPIType() != EAT_VULKAN)
484-
{
485-
auto logger = (m_physicalDevice->getDebugCallback()) ? m_physicalDevice->getDebugCallback()->getLogger() : nullptr;
486-
if (logger)
487-
{
488-
const char* dsDebugName = pDescriptorWrites[i].dstSet->getDebugName();
489-
const char* dsLayoutDebugName = pDescriptorWrites[i].dstSet->getDebugName();
490-
if (dsDebugName && dsLayoutDebugName)
491-
logger->log("Descriptor set layout (%s, %p) of the descriptor set (%s, %p) uses a different backend than the logical device it was created with. Skipping the descriptor set..", system::ILogger::ELL_ERROR, dsLayoutDebugName, layout, dsDebugName, pDescriptorWrites[i].dstSet);
492-
else
493-
logger->log("Descriptor set layout (%p) of the descriptor set (%p) uses a different backend than the logical device it was created with. Skipping the descriptor set..", system::ILogger::ELL_ERROR, layout, pDescriptorWrites[i].dstSet);
494-
495-
}
496-
continue;
497-
}
498-
499482
const CVulkanDescriptorSet* vulkanDescriptorSet = static_cast<const CVulkanDescriptorSet*>(pDescriptorWrites[i].dstSet);
500483
vk_writeDescriptorSets[i].dstSet = vulkanDescriptorSet->getInternalObject();
501484

‎src/nbl/video/IGPUDescriptorSet.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,18 @@ IGPUDescriptorSet::IGPUDescriptorSet(core::smart_refctd_ptr<const IGPUDescriptor
1818

1919
// Default-construct the core::smart_refctd_ptr<IDescriptor>s because even if the user didn't update the descriptor set with ILogicalDevice::updateDescriptorSet we
2020
// won't have uninitialized memory and destruction wouldn't crash in ~IGPUDescriptorSet.
21-
std::uninitialized_default_construct_n(m_pool->getDescriptorStorage(type) + m_storageOffsets.data[i], m_layout->getTotalDescriptorCount(type));
21+
auto descriptors = getAllDescriptors(type);
22+
assert(descriptors);
23+
std::uninitialized_default_construct_n(descriptors, m_layout->getTotalDescriptorCount(type));
2224
}
2325

2426
const auto mutableSamplerCount = m_layout->getTotalMutableSamplerCount();
2527
if (mutableSamplerCount > 0)
26-
std::uninitialized_default_construct_n(m_pool->getMutableSamplerStorage() + m_storageOffsets.data[static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT)], mutableSamplerCount);
28+
{
29+
auto mutableSamplers = getAllMutableSamplers();
30+
assert(mutableSamplers);
31+
std::uninitialized_default_construct_n(mutableSamplers, mutableSamplerCount);
32+
}
2733
}
2834

2935
IGPUDescriptorSet::~IGPUDescriptorSet()
@@ -32,7 +38,7 @@ IGPUDescriptorSet::~IGPUDescriptorSet()
3238
m_pool->deleteSetStorage(m_storageOffsets.getSetOffset());
3339
}
3440

35-
bool IGPUDescriptorSet::validateWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write)
41+
bool IGPUDescriptorSet::validateWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write) const
3642
{
3743
assert(write.dstSet == this);
3844

@@ -92,7 +98,7 @@ void IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSe
9298
incrementVersion();
9399
}
94100

95-
bool IGPUDescriptorSet::validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy)
101+
bool IGPUDescriptorSet::validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy) const
96102
{
97103
assert(copy.dstSet == this);
98104

‎src/nbl/video/ILogicalDevice.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,23 @@ bool ILogicalDevice::updateDescriptorSets(uint32_t descriptorWriteCount, const I
3535
{
3636
const auto& write = pDescriptorWrites[i];
3737
auto* ds = static_cast<IGPUDescriptorSet*>(write.dstSet);
38+
39+
if (!ds->getLayout()->isCompatibleDevicewise(ds))
40+
return false;
41+
3842
if (!ds->validateWrite(write))
3943
return false;
4044
}
4145

4246
for (auto i = 0; i < descriptorCopyCount; ++i)
4347
{
4448
const auto& copy = pDescriptorCopies[i];
45-
auto* dstDS = static_cast<IGPUDescriptorSet*>(pDescriptorCopies[i].dstSet);
49+
const auto* srcDS = static_cast<const IGPUDescriptorSet*>(copy.srcSet);
50+
const auto* dstDS = static_cast<IGPUDescriptorSet*>(copy.dstSet);
51+
52+
if ((!srcDS->getLayout()->isCompatibleDevicewise(srcDS)) || (!dstDS->getLayout()->isCompatibleDevicewise(dstDS)))
53+
return false;
54+
4655
if (!dstDS->validateCopy(copy))
4756
return false;
4857
}

0 commit comments

Comments
 (0)