Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Holding key down sends keyup events with 2.0.5 #2290

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 0 comments
Closed

Holding key down sends keyup events with 2.0.5 #2290

SDLBugzilla opened this issue Feb 11, 2021 · 0 comments

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: 2.0.5
Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2016-10-26 00:39:32 +0000, Jari Ronkainen wrote:

When holding a key down, keyup events are received with SDL2.0.5

Wrote a simple program to illustrate the problem, since I suck at explaining:

#include <SDL.h>
#include
#include <unordered_map>

int main()
{
SDL_Init(SDL_INIT_VIDEO);

SDL_Window* window;

window = SDL_CreateWindow("SDL2 2.0.5 keyboard bug",
                          SDL_WINDOWPOS_CENTERED,
                          SDL_WINDOWPOS_CENTERED,
                          64, 64,
                          SDL_WINDOW_SHOWN);
SDL_Event sdlevent;

std::unordered_map<uint16_t, bool> keys_down;

while(1)
{
    SDL_PollEvent(&sdlevent);

    if (sdlevent.type == SDL_KEYUP) {
        if (keys_down[sdlevent.key.keysym.scancode] == true)
        {
            std::cout << "keyup: scancode: "
                      << sdlevent.key.keysym.scancode
                      << "\n";

            keys_down[sdlevent.key.keysym.scancode] = false;
        }
    } else if (sdlevent.type == SDL_KEYDOWN) {
        if (keys_down[sdlevent.key.keysym.scancode] == false)
        {
            std::cout << "keydown: scancode: "
                      << sdlevent.key.keysym.scancode
                      << "\n";

            keys_down[sdlevent.key.keysym.scancode] = true;
        }

        if (sdlevent.key.keysym.scancode == 41) // quit on esc
            break;
    }
}
SDL_Quit();

}

Holding key down causes following output:
keydown: scancode: 81
keyup: scancode: 81
keydown: scancode: 81
keyup: scancode: 81
keydown: scancode: 81
keyup: scancode: 81
keydown: scancode: 81
keyup: scancode: 81
keydown: scancode: 81
keyup: scancode: 81
keydown: scancode: 81
keyup: scancode: 81
keydown: scancode: 81
keyup: scancode: 81
keydown: scancode: 81
keyup: scancode: 81
keydown: scancode: 81
keyup: scancode: 81
...
etc.

Equivalent output on 2.0.4
keydown: scancode: 81

On 2016-10-27 09:13:37 +0000, Ozkan Sezer wrote:

Possibly related: https://forums.libsdl.org/viewtopic.php?t=12168
which points to https://hg.libsdl.org/SDL/rev/c3874aa1f2d1 as the
culprit. Installing ibus-devel (e.g.: yum install ibus-devel) fixed
the test case for me.

On 2016-10-27 16:55:07 +0000, Alex Baines wrote:

I tried the code, but get the 2.0.4 style output here. Could you provide this info, so I can try and track it down?

  1. Distro + version etc.

  2. Did you build SDL-2.0.5 yourself or use a pre-built version.

  3. If you built it yourself, was it using cmake or autotools, and was anything printed about IME when building?

  4. The output of 'xprop -root | grep XKB'

  5. The output of 'echo $XMODIFIERS'

  6. The output of 'ps ax | grep ibus'

  7. The output of 'ibus-daemon --version'

Thanks

On 2016-10-27 17:21:21 +0000, Jari Ronkainen wrote:

It seems that Ozkan is right, I now built this twice, both with and without ibus headers. Seems that it works when built with the ibus headers available.

Here's requested info, anyways:

  1. Distro + version etc.
    Arch Linux
  1. Did you build SDL-2.0.5 yourself or use a pre-built version.
    Used pre-built
  1. The output of 'xprop -root | grep XKB'
    _XKB_RULES_NAMES(STRING) = "evdev", "pc105", "fi", "", ""
  1. The output of 'echo $XMODIFIERS'
    @im=ibus
  1. The output of 'ps ax | grep ibus'
    1417 ? Ssl 1:19 ibus-daemon -dxr
    1420 ? Sl 0:00 /usr/lib/ibus/ibus-dconf
    1422 ? Sl 0:17 /usr/lib/ibus/ibus-ui-gtk3
    1425 ? Sl 0:45 /usr/lib/ibus/ibus-x11 --kill-daemon
    1460 ? Sl 0:24 /usr/lib/ibus/ibus-engine-simple
    2186 ? Sl 0:00 /usr/lib/ibus/ibus-engine-m17n --ibus
  1. The output of 'ibus-daemon --version'
    ibus-daemon - Version 1.5.14

On 2016-10-27 18:00:58 +0000, Ozkan Sezer wrote:

(In reply to Alex Baines from comment # 2)

  1. Distro + version etc.

Fedora 20 - x86_64

  1. Did you build SDL-2.0.5 yourself or use a pre-built version.

Built myself, first without ibus-devel and dbus-devel installed
which displayed the issue, and second time with ibus-devel and
dbus-devel installed which removed the issue.

  1. If you built it yourself, was it using cmake or autotools, and was
    anything printed about IME when building?

  2. The output of 'xprop -root | grep XKB'

  3. The output of 'echo $XMODIFIERS'

  4. The output of 'ps ax | grep ibus'

  5. The output of 'ibus-daemon --version'

Will post answers to 3-7 tomorrow if you want them, when I have access
to the same system again.

On 2016-10-27 18:39:21 +0000, Alex Baines wrote:

Ok, I think I understand the issue now.

If neither the ibus nor fcitx headers are present, then XSetLocaleModifiers gets called with "", instead of the usual "@im=none". This means for many people they'll get ibus via XIM.

This means that XFilterEvent will return true for some key events, indicating that no futher processing should happen. However for SDL it makes sense to send key events even if they are filtered, otherwise dead keys will not generate an SDL keypress / release.

The problem is that ibus via XIM has a bad habit of returning true from XFilterEvents for everything, and then resending another event later if it didn't really need to filter it.

In SDL 2.0.4, to avoid this case the extra processing didn't happen after XFilterEvent, so dead keys would not generate any events. For 2.0.5 I added the dead key sending back in, along with setting the locale modifiers to "@im=none" if XMODIFIERS was "@im=ibus" to avoid the resending issue.

However with the addition of fcitx, some of the IME stuff was reworked so if SDL is not built with the ibus headers, then the "@im=none" setting doesn't happen.

So, to fix it, we could:

  1. Go back to not generating key presses after XFilterEvent returns true when built without the ibus headers. (2.0.4 behaviour)

  2. Force "@im=none" when XMODIFIERS is "@im=ibus" even if the dbus path won't be used. (should generate events for dead keys but might break some expectations for people who really want to use ibus via xim)

  3. Find another workaround for ibus' resending, maybe by looking at the timestamps / serials of key events to determine if they're resent ones.

On 2016-10-28 00:33:08 +0000, Alex Baines wrote:

Created attachment 2592
Patch to not forward filtered events when compiled without fcitx/ibus

Here is a patch that implements the 1) solution, since that is the simplest.

The drawback is if SDL is compiled without the dbus fcitx / ibus implementation then dead keys won't generate key events. But that is how 2.0.4 worked.

On 2016-10-28 05:24:43 +0000, Ozkan Sezer wrote:

(In reply to Alex Baines from comment # 6)

Created attachment 2592 [details]
Patch to not forward filtered events when compiled without fcitx/ibus

Here is a patch that implements the 1) solution, since that is the simplest.

The drawback is if SDL is compiled without the dbus fcitx / ibus
implementation then dead keys won't generate key events. But that is how
2.0.4 worked.

Did a 'yum remove dbus-devel ibus-devel', applied the proposed patch, and
built 2.0.5: Doesn't fix the issue for me.

On 2016-10-28 16:43:08 +0000, Alex Baines wrote:

Yeah, my bad.

That patch fixes the double events, but the key repeat logic is still broken with it. I'll investigate further.

On 2016-10-28 18:01:59 +0000, Alex Baines wrote:

Created attachment 2594
fix double events & key repeat flag when built without fcitx/ibus headers

Ok, Here's a better patch. I think I tested all the combinations properly this time.

In addition to the last fix, this one uses XkbSetDetectableAutoRepeat to make the key repeat detection work even if ibus is used via xim. If this call fails, then it forces @im=none.

On 2016-10-28 18:38:06 +0000, Ozkan Sezer wrote:

With the new patch applied, 2.0.5 built both in the absence and in the presence
of dbus-devel/ibus-devel seems to work properly with the testcase for me. Thanks.

On 2016-10-29 00:17:20 +0000, Sam Lantinga wrote:

Got it, thanks!
https://hg.libsdl.org/SDL/rev/b48d8a98e261

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant