| Summary: | Keypresses leak to the console with 2.0.5 (didn't happen with 2.0.4) | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Manuel Alfayate Corchete <redwindwanderer> |
| Component: | events | Assignee: | tvc <me> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | critical | ||
| Priority: | P2 | CC: | buzz, philipp.wiesemann |
| Version: | 2.0.5 | ||
| Hardware: | ARM | ||
| OS: | Linux | ||
| Attachments: |
enable keyboard muting
remove tty search and enable keyboard muting |
||
|
Description
Manuel Alfayate Corchete
2016-10-25 09:52:30 UTC
I don't have access to the Raspberry Pi platform. Can you identify which commit caused the regression? I have just took a look at http://hg.libsdl.org/SDL/shortlog, and it's HUGE. Building every "candidate" revision is impossible. However, if you tell me what I should look for, I would be able to do a reasonabe number of builds. I am willing to send you a Raspberry Pi, too. SDL2 is too important on the platform. The patch for bug 3157 may have affected the keyboard handling. (In reply to Philipp Wiesemann from comment #3) > The patch for bug 3157 may have affected the keyboard handling. Can you indicate the revision numbers before and after the suspicious patch was merged? I am not used to this hg and I can't find out. An indication on how to fund out would be great, too. The revision before the patch was applied is "eeb3847d81a6" and the revision after the patch was applied is "2b008531d078". You can see the revision numbers here: https://hg.libsdl.org/SDL/rev/0f79dd727ed6 @Sam Lantiga: I have built both eeb3847d81a6 and 2b008531d078 revisions, and indeed key leakage appears JUST after 2b008531d078. So that's the commit where it appears. Thanks Phillipp for your suggestion as it was spot-on. tvc, can you look at this regression caused by your patch? Thanks! Created attachment 2595 [details]
enable keyboard muting
This was a complete oversight by me, clearly I didn't do proper testing. Sorry all.
I have attached a patch that I believe will fix this bug, however I'm not able to test this personally for a couple of days. Manuel, are you able to test this in the meantime?
me@tvc.id.au This patch doesn't work: keycodes are still leaking to the console and now the console is corrupted after exit: keypresses send wrong codes. Created attachment 2662 [details]
remove tty search and enable keyboard muting
I believe this patch should fix it, instead of looping through all the tty's and seemingly selecting the wrong one and corrupting the console I've just made SDL open /dev/tty which is the console attached to the current process anyway.
I know I took a fair amount of time on this, however, Manuel are you still able to test this to make sure it works on your system?
@tvc, it will be a pleasure to test this patch. SDL2 on the Pi is a fundamental part of it's operating system IMHO... However, I have tried to apply this patch with no success: pi@raspberrypi:~/src/SDL-2.0.5-10739 $ patch -p1 < patch.diff patching file src/core/linux/SDL_evdev.c Hunk #2 succeeded at 134 with fuzz 2. Hunk #3 FAILED at 152. Hunk #4 FAILED at 164. Hunk #5 succeeded at 236 (offset 4 lines). Hunk #6 succeeded at 267 (offset 4 lines). 2 out of 6 hunks FAILED -- saving rejects to file src/core/linux/SDL_evdev.c.rej Do I need an specific SDL2 mercurial commit or something? Sorry I should have specified, it's a patch for the latest mercurial commit. You can grab the latest Mercurial drop from here: http://www.libsdl.org/tmp/SDL-2.0.zip @tvc: This works perfectly well now. No more keypress leaks. As good as 2.0.4 was. So I'd say it's FIXED. Thanks a lot! :) Fixed, thanks! https://hg.libsdl.org/SDL/rev/fe9c7d01a093 tvc, your patch broke SDL on the Steam Link, where the application can run without a tty. I fixed that, but I looked more closely at what the code is doing, and I had some questions for you. It seems like console_fd is being used for two different purposes. It mutes the keyboard so keystrokes only generate evdev events and don't leak through to the console. In this case I think we do want the active console, whatever that is. If the application is running from somebody logged in via ssh, and it opens an evdev keyboard, we want that keyboard input to go to the SDL application and not generate key events on the shell that is logged in at the console. Separately from that, we use the console file descriptor to do translation of key presses into characters. In this case we don't care what tty we get, we just need one that we can query the kernel translation tables from. In bug 3545 someone notes that they're running into the type < 0xf0 case, and I think that's really what we want. We want to put the tty into VC_UNICODE mode, query all the tables into userspace, reset and close the tty, and then translate those unicode values into UTF-8. That still doesn't get us dead key handling and diacritics and so forth, as you noted, but it gets basic US key translation. I haven't dug into the Linux keyboard driver to see how it handles dead keys, but I see posts from people showing that it does work, and we can probably emulate what the keyboard driver does. Remember, SDL programs don't necessarily run from the console, and there's no reason why they can't open keyboards and mice and do the right thing in that case. :) Sorry about the Steam Link break, I didn't realise that corner case. Well we could open /dev/console which is the first VT, switch it to K_UNICODE and read the tables. Ideally this should be done before we open /dev/tty (the current VT) for muting, as if our current VT is the first VT we wouldn't want to unmute it. I'm currently looking at the kernel sources to see how the keyboard driver handles it all, so I should have a working patch soon. (In reply to tvc from comment #19) > Sorry about the Steam Link break, I didn't realise that corner case. > > Well we could open /dev/console which is the first VT, switch it to > K_UNICODE and read the tables. Ideally this should be done before we open > /dev/tty (the current VT) for muting, as if our current VT is the first VT > we wouldn't want to unmute it. I'm currently looking at the kernel sources > to see how the keyboard driver handles it all, so I should have a working > patch soon. Scratch that, I was wrong, /dev/tty0 is current VT, /dev/tty1 is first VT, and /dev/tty is the VT the process was started in. After discussion this is the correct fix. It doesn't cover someone running the application from ssh, but in that case you can only access the virtual console if you're running as root. Should https://hg.libsdl.org/SDL/rev/fe9c7d01a093 also fix this issue if applied to SDL 2.0.5 or does it rely on some additional patches ? Running Emulation Station on Raspberry Pi directly on the framebuffer (on /dev/tty1) I applied https://hg.libsdl.org/SDL/rev/fe9c7d01a093 to sdl 2.0.5 and the problem is still happening. Characters ending up on the console and the terminal being left in an unusable state on exit. The latest SDL2 code does work however - so I assume I need some additional changes ? I also applied https://hg.libsdl.org/SDL/rev/5ede976d922b but that didn't help either. Unusable console on exit was fixed by cherry-picking https://hg.libsdl.org/SDL/rev/6cfccf993c5d Also needed https://hg.libsdl.org/SDL/rev/6cfccf993c5d or else the keyboard didn't work in some parts of Emulation Station. All seems good now. Looking forward to 2.0.6. Thanks. |