libevdev sync without tracking ID

[Peter Hutterer]

On Tue, Mar 18, 2014 at 10:22:05AM -0400, Benjamin Tissoires wrote:
> On Mon, Mar 17, 2014 at 11:07 PM, Peter Hutterer
> <peter.hutterer at who-t.net> wrote:
> > I just spotted something interesting, related to the recent changes with
> > SYN_DROPPED handling.
> >
> > We now handle ABS_MT_TRACKING_ID for devices with a slot, terminating and
> > re-creating touchpoints if the tracking ID changes during SYN_DROPPED. One
> > thing I did notice is that we update the axes even if the tracking ID is -1.
> > This can easily be triggered with the libevdev-events tool we ship:
> >
> > 1. start libevdev-events on a touchpad
> > 2. one finger on the touchpad, move, lift finger off
> > 3. pause libevdev-events
> > 4. one finger on the touchpad, move, lift finger off again
> > 5. resume libevdev-events
> >
> > libevev-events gets a SYN_DROPPED, the tracking ID is -1 before and -1
> > after, we simply missed the whole touchpoint. The other axes may have
> > changed though, so in the sync process we send events for each MT axis that
> > did so, in each slot.
> > The event sequence would thus be something like:
> >
> > ABS_MT_SLOT 0
> > ABS_MT_TRACKING_ID 3
> > ABS_MT_POSITION_X 50
> > ABS_MT_POSITION_X 50
> > SYN_REPORT
> > ABS_MT_TRACKING_ID -1
> > SYN_REPORT
> > SYN_DROPPED
> > ABS_MT_POSITION_X 100
> > SYN_REPORT
> 
> Given the current code, yes, this is expected: we choose to drop the
> touches which appeared and disappeared during SYN_DROPPED.
> 
> >
> > Now, the question is: should we discard these events in libevdev or just
> > tell the clients that they need to expect this to happen?
> 
> My first reaction was to say: yeah, this is expected.
> Actually, after a little bit of thinking, we should simply drop them,
> and put an out of range value in the internal slot state.
> This way, next time we get the axis event, chances are even fewer to
> get the same axis value and the client will be updated accordingly.

will the kernel let us put an out-of-range value in?
because it is the kernel that limits the events, not libevdev. if we discard the
events in libevdev we'd have to remember each axis and value and send a new
event out next time there's a valid touch point.

so something like:
  if (tracking id of current slot == -1)
      libevdev->state[axis] = value
      libevdev->update_needed |= 1 << axis

  if (axis == ABS_MT_TRACKING_ID)
      send_event(axis)
      for each bit libevdev->update_needed:
          send_event(bit)
          clear_bit(bit)

when I write this though I get really worried. this is introducing too much
hidden logic into libevdev's behaviour. and doing so may become limiting in
the future if there is a use-case where this behaviour doesn't apply.

> Forwarding them to the client might introduce spurious events because
> this is not sth it expects to see.

with the above and further thinking about this I reckon this is just one
more thing we should document but not handle otherwise.

fwiw, the xorg evdev driver isn't affected by this. libinput and synaptics
may send wrong events if this happens, though I need more testing to really
be sure.

Cheers,
   Peter

> 
> > afaict if we don't pass on the changed axes we may miss out on events in the
> > future, the kernel wouldn't give us the axis on the next touchpoint if the
> > value is the same. This is unlikely, but still possible.
> 
> See my answer above.
> 
> Cheers,
> Benjamin