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