[PATCH weston v2 01/10] compositor: Track idle as a per-seat property

Bryce Harrington bryce at osg.samsung.com
Wed Apr 6 22:46:15 UTC 2016


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);
        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.

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