libevdev: flushing state forcefully

[Peter Hutterer]

On Thu, Aug 28, 2014 at 07:29:08PM +0200, David Herrmann wrote:
> Hi
> 
> On Thu, Aug 28, 2014 at 3:19 AM, Peter Hutterer
> <peter.hutterer at who-t.net> wrote:
> > On Wed, Aug 27, 2014 at 01:38:31PM +0200, David Herrmann wrote:
> >> Hi
> >>
> >> I'm hacking on some xkb stuff again and stumbled across some
> >> inconsistencies in libevdev state handling. In particular, there is
> >> currently no easy way to parse initial evdev state when opening a
> >> device. Imagine the following setup:
> >>
> >> 1) ctrl+alt+F2 is pressed, a session switch to session #2 occurs. The
> >> session is activated the first time, therefore, it opens evdev devices
> >> (either directly or via TakeDevice on logind) and initializes its
> >> libevdev state.
> >>
> >> 2) F2 button is released.
> >>
> >> 3) F3 button is pressed. No session switch occurs, as the libevdev has
> >> not generated any event for the ctrl and alt buttons, thus, the xkb
> >> state does not know of them.
> >>
> >> If the session has already been loaded and you switch to it, we
> >> usually force a libevdev resync via LIBEVDEV_READ_FLAG_FORCE_SYNC.
> >> Therefore, the new modifiers are updated in xkb. However, during
> >> device setup, there is no way to get events for the initial state.
> >>
> >> Two solutions:
> >>
> >> 1) Iterate over all the available types+codes and generate events manually.
> >> 2) Don't sync initial state during setup. To keep backwards-compat,
> >>    libevdev_set_fd_no_sync() would be needed (name subject
> >>    to change, obviously).
> >>
> >> Now, solution 1) obviously sounds easier as there is no need to
> >> introduce a new API.
> >
> > the reason I did not include that initial set of events in the queue is
> > simple: in most cases it's acceptable to require the user to press a key
> > again. And in the few cases where it isn't (like the one above) we really
> > only care about a handful of keys (modifiers here) and they're easy enough
> > to check separately.
> >
> > and even then you're replacing one problem with another. in XKB, holding
> > ctrl down for 5 seconds triggers sticky keys. when you vt-switch and you
> > sync the state, do you want sticky keys enabled if you keep ctrl down?
> > it's a semantic issue, should a feature be triggered even though it wasn't
> > initiated in the target? roughly similar to: if I press a button down in
> > application A, I shouldn't start dragging an object when I move into
> > application B - an explicit click in B should be required.
> 
> This is all policy, I understand that. But imho the solution is to
> still provide the underlying events, not to hide it.

what events? that's the thing, we're not actually hiding anything here. the
state of the device is what it is. you're asking to _generate_ events that
aren't actually there.

not that there's anything wrong with that, but it's an important
context difference.

[...]

> > it may be annoying for the caller, but it's also conservative from
> > libevdev's point of view. You're concerned about keys here, and this time
> > all of them and not just modifiers. Fair enough.
> >
> > however, a full flush of the state means all of EV_KEY, EV_ABS, EV_SW, etc.
> > and they'll be delivered as events, so the caller is more likely to process
> > them as such. And now you're more likely to run into a couple of issues:
> > * key events may trigger repeats, shortcuts, etc. out-of-order.
> > * SW_FOO may not have changed, but you're now displaying a "you've just
> >   toggled this switch" message
> > * any ABS_MT_* may cause you to create a touchpoint in the caller. now
> >   you're more likely to trigger things like tapping, two-finger scrolling,
> >   or, worse, you don't know what to do because the whole thing is
> >   out-of-order.
> > * you need to reset libevdev's internal state before generating the events.
> >   what are you going to reset ABS_* to? in particular to avoid erroneous
> >   movements, ghost touches, etc.
> >
> > All these are solvable, but they require extra code in the caller's event
> > handling. For SW and ABS the solution is likely "drop them in the caller".
> > But then we're back to: if we need special handling, and we drop most things
> > in the caller after a state flush, why don't we just expect the caller to
> > handle the sync for what they care about using libevdev_get_event_value()?
> 
> Ok, I don't have this problem myself as I handle sync-events
> differently than normal events. But I can understand that it might not
> be that easy to do that in legacy applications.

I'm not just worried about legacy applications. I'm worried that if we make
two things look the same, they may be treated the same which is wrong in
most cases for this set of events.

> >> 3) Add libevdev_reset_state(). This flushes all internal event
> >>    state of an libevdev object. A following forced SYNC will
> >>    then properly resync the device.
> >>    This probably also implies a libevdev_set_fd_no_sync() so
> >>    we avoid the initial state sync.
> >
> > I think a LIBEVDEV_READ_FLAG_FLUSH_STATE could achive the same, no? I don't
> > think we need a separate API call here. And libevdev's internal state should
> > always reflect the kernel state, it should only be out of sync for events
> > pending and not processed yet.
> 
> Yes, that would serve the same purpose.
> 
> > note that the FORCE_SYNC flag doesn't actually reset the state, that's
> > handled on the next call so the libevdev internal state is still correct.
> > For libevdev_reset_state() that would be the same, you'd set a flag in the
> > context but don't actually do anything else. Otherwise, the state is
> > inconsistent.
> >
> > Still not convinced we need that flush though, IMO we need more use-cases
> > that really need the whole state to be flushed _and_ that does something
> > normal with those events rather than more special handling.
> 
> I think I'm fine with both solutions, READ_FLAG_FLUSH_STATE and an
> iterator in the application. As you might guess, I'd prefer the flag,
> but I'm not really pushing on it. I just wanted to check whether you
> had something on your TODO list.

Didn't have anything on my list, no. Those libevdev users I maintain are all
ok to just ignore the initial state and wait for a new event, whether that's
a key press or a new touch starting.

I'll remain on my previous position: we need more use-cases that need that
flush functionality and do something sensible with it before we add it.

Cheers,
   Peter