From 63ae84e140bc145177a6ffc123a0bdd7e7f8ff63 Mon Sep 17 00:00:00 2001 From: Frank Praznik Date: Fri, 8 Dec 2023 12:37:10 -0500 Subject: [PATCH] x11: Improve sync algorithm Track and check move and resize requests separately, and consider them done if either the window is already at the expected location, or at least one configure event which moved or resized the window was processed. The avoids a timeout condition if resizing the window caused it to be implicitly moved in order to keep it within desktop bounds. The automated positioning test now runs on GNOME/X11 without any sync requests timing out. --- src/video/x11/SDL_x11events.c | 13 +++++++++++++ src/video/x11/SDL_x11window.c | 26 +++++++++++++++----------- src/video/x11/SDL_x11window.h | 4 +++- test/testautomation_video.c | 3 +-- 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/video/x11/SDL_x11events.c b/src/video/x11/SDL_x11events.c index 72f6caf1c0..764bfa8e97 100644 --- a/src/video/x11/SDL_x11events.c +++ b/src/video/x11/SDL_x11events.c @@ -1330,6 +1330,7 @@ static void X11_DispatchEvent(SDL_VideoDevice *_this, XEvent *xevent) int x = xevent->xconfigure.x; int y = xevent->xconfigure.y; + data->pending_operation &= ~X11_PENDING_OP_MOVE; SDL_GlobalToRelativeForWindow(data->window, x, y, &x, &y); SDL_SendWindowEvent(data->window, SDL_EVENT_WINDOW_MOVED, x, y); @@ -1349,6 +1350,7 @@ static void X11_DispatchEvent(SDL_VideoDevice *_this, XEvent *xevent) if (xevent->xconfigure.width != data->last_xconfigure.width || xevent->xconfigure.height != data->last_xconfigure.height) { if (!data->skip_size_count) { + data->pending_operation &= ~X11_PENDING_OP_RESIZE; SDL_SendWindowEvent(data->window, SDL_EVENT_WINDOW_RESIZED, xevent->xconfigure.width, xevent->xconfigure.height); @@ -1675,6 +1677,11 @@ static void X11_DispatchEvent(SDL_VideoDevice *_this, XEvent *xevent) /* Restore the last known floating state if leaving maximized mode */ if (!(flags & SDL_WINDOW_FULLSCREEN)) { + data->pending_operation |= X11_PENDING_OP_MOVE | X11_PENDING_OP_RESIZE; + data->expected.x = data->window->floating.x; + data->expected.y = data->window->floating.y; + data->expected.w = data->window->floating.w; + data->expected.h = data->window->floating.h; X11_XMoveWindow(display, data->xwindow, data->window->floating.x, data->window->floating.y); X11_XResizeWindow(display, data->xwindow, data->window->floating.w, data->window->floating.h); } @@ -1696,8 +1703,14 @@ static void X11_DispatchEvent(SDL_VideoDevice *_this, XEvent *xevent) if (data->border_top != 0 || data->border_left != 0 || data->border_right != 0 || data->border_bottom != 0) { /* Adjust if the window size changed to accommodate the borders. */ if (data->window->flags & SDL_WINDOW_MAXIMIZED) { + data->pending_operation |= X11_PENDING_OP_RESIZE; + data->expected.w = data->window->windowed.w; + data->expected.h = data->window->windowed.h; X11_XResizeWindow(display, data->xwindow, data->window->windowed.w, data->window->windowed.h); } else { + data->pending_operation |= X11_PENDING_OP_RESIZE; + data->expected.w = data->window->floating.w; + data->expected.h = data->window->floating.h; X11_XResizeWindow(display, data->xwindow, data->window->floating.w, data->window->floating.h); } } diff --git a/src/video/x11/SDL_x11window.c b/src/video/x11/SDL_x11window.c index 67a0c877c2..5a2de89d94 100644 --- a/src/video/x11/SDL_x11window.c +++ b/src/video/x11/SDL_x11window.c @@ -614,12 +614,12 @@ int X11_CreateWindow(SDL_VideoDevice *_this, SDL_Window *window, SDL_PropertiesI window->floating.x, window->floating.y, &win_x, &win_y); - /* Always create this with the window->windowed.* fields; if we're - creating a windowed mode window, that's fine. If we're creating a - fullscreen window, the window manager will want to know these values - so it can use them if we go _back_ to windowed mode. SDL manages - migration to fullscreen after CreateSDLWindow returns, which will - put all the SDL_Window fields and system state as expected. */ + /* Always create this with the window->floating.* fields; if we're creating a windowed mode window, + * that's fine. If we're creating a maximized or fullscreen window, the window manager will want to + * know these values so it can use them if we go _back_ to the base floating windowed mode. SDL manages + * migration to fullscreen after CreateSDLWindow returns, which will put all the SDL_Window fields and + * system state as expected. + */ w = X11_XCreateWindow(display, RootWindow(display, screen), win_x, win_y, window->floating.w, window->floating.h, 0, depth, InputOutput, visual, @@ -868,9 +868,9 @@ static int X11_SyncWindowTimeout(SDL_VideoDevice *_this, SDL_Window *window, int X11_XSync(display, False); X11_PumpEvents(_this); - if (window->x == data->expected.x && window->y == data->expected.y && - window->w == data->expected.w && window->h == data->expected.h && - data->pending_operation == X11_PENDING_OP_NONE) { + if ((!(data->pending_operation & X11_PENDING_OP_MOVE) || (window->x == data->expected.x && window->y == data->expected.y)) && + (!(data->pending_operation & X11_PENDING_OP_RESIZE) || (window->w == data->expected.w && window->h == data->expected.h)) && + (data->pending_operation & ~(X11_PENDING_OP_MOVE | X11_PENDING_OP_RESIZE)) == X11_PENDING_OP_NONE) { /* The window is where it is wanted and nothing is pending. Done. */ break; } @@ -882,8 +882,6 @@ static int X11_SyncWindowTimeout(SDL_VideoDevice *_this, SDL_Window *window, int data->expected.w = window->w; data->expected.h = window->h; - data->pending_operation = X11_PENDING_OP_NONE; - ret = 1; break; } @@ -891,6 +889,8 @@ static int X11_SyncWindowTimeout(SDL_VideoDevice *_this, SDL_Window *window, int SDL_Delay(10); } + data->pending_operation = X11_PENDING_OP_NONE; + if (!caught_x11_error) { X11_PumpEvents(_this); } else { @@ -973,6 +973,7 @@ void X11_UpdateWindowPosition(SDL_Window *window, SDL_bool use_current_position) &data->expected.x, &data->expected.y); /* Attempt to move the window */ + data->pending_operation |= X11_PENDING_OP_MOVE; X11_XMoveWindow(display, data->xwindow, data->expected.x, data->expected.y); } @@ -1116,6 +1117,7 @@ void X11_SetWindowSize(SDL_VideoDevice *_this, SDL_Window *window) } else { data->expected.w = window->floating.w; data->expected.h = window->floating.h; + data->pending_operation |= X11_PENDING_OP_RESIZE; X11_XResizeWindow(display, data->xwindow, data->expected.w, data->expected.h); } } @@ -1598,10 +1600,12 @@ static int X11_SetWindowFullscreenViaWM(SDL_VideoDevice *_this, SDL_Window *wind data->expected.w = window->floating.w; data->expected.h = window->floating.h; + data->pending_operation |= X11_PENDING_OP_MOVE; X11_XMoveWindow(display, data->xwindow, data->expected.x, data->expected.y); /* If the window is bordered, the size will be set when the borders turn themselves back on. */ if (window->flags & SDL_WINDOW_BORDERLESS) { + data->pending_operation |= X11_PENDING_OP_RESIZE; X11_XResizeWindow(display, data->xwindow, data->expected.w, data->expected.h); } } diff --git a/src/video/x11/SDL_x11window.h b/src/video/x11/SDL_x11window.h index 03be883c7c..11977a57b9 100644 --- a/src/video/x11/SDL_x11window.h +++ b/src/video/x11/SDL_x11window.h @@ -88,7 +88,9 @@ struct SDL_WindowData X11_PENDING_OP_RESTORE = 0x01, X11_PENDING_OP_MINIMIZE = 0x02, X11_PENDING_OP_MAXIMIZE = 0x04, - X11_PENDING_OP_FULLSCREEN = 0x08 + X11_PENDING_OP_FULLSCREEN = 0x08, + X11_PENDING_OP_MOVE = 0x10, + X11_PENDING_OP_RESIZE = 0x20 } pending_operation; SDL_bool initial_border_adjustment; diff --git a/test/testautomation_video.c b/test/testautomation_video.c index 46ba4e3743..7a3c95d776 100644 --- a/test/testautomation_video.c +++ b/test/testautomation_video.c @@ -1134,10 +1134,9 @@ static int video_getSetWindowSize(void *arg) SDL_SetWindowSize(window, desiredW, desiredH); SDLTest_AssertPass("Call to SDL_SetWindowSize(...,%d,%d)", desiredW, desiredH); - /* The sync may time out if changing the size changes the window position. */ result = SDL_SyncWindow(window); SDLTest_AssertPass("SDL_SyncWindow()"); - SDLTest_AssertCheck(result >= 0, "Verify return value; expected: >=0, got: %d", result); + SDLTest_AssertCheck(result == 0, "Verify return value; expected: 0, got: %d", result); /* Get size */ currentW = desiredW + 1;