[PATCH] user32: Return correct showCmd from GetWindowPlacement() for other process window.
Fixes black screen in SWTOR after window minimize / maximize or switching window in full screen mode. Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> --- dlls/user32/tests/win.c | 126 ++++++++++++++++++++++++++++++++++++++-- dlls/user32/winpos.c | 35 +++++++++-- 2 files changed, 153 insertions(+), 8 deletions(-) diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index 639eef4c42..5a35138fdc 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -11733,6 +11733,115 @@ static void test_arrange_iconic_windows(void) ok(ret, "failed to restore minimized metrics, error %u\n", GetLastError()); } +static void other_process_proc(HWND hwnd) +{ + HANDLE window_ready_event, test_done_event; + WINDOWPLACEMENT wp; + DWORD ret; + + window_ready_event = OpenEventA(EVENT_ALL_ACCESS, FALSE, "test_opw_window"); + ok(!!window_ready_event, "OpenEvent failed.\n"); + test_done_event = OpenEventA(EVENT_ALL_ACCESS, FALSE, "test_opw_test"); + ok(!!test_done_event, "OpenEvent failed.\n"); + + /* SW_SHOW */ + ret = WaitForSingleObject(window_ready_event, 5000); + ok(ret == WAIT_OBJECT_0, "Unexpected ret %x.\n", ret); + ret = GetWindowPlacement(hwnd, &wp); + ok(ret, "Unexpected ret %#x.\n", ret); + ok(wp.showCmd == SW_SHOWNORMAL, "Unexpected showCmd %#x.\n", wp.showCmd); + ok(!wp.flags, "Unexpected flags %#x.\n", wp.flags); + SetEvent(test_done_event); + + /* SW_SHOWMAXIMIZED */ + ret = WaitForSingleObject(window_ready_event, 5000); + ok(ret == WAIT_OBJECT_0, "Unexpected ret %x.\n", ret); + ret = GetWindowPlacement(hwnd, &wp); + ok(ret, "Unexpected ret %#x.\n", ret); + ok(wp.showCmd == SW_SHOWMAXIMIZED, "Unexpected showCmd %#x.\n", wp.showCmd); + todo_wine ok(wp.flags == WPF_RESTORETOMAXIMIZED, "Unexpected flags %#x.\n", wp.flags); + SetEvent(test_done_event); + + /* SW_SHOWMINIMIZED */ + ret = WaitForSingleObject(window_ready_event, 5000); + ok(ret == WAIT_OBJECT_0, "Unexpected ret %x.\n", ret); + ret = GetWindowPlacement(hwnd, &wp); + ok(ret, "Unexpected ret %#x.\n", ret); + ok(wp.showCmd == SW_SHOWMINIMIZED, "Unexpected showCmd %#x.\n", wp.showCmd); + todo_wine ok(wp.flags == WPF_RESTORETOMAXIMIZED, "Unexpected flags %#x.\n", wp.flags); + SetEvent(test_done_event); + + /* SW_RESTORE */ + ret = WaitForSingleObject(window_ready_event, 5000); + ok(ret == WAIT_OBJECT_0, "Unexpected ret %x.\n", ret); + ret = GetWindowPlacement(hwnd, &wp); + ok(ret, "Unexpected ret %#x.\n", ret); + ok(wp.showCmd == SW_SHOWMAXIMIZED, "Unexpected showCmd %#x.\n", wp.showCmd); + todo_wine ok(wp.flags == WPF_RESTORETOMAXIMIZED, "Unexpected flags %#x.\n", wp.flags); + SetEvent(test_done_event); + + CloseHandle(window_ready_event); + CloseHandle(test_done_event); +} + +static void test_other_process_window(const char *argv0) +{ + HANDLE window_ready_event, test_done_event; + PROCESS_INFORMATION info; + STARTUPINFOA startup; + char cmd[MAX_PATH]; + HWND hwnd; + BOOL ret; + + hwnd = CreateWindowExA(0, "static", NULL, WS_POPUP, + 100, 100, 100, 100, 0, 0, NULL, NULL); + ok(!!hwnd, "CreateWindowEx failed.\n"); + + window_ready_event = CreateEventA(NULL, FALSE, FALSE, "test_opw_window"); + ok(!!window_ready_event, "CreateEvent failed.\n"); + test_done_event = CreateEventA(NULL, FALSE, FALSE, "test_opw_test"); + ok(!!test_done_event, "CreateEvent failed.\n"); + + + sprintf(cmd, "%s win test_other_process_window %p", argv0, hwnd); + memset(&startup, 0, sizeof(startup)); + startup.cb = sizeof(startup); + + ok(CreateProcessA(NULL, cmd, NULL, NULL, FALSE, 0, NULL, NULL, + &startup, &info), "CreateProcess failed.\n"); + + ret = ShowWindow(hwnd, SW_SHOW); + ok(!ret, "Unexpected ret %#x.\n", ret); + SetEvent(window_ready_event); + ret = WaitForSingleObject(test_done_event, 5000); + ok(ret == WAIT_OBJECT_0, "Unexpected ret %x.\n", ret); + + ret = ShowWindow(hwnd, SW_SHOWMAXIMIZED); + ok(ret, "Unexpected ret %#x.\n", ret); + SetEvent(window_ready_event); + ret = WaitForSingleObject(test_done_event, 5000); + ok(ret == WAIT_OBJECT_0, "Unexpected ret %x.\n", ret); + + ret = ShowWindow(hwnd, SW_SHOWMINIMIZED); + ok(ret, "Unexpected ret %#x.\n", ret); + SetEvent(window_ready_event); + ret = WaitForSingleObject(test_done_event, 5000); + ok(ret == WAIT_OBJECT_0, "Unexpected ret %x.\n", ret); + + ret = ShowWindow(hwnd, SW_RESTORE); + ok(ret, "Unexpected ret %#x.\n", ret); + SetEvent(window_ready_event); + ret = WaitForSingleObject(test_done_event, 5000); + ok(ret == WAIT_OBJECT_0, "Unexpected ret %x.\n", ret); + + winetest_wait_child_process(info.hProcess); + CloseHandle(window_ready_event); + CloseHandle(test_done_event); + CloseHandle(info.hProcess); + CloseHandle(info.hThread); + DestroyWindow(hwnd); +} + START_TEST(win) { char **argv; @@ -11762,16 +11871,24 @@ START_TEST(win) pAdjustWindowRectExForDpi = (void *)GetProcAddress( user32, "AdjustWindowRectExForDpi" ); pSystemParametersInfoForDpi = (void *)GetProcAddress( user32, "SystemParametersInfoForDpi" ); - if (argc==4 && !strcmp(argv[2], "create_children")) + if (argc == 4) { HWND hwnd; sscanf(argv[3], "%p", &hwnd); - window_from_point_proc(hwnd); - return; + if (!strcmp(argv[2], "create_children")) + { + window_from_point_proc(hwnd); + return; + } + else if (!strcmp(argv[2], "test_other_process_window")) + { + other_process_proc(hwnd); + return; + } } - if (argc==3 && !strcmp(argv[2], "winproc_limit")) + if (argc == 3 && !strcmp(argv[2], "winproc_limit")) { test_winproc_limit(); return; @@ -11894,6 +12011,7 @@ START_TEST(win) test_IsWindowEnabled(); test_window_placement(); test_arrange_iconic_windows(); + test_other_process_window(argv[0]); /* add the tests above this line */ if (hhook) UnhookWindowsHookEx(hhook); diff --git a/dlls/user32/winpos.c b/dlls/user32/winpos.c index 5837c179e4..6b8927f020 100644 --- a/dlls/user32/winpos.c +++ b/dlls/user32/winpos.c @@ -1280,11 +1280,38 @@ BOOL WINAPI GetWindowPlacement( HWND hwnd, WINDOWPLACEMENT *wndpl ) } if (pWnd == WND_OTHER_PROCESS) { - if (!IsWindow( hwnd )) return FALSE; - FIXME( "not supported on other process window %p\n", hwnd ); - /* provide some dummy information */ + DWORD style, is_window; + + FIXME( "not fully supported on other process window %p.\n", hwnd ); + + SERVER_START_REQ( set_window_info ) + { + req->handle = wine_server_user_handle( hwnd ); + req->flags = 0; /* don't set anything, just retrieve */ + req->extra_offset = -1; + req->extra_size = 0; + if (wine_server_call_err( req )) + { + style = 0; + is_window = FALSE; + } + else + { + style = reply->old_style; + is_window = TRUE; + } + } + SERVER_END_REQ; + + if (!is_window) + return FALSE; + wndpl->length = sizeof(*wndpl); - wndpl->showCmd = SW_SHOWNORMAL; + if( style & WS_MINIMIZE ) + wndpl->showCmd = SW_SHOWMINIMIZED; + else + wndpl->showCmd = ( style & WS_MAXIMIZE ) ? SW_SHOWMAXIMIZED : SW_SHOWNORMAL; + /* provide some dummy information */ wndpl->flags = 0; wndpl->ptMinPosition.x = -1; wndpl->ptMinPosition.y = -1; -- 2.21.0
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=55260 Your paranoid android. === wvistau64_he (32 bit report) === user32: win.c:9447: Test failed: didn't get start_event win.c:9451: Test failed: WindowFromPoint returned 000201FE, expected 00000000 win.c:9459: Test failed: WindowFromPoint returned 000201FE, expected 00000000 win.c:9360: Test failed: WindowFromPoint returned 0002019E, expected 000201FE win.c:9367: Test failed: WindowFromPoint returned 0002019E, expected 000401AE win.c:9373: Test failed: CreateWindowEx failed win.c:9388: Test failed: transparent window didn't get WM_NCHITTEST message win.c:9389: Test failed: button under static window didn't get WM_LBUTTONUP === w2008s64 (32 bit report) === Report errors: user32:win is missing some failure messages === w1064v1809 (32 bit report) === user32: win.c:3141: Test failed: SetActiveWindow returned 00000000 instead of 000803BE win.c:3199: Test failed: expected 000803BE, got 00000000 win.c:3811: Test failed: hwnd 00040038 message 0060 win.c:3951: Test failed: hwnd 004102F2/004102F2 message 00a1 win.c:3255: Test failed: SetActiveWindow(0) returned 00000000 instead of 000803BE === wvistau64 (task log) === Task errors: The task timed out === w2008s64 (64 bit report) === user32: win: Timeout === w7pro64 (64 bit report) === user32: win: Timeout === w864 (64 bit report) === user32: win: Timeout === w1064v1507 (64 bit report) === user32: win: Timeout === w1064v1809 (64 bit report) === user32: win: Timeout === debian10 (32 bit report) === user32: msg.c:5145: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35223 bytes) === debian10 (32 bit Chinese:China report) === user32: msg.c:5145: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35221 bytes) === debian10 (32 bit WoW report) === user32: msg.c:5145: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35221 bytes) === debian10 (64 bit WoW report) === user32: msg.c:5145: Test succeeded inside todo block: ShowWindow(SW_SHOWMINIMIZED):overlapped: marked "todo_wine" but succeeds Report errors: user32:msg prints too much data (35221 bytes)
Paul Gofman <gofmanp(a)gmail.com> writes:
@@ -1280,11 +1280,38 @@ BOOL WINAPI GetWindowPlacement( HWND hwnd, WINDOWPLACEMENT *wndpl ) } if (pWnd == WND_OTHER_PROCESS) { - if (!IsWindow( hwnd )) return FALSE; - FIXME( "not supported on other process window %p\n", hwnd ); - /* provide some dummy information */ + DWORD style, is_window; + + FIXME( "not fully supported on other process window %p.\n", hwnd ); + + SERVER_START_REQ( set_window_info ) + { + req->handle = wine_server_user_handle( hwnd ); + req->flags = 0; /* don't set anything, just retrieve */ + req->extra_offset = -1; + req->extra_size = 0; + if (wine_server_call_err( req )) + { + style = 0; + is_window = FALSE; + } + else + { + style = reply->old_style; + is_window = TRUE; + } + } + SERVER_END_REQ; + + if (!is_window) + return FALSE;
You can use GetWindowLong() for this, there's no need to add a server call here. -- Alexandre Julliard julliard(a)winehq.org
On 8/13/19 12:12, Alexandre Julliard wrote:
Paul Gofman <gofmanp(a)gmail.com> writes:
@@ -1280,11 +1280,38 @@ BOOL WINAPI GetWindowPlacement( HWND hwnd, WINDOWPLACEMENT *wndpl ) } if (pWnd == WND_OTHER_PROCESS) { - if (!IsWindow( hwnd )) return FALSE; - FIXME( "not supported on other process window %p\n", hwnd ); - /* provide some dummy information */ + DWORD style, is_window; + + FIXME( "not fully supported on other process window %p.\n", hwnd ); + + SERVER_START_REQ( set_window_info ) + { + req->handle = wine_server_user_handle( hwnd ); + req->flags = 0; /* don't set anything, just retrieve */ + req->extra_offset = -1; + req->extra_size = 0; + if (wine_server_call_err( req )) + { + style = 0; + is_window = FALSE; + } + else + { + style = reply->old_style; + is_window = TRUE; + } + } + SERVER_END_REQ; + + if (!is_window) + return FALSE; You can use GetWindowLong() for this, there's no need to add a server call here.
This will add an extra server call for other process window from GetWindowLong(). The two were already there: one for IsWindow(), another for getting rectangle below. I thought it might be better not to introduce the third one. Should I use GetWindowLong() regardless?
Paul Gofman <gofmanp(a)gmail.com> writes:
On 8/13/19 12:12, Alexandre Julliard wrote:
Paul Gofman <gofmanp(a)gmail.com> writes:
@@ -1280,11 +1280,38 @@ BOOL WINAPI GetWindowPlacement( HWND hwnd, WINDOWPLACEMENT *wndpl ) } if (pWnd == WND_OTHER_PROCESS) { - if (!IsWindow( hwnd )) return FALSE; - FIXME( "not supported on other process window %p\n", hwnd ); - /* provide some dummy information */ + DWORD style, is_window; + + FIXME( "not fully supported on other process window %p.\n", hwnd ); + + SERVER_START_REQ( set_window_info ) + { + req->handle = wine_server_user_handle( hwnd ); + req->flags = 0; /* don't set anything, just retrieve */ + req->extra_offset = -1; + req->extra_size = 0; + if (wine_server_call_err( req )) + { + style = 0; + is_window = FALSE; + } + else + { + style = reply->old_style; + is_window = TRUE; + } + } + SERVER_END_REQ; + + if (!is_window) + return FALSE; You can use GetWindowLong() for this, there's no need to add a server call here.
This will add an extra server call for other process window from GetWindowLong(). The two were already there: one for IsWindow(), another for getting rectangle below. I thought it might be better not to introduce the third one. Should I use GetWindowLong() regardless?
Yes. If you want to improve it, you can avoid IsWindow() and check for GetWindowRect() failure or something like that. But either way I don't expect GetWindowPlacement() to be timing-critical. -- Alexandre Julliard julliard(a)winehq.org
participants (3)
-
Alexandre Julliard -
Marvin -
Paul Gofman