Skip to content

Commit b704f27

Browse files
achalpandeyyachalpandeyy
committed
Correct updateDescriptorSets.
1 parent 3dd82fd commit b704f27

File tree

3 files changed

+87
-33
lines changed

3 files changed

+87
-33
lines changed

‎include/nbl/video/IGPUDescriptorSet.h

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

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

7072
// 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.
7173
inline core::smart_refctd_ptr<asset::IDescriptor>* getDescriptors(const asset::IDescriptor::E_TYPE type, const uint32_t binding) const

‎src/nbl/video/IGPUDescriptorSet.cpp

Lines changed: 64 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -32,29 +32,55 @@ IGPUDescriptorSet::~IGPUDescriptorSet()
3232
m_pool->deleteSetStorage(m_storageOffsets.getSetOffset());
3333
}
3434

35-
bool IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write)
35+
bool IGPUDescriptorSet::validateWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write)
3636
{
3737
assert(write.dstSet == this);
3838

39+
const char* debugName = getDebugName();
40+
3941
auto* descriptors = getDescriptors(write.descriptorType, write.binding);
4042
if (!descriptors)
4143
{
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);
44+
if (debugName)
45+
m_pool->m_logger.log("Descriptor set (%s, %p) doesn't allow descriptor of such type at binding %u.", system::ILogger::ELL_ERROR, debugName, this, write.binding);
46+
else
47+
m_pool->m_logger.log("Descriptor set (%p) doesn't allow descriptor of such type at binding %u.", system::ILogger::ELL_ERROR, this, write.binding);
48+
4349
return false;
4450
}
4551

4652
core::smart_refctd_ptr<video::IGPUSampler>* mutableSamplers = nullptr;
47-
4853
if (write.descriptorType == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER && write.info->info.image.sampler)
4954
{
5055
mutableSamplers = getMutableSamplers(write.binding);
5156
if (!mutableSamplers)
5257
{
53-
m_pool->m_logger.log("Descriptor set layout doesn't allow mutable samplers at binding %u.", system::ILogger::ELL_ERROR, write.binding);
58+
if (debugName)
59+
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);
60+
else
61+
m_pool->m_logger.log("Descriptor set (%p) doesn't allow mutable samplers at binding %u.", system::ILogger::ELL_ERROR, this, write.binding);
62+
5463
return false;
5564
}
5665
}
5766

67+
return true;
68+
}
69+
70+
void IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write)
71+
{
72+
assert(write.dstSet == this);
73+
74+
auto* descriptors = getDescriptors(write.descriptorType, write.binding);
75+
assert(descriptors);
76+
77+
core::smart_refctd_ptr<video::IGPUSampler>* mutableSamplers = nullptr;
78+
if ((write.descriptorType == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) && write.info->info.image.sampler)
79+
{
80+
mutableSamplers = getMutableSamplers(write.binding);
81+
assert(mutableSamplers);
82+
}
83+
5884
for (auto j = 0; j < write.count; ++j)
5985
{
6086
descriptors[j] = write.info[j].desc;
@@ -64,53 +90,63 @@ bool IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSe
6490
}
6591

6692
incrementVersion();
67-
68-
return true;
6993
}
7094

71-
bool IGPUDescriptorSet::processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy)
95+
bool IGPUDescriptorSet::validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy)
7296
{
7397
assert(copy.dstSet == this);
7498

99+
const char* srcDebugName = copy.srcSet->getDebugName();
100+
const char* dstDebugName = copy.dstSet->getDebugName();
101+
75102
for (uint32_t t = 0; t < static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT); ++t)
76103
{
77104
const auto type = static_cast<asset::IDescriptor::E_TYPE>(t);
78105

79106
auto* srcDescriptors = copy.srcSet->getDescriptors(type, copy.srcBinding);
80-
if (!srcDescriptors)
81-
{
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);
83-
return false;
84-
}
107+
auto* dstDescriptors = copy.dstSet->getDescriptors(type, copy.dstBinding);
85108

86109
auto* srcSamplers = copy.srcSet->getMutableSamplers(copy.srcBinding);
87-
if (!srcSamplers)
110+
auto* dstSamplers = copy.dstSet->getMutableSamplers(copy.dstBinding);
111+
112+
if ((!srcDescriptors != !dstDescriptors) || (!srcSamplers != !dstSamplers))
88113
{
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);
114+
if (srcDebugName && dstDebugName)
115+
m_pool->m_logger.log("Incompatible copy from descriptor set (%s, %p) at binding %u to descriptor set (%s, %p) at binding %u.", system::ILogger::ELL_ERROR, srcDebugName, copy.srcSet, copy.srcBinding, dstDebugName, copy.dstSet, copy.dstBinding);
116+
else
117+
m_pool->m_logger.log("Incompatible copy from descriptor set (%p) at binding %u to descriptor set (%p) at binding %u.", system::ILogger::ELL_ERROR, copy.srcSet, copy.srcBinding, copy.dstSet, copy.dstBinding);
118+
90119
return false;
91120
}
121+
}
122+
123+
return true;
124+
}
125+
126+
void IGPUDescriptorSet::processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy)
127+
{
128+
assert(copy.dstSet == this);
129+
130+
for (uint32_t t = 0; t < static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT); ++t)
131+
{
132+
const auto type = static_cast<asset::IDescriptor::E_TYPE>(t);
92133

134+
auto* srcDescriptors = copy.srcSet->getDescriptors(type, copy.srcBinding);
93135
auto* dstDescriptors = copy.dstSet->getDescriptors(type, copy.dstBinding);
94-
if (!dstDescriptors)
95-
{
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);
97-
return false;
98-
}
136+
assert(!(!srcDescriptors != !dstDescriptors));
99137

138+
auto* srcSamplers = copy.srcSet->getMutableSamplers(copy.srcBinding);
100139
auto* dstSamplers = copy.dstSet->getMutableSamplers(copy.dstBinding);
101-
if (!dstSamplers)
102-
{
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);
104-
return false;
105-
}
140+
assert(!(!srcSamplers != !dstSamplers));
106141

107-
std::copy_n(srcDescriptors, copy.count, dstDescriptors);
108-
std::copy_n(srcSamplers, copy.count, dstSamplers);
142+
if (srcDescriptors && dstDescriptors)
143+
std::copy_n(srcDescriptors, copy.count, dstDescriptors);
144+
145+
if (srcSamplers && dstSamplers)
146+
std::copy_n(srcSamplers, copy.count, dstSamplers);
109147
}
110148

111149
incrementVersion();
112-
113-
return true;
114150
}
115151

116152
}

‎src/nbl/video/ILogicalDevice.cpp

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,18 +33,34 @@ bool ILogicalDevice::updateDescriptorSets(uint32_t descriptorWriteCount, const I
3333
{
3434
for (auto i = 0; i < descriptorWriteCount; ++i)
3535
{
36-
auto* ds = static_cast<IGPUDescriptorSet*>(pDescriptorWrites[i].dstSet);
37-
if (!ds->processWrite(pDescriptorWrites[i]))
36+
const auto& write = pDescriptorWrites[i];
37+
auto* ds = static_cast<IGPUDescriptorSet*>(write.dstSet);
38+
if (!ds->validateWrite(write))
3839
return false;
3940
}
4041

4142
for (auto i = 0; i < descriptorCopyCount; ++i)
4243
{
44+
const auto& copy = pDescriptorCopies[i];
4345
auto* dstDS = static_cast<IGPUDescriptorSet*>(pDescriptorCopies[i].dstSet);
44-
if (!dstDS->processCopy(pDescriptorCopies[i]))
46+
if (!dstDS->validateCopy(copy))
4547
return false;
4648
}
4749

50+
for (auto i = 0; i < descriptorWriteCount; ++i)
51+
{
52+
auto& write = pDescriptorWrites[i];
53+
auto* ds = static_cast<IGPUDescriptorSet*>(write.dstSet);
54+
ds->processWrite(write);
55+
}
56+
57+
for (auto i = 0; i < descriptorCopyCount; ++i)
58+
{
59+
const auto& copy = pDescriptorCopies[i];
60+
auto* dstDS = static_cast<IGPUDescriptorSet*>(pDescriptorCopies[i].dstSet);
61+
dstDS->processCopy(copy);
62+
}
63+
4864
updateDescriptorSets_impl(descriptorWriteCount, pDescriptorWrites, descriptorCopyCount, pDescriptorCopies);
4965

5066
return true;

0 commit comments

Comments
 (0)