finish prototype implementation, fix a few bugs

Author: devshgraphicsprogramming

include/nbl/video/ILogicalDevice.h

@@ -275,7 +275,7 @@ class NBL_API2 ILogicalDevice : public core::IReferenceCounted, public IDeviceMe
             IDeviceMemoryBacked::SMemoryBinding binding = {};
         };
         //! The counterpart of @see bindBufferMemory for images
-        inline bool bindImageMemory(uint32_t count, const SBindImageMemoryInfo* pBindInfos)
+        [[deprecated]] inline bool bindImageMemory(uint32_t count, const SBindImageMemoryInfo* pBindInfos)
         {
             for (auto i=0u; i<count; i++)
             {
@@ -290,6 +290,10 @@ class NBL_API2 ILogicalDevice : public core::IReferenceCounted, public IDeviceMe
             }
             return bindImageMemory_impl(count,pBindInfos);
         }
+        inline bool bindImageMemory(const std::span<const SBindImageMemoryInfo> bindInfos)
+        {
+            return bindImageMemory(bindInfos.size(),bindInfos.data());
+        }
 
         //! Descriptor Creation
         // Buffer (@see ICPUBuffer)

include/nbl/video/utilities/CResizableSurface.h

@@ -16,57 +16,48 @@ class NBL_API2 IResizableSurface : public ISimpleManagedSurface
 		class ICallback : public ISimpleManagedSurface::ICallback
 		{
 			protected:
-				friend class IResizableSurface;
-				// `recreator` owns the `ISurface`, which refcounts the `IWindow` which refcounts the callback, so fumb pointer to avoid cycles 
-				inline void setSwapchainRecreator(IResizableSurface* recreator) {m_recreator = recreator;}
-
 				// remember to call this when overriding it further down!
 				inline virtual bool onWindowResized_impl(uint32_t w, uint32_t h) override
 				{
-					m_recreator->explicitRecreateSwapchain(w,h);
+					if (m_recreator)
+						m_recreator->explicitRecreateSwapchain(w,h);
 					return true;
 				}
 
-				IResizableSurface* m_recreator;
+			private:
+				friend class IResizableSurface;
+				// `recreator` owns the `ISurface`, which refcounts the `IWindow` which refcounts the callback, so fumb pointer to avoid cycles 
+				inline void setSwapchainRecreator(IResizableSurface* recreator) { m_recreator = recreator; }
+
+				IResizableSurface* m_recreator = nullptr;
 		};
 
 		//
 		class NBL_API2 ISwapchainResources : public core::IReferenceCounted, public ISimpleManagedSurface::ISwapchainResources
 		{
 			protected:
-				// remember to call in the derived class on all of these
-				virtual inline void invalidate_impl() override
-				{
-					std::fill(cmdbufs.begin(),cmdbufs.end(),nullptr);
-				}
-				virtual inline bool onCreateSwapchain_impl(const uint8_t qFam) override
-				{
-					auto device = const_cast<ILogicalDevice*>(swapchain->getOriginDevice());
-
-					// transient commandbuffer and pool to perform the blits or copies to SC images
-					auto pool = device->createCommandPool(qFam,IGPUCommandPool::CREATE_FLAGS::RESET_COMMAND_BUFFER_BIT);
-					if (!pool || !pool->createCommandBuffers(IGPUCommandPool::BUFFER_LEVEL::PRIMARY,{cmdbufs.data(),swapchain->getImageCount()}))
-						return false;
-				
-					return true;
-				}
-
-				// The `cmdbuf` is already begun when given to the callback, and will be ended inside
-				// Returns what stage the submit signal semaphore should signal from for the presentation to wait on 
-				virtual asset::PIPELINE_STAGE_FLAGS tripleBufferPresent(IGPUCommandBuffer* cmdbuf, IGPUImage* source) = 0;
-
-			private:
 				friend class IResizableSurface;
 
-				std::array<core::smart_refctd_ptr<IGPUCommandBuffer>,ISwapchain::MaxImages> cmdbufs = {};
+				// The `cmdbuf` is already begun when given to the callback, and will be ended outside.
+				// Returns what stage the submit signal semaphore should signal from for the presentation to wait on.
+				// User is responsible for transitioning both images' layouts, acquiring ownership etc.
+				virtual asset::PIPELINE_STAGE_FLAGS tripleBufferPresent(IGPUCommandBuffer* cmdbuf, IGPUImage* source, const uint8_t dstIx) = 0;
 		};
 
+		//
+		inline CThreadSafeQueueAdapter* pickQueue(ILogicalDevice* device) const override final
+		{
+			const auto fam = pickQueueFamily(device);
+			return device->getThreadSafeQueue(fam,device->getQueueCount(fam)-1);
+		}
+
 		struct SPresentInfo
 		{
 			inline operator bool() const {return source;}
 
 			IGPUImage* source = nullptr;
 			// TODO: add sourceRegion
+			// only allow waiting for one semaphore, because there's only one source to present!
 			IQueue::SSubmitInfo::SSemaphoreInfo wait;
 			core::IReferenceCounted* frameResources;
 		};		
@@ -101,31 +92,53 @@ class NBL_API2 IResizableSurface : public ISimpleManagedSurface
 
 			// The blit enqueue operations are fast enough to be done under a mutex, this is safer on some platforms
 			// You need to "race to present" to avoid a flicker
-			return present({.source=m_lastPresentSource,.wait=m_lastPresentWait,.frameResources=nullptr});
+			return present_impl({.source=m_lastPresentSource,.wait=m_lastPresentWait,.frameResources=nullptr});
 		}
 
 	protected:
-		inline IResizableSurface(core::smart_refctd_ptr<ISurface>&& _surface, ICallback* _cb) : ISimpleManagedSurface(std::move(_surface),_cb)
-		{
-			_cb->setSwapchainRecreator(this);
-		}
+		using ISimpleManagedSurface::ISimpleManagedSurface;
 		virtual inline ~IResizableSurface()
 		{
 			static_cast<ICallback*>(m_cb)->setSwapchainRecreator(nullptr);
 		}
 
 		//
-		inline CThreadSafeQueueAdapter* pickQueue(ILogicalDevice* device) const override final
+		inline void deinit_impl() override final
 		{
-			const auto fam = pickQueueFamily(device);
-			return device->getThreadSafeQueue(fam,device->getQueueCount(fam)-1);
+			// stop any calls into explicit resizes
+			std::unique_lock guard(m_swapchainResourcesMutex);
+			static_cast<ICallback*>(m_cb)->setSwapchainRecreator(nullptr);
+
+			m_lastPresentWait = {};
+			m_lastPresentSource = {};
+			m_lastPresentSemaphore = nullptr;
+			m_lastPresentSourceImage = nullptr;
+
+			if (m_blitSemaphore)
+			{
+				auto device = const_cast<ILogicalDevice*>(m_blitSemaphore->getOriginDevice());
+				const ISemaphore::SWaitInfo info[1] = {{
+					.semaphore = m_blitSemaphore.get(), .value = getAcquireCount()
+				}};
+				device->blockForSemaphores(info);
+			}
+
+			std::fill(m_cmdbufs.begin(),m_cmdbufs.end(),nullptr);
+			m_blitSemaphore = nullptr;
 		}
 
 		//
-		inline bool init_impl(CThreadSafeQueueAdapter* queue, const ISwapchain::SSharedCreationParams& sharedParams) override final
+		inline bool init_impl(const ISwapchain::SSharedCreationParams& sharedParams) override final
 		{
+			// swapchain callback already deinitialized, so no mutex needed here
+
+			auto queue = getAssignedQueue();
 			auto device = const_cast<ILogicalDevice*>(queue->getOriginDevice());
 
+			m_sharedParams = sharedParams;
+			if (!m_sharedParams.deduce(device->getPhysicalDevice(),getSurface()))
+				return false;
+
 			// want to keep using the same semaphore throughout the lifetime to not run into sync issues
 			if (!m_blitSemaphore)
 			{
@@ -134,11 +147,16 @@ class NBL_API2 IResizableSurface : public ISimpleManagedSurface
 					return false;
 			}
 
-			m_sharedParams = sharedParams;
-			if (!m_sharedParams.deduce(device->getPhysicalDevice(),getSurface()))
+			// transient commandbuffer and pool to perform the blits or copies to SC images
+			auto pool = device->createCommandPool(queue->getFamilyIndex(),IGPUCommandPool::CREATE_FLAGS::RESET_COMMAND_BUFFER_BIT);
+			if (!pool || !pool->createCommandBuffers(IGPUCommandPool::BUFFER_LEVEL::PRIMARY,{m_cmdbufs.data(),getMaxFramesInFlight()}))
 				return false;
 
-			return createSwapchainResources();
+			if (!createSwapchainResources())
+				return false;
+			
+			static_cast<ICallback*>(m_cb)->setSwapchainRecreator(this);
+			return true;
 		}
 
 		//
@@ -205,12 +223,17 @@ class NBL_API2 IResizableSurface : public ISimpleManagedSurface
 		//
 		inline bool present_impl(const SPresentInfo& presentInfo)
 		{
-			if (!presentInfo)
+			// irrecoverable or bad input
+			if (!presentInfo || !getSwapchainResources())
 				return false;
 
 			// delayed init of our swapchain
-			if (!getSwapchainResources() && !recreateSwapchain())
+			if (getSwapchainResources()->getStatus()!=ISwapchainResources::STATUS::USABLE && !recreateSwapchain())
 				return false;
+
+			// now pointer won't change until we get out from under the lock
+			auto swapchainResources = static_cast<ISwapchainResources*>(getSwapchainResources());
+			assert(swapchainResources);
 
 			const uint8_t imageIx = acquireNextImage();
 			if (imageIx==ISwapchain::MaxImages)
@@ -228,40 +251,40 @@ class NBL_API2 IResizableSurface : public ISimpleManagedSurface
 				}
 			};
 			m_lastPresentSourceImage = core::smart_refctd_ptr<IGPUImage>(presentInfo.source);
+			m_lastPresentSemaphore = core::smart_refctd_ptr<ISemaphore>(presentInfo.wait.semaphore);
 			m_lastPresentSource = presentInfo.source;
 			m_lastPresentWait = presentInfo.wait;
-
-			// now pointer won't change until we get out from under the lock
-			auto swapchainResources = static_cast<ISwapchainResources*>(getSwapchainResources());
-			assert(swapchainResources);
 
 			//
 			auto queue = getAssignedQueue();
 			auto device = const_cast<ILogicalDevice*>(queue->getOriginDevice());
 
 			// need to wait before resetting a commandbuffer
-			const auto scImageCount = swapchainResources->getSwapchain()->getImageCount();
-			if (acquireCount>scImageCount)
+			const auto maxFramesInFlight = getMaxFramesInFlight();
+			if (acquireCount>maxFramesInFlight)
 			{
 				const ISemaphore::SWaitInfo cmdbufDonePending[1] = {
 					{ 
 						.semaphore = m_blitSemaphore.get(),
-						.value = acquireCount-scImageCount
+						.value = acquireCount-maxFramesInFlight
 					}
 				};
 				device->blockForSemaphores(cmdbufDonePending);
 			}
 
-			const auto cmdBufIx = acquireCount%swapchainResources->getSwapchain()->getImageCount();
-			auto cmdbuf = swapchainResources->cmdbufs[cmdBufIx].get();
+
+			// Maybe tie the cmbufs to the Managed Surface instead?
+			const auto cmdBufIx = acquireCount%maxFramesInFlight;
+			auto cmdbuf = m_cmdbufs[cmdBufIx].get();
 
 			willBlit &= cmdbuf->begin(IGPUCommandBuffer::USAGE::ONE_TIME_SUBMIT_BIT);
 			// now enqueue the mini-blit
 			const IQueue::SSubmitInfo::SSemaphoreInfo blitted[1] = {
 				{
 					.semaphore = m_blitSemaphore.get(),
 					.value = acquireCount,
-					.stageMask = swapchainResources->tripleBufferPresent(cmdbuf,presentInfo.source)
+					// don't need to predicate with `willBlit` because if `willBlit==false` cmdbuf not properly begun and validation will fail
+					.stageMask = swapchainResources->tripleBufferPresent(cmdbuf,presentInfo.source,imageIx)
 				}
 			};
 			willBlit &= bool(blitted[1].stageMask.value);
@@ -274,54 +297,17 @@ class NBL_API2 IResizableSurface : public ISimpleManagedSurface
 				.signalSemaphores = blitted
 			}};
 			willBlit &= queue->submit(submitInfos)==IQueue::RESULT::SUCCESS;
-#if 0
-					const IQueue::SSubmitInfo::SSemaphoreInfo readyToPresent[1] = {{
-						.semaphore = presentSemaphore,
-						.value = presentValue,
-						.stageMask = raceToPresent_impl(lastSource,cmdbuf)
-					}};
-					const std::span<const IQueue::SSubmitInfo::SSemaphoreInfo> waitSemaphores = {&wait,1};
-
-					bool willBlit = false;
-					// successful blit command enqueue if we have to wait on something
-					if (readyToPresent->stageMask && cmdbuf->end())
-					{
-						const IQueue::SSubmitInfo::SCommandBufferInfo commandBuffers[1] = {{
-							.cmdbuf=cmdbuf
-						}};
-						const IQueue::SSubmitInfo submits[1] = {{
-							.waitSemaphores = waitSemaphores,
-							.commandBuffers = commandBuffers,
-							.signalSemaphores = readyToPresent
-						}};
-						willBlit = queue->submit(submits)==IQueue::RESULT::SUCCESS;
-					}
-					
-					// need to present either way
-					if (willBlit)
-					{
-						switch (swapchain->present({.queue=queue,.imgIndex=acquiredIx,.waitSemaphores=readyToPresent},core::smart_refctd_ptr<core::IReferenceCounted>(cmdbuf)))
-						{
-							case ISwapchain::PRESENT_RESULT::SUBOPTIMAL: [[fallthrough]];
-							case ISwapchain::PRESENT_RESULT::SUCCESS:
-								// all resources can be dropped, the swapchain will hold onto them
-								return true;
-							case ISwapchain::PRESENT_RESULT::OUT_OF_DATE:
-								invalidate();
-								break;
-							default:
-								becomeIrrecoverable();
-								break;
-						}
-						// now in a weird situation where we have pending blit work on the GPU but no present and refcounting to keep the submitted cmdbuf and semaphores alive
-						const ISemaphore::SWaitInfo infos[1] = {{.semaphore=presentSemaphore,.value=presentValue}};
-						const_cast<ILogicalDevice*>(queue->getOriginDevice())->blockForSemaphores(infos);
-					}
-					else
-						swapchain->present({.queue=queue,.imgIndex=acquiredIx,.waitSemaphores=waitSemaphores});
-					return false;
-#endif
-			return ISimpleManagedSurface::present(imageIx,blitted,presentInfo.frameResources);
+
+			// handle two cases of present
+			if (willBlit)
+			{
+				auto frameResources = core::make_refctd_dynamic_array<core::smart_refctd_dynamic_array<ISwapchain::void_refctd_ptr>>(2);
+				frameResources->front() = core::smart_refctd_ptr<core::IReferenceCounted>(presentInfo.frameResources);
+				frameResources->back() = core::smart_refctd_ptr<IGPUCommandBuffer>(cmdbuf);
+				return ISimpleManagedSurface::present(imageIx,blitted,frameResources.get());
+			}
+			else
+				return ISimpleManagedSurface::present(imageIx,{waitSemaphores+1,1},presentInfo.frameResources);
 		}
 
 		// Assume it will execute under a mutex
@@ -331,15 +317,20 @@ class NBL_API2 IResizableSurface : public ISimpleManagedSurface
 		ISwapchain::SSharedCreationParams m_sharedParams = {};
 
 	private:
+		// Have to use a second semaphore to make acquire-present pairs independent of each other, also because there can be no ordering ensured between present->acquire
 		core::smart_refctd_ptr<ISemaphore> m_blitSemaphore;
-		// used to protect access to swapchain resources during acquire, present and immediateBlit
+		// Command Buffers for blitting/copying to 
+		std::array<core::smart_refctd_ptr<IGPUCommandBuffer>,ISwapchain::MaxImages> m_cmdbufs = {};
+
+		// used to protect access to swapchain resources during present and recreateExplicit
 		std::mutex m_swapchainResourcesMutex;
 		// Why do we delay the swapchain recreate present until the rendering of the most recent present source is done? Couldn't we present whatever latest Triple Buffer is done?
 		// No because there can be presents enqueued whose wait semaphores have not signalled yet, meaning there could be images presented in the future.
 		// Unless you like your frames to go backwards in time in a special "rewind glitch" you need to blit the frame that has not been presented yet or is the same as most recently enqueued.
+		IQueue::SSubmitInfo::SSemaphoreInfo m_lastPresentWait = {};
 		decltype(SPresentInfo::source) m_lastPresentSource = {};
+		core::smart_refctd_ptr<ISemaphore> m_lastPresentSemaphore = {};
 		core::smart_refctd_ptr<IGPUImage> m_lastPresentSourceImage = {};
-		IQueue::SSubmitInfo::SSemaphoreInfo m_lastPresentWait = {};
 };
 
 // The use of this class is supposed to be externally synchronized