[PATCH v3 2/2] winex11.drv: Call XIconifyWindow() after XMapWindow() for minimized windows.

[Rémi Bernon]

On 3/14/22 07:57, Zhiyi Zhang wrote:
> Mutter always unminimizes windows when handling map requests. So a window could be in
> normal state as far as Mutter concerns while Wine mistakenly considers it still minimized.
> 
> Fix Disgaea PC black screen after Alt+Tab in fullscreen mode.
> 
> Also see https://gitlab.gnome.org/GNOME/mutter/-/blob/main/src/x11/events.c#L1525 for how map
> requests are handled.
> 
> Signed-off-by: Zhiyi Zhang <zzhang at codeweavers.com>
> ---
>   dlls/user32/tests/win.c   |  1 -
>   dlls/winex11.drv/window.c | 11 +++++++++++
>   2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c
> index f47fcd2fe65..a7c2dc47f9a 100644
> --- a/dlls/user32/tests/win.c
> +++ b/dlls/user32/tests/win.c
> @@ -7714,7 +7714,6 @@ static void test_ShowWindow(void)
>       /* The window rectangle of hwndMain is (100, 100, 300, 300). If the window manager unminimizes
>        * hwnd, hwnd will end up obscuring hwndMain. Mutter is such a window manager that unminimizes
>        * windows when handling map requests and thus can demonstrate this bug */
> -    todo_wine_if(color != 0xffffff)
>       ok(color == 0xffffff, "Expected color %#x, got %#x.\n", 0xffffff, color);
>       ReleaseDC(0, hdc);
>       DestroyWindow(hwnd);
> diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c
> index 36fb41ac710..7ccb139ba5d 100644
> --- a/dlls/winex11.drv/window.c
> +++ b/dlls/winex11.drv/window.c
> @@ -1129,6 +1129,9 @@ static void map_window( HWND hwnd, DWORD new_style )
>               update_net_wm_states( data );
>               sync_window_style( data );
>               XMapWindow( data->display, data->whole_window );
> +            /* Mutter always unminimizes windows when handling map requests. Restore iconic state */
> +            if (new_style & WS_MINIMIZE)
> +                XIconifyWindow( data->display, data->whole_window, data->vis.screen );
>               XFlush( data->display );
>               if (data->surface && data->vis.visualid != default_visual.visualid)
>                   data->surface->funcs->flush( data->surface );


Although it looks to be required indeed, I think this is more like a 
mutter bug, the ICCCM spec indicates that withdrawn -> iconic transition 
should be done by mapping the window with the WM_HINTS initial_state set 
to IconicState, which is what we are doing already.


> @@ -2528,9 +2531,17 @@ void CDECL X11DRV_WindowPosChanged( HWND hwnd, HWND insert_after, UINT swp_flags
>               data->iconic = (new_style & WS_MINIMIZE) != 0;
>               TRACE( "changing win %p iconic state to %u\n", data->hwnd, data->iconic );
>               if (data->iconic)
> +            {
>                   XIconifyWindow( data->display, data->whole_window, data->vis.screen );
> +            }
>               else if (is_window_rect_mapped( rectWindow ))
> +            {
> +                /* whole_window could be both iconic and mapped. Since XMapWindow() doesn't do
> +                 * anything if the window is already mapped, we need to unmap it first */
> +                if (data->mapped)
> +                    XUnmapWindow( data->display, data->whole_window );
>                   XMapWindow( data->display, data->whole_window );
> +            }
>               update_net_wm_states( data );
>           }
>           else


Then this second change doesn't even seem to be required. And the ICCCM 
spec also says that mapping the window should be enough. I also think 
that quick unmap/map requests can lead to more trouble.

-- 
Rémi Bernon <rbernon at codeweavers.com>