[PATCH v3 0/2] MR7780: win32u: Avoid uneeded (and sometimes harmful) RedrawWindow() calls in expose_window_surface().
-- v3: win32u: Don't inflate rect in expose_window_surface(). win32u: Don't redraw window in expose_window_surface() if window has surface. https://gitlab.winehq.org/wine/wine/-/merge_requests/7780
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/win32u/window.c | 58 ++++++++++++-------------------------------- 1 file changed, 15 insertions(+), 43 deletions(-) diff --git a/dlls/win32u/window.c b/dlls/win32u/window.c index 33dc0987b34..ba931163272 100644 --- a/dlls/win32u/window.c +++ b/dlls/win32u/window.c @@ -2202,36 +2202,11 @@ static BOOL apply_window_pos( HWND hwnd, HWND insert_after, UINT swp_flags, stru return ret; } -static HRGN expose_window_surface_rect( struct window_surface *surface, UINT flags, RECT dirty ) -{ - HRGN region, clipped; - - intersect_rect( &dirty, &dirty, &surface->rect ); - add_bounds_rect( &surface->bounds, &dirty ); - - if (!surface->clip_region || !flags) return 0; - - clipped = NtGdiCreateRectRgn( surface->rect.left, surface->rect.top, - surface->rect.right, surface->rect.bottom ); - NtGdiCombineRgn( clipped, clipped, surface->clip_region, RGN_DIFF ); - - region = NtGdiCreateRectRgn( dirty.left, dirty.top, dirty.right, dirty.bottom ); - if (NtGdiCombineRgn( region, region, clipped, RGN_DIFF ) <= NULLREGION) - { - NtGdiDeleteObjectApp( region ); - region = 0; - } - - NtGdiDeleteObjectApp( clipped ); - return region; -} - static BOOL expose_window_surface( HWND hwnd, UINT flags, const RECT *rect, UINT dpi ) { struct window_surface *surface; struct window_rects rects; RECT window_rect; - HRGN region = 0; WND *win; if (!(win = get_win_ptr( hwnd )) || win == WND_DESKTOP || win == WND_OTHER_PROCESS) return FALSE; @@ -2245,27 +2220,24 @@ static BOOL expose_window_surface( HWND hwnd, UINT flags, const RECT *rect, UINT InflateRect( &window_rect, 1, 1 ); /* compensate rounding errors */ } - if (surface) + if (!surface || surface == &dummy_surface) { - window_surface_lock( surface ); - - if (!rect) add_bounds_rect( &surface->bounds, &surface->rect ); - else - { - RECT dirty = window_rect; - OffsetRect( &dirty, rects.client.left - rects.visible.left, rects.client.top - rects.visible.top ); - if (!(region = expose_window_surface_rect( surface, flags, dirty ))) flags = 0; - else NtGdiOffsetRgn( region, rects.client.left - rects.visible.left, rects.client.top - rects.visible.top ); - } - - window_surface_unlock( surface ); - if (surface->alpha_mask) window_surface_flush( surface ); - - window_surface_release( surface ); + NtUserRedrawWindow( hwnd, rect ? &window_rect : NULL, NULL, flags ); + if (surface) window_surface_release( surface ); + return TRUE; } - if (flags) NtUserRedrawWindow( hwnd, rect ? &window_rect : NULL, region, flags ); - if (region) NtGdiDeleteObjectApp( region ); + window_surface_lock( surface ); + if (!rect) add_bounds_rect( &surface->bounds, &surface->rect ); + else + { + OffsetRect( &window_rect, rects.client.left - rects.visible.left, rects.client.top - rects.visible.top ); + intersect_rect( &window_rect, &window_rect, &surface->rect ); + add_bounds_rect( &surface->bounds, &window_rect ); + } + window_surface_unlock( surface ); + if (surface->alpha_mask) window_surface_flush( surface ); + window_surface_release( surface ); return TRUE; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7780
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/win32u/window.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/dlls/win32u/window.c b/dlls/win32u/window.c index ba931163272..94a2a638a2d 100644 --- a/dlls/win32u/window.c +++ b/dlls/win32u/window.c @@ -2214,11 +2214,7 @@ static BOOL expose_window_surface( HWND hwnd, UINT flags, const RECT *rect, UINT rects = win->rects; release_win_ptr( win ); - if (rect) - { - window_rect = map_dpi_rect( *rect, dpi, get_dpi_for_window( hwnd ) ); - InflateRect( &window_rect, 1, 1 ); /* compensate rounding errors */ - } + if (rect) window_rect = map_dpi_rect( *rect, dpi, get_dpi_for_window( hwnd ) ); if (!surface || surface == &dummy_surface) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7780
v3: - don't inflate rect unconditionally. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7780#note_102249
Rémi Bernon (@rbernon) commented about dlls/win32u/window.c:
+ if (surface) window_surface_release( surface ); + return TRUE; + }
+ window_surface_lock( surface ); if (!rect) add_bounds_rect( &surface->bounds, &surface->rect ); else { - RECT dirty = window_rect; - OffsetRect( &dirty, rects.client.left - rects.visible.left, rects.client.top - rects.visible.top ); - if (!(region = expose_window_surface_rect( surface, flags, dirty ))) flags = 0; - else NtGdiOffsetRgn( region, rects.client.left - rects.visible.left, rects.client.top - rects.visible.top ); + OffsetRect( &window_rect, rects.client.left - rects.visible.left, rects.client.top - rects.visible.top ); + intersect_rect( &window_rect, &window_rect, &surface->rect ); + add_bounds_rect( &surface->bounds, &window_rect ); } This now redraws the entire window / marks as dirty and flushes the entire surface on every Expose event, I think we should respect the exposed rect. Or drop rect altogether if that's really the intent, but it seems wasteful.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7780#note_102330
On Fri May 2 06:39:45 2025 +0000, Rémi Bernon wrote:
This now redraws the entire window / marks as dirty and flushes the entire surface on every Expose event, I think we should respect the exposed rect. Or drop rect altogether if that's really the intent, but it seems wasteful. But window_rect comes from input rect (which is the Expose event rect). And this is the rect which is used on RedrawWindow path or intersected with surface rect on surface path, so in both cases it is supposed to update only the expected rect. There is special path for NULL rect (which obviously assumes the whole window). Am I missing something?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7780#note_102364
On Fri May 2 15:43:27 2025 +0000, Paul Gofman wrote:
But window_rect comes from input rect (which is the Expose event rect). And this is the rect which is used on RedrawWindow path or intersected with surface rect on surface path, so in both cases it is supposed to update only the expected rect. There is special path for NULL rect (which obviously assumes the whole window). Am I missing something? Maybe it is window_rect name which is misleading (while this is pre-existing, it is the same before the patch). Might probably rename it to mapped_rect while we are here?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7780#note_102365
On Fri May 2 15:49:37 2025 +0000, Paul Gofman wrote:
Maybe it is window_rect name which is misleading (while this is pre-existing, it is the same before the patch). Might probably rename it to mapped_rect while we are here? Ah right, I misread indeed and thought window_rect was the usual one. Let's rename it to "exposed_rect" for instance.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7780#note_102366
participants (3)
-
Paul Gofman -
Paul Gofman (＠gofman)
-
Rémi Bernon