[PATCH 0/2] MR9049: winemac.drv: Don't crash when a window with a GL context is minimized
Since 7b28c6d17c7da278307756cfcd0da830e5d39baf, we directly use `NtUserGetClientRect` to get the size of the GL target windows. But that returns an empty rect for minimized windows, and attempting to set a zero size on a CGL context in `-[WineOpenGLContext wine_updateBackingSize:]` causes a Metal assertion. You can see this by minimizing e.g. Geometry Dash or Garry's Mod. We could just not call wine_updateBackingSize if we find a zero dimension for the window, but I think we want to attempt to keep the backing size correct. Perhaps there's a better way to get the client rect of a minimized window than the un-AdjustWindowRect approach I'm using here? If the window is minimized fullscreen, `get_win_data(hwnd)->rects.client` is (-32000, -32000) - (-32000, -32000). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9049
From: Tim Clem <tclem(a)codeweavers.com> Doing so results in a Metal assertion on newer OSes. --- dlls/winemac.drv/cocoa_opengl.m | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/dlls/winemac.drv/cocoa_opengl.m b/dlls/winemac.drv/cocoa_opengl.m index ea00f95ff68..8be73b18afb 100644 --- a/dlls/winemac.drv/cocoa_opengl.m +++ b/dlls/winemac.drv/cocoa_opengl.m @@ -94,6 +94,7 @@ - (void) resetSurfaceIfBackingSizeChanged } } + /* The size must not have a zero dimension. */ - (void) wine_updateBackingSize:(const CGSize*)size { GLint enabled; @@ -234,7 +235,8 @@ void macdrv_make_context_current(macdrv_opengl_context c, macdrv_view v, CGRect { if (view == [context view] || view == [context latentView]) { - [context wine_updateBackingSize:&r.size]; + if (!CGRectIsEmpty(r)) + [context wine_updateBackingSize:&r.size]; macdrv_update_opengl_context(c); } else @@ -248,7 +250,8 @@ void macdrv_make_context_current(macdrv_opengl_context c, macdrv_view v, CGRect context.needsReattach = FALSE; if (context.view) [context setView:[[context class] dummyView]]; - [context wine_updateBackingSize:&r.size]; + if (!CGRectIsEmpty(r)) + [context wine_updateBackingSize:&r.size]; [context setView:view]; [context setLatentView:nil]; [context resetSurfaceIfBackingSizeChanged]; @@ -257,7 +260,8 @@ void macdrv_make_context_current(macdrv_opengl_context c, macdrv_view v, CGRect { if ([context view]) [context clearDrawableLeavingSurfaceOnScreen]; - [context wine_updateBackingSize:&r.size]; + if (!CGRectIsEmpty(r)) + [context wine_updateBackingSize:&r.size]; [context setLatentView:view]; } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9049
From: Tim Clem <tclem(a)codeweavers.com> --- dlls/winemac.drv/opengl.c | 53 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/dlls/winemac.drv/opengl.c b/dlls/winemac.drv/opengl.c index 7f145b73711..88f26e6fc24 100644 --- a/dlls/winemac.drv/opengl.c +++ b/dlls/winemac.drv/opengl.c @@ -1489,30 +1489,75 @@ static void macdrv_surface_destroy(struct opengl_drawable *base) TRACE("drawable %s\n", debugstr_opengl_drawable(base)); } +/* Converts a whole window rect to a client rect; the inverse of + AdjustWindowRect. */ +static void window_rect_to_client(HWND hwnd, RECT *rect) +{ + RECT adjustments = { 0 }; + DWORD style = NtUserGetWindowLongW(hwnd, GWL_STYLE); + DWORD ex_style = NtUserGetWindowLongW(hwnd, GWL_EXSTYLE); + UINT dpi = NtUserGetDpiForWindow(hwnd); + BOOL has_menu = !(style & WS_CHILD) && NtUserGetWindowLongW(hwnd, GWLP_ID); + + /* Adjust a zero rect, and use those changes as insets. */ + NtUserAdjustWindowRect(&adjustments, style, has_menu, ex_style, dpi); + rect->top -= adjustments.top; + rect->left -= adjustments.left; + rect->bottom -= adjustments.bottom; + rect->right -= adjustments.right; +} + +/* Returns the client rect of the given window (even if it's minimized), or + CGRectNull on error. */ +static CGRect get_normal_client_rect(HWND hwnd) +{ + RECT rect; + + if (NtUserGetWindowLongW(hwnd, GWL_STYLE) & WS_MINIMIZE) + { + /* NtUserGetClientRect will return a zero rect for minimized windows, so + we have to get creative. */ + WINDOWPLACEMENT placement; + if (!NtUserGetWindowPlacement(hwnd, &placement)) + return CGRectNull; + window_rect_to_client(hwnd, &placement.rcNormalPosition); + return cgrect_from_rect(placement.rcNormalPosition); + } + + if (NtUserGetClientRect(hwnd, &rect, NtUserGetDpiForWindow(hwnd))) + return cgrect_from_rect(rect); + + return CGRectNull; +} + /********************************************************************** * make_context_current */ static void make_context_current(struct macdrv_context *context, BOOL read) { macdrv_view view; - RECT view_rect; + CGRect cg_view_rect = CGRectNull; CGLPBufferObj pbuffer; if (read) { - if (context->read_hwnd) NtUserGetClientRect(context->read_hwnd, &view_rect, NtUserGetDpiForWindow(context->read_hwnd)); + if (context->read_hwnd) cg_view_rect = get_normal_client_rect(context->read_hwnd); view = context->read_view; pbuffer = context->read_pbuffer; } else { - if (context->draw_hwnd) NtUserGetClientRect(context->draw_hwnd, &view_rect, NtUserGetDpiForWindow(context->draw_hwnd)); + if (context->draw_hwnd) cg_view_rect = get_normal_client_rect(context->draw_hwnd); view = context->draw_view; pbuffer = context->draw_pbuffer; } if (view || !pbuffer) - macdrv_make_context_current(context->context, view, cgrect_from_rect(view_rect)); + { + if (CGRectIsEmpty(cg_view_rect)) + WARN("empty view rect for %s hwnd %p\n", read ? "read" : "draw", read ? context->read_hwnd : context->draw_hwnd); + macdrv_make_context_current(context->context, view, cg_view_rect); + } else { GLint enabled; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9049
Rémi Bernon (@rbernon) commented about dlls/winemac.drv/opengl.c:
+ + if (NtUserGetWindowLongW(hwnd, GWL_STYLE) & WS_MINIMIZE) + { + /* NtUserGetClientRect will return a zero rect for minimized windows, so + we have to get creative. */ + WINDOWPLACEMENT placement; + if (!NtUserGetWindowPlacement(hwnd, &placement)) + return CGRectNull; + window_rect_to_client(hwnd, &placement.rcNormalPosition); + return cgrect_from_rect(placement.rcNormalPosition); + } + + if (NtUserGetClientRect(hwnd, &rect, NtUserGetDpiForWindow(hwnd))) + return cgrect_from_rect(rect); + + return CGRectNull; I think it would be better if we could avoid doing that, is it useful to have a non-empty rect for minimized windows? Is the window content still visible somehow? Could we keep track of non-minimized client window rect, instead of having to query and try reconstructing it with AdjustWindowRect?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9049#note_116975
On Mon Sep 29 09:26:20 2025 +0000, Rémi Bernon wrote:
I think it would be better if we could avoid doing that, is it useful to have a non-empty rect for minimized windows? Is the window content still visible somehow? Could we keep track of non-minimized client window rect, instead of having to query and try reconstructing it with AdjustWindowRect? Well, for a while now, macdrv windows have had empty previews in the Dock when minimized. I'm not sure why that is - we let the OS generate that thumbnail if the window isn't empty-shaped (see the various calls to `-[WineWindow grabDockIconSnapshotFromWindow:force:]`). I thought the backing size might be part of it, but it's still empty with this patch. So perhaps we can just have `-wine_updateBackingSize` fall through to the NULL case if either dimension of the size is 0. @bshanks Any idea whether maintaining the backing size is important for minimized windows?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9049#note_117062
On Mon Sep 29 17:27:50 2025 +0000, Tim Clem wrote:
Well, for a while now, macdrv windows have had empty previews in the Dock when minimized. I'm not sure why that is - we let the OS generate that thumbnail if the window isn't empty-shaped (see the various calls to `-[WineWindow grabDockIconSnapshotFromWindow:force:]`). I thought the backing size might be part of it, but it's still empty with this patch. So perhaps we can just have `-wine_updateBackingSize` fall through to the NULL case if either dimension of the size is 0. @bshanks Any idea whether maintaining the backing size is important for minimized windows? @rbernon If possible I think we do want to have a non-empty rect for minimized windows, since they keep displaying when minimized (just shrunk down in the dock).
The backing size only applies for OpenGL content, but it seems like it should stay the same when minimized also -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9049#note_117450
On Thu Oct 2 19:41:16 2025 +0000, Brendan Shanks wrote:
@rbernon If possible I think we do want to have a non-empty rect for minimized windows, since they keep displaying when minimized (just shrunk down in the dock). The backing size only applies for OpenGL content, but it seems like it should stay the same when minimized also Sure, that seems reasonable enough but I don't think we want to try reversing the computation that win32u might have done in the drivers, it seems unreliable. I think it'd be better to keep track of the normal (non-minimized) rects, possibly passing those to the drivers instead of window/client rects in the window_rects struct, when windows are minimized, as it doesn't seem very useful to pass empty rects anyway.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9049#note_117457
participants (4)
-
Brendan Shanks (＠bshanks) -
Rémi Bernon
-
Tim Clem
-
Tim Clem (＠tclem)