[PATCH v3] user32: Don't save maximized position placement for toplevel windows covering the entire work area.

Gabriel Ivăncescu gabrielopcode at gmail.com
Tue Aug 24 11:57:26 CDT 2021


On 24/08/2021 18:12, Huw Davies wrote:
> On Tue, Aug 24, 2021 at 05:44:29PM +0300, Gabriel Ivăncescu wrote:
>> The game "Imperiums: Greek Wars" depends on this to display its window
>> properly. It also fixes the todo_wine in test_window_placement, along with
>> a few new tests that show exactly the threshold at which the "transition"
>> happens.
>>
>> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51672
>> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
>> ---
>>   dlls/user32/tests/win.c | 54 ++++++++++++++++++++++++++++++++---------
>>   dlls/user32/winpos.c    | 47 ++++++++++++++++++++++++++++++++++-
>>   2 files changed, 88 insertions(+), 13 deletions(-)
>>
>> diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c
>> index 228395e..e23ab81 100644
>> --- a/dlls/user32/tests/win.c
>> +++ b/dlls/user32/tests/win.c
>> @@ -11624,8 +11624,9 @@ static void test_IsWindowEnabled(void)
>>   
>>   static void test_window_placement(void)
>>   {
>> -    RECT orig = {100, 200, 300, 400}, orig2 = {200, 300, 400, 500}, rect;
>> +    RECT orig = {100, 200, 300, 400}, orig2 = {200, 300, 400, 500}, rect, work_rect;
>>       WINDOWPLACEMENT wp = {sizeof(wp)};
>> +    MONITORINFO mon_info;
>>       HWND hwnd;
>>       BOOL ret;
>>   
>> @@ -11633,6 +11634,10 @@ static void test_window_placement(void)
>>           orig.left, orig.top, orig.right - orig.left, orig.bottom - orig.top, 0, 0, 0, 0);
>>       ok(!!hwnd, "failed to create window, error %u\n", GetLastError());
>>   
>> +    mon_info.cbSize = sizeof(mon_info);
>> +    GetMonitorInfoW(MonitorFromWindow(hwnd, MONITOR_DEFAULTTOPRIMARY), &mon_info);
>> +    work_rect = mon_info.rcWork;
>> +
>>       ret = GetWindowPlacement(hwnd, &wp);
>>       ok(ret, "failed to get window placement, error %u\n", GetLastError());
>>       ok(wp.showCmd == SW_SHOWNORMAL, "got show cmd %u\n", wp.showCmd);
>> @@ -11675,7 +11680,6 @@ static void test_window_placement(void)
>>       ok(wp.showCmd == SW_SHOWMAXIMIZED, "got show cmd %u\n", wp.showCmd);
>>       ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
>>           "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
>> -todo_wine
>>       ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
>>           "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
>>       ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
>> @@ -11693,6 +11697,42 @@ todo_wine
>>       ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
>>           wine_dbgstr_rect(&wp.rcNormalPosition));
>>   
>> +    SetWindowPos(hwnd, 0, work_rect.left, work_rect.top, work_rect.right - work_rect.left,
>> +                 work_rect.bottom - work_rect.top, SWP_NOZORDER | SWP_NOACTIVATE);
>> +    ret = GetWindowPlacement(hwnd, &wp);
>> +    ok(ret, "failed to get window placement, error %u\n", GetLastError());
>> +    ok(wp.showCmd == SW_SHOWMAXIMIZED, "got show cmd %u\n", wp.showCmd);
>> +    ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
>> +        "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
>> +    ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
>> +        "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
>> +    ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
>> +        wine_dbgstr_rect(&wp.rcNormalPosition));
>> +
>> +    SetWindowPos(hwnd, 0, work_rect.left, work_rect.top, work_rect.right - work_rect.left - 1,
>> +                 work_rect.bottom - work_rect.top, SWP_NOZORDER | SWP_NOACTIVATE);
>> +    ret = GetWindowPlacement(hwnd, &wp);
>> +    ok(ret, "failed to get window placement, error %u\n", GetLastError());
>> +    ok(wp.showCmd == SW_SHOWMAXIMIZED, "got show cmd %u\n", wp.showCmd);
>> +    ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
>> +        "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
>> +    ok(wp.ptMaxPosition.x == work_rect.left && wp.ptMaxPosition.y == work_rect.top,
>> +        "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
>> +    ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
>> +        wine_dbgstr_rect(&wp.rcNormalPosition));
>> +
>> +    SetWindowPos(hwnd, 0, work_rect.left, work_rect.top, work_rect.right - work_rect.left,
>> +                 work_rect.bottom - work_rect.top - 1, SWP_NOZORDER | SWP_NOACTIVATE);
>> +    ret = GetWindowPlacement(hwnd, &wp);
>> +    ok(ret, "failed to get window placement, error %u\n", GetLastError());
>> +    ok(wp.showCmd == SW_SHOWMAXIMIZED, "got show cmd %u\n", wp.showCmd);
>> +    ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
>> +        "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
>> +    ok(wp.ptMaxPosition.x == work_rect.left && wp.ptMaxPosition.y == work_rect.top,
>> +        "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
>> +    ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
>> +        wine_dbgstr_rect(&wp.rcNormalPosition));
>> +
>>       ShowWindow(hwnd, SW_MINIMIZE);
>>   
>>       ret = GetWindowPlacement(hwnd, &wp);
>> @@ -11701,7 +11741,6 @@ todo_wine
>>       ok(wp.showCmd == SW_SHOWMINIMIZED, "got show cmd %u\n", wp.showCmd);
>>       ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
>>           "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
>> -todo_wine
>>       ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
>>           "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
>>       ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
>> @@ -11714,7 +11753,6 @@ todo_wine
>>       ok(wp.showCmd == SW_SHOWMAXIMIZED, "got show cmd %u\n", wp.showCmd);
>>       ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
>>           "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
>> -todo_wine
>>       ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
>>           "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
>>       ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
>> @@ -11727,7 +11765,6 @@ todo_wine
>>       ok(wp.showCmd == SW_SHOWNORMAL, "got show cmd %u\n", wp.showCmd);
>>       ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
>>           "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
>> -todo_wine
>>       ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
>>           "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
>>       ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
>> @@ -11745,7 +11782,6 @@ todo_wine
>>       ok(wp.showCmd == SW_SHOWNORMAL, "got show cmd %u\n", wp.showCmd);
>>       ok(wp.ptMinPosition.x == 100 && wp.ptMinPosition.y == 100,
>>           "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
>> -todo_wine
>>       ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
>>           "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
>>       ok(EqualRect(&wp.rcNormalPosition, &orig2), "got normal pos %s\n",
>> @@ -11761,7 +11797,6 @@ todo_wine
>>       ok(wp.showCmd == SW_SHOWMINIMIZED, "got show cmd %u\n", wp.showCmd);
>>       ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
>>           "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
>> -todo_wine
>>       ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
>>           "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
>>       ok(EqualRect(&wp.rcNormalPosition, &orig2), "got normal pos %s\n",
>> @@ -11783,7 +11818,6 @@ todo_wine
>>       ok(wp.showCmd == SW_SHOWMINIMIZED, "got show cmd %u\n", wp.showCmd);
>>       ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
>>           "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
>> -todo_wine
>>       ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
>>           "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
>>       ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
>> @@ -11804,7 +11838,6 @@ todo_wine
>>       ok(wp.showCmd == SW_SHOWMAXIMIZED, "got show cmd %u\n", wp.showCmd);
>>       ok(wp.ptMinPosition.x == 100 && wp.ptMinPosition.y == 100,
>>           "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
>> -todo_wine
>>       ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
>>           "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
>>       ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
>> @@ -11825,7 +11858,6 @@ todo_wine
>>       ok(wp.showCmd == SW_SHOWMINIMIZED, "got show cmd %u\n", wp.showCmd);
>>       ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
>>           "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
>> -todo_wine
>>       ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
>>           "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
>>       ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
>> @@ -11839,7 +11871,6 @@ todo_wine
>>       ok(wp.showCmd == SW_SHOWMINIMIZED, "got show cmd %u\n", wp.showCmd);
>>       ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
>>           "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
>> -todo_wine
>>       ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
>>           "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
>>       ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
>> @@ -11854,7 +11885,6 @@ todo_wine
>>       ok(wp.showCmd == SW_NORMAL, "got show cmd %u\n", wp.showCmd);
>>       ok(wp.ptMinPosition.x == -32000 && wp.ptMinPosition.y == -32000,
>>           "got minimized pos (%d,%d)\n", wp.ptMinPosition.x, wp.ptMinPosition.y);
>> -todo_wine
>>       ok(wp.ptMaxPosition.x == -1 && wp.ptMaxPosition.y == -1,
>>           "got maximized pos (%d,%d)\n", wp.ptMaxPosition.x, wp.ptMaxPosition.y);
>>       ok(EqualRect(&wp.rcNormalPosition, &orig), "got normal pos %s\n",
>> diff --git a/dlls/user32/winpos.c b/dlls/user32/winpos.c
>> index 6e96a4b..b5733d8 100644
>> --- a/dlls/user32/winpos.c
>> +++ b/dlls/user32/winpos.c
>> @@ -916,6 +916,47 @@ static POINT get_minimized_pos( HWND hwnd, POINT pt )
>>       return pt;
>>   }
>>   
>> +static void update_maximized_pos( WND *wnd )
>> +{
>> +    MONITORINFO mon_info;
>> +    HMONITOR monitor;
>> +    RECT rc_work;
>> +
>> +    /* For top level windows covering the working area, we might
>> +       have to "forget" the maximized position. Windows presumably
>> +       does this to avoid situations where the border style changes,
>> +       which would lead the window to be outside the screen, or the
>> +       window gets reloaded on a different screen, and the "saved"
>> +       position no longer applies to it (despite being maximized).
>> +
>> +       Some applications (e.g. Imperiums: Greek Wars) depend on this.
>> +    */
>> +    if (!wnd->parent || wnd->parent == GetDesktopWindow())
> 
> Returning early if (wnd->parent && wnd->parent != GDW()) would avoid a
> level of indentation here.
> 
>> +    {
>> +        if (wnd->dwStyle & WS_MAXIMIZE)
>> +        {
>> +            if ((monitor = MonitorFromWindow( wnd->obj.handle, MONITOR_DEFAULTTOPRIMARY )))
>> +            {
>> +                mon_info.cbSize = sizeof(mon_info);
>> +                GetMonitorInfoW( monitor, &mon_info );
>> +                rc_work = mon_info.rcMonitor;
>> +
>> +                /* use the work area for windows that maximize without covering it, to match GetMinMaxInfo */
>> +                if ((wnd->dwStyle & WS_CAPTION) == WS_CAPTION || !(wnd->dwStyle & (WS_CHILD | WS_POPUP)))
>> +                    rc_work = mon_info.rcWork;
> 
> The GetMixMaxInfo fixup seems to occur with WS_MAXIMIZEBOX, should
> that be taken into account?  It might be cleaner to add a helper to
> GetMinMaxInfo and here to combine these checks.
> 

Good point, I'll do that.

>> +            }
>> +            else
>> +                SetRect( &rc_work, 0, 0, GetSystemMetrics( SM_CXSCREEN ), GetSystemMetrics( SM_CYSCREEN ) );
>> +
>> +            if (wnd->window_rect.left  <= rc_work.left  && wnd->window_rect.top    <= rc_work.top &&
>> +                wnd->window_rect.right >= rc_work.right && wnd->window_rect.bottom >= rc_work.bottom)
>> +                wnd->max_pos.x = wnd->max_pos.y = -1;
>> +        }
>> +        else
>> +            wnd->max_pos.x = wnd->max_pos.y = -1;
>> +    }
>> +}
>> +
>>   
>>   /***********************************************************************
>>    *           WINPOS_MinMaximize
>> @@ -1337,6 +1378,7 @@ BOOL WINAPI GetWindowPlacement( HWND hwnd, WINDOWPLACEMENT *wndpl )
>>       {
>>           pWnd->normal_rect = pWnd->window_rect;
>>       }
>> +    update_maximized_pos( pWnd );
>>   
>>       wndpl->length  = sizeof(*wndpl);
>>       if( pWnd->dwStyle & WS_MINIMIZE )
>> @@ -1424,8 +1466,11 @@ static BOOL WINPOS_SetPlacement( HWND hwnd, const WINDOWPLACEMENT *wndpl, UINT f
>>   
>>       if (!pWnd || pWnd == WND_OTHER_PROCESS || pWnd == WND_DESKTOP) return FALSE;
>>   
>> +    if (flags & PLACE_MAX) {
> 
> Brace style.
> 
>> +        pWnd->max_pos = point_thread_to_win_dpi( hwnd, wp.ptMaxPosition );
>> +        update_maximized_pos( pWnd );
>> +    }
>>       if (flags & PLACE_MIN) pWnd->min_pos = point_thread_to_win_dpi( hwnd, wp.ptMinPosition );
>> -    if (flags & PLACE_MAX) pWnd->max_pos = point_thread_to_win_dpi( hwnd, wp.ptMaxPosition );
> 
> Any resason to move the PLACE_MAX before PLACE_MIN?  It seems like an unnecessary change.
> 

Not much reason other than having the others next to each other, since 
they're one-liners. Just a style thing, I'll keep it in the middle.

>>       if (flags & PLACE_RECT) pWnd->normal_rect = rect_thread_to_win_dpi( hwnd, wp.rcNormalPosition );
>>   
>>       style = pWnd->dwStyle;
> 
> Huw.
> 




More information about the wine-devel mailing list