Skip to content

Commit 357829e

Browse files
FletterioFletterio
authored
Merge pull request #703 from Devsh-Graphics-Programming/separable_image_sampler_amortized_lookups
Separable image sampler amortized lookups
2 parents 8eb33e1 + 7b76bed commit 357829e

File tree

3 files changed

+116
-76
lines changed

3 files changed

+116
-76
lines changed

‎include/nbl/video/IGPUDescriptorSet.h

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,14 +87,33 @@ class IGPUDescriptorSet : public asset::IDescriptorSet<const IGPUDescriptorSetLa
8787
private:
8888
inline void incrementVersion() { m_version.fetch_add(1ull); }
8989

90+
using redirect_t = IGPUDescriptorSetLayout::CBindingRedirect;
9091
friend class ILogicalDevice;
91-
asset::IDescriptor::E_TYPE validateWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write) const;
92-
void processWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write, const asset::IDescriptor::E_TYPE type);
93-
bool validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy) const;
94-
void processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy);
92+
93+
struct SWriteValidationResult
94+
{
95+
asset::IDescriptor::E_TYPE type;
96+
redirect_t::storage_range_index_t descriptorRedirectBindingIndex;
97+
redirect_t::storage_range_index_t mutableSamplerRedirectBindingIndex;
98+
};
99+
100+
SWriteValidationResult validateWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write) const;
101+
void processWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write, const SWriteValidationResult& validationResult);
102+
103+
struct SCopyValidationResult
104+
{
105+
asset::IDescriptor::E_TYPE type;
106+
redirect_t::storage_range_index_t srcDescriptorRedirectBindingIndex;
107+
redirect_t::storage_range_index_t dstDescriptorRedirectBindingIndex;
108+
redirect_t::storage_range_index_t srcMutableSamplerRedirectBindingIndex;
109+
redirect_t::storage_range_index_t dstMutableSamplerRedirectBindingIndex;
110+
};
111+
112+
SCopyValidationResult validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy) const;
113+
void processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy, const SCopyValidationResult& validationResult);
114+
95115
void dropDescriptors(const IGPUDescriptorSet::SDropDescriptorSet& drop);
96116

97-
using redirect_t = IGPUDescriptorSetLayout::CBindingRedirect;
98117
// 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.
99118
inline core::smart_refctd_ptr<asset::IDescriptor>* getDescriptors(const asset::IDescriptor::E_TYPE type, const redirect_t::binding_number_t binding) const
100119
{

‎src/nbl/video/IGPUDescriptorSet.cpp

Lines changed: 72 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -38,28 +38,30 @@ IGPUDescriptorSet::~IGPUDescriptorSet()
3838
m_pool->deleteSetStorage(m_storageOffsets.getSetOffset());
3939
}
4040

41-
asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write) const
41+
IGPUDescriptorSet::SWriteValidationResult IGPUDescriptorSet::validateWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write) const
4242
{
4343
assert(write.dstSet == this);
4444

4545
const char* debugName = getDebugName();
4646
using redirect_t = IGPUDescriptorSetLayout::CBindingRedirect;
4747
const redirect_t::binding_number_t bindingNumber(write.binding);
4848

49+
SWriteValidationResult validationResult;
50+
4951
// screw it, we'll need to replace the descriptor writing with update templates of descriptor buffer soon anyway
50-
redirect_t::storage_range_index_t descriptorRedirectBindingIndex;
51-
const auto descriptorType = getBindingType(bindingNumber, descriptorRedirectBindingIndex);
52+
const auto descriptorType = getBindingType(bindingNumber, validationResult.descriptorRedirectBindingIndex);
5253
if (asset::IDescriptor::E_TYPE::ET_COUNT == descriptorType)
5354
{
5455
if (debugName)
5556
m_pool->m_logger.log("Descriptor set (%s, %p) has no binding %u.", system::ILogger::ELL_ERROR, debugName, this, write.binding);
5657
else
5758
m_pool->m_logger.log("Descriptor set (%p) has no binding %u.", system::ILogger::ELL_ERROR, this, write.binding);
5859

59-
return asset::IDescriptor::E_TYPE::ET_COUNT;
60+
validationResult.type = asset::IDescriptor::E_TYPE::ET_COUNT;
61+
return validationResult;
6062
}
6163

62-
auto* descriptors = getDescriptors(descriptorType, descriptorRedirectBindingIndex);
64+
auto* descriptors = getDescriptors(descriptorType, validationResult.descriptorRedirectBindingIndex);
6365
// Left this check, but if the above passed then this should never be nullptr I believe
6466
if (!descriptors)
6567
{
@@ -68,7 +70,8 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor
6870
else
6971
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);
7072

71-
return asset::IDescriptor::E_TYPE::ET_COUNT;
73+
validationResult.type = asset::IDescriptor::E_TYPE::ET_COUNT;
74+
return validationResult;
7275
}
7376

7477
for (uint32_t i = 0; i < write.count; ++i)
@@ -80,7 +83,8 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor
8083
else
8184
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);
8285

83-
return asset::IDescriptor::E_TYPE::ET_COUNT;
86+
validationResult.type = asset::IDescriptor::E_TYPE::ET_COUNT;
87+
return validationResult;
8488
}
8589
}
8690

@@ -92,7 +96,8 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor
9296
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);
9397
else
9498
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);
95-
return asset::IDescriptor::E_TYPE::ET_COUNT;
99+
validationResult.type = asset::IDescriptor::E_TYPE::ET_COUNT;
100+
return validationResult;
96101
}
97102

98103
for (uint32_t i=0; i<write.count; ++i)
@@ -103,19 +108,23 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor
103108
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);
104109
else
105110
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);
106-
return asset::IDescriptor::E_TYPE::ET_COUNT;
111+
validationResult.type = asset::IDescriptor::E_TYPE::ET_COUNT;
112+
return validationResult;
107113
}
108114
if (not sampler->isCompatibleDevicewise(write.dstSet)) {
109115
const char* samplerDebugName = sampler->getDebugName();
110116
if (samplerDebugName && debugName)
111117
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);
112118
else
113119
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);
114-
return asset::IDescriptor::E_TYPE::ET_COUNT;
120+
validationResult.type = asset::IDescriptor::E_TYPE::ET_COUNT;
121+
return validationResult;
115122
}
116123
}
117124
if (descriptorType == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) {
118-
core::smart_refctd_ptr<video::IGPUSampler>* mutableSamplers = getMutableCombinedSamplers(bindingNumber);
125+
validationResult.mutableSamplerRedirectBindingIndex = getLayout()->getMutableCombinedSamplerRedirect().findBindingStorageIndex(bindingNumber);
126+
auto mutableSamplers = getMutableCombinedSamplers(validationResult.mutableSamplerRedirectBindingIndex);
127+
119128
// Should never reach here, the GetTypeCategory check earlier should ensure that
120129
if (!mutableSamplers)
121130
{
@@ -124,7 +133,8 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor
124133
else
125134
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);
126135

127-
return asset::IDescriptor::E_TYPE::ET_COUNT;
136+
validationResult.type = asset::IDescriptor::E_TYPE::ET_COUNT;
137+
return validationResult;
128138
}
129139
}
130140
}
@@ -143,24 +153,22 @@ asset::IDescriptor::E_TYPE IGPUDescriptorSet::validateWrite(const IGPUDescriptor
143153
}
144154
}
145155

146-
return descriptorType;
156+
validationResult.type = descriptorType;
157+
return validationResult;
147158
}
148159

149-
void IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write, const asset::IDescriptor::E_TYPE type)
160+
void IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write, const SWriteValidationResult& validationResult)
150161
{
151162
assert(write.dstSet == this);
152163

153-
using redirect_t = IGPUDescriptorSetLayout::CBindingRedirect;
154-
const redirect_t::binding_number_t bindingNumber(write.binding);
155-
auto descriptorRedirectBindingIndex = getLayout()->getDescriptorRedirect(type).findBindingStorageIndex(bindingNumber);
156-
auto* descriptors = getDescriptors(type, descriptorRedirectBindingIndex);
164+
auto* descriptors = getDescriptors(validationResult.type, validationResult.descriptorRedirectBindingIndex);
157165
assert(descriptors);
158166

159167
core::smart_refctd_ptr<video::IGPUSampler>* mutableSamplers = nullptr;
160-
if (type == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER
168+
if (validationResult.type == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER
161169
and write.info->info.combinedImageSampler.sampler)
162170
{
163-
mutableSamplers = getMutableCombinedSamplers(bindingNumber);
171+
mutableSamplers = getMutableCombinedSamplers(validationResult.mutableSamplerRedirectBindingIndex);
164172
assert(mutableSamplers);
165173
}
166174

@@ -171,8 +179,8 @@ void IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSe
171179
if (mutableSamplers)
172180
mutableSamplers[j] = write.info[j].info.combinedImageSampler.sampler;
173181
}
174-
auto& bindingRedirect = m_layout->getDescriptorRedirect(type);
175-
auto bindingCreateFlags = bindingRedirect.getCreateFlags(descriptorRedirectBindingIndex);
182+
auto& bindingRedirect = m_layout->getDescriptorRedirect(validationResult.type);
183+
auto bindingCreateFlags = bindingRedirect.getCreateFlags(validationResult.descriptorRedirectBindingIndex);
176184
if (IGPUDescriptorSetLayout::writeIncrementsVersion(bindingCreateFlags))
177185
incrementVersion();
178186
}
@@ -207,7 +215,7 @@ void IGPUDescriptorSet::dropDescriptors(const IGPUDescriptorSet::SDropDescriptor
207215
incrementVersion();
208216
}
209217

210-
bool IGPUDescriptorSet::validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy) const
218+
IGPUDescriptorSet::SCopyValidationResult IGPUDescriptorSet::validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy) const
211219
{
212220
assert(copy.dstSet == this);
213221

@@ -218,15 +226,35 @@ bool IGPUDescriptorSet::validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet
218226
const char* srcDebugName = copy.srcSet->getDebugName();
219227
const char* dstDebugName = copy.dstSet->getDebugName();
220228

229+
const auto srcLayout = copy.srcSet->getLayout();
230+
const auto dstLayout = copy.dstSet->getLayout();
231+
232+
SCopyValidationResult validationResult;
233+
221234
for (uint32_t t = 0; t < static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT); ++t)
222235
{
223-
const auto type = static_cast<asset::IDescriptor::E_TYPE>(t);
236+
validationResult.type = static_cast<asset::IDescriptor::E_TYPE>(t);
224237

225-
auto* srcDescriptors = copy.srcSet->getDescriptors(type, srcBindingNumber);
226-
auto* dstDescriptors = copy.dstSet->getDescriptors(type, dstBindingNumber);
238+
validationResult.srcDescriptorRedirectBindingIndex = srcLayout->getDescriptorRedirect(validationResult.type).findBindingStorageIndex(srcBindingNumber);
239+
validationResult.dstDescriptorRedirectBindingIndex = dstLayout->getDescriptorRedirect(validationResult.type).findBindingStorageIndex(dstBindingNumber);
240+
auto* srcDescriptors = copy.srcSet->getDescriptors(validationResult.type, validationResult.srcDescriptorRedirectBindingIndex);
241+
auto* dstDescriptors = copy.dstSet->getDescriptors(validationResult.type, validationResult.dstDescriptorRedirectBindingIndex);
227242

228-
auto* srcSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.srcSet->getMutableCombinedSamplers(srcBindingNumber);
229-
auto* dstSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.dstSet->getMutableCombinedSamplers(dstBindingNumber);
243+
if (!srcDescriptors)
244+
{
245+
assert(!dstDescriptors);
246+
continue;
247+
}
248+
assert(dstDescriptors);
249+
250+
core::smart_refctd_ptr<IGPUSampler> *srcSamplers = nullptr, *dstSamplers = nullptr;
251+
if (asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER == validationResult.type)
252+
{
253+
validationResult.srcMutableSamplerRedirectBindingIndex = srcLayout->getMutableCombinedSamplerRedirect().findBindingStorageIndex(srcBindingNumber);
254+
validationResult.dstMutableSamplerRedirectBindingIndex = dstLayout->getMutableCombinedSamplerRedirect().findBindingStorageIndex(dstBindingNumber);
255+
srcSamplers = copy.srcSet->getMutableCombinedSamplers(validationResult.srcMutableSamplerRedirectBindingIndex);
256+
dstSamplers = copy.dstSet->getMutableCombinedSamplers(validationResult.dstMutableSamplerRedirectBindingIndex);
257+
}
230258

231259
if ((!srcDescriptors != !dstDescriptors) || (!srcSamplers != !dstSamplers))
232260
{
@@ -235,53 +263,34 @@ bool IGPUDescriptorSet::validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet
235263
else
236264
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);
237265

238-
return false;
266+
validationResult.type = asset::IDescriptor::E_TYPE::ET_COUNT;
267+
return validationResult;
239268
}
240269
}
241270

242-
return true;
271+
return validationResult;
243272
}
244273

245-
void IGPUDescriptorSet::processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy)
274+
void IGPUDescriptorSet::processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy, const SCopyValidationResult& validationResult)
246275
{
247276
assert(copy.dstSet == this);
248277

249-
using redirect_t = IGPUDescriptorSetLayout::CBindingRedirect;
250-
redirect_t::binding_number_t srcBindingNumber(copy.srcBinding);
251-
redirect_t::binding_number_t dstBindingNumber(copy.dstBinding);
278+
auto& bindingRedirect = m_layout->getDescriptorRedirect(validationResult.type);
252279

253-
for (uint32_t t = 0; t < static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT); ++t)
254-
{
255-
const auto type = static_cast<asset::IDescriptor::E_TYPE>(t);
256-
auto& bindingRedirect = m_layout->getDescriptorRedirect(type);
280+
auto* srcDescriptors = copy.srcSet->getDescriptors(validationResult.type, validationResult.srcDescriptorRedirectBindingIndex);
281+
auto* dstDescriptors = copy.dstSet->getDescriptors(validationResult.type, validationResult.dstDescriptorRedirectBindingIndex);
257282

258-
auto* srcDescriptors = copy.srcSet->getDescriptors(type, srcBindingNumber);
259-
// We can do this because dstSet === this as asserted at the start
260-
auto descriptorRedirectBindingIndex = bindingRedirect.findBindingStorageIndex(dstBindingNumber);
261-
auto* dstDescriptors = copy.dstSet->getDescriptors(type, descriptorRedirectBindingIndex);
262-
if (!srcDescriptors)
263-
{
264-
assert(!dstDescriptors);
265-
continue;
266-
}
267-
assert(dstDescriptors);
268-
269-
auto* srcSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.srcSet->getMutableCombinedSamplers(srcBindingNumber);
270-
auto* dstSamplers = type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.dstSet->getMutableCombinedSamplers(dstBindingNumber);
271-
assert(!(!srcSamplers != !dstSamplers));
272-
273-
274-
auto bindingCreateFlags = bindingRedirect.getCreateFlags(descriptorRedirectBindingIndex);
275-
if (IGPUDescriptorSetLayout::writeIncrementsVersion(bindingCreateFlags))
276-
incrementVersion();
277-
278-
std::copy_n(srcDescriptors, copy.count, dstDescriptors);
283+
auto* srcSamplers = validationResult.type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.srcSet->getMutableCombinedSamplers(validationResult.srcMutableSamplerRedirectBindingIndex);
284+
auto* dstSamplers = validationResult.type != asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER ? nullptr : copy.dstSet->getMutableCombinedSamplers(validationResult.dstMutableSamplerRedirectBindingIndex);
285+
286+
auto bindingCreateFlags = bindingRedirect.getCreateFlags(validationResult.dstDescriptorRedirectBindingIndex);
287+
if (IGPUDescriptorSetLayout::writeIncrementsVersion(bindingCreateFlags))
288+
incrementVersion();
279289

280-
if (srcSamplers && dstSamplers)
281-
std::copy_n(srcSamplers, copy.count, dstSamplers);
290+
std::copy_n(srcDescriptors, copy.count, dstDescriptors);
282291

283-
break;
284-
}
292+
if (srcSamplers && dstSamplers)
293+
std::copy_n(srcSamplers, copy.count, dstSamplers);
285294
}
286295

287296
}

0 commit comments

Comments
 (0)