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

[Gabriel Ivăncescu]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51672
Signed-off-by: Gabriel Ivăncescu 
---
 dlls/user32/tests/win.c | 54 +++++++++++++++++++++------
 dlls/user32/winpos.c    | 81 ++++++++++++++++++++++++++++++++---------
 2 files changed, 106 insertions(+), 29 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..255a40b 100644
--- a/dlls/user32/winpos.c
+++ b/dlls/user32/winpos.c
@@ -675,6 +675,33 @@ BOOL WINAPI MoveWindow( HWND hwnd, INT x, INT y, INT cx, INT cy,
 }

+/*******************************************************************
+ *           get_maximized_area_rect
+ *
+ * Get the work area that a maximized window can cover, depending on style.
+ */
+static BOOL get_work_rect_for_window( HWND hwnd, RECT *rect )
+{
+    HMONITOR monitor = MonitorFromWindow( hwnd, MONITOR_DEFAULTTOPRIMARY );
+    MONITORINFO mon_info;
+    DWORD style;
+
+    if (!monitor) return FALSE;
+
+    mon_info.cbSize = sizeof(mon_info);
+    GetMonitorInfoW( monitor, &mon_info );
+
+    style = GetWindowLongW( hwnd, GWL_STYLE );
+    *rect = mon_info.rcMonitor;
+    if (style & WS_MAXIMIZEBOX)
+    {
+        if ((style & WS_CAPTION) == WS_CAPTION || !(style & (WS_CHILD | WS_POPUP)))
+            *rect = mon_info.rcWork;
+    }
+    return TRUE;
+}
+
+
 /*******************************************************************
  *           WINPOS_GetMinMaxInfo
  *
@@ -683,8 +710,8 @@ BOOL WINAPI MoveWindow( HWND hwnd, INT x, INT y, INT cx, INT cy,
 MINMAXINFO WINPOS_GetMinMaxInfo( HWND hwnd )
 {
     DPI_AWARENESS_CONTEXT context;
+    RECT rc_work, rc_primary;
     MINMAXINFO MinMax;
-    HMONITOR monitor;
     INT xinc, yinc;
     LONG style = GetWindowLongW( hwnd, GWL_STYLE );
     LONG adjustedStyle;
@@ -738,22 +765,8 @@ MINMAXINFO WINPOS_GetMinMaxInfo( HWND hwnd )

/* if the app didn't change the values, adapt them for the current monitor */

-    if ((monitor = MonitorFromWindow( hwnd, MONITOR_DEFAULTTOPRIMARY )))
+    if (get_work_rect_for_window( hwnd, &rc_work ))
     {
-        RECT rc_work, rc_primary;
-        MONITORINFO mon_info;
-
-        mon_info.cbSize = sizeof(mon_info);
-        GetMonitorInfoW( monitor, &mon_info );
-
-        rc_work = mon_info.rcMonitor;
-
-        if (style & WS_MAXIMIZEBOX)
-        {
-            if ((style & WS_CAPTION) == WS_CAPTION || !(style & (WS_CHILD | WS_POPUP)))
-                rc_work = mon_info.rcWork;
-        }
-
         rc_primary = get_primary_monitor_rect();
         if (MinMax.ptMaxSize.x == (rc_primary.right - rc_primary.left) + 2 * xinc &&
             MinMax.ptMaxSize.y == (rc_primary.bottom - rc_primary.top) + 2 * yinc)
@@ -916,6 +929,35 @@ static POINT get_minimized_pos( HWND hwnd, POINT pt )
     return pt;
 }

+static void update_maximized_pos( WND *wnd )
+{
+    RECT rc_work;
+
+    /* For top level windows covering the work 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())
+        return;
+
+    if (wnd->dwStyle & WS_MAXIMIZE)
+    {
+        if (!get_work_rect_for_window( wnd->obj.handle, &rc_work ))
+            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 +1379,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 )
@@ -1425,7 +1468,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_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 );
+    if (flags & PLACE_MAX)
+    {
+        pWnd->max_pos = point_thread_to_win_dpi( hwnd, wp.ptMaxPosition );
+        update_maximized_pos( pWnd );
+    }
     if (flags & PLACE_RECT) pWnd->normal_rect = rect_thread_to_win_dpi( hwnd, wp.rcNormalPosition );

style = pWnd->dwStyle;

[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=96283

Your paranoid android.

=== debiant2 (32 bit WoW report) ===

user32:
win.c:10383: Test failed: Expected foreground window 00150128, got 00E400F2
win.c:10385: Test failed: GetActiveWindow() = 00000000
win.c:10385: Test failed: GetFocus() = 00000000
win.c:10386: Test failed: Received WM_ACTIVATEAPP(1), did not expect it.
win.c:10387: Test failed: Received WM_ACTIVATEAPP(0), did not expect it.
win.c:10395: Test failed: Expected foreground window 00150128, got 00000000
win.c:10397: Test failed: GetActiveWindow() = 00000000
win.c:10397: Test failed: GetFocus() = 00000000
win.c:10405: Test failed: Received WM_ACTIVATEAPP(1), did not expect it.

[Gabriel Ivăncescu]

I realized one thing: this is holding a lock implicitly while calling 
MonitorFromWindow or GetWindowLongW (with GWL_STYLE) via the WND 
pointer. Is that a problem?

I'm not very familiar with this so I don't know if it's a good idea or 
not (I know sending messages isn't). Perhaps due to Windows API hooks?

Fixing it would obviously complicate the code so I'd rather only do it 
if necessary.

[Huw Davies]

On Wed, Aug 25, 2021 at 06:16:47PM +0300, Gabriel Ivăncescu wrote:
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51672
> Signed-off-by: Gabriel Ivăncescu

I meant to mention this earlier but the commit summary message is a
bit long.  Simply deleting the word "placement" would help.
> --- a/dlls/user32/winpos.c
> +++ b/dlls/user32/winpos.c
> @@ -675,6 +675,33 @@ BOOL WINAPI MoveWindow( HWND hwnd, INT x, INT y, INT cx, INT cy,
>  }
>
> +/*******************************************************************
> -
> -
>       get_maximized_area_rect
> -
> -
> -
> - Get the work area that a maximized window can cover, depending on style.
> - */
>
> +static BOOL get_work_rect_for_window( HWND hwnd, RECT *rect )

I'd call this get_work_rect (I don't thing the "for_window" adds anything.
Also, note that the comment above is calling this something else.
> +{
> - HMONITOR monitor = MonitorFromWindow( hwnd, MONITOR_DEFAULTTOPRIMARY );
> - MONITORINFO mon_info;
> - DWORD style;
> -
> - if (!monitor) return FALSE;
> -
> - mon_info.cbSize = sizeof(mon_info);
> - GetMonitorInfoW( monitor, &mon_info );
> -
> - style = GetWindowLongW( hwnd, GWL_STYLE );
> - *rect = mon_info.rcMonitor;
> - if (style & WS_MAXIMIZEBOX)
> - {
> -
>    if ((style & WS_CAPTION) == WS_CAPTION || !(style & (WS_CHILD | WS_POPUP)))
> -
>        *rect = mon_info.rcWork;
> - }
> - return TRUE;
>
> +}
> -
> -
>
> /*******************************************************************
> -
>       WINPOS_GetMinMaxInfo
> -
>
> @@ -916,6 +929,35 @@ static POINT get_minimized_pos( HWND hwnd, POINT pt )
>      return pt;
>  }
>
> +static void update_maximized_pos( WND *wnd )
> +{
> - RECT rc_work;
> -
> - /* For top level windows covering the work 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())
> -
>    return;
> -
> - if (wnd->dwStyle & WS_MAXIMIZE)
> - {
> -
>    if (!get_work_rect_for_window( wnd->obj.handle, &rc_work ))
> -
>        SetRect( &rc_work, 0, 0, GetSystemMetrics( SM_CXSCREEN ), GetSystemMetrics( SM_CYSCREEN ) );

As you pointed out in another email, you now have the user lock.  It
doesn't seem very hard to avoid that, simply call get_work_rect()
before taking the lock and pass the work rect to this function.
There's a small wrinkle in that GetMonitorWindow() calls
GetWindowPlacement() if the window is iconic, so you wouldn't
want to call get_work_rect() in that case.

Huw.

[Gabriel Ivăncescu]

On 26/08/2021 09:47, Huw Davies wrote:
> On Wed, Aug 25, 2021 at 06:16:47PM +0300, Gabriel Ivăncescu wrote:
> > Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51672
> > Signed-off-by: Gabriel Ivăncescu
>
> I meant to mention this earlier but the commit summary message is a
> bit long.  Simply deleting the word "placement" would help.
> > --- a/dlls/user32/winpos.c
> > +++ b/dlls/user32/winpos.c
> > @@ -675,6 +675,33 @@ BOOL WINAPI MoveWindow( HWND hwnd, INT x, INT y, INT cx, INT cy,
> >   }
> >
> > +/*******************************************************************
> > -
> > -
> >       get_maximized_area_rect
> > -
> > -
> > -
> > - Get the work area that a maximized window can cover, depending on style.
> > - */
> >
> > +static BOOL get_work_rect_for_window( HWND hwnd, RECT *rect )
>
> I'd call this get_work_rect (I don't thing the "for_window" adds anything.
> Also, note that the comment above is calling this something else.
> > +{
> > - HMONITOR monitor = MonitorFromWindow( hwnd, MONITOR_DEFAULTTOPRIMARY );
> > - MONITORINFO mon_info;
> > - DWORD style;
> > -
> > - if (!monitor) return FALSE;
> > -
> > - mon_info.cbSize = sizeof(mon_info);
> > - GetMonitorInfoW( monitor, &mon_info );
> > -
> > - style = GetWindowLongW( hwnd, GWL_STYLE );
> > - *rect = mon_info.rcMonitor;
> > - if (style & WS_MAXIMIZEBOX)
> > - {
> > -
> >    if ((style & WS_CAPTION) == WS_CAPTION || !(style & (WS_CHILD | WS_POPUP)))
> > -
> >        *rect = mon_info.rcWork;
> > - }
> > - return TRUE;
> >
> > +}
> > -
> > - /*******************************************************************
> > -
> >       WINPOS_GetMinMaxInfo
> > -
> >
> > @@ -916,6 +929,35 @@ static POINT get_minimized_pos( HWND hwnd, POINT pt )
> >       return pt;
> >   }
> >
> > +static void update_maximized_pos( WND *wnd )
> > +{
> > - RECT rc_work;
> > -
> > - /* For top level windows covering the work 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())
> > -
> >    return;
> > -
> > - if (wnd->dwStyle & WS_MAXIMIZE)
> > - {
> > -
> >    if (!get_work_rect_for_window( wnd->obj.handle, &rc_work ))
> > -
> >        SetRect( &rc_work, 0, 0, GetSystemMetrics( SM_CXSCREEN ), GetSystemMetrics( SM_CYSCREEN ) );
>
> As you pointed out in another email, you now have the user lock.  It
> doesn't seem very hard to avoid that, simply call get_work_rect()
> before taking the lock and pass the work rect to this function.
> There's a small wrinkle in that GetMonitorWindow() calls
> GetWindowPlacement() if the window is iconic, so you wouldn't
> want to call get_work_rect() in that case.
>
> Huw.

Thanks, I didn't really like the IsIconic thing, it seemed fragile, so I 
went with a slightly different approach, which also makes 
update_maximized_pos more typical like the other helpers, in that it 
takes a HWND rather than a WND, and does all the implementation details 
within. In one call site I just moved the call outside the lock, but for 
the other I had to release/re-acquire the lock, I hope it's not too big 
of a deal. It's still less code than special casing the work rect it in 
both callers.

I sent it so you can see if you like it better, if not I'll revert it to 
your suggestion.

Thanks,
Gabriel

[Huw Davies]

On Thu, Aug 26, 2021 at 05:59:45PM +0300, Gabriel Ivăncescu wrote:
> Thanks, I didn't really like the IsIconic thing, it seemed fragile, so I
> went with a slightly different approach, which also makes
> update_maximized_pos more typical like the other helpers, in that it takes a
> HWND rather than a WND, and does all the implementation details within. In
> one call site I just moved the call outside the lock, but for the other I
> had to release/re-acquire the lock, I hope it's not too big of a deal. It's
> still less code than special casing the work rect it in both callers.

Dropping and reacquiring the lock isn't very pretty (and needs care to
keep the window state consistent).

Huw.

[Gabriel Ivăncescu]

On 27/08/2021 17:51, Huw Davies wrote:
> On Thu, Aug 26, 2021 at 05:59:45PM +0300, Gabriel Ivăncescu wrote:
> > Thanks, I didn't really like the IsIconic thing, it seemed fragile, so I
> > went with a slightly different approach, which also makes
> > update_maximized_pos more typical like the other helpers, in that it takes a
> > HWND rather than a WND, and does all the implementation details within. In
> > one call site I just moved the call outside the lock, but for the other I
> > had to release/re-acquire the lock, I hope it's not too big of a deal. It's
> > still less code than special casing the work rect it in both callers.
>
> Dropping and reacquiring the lock isn't very pretty (and needs care to
> keep the window state consistent).
>
> Huw.

Yeah, but I figured it's not a problem since calling GetWindowStyle or 
IsIconic does the same thing (acquires lock, gets style, releases lock), 
but anyway I sent a v7 with this method.

I used a helper function to get the maximized work rect only for 
maximized windows (because that's what we care about, and we only use it 
in such case) which implicitly handles the IsIconic issue, and 
simplifies the callers since it is one line only at initialization.