Snipped a lot of context this time... any snipped review I've already
incorporated into the next patch set.

On 01/07/15 03:56 AM, Jonas Ådahl wrote:
> On Wed, Jun 03, 2015 at 03:53:38PM -0500, Derek Foreman wrote:
>> Keyboards and pointers aren't freed when devices are removed, so we should
>> really be testing keyboard_device_count and pointer_device_count in most
>> cases, not the actual pointers. Otherwise we end up with different
>> behaviour after removing a device than we had before it was inserted.
>>
>> This commit renames the touch/keyboard/pointer pointers and adds helper
>> functions to get them that hide this complexity and return NULL when
>> *_device_count is 0.
>>
>> Signed-off-by: Derek Foreman <[email protected]>
> 
> Overall I think it looks good, but some comments follow.
> 
>> ---
>>  desktop-shell/exposay.c             |  34 +++---
>>  desktop-shell/input-panel.c         |   7 +-
>>  desktop-shell/shell.c               | 191 ++++++++++++++++++------------
>>  fullscreen-shell/fullscreen-shell.c |  20 +++-
>>  ivi-shell/hmi-controller.c          |  30 +++--
>>  ivi-shell/input-panel-ivi.c         |   7 +-
>>  src/compositor-drm.c                |  10 +-
>>  src/compositor-wayland.c            |   6 +-
>>  src/compositor-x11.c                |  18 ++-
>>  src/compositor.c                    |  34 ++++--
>>  src/compositor.h                    |  15 ++-
>>  src/data-device.c                   |  41 ++++---
>>  src/input.c                         | 230 
>> +++++++++++++++++++++++++-----------
>>  src/libinput-seat.c                 |  15 ++-
>>  src/text-backend.c                  |  19 +--
>>  src/zoom.c                          |   8 +-
>>  tests/surface-screenshot.c          |   7 +-
>>  tests/weston-test.c                 |   9 +-
>>  xwayland/dnd.c                      |   3 +-
>>  xwayland/window-manager.c           |  23 ++--
>>  20 files changed, 468 insertions(+), 259 deletions(-)
>>

<snip>

>> -    return surface_resize(shsurf, ws->pointer, edges);
>> +    return surface_resize(shsurf, weston_seat_get_pointer(ws), edges);
>>  }
>>  
>>  static const struct weston_pointer_grab_interface popup_grab_interface;
>> @@ -3097,19 +3126,22 @@ shell_seat_caps_changed(struct wl_listener 
>> *listener, void *data)
>>  
>>      seat = container_of(listener, struct shell_seat, caps_changed_listener);
>>  
>> -    if (seat->seat->keyboard &&
>> +    /* this is one of the few places where seat->keyboard_resource and
>> +    * seat->pointer_resource should be tested directly, instead of
>> +    * through weston_seat_get_* */
> 
> Your comment just says what the code below does. It'd be more helpful
> describing why it is needed, instead of that it is needed.
> 
> Also the old naming is used and the indentation on the two last lines of
> the comment is wrong. The last */ should be on its own line.

Funny thing, I started re-writing that comment and couldn't figure out
WHY it needed to be this way.  Then I changed it all to use the helper
functions and the devices test hung up.

Well turns out calling wl_list_init() on an item in the middle of a
linked list just puts a loop in the list, and we never saw this
behaviour before because we were testing the pointer that never went away.

Fix in a separate patch.  :)

>> +    if (seat->seat->keyboard_ptr &&
>>          wl_list_empty(&seat->keyboard_focus_listener.link)) {
>> -            wl_signal_add(&seat->seat->keyboard->focus_signal,
>> +            wl_signal_add(&seat->seat->keyboard_ptr->focus_signal,
>>                            &seat->keyboard_focus_listener);
>> -    } else if (!seat->seat->keyboard) {
>> +    } else if (!seat->seat->keyboard_ptr) {
>>              wl_list_init(&seat->keyboard_focus_listener.link);
>>      }
>>  
>> -    if (seat->seat->pointer &&
>> +    if (seat->seat->pointer_ptr &&
>>          wl_list_empty(&seat->pointer_focus_listener.link)) {
>> -            wl_signal_add(&seat->seat->pointer->focus_signal,
>> +            wl_signal_add(&seat->seat->pointer_ptr->focus_signal,
>>                            &seat->pointer_focus_listener);
>> -    } else if (!seat->seat->pointer) {
>> +    } else if (!seat->seat->pointer_ptr) {
>>              wl_list_init(&seat->pointer_focus_listener.link);
>>      }
>>  }

<snip>

>> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
>> index 15f58b7..c7ad08a 100644
>> --- a/src/compositor-wayland.c
>> +++ b/src/compositor-wayland.c
>> @@ -1474,7 +1474,7 @@ input_handle_keymap(void *data, struct wl_keyboard 
>> *keyboard, uint32_t format,
>>  
>>      close(fd);
>>  
>> -    if (input->base.keyboard)
>> +    if (weston_seat_get_keyboard(&input->base))
>>              weston_seat_update_keymap(&input->base, keymap);
>>      else
>>              weston_seat_init_keyboard(&input->base, keymap);
>> @@ -1569,6 +1569,7 @@ input_handle_modifiers(void *data, struct wl_keyboard 
>> *keyboard,
>>                     uint32_t mods_latched, uint32_t mods_locked,
>>                     uint32_t group)
>>  {
>> +    struct weston_keyboard *weston_keyboard;
> 
> Personally I'd prefer always calling a struct weston_keyboard *
> "keyboard", because of grep:ability, but no big deal I suppose.

Done.  I left it because of the wl_keyboard *keyboard passed in, but
nothing even uses that here so I just renamed it.

>>      struct wayland_input *input = data;
>>      struct wayland_compositor *c = input->compositor;
>>      uint32_t serial_out;
>> @@ -1581,7 +1582,8 @@ input_handle_modifiers(void *data, struct wl_keyboard 
>> *keyboard,
>>      else
>>              serial_out = wl_display_next_serial(c->base.wl_display);
>>  
>> -    xkb_state_update_mask(input->base.keyboard->xkb_state.state,
>> +    weston_keyboard = weston_seat_get_keyboard(&input->base);
>> +    xkb_state_update_mask(weston_keyboard->xkb_state.state,
>>                            mods_depressed, mods_latched,
>>                            mods_locked, 0, 0, group);
>>      notify_modifiers(&input->base, serial_out);

<snip>

>> diff --git a/src/compositor.h b/src/compositor.h
>> index 36fe3c7..6e4812e 100644
>> --- a/src/compositor.h
>> +++ b/src/compositor.h
>> @@ -509,9 +509,9 @@ struct weston_seat {
>>      struct wl_list base_resource_list;
>>  
>>      struct wl_global *global;
>> -    struct weston_pointer *pointer;
>> -    struct weston_keyboard *keyboard;
>> -    struct weston_touch *touch;
>> +    struct weston_pointer *pointer_ptr;
>> +    struct weston_keyboard *keyboard_ptr;
>> +    struct weston_touch *touch_ptr;
> 
> bikeshed: I think _ptr naming is confusing. It's already a pointer, so
> why is it in the name of these in particular? I think a better naming
> would be _state, considering that they contain state, and are not
> free:ed because the state needs to stay intact.

No strong opinion on my part so I'll name it _state.

>>      int pointer_device_count;
>>      int keyboard_device_count;
>>      int touch_device_count;
>> @@ -1576,6 +1576,15 @@ weston_parse_transform(const char *transform, 
>> uint32_t *out);
>>  const char *
>>  weston_transform_to_string(uint32_t output_transform);
>>  
>> +struct weston_keyboard *
>> +weston_seat_get_keyboard(struct weston_seat *seat);
>> +
>> +struct weston_pointer *
>> +weston_seat_get_pointer(struct weston_seat *seat);
>> +
>> +struct weston_touch *
>> +weston_seat_get_touch(struct weston_seat *seat);
>> +
>>  #ifdef  __cplusplus
>>  }
>>  #endif

<snip>

>> @@ -1503,7 +1508,7 @@ notify_touch(struct weston_seat *seat, uint32_t time, 
>> int touch_id,
>>               wl_fixed_t x, wl_fixed_t y, int touch_type)
>>  {
>>      struct weston_compositor *ec = seat->compositor;
>> -    struct weston_touch *touch = seat->touch;
>> +    struct weston_touch *touch = weston_seat_get_touch(seat);
>>      struct weston_touch_grab *grab = touch->grab;
>>      struct weston_view *ev;
>>      wl_fixed_t sx, sy;
>> @@ -1582,7 +1587,7 @@ notify_touch(struct weston_seat *seat, uint32_t time, 
>> int touch_id,
>>  WL_EXPORT void
>>  notify_touch_frame(struct weston_seat *seat)
>>  {
>> -    struct weston_touch *touch = seat->touch;
>> +    struct weston_touch *touch = weston_seat_get_touch(seat);
>>      struct weston_touch_grab *grab = touch->grab;
>>  
>>      grab->interface->frame(grab);
>> @@ -1702,9 +1707,13 @@ seat_get_pointer(struct wl_client *client, struct 
>> wl_resource *resource,
>>               uint32_t id)
>>  {
>>      struct weston_seat *seat = wl_resource_get_user_data(resource);
>> +    /* We use the pointer_resource directly here because we only
>> +     * want to fail if the seat never contained a pointer.
>> +     */
> 
> Is this true? We'd unadvertise the existance of a pointer capability,
> and "it only takes effect when the seat has the pointer capability".
> I.e. it should be weston_seat_get_pointer() here as well.
> 
> On the other hand, personally I'd rather see wl_seat.get_* always take
> effect no matter what capabilities are advertised so that we don't have
> that protocol violation there any more.
> 
> Same comment applies to the two equivalent ones below.

Well, devices_test currently checks to make sure we behave this way.
The test fails if I make this and the other 2 functions use
weston_seat_get_*().

There's a description of why the test fails in the comments for
TEST(get_device_after_destroy):
        /* There's a race:
         *  1) compositor destroyes device
         *  2) client asks for the device, because it has not got
         *     new capabilities yet
         *  3) compositor gets request with new_id for destroyed device
         *  4) client uses the new_id
         *  5) client gets new capabilities, destroying the objects
         *
         * If compositor just bail out in step 3) and does not create
         * resource, then client gets error in step 4) - even though
         * it followed the protocol (it just didn't know about new
         * capabilities).

Unless I'm mistaken, which I may be, this needs to remain like this to
prevent this race? :(

>> +    struct weston_pointer *pointer = seat->pointer_ptr;
>>      struct wl_resource *cr;
>>  
>> -    if (!seat->pointer)
>> +    if (!pointer)
>>              return;
>>  
>>          cr = wl_resource_create(client, &wl_pointer_interface,

<snip>

>> @@ -1866,12 +1885,12 @@ seat_get_touch(struct wl_client *client, struct 
>> wl_resource *resource,
>>              return;
>>      }
>>  
>> -    if (seat->touch->focus &&
>> -        wl_resource_get_client(seat->touch->focus->surface->resource) == 
>> client) {
>> -            wl_list_insert(&seat->touch->resource_list,
>> +    if (touch->focus &&
>> +        wl_resource_get_client(touch->focus->surface->resource) == client) {
>> +            wl_list_insert(&touch->resource_list,
>>                             wl_resource_get_link(cr));
>>      } else {
>> -            wl_list_insert(&seat->touch->focus_resource_list,
>> +            wl_list_insert(&touch->focus_resource_list,
>>                             wl_resource_get_link(cr));
>>      }
>>      wl_resource_set_implementation(cr, &touch_interface,
>> @@ -1897,11 +1916,11 @@ bind_seat(struct wl_client *client, void *data, 
>> uint32_t version, uint32_t id)
>>      wl_resource_set_implementation(resource, &seat_interface, data,
>>                                     unbind_resource);
>>  
>> -    if (seat->pointer)
>> +    if (weston_seat_get_pointer(seat))
>>              caps |= WL_SEAT_CAPABILITY_POINTER;
>> -    if (seat->keyboard)
>> +    if (weston_seat_get_keyboard(seat))
>>              caps |= WL_SEAT_CAPABILITY_KEYBOARD;
>> -    if (seat->touch)
>> +    if (weston_seat_get_touch(seat))
>>              caps |= WL_SEAT_CAPABILITY_TOUCH;
> 
> Huh, looks like we did the wrong thing when binding the wl_seat prior
> to this patch here.

Yeah - I've moved this hunk into a separate patch for the next round as
it's a bug fix/functional change.

>>  
>>      wl_seat_send_capabilities(resource, caps);
>> @@ -2091,17 +2110,19 @@ weston_compositor_xkb_destroy(struct 
>> weston_compositor *ec)
>>  WL_EXPORT void
>>  weston_seat_update_keymap(struct weston_seat *seat, struct xkb_keymap 
>> *keymap)
>>  {
>> -    if (!seat->keyboard || !keymap)
>> +    struct weston_keyboard *keyboard = weston_seat_get_keyboard(seat);
>> +
>> +    if (!keyboard || !keymap)
>>              return;
>>  
>>  #ifdef ENABLE_XKBCOMMON
>>      if (!seat->compositor->use_xkbcommon)
>>              return;
>>  
>> -    xkb_keymap_unref(seat->keyboard->pending_keymap);
>> -    seat->keyboard->pending_keymap = xkb_keymap_ref(keymap);
>> +    xkb_keymap_unref(keyboard->pending_keymap);
>> +    keyboard->pending_keymap = xkb_keymap_ref(keymap);
>>  
>> -    if (seat->keyboard->keys.size == 0)
>> +    if (keyboard->keys.size == 0)
>>              update_keymap(seat);
>>  #endif
>>  }

<snip>

>> @@ -2330,3 +2351,68 @@ weston_seat_release(struct weston_seat *seat)
>>  
>>      wl_signal_emit(&seat->destroy_signal, seat);
>>  }
>> +
>> +/** Get a seat's keyboard pointer
>> + *
>> + * \param seat The seat to query
>> + * \return The seat's keyboard pointer, or NULL if no keyboard present
> 
> "if no keyboard *is* present"

All following grammar/style changes accepted for next patch.

Thanks,
Derek

>> + *
>> + * The keyboard pointer for a seat isn't freed when all keyboards are 
>> removed,
>> + * so should only be used when the seat's keyboard_device_count is greater
> 
> "so *it* should only be used"
> 
>> + * than zero.  This function does that test and only returns a pointer
>> + * when a keyboard is present.
>> + */
>> +WL_EXPORT struct weston_keyboard *
>> +weston_seat_get_keyboard(struct weston_seat *seat)
>> +{
>> +    if (!seat)
>> +            return NULL;
>> +
>> +    if (seat->keyboard_device_count)
>> +            return seat->keyboard_ptr;
>> +
>> +    return NULL;
>> +}
>> +
>> +/** Get a seat's pointer pointer
>> + *
>> + * \param seat The seat to query
>> + * \return The seat's pointer pointer, or NULL if no pointing device present
> 
> "if no pointer device *is* present"
> 
>> + *
>> + * The pointer pointer for a seat isn't freed when all mice are removed,
>> + * so should only be used when the seat's pointer_device_count is greater
> 
> "so *it* should only be used"
> 
>> + * than zero.  This function does that test and only returns a pointer
>> + * when a pointing device is present.
>> + */
>> +WL_EXPORT struct weston_pointer *
>> +weston_seat_get_pointer(struct weston_seat *seat)
>> +{
>> +    if (!seat)
>> +            return NULL;
>> +
>> +    if (seat->pointer_device_count)
>> +            return seat->pointer_ptr;
> 
> style: No extra \n here while that the two others have.
> 
>> +    return NULL;
>> +}
>> +
>> +/** Get a seat's touch pointer
>> + *
>> + * \param seat The seat to query
>> + * \return The seat's touch pointer, or NULL if no touch device present
> 
> "no touch device *is* present"
> 
>> + *
>> + * The touch pointer for a seat isn't freed when all touch devices are 
>> removed,
>> + * so should only be used when the seat's touch_device_count is greater
> 
> "so *it* should only be used"
> 
> 
> Jonas
> 
>> + * than zero.  This function does that test and only returns a pointer
>> + * when a touch device is present.
>> + */
>> +WL_EXPORT struct weston_touch *
>> +weston_seat_get_touch(struct weston_seat *seat)
>> +{
>> +    if (!seat)
>> +            return NULL;
>> +
>> +    if (seat->touch_device_count)
>> +            return seat->touch_ptr;
>> +
>> +    return NULL;
>> +}
>> diff --git a/src/libinput-seat.c b/src/libinput-seat.c
>> index c0a87ea..92c9cf9 100644
>> --- a/src/libinput-seat.c
>> +++ b/src/libinput-seat.c
>> @@ -56,6 +56,7 @@ device_added(struct udev_input *input, struct 
>> libinput_device *libinput_device)
>>      struct libinput_seat *libinput_seat;
>>      struct weston_seat *seat;
>>      struct udev_seat *udev_seat;
>> +    struct weston_pointer *pointer;
>>  
>>      c = input->compositor;
>>      libinput_seat = libinput_device_get_seat(libinput_device);
>> @@ -73,10 +74,11 @@ device_added(struct udev_input *input, struct 
>> libinput_device *libinput_device)
>>      udev_seat = (struct udev_seat *) seat;
>>      wl_list_insert(udev_seat->devices_list.prev, &device->link);
>>  
>> -    if (seat->output && seat->pointer)
>> -            weston_pointer_clamp(seat->pointer,
>> -                                 &seat->pointer->x,
>> -                                 &seat->pointer->y);
>> +    pointer = weston_seat_get_pointer(seat);
>> +    if (seat->output && pointer)
>> +            weston_pointer_clamp(pointer,
>> +                                 &pointer->x,
>> +                                 &pointer->y);
>>  
>>      output_name = libinput_device_get_output_name(libinput_device);
>>      if (output_name) {
>> @@ -368,8 +370,11 @@ udev_seat_create(struct udev_input *input, const char 
>> *seat_name)
>>  static void
>>  udev_seat_destroy(struct udev_seat *seat)
>>  {
>> +    struct weston_keyboard *keyboard =
>> +            weston_seat_get_keyboard(&seat->base);
>> +
>>      udev_seat_remove_devices(seat);
>> -    if (seat->base.keyboard)
>> +    if (keyboard)
>>              notify_keyboard_focus_out(&seat->base);
>>      weston_seat_release(&seat->base);
>>      wl_list_remove(&seat->output_create_listener.link);
>> diff --git a/src/text-backend.c b/src/text-backend.c
>> index de170e8..1f6121f 100644
>> --- a/src/text-backend.c
>> +++ b/src/text-backend.c
>> @@ -629,7 +629,7 @@ input_method_context_grab_keyboard(struct wl_client 
>> *client,
>>      struct input_method_context *context = 
>> wl_resource_get_user_data(resource);
>>      struct wl_resource *cr;
>>      struct weston_seat *seat = context->input_method->seat;
>> -    struct weston_keyboard *keyboard = seat->keyboard;
>> +    struct weston_keyboard *keyboard = weston_seat_get_keyboard(seat);
>>  
>>      cr = wl_resource_create(client, &wl_keyboard_interface, 1, id);
>>      wl_resource_set_implementation(cr, NULL, context, unbind_keyboard);
>> @@ -657,7 +657,7 @@ input_method_context_key(struct wl_client *client,
>>  {
>>      struct input_method_context *context = 
>> wl_resource_get_user_data(resource);
>>      struct weston_seat *seat = context->input_method->seat;
>> -    struct weston_keyboard *keyboard = seat->keyboard;
>> +    struct weston_keyboard *keyboard = weston_seat_get_keyboard(seat);
>>      struct weston_keyboard_grab *default_grab = &keyboard->default_grab;
>>  
>>      default_grab->interface->key(default_grab, time, key, state_w);
>> @@ -675,7 +675,7 @@ input_method_context_modifiers(struct wl_client *client,
>>      struct input_method_context *context = 
>> wl_resource_get_user_data(resource);
>>  
>>      struct weston_seat *seat = context->input_method->seat;
>> -    struct weston_keyboard *keyboard = seat->keyboard;
>> +    struct weston_keyboard *keyboard = weston_seat_get_keyboard(seat);
>>      struct weston_keyboard_grab *default_grab = &keyboard->default_grab;
>>  
>>      default_grab->interface->modifiers(default_grab,
>> @@ -779,10 +779,11 @@ input_method_context_end_keyboard_grab(struct 
>> input_method_context *context)
>>      struct weston_keyboard_grab *grab;
>>      struct weston_keyboard *keyboard;
>>  
>> -    if (!context->input_method->seat->keyboard)
>> +    keyboard = weston_seat_get_keyboard(context->input_method->seat);
>> +    if (!keyboard)
>>              return;
>>  
>> -    grab = &context->input_method->seat->keyboard->input_method_grab;
>> +    grab = &keyboard->input_method_grab;
>>      keyboard = grab->keyboard;
>>      if (!keyboard)
>>              return;
>> @@ -870,13 +871,15 @@ handle_keyboard_focus(struct wl_listener *listener, 
>> void *data)
>>  static void
>>  input_method_init_seat(struct weston_seat *seat)
>>  {
>> +    struct weston_keyboard *keyboard = weston_seat_get_keyboard(seat);
>> +
>>      if (seat->input_method->focus_listener_initialized)
>>              return;
>>  
>> -    if (seat->keyboard) {
>> +    if (keyboard) {
>>              seat->input_method->keyboard_focus_listener.notify = 
>> handle_keyboard_focus;
>> -            wl_signal_add(&seat->keyboard->focus_signal, 
>> &seat->input_method->keyboard_focus_listener);
>> -            seat->keyboard->input_method_grab.interface = 
>> &input_method_context_grab;
>> +            wl_signal_add(&keyboard->focus_signal, 
>> &seat->input_method->keyboard_focus_listener);
>> +            keyboard->input_method_grab.interface = 
>> &input_method_context_grab;
>>      }
>>  
>>      seat->input_method->focus_listener_initialized = true;
>> diff --git a/src/zoom.c b/src/zoom.c
>> index bee038b..38624e0 100644
>> --- a/src/zoom.c
>> +++ b/src/zoom.c
>> @@ -129,9 +129,10 @@ WL_EXPORT void
>>  weston_output_update_zoom(struct weston_output *output)
>>  {
>>      struct weston_seat *seat = weston_zoom_pick_seat(output->compositor);
>> +    struct weston_pointer *pointer = weston_seat_get_pointer(seat);
>>  
>> -    output->zoom.current.x = seat->pointer->x;
>> -    output->zoom.current.y = seat->pointer->y;
>> +    output->zoom.current.x = pointer->x;
>> +    output->zoom.current.y = pointer->y;
>>  
>>      weston_zoom_transition(output);
>>      weston_output_update_zoom_transform(output);
>> @@ -152,13 +153,14 @@ WL_EXPORT void
>>  weston_output_activate_zoom(struct weston_output *output)
>>  {
>>      struct weston_seat *seat = weston_zoom_pick_seat(output->compositor);
>> +    struct weston_pointer *pointer = weston_seat_get_pointer(seat);
>>  
>>      if (output->zoom.active)
>>              return;
>>  
>>      output->zoom.active = 1;
>>      output->disable_planes++;
>> -    wl_signal_add(&seat->pointer->motion_signal,
>> +    wl_signal_add(&pointer->motion_signal,
>>                    &output->zoom.motion_listener);
>>  }
>>  
>> diff --git a/tests/surface-screenshot.c b/tests/surface-screenshot.c
>> index 8badcf2..54fca41 100644
>> --- a/tests/surface-screenshot.c
>> +++ b/tests/surface-screenshot.c
>> @@ -136,6 +136,7 @@ trigger_binding(struct weston_keyboard *keyboard, 
>> uint32_t time, uint32_t key,
>>      char fname[1024];
>>      struct weston_surface *surface;
>>      struct weston_seat *seat = keyboard->seat;
>> +    struct weston_pointer *pointer = weston_seat_get_pointer(seat);
>>      int width, height;
>>      char desc[512];
>>      void *pixels;
>> @@ -144,12 +145,10 @@ trigger_binding(struct weston_keyboard *keyboard, 
>> uint32_t time, uint32_t key,
>>      int ret;
>>      FILE *fp;
>>  
>> -    if (seat->pointer_device_count == 0 ||
>> -        !seat->pointer ||
>> -        !seat->pointer->focus)
>> +    if (!pointer || !pointer->focus)
>>              return;
>>  
>> -    surface = seat->pointer->focus->surface;
>> +    surface = pointer->focus->surface;
>>  
>>      weston_surface_get_content_size(surface, &width, &height);
>>  
>> diff --git a/tests/weston-test.c b/tests/weston-test.c
>> index 8f8781f..22c42cf 100644
>> --- a/tests/weston-test.c
>> +++ b/tests/weston-test.c
>> @@ -78,7 +78,7 @@ static void
>>  notify_pointer_position(struct weston_test *test, struct wl_resource 
>> *resource)
>>  {
>>      struct weston_seat *seat = get_seat(test);
>> -    struct weston_pointer *pointer = seat->pointer;
>> +    struct weston_pointer *pointer = weston_seat_get_pointer(seat);
>>  
>>      weston_test_send_pointer_position(resource, pointer->x, pointer->y);
>>  }
>> @@ -139,7 +139,7 @@ move_pointer(struct wl_client *client, struct 
>> wl_resource *resource,
>>  {
>>      struct weston_test *test = wl_resource_get_user_data(resource);
>>      struct weston_seat *seat = get_seat(test);
>> -    struct weston_pointer *pointer = seat->pointer;
>> +    struct weston_pointer *pointer = weston_seat_get_pointer(seat);
>>  
>>      notify_motion(seat, 100,
>>                    wl_fixed_from_int(x) - pointer->x,
>> @@ -166,12 +166,13 @@ activate_surface(struct wl_client *client, struct 
>> wl_resource *resource,
>>              wl_resource_get_user_data(surface_resource) : NULL;
>>      struct weston_test *test = wl_resource_get_user_data(resource);
>>      struct weston_seat *seat;
>> +    struct weston_keyboard *keyboard;
>>  
>>      seat = get_seat(test);
>> -
>> +    keyboard = weston_seat_get_keyboard(seat);
>>      if (surface) {
>>              weston_surface_activate(surface, seat);
>> -            notify_keyboard_focus_in(seat, &seat->keyboard->keys,
>> +            notify_keyboard_focus_in(seat, &keyboard->keys,
>>                                       STATE_UPDATE_AUTOMATIC);
>>      }
>>      else {
>> diff --git a/xwayland/dnd.c b/xwayland/dnd.c
>> index f0f2457..944c220 100644
>> --- a/xwayland/dnd.c
>> +++ b/xwayland/dnd.c
>> @@ -151,6 +151,7 @@ handle_enter(struct weston_wm *wm, 
>> xcb_client_message_event_t *client_message)
>>  {
>>      struct dnd_data_source *source;
>>      struct weston_seat *seat = weston_wm_pick_seat(wm);
>> +    struct weston_pointer *pointer = weston_seat_get_pointer(seat);
>>      char **p;
>>      const char *name;
>>      uint32_t *types;
>> @@ -210,7 +211,7 @@ handle_enter(struct weston_wm *wm, 
>> xcb_client_message_event_t *client_message)
>>      }
>>  
>>      free(reply);
>> -    weston_pointer_start_drag(seat->pointer, &source->base, NULL, NULL);
>> +    weston_pointer_start_drag(pointer, &source->base, NULL, NULL);
>>  }
>>  
>>  int
>> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
>> index 1ea2c4c..ea40c1e 100644
>> --- a/xwayland/window-manager.c
>> +++ b/xwayland/window-manager.c
>> @@ -1312,12 +1312,16 @@ weston_wm_pick_seat_for_window(struct 
>> weston_wm_window *window)
>>  
>>      seat = NULL;
>>      wl_list_for_each(s, &wm->server->compositor->seat_list, link) {
>> -            if (s->pointer != NULL && s->pointer->focus &&
>> -                s->pointer->focus->surface == window->surface &&
>> -                s->pointer->button_count > 0 &&
>> -                (seat == NULL ||
>> -                 s->pointer->grab_serial -
>> -                 seat->pointer->grab_serial < (1 << 30)))
>> +            struct weston_pointer *pointer = weston_seat_get_pointer(s);
>> +            struct weston_pointer *old_pointer =
>> +                    weston_seat_get_pointer(seat);
>> +
>> +            if (pointer && pointer->focus &&
>> +                pointer->focus->surface == window->surface &&
>> +                pointer->button_count > 0 &&
>> +                (!seat ||
>> +                 pointer->grab_serial -
>> +                 old_pointer->grab_serial < (1 << 30)))
>>                      seat = s;
>>      }
>>  
>> @@ -1341,13 +1345,14 @@ weston_wm_window_handle_moveresize(struct 
>> weston_wm_window *window,
>>  
>>      struct weston_wm *wm = window->wm;
>>      struct weston_seat *seat = weston_wm_pick_seat_for_window(window);
>> +    struct weston_pointer *pointer = weston_seat_get_pointer(seat);
>>      int detail;
>>      struct weston_shell_interface *shell_interface =
>>              &wm->server->compositor->shell_interface;
>>  
>> -    if (seat == NULL || seat->pointer->button_count != 1
>> -        || !seat->pointer->focus
>> -        || seat->pointer->focus->surface != window->surface)
>> +    if (!pointer || pointer->button_count != 1
>> +        || !pointer->focus
>> +        || pointer->focus->surface != window->surface)
>>              return;
>>  
>>      detail = client_message->data.data32[2];
>> -- 
>> 2.1.4
>>
>> _______________________________________________
>> wayland-devel mailing list
>> [email protected]
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> _______________________________________________
> wayland-devel mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> 

_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to