[PATCH v2 0/8] MR5319: winemac.drv keyboard layout/dead key fixes
This is a series of commits tackling dead key state handling and scan code to vkey mapping improvements (especially for French and German keyboards). * https://gitlab.winehq.org/mzent/wine/-/commit/6c5c90b746cc341049eec216e3bb94...: Simplifies the logic a bit there and also correctly null-terminates dead keys in ToUnicodeEx * https://gitlab.winehq.org/mzent/wine/-/commit/a05e2fc5068a22cae94bc800b49727...: Windows does not do it and some applications misbehave with this, fully fixed later in https://gitlab.winehq.org/mzent/wine/-/commit/7cf1f6a00f17b0626126448f77a1cb... * https://gitlab.winehq.org/mzent/wine/-/commit/4bd96932139a22f2c3b1311e44c43f...: MSDN states single keys are always uppercased there. * https://gitlab.winehq.org/mzent/wine/-/commit/bac8e97d7e6a7494672d76f5fc5868...: There is a bug in `CFStringCompare` with the flags used there currently, resulting in the comparison of the identical buffers "ß" and "ß" to be false. In any case `UCCompareText` behaves correctly and is much faster there. * https://gitlab.winehq.org/mzent/wine/-/commit/bac5f485a09fbddd39efb8f2c0d87e...: I think this was an accidental mistake there, going through all modifiers first per symbol almost always results in the wrong match being made. The logic is adjusted there now to be similar to the other iterations involving `char_matches_string`. * https://gitlab.winehq.org/mzent/wine/-/commit/ffde55df4ae3f6d53198839c583569... https://gitlab.winehq.org/mzent/wine/-/commit/a453d0d4c89c59c3e1ba6c24d2d0f6...: Adds additional symbol vkey mappings (now completely maps a German Macintosh and PC keyboard correctly). There are still some minor defects with the French layout (around the ú region), however changing the symbol mapping there will cause issues with other layouts I think, since the wrong match happens very early there in the priority in these cases. With this change at least the number row is now behaving correctly. Additionally the French Macintosh keyboard layout is further complicated by the "=" key being next to right shift, which is in conflict with pretty much any other keyboard layout. To fix this properly would require a partial rewrite of the heuristic being used or a new approach, but most likely more of a long term project. * https://gitlab.winehq.org/mzent/wine/-/commit/7cf1f6a00f17b0626126448f77a1cb...: Gives dead keys friendly names in `GetKeyNameFriendlyText`, just like Windows does. I tried to be as exhaustive as possible, but if there are any missing the mapping can be easily extended. All in all the incorrect scan code to vkey mapping is usually not that tragic, since an application would need to be aware of the layout and have its own mapping of scan codes of each (FFXIV does this though). The French layout corrections are now "good enough" to behave correctly for all the hotbar slots being assigned there with these changes now though. -- v2: winemac.drv: Give dead keys a friendly name in GetKeyNameText. winemac.drv: Add additional French symbol vkeys. winemac.drv: Add additional German symbol vkeys. winemac.drv: Resolve symbol vkeys first without modifiers. winemac.drv: Use UCCompareText in char_matches_string. winemac.drv: Uppercase single keys in GetKeyNameText. winemac.drv: Do not append " dead" to dead keycodes in GetKeyNameText. winemac.drv: Handle length of dead keycodes in ToUnicodeEx correctly. https://gitlab.winehq.org/wine/wine/-/merge_requests/5319
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/winemac.drv/keyboard.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index 474ca499961..755c6516d77 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -1622,7 +1622,7 @@ INT macdrv_ToUnicodeEx(UINT virtKey, UINT scanCode, const BYTE *lpKeyState, UInt32 modifierKeyState; OptionBits options; UInt32 deadKeyState, savedDeadKeyState; - UniCharCount len; + UniCharCount len = 0; BOOL dead = FALSE; TRACE_(key)("virtKey 0x%04x scanCode 0x%04x lpKeyState %p bufW %p bufW_size %d flags 0x%08x hkl %p\n", @@ -1758,18 +1758,18 @@ INT macdrv_ToUnicodeEx(UINT virtKey, UINT scanCode, const BYTE *lpKeyState, &savedDeadKeyState, bufW_size, &len, bufW); if (status != noErr) { - ERR_(key)("Couldn't translate keycode 0x%04x, status %d\n", keyc, status); + ERR_(key)("Couldn't translate dead keycode 0x%04x, status %d\n", keyc, status); goto done; } - dead = TRUE; + if (len >= 1) + dead = TRUE; } if (len > 0) len = strip_apple_private_chars(bufW, len); - if (dead && len > 0) ret = -1; - else ret = len; + ret = dead ? -len : len; /* Control-Return produces line feed instead of carriage return. */ if (ret > 0 && (lpKeyState[VK_CONTROL] & 0x80) && virtKey == VK_RETURN) @@ -1783,10 +1783,10 @@ INT macdrv_ToUnicodeEx(UINT virtKey, UINT scanCode, const BYTE *lpKeyState, done: /* Null-terminate the buffer, if there's room. MSDN clearly states that the caller must not assume this is done, but some programs (e.g. Audiosurf) do. */ - if (1 <= ret && ret < bufW_size) - bufW[ret] = 0; + if (1 <= len && len < bufW_size) + bufW[len] = 0; - TRACE_(key)("returning %d / %s\n", ret, debugstr_wn(bufW, abs(ret))); + TRACE_(key)("returning %d / %s\n", ret, debugstr_wn(bufW, len)); return ret; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5319
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/winemac.drv/keyboard.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index 755c6516d77..335d8136606 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -1310,7 +1310,6 @@ INT macdrv_GetKeyNameText(LONG lparam, LPWSTR buffer, INT size) { if (thread_data->keyc2scan[keyc] == scan) { - static const WCHAR dead[] = {' ','d','e','a','d',0}; const UCKeyboardLayout *uchr; UInt32 deadKeyState = 0; UniCharCount len; @@ -1370,12 +1369,6 @@ INT macdrv_GetKeyNameText(LONG lparam, LPWSTR buffer, INT size) if (!len) break; - if (status == noErr && deadKeyState) - { - lstrcpynW(buffer + len, dead, size - len); - len = wcslen(buffer); - } - TRACE("lparam 0x%08x -> %s\n", (unsigned int)lparam, debugstr_w(buffer)); return len; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5319
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/winemac.drv/keyboard.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index 335d8136606..250db8a07ba 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -1369,6 +1369,17 @@ INT macdrv_GetKeyNameText(LONG lparam, LPWSTR buffer, INT size) if (!len) break; + if (status == noErr && len == 1) + { + CFStringRef stringRef = CFStringCreateWithCharacters(kCFAllocatorDefault, buffer, len); + CFMutableStringRef mutableStringRef = CFStringCreateMutableCopy(kCFAllocatorDefault, 0, stringRef); + + CFStringUppercase(mutableStringRef, CFLocaleGetSystem()); + CFStringGetCharacters(mutableStringRef, CFRangeMake(0, CFStringGetLength(mutableStringRef)), buffer); + CFRelease(stringRef); + CFRelease(mutableStringRef); + } + TRACE("lparam 0x%08x -> %s\n", (unsigned int)lparam, debugstr_w(buffer)); return len; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5319
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/winemac.drv/keyboard.c | 42 ++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index 250db8a07ba..5188b45caee 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -398,18 +398,18 @@ static const struct { { VK_VOLUME_UP | 0x100, "Volume Up" }, }; -static BOOL char_matches_string(WCHAR wchar, UniChar *string, BOOL ignore_diacritics) +static Boolean char_matches_string(WCHAR wchar, UniChar *string, CollatorRef collatorRef) { - BOOL ret; - CFStringRef s1 = CFStringCreateWithCharactersNoCopy(NULL, (UniChar*)&wchar, 1, kCFAllocatorNull); - CFStringRef s2 = CFStringCreateWithCharactersNoCopy(NULL, string, wcslen(string), kCFAllocatorNull); - CFStringCompareFlags flags = kCFCompareCaseInsensitive | kCFCompareNonliteral | kCFCompareWidthInsensitive; - if (ignore_diacritics) - flags |= kCFCompareDiacriticInsensitive; - ret = (CFStringCompare(s1, s2, flags) == kCFCompareEqualTo); - CFRelease(s1); - CFRelease(s2); - return ret; + Boolean equivalent; + OSStatus status; + + status = UCCompareText(collatorRef, (UniChar*)&wchar, 1, string, wcslen(string), &equivalent, NULL); + if (status != noErr) + { + WARN("Failed to compare %s to %s\n", debugstr_wn(&wchar, 1), debugstr_w(string)); + return FALSE; + } + return equivalent; } @@ -657,6 +657,9 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) int keyc; WCHAR vkey; const UCKeyboardLayout *uchr; + LocaleRef localeRef; + CollatorRef collatorRef, caseInsensitiveCollatorRef, diacriticInsensitiveCollatorRef; + UCCollateOptions collateOptions = 0; const UInt32 modifier_combos[] = { 0, shiftKey >> 8, @@ -778,6 +781,13 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) uchr = (const UCKeyboardLayout*)CFDataGetBytePtr(thread_data->keyboard_layout_uchr); + LocaleRefFromLocaleString("POSIX", &localeRef); + UCCreateCollator(localeRef, 0, collateOptions, &collatorRef); + collateOptions |= kUCCollateComposeInsensitiveMask | kUCCollateWidthInsensitiveMask | kUCCollateCaseInsensitiveMask; + UCCreateCollator(localeRef, 0, collateOptions, &caseInsensitiveCollatorRef); + collateOptions |= kUCCollateDiacritInsensitiveMask; + UCCreateCollator(localeRef, 0, collateOptions, &diacriticInsensitiveCollatorRef); + /* Using the keyboard layout, build a map of key code + modifiers -> characters. */ memset(map, 0, sizeof(map)); for (keyc = 0; keyc < ARRAY_SIZE(map); keyc++) @@ -824,7 +834,7 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) if (thread_data->keyc2vkey[keyc] || !map[keyc][combo][0]) continue; - if (char_matches_string(vkey, map[keyc][combo], ignore_diacritics)) + if (char_matches_string(vkey, map[keyc][combo], ignore_diacritics ? diacriticInsensitiveCollatorRef : caseInsensitiveCollatorRef)) { thread_data->keyc2vkey[keyc] = vkey; vkey_used[vkey] = 1; @@ -850,7 +860,7 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) if (thread_data->keyc2vkey[keyc] || !map[keyc][combo][0]) continue; - if (char_matches_string(vkey, map[keyc][combo], FALSE)) + if (char_matches_string(vkey, map[keyc][combo], collatorRef)) { thread_data->keyc2vkey[keyc] = vkey; vkey_used[vkey] = 1; @@ -879,7 +889,7 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) if (thread_data->keyc2vkey[keyc] || !map[keyc][combo][0]) continue; - if (char_matches_string(symbol_vkeys[i].wchar, map[keyc][combo], FALSE)) + if (char_matches_string(symbol_vkeys[i].wchar, map[keyc][combo], collatorRef)) { thread_data->keyc2vkey[keyc] = vkey; vkey_used[vkey] = 1; @@ -982,6 +992,10 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) vkey_used[vkey] = 1; TRACE("keyc 0x%04x -> vkey 0x%04x (spare vkey)\n", keyc, vkey); } + + UCDisposeCollator(&collatorRef); + UCDisposeCollator(&caseInsensitiveCollatorRef); + UCDisposeCollator(&diacriticInsensitiveCollatorRef); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5319
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/winemac.drv/keyboard.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index 5188b45caee..444e16cadbc 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -874,15 +874,15 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) /* Now try to match key codes for certain common punctuation characters to the most common OEM vkeys (e.g. '.' to VK_OEM_PERIOD). */ - for (i = 0; i < ARRAY_SIZE(symbol_vkeys); i++) + for (combo = 0; combo < ARRAY_SIZE(modifier_combos); combo++) { - vkey = symbol_vkeys[i].vkey; + for (i = 0; i < ARRAY_SIZE(symbol_vkeys); i++) + { + vkey = symbol_vkeys[i].vkey; - if (vkey_used[vkey]) - continue; + if (vkey_used[vkey]) + continue; - for (combo = 0; combo < ARRAY_SIZE(modifier_combos); combo++) - { for (keyc = 0; keyc < ARRAY_SIZE(map); keyc++) { if (!thread_data->keyc2scan[keyc]) continue; /* not a known Mac key code */ @@ -898,9 +898,6 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) break; } } - - if (vkey_used[vkey]) - break; } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5319
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/winemac.drv/keyboard.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index 444e16cadbc..0363f214a80 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -689,6 +689,10 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) { '`', VK_OEM_3 }, { '[', VK_OEM_4 }, { '~', VK_OEM_3 }, + { 0x00DF, VK_OEM_4 }, /* 0x00DF is ESZETT */ + { 0x00FC, VK_OEM_1 }, /* 0x00FC is German U Umlaut */ + { 0x00F6, VK_OEM_3 }, /* 0x00F6 is German O Umlaut */ + { 0x00E4, VK_OEM_7 }, /* 0x00B4 is German A Umlaut */ { '?', VK_OEM_2 }, { ']', VK_OEM_6 }, { '/', VK_OEM_2 }, @@ -711,6 +715,7 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) { '^', VK_OEM_6 }, { '*', VK_OEM_2 }, { '{', VK_OEM_6 }, + { 0x00B4, VK_OEM_6 }, { '~', VK_OEM_1 }, { '?', VK_OEM_PLUS }, { '?', VK_OEM_4 }, @@ -720,6 +725,7 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) { ']', VK_OEM_4 }, { '\'', VK_OEM_3 }, { 0x00A7, VK_OEM_7 }, + { '<', VK_OEM_102 }, }; int i; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5319
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/winemac.drv/keyboard.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index 0363f214a80..3572e66186d 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -723,6 +723,7 @@ void macdrv_compute_keyboard_layout(struct macdrv_thread_data *thread_data) { '?', VK_OEM_COMMA }, { '~', VK_OEM_PLUS }, { ']', VK_OEM_4 }, + { '(', VK_OEM_4 }, { '\'', VK_OEM_3 }, { 0x00A7, VK_OEM_7 }, { '<', VK_OEM_102 }, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5319
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/winemac.drv/keyboard.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index 3572e66186d..01df29a12a6 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -398,6 +398,27 @@ static const struct { { VK_VOLUME_UP | 0x100, "Volume Up" }, }; + +static const struct { + WCHAR wchar; + const char *name; +} dead_key_names[] = { + { '^', "CIRCUMFLEX ACCENT" }, + { '`', "GRAVE ACCENT" }, + { 0x00B4, "ACUTE ACCENT" }, + { '~', "TILDE" }, + { 0x00A8, "DIAERESIS" }, + { 0x00B8, "CEDILLA" }, + { 0x02D8, "BREVE" }, + { 0x02D9, "DOT ABOVE" }, + { 0x00AF, "MACRON" }, + { 0x02DA, "RING ABOVE" }, + { 0x02DB, "OGONEK" }, + { 0x02DC, "SMALL TILDE" }, + { 0x02DD, "DOUBLE ACUTE ACCENT" }, +}; + + static Boolean char_matches_string(WCHAR wchar, UniChar *string, CollatorRef collatorRef) { Boolean equivalent; @@ -1387,6 +1408,20 @@ INT macdrv_GetKeyNameText(LONG lparam, LPWSTR buffer, INT size) if (!len) break; + if (status == noErr && deadKeyState) + { + for (i = 0; i < ARRAY_SIZE(dead_key_names); i++) + { + if (dead_key_names[i].wchar == buffer[0]) + { + len = min(strlen(vkey_names[i].name) + 1, size); + ascii_to_unicode(buffer, dead_key_names[i].name, len); + if (len) buffer[--len] = 0; + break; + } + } + } + if (status == noErr && len == 1) { CFStringRef stringRef = CFStringCreateWithCharacters(kCFAllocatorDefault, buffer, len); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5319
Fixed the build warning and usage of win32 wstring apis. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5319#note_64876
This looks like simple fixes but I'm curious, do you know if it would be possible to build some KBDTABLES similarly to how it's done in winewayland? Like: 1) is macOS providing key scancodes that could be mapped back to hardware scancodes, and 2) ways to inspect the host layout mapping for things like dead key combinations and such? Though 2) / dead key support isn't implemented yet in winewayland / win32u, but libxkbcommon recently introduced functions to inspect the mapping without having to try every combination. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5319#note_64914
On Sat Mar 16 12:41:20 2024 +0000, Rémi Bernon wrote: > This looks like simple fixes but I'm curious, do you know if it would be > possible to build some KBDTABLES similarly to how it's done in > winewayland? Like: > 1) is macOS providing key scancodes that could be mapped back to > hardware scancodes, and > 2) ways to inspect the host layout mapping for things like dead key > combinations and such? > Though 2) / dead key support isn't implemented yet in winewayland / > win32u, but libxkbcommon recently introduced functions to inspect the > mapping without having to try every combination. 1. I do not believe so, the `UC*` class of functions is already an abstraction over the actual hardware scan codes in IOKit, but they still represent hardware keys: ``` /* * Summary: * Virtual keycodes * * Discussion: * These constants are the virtual keycodes defined originally in * Inside Mac Volume V, pg. V-191. They identify physical keys on a * keyboard. Those constants with "ANSI" in the name are labeled * according to the key position on an ANSI-standard US keyboard. * For example, kVK_ANSI_A indicates the virtual keycode for the key * with the letter 'A' in the US keyboard layout. Other keyboard * layouts may have the 'A' key label on a different physical key; * in this case, pressing 'A' will generate a different virtual * keycode. */ ``` 2. Theoretically this is possible, one would have to dig into the `struct UCKeyboardLayout`, which has multiple sub-tables for mappings, and in conjunction with `struct UCKeyModifiersToTableNum`, which it also holds this can be done. What it does though is convert a Mac virtual key code only directly to a utf-16 char. Will take a look at the winewayland implementation to see if something can be improved here, thanks for the hint! -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5319#note_64993
On Sat Mar 16 12:41:20 2024 +0000, Marc-Aurel Zent wrote:
1. I do not believe so, the `UC*` class of functions is already an abstraction over the actual hardware scan codes in IOKit, but they still represent hardware keys: ``` /* * Summary: * Virtual keycodes * * Discussion: * These constants are the virtual keycodes defined originally in * Inside Mac Volume V, pg. V-191. They identify physical keys on a * keyboard. Those constants with "ANSI" in the name are labeled * according to the key position on an ANSI-standard US keyboard. * For example, kVK_ANSI_A indicates the virtual keycode for the key * with the letter 'A' in the US keyboard layout. Other keyboard * layouts may have the 'A' key label on a different physical key; * in this case, pressing 'A' will generate a different virtual * keycode. */ ``` 2. Theoretically this is possible, one would have to dig into the `struct UCKeyboardLayout`, which has multiple sub-tables for mappings, and in conjunction with `struct UCKeyModifiersToTableNum`, which it also holds this can be done. What it does though is convert a Mac virtual key code only directly to a utf-16 char. Will take a look at the winewayland implementation to see if something can be improved here, thanks for the hint! Thanks for the info! And I didn't mean to explore that as an alternative to this MR, it's probably a lot of changes, but I believe it's ultimately the right way to implement keyboard layouts and I was looking for info on how to do that on the macOS side.
(Some applications expect to receive the Windows hardware scan codes, or expect the same mapping between scan codes and virtual keys as native keyboard layouts) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5319#note_64996
participants (3)
-
Marc-Aurel Zent -
Marc-Aurel Zent (＠mzent)
-
Rémi Bernon