[PATCH weston v2 01/10] compositor: Track idle as a per-seat property
Bryce Harrington
bryce at osg.samsung.com
Thu Apr 7 23:03:29 UTC 2016
On Thu, Apr 07, 2016 at 08:44:44AM +0300, Giulio Camuffo wrote:
> 2016-04-07 1:46 GMT+03:00 Bryce Harrington <bryce at osg.samsung.com>:
> > On Mon, Mar 28, 2016 at 10:17:20AM +0300, Giulio Camuffo wrote:
> >> 2016-03-24 20:27 GMT+02:00 Bryce Harrington <bryce at osg.samsung.com>:
> >> > 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.
> >> >
> >> > Signed-off-by: Bryce Harrington <bryce at 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);
> >> > -}
> >> > -
> >> > -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++;
> >>
> >> Seeing you have these two lines (and the decrement ones too) many
> >> times in the code i think it would make sense to have
> >> weston_seat_idle_inhibit/release(), as the
> >> weston_compositor_idle_inhibit/release() you are removing.
> >
> > The problem here is that before we were doing two different operations
> > on the compositor, now we're doing them on two different objects, so the
> > routines would be kind of a hodge-podge:
> >
> > static void
> > weston_seat_inhibit_idle(struct weston_seat *seat,
> > struct weston_compositor *compositor)
> > {
> > weston_compositor_wake(compositor);
>
> But weston_seat has a compositor member, no need to pass it here.
Ah, true enough. That would mostly address the issue that was bugging me.
> > seat->idle_inhibit++;
> > }
> >
> > static void
> > weston_seat_release_idle(struct weston_seat *seat,
> > struct weston_compositor *compositor)
> > {
> > seat->idle_inhibit--;
> > weston_compositor_wake(compositor);
> > }
> >
> > I feel it's clearer to just eliminate the functions and inline their
> > code. The delta is only 3 lines for the first routine and 4 for the
> > second, which doesn't seem worth the overhead of having them split out.
>
> It's sure no biggie, but i think it would be clearer with the functions.
> Either way, Reviewed-By: Giulio Camuffo <giuliocamuffo at gmail.com>
Thanks for reviewing.
Yeah I could go either way with this, especially with the fix you
suggest. I've got the v3 patchset ready to go now, with the code as it
is. Why don't I post it without the helpers, and see if anyone else has
an opinion on it?
Bryce
> Cheers
>
> >
> > Bryce
> >
> >> Cheers,
> >> Giulio
> >>
> >> > 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);
> >> > --
> >> > 1.9.1
> >> >
> >> > _______________________________________________
> >> > wayland-devel mailing list
> >> > wayland-devel at lists.freedesktop.org
> >> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list