[PATCH 0/2] MR9573: win32u: Initialize surface with white colour on creation.
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/user32/tests/win.c | 91 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index 354d6ed5ca3..6baf83e39e5 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -31,6 +31,7 @@ #include "wingdi.h" #include "winuser.h" #include "winreg.h" +#include "dwmapi.h" #include "wine/test.h" @@ -63,6 +64,9 @@ static BOOL (WINAPI *pSystemParametersInfoForDpi)(UINT,UINT,void*,UINT,UINT); static HICON (WINAPI *pInternalGetWindowIcon)(HWND window, UINT type); static BOOL (WINAPI *pSetProcessLaunchForegroundPolicy)(DWORD,DWORD); +static HRESULT (WINAPI *pDwmSetWindowAttribute)(HWND,DWORD,const void *,DWORD); +static HRESULT (WINAPI *pDwmFlush)(void); + static BOOL test_lbuttondown_flag; static DWORD num_gettext_msgs; static DWORD num_settext_msgs; @@ -14255,12 +14259,95 @@ static void test_GW_ENABLEDPOPUP(void) DestroyWindow(parent2); } +static LRESULT WINAPI test_initial_surface_colour_window_proc( HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam ) +{ + COLORREF c; + HDC hdc; + RECT r; + + switch(msg) + { + case WM_NCCREATE: + if (pDwmSetWindowAttribute) + { + BOOL disable = TRUE; + + /* with transition effect enabled the window bits, in the absence of painting those, will be gradually + * transitioned from pixels behind the window to white colour, featuring different colours in the test + * depending on sleep amount in WM_WINDOWPOSCHANGING. */ + pDwmSetWindowAttribute( hwnd, DWMWA_TRANSITIONS_FORCEDISABLED, &disable, sizeof(disable) ); + } + hdc = GetDC( hwnd ); + ok( !!hdc, "got %p.\n", hdc ); + GetClipBox( hdc, &r ); + ok( !r.left && !r.top && !r.right && !r.bottom, "got %s.\n", wine_dbgstr_rect( &r )); + c = GetPixel(hdc, 1, 1); + ReleaseDC(hwnd, hdc); + ok( c == CLR_INVALID, "got %#lx.\n", c ); + return TRUE; + + case WM_NCACTIVATE: + hdc = GetDC( hwnd ); + GetClipBox( hdc, &r ); + c = GetPixel(hdc, 50, 50); + ReleaseDC(hwnd, hdc); + if (!r.left && !r.top && !r.right && !r.bottom) + ok( c == CLR_INVALID, "got %#lx.\n", c ); + else + todo_wine ok( c == 0xffffff, "got %#lx, msg %#x.\n", c, msg ); + return TRUE; + + case WM_WINDOWPOSCHANGING: + /* This is racy on Windows, even without transition effect, if window is displayed too quick it may end up with + * the bits behind the window. */ + if (pDwmFlush) pDwmFlush(); + return TRUE; + + case WM_ERASEBKGND: + case WM_NCPAINT: + hdc = GetDC( hwnd ); + c = GetPixel(hdc, 50, 50); + ReleaseDC(hwnd, hdc); + todo_wine ok( c == 0xffffff, "got %#lx, msg %#x.\n", c, msg ); + return TRUE; + } + return DefWindowProcA( hwnd, msg, wparam, lparam ); +} + +static void test_initial_surface_colour(void) +{ + WNDCLASSA cls; + HWND hwnd; + BOOL bret; + + memset( &cls, 0, sizeof(cls) ); + cls.lpfnWndProc = test_initial_surface_colour_window_proc; + cls.hInstance = GetModuleHandleA( NULL ); + cls.hCursor = LoadCursorA( 0, (LPCSTR)IDC_ARROW ); + cls.hbrBackground = NULL; + cls.lpszClassName = "test_colour_class"; + + bret = RegisterClassA(&cls); + ok( bret, "got error %lu.\n", GetLastError() ); + + hwnd = CreateWindowExA( 0, "test_colour_class", "Test window", WS_VISIBLE | WS_OVERLAPPEDWINDOW | WS_CLIPCHILDREN, + 0, 0, 100, 100, NULL, NULL, GetModuleHandleA( NULL ), NULL ); + ok( !!hwnd, "got NULL.\n" ); + pump_messages(); + DestroyWindow( hwnd ); + + bret = UnregisterClassA( "test_colour_class", GetModuleHandleA( NULL )); + ok( bret, "got error %lu.\n", GetLastError() ); +} + START_TEST(win) { char **argv; int argc = winetest_get_mainargs( &argv ); HMODULE user32 = GetModuleHandleA( "user32.dll" ); HMODULE gdi32 = GetModuleHandleA("gdi32.dll"); + HMODULE dwmapi = LoadLibraryA("dwmapi.dll"); + pGetWindowInfo = (void *)GetProcAddress( user32, "GetWindowInfo" ); pGetWindowModuleFileNameA = (void *)GetProcAddress( user32, "GetWindowModuleFileNameA" ); pGetLayeredWindowAttributes = (void *)GetProcAddress( user32, "GetLayeredWindowAttributes" ); @@ -14281,6 +14368,9 @@ START_TEST(win) pInternalGetWindowIcon = (void *)GetProcAddress( user32, "InternalGetWindowIcon" ); pSetProcessLaunchForegroundPolicy = (void*)GetProcAddress( user32, "SetProcessLaunchForegroundPolicy" ); + pDwmSetWindowAttribute = (void*)GetProcAddress( dwmapi, "DwmSetWindowAttribute" ); + pDwmFlush = (void*)GetProcAddress( dwmapi, "DwmFlush" ); + if (argc == 4) { HWND hwnd; @@ -14451,6 +14541,7 @@ START_TEST(win) test_cascade_windows(); test_tile_windows(); test_GW_ENABLEDPOPUP(); + test_initial_surface_colour(); /* add the tests above this line */ if (hhook) UnhookWindowsHookEx(hhook); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9573
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/user32/tests/win.c | 4 ++-- dlls/win32u/dce.c | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index 6baf83e39e5..cbdd4a7b591 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -14294,7 +14294,7 @@ static LRESULT WINAPI test_initial_surface_colour_window_proc( HWND hwnd, UINT m if (!r.left && !r.top && !r.right && !r.bottom) ok( c == CLR_INVALID, "got %#lx.\n", c ); else - todo_wine ok( c == 0xffffff, "got %#lx, msg %#x.\n", c, msg ); + ok( c == 0xffffff, "got %#lx, msg %#x.\n", c, msg ); return TRUE; case WM_WINDOWPOSCHANGING: @@ -14308,7 +14308,7 @@ static LRESULT WINAPI test_initial_surface_colour_window_proc( HWND hwnd, UINT m hdc = GetDC( hwnd ); c = GetPixel(hdc, 50, 50); ReleaseDC(hwnd, hdc); - todo_wine ok( c == 0xffffff, "got %#lx, msg %#x.\n", c, msg ); + ok( c == 0xffffff, "got %#lx, msg %#x.\n", c, msg ); return TRUE; } return DefWindowProcA( hwnd, msg, wparam, lparam ); diff --git a/dlls/win32u/dce.c b/dlls/win32u/dce.c index d720d686bcd..5d6cbb70194 100644 --- a/dlls/win32u/dce.c +++ b/dlls/win32u/dce.c @@ -548,6 +548,8 @@ W32KAPI struct window_surface *window_surface_create( UINT size, const struct wi pthread_mutex_init( &surface->mutex, NULL ); + memset( window_surface_get_color( surface, info ), 0xff, info->bmiHeader.biSizeImage ); + TRACE( "created surface %p for hwnd %p rect %s\n", surface, hwnd, wine_dbgstr_rect( &surface->rect ) ); return surface; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9573
This fixes black bars in warning windows in Arc Raiders and The Finals. Those windows are dialogs and black bars are would-be scrollbars from edit control created with WM_VSCROLL | WM_HSCROLL. Later, however, before the control is actually shows, the scroll attributes are cleared with SetWindowLong during WM_INITDIALOG processing in custom dialog procedure, without SetWindowPos (... SWP_FRAMECHANGED). Under these circumstances, upon showing parent window the space for scroll bars is reserved but those areas are never actually painted. I extensively unit tested those aspects separately and that is Windows behaviour. Both on Windows and Wine those areas stay with whatever pixels are in the window surface previously (initially). E. g., if the parent window paints the background before showing client, those areas will have parent window background colour. However, the dialog class has NULL background brush and WS_CLIPCHILDREN. I found that on relatively modern Windows at least that initial background is never black, as the test shows. What it actually ends up with through all the cases ultimately depends on window compositor and its window appearance effect. By default, the window surface will start with underlying windows colours and will be transitioned to window colour after inititial WM_WINDOWPOSCHANING / WM_WINDOWPOSCHANGED handling. Under normal circumstances the window should already be painted already. If it (or some parts of it) weren't, DWM will transition it to white colour. Removing DwmSetWindowAttribute( DWMWA_TRANSITIONS_FORCEDISABLED ) in test and adding various amount of sleep in WM_WINDOWPOSCHANGING instead of DwmFlush will deliver, depending on sleep time, intermediate colours between the image behind the window and white, settling at white. In practice, if trying to show window in a way similar to test but without explicit effect and composition disabling may result in various colours in that ranges, but with actual handling of paints and window initialization it looks like in practice it tends to end up in white (it is also that not skipping but default handling of those WM_NCPAINT, WM_ERASEBACKGROUND messages may also have some interference with this compositor behaviour). So while we don't yet implement neither composition nor transition effects, I think ending up with white initial window surface is better default than black surface. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9573#note_123474
I separately tested that the "ultimate default" is white even with selected dark mode (and it would be weird if theme would have an effect on that level), doesn't look like that depends on some setting, unless buried somewhere in undocumented settings. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9573#note_123476
The test sounds a bit unreliable, how confident are we that it's not going to cause flaky failures? I think we could omit it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9573#note_123624
I hope in this form (with explicit DWM bits) it should be reliable. That succeeds on all the Windows Testbot machines starting from Win8 (while the colours on Win7 suggests that the issue there might be is it still does transition but doesn't have dwmapi to to control that). I am afraid that in case of some regressions with this change figuring out neater conditions for this check starting from scratch, without having a base test will be harder. Can we give it a chance? And in worst case if it does prove to be flaky update or just remove later? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9573#note_123676
I've isolated the test with: ```diff diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index cbdd4a7b591a..9cd2bd62fbd9 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -14408,6 +14408,9 @@ START_TEST(win) if (!RegisterWindowClasses()) assert(0); + test_initial_surface_colour(); + return; + hwndMain = CreateWindowExA(/*WS_EX_TOOLWINDOW*/ 0, "MainWindowClass", "Main window", WS_CAPTION | WS_SYSMENU | WS_MINIMIZEBOX | WS_MAXIMIZEBOX | WS_POPUP | WS_VISIBLE, ``` And ran it on the testbot and it seems to me that it fails quite often (https://testbot.winehq.org/JobDetails.pl?Key=161060, https://testbot.winehq.org/JobDetails.pl?Key=161061). It makes me think that maybe the test suffers from the compositor more often than anticipated and that it is going to be failing spuriously already and should be dropped. Adding a `Sleep(1000)` next to the `DwmFlush` call indeed seems to help (https://testbot.winehq.org/JobDetails.pl?Key=161067, https://testbot.winehq.org/JobDetails.pl?Key=161068) but it's probably still going to be timing dependent and I'm fine with the fix without a test. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9573#note_123682
participants (3)
-
Paul Gofman -
Paul Gofman (＠gofman)
-
Rémi Bernon