Skip to content

Commit 79c3a23

Browse files
deprilula28deprilula28
committed
Fix API for nullify
1 parent 5531903 commit 79c3a23

File tree

7 files changed

+54
-45
lines changed

7 files changed

+54
-45
lines changed

‎include/nbl/video/IGPUDescriptorSet.h

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,20 @@ class IGPUDescriptorSet : public asset::IDescriptorSet<const IGPUDescriptorSetLa
5757
inline IDescriptorPool* getPool() const { return m_pool.get(); }
5858
inline bool isZombie() const { return (m_pool.get() == nullptr); }
5959

60+
// why is this private? nothing it uses is private
61+
// small utility
62+
inline asset::IDescriptor::E_TYPE getBindingType(const uint32_t binding) const
63+
{
64+
for (auto t=0u; t<static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT); t++)
65+
{
66+
const auto type = static_cast<asset::IDescriptor::E_TYPE>(t);
67+
const auto& bindingRedirect = getLayout()->getDescriptorRedirect(type);
68+
if (bindingRedirect.getStorageOffset(redirect_t::binding_number_t{binding}).data!=redirect_t::Invalid)
69+
return type;
70+
}
71+
return asset::IDescriptor::E_TYPE::ET_COUNT;
72+
}
73+
6074
protected:
6175
IGPUDescriptorSet(core::smart_refctd_ptr<const IGPUDescriptorSetLayout>&& _layout, core::smart_refctd_ptr<IDescriptorPool>&& pool, IDescriptorPool::SStorageOffsets&& offsets);
6276
virtual ~IGPUDescriptorSet();
@@ -69,7 +83,7 @@ class IGPUDescriptorSet : public asset::IDescriptorSet<const IGPUDescriptorSetLa
6983
void processWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write, const asset::IDescriptor::E_TYPE type);
7084
bool validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy) const;
7185
void processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy);
72-
void dropDescriptors(const IGPUDescriptorSet::SDropDescriptorSet& drop, const asset::IDescriptor::E_TYPE type);
86+
void dropDescriptors(const IGPUDescriptorSet::SDropDescriptorSet& drop);
7387

7488
using redirect_t = IGPUDescriptorSetLayout::CBindingRedirect;
7589
// 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.
@@ -85,18 +99,6 @@ class IGPUDescriptorSet : public asset::IDescriptorSet<const IGPUDescriptorSetLa
8599

86100
return descriptors+localOffset;
87101
}
88-
// small utility
89-
inline asset::IDescriptor::E_TYPE getBindingType(const uint32_t binding) const
90-
{
91-
for (auto t=0u; t<static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT); t++)
92-
{
93-
const auto type = static_cast<asset::IDescriptor::E_TYPE>(t);
94-
const auto& bindingRedirect = getLayout()->getDescriptorRedirect(type);
95-
if (bindingRedirect.getStorageOffset(redirect_t::binding_number_t{binding}).data!=redirect_t::Invalid)
96-
return type;
97-
}
98-
return asset::IDescriptor::E_TYPE::ET_COUNT;
99-
}
100102

101103
inline core::smart_refctd_ptr<IGPUSampler>* getMutableSamplers(const uint32_t binding) const
102104
{

‎include/nbl/video/ILogicalDevice.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -633,7 +633,7 @@ class NBL_API2 ILogicalDevice : public core::IReferenceCounted, public IDeviceMe
633633
}
634634

635635
// should this be joined together with the existing updateDescriptorSets?
636-
bool nullifyDescriptors(const std::span<const IGPUDescriptorSet::SDropDescriptorSet> dropDescriptors, asset::IDescriptor::E_TYPE descriptorType);
636+
bool nullifyDescriptors(const std::span<const IGPUDescriptorSet::SDropDescriptorSet> dropDescriptors);
637637

638638
//! Renderpasses and Framebuffers
639639
core::smart_refctd_ptr<IGPURenderpass> createRenderpass(const IGPURenderpass::SCreationParams& params);
@@ -853,7 +853,7 @@ class NBL_API2 ILogicalDevice : public core::IReferenceCounted, public IDeviceMe
853853

854854
// Drops refcounted references of the descriptors in these indices for the descriptor lifetime tracking
855855
// If the nullDescriptor device feature is enabled, this would also write a null descriptor to the descriptor set
856-
virtual void nullifyDescriptors_impl(const std::span<const IGPUDescriptorSet::SDropDescriptorSet> dropDescriptors, asset::IDescriptor::E_TYPE descriptorType) = 0;
856+
virtual void nullifyDescriptors_impl(const std::span<const IGPUDescriptorSet::SDropDescriptorSet> dropDescriptors) = 0;
857857

858858
virtual core::smart_refctd_ptr<IGPURenderpass> createRenderpass_impl(const IGPURenderpass::SCreationParams& params, IGPURenderpass::SCreationParamValidationResult&& validation) = 0;
859859
virtual core::smart_refctd_ptr<IGPUFramebuffer> createFramebuffer_impl(IGPUFramebuffer::SCreationParams&& params) = 0;

‎include/nbl/video/alloc/SubAllocatedDescriptorSet.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ class SubAllocatedDescriptorSet : public core::IReferenceCounted
284284
nulls.resize(m_totalDeferredFrees);
285285
auto outNulls = nulls.data();
286286
eventHandler->wait(maxWaitPoint, unallocatedSize, outNulls);
287-
m_logicalDevice->nullifyDescriptors({ nulls.data(),outNulls }, range->second.descriptorType);
287+
m_logicalDevice->nullifyDescriptors({ nulls.data(),outNulls });
288288

289289
// always call with the same parameters, otherwise this turns into a mess with the non invalid_address gaps
290290
unallocatedSize = try_multi_allocate(binding,count,outAddresses);
@@ -352,7 +352,7 @@ class SubAllocatedDescriptorSet : public core::IReferenceCounted
352352
auto actualEnd = multi_deallocate(nulls.data(), binding, count, addr);
353353
// This is checked to be valid above
354354
auto range = m_allocatableRanges.find(binding);
355-
m_logicalDevice->nullifyDescriptors({nulls.data(),actualEnd}, range->second.descriptorType);
355+
m_logicalDevice->nullifyDescriptors({nulls.data(),actualEnd});
356356
}
357357
}
358358

@@ -367,9 +367,8 @@ class SubAllocatedDescriptorSet : public core::IReferenceCounted
367367
{
368368
auto& it = m_allocatableRanges[i];
369369
frees += it.eventHandler->poll(outNulls).eventsLeft;
370-
// TODO: this could be optimized to be put outside the loop
371-
m_logicalDevice->nullifyDescriptors({nulls.data(),outNulls}, it.descriptorType);
372370
}
371+
m_logicalDevice->nullifyDescriptors({nulls.data(),outNulls});
373372
return frees;
374373
}
375374
};

‎src/nbl/video/CVulkanLogicalDevice.cpp

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -727,11 +727,11 @@ void CVulkanLogicalDevice::updateDescriptorSets_impl(const SUpdateDescriptorSets
727727
m_devf.vk.vkUpdateDescriptorSets(m_vkdev,vk_writeDescriptorSets.size(),vk_writeDescriptorSets.data(),vk_copyDescriptorSets.size(),vk_copyDescriptorSets.data());
728728
}
729729

730-
void CVulkanLogicalDevice::nullifyDescriptors_impl(const std::span<const IGPUDescriptorSet::SDropDescriptorSet> drops, asset::IDescriptor::E_TYPE descriptorType)
730+
void CVulkanLogicalDevice::nullifyDescriptors_impl(const std::span<const IGPUDescriptorSet::SDropDescriptorSet> drops)
731731
{
732732
if (getEnabledFeatures().nullDescriptor)
733733
{
734-
return
734+
return;
735735
}
736736

737737
core::vector<VkWriteDescriptorSet> vk_writeDescriptorSets(drops.size(),{VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET,nullptr});
@@ -741,26 +741,27 @@ void CVulkanLogicalDevice::nullifyDescriptors_impl(const std::span<const IGPUDes
741741
for (auto i = 0; i < drops.size(); i++)
742742
{
743743
const auto& write = drops[i];
744-
maxSize = core::max(maxSize, write.count);
745-
}
746-
size_t descriptorSize;
747-
switch (asset::IDescriptor::GetTypeCategory(descriptorType))
748-
{
749-
case asset::IDescriptor::EC_BUFFER:
750-
descriptorSize = sizeof(VkDescriptorBufferInfo);
751-
break;
752-
case asset::IDescriptor::EC_IMAGE:
753-
descriptorSize = sizeof(VkDescriptorImageInfo);
754-
break;
755-
case asset::IDescriptor::EC_BUFFER_VIEW:
756-
descriptorSize = sizeof(VkBufferView);
757-
break;
758-
case asset::IDescriptor::EC_ACCELERATION_STRUCTURE:
759-
descriptorSize = sizeof(VkAccelerationStructureKHR);
760-
break;
744+
auto descriptorType = write.dstSet->getBindingType(write.binding);
745+
size_t descriptorSize;
746+
switch (asset::IDescriptor::GetTypeCategory(descriptorType))
747+
{
748+
case asset::IDescriptor::EC_BUFFER:
749+
descriptorSize = sizeof(VkDescriptorBufferInfo);
750+
break;
751+
case asset::IDescriptor::EC_IMAGE:
752+
descriptorSize = sizeof(VkDescriptorImageInfo);
753+
break;
754+
case asset::IDescriptor::EC_BUFFER_VIEW:
755+
descriptorSize = sizeof(VkBufferView);
756+
break;
757+
case asset::IDescriptor::EC_ACCELERATION_STRUCTURE:
758+
descriptorSize = sizeof(VkAccelerationStructureKHR);
759+
break;
760+
}
761+
maxSize = core::max(maxSize, write.count * descriptorSize);
761762
}
762763

763-
core::vector<uint8_t> nullDescriptors(maxSize * descriptorSize, 0u);
764+
core::vector<uint8_t> nullDescriptors(maxSize, 0u);
764765

765766
{
766767
auto outWrite = vk_writeDescriptorSets.data();
@@ -769,6 +770,7 @@ void CVulkanLogicalDevice::nullifyDescriptors_impl(const std::span<const IGPUDes
769770
for (auto i=0; i<drops.size(); i++)
770771
{
771772
const auto& write = drops[i];
773+
auto descriptorType = write.dstSet->getBindingType(write.binding);
772774

773775
outWrite->dstSet = static_cast<const CVulkanDescriptorSet*>(write.dstSet)->getInternalObject();
774776
outWrite->dstBinding = write.binding;

‎src/nbl/video/CVulkanLogicalDevice.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ class CVulkanLogicalDevice final : public ILogicalDevice
282282
// descriptor sets
283283
core::smart_refctd_ptr<IDescriptorPool> createDescriptorPool_impl(const IDescriptorPool::SCreateInfo& createInfo) override;
284284
void updateDescriptorSets_impl(const SUpdateDescriptorSetsParams& params) override;
285-
void nullifyDescriptors_impl(const std::span<const IGPUDescriptorSet::SDropDescriptorSet> dropDescriptors, asset::IDescriptor::E_TYPE descriptorType) override;
285+
void nullifyDescriptors_impl(const std::span<const IGPUDescriptorSet::SDropDescriptorSet> dropDescriptors) override;
286286

287287
// renderpasses and framebuffers
288288
core::smart_refctd_ptr<IGPURenderpass> createRenderpass_impl(const IGPURenderpass::SCreationParams& params, IGPURenderpass::SCreationParamValidationResult&& validation) override;

‎src/nbl/video/IGPUDescriptorSet.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,13 @@ void IGPUDescriptorSet::processWrite(const IGPUDescriptorSet::SWriteDescriptorSe
124124
incrementVersion();
125125
}
126126

127-
void IGPUDescriptorSet::dropDescriptors(const IGPUDescriptorSet::SDropDescriptorSet& drop, const asset::IDescriptor::E_TYPE type)
127+
void IGPUDescriptorSet::dropDescriptors(const IGPUDescriptorSet::SDropDescriptorSet& drop)
128128
{
129129
assert(drop.dstSet == this);
130130

131-
auto* dstDescriptors = drop.dstSet->getDescriptors(type, drop.binding);
131+
const auto descriptorType = getBindingType(drop.binding);
132+
133+
auto* dstDescriptors = drop.dstSet->getDescriptors(descriptorType, drop.binding);
132134
auto* dstSamplers = drop.dstSet->getMutableSamplers(drop.binding);
133135

134136
if (dstDescriptors)

‎src/nbl/video/ILogicalDevice.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -444,18 +444,22 @@ bool ILogicalDevice::updateDescriptorSets(const std::span<const IGPUDescriptorSe
444444
return true;
445445
}
446446

447-
bool ILogicalDevice::nullifyDescriptors(const std::span<const IGPUDescriptorSet::SDropDescriptorSet> dropDescriptors, asset::IDescriptor::E_TYPE descriptorType)
447+
bool ILogicalDevice::nullifyDescriptors(const std::span<const IGPUDescriptorSet::SDropDescriptorSet> dropDescriptors)
448448
{
449449
for (const auto& drop : dropDescriptors)
450450
{
451451
auto ds = drop.dstSet;
452452
if (!ds || !ds->wasCreatedBy(this))
453453
return false;
454+
}
454455

455-
ds->dropDescriptors(drop, descriptorType);
456+
for (const auto& drop : dropDescriptors)
457+
{
458+
auto ds = drop.dstSet;
459+
ds->dropDescriptors(drop);
456460
}
457461

458-
nullifyDescriptors_impl(dropDescriptors, descriptorType);
462+
nullifyDescriptors_impl(dropDescriptors);
459463
return true;
460464
}
461465

0 commit comments

Comments
 (0)