Summary: | RFE: keyboard debouncing | ||
---|---|---|---|
Product: | Wayland | Reporter: | Adam Dingle <adam> |
Component: | libinput | Assignee: | Wayland bug list <wayland-bugs> |
Status: | RESOLVED WONTFIX | QA Contact: | |
Severity: | enhancement | ||
Priority: | medium | CC: | peter.hutterer |
Version: | unspecified | ||
Hardware: | Other | ||
OS: | All | ||
Whiteboard: | |||
i915 platform: | i915 features: | ||
Attachments: |
output from evemu-record
keyboard state keyboard state redux |
Description
Adam Dingle
2018-02-23 09:13:19 UTC
I think what you see here is not actually bouncing but key repeat triggering. On Wayland, key repeat is handled by the clients based on press/release events received from the compositor (as opposed to Xorg where key repeat is handled by the display server, ie the X server). If the compositor gets “busy” doing something else, it may not forward the key release fast enough and the client start repeating the key... There are some mechanisms in place in gtk+ and Xwayland to mitigate the issue, by issuing a “ping” to the Wayland compositor to makre sure it's still well alive and processing events before repeating a key, but it's not perfect and spurious key repeat can still occur under some circumstances. As a workaround, you could try increasing the key repeat delay in your compositor of choice to see if that helps with the key repeating itself. tl;dr: Nothing libinput can do to help there, it;s purely betwee nWayland client and the Wayland compositor. you can verify whether this happens at the device level by running evemu-record against the keyboard device. If you see the key bouncing there or key repeat events (look for value 2) then it's a hw problem. But right now, I'm betting on what olivier said. So let's close this bug and re-open it if it ends up being a hw problem after all. OK - thanks to both of you for the very helpful comments. I had no idea that this could actually be key repeat triggering. For the moment I've turned off key repeating entirely. We'll see if that solves the problem. If it does, that's a really easy fix since I can live without key repeating. If I still see bouncing keys, I'll try to gather evidence using evemu-record as Peter suggested. Unfortunately even with key repeating completely disabled I'm still seeing the double keystrokes. I can easily capture them using either evemu-record or libinput-debug-events. The delay between a keystroke and its bounce seems to fall in the range of approximately 0.02-0.10 sec. Given this, I'm reopening this RFE, since keyboard debouncing in libinput would be very useful for me. I'll need an evemu-record that shows the bouncing effects please, thanks. Created attachment 137584 [details]
output from evemu-record
OK - here is output from evemu-record. While recording, I retyped the first paragraph of my previous comment in a editor window twice. The space bar bounced four times while typing the paragraph the first time, and twice while typing it the second time.
I ended up with this text:
===
Unfortunately even with key repeating completely disabled I'm still seeing the double keystrokes. I can easily capture them using either evemu-record or libinput-debug-events. The delay between a keystroke and its bounce seems to fall in the range of approximately 0.02-0.10 sec.
Unfortunately even with key repeating completely disabled I'm still seeing the double keystrokes. I can easily capture them using either evemu-record or libinput-debug-events. The delay between a keystroke and its bounce seems to fall in the range of approximately 0.02-0.10 sec.
===
The bounces in the first paragraph are the double spaces after the words "even", "with", "I'm" and "still". The first bounce in the second paragraph is the double space before the word "double". The second is the triple space preceding "The delay"; I actually pressed the space bar twice, but ended up with three spaces.
It's not too hard to find these bounces in the evemu-record output. For example, in the first bounce in the first paragraph, the space bar fires at 10.735195 sec and then again at 10.773511, a difference of 0.038 sec.
In the second bounce, the space bar fires at 11.188439 and again at 11.235823, a difference of 0.047 sec.
Created attachment 137597 [details] keyboard state I wrote a tool to visualise what your keyboard does, see here: https://github.com/whot/input-data-analysis/blob/master/keyboard-state/keyboard-state.py The generated output for your event recording is attached and it shows a number of issues. First, there doesn't seem to be a consistent prediction of when the keyboard bounces. Look at the * rows in the output, these mark events with less than 3ms. Initially I ran this on 8ms and that's even more. I was hoping we'd only get these when multiple keys are down but that doesn't seem to be the case. This makes debouncing a lot harder, because we'll inevitable end up having to discard key events. The one you mentioned at 10.735 can only be debounced if the initial space key press is discarded, the second press does not come in until 32ms later. For reference, the button debounce timeouts are 12 and 25ms, and some consider that too long. Googling around for the issue indicates this is a common enough (hardware) problem: https://www.dell.com/community/General/Precision-5510-keyboard-douuble-characters/m-p/4758956 https://www.reddit.com/r/Dell/comments/5qydpb/keyboard_repeat_issues_wdell_xps_15_9560/ and it appears the hw may be the same as the XPS for which you can find even more complaints. I really suggests taking this up with Dell to get a fix. Honestly, this is something I'm very reluctant to fix in libinput. It'll cost a lot of time and effort, is likely to break thinks for other users and appears to be a hardware bug. Created attachment 137713 [details]
keyboard state redux
Peter,
thanks for the quick response and the in-depth analysis. Your keyboard-state.py is a useful tool and I hope it will live on.
You are looking for bounces by looking for any two consecutive events that are closely separated. I believe that approach will not work. Instead, we need to look for two successive key down events from the *same key* that occur too closely together.
I've modified keyboard-state.py to do exactly this, using a bounce threshold of 70 ms. I ran it on my event recording and have attached the output. It detects exactly the bounces that occurred as I was typing, including one that I didn't mention previously (a bounced "n") since I backspaced over it immediately after it occurred. Here they are:
10.773511 - after "even"
11.235823 - after "with"
15.791665 - after "I'm"
16.211357 - after "still"
66.204838 - 'n' in "key repeating"
72.104026 - before "double"
86.092569 - before "The delay"
I believe this shows that bounce detection is viable, at least for my keyboard.
I'll make a pull request at your github site with my changes to keyboard-state.py, in case you want them.
You've said that you are reluctant to fix this in libinput. I guess the question then comes down to this: would you accept a patch to add keyboard debouncing along these lines?
thanks for the fix, it does show the problem a bit better. Main question now is of course: can you make this trigger for normal sequences? e.g. hitting backspace multiple times, typing mm or ll or any of the common sequences that would cause a user to type fast enough? This is quite important to gauge whether we'll have regressions.
What's the minimum debounce time you can get away with? I can get 23ms press->release time but 38ms is the lowest for release->press. Together they are still way above the 30ms that we have in that script measuring between press and press. So I think the debounce may actually be viable after all.
Given the experience we had with wireless mice in the button debouncing code, this should only be enabled for internal (serial bus?) keyboards. Probably only on specific models known to have broken hardware.
> would you accept a patch to add keyboard debouncing along these lines?
I'll be honest, I'm not enthusiastic about it but depending how complex it is and how well it fixes the issue, maybe. This is still a hardware bug and having to work around this in software (and keeping that behaviour for the next decade) doesn't exactly fill me with joy...
Thanks for reopening this feature request and being willing to consider this at least. Actually in the version of the script I gave you the threshold is 70 ms, not 30 ms. Some of the bounces I observed took nearly 70 ms, so a lower threshold would not detect them all. I was typing fairly quickly when I made the key recording that I attached earlier and the script doesn't find any false positives there, so I think a threshold around 70 ms may be reasonable. Still, if we do add a keyboard bouncing feature to libinput I think it should be off by default (or perhaps enabled with a very low threshold, which is basically equivalent). A user who has a bouncy keyboard should be able to turn it on and set the threshold, either via a configuration setting or (maybe at some point) through a GUI panel in the system keyboard settings. I think it may be hard to find a single threshold value that works perfectly for everyone, hence the need to let users set it. Anyway, you said there is some chance you would accept a patch like this if it is not too complex, though you are not enthusiastic. I have a strong personal need for this feature so I may look into this, unless I buy a new laptop first, which is another way to fix the problem. :) ok, if the threshold has to be at 70ms, then that's not a good sign. That threshold will lead to dropped key events if we ever enable this for a non-bouncing keyboard. It will likely also lead to dropped key events when we're bouncing, see my timing of press/release events.
> A user who has a bouncy keyboard should be able to turn it on and set the
> threshold, either via a configuration setting or (maybe at some point)
> through a GUI panel in the system keyboard settings. I think it may be
> hard to find a single threshold value that works perfectly for everyone,
> hence the need to let users set it.
problem here: after implementing libinput's debouncing support you now need
to modify every single Wayland compositor to support that feature and
provide a configuration option for it. In addition, you need a GUI that is
accessible to actually configure it. Which means that the chances of this
ever being consistently supported are almost nil.
Aside from figuring out what the configuration options are going to be. You need an on/off toggle, a threshold, a keyboard selector (?), etc. These are all moving targets because you need all this in place before you can ship it to the public and get testing results.
I think it might be 'easier' to write an evdev wrapper for the keyboard to debounce at the kernel event level. Then you can just run this on your machine and thus filter events there. less integration needed, and a bit more shoulder-shrugging allowed when something doesn't work ;)
Process is basically:
* open keyboard device, EVIOCGRAB so no-one else sees the events
* create a uinput device that looks identical
* read events from physical device, forward to uinput device after filtering
Using python-libevdev this is probably less than 200 LOC and at least should indicate if the solution can work.
Closing this bug as WONTFIX (again), sorry, see comment #11 for the reasons. I don't think we can have a good implementation that is reliable in libinput. I think that if a keyboard debouncing feature like this is implemented, it has to sit in the compositor where it's much easier to control through configuration options. libinput is too low-level for this. XKB already has something similar anyway for accessibility reasons so there's a chance that compositors can re-use that implementation. OK - the decision to close this sounds reasonable to me. Thanks for taking the time to investigate and consider this at least. My own keyboard has been a bit less bouncy lately, by the way - maybe it is somehow affected by the humidity and/or weather? :) |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.