[PATCH 0/2] MR7780: win32u: Avoid uneeded (and sometimes harmful) RedrawWindow() calls in expose_window_surface().
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/win32u/window.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/dlls/win32u/window.c b/dlls/win32u/window.c index 15145667ce1..9e5099f05ee 100644 --- a/dlls/win32u/window.c +++ b/dlls/win32u/window.c @@ -2204,25 +2204,19 @@ static BOOL apply_window_pos( HWND hwnd, HWND insert_after, UINT swp_flags, stru static HRGN expose_window_surface_rect( struct window_surface *surface, UINT flags, RECT dirty ) { - HRGN region, clipped; + HRGN region; 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) + if (NtGdiCombineRgn( region, region, surface->clip_region, RGN_DIFF ) <= NULLREGION) { NtGdiDeleteObjectApp( region ); region = 0; } - - NtGdiDeleteObjectApp( clipped ); return region; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7780
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/win32u/window.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/win32u/window.c b/dlls/win32u/window.c index 9e5099f05ee..69e82713520 100644 --- a/dlls/win32u/window.c +++ b/dlls/win32u/window.c @@ -2226,6 +2226,7 @@ static BOOL expose_window_surface( HWND hwnd, UINT flags, const RECT *rect, UINT struct window_rects rects; RECT window_rect; HRGN region = 0; + UINT win_dpi; WND *win; if (!(win = get_win_ptr( hwnd )) || win == WND_DESKTOP || win == WND_OTHER_PROCESS) return FALSE; @@ -2235,8 +2236,12 @@ static BOOL expose_window_surface( HWND hwnd, UINT flags, const RECT *rect, UINT if (rect) { - window_rect = map_dpi_rect( *rect, dpi, get_dpi_for_window( hwnd ) ); - InflateRect( &window_rect, 1, 1 ); /* compensate rounding errors */ + if ((win_dpi = get_dpi_for_window( hwnd )) != dpi) + { + window_rect = map_dpi_rect( *rect, dpi, win_dpi ); + InflateRect( &window_rect, 1, 1 ); /* compensate rounding errors */ + } + else window_rect = *rect; } if (surface) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7780
This MR fixes a regression in Fallout New Vegas launcher which starts up without text / buttons now. The launcher in question creates a dialog with all the contents being shown through static controls. The dialog with controls is intially shown fine but gets quickly repainted upon receiving X11 Expose events with RedrawWindow( ..., RDW_INVALIDATE | RDW_FRAME | RDW_ALLCHILDREN ). The redraw sequence in this case is such that leads to the background image control being painted over the text / buttons. That is also the case on Windows with this dialog for such a RedrawWindow call. I used the following program to confirm this: ``` #include <windows.h> #include <stdio.h> #define DIALOG_CLASS_ATOM MAKEINTATOM(32770) /* Dialog */ int main(int argc, char *argv[]) { HWND w; w = FindWindowW((WCHAR *)DIALOG_CLASS_ATOM, NULL); printf("w %p.\n", w); RedrawWindow(w, NULL, NULL, RDW_INVALIDATE | RDW_FRAME | RDW_ALLCHILDREN); return 0; } ``` Redrawing window in such a way leads to the same state on Windows with the text controls being overwritten in background. So it is not interesting what is wrong with the dialog itself, same RedrawWindow() behaves the same for it on Wine and Windows. As noted by Rémi on MR https://gitlab.winehq.org/wine/wine/-/merge_requests/7157: ``` This regressed since the window surface clipping region logic changed, as the expose event handling was previously not calling NtUserRedrawWindow on windows with a surface and without a clipping region. The clipping region was also previously not always set, or set later, and we're setting it more consistently since 51b16963, even when it matches the window rect, for compatibility with some old wineandroid logic (where I believe it was used to clip window surfaces to their proper sizes). ``` The same relates here, that change is also responsible for the regression here although is not fixed by the referenced MR. The idea of the region math in winex11.drv:expose_surface() before those changes was that the surface region is subtracted from "to be exposed" region. So resulting region has only the area which is to be exposed but is *not* covered by the surface. Then, if the resulting region is empty, no RedrawWindows happens at all. Now this logic is inverted: the present expose_window_surface_rect() instead returns the intersection of the surface region with the "to be exposed" region. In both old and new variant "to be exposed" rect is added to surface bounds so consequent surface flush will update the exposed rect on screen. There is no reason to relay window redraw event to the app if we have the data to expose in the surface. X11 Expose event doesn't correspond to what happens on Windows, obscuring the window and showing at back doesn't cause window regions invalidation and RDW_ALLCHILDREN redraw as we do now, and the cases when X11 will issue Expose event is undefined and doesn't correspond to anything happening on Windows. So avoiding app-visible repaint when we have the exposed region fully covered by window surface (as it was before and is going to be again the case with my MR) looks correct. There are the cases when we don't have surface or have some regions excluded from it (happens with pixel format set on child windows), in that cases RedrawWindow will still be issued (probably as poor man's fallback when we are currently left without window surface). Patch 2 is not strictly needed for the concerned game launcher, but the rect inflation still results in RedrawWindow with the region containing that extra one pixel, which is less harmful after patch 1 but probably still best avoided. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7780#note_100516
The idea of the region math in winex11.drv:expose_surface() before those changes was that the surface region is subtracted from "to be exposed" region. So resulting region has only the area which is to be exposed but is _not_ covered by the surface.
I don't think this is right or doing anything useful. There is never any part of the window that isn't covered by the surface. And even if there was, redrawing pixels outside of the surface (region) is pointless as they have nowhere to go. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7780#note_100541
participants (3)
-
Paul Gofman -
Paul Gofman (＠gofman)
-
Rémi Bernon