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

Virtual input #251

Merged
merged 1 commit into from
Jan 31, 2023
Merged

Virtual input #251

merged 1 commit into from
Jan 31, 2023

Conversation

ChemicalXandco
Copy link
Contributor

Successor to #189. Fixed conflicts and updated.

Fixes #21
Fixes #133

Tested with:

WLR_BACKENDS=headless WLR_NO_HARDWARE_CURSORS=1 WLR_LIBINPUT_NO_DEVICES=1 WLR_HEADLESS_OUTPUTS=1 ./cage

seat.c Outdated
@@ -6,11 +6,14 @@
* See the LICENSE file accompanying this file.
*/

#define _XOPEN_SOURCE 500
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is for strdup, then #define _POSIX_C_SOURCE 200809L should be enough.

seat.c Outdated
*/
device->name = strdup(event->suggested_output->name);
/* event->suggested_seat should be checked if we handle multiple seats */
handle_new_pointer(seat, wlr_pointer_from_input_device(device));
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to use wlr_pointer_from_input_device() here, we can just pass pointer.

seat.c Outdated
* its name so we do like other devices
*/
device->name = strdup(event->suggested_output->name);
/* event->suggested_seat should be checked if we handle multiple seats */
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that a TODO? If so, please mark it as such.

seat.c Outdated
* sub-optimal (we could just keep the suggested_output), but just copy
* its name so we do like other devices
*/
device->name = strdup(event->suggested_output->name);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, I don't understand this line. device->name is the input device name, it's unrelated to the output name.

Did you mean pointer->output_name?

seat.c Outdated
{
if (virtual)
Copy link
Contributor

Choose a reason for hiding this comment

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

Style: missing brackets

seat.c Outdated
/* We apparently should not group virtual keyboards,
* so create a new group with it
*/
goto create_new;
Copy link
Contributor

Choose a reason for hiding this comment

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

Style: can we avoid this goto? e.g. by wrapping the code below in an if.

seat.c Outdated
/* If multiple seats are supported, check keyboard->seat
* to select the appropriate one */

handle_new_keyboard(seat, wlr_keyboard_from_input_device(device), true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

seat.c Outdated

seat->virtual_pointer = wlr_virtual_pointer_manager_v1_create(server->wl_display);
wl_signal_add(&seat->virtual_pointer->events.new_virtual_pointer, &seat->new_virtual_pointer);
seat->new_virtual_pointer.notify = handle_virtual_pointer;
Copy link
Contributor

Choose a reason for hiding this comment

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

Here we're creating one virtual-keyboard and virtual-pointer manager per seat. We shouldn't do this, instead we should only create one per server.

seat.c Outdated
* sub-optimal (we could just keep the suggested_output), but just copy
* its name so we do like other devices
*/
wlr_pointer->output_name = strdup(event->suggested_output->name);
Copy link
Contributor

Choose a reason for hiding this comment

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

event->suggested_output might be NULL, we should handle this case instead of crashing.

@Hjdskes Hjdskes merged commit bd5b20e into cage-kiosk:master Jan 31, 2023
@martinetd
Copy link

huh.. nice finding out about this when it is merged and the related issue gets closed :/ I'd have appreciated a heads up, would have been happy to rebase/fix things, it's not like I hadn't been responsive or anything...

oh, well, thanks for taking this in anyway. Closing the other PR.

@emersion
Copy link
Contributor

Eeeh, sorry, I completely missed that other PR...

It would've been nicer to at least record the proper attribution in the Git commit history @ChemicalXandco, next time please add yourself with Co-authored-by.

@septatrix
Copy link

I do not think it fixes #21 or at least an explanation how to enable such a virtual keyboard would be necessary cause I have no idea where I should start if I want to enable on-screen keyboards. It seems like #95 as well as some method to configure which virtual keyboard to invoke would be also required for #21 to be closed.

@emersion
Copy link
Contributor

emersion commented Feb 22, 2023

The virtual keyboard protocol is used by on-screen keyboards, but is not enough: as you've figured out OSKs need layer-shell.

This virtual keyboard patch can be used with CLI tools or remote desktop tools.

@septatrix
Copy link

The virtual keyboard protocol is used by on-screen keyboards, but is not enough: as you've figured out OSKs need layer-shell.

Could you please reopen #21 then?

@emersion
Copy link
Contributor

No, #21 is about the virtual keyboard protocol, not OSKs. Follow #95 instead.

@septatrix
Copy link

septatrix commented Feb 22, 2023

The title may suggest so, though looking at the comments themselves it seems that the participants (including myself) are under the impression that "virtual keyboards" referred to OSKs. While technically the protocol of that name is now implemented, also Hjdskes seems to refer to OSKs as there would otherwise be no reason to link #95 for him.

Could you maybe add "on-screen keyboard" to the title of #95 then such that people looking after this are not as confused as me or rename #21 and reopen it as the comments are about OSKs?

@conqp
Copy link

conqp commented Jul 14, 2023

Will there be a release with this feature?

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.

Add support for virtual pointer protocol Add support for virtual keyboards
6 participants