Update wayland keymap handling for protocol 7+. #5397
Conversation
|
I'm also happy to add new_from_fd to Keyboard/KeyboardWithFallback instead of converting to string and back if we want. I'm just about to submit a PR to rewrite the keyboard handling to use the toolkit, so i didn't bother. (The API guarantees that converting it to a string will result in a valid keymap, so it either fails during new_from_fd, or it will successfully convert to a string and back) |
There was a problem hiding this comment.
Thanks for diving in!
I'm just about to submit a PR to rewrite the keyboard handling to use the toolkit, so i didn't bother.
Let's talk about that before you put in any significant effort on it, as we are deliberately not using sctk's handling because it, once upon a time, had significant issues with repeat handling.
Otherwise, this PR looks OK to me; just one minor comment about preserving more context in an error message.
window/src/os/wayland/keyboard.rs
Outdated
| Err(err) => { | ||
| log::error!("Error processing keymap change: {:#}", err); | ||
| log::error!("{}", err); |
There was a problem hiding this comment.
Please restore the original error formatting here; the additional context is useful and error messages tend to render better as {err:#}.
There was a problem hiding this comment.
Whoops, though i reverted that, but must not have staged it, will fix again and push.
There was a problem hiding this comment.
Should be fixed now
Sure. It was like 1-2 hours of work, and was a good intro to the wayland internals of both anyway, so if it ends up not being usable, that's fine - it was a good learning experience. |
Newer wayland protocol expects us to mmap the keymap file descriptor (and do so with certain mmap flags). The current code tries to handle both normal files and pipes, but neither will work in some cases. This updates the code to use xkbcommon's new_from_fd, which is what the toolkit uses, and mmap's the file. This fixes wez#5393, which was caused by hanging when trying to read the file as if it was a pipe.
|
Thanks! |
|
re: toolkit keyboard, if you think that it has some advantages over what we're doing currently and already have something we can discuss in a PR, that's great. Otherwise, I'd like to discuss what you think the advantages are before you spend too much time on it--I don't want you to feel like you've wasted your own time! Thank you! |
I'll create a PR and we can discuss. I did it more to try to see what was going on than anything else, so if we decide it's not worth it, or won't support what we need, it really is no big thing. If I was going to spend days on it, I would have started a discussion beforehand so i didn't throw away a lot of work :) |
Newer wayland protocol expects us to mmap the keymap file
descriptor (and do so with certain mmap flags).
The current code tries to handle both normal files and pipes,
but neither will work in some cases.
This updates the code to use xkbcommon's new_from_fd,
which is what the toolkit uses, and mmap's the file.
This fixes #5393, which was caused by hanging when
trying to read the file as if it was a pipe.