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

[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 <gabrielopcode at gmail.com>
> 
> 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