Skip to content

Fix broken dead key behavior on Wayland #9983

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

Merged
merged 1 commit into from
Jun 8, 2024

Conversation

Hanicef
Copy link

@Hanicef Hanicef commented Jun 6, 2024

Fixes numerous issues regarding dead key behavior on Wayland.

Description

This fixes numerous problems regarding dead keys on Wayland. Most
notably, Wayland was enforcing dead keys on SDL_KEYDOWN and SDL_KEYUP
events, which caused unresponsiveness on keys that were mapped to dead
keys (tilde on US-Intl is most notable for this, commonly used as a
console key).

In addition to this, Wayland was also ignoring SDL_StopTextInput calls,
and proceeded to process dead keys regardless if text input is disabled
or not. This caused issues on console keys where the first keypress
after the console key is merged with the dead key, causing the first key
to appear as à instead of a.

Existing Issue(s)

None

@slouken slouken requested a review from Kontrabant June 6, 2024 16:53
@Kontrabant
Copy link
Contributor

Hmm, there seems to be a deeper problem with the text input protocol not being properly activated at initial window creation time, which this masks, but doesn't fix. Investigating now.

@slouken
Copy link
Collaborator

slouken commented Jun 6, 2024

FYI, text input isn't activated by default in SDL3. You need to call SDL_StartTextInput() if you want to enable text input.

@Kontrabant
Copy link
Contributor

I'm seeing this in testime, which does activate text input. The problem is that the protocol is initially enabled while the window is unmapped, but according to the spec it needs to be re-enabled "every time the active text input changes to a new one, including within the current surface", which means that it needs to be re-enabled when the window is mapped and initially gains focus, since the text input was changed.

The behavior introduced by this patch hacks around this, by basically bypassing it, but doesn't fix the root cause. Easy enough to fix by ensuring that it is re-enabled on keyboard enter events.

@Hanicef
Copy link
Author

Hanicef commented Jun 7, 2024

@Kontrabant You were on the right track, but it wasn't the text input protocol that was causing this. Instead, it was XKB that was not reset upon enabling text input, which is apparently responsible for both composite and dead keys. Since this was not reset, keys were carrying over from that, which is what caused the à to appear on dead keys when text input was disabled.

I still went ahead and re-initialized text input on keyboard enter for correctness, though. Chances are that some compositor out there breaks if this isn't done correctly, so it's best to follow the standards whenever possible.

This fixes numerous problems regarding dead keys on Wayland. Most
notably, Wayland was enforcing dead keys on SDL_KEYDOWN and SDL_KEYUP
events, which caused unresponsiveness on keys that were mapped to dead
keys (tilde on US-Intl is most notable for this, commonly used as a
console key).

When starting text input, not all state was reset properly. The text
input protocol requires to be re-enabled every time text input changes,
which SDL did not do. Also, XKB compose state was not reset at all,
causing composite and dead keys to carry over from when text input was
disabled.
@Kontrabant Kontrabant force-pushed the fix-text-input-ignored branch from 057e1d6 to 64b3e82 Compare June 8, 2024 15:39
@Kontrabant
Copy link
Contributor

Kontrabant commented Jun 8, 2024

I squashed your commits into one and made a few extra cleanups:

  • The extra commits don't seem to be necessary (I think they were trying to race with the window being mapped), so they were removed
  • The higher level SDL code will automatically call StartTextInput automatically when the window focus changes, so all that was needed was to remove the dedup check.
  • The xkb compose state is cleared, even if the text input protocol isn't present, and when text input is stopped.

With this, looks good to me.

@Kontrabant Kontrabant merged commit 1c3090a into libsdl-org:SDL2 Jun 8, 2024
37 checks passed
@Kontrabant
Copy link
Contributor

Merged, thanks!

@slouken
Copy link
Collaborator

slouken commented Jun 8, 2024

Is there a main version of this commit?

@Kontrabant
Copy link
Contributor

Working on it. Trying to cherry-pick results in a merge mess, so doing it manually.

@slouken
Copy link
Collaborator

slouken commented Jun 8, 2024

Thanks!

@Kontrabant
Copy link
Contributor

Manually ported and merged into main.

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

Successfully merging this pull request may close these issues.

3 participants