Browse Source

Fixed Vulkan warning: VkSemaphore is being signaled by VkQueue but it may still be in use by VkSwapchainKHR (#1630)

* Fix stack corruption in Samples when Trace-ing messages of 1023 characters or longer
Jorrit Rouwe 2 months ago
parent
commit
084b8957db

+ 1 - 0
Docs/ReleaseNotes.md

@@ -19,6 +19,7 @@ For breaking API changes see [this document](https://github.com/jrouwe/JoltPhysi
 * Fixed compiling in double precision and fixed issues with floating point contraction that caused unit test failures on LoongArch architecture.
 * Added an epsilon to the `CastRay` / `CastShape` early out condition to avoid dividing by a very small number and overflowing to INF. This can cause a float overflow exception.
 * Fixed Samples requiring Vulkan extension `VK_EXT_device_address_binding_report` without checking if it is available.
+* Fixed Vulkan warning in Samples: VkSemaphore is being signaled by VkQueue but it may still be in use by VkSwapchainKHR.
 
 ## v5.3.0
 

+ 2 - 1
TestFramework/Application/Application.cpp

@@ -271,7 +271,8 @@ bool Application::RenderFrame()
 		UpdateCamera(clock_delta_time);
 
 	// Start rendering
-	mRenderer->BeginFrame(mWorldCamera, GetWorldScale());
+	if (!mRenderer->BeginFrame(mWorldCamera, GetWorldScale()))
+		return true;
 
 	// Draw from light
 	static_cast<DebugRendererImp *>(mDebugRenderer)->DrawShadowPass();

+ 3 - 1
TestFramework/Renderer/DX12/RendererDX12.cpp

@@ -363,7 +363,7 @@ void RendererDX12::OnWindowResize()
 	CreateDepthBuffer();
 }
 
-void RendererDX12::BeginFrame(const CameraState &inCamera, float inWorldScale)
+bool RendererDX12::BeginFrame(const CameraState &inCamera, float inWorldScale)
 {
 	JPH_PROFILE_FUNCTION();
 
@@ -420,6 +420,8 @@ void RendererDX12::BeginFrame(const CameraState &inCamera, float inWorldScale)
 
 	// Start drawing the shadow pass
 	mShadowMap->SetAsRenderTarget(true);
+
+	return true;
 }
 
 void RendererDX12::EndShadowPass()

+ 1 - 1
TestFramework/Renderer/DX12/RendererDX12.h

@@ -20,7 +20,7 @@ public:
 
 	// See: Renderer
 	virtual void					Initialize(ApplicationWindow *inWindow) override;
-	virtual void					BeginFrame(const CameraState &inCamera, float inWorldScale) override;
+	virtual bool					BeginFrame(const CameraState &inCamera, float inWorldScale) override;
 	virtual void					EndShadowPass() override;
 	virtual void					EndFrame() override;
 	virtual void					SetProjectionMode() override;

+ 1 - 1
TestFramework/Renderer/MTL/RendererMTL.h

@@ -17,7 +17,7 @@ public:
 	
 	// See: Renderer
 	virtual void					Initialize(ApplicationWindow *inWindow) override;
-	virtual void					BeginFrame(const CameraState &inCamera, float inWorldScale) override;
+	virtual bool					BeginFrame(const CameraState &inCamera, float inWorldScale) override;
 	virtual void					EndShadowPass() override;
 	virtual void					EndFrame() override;
 	virtual void					SetProjectionMode() override;

+ 3 - 1
TestFramework/Renderer/MTL/RendererMTL.mm

@@ -52,7 +52,7 @@ void RendererMTL::Initialize(ApplicationWindow *inWindow)
 	mCommandQueue = [device newCommandQueue];
 }
 
-void RendererMTL::BeginFrame(const CameraState &inCamera, float inWorldScale)
+bool RendererMTL::BeginFrame(const CameraState &inCamera, float inWorldScale)
 {
 	JPH_PROFILE_FUNCTION();
 
@@ -78,6 +78,8 @@ void RendererMTL::BeginFrame(const CameraState &inCamera, float inWorldScale)
 
 	// Start with projection mode
 	SetProjectionMode();
+
+	return true;
 }
 
 void RendererMTL::EndShadowPass()

+ 3 - 1
TestFramework/Renderer/Renderer.cpp

@@ -27,7 +27,7 @@ static Mat44 sPerspectiveInfiniteReverseZ(float inFovY, float inAspect, float in
 	return Mat44(Vec4(width, 0.0f, 0.0f, 0.0f), Vec4(0.0f, inYSign * height, 0.0f, 0.0f), Vec4(0.0f, 0.0f, 0.0f, -1.0f), Vec4(0.0f, 0.0f, inNear, 0.0f));
 }
 
-void Renderer::BeginFrame(const CameraState &inCamera, float inWorldScale)
+bool Renderer::BeginFrame(const CameraState &inCamera, float inWorldScale)
 {
 	// Mark that we're in the frame
 	JPH_ASSERT(!mInFrame);
@@ -77,6 +77,8 @@ void Renderer::BeginFrame(const CameraState &inCamera, float inWorldScale)
 	// Set constants for pixel shader
 	mPSBuffer.mCameraPos = Vec4(cam_pos, 0);
 	mPSBuffer.mLightPos = Vec4(light_pos, 0);
+
+	return true;
 }
 
 void Renderer::EndFrame()

+ 3 - 3
TestFramework/Renderer/Renderer.h

@@ -39,7 +39,7 @@ public:
 	virtual void					Initialize(ApplicationWindow *inWindow);
 
 	/// Start / end drawing a frame
-	virtual void					BeginFrame(const CameraState &inCamera, float inWorldScale);
+	virtual bool					BeginFrame(const CameraState &inCamera, float inWorldScale);
 	virtual void					EndShadowPass() = 0;
 	virtual void					EndFrame();
 
@@ -80,10 +80,10 @@ public:
 	const Frustum &					GetLightFrustum() const				{ JPH_ASSERT(mInFrame); return mLightFrustum; }
 
 	/// How many frames our pipeline is
-	static const uint				cFrameCount = 2;
+	inline static const uint		cFrameCount = 2;
 
 	/// Size of the shadow map will be cShadowMapSize x cShadowMapSize pixels
-	static const uint				cShadowMapSize = 4096;
+	inline static const uint		cShadowMapSize = 4096;
 
 	/// Which frame is currently rendering (to keep track of which buffers are free to overwrite)
 	uint							GetCurrentFrameIndex() const		{ JPH_ASSERT(mInFrame); return mFrameIndex; }

+ 74 - 28
TestFramework/Renderer/VK/RendererVK.cpp

@@ -79,11 +79,6 @@ RendererVK::~RendererVK()
 	for (VkFence fence : mInFlightFences)
 		vkDestroyFence(mDevice, fence, nullptr);
 
-	for (VkSemaphore semaphore : mRenderFinishedSemaphores) 
-		vkDestroySemaphore(mDevice, semaphore, nullptr);
-	for (VkSemaphore semaphore : mImageAvailableSemaphores) 
-		vkDestroySemaphore(mDevice, semaphore, nullptr);
-
 	vkDestroyCommandPool(mDevice, mCommandPool, nullptr);
 
 	vkDestroyPipelineLayout(mDevice, mPipelineLayout, nullptr);
@@ -376,14 +371,6 @@ void RendererVK::Initialize(ApplicationWindow *inWindow)
 	for (uint32 i = 0; i < cFrameCount; ++i)
 		FatalErrorIfFailed(vkAllocateCommandBuffers(mDevice, &command_buffer_info, &mCommandBuffers[i]));
 
-	VkSemaphoreCreateInfo semaphore_info = {};
-	semaphore_info.sType = VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO;
-	for (uint32 i = 0; i < cFrameCount; ++i)
-	{
-		FatalErrorIfFailed(vkCreateSemaphore(mDevice, &semaphore_info, nullptr, &mImageAvailableSemaphores[i]));
-		FatalErrorIfFailed(vkCreateSemaphore(mDevice, &semaphore_info, nullptr, &mRenderFinishedSemaphores[i]));
-	}
-
 	VkFenceCreateInfo fence_info = {};
 	fence_info.sType = VK_STRUCTURE_TYPE_FENCE_CREATE_INFO;
 	fence_info.flags = VK_FENCE_CREATE_SIGNALED_BIT;
@@ -670,13 +657,14 @@ void RendererVK::CreateSwapChain(VkPhysicalDevice inDevice)
 		mSwapChainExtent = { uint32(mWindow->GetWindowWidth()), uint32(mWindow->GetWindowHeight()) };
 	mSwapChainExtent.width = Clamp(mSwapChainExtent.width, capabilities.minImageExtent.width, capabilities.maxImageExtent.width);
 	mSwapChainExtent.height = Clamp(mSwapChainExtent.height, capabilities.minImageExtent.height, capabilities.maxImageExtent.height);
+	Trace("VK: Create swap chain %ux%u", mSwapChainExtent.width, mSwapChainExtent.height);
 
 	// Early out if our window has been minimized
 	if (mSwapChainExtent.width == 0 || mSwapChainExtent.height == 0)
 		return;
 
 	// Create the swap chain
-	uint32 desired_image_count = max(min(capabilities.minImageCount + 1, capabilities.maxImageCount), capabilities.minImageCount);
+	uint32 desired_image_count = max(min(cFrameCount, capabilities.maxImageCount), capabilities.minImageCount);
 	VkSwapchainCreateInfoKHR swapchain_create_info = {};
 	swapchain_create_info.sType = VK_STRUCTURE_TYPE_SWAPCHAIN_CREATE_INFO_KHR;
 	swapchain_create_info.surface = mSurface;
@@ -756,16 +744,40 @@ void RendererVK::CreateSwapChain(VkPhysicalDevice inDevice)
 		frame_buffer_info.layers = 1;
 		FatalErrorIfFailed(vkCreateFramebuffer(mDevice, &frame_buffer_info, nullptr, &mSwapChainFramebuffers[i]));
 	}
+
+	// Allocate space to remember the image available semaphores
+	mImageAvailableSemaphores.resize(image_count, VK_NULL_HANDLE);
+
+	// Allocate the render finished semaphores
+	mRenderFinishedSemaphores.resize(image_count, VK_NULL_HANDLE);
+	for (uint32 i = 0; i < image_count; ++i)
+		mRenderFinishedSemaphores[i] = AllocateSemaphore();
 }
 
 void RendererVK::DestroySwapChain()
 {
+	// Destroy semaphores
+	for (VkSemaphore semaphore : mImageAvailableSemaphores) 
+		vkDestroySemaphore(mDevice, semaphore, nullptr);
+	mImageAvailableSemaphores.clear();
+
+	for (VkSemaphore semaphore : mRenderFinishedSemaphores) 
+		vkDestroySemaphore(mDevice, semaphore, nullptr);
+	mRenderFinishedSemaphores.clear();
+
+	for (VkSemaphore semaphore : mAvailableSemaphores) 
+		vkDestroySemaphore(mDevice, semaphore, nullptr);
+	mAvailableSemaphores.clear();
+
 	// Destroy depth buffer
 	if (mDepthImageView != VK_NULL_HANDLE)
 	{
 		vkDestroyImageView(mDevice, mDepthImageView, nullptr);
+		mDepthImageView = VK_NULL_HANDLE;
 
 		DestroyImage(mDepthImage, mDepthImageMemory);
+		mDepthImage = VK_NULL_HANDLE;
+		mDepthImageMemory = VK_NULL_HANDLE;
 	}
 
 	for (VkFramebuffer frame_buffer : mSwapChainFramebuffers)
@@ -776,6 +788,8 @@ void RendererVK::DestroySwapChain()
 		vkDestroyImageView(mDevice, view, nullptr);
 	mSwapChainImageViews.clear();
 
+	mSwapChainImages.clear();
+
 	if (mSwapChain != VK_NULL_HANDLE)
 	{
 		vkDestroySwapchainKHR(mDevice, mSwapChain, nullptr);
@@ -790,7 +804,32 @@ void RendererVK::OnWindowResize()
 	CreateSwapChain(mPhysicalDevice);
 }
 
-void RendererVK::BeginFrame(const CameraState &inCamera, float inWorldScale)
+VkSemaphore RendererVK::AllocateSemaphore()
+{
+	VkSemaphore semaphore;
+
+	if (mAvailableSemaphores.empty())
+	{
+		VkSemaphoreCreateInfo semaphore_info = {};
+		semaphore_info.sType = VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO;
+		FatalErrorIfFailed(vkCreateSemaphore(mDevice, &semaphore_info, nullptr, &semaphore));
+	}
+	else
+	{
+		semaphore = mAvailableSemaphores.back();
+		mAvailableSemaphores.pop_back();
+	}
+
+	return semaphore;
+}
+
+void RendererVK::FreeSemaphore(VkSemaphore inSemaphore)
+{
+	if (inSemaphore != VK_NULL_HANDLE)
+		mAvailableSemaphores.push_back(inSemaphore);
+}
+
+bool RendererVK::BeginFrame(const CameraState &inCamera, float inWorldScale)
 {
 	JPH_PROFILE_FUNCTION();
 
@@ -798,7 +837,10 @@ void RendererVK::BeginFrame(const CameraState &inCamera, float inWorldScale)
 
 	// If we have no swap chain, bail out
 	if (mSwapChain == VK_NULL_HANDLE)
-		return;
+	{
+		Renderer::EndFrame();
+		return false;
+	}
 
 	// Update frame index
 	mFrameIndex = (mFrameIndex + 1) % cFrameCount;
@@ -806,15 +848,20 @@ void RendererVK::BeginFrame(const CameraState &inCamera, float inWorldScale)
 	// Wait for this frame to complete
 	vkWaitForFences(mDevice, 1, &mInFlightFences[mFrameIndex], VK_TRUE, UINT64_MAX);
 
-	VkResult result = mSubOptimalSwapChain? VK_ERROR_OUT_OF_DATE_KHR : vkAcquireNextImageKHR(mDevice, mSwapChain, UINT64_MAX, mImageAvailableSemaphores[mFrameIndex], VK_NULL_HANDLE, &mImageIndex);
+	VkSemaphore semaphore = AllocateSemaphore();
+	VkResult result = mSubOptimalSwapChain? VK_ERROR_OUT_OF_DATE_KHR : vkAcquireNextImageKHR(mDevice, mSwapChain, UINT64_MAX, semaphore, VK_NULL_HANDLE, &mImageIndex);
 	if (result == VK_ERROR_OUT_OF_DATE_KHR)
 	{
 		vkDeviceWaitIdle(mDevice);
 		DestroySwapChain();
 		CreateSwapChain(mPhysicalDevice);
 		if (mSwapChain == VK_NULL_HANDLE)
-			return;
-		result = vkAcquireNextImageKHR(mDevice, mSwapChain, UINT64_MAX, mImageAvailableSemaphores[mFrameIndex], VK_NULL_HANDLE, &mImageIndex);
+		{
+			FreeSemaphore(semaphore);
+			Renderer::EndFrame();
+			return false;
+		}
+		result = vkAcquireNextImageKHR(mDevice, mSwapChain, UINT64_MAX, semaphore, VK_NULL_HANDLE, &mImageIndex);
 		mSubOptimalSwapChain = false;
 	}
 	else if (result == VK_SUBOPTIMAL_KHR)
@@ -825,6 +872,10 @@ void RendererVK::BeginFrame(const CameraState &inCamera, float inWorldScale)
 	}
 	FatalErrorIfFailed(result);
 
+	// The previous semaphore is now no longer in use, associate the new semaphore with the image
+	FreeSemaphore(mImageAvailableSemaphores[mImageIndex]);
+	mImageAvailableSemaphores[mImageIndex] = semaphore;
+
 	// Free buffers that weren't used this frame
 	for (BufferCache::value_type &vt : mBufferCache)
 		for (BufferVK &bvk : vt.second)
@@ -876,6 +927,8 @@ void RendererVK::BeginFrame(const CameraState &inCamera, float inWorldScale)
 
 	// Switch to 3d projection mode
 	SetProjectionMode();
+
+	return true;
 }
 
 void RendererVK::EndShadowPass()
@@ -907,20 +960,13 @@ void RendererVK::EndFrame()
 {
 	JPH_PROFILE_FUNCTION();
 
-	// If we have no swap chain, bail out
-	if (mSwapChain == VK_NULL_HANDLE)
-	{
-		Renderer::EndFrame();
-		return;
-	}
-
 	VkCommandBuffer command_buffer = GetCommandBuffer();
 	vkCmdEndRenderPass(command_buffer);
 
 	FatalErrorIfFailed(vkEndCommandBuffer(command_buffer));
 
-	VkSemaphore wait_semaphores[] = { mImageAvailableSemaphores[mFrameIndex] };
-	VkSemaphore signal_semaphores[] = { mRenderFinishedSemaphores[mFrameIndex] };
+	VkSemaphore wait_semaphores[] = { mImageAvailableSemaphores[mImageIndex] };
+	VkSemaphore signal_semaphores[] = { mRenderFinishedSemaphores[mImageIndex] };
 	VkPipelineStageFlags wait_stages[] = { VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT };
 	VkSubmitInfo submit_info = {};
 	submit_info.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO;

+ 6 - 3
TestFramework/Renderer/VK/RendererVK.h

@@ -20,7 +20,7 @@ public:
 
 	// See: Renderer
 	virtual void					Initialize(ApplicationWindow *inWindow) override;
-	virtual void					BeginFrame(const CameraState &inCamera, float inWorldScale) override;
+	virtual bool					BeginFrame(const CameraState &inCamera, float inWorldScale) override;
 	virtual void					EndShadowPass() override;
 	virtual void					EndFrame() override;
 	virtual void					SetProjectionMode() override;
@@ -64,6 +64,8 @@ private:
 	void							CreateSwapChain(VkPhysicalDevice inDevice);
 	void							DestroySwapChain();
 	void							UpdateViewPortAndScissorRect(uint32 inWidth, uint32 inHeight);
+	VkSemaphore						AllocateSemaphore();
+	void							FreeSemaphore(VkSemaphore inSemaphore);
 
 	VkInstance						mInstance = VK_NULL_HANDLE;
 #ifdef JPH_DEBUG
@@ -101,8 +103,9 @@ private:
 	uint32							mImageIndex = 0;
 	VkCommandPool					mCommandPool = VK_NULL_HANDLE;
 	VkCommandBuffer					mCommandBuffers[cFrameCount];
-	VkSemaphore						mImageAvailableSemaphores[cFrameCount];
-	VkSemaphore						mRenderFinishedSemaphores[cFrameCount];
+	Array<VkSemaphore>				mAvailableSemaphores;
+	Array<VkSemaphore>				mImageAvailableSemaphores;
+	Array<VkSemaphore>				mRenderFinishedSemaphores;
 	VkFence							mInFlightFences[cFrameCount];
 	Ref<TextureVK>					mShadowMap;
 	unique_ptr<ConstantBufferVK>	mVertexShaderConstantBufferProjection[cFrameCount];

+ 2 - 2
TestFramework/Utils/Log.cpp

@@ -12,8 +12,8 @@ void TraceImpl(const char *inFMT, ...)
 	// Format the message
 	va_list list;
 	va_start(list, inFMT);
-	char buffer[1024];
-	vsnprintf(buffer, sizeof(buffer), inFMT, list);
+	char buffer[2048];
+	vsnprintf(buffer, sizeof(buffer) - 1 /* leave space for newline char */, inFMT, list);
 	va_end(list);
 
 #ifdef JPH_PLATFORM_WINDOWS