[PATCH v3 0/2] MR7780: win32u: Avoid uneeded (and sometimes harmful) RedrawWindow() calls in expose_window_surface().

[Paul Gofman (＠gofman)]

--
  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

[PATCH v3 1/2] win32u: Don't redraw window in expose_window_surface() if window has surface.

[Paul Gofman]

From: Paul Gofman

---
 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;
 }

[PATCH v3 2/2] win32u: Don't inflate rect in expose_window_surface().

[Paul Gofman]

From: Paul Gofman

---
 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)
     {

[Paul Gofman (＠gofman)]

v3:
 - don't inflate rect unconditionally.

[Rémi Bernon (＠rbernon)]

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.

[Paul Gofman (＠gofman)]

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?

[Paul Gofman (＠gofman)]

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?

[Rémi Bernon (＠rbernon)]

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.