On Thu, 7 Apr 2016 16:44:16 -0700 Bryce Harrington <br...@osg.samsung.com> wrote:
> Instead of having a single global idle tracker, track idling separately
> for each seat. Still treat inhibition on any one seat as inhibiting
> the screensaver globally, for now.
Hi,
the screensaver?
The commit message should explain why track idle per seat.
Already when I read the cover letter, I was left wondering why do you
need to track idleness per seat? What actually is per-seat here?
Why would it matter which seat is getting user activity? It is all
still user activity and should prevent all idle-triggerings, right?
Then again, this patch is not implementing per-seat idle tracking, this
is implementing per-seat implicit idle-inhibit, which is triggered by a
key/botton/touch being down. Idle tracking sounds more like maintaining
a (per-seat) idle timer or a timestamp.
When I look at the weston code as a whole after this complete patch
series, I cannot find any use for per-seat idle_inhibit. It stays
global, just more complicated to compute.
Therefore I think this patch should be dropped.
> Signed-off-by: Bryce Harrington <br...@osg.samsung.com>
> ---
> src/compositor.c | 6 ++++--
> src/compositor.h | 3 ++-
> src/input.c | 39 ++++++++++++++++-----------------------
> 3 files changed, 22 insertions(+), 26 deletions(-)
>
> diff --git a/src/compositor.c b/src/compositor.c
> index 254e9e4..83cabf7 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -3882,9 +3882,11 @@ static int
> idle_handler(void *data)
> {
> struct weston_compositor *compositor = data;
> + struct weston_seat *seat;
>
> - if (compositor->idle_inhibit)
> - return 1;
> + wl_list_for_each(seat, &compositor->seat_list, link)
> + if (seat->idle_inhibit)
> + return 1;
>
> compositor->state = WESTON_COMPOSITOR_IDLE;
> wl_signal_emit(&compositor->idle_signal, compositor);
> diff --git a/src/compositor.h b/src/compositor.h
> index 8a5aa91..df8ef2d 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -563,6 +563,8 @@ struct weston_seat {
>
> struct input_method *input_method;
> char *seat_name;
> +
> + uint32_t idle_inhibit;
> };
>
> enum {
> @@ -724,7 +726,6 @@ struct weston_compositor {
>
> uint32_t state;
> struct wl_event_source *idle_source;
> - uint32_t idle_inhibit;
> int idle_time; /* timeout, s */
>
> const struct weston_pointer_grab_interface *default_pointer_grab;
> diff --git a/src/input.c b/src/input.c
> index f5bb54e..a3d982e 100644
> --- a/src/input.c
> +++ b/src/input.c
> @@ -151,20 +151,6 @@ weston_seat_repick(struct weston_seat *seat)
> }
>
> static void
> -weston_compositor_idle_inhibit(struct weston_compositor *compositor)
> -{
> - weston_compositor_wake(compositor);
> - compositor->idle_inhibit++;
> -}
> -
> -static void
> -weston_compositor_idle_release(struct weston_compositor *compositor)
> -{
> - compositor->idle_inhibit--;
> - weston_compositor_wake(compositor);
> -}
These functions should not be removed and open-coded, but changed to
operate on a weston_seat.
> -
> -static void
> pointer_focus_view_destroyed(struct wl_listener *listener, void *data)
> {
> struct weston_pointer *pointer =
> @@ -1242,7 +1228,8 @@ notify_button(struct weston_seat *seat, uint32_t time,
> int32_t button,
> struct weston_pointer *pointer = weston_seat_get_pointer(seat);
>
> if (state == WL_POINTER_BUTTON_STATE_PRESSED) {
> - weston_compositor_idle_inhibit(compositor);
> + weston_compositor_wake(compositor);
> + seat->idle_inhibit++;
> if (pointer->button_count == 0) {
> pointer->grab_button = button;
> pointer->grab_time = time;
> @@ -1251,7 +1238,8 @@ notify_button(struct weston_seat *seat, uint32_t time,
> int32_t button,
> }
> pointer->button_count++;
> } else {
> - weston_compositor_idle_release(compositor);
> + seat->idle_inhibit--;
> + weston_compositor_wake(compositor);
> pointer->button_count--;
> }
>
> @@ -1540,9 +1528,11 @@ notify_key(struct weston_seat *seat, uint32_t time,
> uint32_t key,
> uint32_t *k, *end;
>
> if (state == WL_KEYBOARD_KEY_STATE_PRESSED) {
> - weston_compositor_idle_inhibit(compositor);
> + weston_compositor_wake(compositor);
> + seat->idle_inhibit++;
> } else {
> - weston_compositor_idle_release(compositor);
> + seat->idle_inhibit--;
> + weston_compositor_wake(compositor);
> }
>
> end = keyboard->keys.data + keyboard->keys.size;
> @@ -1625,7 +1615,8 @@ notify_keyboard_focus_in(struct weston_seat *seat,
> struct wl_array *keys,
> serial = wl_display_next_serial(compositor->wl_display);
> wl_array_copy(&keyboard->keys, keys);
> wl_array_for_each(k, &keyboard->keys) {
> - weston_compositor_idle_inhibit(compositor);
> + weston_compositor_wake(compositor);
> + seat->idle_inhibit++;
> if (update_state == STATE_UPDATE_AUTOMATIC)
> update_modifier_state(seat, serial, *k,
> WL_KEYBOARD_KEY_STATE_PRESSED);
> @@ -1650,7 +1641,8 @@ notify_keyboard_focus_out(struct weston_seat *seat)
>
> serial = wl_display_next_serial(compositor->wl_display);
> wl_array_for_each(k, &keyboard->keys) {
> - weston_compositor_idle_release(compositor);
> + seat->idle_inhibit--;
> + weston_compositor_wake(compositor);
> update_modifier_state(seat, serial, *k,
> WL_KEYBOARD_KEY_STATE_RELEASED);
> }
> @@ -1739,8 +1731,8 @@ notify_touch(struct weston_seat *seat, uint32_t time,
> int touch_id,
>
> switch (touch_type) {
> case WL_TOUCH_DOWN:
> - weston_compositor_idle_inhibit(ec);
> -
> + weston_compositor_wake(ec);
> + seat->idle_inhibit++;
> touch->num_tp++;
>
> /* the first finger down picks the view, and all further go
> @@ -1788,7 +1780,8 @@ notify_touch(struct weston_seat *seat, uint32_t time,
> int touch_id,
> weston_log("unmatched touch up event\n");
> break;
> }
> - weston_compositor_idle_release(ec);
> + seat->idle_inhibit--;
> + weston_compositor_wake(ec);
> touch->num_tp--;
>
> grab->interface->up(grab, time, touch_id);
Thanks,
pq
pgpmJcEUuKImF.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
