On Sat, Feb 22, 2014 at 04:44:06PM +0100, Jonas Ådahl wrote:
> On Tue, Feb 18, 2014 at 04:09:10PM +1000, Peter Hutterer wrote:
> > This gives us the ability to handle SYN_DROPPED transparently to the caller.
> > 
> > Signed-off-by: Peter Hutterer <[email protected]>
> > ---
> >  src/evdev.c | 90 
> > +++++++++++++++++++++++++++++++++++++++----------------------
> >  1 file changed, 58 insertions(+), 32 deletions(-)
> > 
> > diff --git a/src/evdev.c b/src/evdev.c
> > index ba28fc6..836d0af 100644
> > --- a/src/evdev.c
> > +++ b/src/evdev.c
> > @@ -29,7 +29,7 @@
> >  #include <linux/input.h>
> >  #include <unistd.h>
> >  #include <fcntl.h>
> > -#include <mtdev.h>
> > +#include <mtdev-plumbing.h>
> >  #include <assert.h>
> >  
> >  #include "libinput.h"
> > @@ -436,56 +436,82 @@ fallback_dispatch_create(void)
> >     return dispatch;
> >  }
> >  
> > -static void
> > -evdev_process_events(struct evdev_device *device,
> > -                struct input_event *ev, int count)
> > +static inline void
> > +evdev_process_event(struct evdev_device *device, struct input_event *e)
> >  {
> >     struct evdev_dispatch *dispatch = device->dispatch;
> > -   struct input_event *e, *end;
> > -   uint32_t time = 0;
> > +   uint32_t time = e->time.tv_sec * 1000 + e->time.tv_usec / 1000;
> >  
> > -   e = ev;
> > -   end = e + count;
> > -   for (e = ev; e < end; e++) {
> > -           time = e->time.tv_sec * 1000 + e->time.tv_usec / 1000;
> > +   dispatch->interface->process(dispatch, device, e, time);
> > +}
> >  
> > -           dispatch->interface->process(dispatch, device, e, time);
> > +static inline void
> > +evdev_device_dispatch_one(struct evdev_device *device,
> > +                     struct input_event *ev)
> > +{
> > +   if (!device->mtdev) {
> > +           evdev_process_event(device, ev);
> > +   } else {
> > +           mtdev_put_event(device->mtdev, ev);
> > +           if (libevdev_event_is_code(ev, EV_SYN, SYN_REPORT)) {
> > +                   while(!mtdev_empty(device->mtdev)) {
> > +                           struct input_event e;
> > +                           mtdev_get_event(device->mtdev, &e);
> > +                           evdev_process_event(device, &e);
> > +                   }
> > +           }
> >     }
> >  }
> >  
> > +static int
> > +evdev_sync_device(struct evdev_device *device)
> > +{
> > +   struct input_event ev;
> > +   int rc;
> > +
> > +   do {
> > +           rc = libevdev_next_event(device->evdev,
> > +                                    LIBEVDEV_READ_FLAG_SYNC, &ev);
> > +           if (rc < 0)
> > +                   break;
> > +           evdev_device_dispatch_one(device, &ev);
> > +   } while (rc == LIBEVDEV_READ_STATUS_SYNC);
> > +
> > +   return rc == -EAGAIN ? 0 : rc;
> > +}
> > +
> >  static void
> >  evdev_device_dispatch(void *data)
> >  {
> >     struct evdev_device *device = data;
> >     struct libinput *libinput = device->base.seat->libinput;
> > -   int fd = device->fd;
> > -   struct input_event ev[32];
> > -   int len;
> > +   struct input_event ev;
> > +   int rc;
> >  
> >     /* If the compositor is repainting, this function is called only once
> >      * per frame and we have to process all the events available on the
> >      * fd, otherwise there will be input lag. */
> >     do {
> > -           if (device->mtdev)
> > -                   len = mtdev_get(device->mtdev, fd, ev,
> > -                                   ARRAY_LENGTH(ev)) *
> > -                           sizeof (struct input_event);
> > -           else
> > -                   len = read(fd, &ev, sizeof ev);
> > +           rc = libevdev_next_event(device->evdev,
> > +                                    LIBEVDEV_READ_FLAG_NORMAL, &ev);
> > +           if (rc == LIBEVDEV_READ_STATUS_SYNC) {
> > +                   /* send one more sync event so we handle all
> > +                      currently pending events before we sync up
> > +                      to the current state */
> > +                   ev.code = SYN_REPORT;
> > +                   evdev_device_dispatch_one(device, &ev);
> 
> Is this really correct? Shouldn't calling libevdev_next_event() in SYNC
> mode return a SYN_REPORT event as part of the synchronization? If we do
> like this it looks like we might "cut" one series of events that would
> otherwise be grouped using SYN_REPORT in half.

I think you may have misread the diff, this code is only ever called once,
on the SYN_DROPPED, all the other events are handled in evdev_sync_device().

libevdev guarantees that the event you pass in when you get
LIBEVDEV_READ_STATUS_SYNC is the SYN_DROPPED event that triggered the sync.
All we do here is change from EV_SYN/SYN_DROPPED to EV_SYN/SYN_REPORT and
process that normally. That finishes the current event sequence.

We then call evdev_sync_device() which empties and processes the sync queue
until -EAGAIN. That queue is also guaranteed to end with a
EV_SYN/SYN_REPORT. Once that is done, we jump back here and continue with
the loop, which now hopefully has SUCCESS on the next read.

> >  
> > -           if (len < 0 || len % sizeof ev[0] != 0) {
> > -                   if (len < 0 && errno != EAGAIN && errno != EINTR) {
> > -                           libinput_remove_source(libinput,
> > -                                                  device->source);
> > -                           device->source = NULL;
> > -                   }
> > +                   rc = evdev_sync_device(device);

                                ^^^ this is where the actual sync happens

> > +                   if (rc == 0)
> > +                           rc = LIBEVDEV_READ_STATUS_SUCCESS;


> > +           } else if (rc == LIBEVDEV_READ_STATUS_SUCCESS)
> > +                   evdev_device_dispatch_one(device, &ev);
> 
> nit: if any branch of an if have braces, all should.

amended, thanks.

Cheers,
   Peter

> > +   } while (rc == LIBEVDEV_READ_STATUS_SUCCESS);
> >  
> > -                   return;
> > -           }
> > -
> > -           evdev_process_events(device, ev, len / sizeof ev[0]);
> > -
> > -   } while (len > 0);
> > +   if (rc != -EAGAIN && rc != -EINTR) {
> > +           libinput_remove_source(libinput, device->source);
> > +           device->source = NULL;
> > +   }
> >  }
> >  
> >  static int
> > -- 
> > 1.8.4.2
> 
> 
> Jonas
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to