[PATCH weston 2/2] input: Move weston_seat_set_keyboard_focus and document

[Pekka Paalanen]

On Wed, 29 Jun 2016 19:04:07 -0700
Bryce Harrington <bryce at osg.samsung.com> wrote:

> Place it with the other weston_seat functions.
> 
> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> ---
>  libweston/compositor.h |  7 ++++---
>  libweston/input.c      | 34 +++++++++++++++++++---------------
>  2 files changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/libweston/compositor.h b/libweston/compositor.h
> index 5701a05..49ef063 100644
> --- a/libweston/compositor.h
> +++ b/libweston/compositor.h
> @@ -1167,9 +1167,6 @@ int
>  weston_spring_done(struct weston_spring *spring);
>  
>  void
> -weston_seat_set_keyboard_focus(struct weston_seat *seat,
> -			       struct weston_surface *surface);
> -void
>  notify_motion(struct weston_seat *seat, uint32_t time,
>  	      struct weston_pointer_motion_event *event);
>  void
> @@ -1717,6 +1714,10 @@ weston_seat_get_pointer(struct weston_seat *seat);
>  struct weston_touch *
>  weston_seat_get_touch(struct weston_seat *seat);
>  
> +void
> +weston_seat_set_keyboard_focus(struct weston_seat *seat,
> +			       struct weston_surface *surface);
> +
>  #ifdef  __cplusplus
>  }
>  #endif
> diff --git a/libweston/input.c b/libweston/input.c
> index e8c060e..8f46698 100644
> --- a/libweston/input.c
> +++ b/libweston/input.c
> @@ -1297,21 +1297,6 @@ notify_motion_absolute(struct weston_seat *seat,
>  }
>  
>  WL_EXPORT void
> -weston_seat_set_keyboard_focus(struct weston_seat *seat,
> -			       struct weston_surface *surface)
> -{
> -	struct weston_compositor *compositor = seat->compositor;
> -	struct weston_keyboard *keyboard = weston_seat_get_keyboard(seat);
> -
> -	if (keyboard) {
> -		weston_keyboard_set_focus(keyboard, surface);
> -		wl_data_device_set_keyboard_focus(seat);
> -	}
> -
> -	wl_signal_emit(&compositor->activate_signal, surface);
> -}
> -
> -WL_EXPORT void
>  notify_button(struct weston_seat *seat, uint32_t time, int32_t button,
>  	      enum wl_pointer_button_state state)
>  {
> @@ -2763,3 +2748,22 @@ weston_seat_get_touch(struct weston_seat *seat)
>  
>  	return NULL;
>  }
> +
> +/** Sets the keyboard focus to the given surface
> + *
> + * \param seat The seat to query
> + */
> +WL_EXPORT void
> +weston_seat_set_keyboard_focus(struct weston_seat *seat,
> +			       struct weston_surface *surface)
> +{
> +	struct weston_compositor *compositor = seat->compositor;
> +	struct weston_keyboard *keyboard = weston_seat_get_keyboard(seat);
> +
> +	if (keyboard) {
> +		weston_keyboard_set_focus(keyboard, surface);
> +		wl_data_device_set_keyboard_focus(seat);
> +	}
> +
> +	wl_signal_emit(&compositor->activate_signal, surface);
> +}

Hi Bryce,

both pushed:
   a5bb91d..24f917e  master -> master


The activate_signal will be a lot more difficult to handle. The only
in-tree user of it is xwayland's weston_wm_window_activate() which
assumes there can only be one surface active at a time, and it's not
even per seat, it's really just one. There might also be other users
out there I do not know of.

If we go with your plan to be able to have arbitrary many surfaces
active at the same time, without changes XWM would raise and activate
the most recent surface set active, regardless of what is already
active. Is that desireable, I do not know.

To be able to support any other kind of behaviour, we'd need signals for
both activate and deactivate. Or, we have to decide that you can only
have one surface active per seat, in which case we'd have a per-seat
activate_signal, and the active surface would be tracked just like
keyboard focus except in struct weston_seat, not in struct
weston_keyboard.

Looks like all of weston_keyboard, weston_pointer, and weston_touch
have their own focus_signals, too, which are naturally conditional on
having the capability in the seat.

These are the design questions we need to solve to add activeness
tracking in Weston core. If we put "active" as a thing in the core,
how should it behave?

Therefore I'd suggest to reconsider "activeness" for idle-inhibit. If
you just want a shell-controllable surface flag for "may inhibit idle
triggers", then we should not talk about active because active is
something else. Then I'd also ask why is it a surface and not a view.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160630/bb91cbd6/attachment.sig>