Refactor the MultiTimelineEventHandlerST to not refcount the device (so it can be used in the IQueue)

Author: devshgraphicsprogramming

include/nbl/video/TimelineEventHandlers.h

@@ -34,13 +34,10 @@ class TimelineEventHandlerBase : core::Unmovable, core::Uncopyable
         core::smart_refctd_ptr<const ISemaphore> m_sema;
 };
 
-template<typename Functor>
-class MultiTimelineEventHandlerST;
-
 // Could be made MT and relatively lockless, if only had a good lock-few circular buffer impl
 // Not sure its worth the effort as anything using this will probably need to be lockful to be MT
 template<typename Functor>
-class TimelineEventHandlerST final : TimelineEventHandlerBase
+class TimelineEventHandlerST final : public TimelineEventHandlerBase
 {
     public:
         // Theoretically could make a factory function cause passing a null semaphore is invalid, but counting on users to be relatively intelligent.
@@ -160,7 +157,8 @@ class TimelineEventHandlerST final : TimelineEventHandlerBase
         inline void abortAll(Args&&... args) {abortOldest(~0ull,std::forward<Args>(args)...);}
 
     private:
-        friend MultiTimelineEventHandlerST<Functor>;
+        // To get access to almost everything
+        template<typename Functor_, bool RefcountTheDevice> friend class MultiTimelineEventHandlerST;
 
         struct FunctorValuePair
         {
@@ -227,21 +225,53 @@ class TimelineEventHandlerST final : TimelineEventHandlerBase
         }
 };
 
-//
-template<typename Functor>
+// `RefcountTheDevice` should be false for any "internal user" of the Handler inside the Logical Device, such as the IQueue to avoid circular refs
+/*
+template<bool RefcountTheDevice>
+class MultiTimelineEventHandlerBase : core::Unmovable, core::Uncopyable
+{
+    public:
+        using device_ptr_t = std::conditional_t<RefcountTheDevice,core::smart_refctd_ptr<ILogicalDevice>,ILogicalDevice*>;
+        inline MultiTimelineEventHandlerBase(device_ptr_t&& device) : m_device(std::move(device)) {}
+
+        inline ILogicalDevice* getLogicalDevice() const {return m_device.get();}
+
+    protected:
+        template<typename Functor>
+        static inline auto getGreatestSignal(const TimelineEventHandlerST<Functor>* handler) {return handler->m_greatestSignal;}
+        template<typename Functor>
+        static inline auto getEmpty(const TimelineEventHandlerST<Functor>* handler) {return handler->m_cb.empty();}
+
+        device_ptr_t m_device;
+};
+*/
+template<typename Functor, bool RefcountTheDevice=true>
 class MultiTimelineEventHandlerST final : core::Unmovable, core::Uncopyable
 {
     public:
         using TimelineEventHandler = TimelineEventHandlerST<Functor>;
+        using device_ptr_t = std::conditional_t<RefcountTheDevice,core::smart_refctd_ptr<ILogicalDevice>,ILogicalDevice*>;
 
-        inline MultiTimelineEventHandlerST(core::smart_refctd_ptr<ILogicalDevice>&& device) : m_device(std::move(device)) {}
+        inline MultiTimelineEventHandlerST(ILogicalDevice* device) : m_device(device) {}
+        MultiTimelineEventHandlerST(const MultiTimelineEventHandlerST&) = delete;
         inline ~MultiTimelineEventHandlerST()
         {
             clear();
         }
 
-        inline ILogicalDevice* getLogicalDevice() const {return m_device.get();}
+        //
+        MultiTimelineEventHandlerST& operator=(const MultiTimelineEventHandlerST&) = delete;
+        
+        //
+        inline ILogicalDevice* getLogicalDevice() const
+        {
+            if constexpr (RefcountTheDevice)
+                return m_device.get();
+            else
+                return m_device;
+        }
 
+        //
         inline const auto& getTimelines() const {return m_timelines;}
 
         // all the members are counteparts of the single timeline version
@@ -258,7 +288,7 @@ class MultiTimelineEventHandlerST final : core::Unmovable, core::Uncopyable
             auto found = m_timelines.find(futureWait.semaphore);
             if (found==m_timelines.end())
             {
-                if (futureWait.semaphore->getOriginDevice()!=m_device.get())
+                if (futureWait.semaphore->getOriginDevice()!=getLogicalDevice())
                     return false;
                 STimeline newTimeline = {
                     .handler = new TimelineEventHandler(core::smart_refctd_ptr<const ISemaphore>(futureWait.semaphore)),
@@ -441,7 +471,7 @@ class MultiTimelineEventHandlerST final : core::Unmovable, core::Uncopyable
 
         container_t m_timelines;
         core::vector<ISemaphore::SWaitInfo> m_scratchWaitInfos;
-        core::smart_refctd_ptr<ILogicalDevice> m_device;
+        device_ptr_t m_device;
 };
 
 }

include/nbl/video/alloc/CAsyncSingleBufferSubAllocator.h

@@ -118,7 +118,7 @@ class CAsyncSingleBufferSubAllocator
         // perfect forward ctor to `CSingleBufferSubAllocator`
         template<typename... Args>
         inline CAsyncSingleBufferSubAllocator(Args&&... args) : m_composed(std::forward<Args>(args)...),
-            deferredFrees(core::smart_refctd_ptr<ILogicalDevice>(const_cast<ILogicalDevice*>(m_composed.getBuffer()->getOriginDevice()))) {}
+            deferredFrees(const_cast<ILogicalDevice*>(m_composed.getBuffer()->getOriginDevice())) {}
         virtual ~CAsyncSingleBufferSubAllocator() {}
 
 

include/nbl/video/utilities/ICommandPoolCache.h

@@ -119,7 +119,7 @@ class ICommandPoolCache : public core::IReferenceCounted
 	protected:
 		friend class DeferredCommandPoolResetter;
 		inline ICommandPoolCache(core::smart_refctd_ptr<ILogicalDevice>&& device, core::smart_refctd_ptr<IGPUCommandPool>* cache, const uint32_t capacity, void* reserved) :
-			m_cache(cache),	m_reserved(malloc(CommandPoolAllocator::reserved_size(1u,capacity,1u))), m_cmdPoolAllocator(m_reserved,0u,0u,1u,capacity,1u), m_deferredResets(std::move(device)) {}
+			m_cache(cache),	m_reserved(malloc(CommandPoolAllocator::reserved_size(1u,capacity,1u))), m_cmdPoolAllocator(m_reserved,0u,0u,1u,capacity,1u), m_deferredResets(device.get()) {}
 		inline virtual ~ICommandPoolCache()
 		{
 			// normally the dtor would do this, but we need all the events to run before we delete the storage they reference

include/nbl/video/utilities/IDescriptorSetCache.h

@@ -137,7 +137,7 @@ class IDescriptorSetCache : public core::IReferenceCounted
 			void* const reserved
 		) : m_descPool(std::move(pool)), m_canonicalLayout(std::move(canonicalLayout)), m_cache(cache),
 			m_reserved(reserved), m_setAllocator(m_reserved,0u,0u,1u,m_descPool->getCapacity(),1u),
-			m_deferredReclaims(core::smart_refctd_ptr<ILogicalDevice>(const_cast<ILogicalDevice*>(m_descPool->getOriginDevice())))
+			m_deferredReclaims(const_cast<ILogicalDevice*>(m_descPool->getOriginDevice()))
 		{}
 		virtual inline ~IDescriptorSetCache()
 		{

src/nbl/video/ISwapchain.cpp

@@ -28,10 +28,7 @@ ISwapchain::ISwapchain(core::smart_refctd_ptr<const ILogicalDevice>&& dev, SCrea
     m_params.queueFamilyIndices = {m_queueFamilies.data(),m_params.queueFamilyIndices.size()};
 
     for (auto i=0; i<m_imageCount; i++)
-    {
-        auto device = core::smart_refctd_ptr<ILogicalDevice>(const_cast<ILogicalDevice*>(getOriginDevice()));
-        m_frameResources[i] = std::make_unique<MultiTimelineEventHandlerST<DeferredFrameResourceDrop>>(std::move(device));
-    }
+        m_frameResources[i] = std::make_unique<MultiTimelineEventHandlerST<DeferredFrameResourceDrop>>(const_cast<ILogicalDevice*>(getOriginDevice()));
 } 
 
 }