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

keyboard: allow setting keymap directly #1218

Merged
merged 1 commit into from
Nov 10, 2023

Conversation

nicolasavru
Copy link
Contributor

Also make the Layout.0 public to allow constructing Layouts from u32s.

Also make the Layout.0 public to allow constructing Layouts from u32s.
@Drakulix
Copy link
Member

To be clear: I don't oppose this change.
But what is the use-case for it? It would be easier to review and judge if this should be part of the public api, if I would knew what exactly you do with it.

@nicolasavru
Copy link
Contributor Author

A passthrough compositor. In the wayland client part of my code, I'm getting the keymap as a string/file descriptor and the layout index from update_modifiers (Smithay/client-toolkit#430 for that), and need to pass that information on to clients running against my compositor. So I have the full keymap file and can construct a Keymap, but I don't have the RMLVO string which is what compositors usually construct a keymap from.

@Drakulix Drakulix merged commit 003ca51 into Smithay:master Nov 10, 2023
13 checks passed
@ids1024
Copy link
Member

ids1024 commented Nov 10, 2023

#750 has something similar. But I noticed the API is potentially unsound since xkb::Keymap is reference-counted, in a way that isn't thread safe. (See the unsafe Send impl in src/input/keyboard/mod.rs).

@nicolasavru
Copy link
Contributor Author

I can modify this api to accept a String instead and construct the keymap inside set_keymap with Keymap::new_from_string (that's how I'm constructing the Keymap outside the function currently). I'll send a pr for that in a few min.

nicolasavru added a commit to nicolasavru/smithay that referenced this pull request Nov 10, 2023
@nicolasavru
Copy link
Contributor Author

#1221 for that change. Thanks for catching that.

Drakulix pushed a commit that referenced this pull request Nov 13, 2023
chrisduerr pushed a commit to chrisduerr/smithay that referenced this pull request Nov 15, 2023
chrisduerr pushed a commit to chrisduerr/smithay that referenced this pull request Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants