Skip to content

Commit 89e6440

Browse files
committed
Fix multi timeline functionality
1 parent 912ed7a commit 89e6440

File tree

4 files changed

+40
-34
lines changed

4 files changed

+40
-34
lines changed

include/nbl/video/IGPUDescriptorSet.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class IGPUDescriptorSet : public asset::IDescriptorSet<const IGPUDescriptorSetLa
6969
void processWrite(const IGPUDescriptorSet::SWriteDescriptorSet& write, const asset::IDescriptor::E_TYPE type);
7070
bool validateCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy) const;
7171
void processCopy(const IGPUDescriptorSet::SCopyDescriptorSet& copy);
72-
void dropDescriptors(const IGPUDescriptorSet::SDropDescriptorSet& drop);
72+
void dropDescriptors(const IGPUDescriptorSet::SDropDescriptorSet& drop, const asset::IDescriptor::E_TYPE type);
7373

7474
using redirect_t = IGPUDescriptorSetLayout::CBindingRedirect;
7575
// 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/alloc/SubAllocatedDescriptorSet.h

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -99,31 +99,34 @@ class SubAllocatedDescriptorSet : public core::IReferenceCounted
9999
using EventHandler = MultiTimelineEventHandlerST<DeferredFreeFunctor>;
100100
protected:
101101
struct SubAllocDescriptorSetRange {
102-
EventHandler eventHandler = EventHandler({});
102+
std::unique_ptr<EventHandler> eventHandler = nullptr;
103103
std::unique_ptr<AddressAllocator> addressAllocator = nullptr;
104104
std::unique_ptr<ReservedAllocator> reservedAllocator = nullptr;
105105
size_t reservedSize = 0;
106106
asset::IDescriptor::E_TYPE descriptorType = asset::IDescriptor::E_TYPE::ET_COUNT;
107107

108108
SubAllocDescriptorSetRange(
109+
std::unique_ptr<EventHandler>&& inEventHandler,
109110
std::unique_ptr<AddressAllocator>&& inAddressAllocator,
110111
std::unique_ptr<ReservedAllocator>&& inReservedAllocator,
111112
size_t inReservedSize,
112113
asset::IDescriptor::E_TYPE inDescriptorType) :
113-
eventHandler({}), addressAllocator(std::move(inAddressAllocator)),
114+
eventHandler(std::move(inEventHandler)), addressAllocator(std::move(inAddressAllocator)),
114115
reservedAllocator(std::move(inReservedAllocator)),
115116
reservedSize(inReservedSize),
116117
descriptorType(inDescriptorType) {}
117118
SubAllocDescriptorSetRange() {}
118119

119120
SubAllocDescriptorSetRange& operator=(SubAllocDescriptorSetRange&& other)
120121
{
122+
eventHandler = std::move(other.eventHandler);
121123
addressAllocator = std::move(other.addressAllocator);
122124
reservedAllocator = std::move(other.reservedAllocator);
123125
reservedSize = other.reservedSize;
124126
descriptorType = other.descriptorType;
125127

126128
// Nullify other
129+
other.eventHandler = nullptr;
127130
other.addressAllocator = nullptr;
128131
other.reservedAllocator = nullptr;
129132
other.reservedSize = 0u;
@@ -147,7 +150,7 @@ class SubAllocatedDescriptorSet : public core::IReferenceCounted
147150

148151
// constructors
149152
inline SubAllocatedDescriptorSet(core::smart_refctd_ptr<video::IGPUDescriptorSet>&& descriptorSet,
150-
core::smart_refctd_ptr<video::ILogicalDevice>&& logicalDevice) : m_logicalDevice(std::move(logicalDevice))
153+
core::smart_refctd_ptr<video::ILogicalDevice>&& logicalDevice)
151154
{
152155
auto layout = descriptorSet->getLayout();
153156
for (uint32_t descriptorType = 0; descriptorType < static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT); descriptorType++)
@@ -175,12 +178,15 @@ class SubAllocatedDescriptorSet : public core::IReferenceCounted
175178
static_cast<size_type>(0), 0u, MaxDescriptorSetAllocationAlignment, static_cast<size_type>(count),
176179
MinDescriptorSetAllocationSize
177180
));
181+
auto eventHandler = std::unique_ptr<EventHandler>(new EventHandler(core::smart_refctd_ptr<ILogicalDevice>(logicalDevice)));
178182

179-
m_allocatableRanges[binding.data] = SubAllocDescriptorSetRange(std::move(addressAllocator), std::move(reservedAllocator), reservedSize, descType);
183+
m_allocatableRanges[binding.data] = SubAllocDescriptorSetRange(std::move(eventHandler), std::move(addressAllocator), std::move(reservedAllocator), reservedSize, descType);
184+
assert(m_allocatableRanges[binding.data].eventHandler->getLogicalDevice());
180185
}
181186
}
182187
}
183188
m_descriptorSet = std::move(descriptorSet);
189+
m_logicalDevice = std::move(logicalDevice);
184190
}
185191

186192
inline ~SubAllocatedDescriptorSet()
@@ -272,7 +278,7 @@ class SubAllocatedDescriptorSet : public core::IReferenceCounted
272278
// FUTURE TODO: later we could only nullify the descriptors we don't end up reallocating if without robustness features
273279
nulls.resize(m_totalDeferredFrees);
274280
auto outNulls = nulls.data();
275-
eventHandler.wait(maxWaitPoint, unallocatedSize, outNulls);
281+
eventHandler->wait(maxWaitPoint, unallocatedSize, outNulls);
276282
m_logicalDevice->nullifyDescriptors({ nulls.data(),outNulls }, range->second.descriptorType);
277283

278284
// always call with the same parameters, otherwise this turns into a mess with the non invalid_address gaps
@@ -299,17 +305,19 @@ class SubAllocatedDescriptorSet : public core::IReferenceCounted
299305

300306
auto allocator = getBindingAllocator(binding);
301307
if (allocator)
302-
for (size_type i=0; i<count; i++)
303308
{
304-
if (addr[i]==AddressAllocator::invalid_address)
305-
continue;
306-
307-
allocator->free_addr(addr[i],1);
308-
outNullify->dstSet = m_descriptorSet.get();
309-
outNullify->binding = binding;
310-
outNullify->arrayElement = i;
311-
outNullify->count = 1;
312-
outNullify++;
309+
for (size_type i = 0; i < count; i++)
310+
{
311+
if (addr[i] == AddressAllocator::invalid_address)
312+
continue;
313+
314+
allocator->free_addr(addr[i], 1);
315+
outNullify->dstSet = m_descriptorSet.get();
316+
outNullify->binding = binding;
317+
outNullify->arrayElement = i;
318+
outNullify->count = 1;
319+
outNullify++;
320+
}
313321
}
314322
return outNullify;
315323
}
@@ -325,7 +333,7 @@ class SubAllocatedDescriptorSet : public core::IReferenceCounted
325333
auto& eventHandler = range->second.eventHandler;
326334
auto debugGuard = stAccessVerifyDebugGuard();
327335
m_totalDeferredFrees += functor.getWorstCaseCount();
328-
eventHandler.latch(futureWait,std::move(functor));
336+
eventHandler->latch(futureWait,std::move(functor));
329337
}
330338

331339
// defers based on the conservative estimation if `futureWait` needs to be waited on, if doesn't will call nullify descriiptors internally immediately
@@ -353,7 +361,7 @@ class SubAllocatedDescriptorSet : public core::IReferenceCounted
353361
for (uint32_t i = 0; i < m_allocatableRanges.size(); i++)
354362
{
355363
auto& it = m_allocatableRanges[i];
356-
frees += it.eventHandler.poll(outNulls).eventsLeft;
364+
frees += it.eventHandler->poll(outNulls).eventsLeft;
357365
// TODO: this could be optimized to be put outside the loop
358366
m_logicalDevice->nullifyDescriptors({nulls.data(),outNulls}, it.descriptorType);
359367
}

src/nbl/video/IGPUDescriptorSet.cpp

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

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

131-
for (uint32_t t = 0; t < static_cast<uint32_t>(asset::IDescriptor::E_TYPE::ET_COUNT); ++t)
132-
{
133-
const auto type = static_cast<asset::IDescriptor::E_TYPE>(t);
131+
auto* dstDescriptors = drop.dstSet->getDescriptors(type, drop.binding);
132+
auto* dstSamplers = drop.dstSet->getMutableSamplers(drop.binding);
133+
assert(dstDescriptors);
134+
// Samplers are only used in the combined image sampler descriptors
135+
if (type == asset::IDescriptor::E_TYPE::ET_COMBINED_IMAGE_SAMPLER)
136+
assert(dstSamplers);
134137

135-
auto* dstDescriptors = drop.dstSet->getDescriptors(type, drop.binding);
136-
auto* dstSamplers = drop.dstSet->getMutableSamplers(drop.binding);
137-
assert(dstDescriptors);
138-
assert(dstSamplers);
138+
if (dstDescriptors)
139+
for (uint32_t i = 0; i < drop.count; i++)
140+
dstDescriptors[drop.arrayElement + i] = nullptr;
139141

140-
if (dstDescriptors)
141-
for (uint32_t i = 0; i < drop.count; i++)
142-
dstDescriptors[drop.arrayElement + i] = nullptr;
142+
if (dstSamplers)
143+
for (uint32_t i = 0; i < drop.count; i++)
144+
dstSamplers[drop.arrayElement + i] = nullptr;
143145

144-
if (dstSamplers)
145-
for (uint32_t i = 0; i < drop.count; i++)
146-
dstSamplers[drop.arrayElement + i] = nullptr;
147-
}
148146
// we only increment the version to detect UPDATE-AFTER-BIND and automagically invalidate descriptor sets
149147
// so, only if we do the path that writes descriptors, do we want to increment version
150148
if (getOriginDevice()->getEnabledFeatures().nullDescriptor)

src/nbl/video/ILogicalDevice.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ bool ILogicalDevice::nullifyDescriptors(const std::span<const IGPUDescriptorSet:
452452
if (!ds || !ds->wasCreatedBy(this))
453453
return false;
454454

455-
ds->dropDescriptors(drop);
455+
ds->dropDescriptors(drop, descriptorType);
456456
}
457457

458458
nullifyDescriptors_impl(dropDescriptors, descriptorType);

0 commit comments

Comments
 (0)