From 7dc4a3ecd71cf41dd1800c6afd3b16c83a90f031 Mon Sep 17 00:00:00 2001 From: Simon Zeni Date: Tue, 1 Mar 2022 14:49:30 -0500 Subject: [PATCH] interface/wlr_keyboard: rework destroy sequence The destroy member in wlr_keyboard_impl has been removed. The function `wlr_keyboard_finish` has been introduce to clean up the resources owned by a wlr_keyboard. `wlr_input_device_destroy` no longer destroys the wlr_keyboard, attempting to destroy a wlr_keyboard will result in a no-op. The field `name` has been added to the wlr_keyboard_impl to be able to identify a given wlr_keyboard device. --- backend/libinput/events.c | 2 +- backend/libinput/keyboard.c | 6 +---- backend/wayland/seat.c | 15 +++++++++-- backend/x11/backend.c | 5 ++-- backend/x11/input_device.c | 6 +---- include/wlr/interfaces/wlr_keyboard.h | 11 +++++--- types/wlr_input_device.c | 3 +-- types/wlr_keyboard.c | 25 ++++++++++++------- types/wlr_keyboard_group.c | 16 +++--------- types/wlr_virtual_keyboard_v1.c | 36 ++++----------------------- 10 files changed, 52 insertions(+), 73 deletions(-) diff --git a/backend/libinput/events.c b/backend/libinput/events.c index af5bb184..af54177c 100644 --- a/backend/libinput/events.c +++ b/backend/libinput/events.c @@ -15,7 +15,7 @@ void destroy_libinput_input_device(struct wlr_libinput_input_device *dev) { if (dev->keyboard.impl) { - wlr_keyboard_destroy(&dev->keyboard); + wlr_keyboard_finish(&dev->keyboard); } if (dev->pointer.impl) { wlr_pointer_destroy(&dev->pointer); diff --git a/backend/libinput/keyboard.c b/backend/libinput/keyboard.c index cf6b9836..210fd450 100644 --- a/backend/libinput/keyboard.c +++ b/backend/libinput/keyboard.c @@ -17,12 +17,8 @@ static void keyboard_set_leds(struct wlr_keyboard *wlr_kb, uint32_t leds) { libinput_device_led_update(dev->handle, leds); } -static void keyboard_destroy(struct wlr_keyboard *wlr_kb) { - /* wlr_keyboard belongs to the wlr_libinput_input_device */ -} - const struct wlr_keyboard_impl libinput_keyboard_impl = { - .destroy = keyboard_destroy, + .name = "libinput-keyboard", .led_update = keyboard_set_leds }; diff --git a/backend/wayland/seat.c b/backend/wayland/seat.c index 28466bb3..e2a61bd4 100644 --- a/backend/wayland/seat.c +++ b/backend/wayland/seat.c @@ -24,7 +24,9 @@ #include "util/time.h" static const struct wlr_pointer_impl pointer_impl; -static const struct wlr_keyboard_impl keyboard_impl; +static const struct wlr_keyboard_impl keyboard_impl = { + .name = "wl-keyboard", +}; static const struct wlr_touch_impl touch_impl; static struct wlr_wl_pointer *output_get_pointer( @@ -506,7 +508,16 @@ void destroy_wl_input_device(struct wlr_wl_input_device *dev) { */ wlr_input_device_finish(&dev->wlr_input_device); if (dev->wlr_input_device._device) { - wlr_input_device_destroy(&dev->wlr_input_device); + struct wlr_input_device *wlr_dev = &dev->wlr_input_device; + switch (wlr_dev->type) { + case WLR_INPUT_DEVICE_KEYBOARD: + wlr_keyboard_finish(wlr_dev->keyboard); + free(wlr_dev->keyboard); + break; + default: + wlr_input_device_destroy(wlr_dev); + break; + } } wl_list_remove(&dev->link); free(dev); diff --git a/backend/x11/backend.c b/backend/x11/backend.c index 64bcf475..dbbd74e8 100644 --- a/backend/x11/backend.c +++ b/backend/x11/backend.c @@ -185,7 +185,7 @@ static void backend_destroy(struct wlr_backend *backend) { wlr_output_destroy(&output->wlr_output); } - wlr_keyboard_destroy(&x11->keyboard); + wlr_keyboard_finish(&x11->keyboard); wlr_backend_finish(backend); @@ -637,7 +637,8 @@ struct wlr_backend *wlr_x11_backend_create(struct wl_display *display, } #endif - wlr_keyboard_init(&x11->keyboard, &x11_keyboard_impl, "x11-keyboard"); + wlr_keyboard_init(&x11->keyboard, &x11_keyboard_impl, + x11_keyboard_impl.name); x11->display_destroy.notify = handle_display_destroy; wl_display_add_destroy_listener(display, &x11->display_destroy); diff --git a/backend/x11/input_device.c b/backend/x11/input_device.c index f87f53a0..3bbbe594 100644 --- a/backend/x11/input_device.c +++ b/backend/x11/input_device.c @@ -285,12 +285,8 @@ void handle_x11_xinput_event(struct wlr_x11_backend *x11, } } -static void keyboard_destroy(struct wlr_keyboard *wlr_keyboard) { - // Don't free the keyboard, it's on the stack -} - const struct wlr_keyboard_impl x11_keyboard_impl = { - .destroy = keyboard_destroy, + .name = "x11-keyboard", }; static void pointer_destroy(struct wlr_pointer *wlr_pointer) { diff --git a/include/wlr/interfaces/wlr_keyboard.h b/include/wlr/interfaces/wlr_keyboard.h index 4bcef57b..3f4c4255 100644 --- a/include/wlr/interfaces/wlr_keyboard.h +++ b/include/wlr/interfaces/wlr_keyboard.h @@ -13,13 +13,18 @@ #include struct wlr_keyboard_impl { - void (*destroy)(struct wlr_keyboard *keyboard); + const char *name; void (*led_update)(struct wlr_keyboard *keyboard, uint32_t leds); }; void wlr_keyboard_init(struct wlr_keyboard *keyboard, - const struct wlr_keyboard_impl *impl, const char *name); -void wlr_keyboard_destroy(struct wlr_keyboard *keyboard); + const struct wlr_keyboard_impl *impl, const char *name); + +/** + * Cleans up all of the resources owned by wlr_keyboard. + */ +void wlr_keyboard_finish(struct wlr_keyboard *keyboard); + void wlr_keyboard_notify_key(struct wlr_keyboard *keyboard, struct wlr_event_keyboard_key *event); void wlr_keyboard_notify_modifiers(struct wlr_keyboard *keyboard, diff --git a/types/wlr_input_device.c b/types/wlr_input_device.c index 6897febe..4a9ef806 100644 --- a/types/wlr_input_device.c +++ b/types/wlr_input_device.c @@ -2,7 +2,6 @@ #include #include #include -#include #include #include #include @@ -41,7 +40,7 @@ void wlr_input_device_destroy(struct wlr_input_device *dev) { if (dev->_device) { switch (dev->type) { case WLR_INPUT_DEVICE_KEYBOARD: - wlr_keyboard_destroy(dev->keyboard); + wlr_log(WLR_ERROR, "wlr_keyboard will not be destroyed"); break; case WLR_INPUT_DEVICE_POINTER: wlr_pointer_destroy(dev->pointer); diff --git a/types/wlr_keyboard.c b/types/wlr_keyboard.c index afa689b1..372efcb5 100644 --- a/types/wlr_keyboard.c +++ b/types/wlr_keyboard.c @@ -11,6 +11,7 @@ #include "util/array.h" #include "util/shm.h" #include "util/signal.h" +#include "util/time.h" void keyboard_led_update(struct wlr_keyboard *keyboard) { if (keyboard->xkb_state == NULL) { @@ -132,25 +133,31 @@ void wlr_keyboard_init(struct wlr_keyboard *kb, kb->repeat_info.delay = 600; } -void wlr_keyboard_destroy(struct wlr_keyboard *kb) { - if (kb == NULL) { - return; +void wlr_keyboard_finish(struct wlr_keyboard *kb) { + /* Release pressed keys */ + size_t orig_num_keycodes = kb->num_keycodes; + for (size_t i = 0; i < orig_num_keycodes; ++i) { + assert(kb->num_keycodes == orig_num_keycodes - i); + struct wlr_event_keyboard_key event = { + .time_msec = get_current_time_msec(), + .keycode = kb->keycodes[orig_num_keycodes - i - 1], + .update_state = false, + .state = WL_KEYBOARD_KEY_STATE_RELEASED, + }; + wlr_keyboard_notify_key(kb, &event); // updates num_keycodes } + wlr_signal_emit_safe(&kb->events.destroy, kb); + wlr_input_device_finish(&kb->base); + /* Finish xkbcommon resources */ xkb_state_unref(kb->xkb_state); xkb_keymap_unref(kb->keymap); free(kb->keymap_string); if (kb->keymap_fd >= 0) { close(kb->keymap_fd); } - if (kb->impl && kb->impl->destroy) { - kb->impl->destroy(kb); - } else { - wl_list_remove(&kb->events.key.listener_list); - free(kb); - } } void wlr_keyboard_led_update(struct wlr_keyboard *kb, uint32_t leds) { diff --git a/types/wlr_keyboard_group.c b/types/wlr_keyboard_group.c index 35fc5470..465e9944 100644 --- a/types/wlr_keyboard_group.c +++ b/types/wlr_keyboard_group.c @@ -37,18 +37,8 @@ static void keyboard_set_leds(struct wlr_keyboard *kb, uint32_t leds) { } } -static void keyboard_destroy(struct wlr_keyboard *kb) { - // Just remove the event listeners. The keyboard will be freed as part of - // the wlr_keyboard_group in wlr_keyboard_group_destroy. - wl_list_remove(&kb->events.key.listener_list); - wl_list_remove(&kb->events.modifiers.listener_list); - wl_list_remove(&kb->events.keymap.listener_list); - wl_list_remove(&kb->events.repeat_info.listener_list); - wl_list_remove(&kb->events.destroy.listener_list); -} - static const struct wlr_keyboard_impl impl = { - .destroy = keyboard_destroy, + .name = "keyboard-group", .led_update = keyboard_set_leds }; @@ -60,7 +50,7 @@ struct wlr_keyboard_group *wlr_keyboard_group_create(void) { return NULL; } - wlr_keyboard_init(&group->keyboard, &impl, "keyboard-group"); + wlr_keyboard_init(&group->keyboard, &impl, "wlr_keyboard_group"); wl_list_init(&group->devices); wl_list_init(&group->keys); @@ -325,7 +315,7 @@ void wlr_keyboard_group_destroy(struct wlr_keyboard_group *group) { wl_list_for_each_safe(device, tmp, &group->devices, link) { wlr_keyboard_group_remove_keyboard(group, device->keyboard); } - wlr_keyboard_destroy(&group->keyboard); + wlr_keyboard_finish(&group->keyboard); wl_list_remove(&group->events.enter.listener_list); wl_list_remove(&group->events.leave.listener_list); free(group); diff --git a/types/wlr_virtual_keyboard_v1.c b/types/wlr_virtual_keyboard_v1.c index e40a97e8..5a5ce630 100644 --- a/types/wlr_virtual_keyboard_v1.c +++ b/types/wlr_virtual_keyboard_v1.c @@ -9,36 +9,10 @@ #include #include #include "util/signal.h" -#include "util/time.h" #include "virtual-keyboard-unstable-v1-protocol.h" -/** - * Send release event for each pressed key to bring the keyboard back to - * neutral state. - * - * This may be needed for virtual keyboards. For physical devices, kernel - * or libinput will deal with the removal of devices. - */ -static void keyboard_release_pressed_keys(struct wlr_keyboard *keyboard) { - size_t orig_num_keycodes = keyboard->num_keycodes; - for (size_t i = 0; i < orig_num_keycodes; ++i) { - assert(keyboard->num_keycodes == orig_num_keycodes - i); - struct wlr_event_keyboard_key event = { - .time_msec = get_current_time_msec(), - .keycode = keyboard->keycodes[orig_num_keycodes - i - 1], - .update_state = false, - .state = WL_KEYBOARD_KEY_STATE_RELEASED, - }; - wlr_keyboard_notify_key(keyboard, &event); // updates num_keycodes - } -} - -static void keyboard_destroy(struct wlr_keyboard *wlr_kb) { - /* no-op, keyboard belongs to the wlr_virtual_keyboard_v1 */ -} - static const struct wlr_keyboard_impl keyboard_impl = { - .destroy = keyboard_destroy, + .name = "virtual-keyboard", }; static const struct zwp_virtual_keyboard_v1_interface virtual_keyboard_impl; @@ -135,10 +109,10 @@ static void virtual_keyboard_destroy_resource(struct wl_resource *resource) { return; } - /* TODO: rework wlr_keyboard device destruction */ - keyboard_release_pressed_keys(&keyboard->keyboard); wlr_signal_emit_safe(&keyboard->events.destroy, keyboard); - wlr_keyboard_destroy(&keyboard->keyboard); + + wlr_keyboard_finish(&keyboard->keyboard); + wl_resource_set_user_data(keyboard->resource, NULL); wl_list_remove(&keyboard->link); free(keyboard); @@ -179,7 +153,7 @@ static void virtual_keyboard_manager_create_virtual_keyboard( } wlr_keyboard_init(&virtual_keyboard->keyboard, &keyboard_impl, - "virtual-keyboard"); + "wlr_virtual_keyboard_v1"); struct wl_resource *keyboard_resource = wl_resource_create(client, &zwp_virtual_keyboard_v1_interface, wl_resource_get_version(resource),