[PATCH v4 0/2] MR6569: Draft: winex11: Keep a list of pending window config requests.
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. -- v4: winex11: Also discard near-matching configs with similar sizes. winex11: Keep a list of pending window config requests. https://gitlab.winehq.org/wine/wine/-/merge_requests/6569
From: Rémi Bernon <rbernon(a)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 | 93 +++++++++++++++++++++++++++++++++++++-- dlls/winex11.drv/x11drv.h | 11 ++++- 3 files changed, 105 insertions(+), 12 deletions(-) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index c8fc76f5048..8f4a80bcc7e 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 (has_pending_window_config( data, &rect, event->serial )) + { + TRACE( "Found newer/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..40abbde74a1 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,77 @@ 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, unsigned long serial ) +{ + struct config_entry *entry, *next; + UINT ticks = NtGetTickCount(); + + LIST_FOR_EACH_ENTRY_SAFE( entry, next, &data->pending_configs, struct config_entry, entry ) + { + /* discard all events, or any event with older serial, or queued more than 1s ago */ + if (!all && entry->serial >= serial && 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 or supersedes the received event rect / serial */ +BOOL has_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; + struct list *ptr; + + TRACE( "window %p/%lx, visible_rect %s, serial %ld, ticks %u\n", data->hwnd, data->whole_window, + wine_dbgstr_rect(visible_rect), serial, ticks ); + + if (!(ptr = list_head( &data->pending_configs ))) return FALSE; + entry = LIST_ENTRY( ptr, struct config_entry, entry ); + if (entry->serial > serial) return TRUE; /* ignore any serial older than our earliest queued event */ + + /* lookup for a matching pending window config */ + LIST_FOR_EACH_ENTRY( entry, &data->pending_configs, struct config_entry, entry ) + { + if (EqualRect( &entry->visible_rect, visible_rect )) exact = entry; + if (exact) break; + } + + /* discard every event before the matching one */ + 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; + } + + if (exact) discard_pending_configs( data, FALSE, serial ); + 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, 0 ); + + 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 +1318,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 +1430,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 +1924,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, 0 ); free( data ); destroy_gl_drawable( hwnd ); } @@ -2675,9 +2750,15 @@ void X11DRV_WindowPosChanged( HWND hwnd, HWND insert_after, UINT swp_flags, BOOL data->iconic = (new_style & WS_MINIMIZE) != 0; TRACE( "changing win %p iconic state to %u\n", data->hwnd, data->iconic ); if (data->iconic) + { + queue_pending_config( data, &data->rects.visible, NextRequest( data->display ) ); XIconifyWindow( data->display, data->whole_window, data->vis.screen ); + } else if (is_window_rect_mapped( &new_rects->window )) + { + queue_pending_config( data, &data->rects.visible, NextRequest( data->display ) ); XMapWindow( data->display, data->whole_window ); + } update_net_wm_states( data ); } else @@ -2936,7 +3017,11 @@ LRESULT X11DRV_WindowMessage( HWND hwnd, UINT msg, WPARAM wp, LPARAM lp ) if (old_pos.x != pos.x) mask |= CWX; if (old_pos.y != pos.y) mask |= CWY; - if (mask) XReconfigureWMWindow( data->display, data->whole_window, data->vis.screen, mask, &changes ); + if (mask) + { + if (data->mapped) queue_pending_config( data, &data->rects.visible, NextRequest( data->display ) ); + XReconfigureWMWindow( data->display, data->whole_window, data->vis.screen, mask, &changes ); + } } release_win_data( data ); diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index d2eb35b454e..cb4acdbbf27 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 has_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 ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6569
From: Rémi Bernon <rbernon(a)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, 12 insertions(+), 1 deletion(-) diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index 40abbde74a1..33c775b4e7c 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1243,7 +1243,8 @@ static void discard_pending_configs( struct x11drv_win_data *data, BOOL all, uns /* lookup a window config request that matches or supersedes the received event rect / serial */ BOOL has_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; struct list *ptr; @@ -1256,15 +1257,25 @@ BOOL has_pending_window_config( struct x11drv_win_data *data, const RECT *visibl if (entry->serial > serial) return TRUE; /* ignore any serial older than our earliest queued event */ /* 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( entry, &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; + else if (exact) near = NULL; + /* discard every event before the matching one */ LIST_FOR_EACH_ENTRY_SAFE( entry, next, &data->pending_configs, struct config_entry, entry ) { + if ((found = (entry == near))) break; /* keep near matches in case we can later match them exactly */ 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 ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6569
participants (1)
-
Rémi Bernon