[PATCH v2 1/3] user32: Move ToUnicodeEx from wineandroid.drv to user32.
As a default implementation for en_US keyboard layout, calling the driver implementation next so that it can override the result. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- v2: * Use the english keyboard layout for the default impl, let the driver override the result if they can. * Only copy default buffer to user buffer in nulldrv, to leave user buffer untouched for drivers. ToUnicodeEx is not re-entrant when dealing with dead keys (although I don't think Wine uses that). * Use lstrcpynW instead of wcscpy. * Don't call debugstr_wn if len is < 0. * Drop controversial (Un)LoadKeyboardLayoutW patch for now. Supersedes: 204512, 204513, 204518, 204519 dlls/user32/driver.c | 11 ++- dlls/user32/input.c | 80 ++++++++++++++++++- dlls/user32/user_private.h | 2 +- dlls/wineandroid.drv/keyboard.c | 94 ----------------------- dlls/wineandroid.drv/wineandroid.drv.spec | 1 - dlls/winemac.drv/keyboard.c | 3 +- dlls/winemac.drv/winemac.drv.spec | 2 +- dlls/winex11.drv/keyboard.c | 3 +- dlls/winex11.drv/winex11.drv.spec | 2 +- 9 files changed, 90 insertions(+), 108 deletions(-) diff --git a/dlls/user32/driver.c b/dlls/user32/driver.c index 7ac77141696..c385ce0599c 100644 --- a/dlls/user32/driver.c +++ b/dlls/user32/driver.c @@ -284,9 +284,11 @@ static BOOL CDECL nulldrv_RegisterHotKey( HWND hwnd, UINT modifiers, UINT vk ) } static INT CDECL nulldrv_ToUnicodeEx( UINT virt, UINT scan, const BYTE *state, LPWSTR str, - int size, UINT flags, HKL layout ) + int size, UINT flags, HKL layout, + const WCHAR *default_buf, INT default_len ) { - return 0; + lstrcpynW( str, default_buf, size ); + return default_len; } static BOOL CDECL nulldrv_UnloadKeyboardLayout( HKL layout ) @@ -592,9 +594,10 @@ static BOOL CDECL loaderdrv_RegisterHotKey( HWND hwnd, UINT modifiers, UINT vk ) } static INT CDECL loaderdrv_ToUnicodeEx( UINT virt, UINT scan, const BYTE *state, LPWSTR str, - int size, UINT flags, HKL layout ) + int size, UINT flags, HKL layout, + const WCHAR *default_buf, INT default_len ) { - return load_driver()->pToUnicodeEx( virt, scan, state, str, size, flags, layout ); + return load_driver()->pToUnicodeEx( virt, scan, state, str, size, flags, layout, default_buf, default_len ); } static BOOL CDECL loaderdrv_UnloadKeyboardLayout( HKL layout ) diff --git a/dlls/user32/input.c b/dlls/user32/input.c index 805bfe3e9de..51a08d0786b 100644 --- a/dlls/user32/input.c +++ b/dlls/user32/input.c @@ -877,11 +877,83 @@ INT WINAPI ToUnicode(UINT virtKey, UINT scanCode, const BYTE *lpKeyState, /**************************************************************************** * ToUnicodeEx (USER32.@) */ -INT WINAPI ToUnicodeEx(UINT virtKey, UINT scanCode, const BYTE *lpKeyState, - LPWSTR lpwStr, int size, UINT flags, HKL hkl) +INT WINAPI ToUnicodeEx( UINT virt, UINT scan, const BYTE *state, + WCHAR *str, int size, UINT flags, HKL layout ) { - if (!lpKeyState) return 0; - return USER_Driver->pToUnicodeEx(virtKey, scanCode, lpKeyState, lpwStr, size, flags, hkl); + BOOL shift, ctrl, numlock; + WCHAR buffer[2]; + INT len; + + TRACE_(keyboard)( "virt %u, scan %u, state %p, str %p, size %d, flags %x, layout %p.\n", + virt, scan, state, str, size, flags, layout ); + + if (!state) return 0; + + shift = state[VK_SHIFT] & 0x80; + ctrl = state[VK_CONTROL] & 0x80; + numlock = state[VK_NUMLOCK] & 0x01; + + /* FIXME: English keyboard layout specific */ + + if (scan & 0x8000) buffer[0] = 0; /* key up */ + else if (!ctrl) + { + switch (virt) + { + case VK_BACK: buffer[0] = '\b'; break; + case VK_OEM_1: buffer[0] = shift ? ':' : ';'; break; + case VK_OEM_2: buffer[0] = shift ? '?' : '/'; break; + case VK_OEM_3: buffer[0] = shift ? '~' : '`'; break; + case VK_OEM_4: buffer[0] = shift ? '{' : '['; break; + case VK_OEM_5: buffer[0] = shift ? '|' : '\\'; break; + case VK_OEM_6: buffer[0] = shift ? '}' : ']'; break; + case VK_OEM_7: buffer[0] = shift ? '"' : '\''; break; + case VK_OEM_COMMA: buffer[0] = shift ? '<' : ','; break; + case VK_OEM_MINUS: buffer[0] = shift ? '_' : '-'; break; + case VK_OEM_PERIOD: buffer[0] = shift ? '>' : '.'; break; + case VK_OEM_PLUS: buffer[0] = shift ? '+' : '='; break; + case VK_RETURN: buffer[0] = '\r'; break; + case VK_SPACE: buffer[0] = ' '; break; + case VK_TAB: buffer[0] = '\t'; break; + case VK_MULTIPLY: buffer[0] = '*'; break; + case VK_ADD: buffer[0] = '+'; break; + case VK_SUBTRACT: buffer[0] = '-'; break; + case VK_DIVIDE: buffer[0] = '/'; break; + default: + if (virt >= '0' && virt <= '9') + buffer[0] = shift ? ")!@#$%^&*("[virt - '0'] : virt; + else if (virt >= 'A' && virt <= 'Z') + buffer[0] = shift || (state[VK_CAPITAL] & 0x01) ? virt : virt + 'a' - 'A'; + else if (virt >= VK_NUMPAD0 && virt <= VK_NUMPAD9 && numlock && !shift) + buffer[0] = '0' + virt - VK_NUMPAD0; + else if (virt == VK_DECIMAL && numlock && !shift) + buffer[0] = '.'; + else + buffer[0] = 0; + break; + } + } + else /* Control codes */ + { + switch (virt) + { + case VK_OEM_4: buffer[0] = 0x1b; break; + case VK_OEM_5: buffer[0] = 0x1c; break; + case VK_OEM_6: buffer[0] = 0x1d; break; + case VK_SUBTRACT: buffer[0] = 0x1e; break; + default: + if (virt >= 'A' && virt <= 'Z') buffer[0] = virt - 'A' + 1; + else buffer[0] = 0; + break; + } + } + buffer[1] = 0; + len = wcslen( buffer ); + + len = USER_Driver->pToUnicodeEx( virt, scan, state, str, size, flags, layout, buffer, len ); + + TRACE_(keyboard)( "ret %d, str %s.\n", len, len < 0 ? "" : debugstr_wn(str, len) ); + return len; } /**************************************************************************** diff --git a/dlls/user32/user_private.h b/dlls/user32/user_private.h index 5f8059a12a0..8f9841ec48b 100644 --- a/dlls/user32/user_private.h +++ b/dlls/user32/user_private.h @@ -71,7 +71,7 @@ typedef struct tagUSER_DRIVER { HKL (CDECL *pLoadKeyboardLayout)(LPCWSTR, UINT); UINT (CDECL *pMapVirtualKeyEx)(UINT, UINT, HKL); BOOL (CDECL *pRegisterHotKey)(HWND, UINT, UINT); - INT (CDECL *pToUnicodeEx)(UINT, UINT, const BYTE *, LPWSTR, int, UINT, HKL); + INT (CDECL *pToUnicodeEx)(UINT, UINT, const BYTE *, LPWSTR, int, UINT, HKL, const WCHAR *, INT); BOOL (CDECL *pUnloadKeyboardLayout)(HKL); void (CDECL *pUnregisterHotKey)(HWND, UINT, UINT); SHORT (CDECL *pVkKeyScanEx)(WCHAR, HKL); diff --git a/dlls/wineandroid.drv/keyboard.c b/dlls/wineandroid.drv/keyboard.c index 0a6ede0ec5f..04565c3fad8 100644 --- a/dlls/wineandroid.drv/keyboard.c +++ b/dlls/wineandroid.drv/keyboard.c @@ -750,100 +750,6 @@ jboolean keyboard_event( JNIEnv *env, jobject obj, jint win, jint action, jint k } -/*********************************************************************** - * ANDROID_ToUnicodeEx - */ -INT CDECL ANDROID_ToUnicodeEx( UINT virt, UINT scan, const BYTE *state, - LPWSTR buf, int size, UINT flags, HKL hkl ) -{ - WCHAR buffer[2]; - BOOL shift = state[VK_SHIFT] & 0x80; - BOOL ctrl = state[VK_CONTROL] & 0x80; - BOOL numlock = state[VK_NUMLOCK] & 0x01; - - buffer[0] = buffer[1] = 0; - - if (scan & 0x8000) return 0; /* key up */ - - /* FIXME: hardcoded layout */ - - if (!ctrl) - { - switch (virt) - { - case VK_BACK: buffer[0] = '\b'; break; - case VK_OEM_1: buffer[0] = shift ? ':' : ';'; break; - case VK_OEM_2: buffer[0] = shift ? '?' : '/'; break; - case VK_OEM_3: buffer[0] = shift ? '~' : '`'; break; - case VK_OEM_4: buffer[0] = shift ? '{' : '['; break; - case VK_OEM_5: buffer[0] = shift ? '|' : '\\'; break; - case VK_OEM_6: buffer[0] = shift ? '}' : ']'; break; - case VK_OEM_7: buffer[0] = shift ? '"' : '\''; break; - case VK_OEM_COMMA: buffer[0] = shift ? '<' : ','; break; - case VK_OEM_MINUS: buffer[0] = shift ? '_' : '-'; break; - case VK_OEM_PERIOD: buffer[0] = shift ? '>' : '.'; break; - case VK_OEM_PLUS: buffer[0] = shift ? '+' : '='; break; - case VK_RETURN: buffer[0] = '\r'; break; - case VK_SPACE: buffer[0] = ' '; break; - case VK_TAB: buffer[0] = '\t'; break; - case VK_MULTIPLY: buffer[0] = '*'; break; - case VK_ADD: buffer[0] = '+'; break; - case VK_SUBTRACT: buffer[0] = '-'; break; - case VK_DIVIDE: buffer[0] = '/'; break; - default: - if (virt >= '0' && virt <= '9') - { - buffer[0] = shift ? ")!@#$%^&*("[virt - '0'] : virt; - break; - } - if (virt >= 'A' && virt <= 'Z') - { - buffer[0] = shift || (state[VK_CAPITAL] & 0x01) ? virt : virt + 'a' - 'A'; - break; - } - if (virt >= VK_NUMPAD0 && virt <= VK_NUMPAD9 && numlock && !shift) - { - buffer[0] = '0' + virt - VK_NUMPAD0; - break; - } - if (virt == VK_DECIMAL && numlock && !shift) - { - buffer[0] = '.'; - break; - } - break; - } - } - else /* Control codes */ - { - if (virt >= 'A' && virt <= 'Z') - buffer[0] = virt - 'A' + 1; - else - { - switch (virt) - { - case VK_OEM_4: - buffer[0] = 0x1b; - break; - case VK_OEM_5: - buffer[0] = 0x1c; - break; - case VK_OEM_6: - buffer[0] = 0x1d; - break; - case VK_SUBTRACT: - buffer[0] = 0x1e; - break; - } - } - } - - lstrcpynW( buf, buffer, size ); - TRACE( "returning %d / %s\n", strlenW( buffer ), debugstr_wn(buf, strlenW( buffer ))); - return strlenW( buffer ); -} - - /*********************************************************************** * ANDROID_GetKeyNameText */ diff --git a/dlls/wineandroid.drv/wineandroid.drv.spec b/dlls/wineandroid.drv/wineandroid.drv.spec index 00de23de27e..6f27c1586b3 100644 --- a/dlls/wineandroid.drv/wineandroid.drv.spec +++ b/dlls/wineandroid.drv/wineandroid.drv.spec @@ -7,7 +7,6 @@ @ cdecl GetKeyNameText(long ptr long) ANDROID_GetKeyNameText @ cdecl GetKeyboardLayout(long) ANDROID_GetKeyboardLayout @ cdecl MapVirtualKeyEx(long long long) ANDROID_MapVirtualKeyEx -@ cdecl ToUnicodeEx(long long ptr ptr long long long) ANDROID_ToUnicodeEx @ cdecl VkKeyScanEx(long long) ANDROID_VkKeyScanEx @ cdecl SetCursor(long) ANDROID_SetCursor @ cdecl ChangeDisplaySettingsEx(ptr ptr long long long) ANDROID_ChangeDisplaySettingsEx diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index 1ea15f59341..d9c819431f2 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -1553,7 +1553,8 @@ BOOL CDECL macdrv_RegisterHotKey(HWND hwnd, UINT mod_flags, UINT vkey) * */ INT CDECL macdrv_ToUnicodeEx(UINT virtKey, UINT scanCode, const BYTE *lpKeyState, - LPWSTR bufW, int bufW_size, UINT flags, HKL hkl) + LPWSTR bufW, int bufW_size, UINT flags, HKL hkl, + const WCHAR *default_buf, INT default_len) { struct macdrv_thread_data *thread_data = macdrv_init_thread_data(); INT ret = 0; diff --git a/dlls/winemac.drv/winemac.drv.spec b/dlls/winemac.drv/winemac.drv.spec index 39cf33370b4..97c94b6ee88 100644 --- a/dlls/winemac.drv/winemac.drv.spec +++ b/dlls/winemac.drv/winemac.drv.spec @@ -34,7 +34,7 @@ @ cdecl SysCommand(long long long) macdrv_SysCommand @ cdecl SystemParametersInfo(long long ptr long) macdrv_SystemParametersInfo @ cdecl ThreadDetach() macdrv_ThreadDetach -@ cdecl ToUnicodeEx(long long ptr ptr long long long) macdrv_ToUnicodeEx +@ cdecl ToUnicodeEx(long long ptr ptr long long long ptr long) macdrv_ToUnicodeEx @ cdecl UnregisterHotKey(long long long) macdrv_UnregisterHotKey @ cdecl UpdateClipboard() macdrv_UpdateClipboard @ cdecl UpdateLayeredWindow(long ptr ptr) macdrv_UpdateLayeredWindow diff --git a/dlls/winex11.drv/keyboard.c b/dlls/winex11.drv/keyboard.c index 01620c5e4a4..c45d12c7517 100644 --- a/dlls/winex11.drv/keyboard.c +++ b/dlls/winex11.drv/keyboard.c @@ -2432,7 +2432,8 @@ static char KEYBOARD_MapDeadKeysym(KeySym keysym) * */ INT CDECL X11DRV_ToUnicodeEx(UINT virtKey, UINT scanCode, const BYTE *lpKeyState, - LPWSTR bufW, int bufW_size, UINT flags, HKL hkl) + LPWSTR bufW, int bufW_size, UINT flags, HKL hkl, + const WCHAR *default_buf, INT default_len) { Display *display = thread_init_display(); XKeyEvent e; diff --git a/dlls/winex11.drv/winex11.drv.spec b/dlls/winex11.drv/winex11.drv.spec index c0e24d8fe82..b73ae87b929 100644 --- a/dlls/winex11.drv/winex11.drv.spec +++ b/dlls/winex11.drv/winex11.drv.spec @@ -11,7 +11,7 @@ @ cdecl GetKeyboardLayoutName(ptr) X11DRV_GetKeyboardLayoutName @ cdecl LoadKeyboardLayout(wstr long) X11DRV_LoadKeyboardLayout @ cdecl MapVirtualKeyEx(long long long) X11DRV_MapVirtualKeyEx -@ cdecl ToUnicodeEx(long long ptr ptr long long long) X11DRV_ToUnicodeEx +@ cdecl ToUnicodeEx(long long ptr ptr long long long ptr long) X11DRV_ToUnicodeEx @ cdecl UnloadKeyboardLayout(long) X11DRV_UnloadKeyboardLayout @ cdecl VkKeyScanEx(long long) X11DRV_VkKeyScanEx @ cdecl DestroyCursorIcon(long) X11DRV_DestroyCursorIcon -- 2.31.0
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/input.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/user32/input.c b/dlls/user32/input.c index 51a08d0786b..9a58537f4b3 100644 --- a/dlls/user32/input.c +++ b/dlls/user32/input.c @@ -880,7 +880,7 @@ INT WINAPI ToUnicode(UINT virtKey, UINT scanCode, const BYTE *lpKeyState, INT WINAPI ToUnicodeEx( UINT virt, UINT scan, const BYTE *state, WCHAR *str, int size, UINT flags, HKL layout ) { - BOOL shift, ctrl, numlock; + BOOL shift, ctrl, alt, numlock; WCHAR buffer[2]; INT len; @@ -889,6 +889,7 @@ INT WINAPI ToUnicodeEx( UINT virt, UINT scan, const BYTE *state, if (!state) return 0; + alt = state[VK_MENU] & 0x80; shift = state[VK_SHIFT] & 0x80; ctrl = state[VK_CONTROL] & 0x80; numlock = state[VK_NUMLOCK] & 0x01; @@ -933,7 +934,7 @@ INT WINAPI ToUnicodeEx( UINT virt, UINT scan, const BYTE *state, break; } } - else /* Control codes */ + else if (!alt) /* Control codes */ { switch (virt) { @@ -947,6 +948,7 @@ INT WINAPI ToUnicodeEx( UINT virt, UINT scan, const BYTE *state, break; } } + else buffer[0] = 0; buffer[1] = 0; len = wcslen( buffer ); -- 2.31.0
As shown by user32 input and msg tests. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/input.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dlls/user32/input.c b/dlls/user32/input.c index 9a58537f4b3..a49c6451f8b 100644 --- a/dlls/user32/input.c +++ b/dlls/user32/input.c @@ -897,6 +897,7 @@ INT WINAPI ToUnicodeEx( UINT virt, UINT scan, const BYTE *state, /* FIXME: English keyboard layout specific */ if (scan & 0x8000) buffer[0] = 0; /* key up */ + else if (virt == VK_ESCAPE) buffer[0] = VK_ESCAPE; else if (!ctrl) { switch (virt) @@ -942,6 +943,8 @@ INT WINAPI ToUnicodeEx( UINT virt, UINT scan, const BYTE *state, case VK_OEM_5: buffer[0] = 0x1c; break; case VK_OEM_6: buffer[0] = 0x1d; break; case VK_SUBTRACT: buffer[0] = 0x1e; break; + case VK_RETURN: buffer[0] = shift ? 0 : '\n'; break; + case VK_SPACE: buffer[0] = ' '; break; default: if (virt >= 'A' && virt <= 'Z') buffer[0] = virt - 'A' + 1; else buffer[0] = 0; -- 2.31.0
Rémi Bernon <rbernon(a)codeweavers.com> writes:
As a default implementation for en_US keyboard layout, calling the driver implementation next so that it can override the result.
Is there really a need to build the default result first and pass it to the driver? Otherwise I'd suggest to call the driver first, and use a special return value like -2 to indicate fallback. -- Alexandre Julliard julliard(a)winehq.org
On 4/27/21 3:41 PM, Alexandre Julliard wrote:
Rémi Bernon <rbernon(a)codeweavers.com> writes:
As a default implementation for en_US keyboard layout, calling the driver implementation next so that it can override the result.
Is there really a need to build the default result first and pass it to the driver? Otherwise I'd suggest to call the driver first, and use a special return value like -2 to indicate fallback.
I wanted to avoid special values like this, because they could have a meaning. Maybe not for ToUnicodeEx, but other driver calls for which I'd like to add a default implementation too. I thought about adding NTSTATUS as a return code instead, and treating for instance STATUS_NOT_IMPLEMENTED as an indication that the driver doesn't implement the call and that we should use the default impl. But then it would require to move the return value as an output parameter instead, changing more driver code and especially changing their return path, which is sometimes spread out. Also some driver call actually do SetLastError(STATUS_NOT_IMPLEMENTED); return 0; and I found it confusing to have return STATUS_SUCCESS here. In the end, having the default implementation run all the time didn't seem too bad, especially if as it's not doing much, and if we consider it could then be extended to cover more cases and reduce the amount of driver code. If we consider keyboard layouts I would think that it could be decided based on the HKL, with some special value(s) meaning the layout is handled by the driver while "standard" layout would be implemented in user32. Anyway, If you like the NTSTATUS style more, I can do it like this. -- Rémi Bernon <rbernon(a)codeweavers.com>
Rémi Bernon <rbernon(a)codeweavers.com> writes:
In the end, having the default implementation run all the time didn't seem too bad, especially if as it's not doing much, and if we consider it could then be extended to cover more cases and reduce the amount of driver code.
If we consider keyboard layouts I would think that it could be decided based on the HKL, with some special value(s) meaning the layout is handled by the driver while "standard" layout would be implemented in user32.
Anyway, If you like the NTSTATUS style more, I can do it like this.
I still find it better to call the driver first, and do the extra work only if needed. If we do it right, the default implementation is only going to become more complex, and that would be all the more reason to ask the driver first. I'm not sure that I see a need to use NTSTATUS here, plus it wouldn't quite fit with the usual user32 calling conventions. At some point we may want to turn this into a syscall interface and use NTSTATUS everywhere, but that would be a complete redesign of the driver interface. -- Alexandre Julliard julliard(a)winehq.org
On 4/27/21 4:26 PM, Alexandre Julliard wrote:
Rémi Bernon <rbernon(a)codeweavers.com> writes:
In the end, having the default implementation run all the time didn't seem too bad, especially if as it's not doing much, and if we consider it could then be extended to cover more cases and reduce the amount of driver code.
If we consider keyboard layouts I would think that it could be decided based on the HKL, with some special value(s) meaning the layout is handled by the driver while "standard" layout would be implemented in user32.
Anyway, If you like the NTSTATUS style more, I can do it like this.
I still find it better to call the driver first, and do the extra work only if needed. If we do it right, the default implementation is only going to become more complex, and that would be all the more reason to ask the driver first.
I'm not sure that I see a need to use NTSTATUS here, plus it wouldn't quite fit with the usual user32 calling conventions. At some point we may want to turn this into a syscall interface and use NTSTATUS everywhere, but that would be a complete redesign of the driver interface.
Yes, and I said NTSTATUS with that in mind, but it could very well be BOOL for now and return TRUE/FALSE if it's implemented or not, the inconvenience of changing the driver return paths is pretty much the same though. Ok it's maybe not that bad... -- Rémi Bernon <rbernon(a)codeweavers.com>
Rémi Bernon <rbernon(a)codeweavers.com> writes:
On 4/27/21 4:26 PM, Alexandre Julliard wrote:
Rémi Bernon <rbernon(a)codeweavers.com> writes:
In the end, having the default implementation run all the time didn't seem too bad, especially if as it's not doing much, and if we consider it could then be extended to cover more cases and reduce the amount of driver code.
If we consider keyboard layouts I would think that it could be decided based on the HKL, with some special value(s) meaning the layout is handled by the driver while "standard" layout would be implemented in user32.
Anyway, If you like the NTSTATUS style more, I can do it like this. I still find it better to call the driver first, and do the extra work only if needed. If we do it right, the default implementation is only going to become more complex, and that would be all the more reason to ask the driver first. I'm not sure that I see a need to use NTSTATUS here, plus it wouldn't quite fit with the usual user32 calling conventions. At some point we may want to turn this into a syscall interface and use NTSTATUS everywhere, but that would be a complete redesign of the driver interface.
Yes, and I said NTSTATUS with that in mind, but it could very well be BOOL for now and return TRUE/FALSE if it's implemented or not, the inconvenience of changing the driver return paths is pretty much the same though.
Ok it's maybe not that bad...
I find return -2 in the null driver a lot easier, but maybe that's just me ;-) -- Alexandre Julliard julliard(a)winehq.org
On 4/27/21 4:55 PM, Alexandre Julliard wrote:
Rémi Bernon <rbernon(a)codeweavers.com> writes:
On 4/27/21 4:26 PM, Alexandre Julliard wrote:
Rémi Bernon <rbernon(a)codeweavers.com> writes:
In the end, having the default implementation run all the time didn't seem too bad, especially if as it's not doing much, and if we consider it could then be extended to cover more cases and reduce the amount of driver code.
If we consider keyboard layouts I would think that it could be decided based on the HKL, with some special value(s) meaning the layout is handled by the driver while "standard" layout would be implemented in user32.
Anyway, If you like the NTSTATUS style more, I can do it like this. I still find it better to call the driver first, and do the extra work only if needed. If we do it right, the default implementation is only going to become more complex, and that would be all the more reason to ask the driver first. I'm not sure that I see a need to use NTSTATUS here, plus it wouldn't quite fit with the usual user32 calling conventions. At some point we may want to turn this into a syscall interface and use NTSTATUS everywhere, but that would be a complete redesign of the driver interface.
Yes, and I said NTSTATUS with that in mind, but it could very well be BOOL for now and return TRUE/FALSE if it's implemented or not, the inconvenience of changing the driver return paths is pretty much the same though.
Ok it's maybe not that bad...
I find return -2 in the null driver a lot easier, but maybe that's just me ;-)
In this case sure, but then there's things like MapVirtualKeyEx or the other calls. But maybe I'm trying too hard to have a consistent pattern for something that is subject to change anyway. -- Rémi Bernon <rbernon(a)codeweavers.com>
Rémi Bernon <rbernon(a)codeweavers.com> writes:
On 4/27/21 4:55 PM, Alexandre Julliard wrote:
Rémi Bernon <rbernon(a)codeweavers.com> writes:
On 4/27/21 4:26 PM, Alexandre Julliard wrote:
Rémi Bernon <rbernon(a)codeweavers.com> writes:
In the end, having the default implementation run all the time didn't seem too bad, especially if as it's not doing much, and if we consider it could then be extended to cover more cases and reduce the amount of driver code.
If we consider keyboard layouts I would think that it could be decided based on the HKL, with some special value(s) meaning the layout is handled by the driver while "standard" layout would be implemented in user32.
Anyway, If you like the NTSTATUS style more, I can do it like this. I still find it better to call the driver first, and do the extra work only if needed. If we do it right, the default implementation is only going to become more complex, and that would be all the more reason to ask the driver first. I'm not sure that I see a need to use NTSTATUS here, plus it wouldn't quite fit with the usual user32 calling conventions. At some point we may want to turn this into a syscall interface and use NTSTATUS everywhere, but that would be a complete redesign of the driver interface.
Yes, and I said NTSTATUS with that in mind, but it could very well be BOOL for now and return TRUE/FALSE if it's implemented or not, the inconvenience of changing the driver return paths is pretty much the same though.
Ok it's maybe not that bad... I find return -2 in the null driver a lot easier, but maybe that's just me ;-)
In this case sure, but then there's things like MapVirtualKeyEx or the other calls. But maybe I'm trying too hard to have a consistent pattern for something that is subject to change anyway.
Yes, the driver interface is pretty much ad-hoc, there's no need to try hard to be consistent. In general, the idea is for the driver entry points to look as much as possible like their user32 counterparts, so in that sense a magic return value is better than extra parameters. -- Alexandre Julliard julliard(a)winehq.org
participants (2)
-
Alexandre Julliard -
Rémi Bernon