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
This commit is contained in:
Markus Ongyerth 2018-07-14 10:29:22 +02:00
parent 74ca2f8fcf
commit 9a6f77fc2c
5 changed files with 54 additions and 76 deletions

View file

@ -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 wlr_seat_client *wlr_seat_client_from_resource(
struct wl_resource *resource); struct wl_resource *resource);
bool wlr_surface_is_pointer_cursor(struct wlr_surface *surface);
#endif #endif

View file

@ -355,7 +355,3 @@ void seat_client_destroy_pointer(struct wl_resource *resource) {
} }
wl_resource_set_user_data(resource, NULL); wl_resource_set_user_data(resource, NULL);
} }
bool wlr_surface_is_pointer_cursor(struct wlr_surface *surface) {
return surface->role == &pointer_cursor_surface_role;
}

View file

@ -16,47 +16,6 @@ struct tablet_pad_auxiliary_user_data {
size_t index; 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, static void handle_tablet_pad_v2_destroy(struct wl_client *client,
struct wl_resource *resource) { struct wl_resource *resource) {
wl_resource_destroy(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); 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, static void handle_tablet_pad_group_v2_destroy(struct wl_client *client,
struct wl_resource *resource) { struct wl_resource *resource) {
wl_resource_destroy(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; size_t i = 0;
struct wlr_tablet_pad_group *group; struct wlr_tablet_pad_group *group;
client->group_count = pad->group_count;
wl_list_for_each(group, &pad->wlr_pad->groups, link) { wl_list_for_each(group, &pad->wlr_pad->groups, link) {
add_tablet_pad_group(pad, client, group, i++); add_tablet_pad_group(pad, client, group, i++);
} }
zwp_tablet_pad_v2_send_done(client->resource); zwp_tablet_pad_v2_send_done(client->resource);
wl_list_insert(&seat->pads, &client->seat_link); 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 = struct wlr_tablet_v2_tablet_pad *pad =
wl_container_of(listener, pad, pad_destroy); 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 *client;
struct wlr_tablet_pad_client_v2 *tmp_client; struct wlr_tablet_pad_client_v2 *tmp_client;
wl_list_for_each_safe(client, tmp_client, &pad->clients, pad_link) { wl_list_for_each_safe(client, tmp_client, &pad->clients, pad_link) {
zwp_tablet_pad_v2_send_removed(client->resource); zwp_tablet_pad_v2_send_removed(client->resource);
destroy_tablet_pad_v2(client->resource); destroy_tablet_pad_v2(client->resource);
} }
wl_list_remove(&pad->clients); wl_list_remove(&pad->clients);

View file

@ -12,8 +12,8 @@
#include <wlr/types/wlr_tablet_v2.h> #include <wlr/types/wlr_tablet_v2.h>
#include <wlr/util/log.h> #include <wlr/util/log.h>
static const struct wlr_surface_role pointer_cursor_surface_role = { static const struct wlr_surface_role tablet_tool_cursor_surface_role = {
.name = "wl_pointer-cursor", .name = "wp_tablet_tool-cursor",
}; };
static void handle_tablet_tool_v2_set_cursor(struct wl_client *client, 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; struct wlr_surface *surface = NULL;
if (surface_resource != NULL) { if (surface_resource != NULL) {
surface = wlr_surface_from_resource(surface_resource); 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)) { surface_resource, ZWP_TABLET_TOOL_V2_ERROR_ROLE)) {
return; 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) { for (; i < tool->num_buttons; ++i) {
if (tool->pressed_buttons[i] == button) { if (tool->pressed_buttons[i] == button) {
found = true; found = true;
wlr_log(WLR_DEBUG, "Found the button \\o/: %u", button);
break; 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 (state == ZWP_TABLET_PAD_V2_BUTTON_STATE_PRESSED && !found) {
if (tool->num_buttons < WLR_TABLET_V2_TOOL_BUTTONS_CAP) { if (tool->num_buttons < WLR_TABLET_V2_TOOL_BUTTONS_CAP) {
i = tool->num_buttons++; 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_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); WLR_TABLET_V2_TOOL_BUTTONS_CAP);
} }
} else {
i = -1;
} }
if (state == ZWP_TABLET_PAD_V2_BUTTON_STATE_RELEASED && found) { 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_buttons[i] = 0;
tool->pressed_serials[i] = 0; tool->pressed_serials[i] = 0;
tool->num_buttons = push_zeroes_to_end(tool->pressed_buttons, WLR_TABLET_V2_TOOL_BUTTONS_CAP); tool->num_buttons = push_zeroes_to_end(tool->pressed_buttons, WLR_TABLET_V2_TOOL_BUTTONS_CAP);

View file

@ -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, void wlr_output_cursor_set_surface(struct wlr_output_cursor *cursor,
struct wlr_surface *surface, int32_t hotspot_x, int32_t hotspot_y) { 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_x *= cursor->output->scale;
hotspot_y *= cursor->output->scale; hotspot_y *= cursor->output->scale;