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

forward x11 and wayland features to glutin #5391

Merged
merged 1 commit into from
Nov 26, 2024
Merged

Conversation

e00E
Copy link
Contributor

@e00E e00E commented Nov 20, 2024

eframe has features for selecting between x11 and wayland. eframe does not forward the features to glutin. This makes glutin always compile with both backends enabled. This change forwards the feature. This allows users of egui to compile less dependencies when they only need one of x11, wayland.

To understand this change, read the glutin Cargo.toml [1] and the glutin build.rs [2]. You always have to enable glutin's glx feature with the x11 feature. The other default features (egl, wgl) stay enabled. This is intentional so that everything continues to work as before. We could further minimize when egl and wgl are enabled, but that is not part of this change. There is little reason to do so because those feature already only add dependencies when you compile glutin for the right platform (for example wgl on windows).

[1] https://github.com/rust-windowing/glutin/blob/v0.32.1/glutin/Cargo.toml
[2] https://github.com/rust-windowing/glutin/blob/v0.32.1/glutin/build.rs

eframe has features for selecting between x11 and wayland. eframe does not forward the features to glutin. This makes glutin always compile with both backends enabled. This change forwards the feature. This allows users of egui to compile less dependencies when they only need one of x11, wayland.

To understand this change, read the glutin Cargo.toml [1] and the glutin build.rs [2]. You always have to enable glutin's glx feature with the x11 feature. The other default features (egl, wgl) stay enabled. This is intentional so that everything continues to work as before. We could further minimize when egl and wgl are enabled, but that is not part of this change. There is little reason to do so because those feature already only add dependencies when you compile glutin for the right platform (for example wgl on windows).

[1] https://github.com/rust-windowing/glutin/blob/v0.32.1/glutin/Cargo.toml
[2] https://github.com/rust-windowing/glutin/blob/v0.32.1/glutin/build.rs
Copy link

Preview available at https://egui-pr-preview.github.io/pr/5391-glutin-features
Note that it might take a couple seconds for the update to show up after the preview_build workflow has completed.

@emilk emilk added eframe Relates to epi and eframe native-linux Problem specific to Linux labels Nov 26, 2024
@emilk emilk merged commit 6359ba7 into emilk:master Nov 26, 2024
26 of 27 checks passed
@emilk
Copy link
Owner

emilk commented Dec 16, 2024

I suspect this PR is the cause of a new problem:

https://github.com/emilk/eframe_template/actions/runs/12357896151/job/34487194281

@emilk
Copy link
Owner

emilk commented Dec 16, 2024

Confirmed that reverting this fixes eframe_template:

@emilk
Copy link
Owner

emilk commented Dec 16, 2024

EDIT: the fix is to add "wayland" as a feature AND update to Rust 1.81

@fluxxcode
Copy link

EDIT: the fix is to add "wayland" as a feature AND update to Rust 1.81

In the changelog and release the change is listed as a normal change. But this should be a breaking change because it is now required to activate this feature. Otherwise there is a lot of confusion if Linux is no longer supported after an eframe update with default features disabled.

@emilk
Copy link
Owner

emilk commented Dec 17, 2024

@fluxxcode I've updated https://github.com/emilk/egui/releases/tag/0.30.0 with a "BREAKING: " prefix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
eframe Relates to epi and eframe native-linux Problem specific to Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants