Skip to content

Commit f8c998c

Browse files
achalpandeyyachalpandeyy
committed
Some cleanups.
1 parent 65a3ad3 commit f8c998c

File tree

6 files changed

+101
-77
lines changed

6 files changed

+101
-77
lines changed

‎include/nbl/asset/IDescriptorSetLayout.h

Lines changed: 43 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -80,79 +80,85 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted
8080
uint32_t data;
8181
};
8282

83+
struct storage_range_index_t
84+
{
85+
inline storage_range_index_t(const uint32_t d) : data(d) {}
86+
uint32_t data;
87+
};
88+
8389
inline uint32_t getBindingCount() const { return m_count; }
8490

8591
// Returns index into the binding property arrays below (including `m_storageOffsets`), for the given binding number `binding`.
8692
// Assumes `m_bindingNumbers` is sorted and that there are no duplicate values in it.
87-
inline uint32_t searchForBinding(const binding_number_t binding) const
93+
inline storage_range_index_t findBindingStorageIndex(const binding_number_t binding) const
8894
{
8995
if (!m_bindingNumbers)
90-
return Invalid;
96+
return { Invalid };
9197

9298
assert(m_storageOffsets && (m_count != 0u));
9399

94100
auto found = std::lower_bound(m_bindingNumbers, m_bindingNumbers + m_count, binding, [](binding_number_t a, binding_number_t b) -> bool {return a.data < b.data; });
95101

96102
if ((found >= m_bindingNumbers + m_count) || (found->data != binding.data))
97-
return Invalid;
103+
return { Invalid };
98104

99105
const uint32_t foundIndex = found - m_bindingNumbers;
100106
assert(foundIndex < m_count);
101-
return foundIndex;
107+
return { foundIndex };
102108
}
103109

104-
inline binding_number_t getBindingNumber(const uint32_t index) const
110+
inline binding_number_t getBindingFromStorageIndex(const storage_range_index_t index) const
105111
{
106-
assert(index < m_count);
107-
return m_bindingNumbers[index];
112+
assert(index.data < m_count);
113+
return m_bindingNumbers[index.data];
108114
}
109115

110-
inline core::bitflag<IShader::E_SHADER_STAGE> getStageFlags(const uint32_t index) const
116+
inline core::bitflag<IShader::E_SHADER_STAGE> getStageFlagsFromStorageIndex(const storage_range_index_t index) const
111117
{
112-
assert(index < m_count);
113-
return m_stageFlags[index];
118+
assert(index.data < m_count);
119+
return m_stageFlags[index.data];
114120
}
115121

116-
inline uint32_t getCount(const uint32_t index) const
122+
inline uint32_t getCountFromStorageIndex(const storage_range_index_t index) const
117123
{
118-
assert(index < m_count);
119-
return (index == 0u) ? m_storageOffsets[index].data : m_storageOffsets[index].data - m_storageOffsets[index - 1].data;
124+
assert(index.data < m_count);
125+
return (index.data == 0u) ? m_storageOffsets[index.data].data : m_storageOffsets[index.data].data - m_storageOffsets[index.data - 1].data;
120126
}
121127

122-
inline storage_offset_t getStorageOffset(const uint32_t index) const
128+
inline storage_offset_t getStorageOffsetFromStorageIndex(const storage_range_index_t index) const
123129
{
124-
assert(index < m_count);
125-
return (index == 0u) ? 0u : m_storageOffsets[index - 1];
130+
assert(index.data < m_count);
131+
return (index.data == 0u) ? 0u : m_storageOffsets[index.data - 1];
126132
}
127133

128-
// The follwoing are merely convenience functions for one off use.
129-
// If you already have an index (the result of `searchForBinding`) lying around use the above functions for quick lookups, and to avoid unnecessary binary searches.
134+
// The following are merely convenience functions for one off use.
135+
// If you already have an index (the result of `findBindingStorageIndex`) lying around use the above functions for quick lookups, and to avoid unnecessary binary searches.
130136

131137
inline core::bitflag<IShader::E_SHADER_STAGE> getStageFlags(const binding_number_t binding) const
132138
{
133-
const auto index = searchForBinding(binding);
139+
const auto index = findBindingStorageIndex(binding);
134140
if (index == Invalid)
135-
return Invalid;
141+
return IShader::ESS_UNKNOWN;
136142

137143
return getStageFlags(index);
138144
}
139145

140146
inline uint32_t getCount(const binding_number_t binding) const
141147
{
142-
const auto index = searchForBinding(binding);
148+
const auto index = findBindingStorageIndex(binding);
143149
if (index == Invalid)
144-
return Invalid;
150+
return 0;
145151

146152
return getDescriptorCount(index);
147153
}
148154

149155
inline storage_offset_t getStorageOffset(const binding_number_t binding) const
150156
{
151-
const auto index = searchForBinding(binding);
152-
if (index == Invalid)
153-
return Invalid;
157+
const auto index = findBindingStorageIndex(binding);
158+
if (index.data == Invalid)
159+
return { Invalid };
154160

155-
return getStorageOffset(index);
161+
return getStorageOffsetFromStorageIndex(index);
156162
}
157163

158164
inline uint32_t getTotalCount() const { return (m_count == 0ull) ? 0u : m_storageOffsets[m_count - 1].data; }
@@ -203,17 +209,26 @@ class IDescriptorSetLayout : public virtual core::IReferenceCounted
203209

204210
uint64_t offset = 0ull;
205211

212+
// Allocations ordered from fattest alignment to smallest alignment, because there could be problem on ARM.
206213
m_bindingNumbers = reinterpret_cast<binding_number_t*>(m_data.get() + offset);
207214
offset += m_count * sizeof(binding_number_t);
215+
assert(core::is_aligned_ptr(m_bindingNumbers));
208216

209-
m_createFlags = reinterpret_cast<core::bitflag<typename SBinding::E_CREATE_FLAGS>*>(m_data.get() + offset);
210-
offset += m_count * sizeof(core::bitflag<typename SBinding::E_CREATE_FLAGS>);
217+
assert(alignof(core::bitflag<IShader::E_SHADER_STAGE>) <= alignof(decltype(m_bindingNumbers[0])));
211218

212219
m_stageFlags = reinterpret_cast<core::bitflag<IShader::E_SHADER_STAGE>*>(m_data.get() + offset);
213220
offset += m_count * sizeof(core::bitflag<IShader::E_SHADER_STAGE>);
221+
assert(core::is_aligned_ptr(m_stageFlags));
222+
223+
assert(alignof(core::bitflag<IShader::E_SHADER_STAGE>) >= alignof(storage_offset_t));
214224

215225
m_storageOffsets = reinterpret_cast<storage_offset_t*>(m_data.get() + offset);
216226
offset += m_count * sizeof(storage_offset_t);
227+
assert(core::is_aligned_ptr(m_storageOffsets));
228+
229+
m_createFlags = reinterpret_cast<core::bitflag<typename SBinding::E_CREATE_FLAGS>*>(m_data.get() + offset);
230+
offset += m_count * sizeof(core::bitflag<typename SBinding::E_CREATE_FLAGS>);
231+
assert(core::is_aligned_ptr(m_createFlags));
217232

218233
assert(offset == requiredMemSize);
219234
}

‎include/nbl/core/memory/memory.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,12 @@ inline bool is_aligned_to(const void* value, size_t alignment)
9393
return core::is_aligned_to(reinterpret_cast<size_t>(value),alignment);
9494
}
9595

96+
template <typename T>
97+
constexpr inline bool is_aligned_ptr(T* ptr)
98+
{
99+
return is_aligned_to(ptr, alignof(T));
100+
}
101+
96102
}
97103
}
98104

‎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.getBindingNumber(i).data;
487+
write.binding = redirect.getBindingFromStorageIndex(i).data;
488488
write.arrayElement = 0u;
489-
write.count = redirect.getCount(i);
489+
write.count = redirect.getCountFromStorageIndex(i);
490490
write.info = &infos[i];
491491
write.descriptorType = type;
492492
}

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1382,17 +1382,17 @@ 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.getBindingNumber(b).data;
1385+
gpuBinding.binding = descriptorBindingRedirect.getBindingFromStorageIndex(b).data;
13861386
gpuBinding.type = type;
1387-
gpuBinding.count = descriptorBindingRedirect.getCount(b);
1388-
gpuBinding.stageFlags = descriptorBindingRedirect.getStageFlags(b);
1387+
gpuBinding.count = descriptorBindingRedirect.getCountFromStorageIndex(b);
1388+
gpuBinding.stageFlags = descriptorBindingRedirect.getStageFlagsFromStorageIndex(b);
13891389
gpuBinding.samplers = nullptr;
13901390

13911391
// If this DS layout has any immutable samplers..
13921392
if ((gpuBinding.type == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER) && (samplerBindingCount > 0))
13931393
{
13941394
// If this binding number has any immutable samplers..
1395-
if (samplerBindingRedirect.searchForBinding(asset::ICPUDescriptorSetLayout::CBindingRedirect::binding_number_t{ gpuBinding.binding }) == samplerBindingRedirect.Invalid)
1395+
if (samplerBindingRedirect.findBindingStorageIndex(asset::ICPUDescriptorSetLayout::CBindingRedirect::binding_number_t{ gpuBinding.binding }).data == samplerBindingRedirect.Invalid)
13961396
continue;
13971397

13981398
// Copy in tmpSamplers.
@@ -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.getBindingNumber(b).data;
1724+
write_it->binding = descriptorBindingRedirect.getBindingFromStorageIndex(b).data;
17251725
write_it->arrayElement = 0u;
17261726

1727-
const uint32_t descriptorCount = descriptorBindingRedirect.getCount(b);
1727+
const uint32_t descriptorCount = descriptorBindingRedirect.getCountFromStorageIndex(b);
17281728
write_it->count = descriptorCount;
17291729
write_it->descriptorType = type;
17301730
write_it->info = &(*info);
17311731

1732-
const uint32_t offset = descriptorBindingRedirect.getStorageOffset(b).data;
1732+
const uint32_t offset = descriptorBindingRedirect.getStorageOffsetFromStorageIndex(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
@@ -1771,7 +1771,7 @@ inline created_gpu_object_array<asset::ICPUDescriptorSet> IGPUObjectFromAssetCon
17711771

17721772
if (!isStorageImgDesc(type))
17731773
{
1774-
const bool isMutableSamplerBinding = (mutableSamplerBindingRedirect.searchForBinding(asset::ICPUDescriptorSetLayout::CBindingRedirect::binding_number_t{ write_it->binding }) != mutableSamplerBindingRedirect.Invalid);
1774+
const bool isMutableSamplerBinding = (mutableSamplerBindingRedirect.findBindingStorageIndex(asset::ICPUDescriptorSetLayout::CBindingRedirect::binding_number_t{ write_it->binding }).data != mutableSamplerBindingRedirect.Invalid);
17751775
if (descriptorInfos.begin()[offset + d].info.image.sampler && isMutableSamplerBinding)
17761776
info->info.image.sampler = gpuSamplers->operator[](smplrRedirs[si++]);
17771777
}

‎src/nbl/asset/ICPUDescriptorSet.cpp

Lines changed: 41 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ core::SRange<const ICPUDescriptorSet::SDescriptorInfo> ICPUDescriptorSet::getDes
1818
{
1919
const auto possibleType = static_cast<IDescriptor::E_TYPE>(t);
2020
const auto& redirect = getLayout()->getDescriptorRedirect(possibleType);
21-
if (redirect.searchForBinding(binding) != redirect.Invalid)
21+
if (redirect.findBindingStorageIndex(binding).data != redirect.Invalid)
2222
{
2323
type = possibleType;
2424
break;
@@ -30,12 +30,12 @@ core::SRange<const ICPUDescriptorSet::SDescriptorInfo> ICPUDescriptorSet::getDes
3030
}
3131

3232
const auto& redirect = getLayout()->getDescriptorRedirect(type);
33-
const auto bindingNumberIndex = redirect.searchForBinding(binding);
34-
if (bindingNumberIndex == redirect.Invalid)
33+
const auto bindingNumberIndex = redirect.findBindingStorageIndex(binding);
34+
if (bindingNumberIndex.data == redirect.Invalid)
3535
return { nullptr, nullptr };
3636

37-
const auto offset = redirect.getStorageOffset(bindingNumberIndex).data;
38-
const auto count = redirect.getCount(bindingNumberIndex);
37+
const auto offset = redirect.getStorageOffsetFromStorageIndex(bindingNumberIndex).data;
38+
const auto count = redirect.getCountFromStorageIndex(bindingNumberIndex);
3939

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

@@ -112,7 +112,6 @@ void ICPUDescriptorSet::convertToDummyObject(uint32_t referenceLevelsBelowToConv
112112
if (referenceLevelsBelowToConvert)
113113
{
114114
--referenceLevelsBelowToConvert;
115-
m_layout->convertToDummyObject(referenceLevelsBelowToConvert);
116115

117116
for (uint32_t t = 0u; t < static_cast<uint32_t>(IDescriptor::E_TYPE::ET_COUNT); ++t)
118117
{
@@ -149,6 +148,8 @@ void ICPUDescriptorSet::convertToDummyObject(uint32_t referenceLevelsBelowToConv
149148
}
150149
}
151150
}
151+
152+
m_layout->convertToDummyObject(referenceLevelsBelowToConvert);
152153
}
153154
}
154155

@@ -204,49 +205,51 @@ void ICPUDescriptorSet::restoreFromDummy_impl(IAsset* _other, uint32_t _levelsBe
204205
bool ICPUDescriptorSet::isAnyDependencyDummy_impl(uint32_t _levelsBelow) const
205206
{
206207
--_levelsBelow;
207-
if (m_layout->isAnyDependencyDummy(_levelsBelow))
208-
return true;
209-
210-
for (uint32_t t = 0u; t < static_cast<uint32_t>(IDescriptor::E_TYPE::ET_COUNT); ++t)
208+
if (_levelsBelow)
211209
{
212-
const auto type = static_cast<IDescriptor::E_TYPE>(t);
213-
const auto descriptorCount = m_layout->getTotalDescriptorCount(type);
214-
if (descriptorCount == 0ull)
215-
continue;
210+
if (m_layout->isAnyDependencyDummy(_levelsBelow))
211+
return true;
216212

217-
auto descriptorInfos = m_descriptorInfos[t]->begin();
218-
assert(descriptorInfos);
219-
220-
const auto category = getCategoryFromType(type);
221-
for (uint32_t i = 0u; i < descriptorCount; ++i)
213+
for (uint32_t t = 0u; t < static_cast<uint32_t>(IDescriptor::E_TYPE::ET_COUNT); ++t)
222214
{
223-
switch (category)
224-
{
225-
case IDescriptor::EC_BUFFER:
226-
if (static_cast<ICPUBuffer*>(descriptorInfos[i].desc.get())->isAnyDependencyDummy(_levelsBelow))
227-
return true;
228-
break;
215+
const auto type = static_cast<IDescriptor::E_TYPE>(t);
216+
const auto descriptorCount = m_layout->getTotalDescriptorCount(type);
217+
if (descriptorCount == 0ull)
218+
continue;
219+
220+
auto descriptorInfos = m_descriptorInfos[t]->begin();
221+
assert(descriptorInfos);
229222

230-
case IDescriptor::EC_IMAGE:
223+
const auto category = getCategoryFromType(type);
224+
for (uint32_t i = 0u; i < descriptorCount; ++i)
231225
{
232-
if (static_cast<ICPUImageView*>(descriptorInfos[i].desc.get())->isAnyDependencyDummy(_levelsBelow))
233-
return true;
226+
switch (category)
227+
{
228+
case IDescriptor::EC_BUFFER:
229+
if (static_cast<ICPUBuffer*>(descriptorInfos[i].desc.get())->isAnyDependencyDummy(_levelsBelow))
230+
return true;
231+
break;
234232

235-
if (descriptorInfos[i].info.image.sampler && descriptorInfos[i].info.image.sampler->isAnyDependencyDummy(_levelsBelow))
236-
return true;
237-
} break;
233+
case IDescriptor::EC_IMAGE:
234+
{
235+
if (static_cast<ICPUImageView*>(descriptorInfos[i].desc.get())->isAnyDependencyDummy(_levelsBelow))
236+
return true;
238237

239-
case IDescriptor::EC_BUFFER_VIEW:
240-
if (static_cast<ICPUBufferView*>(descriptorInfos[i].desc.get())->isAnyDependencyDummy(_levelsBelow))
241-
return true;
242-
break;
238+
if (descriptorInfos[i].info.image.sampler && descriptorInfos[i].info.image.sampler->isAnyDependencyDummy(_levelsBelow))
239+
return true;
240+
} break;
241+
242+
case IDescriptor::EC_BUFFER_VIEW:
243+
if (static_cast<ICPUBufferView*>(descriptorInfos[i].desc.get())->isAnyDependencyDummy(_levelsBelow))
244+
return true;
245+
break;
243246

244-
default:
245-
assert(!"Invalid code path.");
247+
default:
248+
assert(!"Invalid code path.");
249+
}
246250
}
247251
}
248252
}
249-
250253
return false;
251254
}
252255

‎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).getBindingNumber(0).data;
51+
semantic.descriptorSection.uniformBufferObject.binding = ds1layout->getDescriptorRedirect(IDescriptor::E_TYPE::ET_UNIFORM_BUFFER).getBindingFromStorageIndex(0).data;
5252
semantic.descriptorSection.uniformBufferObject.set = 1u;
5353
semantic.descriptorSection.uniformBufferObject.relByteoffset = relOffsets[i];
5454
semantic.descriptorSection.uniformBufferObject.bytesize = sizes[i];

0 commit comments

Comments
 (0)