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

(Regression) TrackMania Nations Forever under Wine does not always register key releases under IBus 1.5.28 #2480

Closed
joanbm opened this issue Feb 26, 2023 · 13 comments

Comments

@joanbm
Copy link

joanbm commented Feb 26, 2023

Which distribution and version?: Arch Linux (updated as of 2023-02-25)
Which desktop environment and version?: Sway 1.8.1
Which session type?: Wayland
Which application and version?: Wine 8.2, Trackmania Nations Forever
IBus version?: IBus 1.5.28

Issue description:

While playing TrackMania Nations Forever under Wine, I noticed that sometimes (once in a few minutes) the car continued steering after fully releasing the corresponding input key on my keyboard. Pressing and re-releasing the corresponding input fixed the problem. So, it appears that the game failed to register the key release event.

After some troubleshooting it appears the problem appeared with IBus 1.5.28. Killing IBus or downgrading to IBus 1.5.27 fixes the problem.

The problem seems related to commit 506ac99. Setting IBUS_ENABLE_SYNC_MODE=1 when launching the IBus 1.5.28 daemon fixes the problem as well.

Steps to reproduce:

  1. Install and enable IBus 1.5.28
  2. Install TrackMania Nations Forever under Wine
  3. Try to play the game normally
  4. Notice that sometimes the car steers to some direction without pressing the coresponding input

Can you reproduce your problem when you restart ibus-daemon?: Yes
Do you see any errors when you run ibus-daemon with the verbose option?: No
Can you reproduce your problem with a new user account instead of the current your account? Yes

Additional information & test program

Since reproducing the bug in Wine is quite hard and uninformative I tried writing a standalone test program (see below) to try to reproduce and debug the issue.

I observe the following problems under IBUS_ENABLE_SYNC_MODE=2 but not under IBUS_ENABLE_SYNC_MODE=1:

  1. When randomly pressing keys the errors "ERROR: PRESSING ALREADY PRESSED KEY?" and "RELEASING ALREADY RELEASED KEY?" are printed quite often.

  2. Often both the key release and key press events are delived out-of-order (release then press). Most likely this is what causes TMNF to still think the key is pressed.

This reproduces on both my Arch Linux install and also on a clean Fedora Rawhide 2023-02-25 i3 Spin live session.

Standalone test program

#include <X11/Xlib.h>
#include <X11/Xutil.h>
#include <X11/Xos.h>
#include <stdlib.h>
#include <stdio.h>
#include <locale.h>
#include <stdbool.h>
#include <unistd.h>
#include <time.h>

int main() {
	// START Boilerplate thanks to Ted Yin (Determinant): https://gist.github.com/Determinant/19bbecb6db35312861f6cf9f54fdd3a5

	/* fallback to LC_CTYPE in env */
	setlocale(LC_CTYPE, "");
	/* implementation-dependent behavior, on my machine it defaults to
	 * XMODIFIERS in env */
	XSetLocaleModifiers("");

	/* setting up a simple window */
	Display *dpy = XOpenDisplay(NULL);
	int scr = DefaultScreen(dpy);
	Window win = XCreateSimpleWindow(dpy,
	                                 XDefaultRootWindow(dpy),
	                                 0, 0, 100, 100, 5,
	                                 BlackPixel(dpy, scr),
	                                 BlackPixel(dpy, scr));
	XMapWindow(dpy, win);

	/* initialize IM and IC */
	XIM xim = XOpenIM(dpy, NULL, NULL, NULL);
	XIC ic = XCreateIC(xim,
	                   /* the following are in attr, val format, terminated by NULL */
	                   XNInputStyle, XIMPreeditNothing | XIMStatusNothing,
	                   XNClientWindow, win,
	                   NULL);

	/* focus on the only IC */
	XSetICFocus(ic);
	/* capture the input */
	XSelectInput(dpy, win, KeyPressMask | KeyReleaseMask);
	XPoint spot;
	spot.x = 0;
	spot.y = 0;
	XVaNestedList preedit_attr;
	preedit_attr = XVaCreateNestedList(0, XNSpotLocation, &spot, NULL);
	XSetICValues(ic, XNPreeditAttributes, preedit_attr, NULL);
	XFree(preedit_attr);

	// END Boilerplate thanks to Ted Yin (Determinant): https://gist.github.com/Determinant/19bbecb6db35312861f6cf9f54fdd3a5

	srand(time(NULL));

	bool keystate[256] = {0};
	for (;;)
	{
			XEvent ev;
			XNextEvent(dpy, &ev);
			if (XFilterEvent(&ev, None))
					continue;
			usleep(rand() % 100000);
			if (ev.type == KeyPress)
			{
				printf("Press KeyCode: %d\n", ev.xkey.keycode);
				if (ev.xkey.keycode < 256) {
					if (keystate[ev.xkey.keycode]) {
						printf("ERROR: PRESSING ALREADY PRESSED KEY?\n");
					}
					keystate[ev.xkey.keycode] = true;
				}
			}
			else if (ev.type == KeyRelease)
			{
				printf("Release KeyCode: %d\n", ev.xkey.keycode);
				if (ev.xkey.keycode < 256) {
					if (!keystate[ev.xkey.keycode]) {
						printf("ERROR: RELEASING ALREADY RELEASED KEY?\n");
					}
					keystate[ev.xkey.keycode] = false;
				}
			}
	}
}

Sample bad output

Press KeyCode: 46
Press KeyCode: 45
Press KeyCode: 44
Release KeyCode: 46
Release KeyCode: 45
Release KeyCode: 44
Press KeyCode: 45
Press KeyCode: 46
Release KeyCode: 44
ERROR: RELEASING ALREADY RELEASED KEY?
Release KeyCode: 45
Press KeyCode: 44
Release KeyCode: 46
Release KeyCode: 44
Release KeyCode: 45
ERROR: RELEASING ALREADY RELEASED KEY?
Press KeyCode: 44
Release KeyCode: 46
ERROR: RELEASING ALREADY RELEASED KEY?
Press KeyCode: 45
Press KeyCode: 46
[NOTE: All actual keyboard keys are released at this point]
@fujiwarat
Copy link
Member

Thank you for the test case.

@fujiwarat fujiwarat self-assigned this Feb 28, 2023
fujiwarat added a commit to fujiwarat/ibus that referenced this issue Mar 8, 2023
ibus-x11 now also uses the hybrid process key events with
IBUS_ENABLE_SYNC_MODE=2 and it waits for the async API
with GSource and g_main_context_iteration() in xim_forward_event().

But g_main_context_iteration() calls gdk_event_source_dispatch()
and it can call another xim_forward_event() and the callbacks
of ibus_input_context_process_key_event_async() can be nested.
So if the forwarding API is called out of the callbacks of
ibus_input_context_process_key_event_async(), the key events
order is swapped due to the delayed return of
g_main_context_iteration().

To resolve this issue, the forwarding API should be called in
the callbacks of ibus_input_context_process_key_event_async().

BUG=ibus#2480
@fujiwarat
Copy link
Member

I created the tentative patch above. Could you try it?

@joanbm
Copy link
Author

joanbm commented Mar 8, 2023

@fujiwarat I tried your patch and it appears to fix the problem, I was not able to reproduce the problem in 1h of gameplay of TrackMania Nations Forever (whereas without the patch I reproduced it thrice in 20 minutes).

@scramble45
Copy link

I created the tentative patch above. Could you try it?

Thanks for getting that fixed, any timeline for when this will get merged in?

@fujiwarat
Copy link
Member

Sorry for the late response. Seems #2484 still happens with this patch and I have investigated it. I added an additional patch.

@fujiwarat fujiwarat added this to the 1.5.29 milestone Mar 15, 2023
@joanbm
Copy link
Author

joanbm commented Mar 15, 2023

Hmmmm FWIW some notes from my testing those days:

  • Using only 9d9dca9 fixed the original issue and I never experienced a crash during regular use.

  • However, while I was doing some testing with the test program above, I noticed that if I pressed as many keys as fast as possible by mashing a N-Key Rollover keyboard with the palm of my hand for a few seconds, I could get ibus-x11 to crash with valgrind reporting invalid free() issues at the same function you changed in 33146f4.

  • With both 9d9dca9 and 33146f4 everything seems fine, I can't reproduce the crash above nor the original issue so far.

@fujiwarat
Copy link
Member

Thank you for the tests.
Let's continue to test the two patches and I added the patches in Fedora 38 updates-testing repository for the tests.
If we don't see any other regressions, I will integrate those to the IBus upstream.

fujiwarat added a commit that referenced this issue Mar 23, 2023
ibus-x11 now also uses the hybrid process key events with
IBUS_ENABLE_SYNC_MODE=2 and it waits for the async API
with GSource and g_main_context_iteration() in xim_forward_event().

But g_main_context_iteration() calls gdk_event_source_dispatch()
and it can call another xim_forward_event() and the callbacks
of ibus_input_context_process_key_event_async() can be nested.
So if the forwarding API is called out of the callbacks of
ibus_input_context_process_key_event_async(), the key events
order is swapped due to the delayed return of
g_main_context_iteration().

To resolve this issue, the forwarding API should be called in
the callbacks of ibus_input_context_process_key_event_async().

Fixes: 506ac99

BUG=#2480
@fabiscafe
Copy link

@fujiwarat Thanks for this! Is it OK to apply only commit 497f0c7 on top of standard ibus 1.15.28 or is there anything else that is required?

@Plagman
Copy link

Plagman commented Jun 12, 2023

Any plans to cut a new release with this fix?

@johnnybubonic
Copy link

It will be in 1.5.29 (barring some major breakage with the fix is discovered). It's already in 1.5.29-rc1.

arachnist added a commit to arachnist/nixpkgs that referenced this issue Dec 16, 2023
arachnist added a commit to arachnist/nixpkgs that referenced this issue Dec 18, 2023
arachnist added a commit to arachnist/nixpkgs that referenced this issue Dec 19, 2023
arachnist added a commit to arachnist/nixpkgs that referenced this issue Dec 20, 2023
arachnist added a commit to arachnist/nixpkgs that referenced this issue Dec 26, 2023
@fansari
Copy link

fansari commented Jan 5, 2024

I still have this issue with ibus-1.5.29~rc2-6.fc39.x86_64 (if it is the same issue).
While playing Lacuna it is often not possible to change the direction of the character.
workaround: "ibus restart" after starting Steam or during the game prevents or stops the issue.

When I put this in my ~/.bashrc the issue is also fixed:

export IBUS_ENABLE_SYNC_MODE=2

I also tested to set this environment variable to 1 but this has no effect - in this case the issue is still there.

@oemsysadm
Copy link

oemsysadm commented May 24, 2024

Manjaro Gnome, ibus 1.5.30-1, keys sticking in multiple games (Subnautica, Raft, among others) when running on Proton (GE 9.5, Proton Experimental) through Steam. Seems like maybe this issue?

Keys stick and won't release, then suddenly releases after a while on its own. This started happening somewhat recently. I tried using two different keyboards to rule out hardware issues.

Adding the following to the command line args in Steam did NOT work:

IBUS_ENABLE_SYNC_MODE=2 %command%

EDIT: Installing "fcitx5" and adding the following in steam game command line args has helped so far (I'll edit this post if it did not work):

XMODIFIERS=@im=fcitx %command%

For those coming here for a Manjaro (possibly Arch) solution.

@oemsysadm
Copy link

I still have this issue with ibus-1.5.29~rc2-6.fc39.x86_64 (if it is the same issue). While playing Lacuna it is often not possible to change the direction of the character. workaround: "ibus restart" after starting Steam or during the game prevents or stops the issue.

When I put this in my ~/.bashrc the issue is also fixed:

export IBUS_ENABLE_SYNC_MODE=2

I also tested to set this environment variable to 1 but this has no effect - in this case the issue is still there.

Have you tried just adding this to the command line args for games? Did not work for me, but I'd be curious to see if this is because of my setup or not.

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

No branches or pull requests

8 participants