From 5270e7d2fd747c640cc82d9483a967c1e8d9af6a Mon Sep 17 00:00:00 2001 From: "Matias N. Goldberg" Date: Mon, 26 Feb 2024 19:21:48 -0300 Subject: [PATCH] [Vk] Fix assert in complex conditions VulkanRenderSystem::flushBoundGpuProgramParameters would assert inside constBuffer->regressFrame(); The steps to trigger are: 1. Use mAutoParamsBuffer (i.e. a low level material with parameters) so that mFirstUnflushedAutoParamsBuffer becomes non-zero. 2. Stall twice in a row without presenting to screen. In real world scenarios this could happen if a lot of texture memory is streaming and TextureGpuManager is forced to stall. --- .../Vulkan/include/OgreVulkanRenderSystem.h | 1 - RenderSystems/Vulkan/src/OgreVulkanDevice.cpp | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/RenderSystems/Vulkan/include/OgreVulkanRenderSystem.h b/RenderSystems/Vulkan/include/OgreVulkanRenderSystem.h index d0f0a460309..fbd30244256 100644 --- a/RenderSystems/Vulkan/include/OgreVulkanRenderSystem.h +++ b/RenderSystems/Vulkan/include/OgreVulkanRenderSystem.h @@ -272,7 +272,6 @@ namespace Ogre uint16 variabilityMask ) override; void bindGpuProgramPassIterationParameters( GpuProgramType gptype ) override; - protected: /** Low Level Materials use a params buffer to pass all uniforms. We emulate this using a large const buffer to which we write to and bind the regions we need. This is done in bindGpuProgramParameters(). diff --git a/RenderSystems/Vulkan/src/OgreVulkanDevice.cpp b/RenderSystems/Vulkan/src/OgreVulkanDevice.cpp index 14081c6ab85..bcdc6299357 100644 --- a/RenderSystems/Vulkan/src/OgreVulkanDevice.cpp +++ b/RenderSystems/Vulkan/src/OgreVulkanDevice.cpp @@ -678,6 +678,20 @@ namespace Ogre //------------------------------------------------------------------------- void VulkanDevice::stall() { + // We must call flushBoundGpuProgramParameters() now because commitAndNextCommandBuffer() will + // call flushBoundGpuProgramParameters( SubmissionType::FlushOnly ). + // + // Unfortunately that's not enough because that assumes VaoManager::mFrameCount + // won't change (which normally wouldn't w/ FlushOnly). However our caller is + // about to do mFrameCount += mDynamicBufferMultiplier. + // + // The solution is to tell flushBoundGpuProgramParameters() we're changing frame idx. + // Calling it again becomes a no-op since mFirstUnflushedAutoParamsBuffer becomes 0. + // + // We also *want* mCurrentAutoParamsBufferPtr to become nullptr since it can be reused from + // scratch. Because we're stalling, it is safe to do so. + mRenderSystem->flushBoundGpuProgramParameters( SubmissionType::NewFrameIdx ); + // We must flush the cmd buffer and our bindings because we take the // moment to delete all delayed buffers and API handles after a stall. //