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

input: Add a way to add a keyboard to the seat from an xkb::keymap #750

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ids1024
Copy link
Member

@ids1024 ids1024 commented Sep 16, 2022

This should provide a way to fix things like pop-os/xdg-shell-wrapper#8.

Cosmic's panel is implemented as a compositor that hosts applets as clients. It should just pass through the same keymap that is provided by the host compositor, rather than trying to create one from any configuration.

This could also be useful for the Wayland backend (#676), depending on use case. Then a compositor using the Wayland backend would respect the host compositors keymap settings, but for some testing it may be helpful to do otherwise.

Leaving this as a draft until I've tested it by incorporating it into xdg-shell-wrapper/cosmic-panel, and also making sure it passes through changes to the group communicated by wl_keyboard::modifiers.

ids1024 added a commit to ids1024/client-toolkit that referenced this pull request Sep 19, 2022
This is meant to provide a way to address
pop-os/xdg-shell-wrapper#8, in combination
with Smithay/smithay#750.

This is currently potentially unsound due to the
`unsafe impl Send for KeyboardData {}`. `xdg::Keymap` uses
non-thread-safe ref-counting, but sctk assumes it is only used from one
thread. This is difficult to address.
ids1024 added a commit to pop-os/xdg-shell-wrapper that referenced this pull request Sep 19, 2022
Fix for #8, though due
to #10, changes to the
selected layout group aren't forwarded from the server.

Requires Smithay/client-toolkit#299 and
Smithay/smithay#750.

I believe this results in unregistering the `wl_keyboard` and creating a
new one. Having a way in smithay to alter the existing `wl_keyboard` may
work better.

The above PRs also have soundness issues due to the non-thread-safe
ref-counting in xdgcommon, and how smithay/sctk assume they
hold the only references.
@ids1024
Copy link
Member Author

ids1024 commented Sep 20, 2022

This is potentially unsound since xkb::Keymap is refcounted (in a non-thread safe way), and Smithay's keyboard code assumes it owns the only references and can move it between threads.

This should provide a way to fix things like
pop-os/xdg-shell-wrapper#8.

Cosmic's panel is implemented as a compositor that hosts applets as
clients. It should just pass through the same keymap that is provided by
the host compositor, rather than trying to create one from any
configuration.

This could also be useful for the Wayland backend
(Smithay#676), depending on use case.
Then a compositor using the Wayland backend would respect the host
compositors keymap settings, but for some testing it may be helpful to
do otherwise.

This cannot directly accept an `xkb::Keymap` due to how `Keymap` uses
reference counting and is not thread safe. Or rather it could, but only
by converting it to a string and back internally.
@ids1024 ids1024 force-pushed the add_keyboard_from_keymap branch from 80a4227 to bb01e4c Compare September 22, 2022 23:22
ids1024 added a commit to ids1024/client-toolkit that referenced this pull request Sep 23, 2022
This is meant to provide a way to address
pop-os/xdg-shell-wrapper#8, in combination
with Smithay/smithay#750.

For soundness reasons, and to avoid allocating when unused (in most
clients), this has an awkward API that provides a way to get the keymap
as a string.
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.

1 participant