[PATCH v2 0/5] MR10779: winex11: Keyboard layouts rework, part 2
-- v2: winex11: Store Xkb keyboard layout LANGID. winex11: Keep a list of every known Xkb keyboard layout. winex11: Move Xkb layouts initialization to a separate function. win32u: Simplify layout name generation for base keyboard layouts. win32u: Tweak a couple of comments. https://gitlab.winehq.org/wine/wine/-/merge_requests/10779
From: Matteo Bruni <mbruni@codeweavers.com> --- dlls/win32u/input.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/dlls/win32u/input.c b/dlls/win32u/input.c index ab09c3b7d42..da203666549 100644 --- a/dlls/win32u/input.c +++ b/dlls/win32u/input.c @@ -872,7 +872,7 @@ DWORD WINAPI NtUserGetQueueStatus( UINT flags ) } /******************************************************************* - * NtUserGetThreadInfo (win32u.@) + * NtUserGetThreadState (win32u.@) */ ULONG_PTR WINAPI NtUserGetThreadState( USERTHREADSTATECLASS cls ) { @@ -977,9 +977,6 @@ static HKL get_locale_kbd_layout(void) /*********************************************************************** * NtUserGetKeyboardLayout (win32u.@) - * - * Device handle for keyboard layout defaulted to - * the language id. This is the way Windows default works. */ HKL WINAPI NtUserGetKeyboardLayout( DWORD thread_id ) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10779
From: Matteo Bruni <mbruni@codeweavers.com> Registry access is unnecessary, the layout name matches the high word of the HKL in those cases. --- dlls/win32u/input.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/dlls/win32u/input.c b/dlls/win32u/input.c index da203666549..cf2e106e4dd 100644 --- a/dlls/win32u/input.c +++ b/dlls/win32u/input.c @@ -1454,10 +1454,20 @@ BOOL WINAPI NtUserGetKeyboardLayoutName( WCHAR *name ) layout = NtUserGetKeyboardLayout( 0 ); id = HandleToUlong( layout ); - if (HIWORD( id ) == LOWORD( id )) id = LOWORD( id ); + TRACE_(keyboard)("hkl %08x\n", id); + if ((id & 0xf0000000) == 0xf0000000) id &= 0x0fffffff; + else id = HIWORD(id); snprintf( buffer, sizeof(buffer), "%08X", id ); asciiz_to_unicode( name, buffer ); + if ((HandleToUlong( layout ) & 0xf0000000) != 0xf0000000) + { + info->kbd_layout_id = id; + + TRACE_(keyboard)( "ret %s\n", debugstr_w( name ) ); + return TRUE; + } + if ((hkey = reg_open_key( NULL, keyboard_layouts_keyW, sizeof(keyboard_layouts_keyW) ))) { while (!NtEnumerateKey( hkey, i++, KeyNodeInformation, key, @@ -1469,8 +1479,6 @@ BOOL WINAPI NtUserGetKeyboardLayoutName( WCHAR *name ) if (query_reg_ascii_value( subkey, "Layout Id", value, sizeof(buffer) ) && value->Type == REG_SZ) id = 0xf000 | (wcstoul( (const WCHAR *)value->Data, NULL, 16 ) & 0xfff); - else - id = wcstoul( klid, NULL, 16 ); NtClose( subkey ); if (HIWORD( layout ) == id) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10779
From: Matteo Bruni <mbruni@codeweavers.com> --- dlls/winex11.drv/keyboard.c | 73 ++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 34 deletions(-) diff --git a/dlls/winex11.drv/keyboard.c b/dlls/winex11.drv/keyboard.c index 65de84678ec..1d75ae78046 100644 --- a/dlls/winex11.drv/keyboard.c +++ b/dlls/winex11.drv/keyboard.c @@ -1446,9 +1446,10 @@ BOOL X11DRV_KeyEvent( HWND hwnd, XEvent *xev ) return TRUE; } +#ifdef SONAME_LIBXKBREGISTRY + static BOOL find_xkb_layout_variant( const char *name, const char **layout, const char **variant ) { -#ifdef SONAME_LIBXKBREGISTRY struct rxkb_layout *iter; if (rxkb_context) @@ -1469,9 +1470,6 @@ static BOOL find_xkb_layout_variant( const char *name, const char **layout, cons } else WARN( "libxkbregistry not available, falling back to fuzzy layout detection\n" ); -#else - WARN( "libxkbregistry support not compiled in, falling back to fuzzy layout detection\n" ); -#endif return FALSE; } @@ -1597,6 +1595,42 @@ static LANGID langid_from_xkb_layout( const char *layout ) return MAKELANGID(LANG_NEUTRAL, SUBLANG_CUSTOM_UNSPECIFIED); }; +#endif + +static BOOL init_xkb_layouts( Display *display, XkbDescRec *xkb_desc, unsigned int xkb_group ) +{ +#ifdef SONAME_LIBXKBREGISTRY + unsigned int count; + char *names[4]; + + XkbGetNames( display, XkbGroupNamesMask, xkb_desc ); + for (count = 0; count < ARRAY_SIZE(xkb_desc->names->groups); count++) + if (!xkb_desc->names->groups[count]) break; + + if (!XGetAtomNames( display, xkb_desc->names->groups, count, names )) count = 0; + for (int i = 0; i < count; i++) + { + const char *layout, *variant = NULL; + LANGID lang; + + if (!names[i]) continue; + if (find_xkb_layout_variant( names[i], &layout, &variant )) + { + lang = langid_from_xkb_layout( layout ); + + TRACE( "Found group %u with name %s -> layout %s:%s, lang %04x\n", i, debugstr_a(names[i]), + debugstr_a(layout), debugstr_a(variant), lang ); + } + XFree( names[i] ); + } + + return TRUE; +#else + WARN( "libxkbregistry support not compiled in, falling back to fuzzy layout detection\n" ); +#endif + return FALSE; +} + /* fuzzy layout detection through keysym / keycode matching, kbd_section must be held */ static void detect_keyboard_layout( Display *display, XModifierKeymap *modmap, unsigned int xkb_group ) { @@ -1960,7 +1994,6 @@ void init_keyboard_layouts( Display *display ) XkbStateRec xkb_state; XModifierKeymap *mmp; XkbDescRec *xkb_desc; - LANGID xkb_lang = 0; Status status; KeyCode *kcp; @@ -1996,41 +2029,13 @@ void init_keyboard_layouts( Display *display ) if ((xkb_desc = XkbGetMap( display, XkbAllClientInfoMask, XkbUseCoreKbd ))) { - char *names[4]; - int count; - - XkbGetNames( display, XkbGroupNamesMask, xkb_desc ); - for (count = 0; count < ARRAY_SIZE(xkb_desc->names->groups); count++) - if (!xkb_desc->names->groups[count]) break; - - if (!XGetAtomNames( display, xkb_desc->names->groups, count, names )) count = 0; - for (int i = 0; i < count; i++) - { - const char *layout, *variant = NULL; - LANGID lang; - - if (!names[i]) continue; - if (find_xkb_layout_variant( names[i], &layout, &variant )) - { - lang = langid_from_xkb_layout( layout ); - if (i == xkb_group) xkb_lang = lang; - - TRACE( "Found group %u with name %s -> layout %s:%s, lang %04x\n", i, debugstr_a(names[i]), - debugstr_a(layout), debugstr_a(variant), lang ); - } - XFree( names[i] ); - } - + init_xkb_layouts( display, xkb_desc, xkb_group ); XkbFreeKeyboard( xkb_desc, 0, True ); } detect_keyboard_layout( display, mmp, xkb_group ); XFreeModifiermap( mmp ); - if (xkb_lang && xkb_lang != main_key_tab[kbd_layout].lcid) - WARN( "Xkb langid %04x differs from detected langid %04x\n", - xkb_lang, main_key_tab[kbd_layout].lcid ); - init_keycode_mappings( display ); pthread_mutex_unlock( &kbd_mutex ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10779
From: Matteo Bruni <mbruni@codeweavers.com> Very much inspired by a patch by Rémi Bernon. Similarly for some of the following patches. --- dlls/winex11.drv/keyboard.c | 53 +++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/dlls/winex11.drv/keyboard.c b/dlls/winex11.drv/keyboard.c index 1d75ae78046..e52a916b7f7 100644 --- a/dlls/winex11.drv/keyboard.c +++ b/dlls/winex11.drv/keyboard.c @@ -55,6 +55,7 @@ #include "kbd.h" #include "wine/server.h" #include "wine/debug.h" +#include "wine/list.h" /* log format (add 0-padding as appropriate): keycode %u as in output from xev @@ -65,6 +66,14 @@ WINE_DEFAULT_DEBUG_CHANNEL(keyboard); WINE_DECLARE_DEBUG_CHANNEL(key); +struct layout +{ + struct list entry; + + int xkb_group; + char *xkb_layout; +}; + static const unsigned int ControlMask = 1 << 2; static int min_keycode, max_keycode, keysyms_per_keycode; @@ -73,9 +82,41 @@ static WORD keyc2vkey[256], keyc2scan[256]; static int NumLockMask, ScrollLockMask, AltGrMask; /* mask in the XKeyEvent state */ static pthread_mutex_t kbd_mutex = PTHREAD_MUTEX_INITIALIZER; +static struct list xkb_layouts = LIST_INIT( xkb_layouts ); static char KEYBOARD_MapDeadKeysym(KeySym keysym); +#ifdef SONAME_LIBXKBREGISTRY +static void create_layout_from_xkb( int xkb_group, const char *xkb_layout ) +{ + struct layout *layout; + + TRACE( "xkb_group %u, xkb_layout %s\n", xkb_group, xkb_layout ); + + LIST_FOR_EACH_ENTRY( layout, &xkb_layouts, struct layout, entry ) + { + if (!strcmp( layout->xkb_layout, xkb_layout )) + { + TRACE( "Found existing layout entry %p\n", layout ); + if (layout->xkb_group == -1) layout->xkb_group = xkb_group; + return; + } + } + + if (!(layout = calloc( 1, sizeof(*layout) + strlen( xkb_layout ) + 1 ))) + { + WARN( "Failed to allocate memory for Xkb layout entry\n" ); + return; + } + list_add_tail( &xkb_layouts, &layout->entry ); + + layout->xkb_group = xkb_group; + layout->xkb_layout = strcpy( (char *)(layout + 1), xkb_layout ); + + TRACE( "Created layout entry %p\n", layout ); +} +#endif + /* Keyboard translation tables */ #define MAIN_LEN 49 static const WORD main_key_scan_qwerty[MAIN_LEN] = @@ -1599,18 +1640,27 @@ static LANGID langid_from_xkb_layout( const char *layout ) static BOOL init_xkb_layouts( Display *display, XkbDescRec *xkb_desc, unsigned int xkb_group ) { + struct layout *entry; #ifdef SONAME_LIBXKBREGISTRY unsigned int count; char *names[4]; +#endif + /* Flag any previously created Xkb layout as invalid */ + LIST_FOR_EACH_ENTRY( entry, &xkb_layouts, struct layout, entry ) + entry->xkb_group = -1; + +#ifdef SONAME_LIBXKBREGISTRY XkbGetNames( display, XkbGroupNamesMask, xkb_desc ); for (count = 0; count < ARRAY_SIZE(xkb_desc->names->groups); count++) if (!xkb_desc->names->groups[count]) break; if (!XGetAtomNames( display, xkb_desc->names->groups, count, names )) count = 0; + TRACE("Found %u group names\n", count); for (int i = 0; i < count; i++) { const char *layout, *variant = NULL; + char buffer[1024]; LANGID lang; if (!names[i]) continue; @@ -1620,6 +1670,9 @@ static BOOL init_xkb_layouts( Display *display, XkbDescRec *xkb_desc, unsigned i TRACE( "Found group %u with name %s -> layout %s:%s, lang %04x\n", i, debugstr_a(names[i]), debugstr_a(layout), debugstr_a(variant), lang ); + + snprintf( buffer, ARRAY_SIZE(buffer), "%s:%s", layout, variant ); + create_layout_from_xkb( i, buffer ); } XFree( names[i] ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10779
From: Matteo Bruni <mbruni@codeweavers.com> --- dlls/winex11.drv/keyboard.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/dlls/winex11.drv/keyboard.c b/dlls/winex11.drv/keyboard.c index e52a916b7f7..2c8ddf82114 100644 --- a/dlls/winex11.drv/keyboard.c +++ b/dlls/winex11.drv/keyboard.c @@ -72,6 +72,8 @@ struct layout int xkb_group; char *xkb_layout; + + LANGID lang; }; static const unsigned int ControlMask = 1 << 2; @@ -87,17 +89,17 @@ static struct list xkb_layouts = LIST_INIT( xkb_layouts ); static char KEYBOARD_MapDeadKeysym(KeySym keysym); #ifdef SONAME_LIBXKBREGISTRY -static void create_layout_from_xkb( int xkb_group, const char *xkb_layout ) +static void create_layout_from_xkb( int xkb_group, const char *xkb_layout, LANGID lang ) { struct layout *layout; - TRACE( "xkb_group %u, xkb_layout %s\n", xkb_group, xkb_layout ); + TRACE( "xkb_group %u, xkb_layout %s, lang %04x\n", xkb_group, xkb_layout, lang ); LIST_FOR_EACH_ENTRY( layout, &xkb_layouts, struct layout, entry ) { if (!strcmp( layout->xkb_layout, xkb_layout )) { - TRACE( "Found existing layout entry %p\n", layout ); + TRACE( "Found existing layout entry %p, lang %04x\n", layout, layout->lang ); if (layout->xkb_group == -1) layout->xkb_group = xkb_group; return; } @@ -113,7 +115,9 @@ static void create_layout_from_xkb( int xkb_group, const char *xkb_layout ) layout->xkb_group = xkb_group; layout->xkb_layout = strcpy( (char *)(layout + 1), xkb_layout ); - TRACE( "Created layout entry %p\n", layout ); + layout->lang = lang; + + TRACE( "Created layout entry %p, lang %04x\n", layout, layout->lang ); } #endif @@ -1672,7 +1676,7 @@ static BOOL init_xkb_layouts( Display *display, XkbDescRec *xkb_desc, unsigned i debugstr_a(layout), debugstr_a(variant), lang ); snprintf( buffer, ARRAY_SIZE(buffer), "%s:%s", layout, variant ); - create_layout_from_xkb( i, buffer ); + create_layout_from_xkb( i, buffer, lang ); } XFree( names[i] ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10779
I pushed my updated branch to [hl-kbl](https://gitlab.winehq.org/Mystral/wine/-/commits/hl-kbl?ref_type=heads), for reference. The commits up to https://gitlab.winehq.org/Mystral/wine/-/commit/4fa555a09c408d71a5b866c355cf... [winex11: Activate initial keyboard layout.] are probably the most relevant for this MR. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10779#note_138412
Rémi Bernon (@rbernon) commented about dlls/win32u/input.c:
if (query_reg_ascii_value( subkey, "Layout Id", value, sizeof(buffer) ) && value->Type == REG_SZ) id = 0xf000 | (wcstoul( (const WCHAR *)value->Data, NULL, 16 ) & 0xfff); - else - id = wcstoul( klid, NULL, 16 ); NtClose( subkey );
if (HIWORD( layout ) == id)
This will leave id to the previously set value when "Layout Id" value is missing for some reason, I don't think it is right. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10779#note_139403
Rémi Bernon (@rbernon) commented about dlls/win32u/input.c:
- if (HIWORD( id ) == LOWORD( id )) id = LOWORD( id ); + TRACE_(keyboard)("hkl %08x\n", id); + if ((id & 0xf0000000) == 0xf0000000) id &= 0x0fffffff; + else id = HIWORD(id); snprintf( buffer, sizeof(buffer), "%08X", id ); asciiz_to_unicode( name, buffer );
+ if ((HandleToUlong( layout ) & 0xf0000000) != 0xf0000000) + { + info->kbd_layout_id = id; + + TRACE_(keyboard)( "ret %s\n", debugstr_w( name ) ); + return TRUE; + } + if ((hkey = reg_open_key( NULL, keyboard_layouts_keyW, sizeof(keyboard_layouts_keyW) ))) There's also IME layouts to consider, they have 0xe000 high bits (instead of 0xf000 for alt layouts). It looks like they don't have a "Layout Id" value assigned and that GetKeyboardLayoutName returns the HKL with the highest bits kept.
I'm also not sure we should use the HKL high word as KLID if registry keys are missing. The high word seems to be a globally unique counter, while the KLID is supposed to be a per-language monotonic counter with predefined values. If we're missing the registry keys, I'd suggest using completely distinct names as we do already. Something like that should cover both, what do you think? ```suggestion:-14+0 if (!(HIWORD( id ) & 0xf000)) id = HIWORD( id ); snprintf( buffer, sizeof(buffer), "%08X", id ); asciiz_to_unicode( name, buffer ); if ((HIWORD( id ) & 0x1000) && (hkey = reg_open_key( NULL, keyboard_layouts_keyW, sizeof(keyboard_layouts_keyW) ))) ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10779#note_139404
Rémi Bernon (@rbernon) commented about dlls/winex11.drv/keyboard.c:
return MAKELANGID(LANG_NEUTRAL, SUBLANG_CUSTOM_UNSPECIFIED); };
+#endif + +static BOOL init_xkb_layouts( Display *display, XkbDescRec *xkb_desc, unsigned int xkb_group ) +{ +#ifdef SONAME_LIBXKBREGISTRY + unsigned int count; + char *names[4];
I still don't think this should be conditionally compiled. There could be other ways to lookup xkb layouts than the xkbregistry library, and that's something that is already isolated in the `find_xkb_layout_variant` function, which could be extended. The conditional compilation only adds more complexity and requires more ifdefs in the next patch. And without the need for more ifdefs, this commit also doesn't seem to be worth doing, especially as we just introduced this block of code. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10779#note_139405
Rémi Bernon (@rbernon) commented about dlls/winex11.drv/keyboard.c:
int xkb_group; char *xkb_layout; + + LANGID lang;
This seems like something to be squashed with the preceding commit. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10779#note_139406
On Mon May 11 10:15:54 2026 +0000, Rémi Bernon wrote:
This will leave id to the previously set value when "Layout Id" value is missing for some reason, I don't think it is right. Ah, indeed, I think I had misunderstood this piece of code :/
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10779#note_139691
On Mon May 11 10:15:54 2026 +0000, Rémi Bernon wrote:
There's also IME layouts to consider, they have 0xe000 high bits (instead of 0xf000 for alt layouts). It looks like they don't have a "Layout Id" value assigned and that GetKeyboardLayoutName returns the HKL with the highest bits kept. I'm also not sure we should use the HKL high word as KLID if registry keys are missing. The high word seems to be a globally unique counter, while the KLID is supposed to be a per-language monotonic counter with predefined values. If we're missing the registry keys, I'd suggest using completely distinct names as we do already. Something like that should cover both, what do you think? ```suggestion:-14+0 if (!(HIWORD( id ) & 0xf000)) id = HIWORD( id ); snprintf( buffer, sizeof(buffer), "%08X", id ); asciiz_to_unicode( name, buffer ); if ((HIWORD( id ) & 0x1000) && (hkey = reg_open_key( NULL, keyboard_layouts_keyW, sizeof(keyboard_layouts_keyW) ))) ``` I like it! I'm going to double check that this works well enough until the registry entries come in, but it should.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10779#note_139692
On Mon May 11 10:15:54 2026 +0000, Rémi Bernon wrote:
I still don't think this should be conditionally compiled. There could be other ways to lookup xkb layouts than the xkbregistry library, and that's something that is already isolated in the `find_xkb_layout_variant` function, which could be extended. The conditional compilation only adds more complexity and requires more ifdefs in the next patch. And without the need for more ifdefs, this commit also doesn't seem to be worth doing, especially as we just introduced this block of code. Okay, I see your point. Hopefully rebasing the rest won't be too tricky...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10779#note_139693
On Mon May 11 10:15:54 2026 +0000, Rémi Bernon wrote:
This seems like something to be squashed with the preceding commit. This is currently followed by "winex11: Store Xkb keyboard layout KLID." and "winex11: Store Xkb keyboard layout id.", which are quite similar to this one in principle but a bit larger and not as trivial. I can merge this one in particular though.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10779#note_139694
On Wed May 13 11:40:33 2026 +0000, Matteo Bruni wrote:
This is currently followed by "winex11: Store Xkb keyboard layout KLID." and "winex11: Store Xkb keyboard layout id.", which are quite similar to this one in principle but a bit larger and not as trivial. I can merge this one in particular though. If it is only adding the data we already have around as local variables into the entries I think it can all be squashed. If it introduces new logic to compute these data and add them into the struct it can probably be kept as separate changes.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10779#note_139696
participants (3)
-
Matteo Bruni -
Matteo Bruni (＠Mystral)
-
Rémi Bernon (＠rbernon)