Browse Source

Vulkan: Fixing an issue where waiting on a queue didn't necessarily guarantee fences of submitted command buffers were signaled

BearishSun 9 years ago
parent
commit
10523f6750

+ 6 - 2
Source/BansheeVulkanRenderAPI/Include/BsVulkanCommandBuffer.h

@@ -177,8 +177,12 @@ namespace bs { namespace ct
 		/** Returns true if the command buffer is currently recording a render pass. */
 		/** Returns true if the command buffer is currently recording a render pass. */
 		bool isInRenderPass() const { return mState == State::RecordingRenderPass; }
 		bool isInRenderPass() const { return mState == State::RecordingRenderPass; }
 
 
-		/** Checks the internal fence if done executing. */
-		bool checkFenceStatus() const;
+		/** 
+		 * Checks the internal fence if done executing. 
+		 * 
+		 * @param[in]	block	If true, the system will block until the fence is signaled.
+		 */
+		bool checkFenceStatus(bool block) const;
 
 
 		/** 
 		/** 
 		 * Resets the command buffer back in Ready state. Should be called when command buffer is done executing on a 
 		 * Resets the command buffer back in Ready state. Should be called when command buffer is done executing on a 

+ 1 - 1
Source/BansheeVulkanRenderAPI/Include/BsVulkanCommandBufferManager.h

@@ -111,7 +111,7 @@ namespace bs { namespace ct
 		 * Checks if any of the active command buffers finished executing on the device and updates their states 
 		 * Checks if any of the active command buffers finished executing on the device and updates their states 
 		 * accordingly. 
 		 * accordingly. 
 		 */
 		 */
-		void refreshStates(UINT32 deviceIdx);
+		void refreshStates(UINT32 deviceIdx, bool forceWait = false);
 
 
 		/** 
 		/** 
 		 * Returns an command buffer that can be used for executing transfer operations on the specified queue. 
 		 * Returns an command buffer that can be used for executing transfer operations on the specified queue. 

+ 2 - 1
Source/BansheeVulkanRenderAPI/Include/BsVulkanQueue.h

@@ -64,10 +64,11 @@ namespace bs { namespace ct
 		 * Checks if any of the active command buffers finished executing on the queue and updates their states 
 		 * Checks if any of the active command buffers finished executing on the queue and updates their states 
 		 * accordingly. 
 		 * accordingly. 
 		 * 
 		 * 
+		 * @param[in]	forceWait	Set to true if the system should wait until all command buffers finish executing.
 		 * @param[in]	queueEmpty	Set to true if the caller guarantees the queue will be empty (e.g. on shutdown). This
 		 * @param[in]	queueEmpty	Set to true if the caller guarantees the queue will be empty (e.g. on shutdown). This
 		 *							allows the system to free all needed resources.
 		 *							allows the system to free all needed resources.
 		 */
 		 */
-		void refreshStates(bool queueEmpty = false);
+		void refreshStates(bool forceWait, bool queueEmpty = false);
 
 
 		/** Returns the last command buffer that was submitted on this queue. */
 		/** Returns the last command buffer that was submitted on this queue. */
 		VulkanCmdBuffer* getLastCommandBuffer() const { return mLastCommandBuffer; }
 		VulkanCmdBuffer* getLastCommandBuffer() const { return mLastCommandBuffer; }

+ 3 - 3
Source/BansheeVulkanRenderAPI/Source/BsVulkanCommandBuffer.cpp

@@ -702,10 +702,10 @@ namespace bs { namespace ct
 		mSwapChains.clear();
 		mSwapChains.clear();
 	}
 	}
 
 
-	bool VulkanCmdBuffer::checkFenceStatus() const
+	bool VulkanCmdBuffer::checkFenceStatus(bool block) const
 	{
 	{
-		VkResult result = vkGetFenceStatus(mDevice.getLogical(), mFence);
-		assert(result == VK_SUCCESS || result == VK_NOT_READY);
+		VkResult result = vkWaitForFences(mDevice.getLogical(), 1, &mFence, true, block ? 1'000'000'000 : 0);
+		assert(result == VK_SUCCESS || result == VK_TIMEOUT);
 
 
 		return result == VK_SUCCESS;
 		return result == VK_SUCCESS;
 	}
 	}

+ 5 - 3
Source/BansheeVulkanRenderAPI/Source/BsVulkanCommandBufferManager.cpp

@@ -142,7 +142,9 @@ namespace bs { namespace ct
 		if (wait)
 		if (wait)
 		{
 		{
 			mQueue->waitIdle();
 			mQueue->waitIdle();
-			gVulkanCBManager().refreshStates(mDevice->getIndex());
+			gVulkanCBManager().refreshStates(mDevice->getIndex(), true);
+
+			assert(!mCB->isSubmitted());
 		}
 		}
 
 
 		mCB = nullptr;
 		mCB = nullptr;
@@ -239,7 +241,7 @@ namespace bs { namespace ct
 		}
 		}
 	}
 	}
 
 
-	void VulkanCommandBufferManager::refreshStates(UINT32 deviceIdx)
+	void VulkanCommandBufferManager::refreshStates(UINT32 deviceIdx, bool forceWait)
 	{
 	{
 		SPtr<VulkanDevice> device = mRapi._getDevice(deviceIdx);
 		SPtr<VulkanDevice> device = mRapi._getDevice(deviceIdx);
 
 
@@ -249,7 +251,7 @@ namespace bs { namespace ct
 			for (UINT32 j = 0; j < numQueues; j++)
 			for (UINT32 j = 0; j < numQueues; j++)
 			{
 			{
 				VulkanQueue* queue = device->getQueue((GpuQueueType)i, j);
 				VulkanQueue* queue = device->getQueue((GpuQueueType)i, j);
-				queue->refreshStates();
+				queue->refreshStates(forceWait, false);
 			}
 			}
 		}
 		}
 	}
 	}

+ 1 - 1
Source/BansheeVulkanRenderAPI/Source/BsVulkanDevice.cpp

@@ -125,7 +125,7 @@ namespace bs { namespace ct
 			UINT32 numQueues = (UINT32)mQueueInfos[i].queues.size();
 			UINT32 numQueues = (UINT32)mQueueInfos[i].queues.size();
 			for (UINT32 j = 0; j < numQueues; j++)
 			for (UINT32 j = 0; j < numQueues; j++)
 			{
 			{
-				mQueueInfos[i].queues[j]->refreshStates(true);
+				mQueueInfos[i].queues[j]->refreshStates(true, true);
 				bs_delete(mQueueInfos[i].queues[j]);
 				bs_delete(mQueueInfos[i].queues[j]);
 			}
 			}
 		}
 		}

+ 6 - 3
Source/BansheeVulkanRenderAPI/Source/BsVulkanQueue.cpp

@@ -185,7 +185,7 @@ namespace bs { namespace ct
 		assert(result == VK_SUCCESS);
 		assert(result == VK_SUCCESS);
 	}
 	}
 
 
-	void VulkanQueue::refreshStates(bool queueEmpty)
+	void VulkanQueue::refreshStates(bool forceWait, bool queueEmpty)
 	{
 	{
 		UINT32 lastFinishedSubmission = 0;
 		UINT32 lastFinishedSubmission = 0;
 
 
@@ -199,9 +199,12 @@ namespace bs { namespace ct
 				continue;
 				continue;
 			}
 			}
 
 
-			if(!cmdBuffer->checkFenceStatus())
+			if (!cmdBuffer->checkFenceStatus(forceWait))
+			{
+				assert(!forceWait);
 				break; // No chance of any later CBs of being done either
 				break; // No chance of any later CBs of being done either
-				
+			}
+
 			lastFinishedSubmission = iter->submitIdx;
 			lastFinishedSubmission = iter->submitIdx;
 			++iter;
 			++iter;
 		}
 		}