Skip to content

Simplify the tapping state machine a bit

satrmb requested to merge satrmb/libinput:eliminate-multitap-states into master

The tapping state machine contains a few states that can be optimized away. In particular, MULTITAP and MULTITAP_PALM can be replaced by TAPPED, MULTITAP_DOWN can be replaced by DRAGGING_OR_DOUBLETAP. Intuitively, you would expect a touchpad to behave identically for all possible actions after any number of single-finger taps (e.g. starting a tap-and-drag when placing the finger back down and moving it). However, the current state machine makes an unnecessary distinction between these: one tap brings you to TAPPED, two or more taps bring you to MULTITAP.

Implementing and testing this uncovered an issue with the touchpad_tap_palm_multitap_down_again test - it was failing afterwards when it didn't without the changes. The for loops with <= as condition immediately caught my attention; I rarely use that combination due to the danger of off-by-one bugs. And indeed, this looks like one, because if the test parameter _i is 4, the first two of the loops generate not 4 but 5 taps each, and the last loop checks for 9 taps. A quick comparison with other nearby tests showed that this one was also missing a litest_timeout_tap() call before checking the number of taps, which explains why this discrepancy didn't cause failures earlier: the last tap was still waiting for the tap timer to run out while the button events were already being counted. With my state machine change, one half (button pressed) must've made it through in time or something like that. Anyway, it now likes the unmodified and the modified state machine, and it looks correct to me. The libinput_dispatch(li) line is something I copied from another test: touchpad_tap_palm_multitap (which is right above) has that right after its litest_timeout_tap(). Not sure if it's needed, but I copied it anyway for consistency if nothing else.

Merge request reports