Skip to content

use set_keymap and get_keymap to proper handle keymap #1903

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.

Sign up for GitHub

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

Jump to bottom
Open
Bot-wxt1221 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

use set_keymap and get_keymap to proper handle keymap #1903

Bot-wxt1221 wants to merge 2 commits into from

Conversation

@Bot-wxt1221
Copy link
Contributor

@Bot-wxt1221 Bot-wxt1221 commented Jan 11, 2026 •
edited
Loading

WM need keymap to process input. So change_keymap is neccessary.

Follow by #1803.

Applied #1907 (I changed it a little bit as I was overflowded by the complie error. I wonder if it's the best practice.)

Problem mainly on YaLTeR/niri#2314

Current impl is stupid. The correct way should be simulate a virtual keyboard handler like libinput backend.

But I have no interest in it. So let's just make it possible to solve all issue.

I believe it will solve all the problem.

By the way, I'm not familiar with rust so maybe a rust expert should review my code.

@Bot-wxt1221 Bot-wxt1221 force-pushed the virtual_keyboard branch 2 times, most recently from 3d6c503 to a9a6e0a Compare January 11, 2026 10:22
@Bot-wxt1221
Copy link
Contributor Author

Bot-wxt1221 commented Jan 11, 2026

@PolyMeilex

@Bot-wxt1221 Bot-wxt1221 changed the title fix: use change_keymap to allow WM handle keycode with the correct keymap fix: allow ignore keymap reset to fix virtual_keyboard Jan 12, 2026
@Bot-wxt1221 Bot-wxt1221 force-pushed the virtual_keyboard branch 5 times, most recently from 67f4c7e to b114ff2 Compare January 12, 2026 15:45
@Bot-wxt1221 Bot-wxt1221 marked this pull request as draft January 12, 2026 15:47
@Bot-wxt1221 Bot-wxt1221 changed the title fix: allow ignore keymap reset to fix virtual_keyboard fix: use change_keymap to edit virtual_keyboard's keymap Jan 17, 2026
@Bot-wxt1221 Bot-wxt1221 marked this pull request as ready for review January 17, 2026 16:00
@Bot-wxt1221
Copy link
Contributor Author

Bot-wxt1221 commented Jan 17, 2026

@PolyMeilex

@Bot-wxt1221 Bot-wxt1221 mentioned this pull request Jan 18, 2026
Open
@YaLTeR
Copy link
Contributor

YaLTeR commented Jan 18, 2026

Seems this PR is necessary to unbreak wtype that broke after the last virtual keyboard PR.

@Bot-wxt1221 with this PR, wtype works for clients, but for compositor binds it still uses the wrong keymap. I'm actually not sure what's happening. In winit backend, wtype -M alt -k p -m alt results in

2026-01-18T05:53:17.396689Z DEBUG niri::input: on_keyboard modified=XK_Escape pressed=true
2026-01-18T05:53:17.400344Z DEBUG niri::input: on_keyboard modified=XK_Escape pressed=false

inside the self.niri.seat.get_keyboard().unwrap().input(... closure (and no r somehow?) and ^[r produced in the terminal.

If I do wtype -M alt then something really weird happens where niri still sees all the keys as normal, and compositor binds work, but keys stop doing input to any client.

@chenkeao chenkeao mentioned this pull request Jan 22, 2026
Open
@Bot-wxt1221 Bot-wxt1221 mentioned this pull request Jan 24, 2026
Open
@Bot-wxt1221 Bot-wxt1221 force-pushed the virtual_keyboard branch 2 times, most recently from 721e9ca to 13e102c Compare January 24, 2026 16:22
@Bot-wxt1221 Bot-wxt1221 changed the title fix: use change_keymap to edit virtual_keyboard's keymap use set_keymap and get_keymap to proper handle keymap Jan 24, 2026
@Bot-wxt1221 Bot-wxt1221 force-pushed the virtual_keyboard branch 4 times, most recently from 43ef44c to e18b18b Compare January 25, 2026 02:38
@Bot-wxt1221 Bot-wxt1221 mentioned this pull request Jan 25, 2026
Open
YaLTeR
YaLTeR reviewed Jan 25, 2026
View reviewed changes
YaLTeR
YaLTeR reviewed Jan 25, 2026
View reviewed changes
src/wayland/virtual_keyboard/virtual_keyboard_handle.rs
Comment on lines +158 to +157
let _ = keyboard_handle.set_keymap(user_data, &old_keymap);
let _ = keyboard_handle.set_modifier_state(old_modifiers);
keyboard_handle.advertise_modifier_state(user_data);
Copy link
Contributor

@YaLTeR YaLTeR Jan 25, 2026

Choose a reason for hiding this comment

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

Is it a good idea to reset the keymap right away after every keyboard event? How does it work with modifiers, i.e. won't this code immediately clear the Ctrl modifier after pressing down Ctrl on the virtual kb?

Copy link
Contributor Author

@Bot-wxt1221 Bot-wxt1221 Jan 28, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

wvkbd only send modifier without key event while some send both. Keymap update won't cost too much(in my opinion.)

won't this code immediately clear the Ctrl modifier after pressing down Ctrl on the virtual kb?

A great question but I don't know what's the correct solution. Do you think combine all modifier into one instead of maintaining different is ok?

Paraworker
Paraworker reviewed Jan 25, 2026
View reviewed changes
@my4ng @Bot-wxt1221
feat(input): add pre-compiled keymap
f8b0967 
This allows setting a pre-compiled keymap while ensuring the keymap
was created under the same keyboard handle, and more importantly, the
same xkbcommon context within the same thread.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@YaLTeR YaLTeR YaLTeR left review comments

@Paraworker Paraworker Paraworker left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Bot-wxt1221 @YaLTeR @Paraworker @my4ng