[PATCH weston 1/3] input: Keep per client pointer resources in their own structs
Jonas Ådahl
jadahl at gmail.com
Wed Sep 2 19:18:46 PDT 2015
On Wed, Sep 02, 2015 at 01:15:17PM -0500, Derek Foreman wrote:
> On 29/07/15 01:39 AM, Jonas Ådahl wrote:
> > Keep all per client wl_pointer resources in a new struct called
> > 'weston_pointer_client'. When focus changes, instead of moving a list
> > of resources between different lists, just change the focused pointer
> > client.
> >
> > The intention with this is to make it easier to add wl_pointer
> > extensions that share the same focus as the corresponding wl_pointer.
> > ---
> > desktop-shell/shell.c | 16 +++--
> > src/compositor.h | 11 ++-
> > src/input.c | 186 ++++++++++++++++++++++++++++++++++++++++----------
> > 3 files changed, 169 insertions(+), 44 deletions(-)
> >
> > diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> > index 6044cc9..4d7a484 100644
> > --- a/desktop-shell/shell.c
> > +++ b/desktop-shell/shell.c
> > @@ -3209,6 +3209,7 @@ popup_grab_motion(struct weston_pointer_grab *grab, uint32_t time,
> > {
> > struct weston_pointer *pointer = grab->pointer;
> > struct wl_resource *resource;
> > + struct wl_list *resource_list;
> > wl_fixed_t x, y;
> > wl_fixed_t sx, sy;
> >
> > @@ -3220,7 +3221,8 @@ popup_grab_motion(struct weston_pointer_grab *grab, uint32_t time,
> >
> > weston_pointer_move(pointer, event);
> >
> > - wl_resource_for_each(resource, &pointer->focus_resource_list) {
> > + resource_list = &pointer->focus_client->pointer_resources;
> > + wl_resource_for_each(resource, resource_list) {
> > weston_view_from_global_fixed(pointer->focus,
> > pointer->x, pointer->y,
> > &sx, &sy);
> > @@ -3238,10 +3240,11 @@ popup_grab_button(struct weston_pointer_grab *grab,
> > struct wl_display *display = shseat->seat->compositor->wl_display;
> > enum wl_pointer_button_state state = state_w;
> > uint32_t serial;
> > - struct wl_list *resource_list;
> > + struct wl_list *resource_list = NULL;
> >
> > - resource_list = &grab->pointer->focus_resource_list;
> > - if (!wl_list_empty(resource_list)) {
> > + if (grab->pointer->focus_client)
> > + resource_list = &grab->pointer->focus_client->pointer_resources;
> > + if (resource_list && !wl_list_empty(resource_list)) {
> > serial = wl_display_get_serial(display);
> > wl_resource_for_each(resource, resource_list) {
> > wl_pointer_send_button(resource, serial,
> > @@ -3265,7 +3268,10 @@ popup_grab_axis(struct weston_pointer_grab *grab,
> > struct wl_resource *resource;
> > struct wl_list *resource_list;
> >
> > - resource_list = &pointer->focus_resource_list;
> > + if (!pointer->focus_client)
> > + return;
> > +
> > + resource_list = &pointer->focus_client->pointer_resources;
> > wl_resource_for_each(resource, resource_list)
> > wl_pointer_send_axis(resource, time, axis, value);
> > }
> > diff --git a/src/compositor.h b/src/compositor.h
> > index b98119e..4d5b0e1 100644
> > --- a/src/compositor.h
> > +++ b/src/compositor.h
> > @@ -329,12 +329,19 @@ struct weston_data_source {
> > void (*cancel)(struct weston_data_source *source);
> > };
> >
> > +struct weston_pointer_client {
> > + struct wl_list link;
> > + struct wl_client *client;
> > + struct wl_list pointer_resources;
> > +};
> > +
> > struct weston_pointer {
> > struct weston_seat *seat;
> >
> > - struct wl_list resource_list;
> > - struct wl_list focus_resource_list;
> > + struct wl_list pointer_clients;
> > +
> > struct weston_view *focus;
> > + struct weston_pointer_client *focus_client;
> > uint32_t focus_serial;
> > struct wl_listener focus_view_listener;
> > struct wl_listener focus_resource_listener;
> > diff --git a/src/input.c b/src/input.c
> > index cbe9911..3338fdb 100644
> > --- a/src/input.c
> > +++ b/src/input.c
> > @@ -45,6 +45,95 @@ empty_region(pixman_region32_t *region)
> > pixman_region32_init(region);
> > }
> >
> > +static struct weston_pointer_client *
> > +weston_pointer_client_create(struct wl_client *client)
> > +{
> > + struct weston_pointer_client *pointer_client;
> > +
> > + pointer_client = zalloc(sizeof *pointer_client);
> > + if (!pointer_client)
> > + return NULL;
> > +
> > + pointer_client->client = client;
> > + wl_list_init(&pointer_client->pointer_resources);
> > +
> > + return pointer_client;
> > +}
> > +
> > +static void
> > +weston_pointer_client_destroy(struct weston_pointer_client *pointer_client)
> > +{
> > + free(pointer_client);
> > +}
> > +
> > +static bool
> > +weston_pointer_client_is_empty(struct weston_pointer_client *pointer_client)
> > +{
> > + return wl_list_empty(&pointer_client->pointer_resources);
> > +}
> > +
> > +static struct weston_pointer_client *
> > +weston_pointer_get_pointer_client(struct weston_pointer *pointer,
> > + struct wl_client *client)
> > +{
> > + struct weston_pointer_client *pointer_client;
> > +
> > + wl_list_for_each(pointer_client, &pointer->pointer_clients, link) {
> > + if (pointer_client->client == client)
> > + return pointer_client;
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +static struct weston_pointer_client *
> > +weston_pointer_ensure_pointer_client(struct weston_pointer *pointer,
> > + struct wl_client *client)
> > +{
> > + struct weston_pointer_client *pointer_client;
> > +
> > + pointer_client = weston_pointer_get_pointer_client(pointer, client);
> > + if (pointer_client)
> > + return pointer_client;
> > +
> > + pointer_client = weston_pointer_client_create(client);
> > + wl_list_insert(&pointer->pointer_clients, &pointer_client->link);
> > +
> > + if (pointer->focus &&
> > + pointer->focus->surface->resource &&
> > + wl_resource_get_client(pointer->focus->surface->resource) == client) {
> > + pointer->focus_client = pointer_client;
> > + }
> > +
> > + return pointer_client;
> > +}
> > +
> > +static void
> > +weston_pointer_cleanup_pointer_client(struct weston_pointer *pointer,
> > + struct weston_pointer_client *pointer_client)
> > +{
> > + if (weston_pointer_client_is_empty(pointer_client)) {
> > + if (pointer->focus_client == pointer_client)
> > + pointer->focus_client = NULL;
> > + wl_list_remove(&pointer_client->link);
> > + weston_pointer_client_destroy(pointer_client);
> > + }
> > +}
> > +
> > +static void
> > +unbind_pointer_client_resource(struct wl_resource *resource)
> > +{
> > + struct weston_pointer *pointer = wl_resource_get_user_data(resource);
> > + struct wl_client *client = wl_resource_get_client(resource);
> > + struct weston_pointer_client *pointer_client;
> > +
> > + pointer_client = weston_pointer_get_pointer_client(pointer, client);
> > + assert(pointer_client);
> > +
> > + wl_list_remove(wl_resource_get_link(resource));
> > + weston_pointer_cleanup_pointer_client(pointer, pointer_client);
> > +}
> > +
> > static void unbind_resource(struct wl_resource *resource)
> > {
> > wl_list_remove(wl_resource_get_link(resource));
> > @@ -184,8 +273,9 @@ default_grab_pointer_motion(struct weston_pointer_grab *grab, uint32_t time,
> >
> > weston_pointer_move(pointer, event);
> >
> > - if (old_sx != pointer->sx || old_sy != pointer->sy) {
> > - resource_list = &pointer->focus_resource_list;
> > + if (pointer->focus_client &&
>
> Can we actually get here when pointer->focus_client is NULL? (ie:
> should this be an assert instead?)
If the pointer focus is on a surface which client never requested a
wl_pointer, we'd still notify_motion(), and notify_motion() would still
call the motion vfunc on the grab (for example the default grab, and
thus we need to check if there is actually any resource (no resource
means no focus_client) to send a motion event to.
>
> Anyway...
>
> I like this patch... the move_resources() thing always seemed a bit
> confusing to me. I think in addition to its stated goals, this also
> makes a bit of weston a little easier to follow - which is nice if this
> is supposed to be reference code. :)
>
> Reviewed-by: Derek Foreman <derekf at osg.samsung.com>
Thanks
Jonas
>
> > + (old_sx != pointer->sx || old_sy != pointer->sy)) {
> > + resource_list = &pointer->focus_client->pointer_resources;
> > wl_resource_for_each(resource, resource_list) {
> > wl_pointer_send_motion(resource, time,
> > pointer->sx, pointer->sy);
> > @@ -205,10 +295,12 @@ default_grab_pointer_button(struct weston_pointer_grab *grab,
> > enum wl_pointer_button_state state = state_w;
> > struct wl_display *display = compositor->wl_display;
> > wl_fixed_t sx, sy;
> > - struct wl_list *resource_list;
> > + struct wl_list *resource_list = NULL;
> >
> > - resource_list = &pointer->focus_resource_list;
> > - if (!wl_list_empty(resource_list)) {
> > + if (pointer->focus_client)
> > + resource_list = &pointer->focus_client->pointer_resources;
> > + if (resource_list && !wl_list_empty(resource_list)) {
> > + resource_list = &pointer->focus_client->pointer_resources;
> > serial = wl_display_next_serial(display);
> > wl_resource_for_each(resource, resource_list)
> > wl_pointer_send_button(resource,
> > @@ -246,7 +338,10 @@ weston_pointer_send_axis(struct weston_pointer *pointer,
> > struct wl_resource *resource;
> > struct wl_list *resource_list;
> >
> > - resource_list = &pointer->focus_resource_list;
> > + if (!pointer->focus_client)
> > + return;
> > +
> > + resource_list = &pointer->focus_client->pointer_resources;
> > wl_resource_for_each(resource, resource_list)
> > wl_pointer_send_axis(resource, time, axis, value);
> > }
> > @@ -401,25 +496,41 @@ send_modifiers_to_client_in_list(struct wl_client *client,
> > }
> > }
> >
> > -static struct wl_resource *
> > -find_resource_for_surface(struct wl_list *list, struct weston_surface *surface)
> > +static struct weston_pointer_client *
> > +find_pointer_client_for_surface(struct weston_pointer *pointer,
> > + struct weston_surface *surface)
> > {
> > + struct wl_client *client;
> > +
> > if (!surface)
> > return NULL;
> >
> > if (!surface->resource)
> > return NULL;
> >
> > - return wl_resource_find_for_client(list, wl_resource_get_client(surface->resource));
> > + client = wl_resource_get_client(surface->resource);
> > + return weston_pointer_get_pointer_client(pointer, client);
> > }
> >
> > -static struct wl_resource *
> > -find_resource_for_view(struct wl_list *list, struct weston_view *view)
> > +static struct weston_pointer_client *
> > +find_pointer_client_for_view(struct weston_pointer *pointer, struct weston_view *view)
> > {
> > if (!view)
> > return NULL;
> >
> > - return find_resource_for_surface(list, view->surface);
> > + return find_pointer_client_for_surface(pointer, view->surface);
> > +}
> > +
> > +static struct wl_resource *
> > +find_resource_for_surface(struct wl_list *list, struct weston_surface *surface)
> > +{
> > + if (!surface)
> > + return NULL;
> > +
> > + if (!surface->resource)
> > + return NULL;
> > +
> > + return wl_resource_find_for_client(list, wl_resource_get_client(surface->resource));
> > }
> >
> > static void
> > @@ -506,8 +617,7 @@ weston_pointer_create(struct weston_seat *seat)
> > if (pointer == NULL)
> > return NULL;
> >
> > - wl_list_init(&pointer->resource_list);
> > - wl_list_init(&pointer->focus_resource_list);
> > + wl_list_init(&pointer->pointer_clients);
> > weston_pointer_set_default_grab(pointer,
> > seat->compositor->default_pointer_grab);
> > wl_list_init(&pointer->focus_resource_listener.link);
> > @@ -664,8 +774,10 @@ weston_pointer_set_focus(struct weston_pointer *pointer,
> > struct weston_view *view,
> > wl_fixed_t sx, wl_fixed_t sy)
> > {
> > + struct weston_pointer_client *pointer_client;
> > struct weston_keyboard *kbd = pointer->seat->keyboard;
> > struct wl_resource *resource;
> > + struct wl_resource *surface_resource;
> > struct wl_display *display = pointer->seat->compositor->wl_display;
> > uint32_t serial;
> > struct wl_list *focus_resource_list;
> > @@ -677,21 +789,24 @@ weston_pointer_set_focus(struct weston_pointer *pointer,
> > pointer->sx != sx || pointer->sy != sy)
> > refocus = 1;
> >
> > - focus_resource_list = &pointer->focus_resource_list;
> >
> > - if (!wl_list_empty(focus_resource_list) && refocus) {
> > - serial = wl_display_next_serial(display);
> > - wl_resource_for_each(resource, focus_resource_list) {
> > - wl_pointer_send_leave(resource, serial,
> > - pointer->focus->surface->resource);
> > + if (pointer->focus_client) {
> > + focus_resource_list = &pointer->focus_client->pointer_resources;
> > + if (!wl_list_empty(focus_resource_list)) {
> > + serial = wl_display_next_serial(display);
> > + surface_resource = pointer->focus->surface->resource;
> > + wl_resource_for_each(resource, focus_resource_list) {
> > + wl_pointer_send_leave(resource, serial,
> > + surface_resource);
> > + }
> > }
> >
> > - move_resources(&pointer->resource_list, focus_resource_list);
> > + pointer->focus_client = NULL;
> > }
> >
> > - if (find_resource_for_view(&pointer->resource_list, view) && refocus) {
> > - struct wl_client *surface_client =
> > - wl_resource_get_client(view->surface->resource);
> > + pointer_client = find_pointer_client_for_view(pointer, view);
> > + if (pointer_client && refocus) {
> > + struct wl_client *surface_client = pointer_client->client;
> >
> > serial = wl_display_next_serial(display);
> >
> > @@ -701,10 +816,9 @@ weston_pointer_set_focus(struct weston_pointer *pointer,
> > serial,
> > kbd);
> >
> > - move_resources_for_client(focus_resource_list,
> > - &pointer->resource_list,
> > - surface_client);
> > + pointer->focus_client = pointer_client;
> >
> > + focus_resource_list = &pointer->focus_client->pointer_resources;
> > wl_resource_for_each(resource, focus_resource_list) {
> > wl_pointer_send_enter(resource,
> > serial,
> > @@ -1805,9 +1919,11 @@ seat_get_pointer(struct wl_client *client, struct wl_resource *resource,
> > uint32_t id)
> > {
> > struct weston_seat *seat = wl_resource_get_user_data(resource);
> > + struct weston_pointer *pointer = seat->pointer;
> > struct wl_resource *cr;
> > + struct weston_pointer_client *pointer_client;
> >
> > - if (!seat->pointer)
> > + if (!pointer)
> > return;
> >
> > cr = wl_resource_create(client, &wl_pointer_interface,
> > @@ -1816,13 +1932,12 @@ seat_get_pointer(struct wl_client *client, struct wl_resource *resource,
> > wl_client_post_no_memory(client);
> > return;
> > }
> > -
> > - /* May be moved to focused list later by either
> > - * weston_pointer_set_focus or directly if this client is already
> > - * focused */
> > - wl_list_insert(&seat->pointer->resource_list, wl_resource_get_link(cr));
> > wl_resource_set_implementation(cr, &pointer_interface, seat->pointer,
> > - unbind_resource);
> > + unbind_pointer_client_resource);
> > +
> > + pointer_client = weston_pointer_ensure_pointer_client(pointer, client);
> > + wl_list_insert(&pointer_client->pointer_resources,
> > + wl_resource_get_link(cr));
> >
> > if (seat->pointer->focus && seat->pointer->focus->surface->resource &&
> > wl_resource_get_client(seat->pointer->focus->surface->resource) == client) {
> > @@ -1833,9 +1948,6 @@ seat_get_pointer(struct wl_client *client, struct wl_resource *resource,
> > seat->pointer->y,
> > &sx, &sy);
> >
> > - wl_list_remove(wl_resource_get_link(cr));
> > - wl_list_insert(&seat->pointer->focus_resource_list,
> > - wl_resource_get_link(cr));
> > wl_pointer_send_enter(cr,
> > seat->pointer->focus_serial,
> > seat->pointer->focus->surface->resource,
> >
>
More information about the wayland-devel
mailing list