From 9a6f77fc2ceb59f4b5bcd1e1f8c00aa974b5192b Mon Sep 17 00:00:00 2001 From: Markus Ongyerth Date: Sat, 14 Jul 2018 10:29:22 +0200 Subject: [PATCH] tablet-v2: fix merge commits and test again There were a few issues after rebase, that the merge algorithm didn't throw at my face: wlr_output did a check on the actual role, not a string anymore, so that had to go to allow tablet-v2 to set cursor surfaces. A few L_DEBUG/L_ERRORs were still around There was a user-after-free in tablet-group free()ing, probably after insufficient testing from a previous feedback pass --- include/wlr/types/wlr_seat.h | 2 - types/seat/wlr_seat_pointer.c | 4 -- types/tablet_v2/wlr_tablet_v2_pad.c | 103 +++++++++++---------------- types/tablet_v2/wlr_tablet_v2_tool.c | 16 +++-- types/wlr_output.c | 5 -- 5 files changed, 54 insertions(+), 76 deletions(-) diff --git a/include/wlr/types/wlr_seat.h b/include/wlr/types/wlr_seat.h index 1c7a1472..92d0a233 100644 --- a/include/wlr/types/wlr_seat.h +++ b/include/wlr/types/wlr_seat.h @@ -540,6 +540,4 @@ bool wlr_seat_validate_grab_serial(struct wlr_seat *seat, uint32_t serial); struct wlr_seat_client *wlr_seat_client_from_resource( struct wl_resource *resource); -bool wlr_surface_is_pointer_cursor(struct wlr_surface *surface); - #endif diff --git a/types/seat/wlr_seat_pointer.c b/types/seat/wlr_seat_pointer.c index e54b79bf..899fb64f 100644 --- a/types/seat/wlr_seat_pointer.c +++ b/types/seat/wlr_seat_pointer.c @@ -355,7 +355,3 @@ void seat_client_destroy_pointer(struct wl_resource *resource) { } wl_resource_set_user_data(resource, NULL); } - -bool wlr_surface_is_pointer_cursor(struct wlr_surface *surface) { - return surface->role == &pointer_cursor_surface_role; -} diff --git a/types/tablet_v2/wlr_tablet_v2_pad.c b/types/tablet_v2/wlr_tablet_v2_pad.c index 899f60cd..1fcfa38e 100644 --- a/types/tablet_v2/wlr_tablet_v2_pad.c +++ b/types/tablet_v2/wlr_tablet_v2_pad.c @@ -16,47 +16,6 @@ struct tablet_pad_auxiliary_user_data { size_t index; }; -void destroy_tablet_pad_v2(struct wl_resource *resource) { - struct wlr_tablet_pad_client_v2 *pad = - tablet_pad_client_from_resource(resource); - - if (!pad) { - return; - } - - wl_list_remove(&pad->seat_link); - wl_list_remove(&pad->pad_link); - - /* This isn't optimal, if the client destroys the resources in another - * order, it will be disconnected. - * But this makes things *way* easier for us, and (untested) I doubt - * clients will destroy it in another order. - */ - for (size_t i = 0; i < pad->group_count; ++i) { - if (pad->groups[i]) { - wl_resource_destroy(pad->groups[i]); - } - } - free(pad->groups); - - for (size_t i = 0; i < pad->ring_count; ++i) { - if (pad->rings[i]) { - wl_resource_destroy(pad->rings[i]); - } - } - free(pad->rings); - - for (size_t i = 0; i < pad->strip_count; ++i) { - if (pad->strips[i]) { - wl_resource_destroy(pad->strips[i]); - } - } - free(pad->strips); - - free(pad); - wl_resource_set_user_data(resource, NULL); -} - static void handle_tablet_pad_v2_destroy(struct wl_client *client, struct wl_resource *resource) { wl_resource_destroy(resource); @@ -173,6 +132,47 @@ static void destroy_tablet_pad_group_v2(struct wl_resource *resource) { wl_resource_set_user_data(resource, NULL); } +void destroy_tablet_pad_v2(struct wl_resource *resource) { + struct wlr_tablet_pad_client_v2 *pad = + tablet_pad_client_from_resource(resource); + + if (!pad) { + return; + } + + wl_list_remove(&pad->seat_link); + wl_list_remove(&pad->pad_link); + + /* This isn't optimal, if the client destroys the resources in another + * order, it will be disconnected. + * But this makes things *way* easier for us, and (untested) I doubt + * clients will destroy it in another order. + */ + for (size_t i = 0; i < pad->group_count; ++i) { + if (pad->groups[i]) { + destroy_tablet_pad_group_v2(pad->groups[i]); + } + } + free(pad->groups); + + for (size_t i = 0; i < pad->ring_count; ++i) { + if (pad->rings[i]) { + destroy_tablet_pad_ring_v2(pad->rings[i]); + } + } + free(pad->rings); + + for (size_t i = 0; i < pad->strip_count; ++i) { + if (pad->strips[i]) { + destroy_tablet_pad_strip_v2(pad->strips[i]); + } + } + free(pad->strips); + + free(pad); + wl_resource_set_user_data(resource, NULL); +} + static void handle_tablet_pad_group_v2_destroy(struct wl_client *client, struct wl_resource *resource) { wl_resource_destroy(resource); @@ -321,9 +321,11 @@ void add_tablet_pad_client(struct wlr_tablet_seat_client_v2 *seat, } size_t i = 0; struct wlr_tablet_pad_group *group; + client->group_count = pad->group_count; wl_list_for_each(group, &pad->wlr_pad->groups, link) { add_tablet_pad_group(pad, client, group, i++); } + zwp_tablet_pad_v2_send_done(client->resource); wl_list_insert(&seat->pads, &client->seat_link); @@ -334,30 +336,11 @@ static void handle_wlr_tablet_pad_destroy(struct wl_listener *listener, void *da struct wlr_tablet_v2_tablet_pad *pad = wl_container_of(listener, pad, pad_destroy); - struct wlr_tablet_pad_client_v2 *pos; - struct wlr_tablet_pad_client_v2 *tmp; - wl_list_for_each_safe(pos, tmp, &pad->clients, pad_link) { - zwp_tablet_pad_v2_send_removed(pos->resource); - - for (size_t i = 0; i < pos->group_count; ++i) { - destroy_tablet_pad_group_v2(pos->groups[i]); - } - - for (size_t i = 0; i < pos->strip_count; ++i) { - destroy_tablet_pad_strip_v2(pos->strips[i]); - } - - for (size_t i = 0; i < pos->ring_count; ++i) { - destroy_tablet_pad_ring_v2(pos->rings[i]); - } - } - struct wlr_tablet_pad_client_v2 *client; struct wlr_tablet_pad_client_v2 *tmp_client; wl_list_for_each_safe(client, tmp_client, &pad->clients, pad_link) { zwp_tablet_pad_v2_send_removed(client->resource); destroy_tablet_pad_v2(client->resource); - } wl_list_remove(&pad->clients); diff --git a/types/tablet_v2/wlr_tablet_v2_tool.c b/types/tablet_v2/wlr_tablet_v2_tool.c index c7a1fa54..62c00a37 100644 --- a/types/tablet_v2/wlr_tablet_v2_tool.c +++ b/types/tablet_v2/wlr_tablet_v2_tool.c @@ -12,8 +12,8 @@ #include #include -static const struct wlr_surface_role pointer_cursor_surface_role = { - .name = "wl_pointer-cursor", +static const struct wlr_surface_role tablet_tool_cursor_surface_role = { + .name = "wp_tablet_tool-cursor", }; static void handle_tablet_tool_v2_set_cursor(struct wl_client *client, @@ -28,7 +28,7 @@ static void handle_tablet_tool_v2_set_cursor(struct wl_client *client, struct wlr_surface *surface = NULL; if (surface_resource != NULL) { surface = wlr_surface_from_resource(surface_resource); - if (!wlr_surface_set_role(surface, &pointer_cursor_surface_role, NULL, + if (!wlr_surface_set_role(surface, &tablet_tool_cursor_surface_role, NULL, surface_resource, ZWP_TABLET_TOOL_V2_ERROR_ROLE)) { return; } @@ -241,10 +241,17 @@ static ssize_t tablet_tool_button_update(struct wlr_tablet_v2_tablet_tool *tool, for (; i < tool->num_buttons; ++i) { if (tool->pressed_buttons[i] == button) { found = true; + wlr_log(WLR_DEBUG, "Found the button \\o/: %u", button); break; + } } + if (state == ZWP_TABLET_PAD_V2_BUTTON_STATE_PRESSED && found) { + /* Already have the button saved, durr */ + return -1; + } + if (state == ZWP_TABLET_PAD_V2_BUTTON_STATE_PRESSED && !found) { if (tool->num_buttons < WLR_TABLET_V2_TOOL_BUTTONS_CAP) { i = tool->num_buttons++; @@ -255,10 +262,9 @@ static ssize_t tablet_tool_button_update(struct wlr_tablet_v2_tablet_tool *tool, wlr_log(WLR_ERROR, "You pressed more than %d tablet tool buttons. This is currently not supporte by wlroots. Please report this with a description of your tablet, since this is either a bug, or fancy hardware", WLR_TABLET_V2_TOOL_BUTTONS_CAP); } - } else { - i = -1; } if (state == ZWP_TABLET_PAD_V2_BUTTON_STATE_RELEASED && found) { + wlr_log(WLR_DEBUG, "Removed the button \\o/: %u", button); tool->pressed_buttons[i] = 0; tool->pressed_serials[i] = 0; tool->num_buttons = push_zeroes_to_end(tool->pressed_buttons, WLR_TABLET_V2_TOOL_BUTTONS_CAP); diff --git a/types/wlr_output.c b/types/wlr_output.c index 35b8c416..a1d00c0c 100644 --- a/types/wlr_output.c +++ b/types/wlr_output.c @@ -817,11 +817,6 @@ static void output_cursor_handle_destroy(struct wl_listener *listener, void wlr_output_cursor_set_surface(struct wlr_output_cursor *cursor, struct wlr_surface *surface, int32_t hotspot_x, int32_t hotspot_y) { - if (surface && !wlr_surface_is_pointer_cursor(surface)) { - wlr_log(WLR_ERROR, "Tried to set a cursor surface with invalid role"); - return; - } - hotspot_x *= cursor->output->scale; hotspot_y *= cursor->output->scale;