[PATCH weston v2 01/10] compositor: Track idle as a per-seat property
Giulio Camuffo
giuliocamuffo at gmail.com
Thu Apr 7 05:44:44 UTC 2016
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.
> 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>
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