DolphinQt: Add support for Vulkan on Wayland

[artemist]

Currently, Linux users of DolphinQt can only use the X11 windowing
system. If someone tries to run DolphinQt on top of Wayland then they are greeted with an error when they try to start running a game.

This commit adds support for running on top of Wayland, which
some compositors require in order to access full resolution on HiDPI
displays (such as sway). This uses the Vulkan ICD to get a Vulkan
surface, though does not support using wayland-egl to get an EGL
surface or software rendering to a native Wayland surface.

This commit does not set Wayland as the default backend when it is availaible.
Users who want to use it must set -DENABLE_WAYLAND=ON when compiling Dolphin
and run Dolphin with QT_QPA_PLATFORM=wayland in the environment.

I have done some work on DolphinNoGUI and OpenGL (with wayland-egl) in the wayland branch of my repository but they are in no state for a pull request. Once ready I'll separate them out and make separate pull requests.

I've tested this on NixOS on top of Sway. I have not tested it on compositors that use client-side decorations or on other Linux distributions. I would appreciate testing on those systems.

[Antidote]

Why not just default the parameters to -1 and avoid the overload altogether?

[stenzek]

Most of the work for Wayland (including input) has been done here: #7452. Although I might have missed something ;)

I never ended up merging it mainly because of the lack of client-side decorations in mutter/gnome-shell - Qt doesn't create a subsurface, instead the GL/Vulkan surface takes over the whole window and draws over the decorations.

I was hoping that by moving the GL/Vulkan context management to the frontend and letting Qt take care of it that it would use a subsurface and keep the decorations intact, but I trialed that approach in DuckStation and the result was sadly exactly the same as Dolphin - no decorations.

So, I'm not sure how to best handle it. Do we just forget about gnome-shell and rely on server-side decorations? It seems in some ways that is what Qt has done - the theme/style is not the same as the GTK theme when running under Wayland...

[jackoalan]

I've been trying my hand at getting Wayland support to be more stable.

Most of the work for Wayland (including input) has been done here: #7452. Although I might have missed something ;)

I never ended up merging it mainly because of the lack of client-side decorations in mutter/gnome-shell - Qt doesn't create a subsurface, instead the GL/Vulkan surface takes over the whole window and draws over the decorations.

The trick with this one is to wrap the RenderWidget in a QStackedWidget which becomes the actual window. There's even a trick to detect whether client-side decorations are in-use:
https://github.com/jackoalan/dolphin/blob/wayland/Source/Core/DolphinQt/MainWindow.cpp#L1009-L1019

There are also other problems such as running in the main window and switching to fullscreen. QtWayland will destroy and recreate the wl_surface whenever the widget hierarchy changes. A widget event is exposed specifically for this case:
https://github.com/jackoalan/dolphin/blob/wayland/Source/Core/DolphinQt/RenderWidget.cpp#L207-L233

Here I've added an alternate Renderer API alongside SurfaceChange. It works by blocking the main thread on the SurfaceAboutToBeDestroyed event until the emulator thread reaches the point where the renderer surface (egl window, vulkan surface) is destroyed. Control is returned to the main thread and the renderer blocks until the SurfaceCreated event comes around, at which point nativeResourceForWindow can be called for the new wl_surface.

I made a PR containing this new interlock mechanism:
artemist#1

[stenzek]

@jackoalan You shouldn't need to add a new renderer method for this. I thought I designed the surface changing stuff so it could switch from a valid surface to null and then back again, at least for Vulkan, but apparently not... might have to fix that.

An interesting way around it regardless. My only concern would be the potential performance implications (if any) of having the extra window in the chain for non-wayland targets, and what happens when it goes fullscreen (if you fullscreen the stack widget, you'd have top-level windows for both the stack and the parent window).

[jackoalan]

In my branch, the only time the parent QStackedWidget is active is when the RenderWidget is in a standalone window and m_render_parent (the added QStackedWidget) has been constructed:
https://github.com/jackoalan/dolphin/blob/wayland/Source/Core/DolphinQt/MainWindow.cpp#L1035-L1044

If it's in the main window or fullscreened, it's exactly the same widget hierarchy as non-wayland.

In case server-side decorations are detected, it's the same as non-wayland entirely.

[jackoalan]

I can't think of a better solution for orchestrating the surface change. I was getting deadlocks and a very unhappy mesa when QtWayland was allowed to destroy the surface while the emulator thread was about to swap. The interlock is the best logical solution I can think of.

[stenzek]

The intended way a surface change should happen is the main thread gets the change event, calls SetSurface(new_surface), which holds a lock and prevents the video thread from presenting/swapping buffers. When the video thread runs again, it'll recreate the swap chain. But, this hasn't really been tested, because nothing thus far has actually required surface changes ;)

But, if there's a period of time where the surface does not exist all, then the current API will not support it. In which case I think it'd be better for us to temporarily switch to headless, or pausing the emulator while the changes are happening.

Edit: If the surface must be destroyed before the UI thread can continue, that creates a potential deadlock too. Where the buffer swap is dependent on the UI thread processing some message, and UI thread being stuck waiting for the video thread. We've hit this with D3D, not sure if it's applicable for Wayland.

[jackoalan]

The intended way a surface change should happen is the main thread gets the change event, calls SetSurface(new_surface)

The thing is there's actually a pair of events. One for creation and one for destruction. Changes to the widget hierarchy (like MainWindow fullscreening) run a destroy and a create shortly after. So there's a period of the event sequence where there's no surface available.

To put it in context, QtWayland synchronously issues SurfaceAboutToBeDestroyed here:
https://github.com/qt/qtwayland/blob/dev/src/client/qwaylandwindow.cpp#L240-L241

Then destroys the wl_surface just a few statements later with no way to release its ownership to the application.

To quote the Vulkan spec:

Applications must therefore ensure that both the wl_display and the wl_surface remain valid for the lifetime of any VkSwapchainKHR objects created from a particular wl_display and wl_surface.

In other words, the renderer must be brought into sync while that event is being dispatched, and it would take an interlocking Host <-> Renderer API to do so. There is also no "null" surface as far as VkWaylandSurfaceCreateInfoKHR is concerned, so blocking the renderer is really the only option. It's a similar story for wayland-egl in my OpenGL experiments.

I propose the following mechanism:
https://github.com/jackoalan/dolphin/blob/wayland/Source/Core/VideoCommon/SurfaceChangeInterlock.cpp

• SurfaceChangeInterlock::BlockHostForSurfaceDestroy when the destroy event comes in on MainThread.
• SurfaceChangeInterlock::WaitForNewSurface when the EmuThread reaches the existing point after destroying swapchain/eglwindow for handle changes. This will unblock the main thread (which will immediately destroy the wl_surface and continue on to eventually create the replacement surface) and block the renderer thread (which has no valid surface to present into until a bit later).
• SurfaceChangeInterlock::UnblockRendererWithNewSurface once the create event comes in, unblocking the renderer and returning the new wl_surface from the SurfaceChangeInterlock::WaitForNewSurface call.

[jackoalan]

This is how I integrated it in Vulkan:
https://github.com/jackoalan/dolphin/blob/wayland/Source/Core/VideoBackends/Vulkan/SwapChain.cpp#L574-L577

Note how it's conditioned on a null handle being supplied, so the non-wayland platforms work exactly how they did before. The alternate interlocking API exposed via Host simply doesn't set the new surface pointer, but still sets the change flag so RecreateSurface gets called like usual.

[stenzek]

I think we're on the same train of thought. We can probably simplify it to SetSurface(nullptr, true /* wait_for_completion */) on the UI thread when it receives SurfaceAboutToBeDestroyed, the video thread gets to the point of presenting the next frame and destroys the swap chain and notifies the main thread, which then asynchronously sets the next surface once it's created.

The wait_for_completion is a flag because of the deadlock situation I mentioned earlier for D3D, we have to allow surface changes to be non-blocking.

There's still a few potential issues there - namely if the emulator is paused we'd need to push a frame so that the video thread wakes up, otherwise it'll sleep and never pick up the surface change. Which will require a different method depending on whether it's in single or dual core mode.

[LaserEyess]

I've been testing this PR (also on sway) and while it works I've found some perf issues compared to xwayland. A lot of stuttering and low framerate, the GPU/CPU loads are comparable so I'm assuming it's some inefficiencies in the code. I've never really debugged performance issues before but I'd be willing to try if someone pointed me to some instructions.

Also, with respect to client side decorations, I do not believe they should be a blocker. GNOME makes the active and deliberate choice to not include server-side decorations, and while it is not required for the server to provide these, it is also not required for the client to provide them. Which means you are not doing anything wrong if the first iteration of wayland support does not have client side decorations.

Again, the subset of users affected by this will only be GNOME wayland users. And they know exactly what the issue is, as many other toolkits such as GLFW, SDL, and applications like mpv, do not provide decorations when running as wayland applications.

[myownfriend]

So is this dead?

[cyrozap]

I tested this patch after rebasing it on 48c9c22. Here's what I've found works and doesn't work with this patch on Wayland:

Works:

• Emulation
• Graphics with HiDPI on Vulkan on KDE Plasma
• Audio
• Controller input via GameCube Adapter for Wii U

Doesn't work:

• Controller input via keyboard/mouse (configuring the "Standard Controller" seems to want to use XInput)
• Hotkeys
• Inhibit Screensaver During Emulation
• Remembering the size of the emulation window (it always starts really tiny--not even big enough to show all the minimize/maximize/close buttons)

If anyone wants me to test any other features or functionality with this patch under Wayland, please let me know.

[r3claimer]

I tested this patch after rebasing it on 48c9c22. Here's what I've found works and doesn't work with this patch on Wayland:

Works:

• Emulation
• Graphics with HiDPI on Vulkan on KDE Plasma
• Audio
• Controller input via GameCube Adapter for Wii U

Doesn't work:

• Controller input via keyboard/mouse (configuring the "Standard Controller" seems to want to use XInput)
• Hotkeys
• Inhibit Screensaver During Emulation
• Remembering the size of the emulation window (it always starts really tiny--not even big enough to show all the minimize/maximize/close buttons)

If anyone wants me to test any other features or functionality with this patch under Wayland, please let me know.

Do you have a repo with your changes / would mind uploading them?

[cyrozap]

@brooksytech I really didn't make any changes--I just ran the following commands:

git clone https://github.com/dolphin-emu/dolphin.git &&
        cd dolphin &&
        git remote add -t wayland-vulkan artemist https://github.com/artemist/dolphin.git &&
        git fetch artemist wayland-vulkan &&
        git checkout wayland-vulkan &&
        git rebase 48c9c22

There's not much to share beyond that since git handles all of the patching and there weren't any merge conflicts. If you want to rebase on git master, simply replace that last git rebase 48c9c22 command with git rebase master.

If you're running Arch Linux and you want to try this patch yourself, I have an AUR package called dolphin-emu-wayland that makes it simple to build and install Dolphin with this patch.

[martinlindhe]

@cyrozap Thanks, your AUR is working great as a drop-in on Wayland.

[emansom]

If you're running Arch Linux and you want to try this patch yourself, I have an AUR package called dolphin-emu-wayland that makes it simple to build and install Dolphin with this patch.

@cyrozap One can also generate patch files from PRs by adding .patch to the PR URL, it redirects to their patch-diff service. Use that as one of the sources in the PKGBUILD.

[cyrozap]

@emansom I'm aware, but as the commit in this PR is based on a rather old version of Dolphin (it's over two years old now), it doesn't apply cleanly on the current codebase:

$ git clone https://github.com/dolphin-emu/dolphin.git
...
$ cd dolphin
$ wget https://github.com/dolphin-emu/dolphin/pull/8727.patch
...
$ patch -p1 < 8727.patch 
patching file CMake/FindWayland.cmake
patching file CMakeLists.txt
Hunk #1 succeeded at 43 with fuzz 2 (offset 24 lines).
Hunk #2 succeeded at 550 (offset 101 lines).
patching file Source/Core/Common/WindowSystemInfo.h
patching file Source/Core/DolphinQt/Host.cpp
Hunk #1 succeeded at 191 with fuzz 1 (offset 107 lines).
patching file Source/Core/DolphinQt/MainWindow.cpp
Hunk #1 succeeded at 186 (offset 18 lines).
Hunk #2 succeeded at 1253 (offset 111 lines).
patching file Source/Core/VideoBackends/OGL/CMakeLists.txt
Hunk #1 succeeded at 32 with fuzz 2.
can't find file to patch at input line 223
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/Source/Core/VideoBackends/Vulkan/Renderer.cpp b/Source/Core/VideoBackends/Vulkan/Renderer.cpp
|index 9d096e19e47c..8f4568d75024 100644
|--- a/Source/Core/VideoBackends/Vulkan/Renderer.cpp
|+++ b/Source/Core/VideoBackends/Vulkan/Renderer.cpp
--------------------------
File to patch:

The patch in the AUR package is generated by rebasing the commit in this PR onto the latest version of Dolphin in the Arch repos. By tracking file moves and other changes, git is able to cleanly apply the changes, avoiding the issue shown here.

[AdmiralCurtiss]

Try PR #11257 instead.