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

Add an optional app_id field to eframe's NativeOptions for Wayland #3007

Merged
merged 4 commits into from
May 22, 2023

Conversation

thomaskrause
Copy link
Contributor

@thomaskrause thomaskrause commented May 21, 2023

Closes #1600.

The new app_id is used in the window builder to set the Wayland application ID using the winit API, which is e.g. important for a proper configuration in .desktop files.
There should not be any changes in the behavior for existing applications.

The new field is only available when the target_os is set to "linux" and the "wayland" feature is enabled (similar to the fullsize_content field in macOS). The limitation to target_os = "linux" is needed because the winit crate does not export the needed trait on Windows and other Non-Unix systems (causing compilation issues otherwise).
Because this makes the configuration a little bit more difficult to use, I added an example in its documentation.

This is used in the window builder to set the application ID, which is e.g. important for a proper configuration in `.desktop` files under Wayland.
When no application ID is explicitly set, it defaults to the title of the window.
The wayland feature is not sufficent as constraint and it won't compile e.g. under Windows.
While Wayland could also be used on other Unix-Systems like FreeBSD, this would probably need some specific testing. Winit uses the following definition as "wayland_platform" and on which the required packages are available:

> wayland_platform: { all(feature = "wayland", free_unix, not(wasm), not(redox)) },
The title might be used to also communicate state (opened file, ...) to the user and this might have unforeseen consequences for the application ID. It seems to be better to use the old behavior of not setting an application ID in this case. Also add an example on how to set the application ID in the documentation.
@thomaskrause thomaskrause marked this pull request as ready for review May 21, 2023 12:20
@emilk emilk added the eframe Relates to epi and eframe label May 22, 2023
crates/eframe/src/native/epi_integration.rs Outdated Show resolved Hide resolved
Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
@thomaskrause
Copy link
Contributor Author

Thanks for the review! I agree with all the proposed (it is already committed), and would also like the idea of using the app_id for storage (but in a separate PR).

@emilk emilk merged commit cc9f1ad into emilk:master May 22, 2023
@thomaskrause thomaskrause deleted the feature/wayland-app-id branch May 23, 2023 07:32
@emilk emilk mentioned this pull request May 23, 2023
1 task
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support setting application ID for Wayland
3 participants