dolphin-emu
Vulkan: Remove unnecessary command buffer execution
When resizing the game window there is a chance that the swapchain state is bad. This will result in a deadlock on Linux + Nvidia's proprietary driver. This code can be removed and the next command buffer execution will simply occur when the swapchain has been recreated.
The root issue may be a Linux Nvidia driver implementation problem. I wasn't able to reproduce the original problem on Windows, nor on my Steam Deck. I tested this change on Fedora 40+Nvidia & SteamOS+AMD.
Fixes https://bugs.dolphin-emu.org/issues/13523
I'll test this and report back, this has been annoying me for a long time.
Did a brief (50 restarts) test on Windows. I no longer am getting the Vulkan Hook crash with OBS as a possible side effect.
NVIDIA RTX 3080 | Driver 560.94 | OBS 30.2.3
Tested on Arch
NVIDIA RTX 3080 | 560.35.03
Observed behavior:
On resizing a VK_ERROR_OUT_OF_DATE_KHR alerts.
"Ignore for this session" successfully completes the resize. Then, I can resize as many times as I want IFF I am increasing the size without pausing the emulation. It works as expected.
However the moment I attempt to shrink the window without pausing the emulation the deadlock occurs again.
Manjaro Linux here, using an Nvidia GeForce GTX 980 Ti with driver version 550.107.02.
This change seemingly has fixed the deadlock when rendering to main window, or at least I cannot get the deadlock to occur with repeated and excessive resizing. Resizing when rendering to the floating window is still problematic. Same as Dreamsyntax, I get the message Warning: Failed to grab image from swap chain: -0X3B9ACDEC VK_ERROR_OUT_OF_DATE_KHR almost immediately, but for me that's where it gets stuck no matter what. There is a function call ExecuteCommandBuffer(false, true) in VKGfx::CheckForSurfaceResize() identical to the one that was removed in this PR which ultimately leads to the vkWaitForFences deadlock at the core of this issue, but it is executed by both render to main window and render to floating window, so I don't know how one works and the other doesn't.
Potentially partially obsolete per https://github.com/dolphin-emu/dolphin/pull/13087
However, if the OBS crash persists in 13087, I'll re-test this one once rebased with 13087's changes.
@homeisfar Can you please rebase your branch on master? I still experience the OBS crash in 13087, so I think this PR still would be good to keep, but I want to test it with the latest changes.
@dreamsyntax done. I did a cursory build and check locally to ensure that with TellowKrinkle's latest work this still wouldn't crash. My test "script" involves programmatically resizing the window aggressively in a loop. Things still look good on my end.
Unfortunately, I can getting the OBS hook crash still even with this version. I guess it was a fluke before where it was working.
That's under MS Windows, right? If it's also happening under Linux I can investigate it but otherwise I'll re-close this issue.
That's under MS Windows, right? If it's also happening under Linux I can investigate it but otherwise I'll re-close this issue.
Yeah, Windows only in my experience. I guess let's close this unless you think this change provides other benefits we are overlooking.
Now that the OBS bug has been identified and patched (issue was in their game capture code), I'm not sure if this change affects anything.
Does this improve performance or otherwise by removing this call?
I think this call is still unnecessary. If it improves performance, it's not substantial. I'll try benchmarking this weekend, and if I don't observe any noticeable difference I'll close the PR
I didnt notice any issues with the PR, so id be fine keeping this remove
This may be something I come back and revisit with a fresh pair of eyes. I apologize for dropping this, but many things have taken my attention between then and now.
Fair enough. The underlying issue this was originally for is fixed in mainline anyway.