libevdev: flushing state forcefully

[David Herrmann]

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.

>> However, there's one more issue: Imagine you
>> change the xkb-keymap of an in-flight session. We have to destroy the
>> xkb_state object and recreate it for the new keymap. However, this
>> looses all state and we need to resync it. This is impossible to
>> achieve with libevdev, as it assumes we already parsed the state.
>
> see below for a comment.
>
>> Again, two solutions:
>>
>> 1) Iterate over all the available types+codes and generate
>>    events for them.
>> 2) Destroy the libevdev object and recreate it via
>>    libevdev_set_fd_no_sync() and sync yourself.
>>
>> Merging both problems, I see the following proper solutions:
>>
>> 1) Let applications deal with it.
>
> my favourite approach ;)
> libevdev_event_type_get_max() was added as public API to make this possible.

I kinda knew you would say that ;)

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

>> 2) Add LIBEVDEV_FOREACH_CODE() which is a convenience
>>    for-loop that iterates all available type+code combinations
>>    of the libevdev device. This can be easily used to generate
>>    required events in the application itself.
>
> no. no macros in public headers.

Meh.. ok.

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

Thanks
David