Skip to content

Commit

Permalink
[Vk] Fix assert in complex conditions
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
darksylinc committed Feb 26, 2024
1 parent d56ad08 commit 5270e7d
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
1 change: 0 additions & 1 deletion RenderSystems/Vulkan/include/OgreVulkanRenderSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -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().
Expand Down
14 changes: 14 additions & 0 deletions RenderSystems/Vulkan/src/OgreVulkanDevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down

0 comments on commit 5270e7d

Please sign in to comment.