Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

renderer_vulkan: fix deadlock when resizing the SDL window #1860

Merged
merged 2 commits into from
Dec 29, 2024
Merged

renderer_vulkan: fix deadlock when resizing the SDL window #1860

merged 2 commits into from
Dec 29, 2024

Conversation

ngoquang2708
Copy link
Contributor

@ngoquang2708 ngoquang2708 commented Dec 24, 2024 •
edited
Loading

Commit 400da1a add ability to handle recreation of the swapchain which fix black screen under XWayland. Unfortunately, that commit doesn't account for rapid swapchain recreation caused by resizing window.

This commit aim to solve that issue by skipping frames to recreate the swapchain and delay resetting the frame's present done fence until after we know for sure we will be submitting work with it.

@ngoquang2708
Copy link
Contributor Author

@JetPackAndButlers @C0rn3j Please help me test this.

@setepenre
Copy link
Contributor

setepenre commented Dec 24, 2024 •
edited
Loading

quickly tested on arch+cinnamon desktop environment, could freely resize sdl window, game (Bloodborne main menu and in game) kept running responsively
EDIT: on current main resizing causes crash

@C0rn3j
Copy link
Contributor

C0rn3j commented Dec 24, 2024 •
edited
Loading

I can resize fine on master on Arch Linux with Plasma, did not test this branch (no time atm for that too)

EDIT: Eventually after STOPPING resizing it died after a couple seconds and crashed, so I guess the bug happens there too - again, NOT with this branch, but on master, so this comment is not that useful.

@ngoquang2708 ngoquang2708 marked this pull request as draft December 24, 2024 15:11
@ngoquang2708
Copy link
Contributor Author

I don't know why but 2D games like Dead Cells don't like this. If I keep resizing slowly it will eventually hang. Don't happen with 3D games.

@ngoquang2708 ngoquang2708 force-pushed the fix-window-resizing branch 3 times, most recently from 1857eed to 54d71cf Compare December 24, 2024 19:11
@ngoquang2708
Copy link
Contributor Author

Latest pushes work when abusing resizing window now.

2024-12-25.10-32-17.mp4

@ngoquang2708 ngoquang2708 marked this pull request as ready for review December 25, 2024 03:34
@ngoquang2708
Copy link
Contributor Author

Rebase.

@ngoquang2708 ngoquang2708 mentioned this pull request Dec 25, 2024
Merged
@rharish101
Copy link

This fixes the blackscreen issue for me, which #1830 didn't. I'm running Hyprland on Arch Linux, with an AMD RX 7900 XTX (RADV driver; mesa 24.3.1), and compared this branch with the latest git main.

@C0rn3j
Copy link
Contributor

C0rn3j commented Dec 25, 2024

It's a bit of a shame that the patch kills responsivity, but at least it's better than crashing

@ngoquang2708
Copy link
Contributor Author

ngoquang2708 commented Dec 25, 2024 •
edited
Loading

It's a bit of a shame that the patch kills responsivity, but at least it's better than crashing

It because of XWayland/NVidia combos. When use native Wayland or when not using NVidia GPU, swapchain recreation (which is slow) is not happened at all.

2024-12-26.00-35-50.mp4

@C0rn3j
Copy link
Contributor

C0rn3j commented Dec 25, 2024 •
edited
Loading

Ahhh, I think get it now.

DISPLAY=:0 shadps4 -> Xwayland -> freeze bug without the previous commit/resize crash with it + the resize is not responsive with this PR*

unset DISPLAY shadps4 -> Wayland -> no issues

The fact that if DISPLAY is set it defaults to Xwayland is imo another bug, I'll report that separately... #1878

* my resize is fully responsive on Arch Linux with Wayland Plasma on Nvidia while running the game under Xwayland on latest master + this patch.

Screencast_20241225_191129.webm

@JetPackAndButlers
Copy link

Resize is fully responsive on Wayland while choppier on X11 (Nvidia issue as mentioned previously), Bloodborne seems to handle the resize flawlessly on both menu and gameplay, same with GrimGrimoire OnceMore, 13 Sentinels and Persona 3-4 Dancing, Odin Sphere and Dragon's Crown Pro after some resizing seem to freeze while throwing

Debug] <Critical> vk_presenter.cpp:operator():769: Assertion Failed!
Device lost during waiting for a frame

or

[Render.Vulkan] <Warning> vk_swapchain.cpp:Destroy:203: Failed to wait for device to become idle: ErrorDeviceLost

@ngoquang2708
Copy link
Contributor Author

@JetPackAndButlers What is your PC specs?

@ngoquang2708 ngoquang2708 mentioned this pull request Dec 26, 2024
Closed
@ngoquang2708
Copy link
Contributor Author

  • my resize is fully responsive on Arch Linux with Wayland Plasma on Nvidia while running the game under Xwayland on latest master + this patch.

It slow on mine because my PC is a optimus laptop with hybrid graphics so my DE run on the Intel integrated GPU.

@ghost
Copy link

ghost commented Dec 26, 2024

Launching with Fullscreen doesn't result in a blackscreen anymore.
Pressing F11 in window mode doesn't crash anymore.
And resizing the windows works without issues!

-On Fedora/Bazzite with an AMD GPU and open source drivers.

@JetPackAndButlers
Copy link

@JetPackAndButlers What is your PC specs?

i7 9700k, RTX 2070 and 16 Gigabytes of Ram
CachyOS Linux

@ghost
Copy link

ghost commented Dec 27, 2024

So will this go into the mainbuild? because it seems like it fixes the stuff.
Or are there any downsides?

@ngoquang2708 ngoquang2708 marked this pull request as draft December 27, 2024 14:45
@ngoquang2708 ngoquang2708 reopened this Dec 28, 2024
@ngoquang2708
Copy link
Contributor Author

Close by accident.

@ngoquang2708
Copy link
Contributor Author

ngoquang2708 commented Dec 28, 2024 •
edited
Loading

Please retest. Try to abuse window resizing.

@JetPackAndButlers Please retest if device lost still happen in your case.

@rharish101
Copy link

Still works for me, on Hyprland (Wayland, keeping the DISPLAY variable, i.e. not overriding it) with Arch Linux. Abusing resizing at launch with Bloodborne doesn't change anything.

@ngoquang2708 ngoquang2708 marked this pull request as ready for review December 28, 2024 13:23
@ngoquang2708
Copy link
Contributor Author

@raphaelthegreat I think the current code is in good shape. Can I request a review?

@raphaelthegreat raphaelthegreat merged commit 1bc2713 into shadps4-emu:main Dec 29, 2024
8 of 9 checks passed
@ngoquang2708 ngoquang2708 deleted the fix-window-resizing branch December 29, 2024 11:23
Younes-Bel pushed a commit to Younes-Bel/shadPS4 that referenced this pull request Dec 29, 2024
@ngoquang2708 @Younes-Bel
renderer_vulkan: fix deadlock when resizing the SDL window (shadps4-e…
67b5bac 
…mu#1860)

* renderer_vulkan: Fix deadlock when resizing the SDL window

* Address review comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raphaelthegreat raphaelthegreat raphaelthegreat approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ngoquang2708 @setepenre @C0rn3j @rharish101 @JetPackAndButlers @raphaelthegreat