SDL: x11: Improve sync algorithm

From 63ae84e140bc145177a6ffc123a0bdd7e7f8ff63 Mon Sep 17 00:00:00 2001
From: Frank Praznik <[EMAIL REDACTED]>
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 72f6caf1c07a..764bfa8e9738 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 67a0c877c2a5..5a2de89d94cb 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 03be883c7c9a..11977a57b957 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 46ba4e37433c..7a3c95d7766f 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;