[PATCH weston] input: add a weston_pointer_clear_focus() helper function

Derek Foreman derekf at osg.samsung.com
Mon May 11 13:38:28 PDT 2015


On 10/05/15 04:02 AM, Giulio Camuffo wrote:
> 2015-04-17 0:02 GMT+03:00 Derek Foreman <derekf at osg.samsung.com>:
>> This adds a function to clear pointer focus and also set the sx,sy
>> co-ordinates to an arbitrary value we shouldn't compute with.
>>
>> Assertions are added to make sure any time pointer focus is set to NULL
>> these values are used.
>>
>> weston_compositor_pick_view() now returns these values too.
> 
> Hi,
> code-wise this look ok to me, however i'm not sure i see what this
> fixes which justifies the added complexity. Maybe you could write it
> in the commit message?

Hmm, it's more complex?  Previously to clear focus you were always
supposed to call
weston_pointer_set_focus(seat->pointer, NULL, wl_fixed_from_int(0),
wl_fixed_from_int(0));

However it's been called with regular integer 0 (same as a fixed 0, so I
guess no harm in it...) and even the pointer co-ordinates (not so good).

wl_pointer_clear_focus(seat->pointer) seems simpler and less error prone.

The sentinels are to give us something safe for comparisons but unsafe
for any kind of actual math.  I'll add more detail to the commit log for
that.

> 
> Thanks,
> Giulio
> 
>>
>> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
>> ---
>> Ok, so I didn't do quite what I set out to, but I think this is better :)
>>
>> Pekka, does this roughly cover what we talked about?  Now everyone sets sx,
>> sx "properly".
>>
>> Not sure how I feel about the previous patch.  I think it's superfluous now,
>> as the pointer co-ordinates test should fail and stop the focus signal from
>> firing.
>>
>>
>>  desktop-shell/exposay.c |  3 +--
>>  desktop-shell/shell.c   |  4 +---
>>  src/compositor.c        |  7 +++----
>>  src/compositor.h        |  2 ++
>>  src/data-device.c       |  3 +--
>>  src/input.c             | 34 +++++++++++++++++++++++++++++-----
>>  6 files changed, 37 insertions(+), 16 deletions(-)
>>
>> diff --git a/desktop-shell/exposay.c b/desktop-shell/exposay.c
>> index 4b65cbd..5d77893 100644
>> --- a/desktop-shell/exposay.c
>> +++ b/desktop-shell/exposay.c
>> @@ -572,8 +572,7 @@ exposay_transition_active(struct desktop_shell *shell)
>>         shell->exposay.grab_ptr.interface = &exposay_ptr_grab;
>>         weston_pointer_start_grab(seat->pointer,
>>                                   &shell->exposay.grab_ptr);
>> -       weston_pointer_set_focus(seat->pointer, NULL,
>> -                                seat->pointer->x, seat->pointer->y);
>> +       weston_pointer_clear_focus(seat->pointer);
>>
>>         wl_list_for_each(shell_output, &shell->output_list, link) {
>>                 enum exposay_layout_state state;
>> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
>> index f7c928e..4e9f9c2 100644
>> --- a/desktop-shell/shell.c
>> +++ b/desktop-shell/shell.c
>> @@ -3160,9 +3160,7 @@ popup_grab_focus(struct weston_pointer_grab *grab)
>>             wl_resource_get_client(view->surface->resource) == client) {
>>                 weston_pointer_set_focus(pointer, view, sx, sy);
>>         } else {
>> -               weston_pointer_set_focus(pointer, NULL,
>> -                                        wl_fixed_from_int(0),
>> -                                        wl_fixed_from_int(0));
>> +               weston_pointer_clear_focus(pointer);
>>         }
>>  }
>>
>> diff --git a/src/compositor.c b/src/compositor.c
>> index e6a60bd..8bd11ce 100644
>> --- a/src/compositor.c
>> +++ b/src/compositor.c
>> @@ -1770,6 +1770,8 @@ weston_compositor_pick_view(struct weston_compositor *compositor,
>>                 return view;
>>         }
>>
>> +       *vx = wl_fixed_from_int(-1000000);
>> +       *vy = wl_fixed_from_int(-1000000);
>>         return NULL;
>>  }
>>
>> @@ -1809,10 +1811,7 @@ weston_view_unmap(struct weston_view *view)
>>                 if (seat->keyboard && seat->keyboard->focus == view->surface)
>>                         weston_keyboard_set_focus(seat->keyboard, NULL);
>>                 if (seat->pointer && seat->pointer->focus == view)
>> -                       weston_pointer_set_focus(seat->pointer,
>> -                                                NULL,
>> -                                                wl_fixed_from_int(0),
>> -                                                wl_fixed_from_int(0));
>> +                       weston_pointer_clear_focus(seat->pointer);
>>                 if (seat->touch && seat->touch->focus == view)
>>                         weston_touch_set_focus(seat, NULL);
>>         }
>> diff --git a/src/compositor.h b/src/compositor.h
>> index 5f49237..8b694aa 100644
>> --- a/src/compositor.h
>> +++ b/src/compositor.h
>> @@ -379,6 +379,8 @@ weston_pointer_set_focus(struct weston_pointer *pointer,
>>                          struct weston_view *view,
>>                          wl_fixed_t sx, wl_fixed_t sy);
>>  void
>> +weston_pointer_clear_focus(struct weston_pointer *pointer);
>> +void
>>  weston_pointer_start_grab(struct weston_pointer *pointer,
>>                           struct weston_pointer_grab *grab);
>>  void
>> diff --git a/src/data-device.c b/src/data-device.c
>> index a0913a2..f879daa 100644
>> --- a/src/data-device.c
>> +++ b/src/data-device.c
>> @@ -587,8 +587,7 @@ weston_pointer_start_drag(struct weston_pointer *pointer,
>>                               &drag->base.data_source_listener);
>>         }
>>
>> -       weston_pointer_set_focus(pointer, NULL,
>> -                                wl_fixed_from_int(0), wl_fixed_from_int(0));
>> +       weston_pointer_clear_focus(pointer);
>>         weston_pointer_start_grab(pointer, &drag->grab);
>>
>>         return 0;
>> diff --git a/src/input.c b/src/input.c
>> index 18d1262..97a0284 100644
>> --- a/src/input.c
>> +++ b/src/input.c
>> @@ -78,7 +78,7 @@ pointer_focus_view_destroyed(struct wl_listener *listener, void *data)
>>                 container_of(listener, struct weston_pointer,
>>                              focus_view_listener);
>>
>> -       weston_pointer_set_focus(pointer, NULL, 0, 0);
>> +       weston_pointer_clear_focus(pointer);
>>  }
>>
>>  static void
>> @@ -88,7 +88,7 @@ pointer_focus_resource_destroyed(struct wl_listener *listener, void *data)
>>                 container_of(listener, struct weston_pointer,
>>                              focus_resource_listener);
>>
>> -       weston_pointer_set_focus(pointer, NULL, 0, 0);
>> +       weston_pointer_clear_focus(pointer);
>>  }
>>
>>  static void
>> @@ -490,6 +490,9 @@ weston_pointer_create(struct weston_seat *seat)
>>         wl_signal_add(&seat->compositor->output_destroyed_signal,
>>                       &pointer->output_destroy_listener);
>>
>> +       pointer->sx = wl_fixed_from_int(-1000000);
>> +       pointer->sy = wl_fixed_from_int(-1000000);
>> +
>>         return pointer;
>>  }
>>
>> @@ -620,6 +623,26 @@ seat_send_updated_caps(struct weston_seat *seat)
>>         wl_signal_emit(&seat->updated_caps_signal, seat);
>>  }
>>
>> +
>> +/** Clear the pointer focus
>> + *
>> + * \param pointer the pointer to clear focus for.
>> + *
>> + * This can be used to unset pointer focus and set the co-ordinates to the
>> + * arbitrary values we use for the no focus case.
>> + *
>> + * There's no requirement to use this function.  For example, passing the
>> + * results of a weston_compositor_pick_view() directly to
>> + * weston_pointer_set_focus() will do the right thing when no view is found.
>> + */
>> +WL_EXPORT void
>> +weston_pointer_clear_focus(struct weston_pointer *pointer)
>> +{
>> +       weston_pointer_set_focus(pointer, NULL,
>> +                                wl_fixed_from_int(-1000000),
>> +                                wl_fixed_from_int(-1000000));
>> +}
>> +
>>  WL_EXPORT void
>>  weston_pointer_set_focus(struct weston_pointer *pointer,
>>                          struct weston_view *view,
>> @@ -691,6 +714,9 @@ weston_pointer_set_focus(struct weston_pointer *pointer,
>>         pointer->sx = sx;
>>         pointer->sy = sy;
>>
>> +       assert(view || sx == wl_fixed_from_int(-1000000));
>> +       assert(view || sy == wl_fixed_from_int(-1000000));
>> +
>>         wl_signal_emit(&pointer->focus_signal, pointer);
>>  }
>>
>> @@ -2232,9 +2258,7 @@ weston_seat_release_pointer(struct weston_seat *seat)
>>
>>         seat->pointer_device_count--;
>>         if (seat->pointer_device_count == 0) {
>> -               weston_pointer_set_focus(pointer, NULL,
>> -                                        wl_fixed_from_int(0),
>> -                                        wl_fixed_from_int(0));
>> +               weston_pointer_clear_focus(pointer);
>>                 weston_pointer_cancel_grab(pointer);
>>
>>                 if (pointer->sprite)
>> --
>> 2.1.4
>>
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel



More information about the wayland-devel mailing list