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

24 Aug 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@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.

    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

2003

2002

2001

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