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

Enable Wayland by default #10792

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Enable Wayland by default #10792

wants to merge 5 commits into from

Conversation

amytimed
Copy link

@amytimed amytimed commented Nov 29, 2023

Objective

Solution

Just one commit to add wayland to the default features indeed

Why should it be merged?

The discussion in #4106 seems to be overwhelmingly positive toward adding wayland to the default features, as it can greatly improve user experience for Linux users who use Wayland.

Many of the most popular Linux Desktop Environments (DE) like KDE Plasma Desktop and GNOME have Wayland support. With the release of Plasma 6, Wayland is now used by default, and in GNOME, Wayland has been the default for quite a while now.

With Plasma and GNOME, the most popular Linux DEs, now defaulting to Wayland, it seems like enabling Wayland support would truly indeed positively affect quite a lot of Linux users indeed.

Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

i was told not having one was valid toml after passing it through https://www.toml-lint.com/, but github actions hates me
Copy link
Contributor

You added a new feature but didn't update the readme. Please run cargo run -p build-templated-pages -- update features to update it, and commit the file change.

1 similar comment
Copy link
Contributor

You added a new feature but didn't update the readme. Please run cargo run -p build-templated-pages -- update features to update it, and commit the file change.

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Nov 29, 2023
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Based on user feedback on Linux (and my own experiments), I think this is the right default now. Thanks!

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Nov 29, 2023
@tim-blackbird
Copy link
Contributor

Wayland is currently broken for me. It renders one frame and then the window becomes unresponsive or segfaults outright.
Also doesn't work on #10702.

Winit 0.29's own examples run just fine for me on Wayland, though interestingly enough on 0.27 and 0.28 no window appears at all (no crash).

@alice-i-cecile alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Nov 30, 2023
@Friz64
Copy link
Contributor

Friz64 commented Nov 30, 2023

Wayland is currently broken for me. It renders one frame and then the window becomes unresponsive or segfaults outright.

Do wgpu's examples show the same behavior?

though interestingly enough on 0.27 and 0.28 no window appears at all (no crash)

This, at least, is just because in order for a wayland window to appear, the application needs to have rendered anything, which winit's examples prior to 0.29 did not do.

@tim-blackbird
Copy link
Contributor

Do wgpu's examples show the same behavior?

Yes, no segfault's though.

This, at least, is just because in order for a wayland window to appear, the application needs to have rendered anything, which winit's examples prior to 0.29 did not do.

Aha.

@Friz64
Copy link
Contributor

Friz64 commented Nov 30, 2023

Seems like it's a problem between wgpu and a bug in the latest NVIDIA driver:

@mockersf
Copy link
Member

mockersf commented Dec 4, 2023

I think it's worth discussing going one step further, and remove both x11 and wayland as top level features of Bevy, and just always enable them on winit

@amytimed
Copy link
Author

amytimed commented Dec 4, 2023

perhaps but the Customization directly from Bevy features sure is nice indeed truly

@Friz64
Copy link
Contributor

Friz64 commented Jan 22, 2024

gfx-rs/wgpu#4967 has been merged, which should fix the NVIDIA issue. Unfortunately, it couldn't make it into wgpu 0.19.1 because of breaking API changes: gfx-rs/wgpu#5114 (comment)

@inodentry
Copy link
Contributor

I support keeping the cargo features on the Bevy crate to allow for customizability. Do not remove them in favor of unconditionally enabling both on the winit dependency.

I fully support adding Wayland to default features. I have had this position for years.

It is no longer the case that Wayland users are a small minority or early-adopters of new tech. Most Linux distros nowadays default to Wayland. Wayland should be enabled by default to provide the best experience for users.

Also it is no longer the case that XWayland is just fine for everyone and any Wayland users can be assumed to have a reasonably good experience with XWayland anyway. A few years ago, Wayland was mostly being adopted by users on systems that already had great support for X11. Nowadays, there are many platforms and environments where X11 support is neglected (usually available but works poorly) and only Wayland is well-supported. I personally have quite a few "unusual" systems where I have had to build Bevy with Wayland support for it to be usable and not run into weird issues; to name a few: Asahi Linux, WSLg in WSL2, Chromebooks, ...

Modern Linux distros and desktop development focus on Wayland. X11 is bitrotting.

@amytimed
Copy link
Author

amytimed commented Feb 6, 2024

fixed merge conflicts

@mnmaita
Copy link
Member

mnmaita commented May 21, 2024

@amytimed This also fixes #13340 so maybe you want to add that to the PR description too.

@mockersf
Copy link
Member

The latest number I could find was 33.68% of linux users on wayland: https://www.gamingonlinux.com/users/statistics/#SessionType-top, so still a minority

Is there an equivalent to xvfb for wayland?

@Friz64
Copy link
Contributor

Friz64 commented May 21, 2024

Is there an equivalent to xvfb for wayland?

Take a look at the xwayland-run project.


gfx-rs/wgpu#4967 has been merged, which should fix the NVIDIA issue. Unfortunately, it couldn't make it into wgpu 0.19.1 because of breaking API changes: gfx-rs/wgpu#5114 (comment)

Even though Nvidia has also fixed this issue on their side with a newer driver, I think we should wait until we have updated to wgpu 0.20 to merge this, to be on the safe side. After that, I don't think there's any big blocker left.

Copy link
Contributor

You added a new feature but didn't update the readme. Please run cargo run -p build-templated-pages -- update features to update it, and commit the file change.

Copy link
Contributor

@Friz64 Friz64 left a comment

Choose a reason for hiding this comment

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

Now that #13186 was merged, this can be looked at again.

@@ -78,6 +78,8 @@ default = [
"tonemapping_luts",
"default_font",
"webgl2",
"wayland",
"bevy_debug_stepping",
Copy link
Contributor

@Friz64 Friz64 Jun 16, 2024

Choose a reason for hiding this comment

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

I don't think you meant to enable the bevy_debug_stepping feature here? @amytimed

@benfrankel
Copy link
Contributor

benfrankel commented Jul 19, 2024

To fix CI, this PR should also add libwayland-dev libxkbcommon-dev to every Install Linux dependencies step.

@Friz64
Copy link
Contributor

Friz64 commented Jul 19, 2024

I want to draw attention to this issue: gfx-rs/wgpu#5505

Defaulting to Wayland will break Bevy for Wayland users with no access to Vulkan drivers.

I already have a fix cooking for this, but haven't had the time to finish it yet.

@alice-i-cecile alice-i-cecile added S-Blocked This cannot move forward until something else changes and removed S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Jul 19, 2024
@janhohenheim
Copy link
Member

janhohenheim commented Jul 20, 2024

Removing the X-Controversial label because I think there is a consensus nowadays, based on discussions on GitHub and Discord. Indeed, new users are regularly surprised that Wayland is not already the default. Feel free to add it again if I misread the room :)

@janhohenheim janhohenheim removed the X-Controversial There is active debate or serious implications around merging this PR label Jul 20, 2024
@alice-i-cecile
Copy link
Member

I think "this is blocked on higher quality" is a more accurate read than "this is controversial".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Blocked This cannot move forward until something else changes
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Enable wayland support in examples. Enable wayland by default
10 participants