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.

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

CI/Linux: Disable Wayland and spring cleaning #10179

Merged
merged 5 commits into from
Oct 25, 2023
Merged

CI/Linux: Disable Wayland and spring cleaning #10179

merged 5 commits into from
Oct 25, 2023

Conversation

stenzek
Copy link
Contributor

@stenzek stenzek commented Oct 25, 2023

For those picking up on this as "news", please read the following list:

  1. PCSX2 still supports Wayland. It just prefers the XCB/XWayland platform by default. You can set the I_WANT_A_BROKEN_WAYLAND_UI environment variable and experience the brokenness for yourself on the AppImage builds, or add the wayland socket to the flatpak.
  2. This PR is over a month old. The code's been in PCSX2 for a month. XWayland works fine, nothing changed, the roof didn't cave in, nobody actually cared until apparently some youtuber made a video out of it. The only change was a reduction in support requests. You're being drawn in to drama where there is none.
  3. I was the one who implemented Wayland support in the first place, this isn't some "anti wayland" crusade. It causes issues, most of which are caused by QtWayland, some are caused by the protocol itself. I want nothing more than to see Wayland succeed, but at the moment, it is unusable for a majority of users.
  4. Both myself and the rest of the PCSX2 team are open to suggestions on how to fix these issues. But most of it is out of our control, and we don't want more platform-specific, unmaintainable hacks, that don't work reliably in the first place. We're not Firefox/GIMP/etc, we're too small and irrelevant for any of the DE maintainers to actually care about.
  5. We can't just "disable wayland on nvidia". Qt platform selection happens long before we know what type of GPU a user has. We could disable it on GNOME, but many of the issues happen on KDE too.

Description of Changes

Disables Wayland, it's super broken/buggy in basically every scenario. KDE isn't too buggy, GNOME is a complete disaster.

  • Stupid obsession with CSD in Gnome => inconsistency
  • Inability to position windows => window position saving doesn't work, log window attaching (not merged yet) doesn't work
  • Hacks in render-to-main because WL craps itself otherwise
  • Despite said hacks, game list still glitches after stopping emulation, happens more often in gnome
  • NVIDIA just crashes in swap chain creation under Wayland
  • Broken global menus
  • and many others

Until they sort their s**t out, which is unlikely, since there's been very little progress over the last decade, just keep it disabled. For the Flatpaks, users can re-enable it with flatseal if they really want the crappy experience.

Bumps the Flatpak to the KDE/Qt 6.6 runtime, now that it's available.

Bumps the compiler to Clang/LLVM 17, as that's all that's available in 6.6

Removes the host:ro filesystem permission from the PCSX2 flatpak. It's not needed, since we go through portals for adding game directories, and gets rid of the big ass warning about filesystem access on flathub.

NOTE: If you're using third party launchers/frontends with the flatpak, this will break them, since they're probably not using portal paths. Go complain to your launcher/frontend about it, not us, there are ways they can work around it (but I'm not going to do their job for them, since a good number of them are scummy paid products).

Rationale behind Changes

See above.

Suggested Testing Steps

Make sure AppImage and Flatpak still work.

Copy link
Contributor

@kamfretoz kamfretoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

image

@stenzek stenzek merged commit 7205f10 into PCSX2:master Oct 25, 2023
12 checks passed
@stenzek stenzek deleted the linux branch October 25, 2023 07:53
@efeme4
Copy link

efeme4 commented Oct 25, 2023

It is absurd to remove wayland support completely just because of insignificant bugged features like window position saving or csd. It was working perfectly at least for me and even if everything said above was true the emulator was completely usable with no noticeable gameplay inconveniences. This PR is completely delirious and says a lot (but nothing good) about PCSX2 developing standards.

@refractionpcsx2
Copy link
Member

refractionpcsx2 commented Oct 25, 2023

it's not just bugged windows position, if you did more reading of the OP before raging through your keyboard, you'd see there are other problems.

if you want to use it, you're free to re-enable it and build it yourself, but for us, wayland is a constant pain in the neck and an inferior experience.

@stenzek
Copy link
Contributor Author

stenzek commented Oct 25, 2023

It's not removed at all. If you read the title or description, you'd see it is disabled on our release builds, because it's nothing but headaches for us, because of its broken by design nature causing issues for users. I listed a bunch of them in the OP as well.

We're sick of getting blamed for bugs in wayland compositors, while the various committees sit around arguing with each other, finally decide on standard ways of doing things after half a decade, then GNOME ruins it all by refusing to implement it.

If you use the flatpak, it's even just a single flatseal command to re-enable.

PCSX2 runs fine under XWayland. In fact, it runs much better than under native WL, because it doesn't have all the above issues.

Don't get me wrong, X11 is terrible code, and should've been purged a decade ago. But Wayland is just broken, and everyone would rather sit around arguing with each other instead of actually addressing the design flaws.

Edit: I probably should clarify the reason the libs were removed from the AppImage - it's because I couldn't find a way to override environment variables without using a custom AppRun script, and the whole reason to moving to linuxdeploy was so that it'd be less error-prone and maintenance work. Plus, if we overrode it in AppRun, then you couldn't override it yourself anyway.

@efeme4
Copy link

efeme4 commented Oct 25, 2023

I'm sorry if the FR issue was an inconvenience or if my comment was seen as an insult or a lack of respect, it wasn't my intention. In my case, the appImage doesn't boot after the update with wayland enabled, which made me think that it was removed, and I just wanted to know if that's the case and if it could be brought back (the reason for the FR issue was to formalize my pettition).

@refractionpcsx2
Copy link
Member

refractionpcsx2 commented Oct 25, 2023

It's only disabled in the app image, if you build it yourself it will still have wayland. Flatpak also has a switch to enable wayland.

@stenzek
Copy link
Contributor Author

stenzek commented Oct 25, 2023

It works fine under WL sessions, with XWayland. Kam and I both checked it.

If you solve all the issues I listed above with Wayland, then I'd be more than happy to re-enable it in Release builds. But I can't see how that is possible, especially with GNOME, and we're tired of users blaming us for its brokenness.

@stenzek
Copy link
Contributor Author

stenzek commented Oct 25, 2023

For some perspective on why we, as developers, are frustrated with Wayland:

Have a read of this latest mess: https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/247

It's not the first time such a proposal has been put forward. Something that developers need for their applications to work properly on WL (particularly multi-window applications), and it gets vetoed. Every other OS manages this fine. But apparently we're in the wrong for not conforming to some warped view of how applications should be, despite our applications working fine on every other platform. Especially the GNOME folks, not everyone wants to use the rubbish that they call GTK.

And don't give me the "but I don't use GNOME, I use wlroots/kwin/whatever, it works fine", that's good for you, then build from source, or use the flatpak+flatseal. Unfortunately, a good chunk, if not a majority of Linux users end up with GNOME, and they get the crap experience, and we can't really say "if GNOME, disable WL", since Qt makes that decision, not us.

Have a look through the PCSX2 source for references to Wayland. Here's some:

$ grep -ri "wayland" pcsx2-qt/
pcsx2-qt/MainWindow.cpp:// Qt Wayland is broken. Any sort of stacked widget usage fails to update,
pcsx2-qt/MainWindow.cpp:        // .. except on Wayland, where everything tends to break if you don't recreate.
pcsx2-qt/MainWindow.cpp:        // make sure we're visible before trying to add ourselves. Otherwise Wayland breaks.
pcsx2-qt/MainWindow.cpp:                // Don't risk doing this on Wayland, it really doesn't like window state changes,
pcsx2-qt/MainWindow.cpp:        // Wayland's a pain as usual, we need to recreate the window, which means there'll be a brief
pcsx2-qt/DisplayWidget.h:       // Wayland is broken in lots of ways, so we need to check for it.
pcsx2-qt/DisplayWidget.cpp:     // We only need this on Wayland because of client-side decorations...

And you wonder why we just throw our hands in the air and say "bugger this". As an OS developer, you need to have applications, or it's kinda pointless. By pushing developers away from your platform, because they're tired of dealing with complaining users, about things that are 100% out of their control, you have nobody to blame but yourself.

@efeme4
Copy link

efeme4 commented Oct 25, 2023

Thanks for the detailed explanation, I think I know really understand the purpose of the PR

@ghost
Copy link

ghost commented Oct 26, 2023

Software developers when they actually have to maintain their software:

We're sick of getting blamed for bugs in wayland compositors, while the various committees sit around arguing with each other, finally decide on standard ways of doing things after half a decade, then GNOME ruins it all by refusing to implement it.

Have you considered not supporting GNOME? wlroots is the only good wayland library, if more popular applications only support wlroots then GNOME and KDE will effectively be forced to adopt wlroots instead of their own crap code.

@stenzek
Copy link
Contributor Author

stenzek commented Oct 27, 2023

Have you considered not supporting GNOME? wlroots is the only good wayland library, if more popular applications only support wlroots then GNOME and KDE will effectively be forced to adopt wlroots instead of their own crap code.

Did you read my last post? I specifically addressed this:

And don't give me the "but I don't use GNOME, I use wlroots/kwin/whatever, it works fine", that's good for you, then build from source, or use the flatpak+flatseal. Unfortunately, a good chunk, if not a majority of Linux users end up with GNOME, and they get the crap experience, and we can't really say "if GNOME, disable WL", since Qt makes that decision, not us.

@ghost
Copy link

ghost commented Oct 27, 2023

My bad I didn't see that,

A warning for GNOME users to use XCB? Default to XCB but allow wlroots users to use Wayland? I get why this choice was made but it does hurt users who would prefer a prebuilt binary that supports Wayland, even if via some random config or whatever... Good luck trying to build PCSX2 on my computer 😃

@stenzek
Copy link
Contributor Author

stenzek commented Oct 27, 2023

A warning for GNOME users to use XCB?

Users don't read warnings. They just complain that it's broken.

Default to XCB but allow wlroots users to use Wayland?

Not possible, because the decision gets made by Qt before our code runs in the first place. Unless we put a shell script in place, but then self builds versus release builds have behaviour differences, which I'd very much like to avoid.

I get why this choice was made but it does hurt users who would prefer a prebuilt binary that supports Wayland, even if via some random config or whatever... Good luck trying to build PCSX2 on my computer

That's what's happening, at least with the flatpak. As far as I'm aware, flatseal can add additional arguments, so you can override QT_QPA_PLATFORM and add --share=wayland. The code for Wayland is still in the binary, so it'll "just work".

AppImage had to have the libs removed because I didn't want to mess around with a custom AppRun script, and as I said above, it'd take precedence over anything user supplied anyway.

Edit: I don't really see the issue with PCSX2 running under XWayland. As far as I'm aware, there isn't even a performance hit.

@ghost
Copy link

ghost commented Oct 27, 2023

That's what's happening, at least with the flatpak. As far as I'm aware, flatseal can add additional arguments, so you can override QT_QPA_PLATFORM and add --share=wayland. The code for Wayland is still in the binary, so it'll "just work".

Good to know I'll try that.

I don't really see the issue with PCSX2 running under XWayland

The issue is not PCSX2 but rather my computer. XWayland performs rather poorly on my computer due to some obscure driver bug that's probably not gonna get fixed... Wayland doesn't perform great either but having both just screws things up.

Unless we put a shell script in place, but then self builds versus release builds have behaviour differences, which I'd very much like to avoid.

I'm sure there's probably a way to implement the shell script for self builds too? The script itself should be pretty easy through XDG_CURRENT_DESKTOP and XDG_SESSION_TYPE, but you can probably replace the actual executable with a shell script that sets QPA platform according to those variables then runs the application. Doubtless someone could PR that.

Users don't read warnings. They just complain that it's broken.

True. 😆 GNOME is really good at making the entire software development community hate their guts.

@stenzek
Copy link
Contributor Author

stenzek commented Oct 27, 2023

I'm sure there's probably a way to implement the shell script for self builds too?

All this mess for something broken by design. No thanks. I already maintain Linux support, despite not using it on the desktop myself, not adding another thing which can break.

@Sigma0004
Copy link

Sigma0004 commented Oct 30, 2023

"And don't give me the "but I don't use GNOME... ..."
For the sake of your blood pressure you may wish to find a way to blacklist GNOME (similar to what was done previously with AMD's drivers regarding OpenGL) support entirely because they are extremely gung ho (in a very Three Stooges, Manjaro-esque manner [first MR on this had to be reverted due to it borking XWayland]) in dropping Xorg despite Wayland still being far more dough than cookies at this point.
https://news.itsfoss.com/gnome-wayland-xorg/
https://youtu.be/aHon2wGEXuU?si=we2ehBJva2d_X8a-&t=104

@stenzek
Copy link
Contributor Author

stenzek commented Oct 31, 2023

AFAIK they're not going to be dropping Xwayland, which is what PCSX2 falls back to.

@Sigma0004
Copy link

That's why I made the Manjaro crack at their expense. Just because they aren't dropping Xwayland (yet?) doesn't mean they won't find new, irritating and idiotic ways to cause it to break. With them tentatively dropping Xorg support with the major release following the next big one, Wayland still only half-baked (and their implementation of it being particularly clownshoes): it may be prudent to sketch out additional contingencies.

@stenzek
Copy link
Contributor Author

stenzek commented Oct 31, 2023

Like what?

The protocol is fundamentally broken. Things like fullscreen switching opening on the correct monitor will never work, because we can't absolute position a window. Qt has tons of WL-specific issues. None of these are in our control.

If any distros drop XWayland support, then PCSX2 won't be the only thing affected. Steam, tons of games, etc, won't work either. AFAIK even the browsers haven't switched to native WL-by-default yet.

@Sigma0004
Copy link

Sigma0004 commented Oct 31, 2023

The immediately available and the most painful (especially file size) option I can think of would be to include Xwayland as a bundled library/dependency for Flatpak/AppImage builds. Then it wouldn't matter what compositor (or what version for that matter) the Host system has or doesn't have as PCSX2 would have its own instance of Xwayland.
For package manager based distribution in the distant future I guess would be to include a way to test for Wayland Standards Conformance ( the best I've found so far is https://github.com/MirServer/wlcs ) in the dependencies so that an audit of the installed Wayland compositor can be made on loading PCSX2 and whether or not it will lead to a compromised experience.

@stenzek
Copy link
Contributor Author

stenzek commented Oct 31, 2023

Yeah, no, that's definitely not happening.

I think dropping XWayland is a bluff. Nobody's actually going to do it, because they know damn well that so many applications simply won't work. Regardless, if it does happen, we'll deal with it then.

@orowith2os
Copy link

AFAIK even the browsers haven't switched to native WL-by-default yet.

Firefox will default to Wayland when the GNOME camera privacy indicator changes are merged and released. Chrome is Chrome, as always.

If you don't want to deal with CSDs, use libdecor, or let QT deal with them; QT has plugins for each relevant Wayland platform, including GNOME.

Many other "problems" you listed sound more like you problems, not Wayland problems.

@stenzek
Copy link
Contributor Author

stenzek commented Nov 2, 2023

Many other "problems" you listed sound more like you problems, not Wayland problems.

How is not being able to show a window on the screen at a specific position an "us" problem? At the very least, it's a Qt problem, because we're calling a Qt method to do so, but it's a noop on Wayland because the protocol doesn't support it.

If you don't want to deal with CSDs, use libdecor, or let QT deal with them; QT has plugins for each relevant Wayland platform, including GNOME.

Great, I'll look forward to your pull request, and to solve all the other problems I listed above with Qt. I'd very much like to be absolved of maintaining this broken-by-design platform, so I can spend my time working on fun things, and not fighting the OS, or coming up with ugly workarounds (see #10179 (comment)).

Edit: Or, what about NVIDIA crashing when you try to create a swap chain under Wayland? We can't fix these things. All we can do is disable the platform, so that users don't blame and create support headaches for us, for problems outside of our control. Which is exactly what I've done, to ensure a non-broken UX.

If you want to suffer, there's the I_WANT_A_BROKEN_WAYLAND_UI environment variable which I added in #10200, or flatseal (which admittedly I haven't checked).

@Rustmilian

This comment was marked as off-topic.

@PCSX2 PCSX2 locked as off-topic and limited conversation to collaborators Nov 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants