Update Swapchain image count to obey present mode constraints

[SRSaunders]

This PR separates and updates how semaphores are used for Vulkan image acquisition and presentation sync. This follows best practices documented in https://docs.vulkan.org/guide/latest/swapchain_semaphore_reuse.html.

1. Separates image presentation from image acquisition semaphores.
2. Allocates NUM_FRAME_DATA image acquisition semaphores to match the number of frames-in-flight for the renderer, and continues to use a circular queue for cycling through image acquisition semaphores.
3. Dynamically allocates an addressable vector of presentation semaphores, sized to match the number of swapchain images allocated during swapchain creation.
4. If VK_EXT_surface_maintenance1 is available in the implementation, requests minImageCount and maxImageCount for the specific presentation mode selected for the swapchain, and uses these to bound the requested swapchain image count during swapchain creation.
5. Uses the m_SwapChainIndex returned from vkAcquireNextImageKHR() to index into the presentation semaphore vector when synchronizing within vkPresentKHR(). This is to support potential out-of-order image acquisition which can occur in certain circumstances depending on the presentation engine.
6. Dynamically destroys, resizes, and recreates the presentation semaphore vector to match the swapchain image count when changing present (vsync) modes.

This PR also updates a macOS-specific item:

7. macOS-only: Simplifies and updates the MoltenVK-specific Optick trace calculations (command buffer submit, image acquisition, and Metal encode timings) for better accuracy. I decided to do this since I was using Optick to verify the semaphore changes above.

Fixes #1080.

Tested on Windows 11 Vulkan, Manjaro Linux, and macOS Sequoia.

[klaussilveira]

Running this locally, I have noticed drops to 15 FPS and freezes on simple maps, such as testbox. Debian 12, RTX 4060 with 580.82.09 drivers.

Next weekend I'll try to run it with optick and see what could be going on. Any tips on which debug cvars to enable for Vulkan?

[SRSaunders]

Thanks @klaussilveira for testing this, but I definitely was not expecting a performance hit. I haven't seen that when testing on Win, Linux, or Mac. I have a few questions:

1. Did the performance hit occur on a particular present mode, e.g. Vsync Disabled, Vsync Smart, or Vsync Enabled?
2. If the problem occurs with VSync Disabled, what is your setting for r_vkPreferFastSync (default is 1 or enabled for Vulkan Mailbox Mode for reduced tearing if your driver supports it). However, if VSync Disabled is showing problems, you can try settingr_vkPreferFastSync=0 which disables Mailbox Mode for VSync Off.
3. Did you build out of the box with NUM_FRAME_DATA = 3? Or did you build with a custom setting (e.g. 2)?
4. Did you see this problem in windowed or fullscreen modes?

I would like to try to duplicate this issue if possible, so any setup info you remember would be helpful. Thanks.

Quick Q: How do you load textbox? I have test_box_valve220.map in my testmaps folder, but get the following error when trying to load: idRenderWorldLocal::InitFromMap: maps/testmaps/test_box.proc not found. Am I missing some data files?.

Scratch that, I renamed and compiled test_box using rbdmap. Loads and runs fine without any performance issues on macOS + AMD 6600XT. Will try Manjaro linux + AMD later today. Update: Test_box map works fine there too.

I wonder if there’s a difference with Nvidia? However, please come back first with answers 1-4 above.

[klaussilveira]

This is not consistent at all, which makes me think it is some driver issue. I also intermittently get this crash:

Sys_Error: Binding set in slot 0 does not match the layout in pipeline slot 0

I can't pinpoint the cause yet. r_vkPreferFastSync is 1, vsync disabled. I'll keep updating this with more info as I test more.

[SRSaunders]

A couple of things:

1. Are you sure this pull request is causing your perf issues? Have you tested recently with the master branch?
2. The binding set mismatch error is not related to this PR. I thought I had solved that one a while back, but perhaps it is still hanging around. Let me look into that a bit deeper, but again, that is not related to this PR.

UPDATE: I can't reproduce the binding set mismatch error on my Manjaro linux setup using an AMD 6600XT GPU, nor on macOS. If you can reproduce this error reliably, I can provide a few debugging changes to perhaps help track this down. Please advise.

UPDATE 2: I may have found the binding set mismatch issue, but can't test the solution since I can't reliably reproduce. If you are still getting those mismatch errors, I can provide you with a patch to try out. Let me know.

[SRSaunders]

@klaussilveira I have created a new branch (off of master) with a possible fix for the binding set mismatch problem. I would appreciate if you could try this and see if those errors go away. Thanks.

https://github.com/SRSaunders/RBDOOM-3-BFG/tree/bindingset-mismatch-fix

If this works, I will post a separate PR.

[klaussilveira]

Nope, still happens:

TODO: Sys_SetPhysicalWorkMemory
  1739 msec to load test
Sys_Error: Binding set in slot 0 does not match the layout in pipeline slot 0

[SRSaunders]

Hmm. I have a few more ideas. Try each of these separately:

1. Try setting the following variable (in RenderProgs_NVRHI.cpp) to true and rebuild, then retest:

	void idRenderProgManager::SetUniformValue( const renderParm_t rp, const float value[4] )
	{
-->		bool rpChanged = true; //false;

		for( int i = 0; i < 4; i++ )
		{

2. Set cvar r_useNewSSAOPass=0 and retest.
3. Set cvar r_vkUsePushConstants=0 (in autoexec or +option on command line) and retest.

[klaussilveira]

bool rpChanged = true; seems to have solved it. I will continue testing for more time, but so far, no crashes.

[SRSaunders]

Thanks @klaussilveira. Did you apply bool rpChanged = true to my test branch, or to master?

Actually, there is one more test you could do to help narrow things down. Revert the bool to false, and set r_vkUsePushConstants=0. I would like to determine if the renderParms change detection logic is a problem only when Push Constants are enabled.

[SRSaunders]

@klaussilveira I have added an additional commit which should help. Please ignore the bool rpChanged = true workaround for now, and instead test the new update in my bindingset-mismatch-fix branch.

Please let me know if this works. Thanks.

UPDATE: I have deleted this test branch and will post a separate PR regarding the Sys_Error: Binding set in slot 0 does not match the layout in pipeline slot 0 error condition. The solution was a bit trickier than I first thought. Once the new PR is posted we can continue the discussion there.

As far as I am concerned this PR above (Swapchain image count) is working fine and does not need any updates.