Instead of a single configure serial, which is simply not enough to mitigate event feedback loops.
This should (hopefully) address the recent d3d8:device / d3d9:device test failures on the Gitlab CI, without having to add more `flush_events` calls to the tests.
I believe it should also provide a more general fix to this race condition than the configure_serial mechanism.
From: Rémi Bernon rbernon@codeweavers.com
Instead of a single configure serial, which is simply not enough to mitigate event feedback loops. --- dlls/winex11.drv/event.c | 13 ++++--- dlls/winex11.drv/window.c | 75 +++++++++++++++++++++++++++++++++++++-- dlls/winex11.drv/x11drv.h | 11 +++++- 3 files changed, 88 insertions(+), 11 deletions(-)
diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index c8fc76f5048..3f354402b3a 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -1043,13 +1043,6 @@ static BOOL X11DRV_ConfigureNotify( HWND hwnd, XEvent *xev ) if (data->whole_window && !data->managed) goto done; /* ignore synthetic events on foreign windows */ if (event->send_event && !data->whole_window) goto done; - if (data->configure_serial && (long)(data->configure_serial - event->serial) > 0) - { - TRACE( "win %p/%lx event %d,%d,%dx%d ignoring old serial %lu/%lu\n", - hwnd, data->whole_window, event->x, event->y, event->width, event->height, - event->serial, data->configure_serial ); - goto done; - }
/* Get geometry */
@@ -1072,6 +1065,12 @@ static BOOL X11DRV_ConfigureNotify( HWND hwnd, XEvent *xev ) else pos = root_to_virtual_screen( x, y );
SetRect( &rect, pos.x, pos.y, pos.x + event->width, pos.y + event->height ); + if (find_pending_window_config( data, &rect, event->serial )) + { + TRACE( "Found matching pending config, ignoring event\n" ); + goto done; + } + rect = window_rect_from_visible( &data->rects, rect ); if (root_coords) NtUserMapWindowPoints( 0, parent, (POINT *)&rect, 2, 0 /* per-monitor DPI */ );
diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index f18ab302e5e..2325171aa6b 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -226,6 +226,7 @@ static struct x11drv_win_data *alloc_win_data( Display *display, HWND hwnd ) data->display = display; data->vis = default_visual; data->hwnd = hwnd; + list_init( &data->pending_configs ); pthread_mutex_lock( &win_data_mutex ); XSaveContext( gdi_display, (XID)hwnd, win_data_context, (char *)data ); } @@ -1222,6 +1223,71 @@ static void set_xembed_flags( struct x11drv_win_data *data, unsigned long flags x11drv_atom(_XEMBED_INFO), 32, PropModeReplace, (unsigned char*)info, 2 ); }
+/* discard old/all pending window config requests */ +static void discard_pending_configs( struct x11drv_win_data *data, BOOL all ) +{ + struct config_entry *entry, *next; + UINT ticks = NtGetTickCount(); + + LIST_FOR_EACH_ENTRY_SAFE( entry, next, &data->pending_configs, struct config_entry, entry ) + { + if (all || abs( (int)ticks - (int)entry->ticks ) < 1000) break; + WARN( "Discarding old window %p/%lx entry %p, visible_rect %s, serial %ld, ticks %u\n", data->hwnd, + data->whole_window, entry, wine_dbgstr_rect(&entry->visible_rect), entry->serial, entry->ticks); + list_remove( &entry->entry ); + free( entry ); + } +} + +/* lookup a window config request that matches the received event rect */ +BOOL find_pending_window_config( struct x11drv_win_data *data, const RECT *visible_rect, unsigned long serial ) +{ + struct config_entry *entry, *next, *exact = NULL; + UINT ticks = NtGetTickCount(); + BOOL found = FALSE; + + TRACE( "window %p/%lx, visible_rect %s, serial %ld, ticks %u\n", data->hwnd, data->whole_window, + wine_dbgstr_rect(visible_rect), serial, ticks ); + + discard_pending_configs( data, FALSE ); + + /* lookup for a matching pending window config */ + LIST_FOR_EACH_ENTRY_SAFE( entry, next, &data->pending_configs, struct config_entry, entry ) + { + if (EqualRect( &entry->visible_rect, visible_rect )) exact = entry; + if (exact) break; + } + + LIST_FOR_EACH_ENTRY_SAFE( entry, next, &data->pending_configs, struct config_entry, entry ) + { + found = entry == exact; + if (!found) WARN( "Discarding window %p/%lx entry %p, visible_rect %s, serial %ld, ticks %u\n", data->hwnd, + data->whole_window, entry, wine_dbgstr_rect(&entry->visible_rect), entry->serial, entry->ticks); + list_remove( &entry->entry ); + free( entry ); + if (found) break; + } + + return found; +} + +/* add a pending window config request with the given visible rect */ +static void queue_pending_config( struct x11drv_win_data *data, const RECT *visible_rect, unsigned long serial ) +{ + UINT ticks = NtGetTickCount(); + struct config_entry *entry; + + discard_pending_configs( data, FALSE ); + + if (!(entry = calloc( 1, sizeof(*entry) ))) return; + entry->visible_rect = *visible_rect; + entry->serial = serial; + entry->ticks = ticks; + list_add_tail( &data->pending_configs, &entry->entry ); + + TRACE( "Queued window %p/%lx entry %p, visible_rect %s, serial %ld, ticks %u\n", data->hwnd, + data->whole_window, entry, wine_dbgstr_rect(&entry->visible_rect), entry->serial, entry->ticks); +}
/*********************************************************************** * map_window @@ -1246,6 +1312,7 @@ static void map_window( HWND hwnd, DWORD new_style ) { update_net_wm_states( data ); sync_window_style( data ); + queue_pending_config( data, &data->rects.visible, NextRequest( data->display ) ); XMapWindow( data->display, data->whole_window ); XFlush( data->display ); } @@ -1357,14 +1424,15 @@ static void sync_window_position( struct x11drv_win_data *data, UINT swp_flags ) set_size_hints( data, style ); set_mwm_hints( data, style, ex_style ); update_net_wm_states( data ); - data->configure_serial = NextRequest( data->display ); + + if (data->mapped) queue_pending_config( data, &data->rects.visible, NextRequest( data->display ) ); XReconfigureWMWindow( data->display, data->whole_window, data->vis.screen, mask, &changes );
- TRACE( "win %p/%lx pos %d,%d,%dx%d after %lx changes=%x serial=%lu\n", + TRACE( "win %p/%lx pos %d,%d,%dx%d after %lx changes=%x\n", data->hwnd, data->whole_window, (int)data->rects.visible.left, (int)data->rects.visible.top, (int)(data->rects.visible.right - data->rects.visible.left), (int)(data->rects.visible.bottom - data->rects.visible.top), - changes.sibling, mask, data->configure_serial ); + changes.sibling, mask ); }
@@ -1850,6 +1918,7 @@ void X11DRV_DestroyWindow( HWND hwnd ) free( data->icon_bits ); XDeleteContext( gdi_display, (XID)hwnd, win_data_context ); release_win_data( data ); + discard_pending_configs( data, TRUE ); free( data ); destroy_gl_drawable( hwnd ); } diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index d2eb35b454e..9ff56203876 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -591,6 +591,14 @@ enum x11drv_net_wm_state NB_NET_WM_STATES };
+struct config_entry +{ + struct list entry; + unsigned long serial; + RECT visible_rect; + UINT ticks; +}; + /* x11drv private window data */ struct x11drv_win_data { @@ -616,7 +624,7 @@ struct x11drv_win_data int wm_state; /* current value of the WM_STATE property */ DWORD net_wm_state; /* bit mask of active x11drv_net_wm_state values */ Window embedder; /* window id of embedder */ - unsigned long configure_serial; /* serial number of last configure request */ + struct list pending_configs; /* list of pending struct config_entry */ Pixmap icon_pixmap; Pixmap icon_mask; unsigned long *icon_bits; @@ -627,6 +635,7 @@ extern struct x11drv_win_data *get_win_data( HWND hwnd ); extern void release_win_data( struct x11drv_win_data *data ); extern Window X11DRV_get_whole_window( HWND hwnd ); extern Window get_dummy_parent(void); +extern BOOL find_pending_window_config( struct x11drv_win_data *data, const RECT *visible_rect, unsigned long serial );
extern void sync_gl_drawable( HWND hwnd, BOOL known_child ); extern void set_gl_drawable_parent( HWND hwnd, HWND parent );
From: Rémi Bernon rbernon@codeweavers.com
When more reconfigurations are known to be pending, and no exact match is found. --- dlls/winex11.drv/window.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index 2325171aa6b..5ba84acab0c 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1242,7 +1242,8 @@ static void discard_pending_configs( struct x11drv_win_data *data, BOOL all ) /* lookup a window config request that matches the received event rect */ BOOL find_pending_window_config( struct x11drv_win_data *data, const RECT *visible_rect, unsigned long serial ) { - struct config_entry *entry, *next, *exact = NULL; + struct config_entry *entry, *next, *exact = NULL, *near = NULL; + SIZE current_size, config_size; UINT ticks = NtGetTickCount(); BOOL found = FALSE;
@@ -1252,15 +1253,23 @@ BOOL find_pending_window_config( struct x11drv_win_data *data, const RECT *visib discard_pending_configs( data, FALSE );
/* lookup for a matching pending window config */ + current_size.cx = visible_rect->right - visible_rect->left; + current_size.cy = visible_rect->bottom - visible_rect->top; LIST_FOR_EACH_ENTRY_SAFE( entry, next, &data->pending_configs, struct config_entry, entry ) { + config_size.cx = entry->visible_rect.right - entry->visible_rect.left; + config_size.cy = entry->visible_rect.bottom - entry->visible_rect.top; + if (!near && current_size.cx == config_size.cx && current_size.cy == config_size.cy) near = entry; if (EqualRect( &entry->visible_rect, visible_rect )) exact = entry; if (exact) break; }
+ /* only match with near configs if we have more pending configs */ + if (near && !list_next( &data->pending_configs, &near->entry )) near = NULL; + LIST_FOR_EACH_ENTRY_SAFE( entry, next, &data->pending_configs, struct config_entry, entry ) { - found = entry == exact; + found = entry == exact || entry == near; if (!found) WARN( "Discarding window %p/%lx entry %p, visible_rect %s, serial %ld, ticks %u\n", data->hwnd, data->whole_window, entry, wine_dbgstr_rect(&entry->visible_rect), entry->serial, entry->ticks); list_remove( &entry->entry );
Marking as draft for now, to see if it actually makes the tests happy (because I wasn't able to reproduce the failures locally).
On Thu Sep 26 12:22:42 2024 +0000, Rémi Bernon wrote:
Marking as draft for now, to see if it actually makes the tests happy (because I wasn't able to reproduce the failures locally).
It probably didn't: ``` device.c:14970: Test failed: Adapter 0: Expect window rect (0,0)-(1024,768), got (4,21)-(644,501). device.c:14991: Test marked todo: Adapter 0: Expect window rect (0,0)-(1024,768), got (4,21)-(643,500). 0664:device: 60540 tests executed (165 marked as todo, 0 as flaky, 1 failure), 10 skipped. d3d9:device:0664 done (1) in 26s 15764B ```
-
Aida Jonikienė (＠DodoGTA) -
Rémi Bernon
-
Rémi Bernon (＠rbernon)