[PATCH v2 0/2] user32: Change GetKeyState() / GetKeyboardState() to fix some programs.

[Markus Engel]

Hi,
the documentation for these two functions isn't clear about the contents
of the bits of the return value, except for the highest and lowest bit.
Some programs assume that the others are always set to zero, which seems
to be the actual behavior in Windows.

Specifically, this leads to stuck keys in "Age of Empires 2 HD" and "Age
of Empires 2: Definitive Edition" when tabbing out as reported in
https://bugs.winehq.org/show_bug.cgi?id=30814 .

The following patch makes sure that all undefined bits in the first byte
are set to zero.

Markus

v2: As the test for the first version revealed, Windows XP indeed
behaves differently from all newer versions. The patch now only clears
bits 1-6 in the first byte and uses sign extension to create the SHORT
return value again.

Signed-off-by: Markus Engel

Markus Engel (2):
  user32: Force undefined bits in GetKeyState() and GetKeyboardState()
    to zero.
  user32/tests: Add more tests for GetKeyState().

dlls/user32/input.c       | 6 +++++-
 dlls/user32/tests/input.c | 6 ++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

[PATCH v2 1/2] user32: Force undefined bits in GetKeyState() and GetKeyboardState() to zero.

[Markus Engel]

Only the highest and lowest bits in the return values of these functions
have a meaning, the others are undefined. While the other bits are
always cleared in Windows, wine stores information there. Some programs
expect these undefined bits to be zero, though, so make sure they are
not set.

This is a slightly adjusted version of the patch by Raditz12 that is
attached to the bug report.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=30814
Signed-off-by: Markus Engel 
---
 dlls/user32/input.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/dlls/user32/input.c b/dlls/user32/input.c
index b7cdbd84ef..a5524955e8 100644
--- a/dlls/user32/input.c
+++ b/dlls/user32/input.c
@@ -555,7 +555,7 @@ SHORT WINAPI DECLSPEC_HOTPATCH GetKeyState(INT vkey)
     {
         req->tid = GetCurrentThreadId();
         req->key = vkey;
-        if (!wine_server_call( req )) retval = (signed char)reply->state;
+        if (!wine_server_call( req )) retval = (signed char)(reply->state & ~0x7e);
     }
     SERVER_END_REQ;
     TRACE("key (0x%x) -> %x\n", vkey, retval);
@@ -569,6 +569,7 @@ SHORT WINAPI DECLSPEC_HOTPATCH GetKeyState(INT vkey)
 BOOL WINAPI DECLSPEC_HOTPATCH GetKeyboardState( LPBYTE state )
 {
     BOOL ret;
+    UINT i;

TRACE("(%p)\n", state);

@@ -579,6 +580,9 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetKeyboardState( LPBYTE state )
         req->key = -1;
         wine_server_set_reply( req, state, 256 );
         ret = !wine_server_call_err( req );
+
+        for (i = 0; i < 256; i++)
+            state[i] &= ~0x7e;
     }
     SERVER_END_REQ;
     return ret;

[PATCH v2 1/2] user32: Force undefined bits in GetKeyState() and GetKeyboardState() to zero.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=71944

Your paranoid android.

=== debiant (32 bit WoW report) ===

user32:
win.c:10149: Test failed: Expected foreground window 000D0120, got 00E300D4

=== debiant (64 bit WoW report) ===

user32:
win.c:10813: Test failed: 01280094: expected prev 0022011E, got 00000000
win.c:10827: Test failed: hwnd should NOT be topmost
win.c:10829: Test failed: 01280094: expected NOT topmost
win.c:10773: Test failed: 1: hwnd 01280094 is still topmost

[PATCH v2 2/2] user32/tests: Add more tests for GetKeyState().

[Markus Engel]

Signed-off-by: Markus Engel 
---
 dlls/user32/tests/input.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/dlls/user32/tests/input.c b/dlls/user32/tests/input.c
index 43fdd34191..92b18becd2 100644
--- a/dlls/user32/tests/input.c
+++ b/dlls/user32/tests/input.c
@@ -2759,6 +2759,9 @@ static DWORD WINAPI get_key_state_thread(void *arg)
     ok((result & 0x8000) || broken(!(result & 0x8000)), /* > Win 2003 */
        "expected that highest bit is set, got %x\n", result);

+    ok((SHORT)(result & 0x007e) == 0,
+        "expected that undefined bits are unset, got %x\n", result);
+
     ReleaseSemaphore(semaphores[0], 1, NULL);
     result = WaitForSingleObject(semaphores[1], 1000);
     ok(result == WAIT_OBJECT_0, "WaitForSingleObject returned %u\n", result);
@@ -2766,6 +2769,9 @@ static DWORD WINAPI get_key_state_thread(void *arg)
     result = GetKeyState('X');
     ok(!(result & 0x8000), "expected that highest bit is unset, got %x\n", result);

+    ok((SHORT)(result & 0x007e) == 0,
+        "expected that undefined bits are unset, got %x\n", result);
+
     return 0;
 }

[PATCH v2 2/2] user32/tests: Add more tests for GetKeyState().

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=71945

Your paranoid android.

=== w1064v1809 (32 bit report) ===

user32:
input.c:1291: Test failed: Wrong set pos: (100,100)
input.c:1311: Test failed: GetCursorPos: (100,100)

=== w1064v1809_ja (32 bit report) ===

user32:
input.c:1291: Test failed: Wrong set pos: (99,100)
input.c:1311: Test failed: GetCursorPos: (99,100)