backport x11/sdl2 fixes

4b1378f

X11: fix size/position (test video_setWindowCenteredOnDisplay)

this fix x11 backend to correctly pass video_setWindowCenteredOnDisplay()

    get border values early (eg status bar)
    wait for size/position change to get valid values

d4d26e0

testautomation_video: if SDL_SetWindowSize/Position isn't honored, we should check there is an event
x11: send the events if various occasions

src/video/x11/SDL_x11events.c

@@ -755,6 +755,31 @@ static int XLookupStringAsUTF8(XKeyEvent *event_struct, char *buffer_return, int
     return result;
 }
 
+void X11_GetBorderValues(void /* SDL_WindowData */ *data_)
+{
+    SDL_WindowData *data = (SDL_WindowData *)data_;
+    SDL_VideoData *videodata = data->videodata;
+    Display *display = videodata->display;
+
+    Atom type;
+    int format;
+    unsigned long nitems, bytes_after;
+    unsigned char *property;
+    if (X11_XGetWindowProperty(display, data->xwindow, videodata->_NET_FRAME_EXTENTS, 0, 16, 0, XA_CARDINAL, &type, &format, &nitems, &bytes_after, &property) == Success) {
+        if (type != None && nitems == 4) {
+            data->border_left = (int)((long *)property)[0];
+            data->border_right = (int)((long *)property)[1];
+            data->border_top = (int)((long *)property)[2];
+            data->border_bottom = (int)((long *)property)[3];
+        }
+        X11_XFree(property);
+
+#ifdef DEBUG_XEVENTS
+        printf("New _NET_FRAME_EXTENTS: left=%d right=%d, top=%d, bottom=%d\n", data->border_left, data->border_right, data->border_top, data->border_bottom);
+#endif
+    }
+}
+
 static void X11_DispatchEvent(_THIS, XEvent *xevent)
 {
     SDL_VideoData *videodata = (SDL_VideoData *)_this->driverdata;
@@ -1490,23 +1515,7 @@ static void X11_DispatchEvent(_THIS, XEvent *xevent)
                right approach, but it seems to work. */
             X11_UpdateKeymap(_this, SDL_TRUE);
         } else if (xevent->xproperty.atom == videodata->_NET_FRAME_EXTENTS) {
-            Atom type;
+            X11_GetBorderValues(data);
-            int format;
-            unsigned long nitems, bytes_after;
-            unsigned char *property;
-            if (X11_XGetWindowProperty(display, data->xwindow, videodata->_NET_FRAME_EXTENTS, 0, 16, 0, XA_CARDINAL, &type, &format, &nitems, &bytes_after, &property) == Success) {
-                if (type != None && nitems == 4) {
-                    data->border_left = (int)((long *)property)[0];
-                    data->border_right = (int)((long *)property)[1];
-                    data->border_top = (int)((long *)property)[2];
-                    data->border_bottom = (int)((long *)property)[3];
-                }
-                X11_XFree(property);
-
-#ifdef DEBUG_XEVENTS
-                printf("New _NET_FRAME_EXTENTS: left=%d right=%d, top=%d, bottom=%d\n", data->border_left, data->border_right, data->border_top, data->border_bottom);
-#endif
-            }
         }
     } break;
 

src/video/x11/SDL_x11events.h

@@ -28,6 +28,7 @@ extern int X11_WaitEventTimeout(_THIS, int timeout);
 extern void X11_SendWakeupEvent(_THIS, SDL_Window *window);
 extern void X11_SuspendScreenSaver(_THIS);
 extern void X11_ReconcileKeyboardState(_THIS);
+extern void X11_GetBorderValues(void /*SDL_WindowData*/ *data);
 
 #endif /* SDL_x11events_h_ */
 

src/video/x11/SDL_x11window.c

@@ -797,6 +797,7 @@ void X11_SetWindowPosition(_THIS, SDL_Window *window)
     Window childReturn, root, parent;
     Window *children;
     XWindowAttributes attrs;
+    int x, y;
     int orig_x, orig_y;
     Uint32 timeout;
 
@@ -816,8 +817,6 @@ void X11_SetWindowPosition(_THIS, SDL_Window *window)
 
     timeout = SDL_GetTicks() + 100;
     while (SDL_TRUE) {
-        int x, y;
-
         caught_x11_error = SDL_FALSE;
         X11_XSync(display, False);
         X11_XGetWindowAttributes(display, data->xwindow, &attrs);
@@ -826,8 +825,6 @@ void X11_SetWindowPosition(_THIS, SDL_Window *window)
 
         if (!caught_x11_error) {
             if ((x != orig_x) || (y != orig_y)) {
-                window->x = x;
-                window->y = y;
                 break; /* window moved, time to go. */
             } else if ((x == window->x) && (y == window->y)) {
                 break; /* we're at the place we wanted to be anyhow, drop out. */
@@ -841,6 +838,11 @@ void X11_SetWindowPosition(_THIS, SDL_Window *window)
         SDL_Delay(10);
     }
 
+    if (!caught_x11_error) {
+        SDL_SendWindowEvent(window, SDL_WINDOWEVENT_MOVED, x, y);
+        SDL_SendWindowEvent(window, SDL_WINDOWEVENT_RESIZED, attrs.width, attrs.height);
+    }
+
     X11_XSetErrorHandler(prev_handler);
     caught_x11_error = SDL_FALSE;
 }
@@ -970,8 +972,6 @@ void X11_SetWindowSize(_THIS, SDL_Window *window)
 
         if (!caught_x11_error) {
             if ((attrs.width != orig_w) || (attrs.height != orig_h)) {
-                window->w = attrs.width;
-                window->h = attrs.height;
                 break; /* window changed, time to go. */
             } else if ((attrs.width == window->w) && (attrs.height == window->h)) {
                 break; /* we're at the place we wanted to be anyhow, drop out. */
@@ -980,15 +980,24 @@ void X11_SetWindowSize(_THIS, SDL_Window *window)
 
         if (SDL_TICKS_PASSED(SDL_GetTicks(), timeout)) {
             /* Timeout occurred and window size didn't change
-             * window manager likely denied the resize. */
+             * window manager likely denied the resize,
-            window->w = orig_w;
+             * or the new size is the same as the existing:
-            window->h = orig_h;
+             * - current width: is 'full width'.
+             * - try to set new width at 'full width + 1', which get truncated to 'full width'.
+             * - new width is/remains 'full width'
+             * So, even if we break here as a timeout, we can send an event, since the requested size isn't the same
+             * as the final size. (even if final size is same as original size).
+             */
             break;
         }
 
         SDL_Delay(10);
     }
 
+    if (!caught_x11_error) {
+        SDL_SendWindowEvent(window, SDL_WINDOWEVENT_RESIZED, attrs.width, attrs.height);
+    }
+
     X11_XSetErrorHandler(prev_handler);
     caught_x11_error = SDL_FALSE;
 }
@@ -1170,6 +1179,11 @@ void X11_ShowWindow(_THIS, SDL_Window *window)
         X11_XSetInputFocus(display, data->xwindow, RevertToNone, CurrentTime);
         X11_XFlush(display);
     }
+
+    /* Get some valid border values, if we haven't them yet */
+    if (data->border_left == 0 && data->border_right == 0 && data->border_top == 0 && data->border_bottom == 0) {
+        X11_GetBorderValues(data);
+    }
 }
 
 void X11_HideWindow(_THIS, SDL_Window *window)
@@ -1305,6 +1319,8 @@ static void X11_SetWindowFullscreenViaWM(_THIS, SDL_Window *window, SDL_VideoDis
     Display *display = data->videodata->display;
     Atom _NET_WM_STATE = data->videodata->_NET_WM_STATE;
     Atom _NET_WM_STATE_FULLSCREEN = data->videodata->_NET_WM_STATE_FULLSCREEN;
+    SDL_bool window_size_changed = SDL_FALSE;
+    int window_position_changed = 0;
 
     if (X11_IsWindowMapped(_this, window)) {
         XEvent e;
@@ -1314,6 +1330,7 @@ static void X11_SetWindowFullscreenViaWM(_THIS, SDL_Window *window, SDL_VideoDis
         Window childReturn, root, parent;
         Window *children;
         XWindowAttributes attrs;
+        int x, y;
         int orig_w, orig_h, orig_x, orig_y;
         Uint64 timeout;
 
@@ -1378,6 +1395,16 @@ static void X11_SetWindowFullscreenViaWM(_THIS, SDL_Window *window, SDL_VideoDis
                            SubstructureNotifyMask | SubstructureRedirectMask, &e);
         }
 
+        if (!fullscreen) {
+            int dest_x = 0, dest_y = 0;
+            dest_x = window->windowed.x - data->border_left;
+            dest_y = window->windowed.y - data->border_top;
+
+            /* Attempt to move the window */
+            X11_XMoveWindow(display, data->xwindow, dest_x, dest_y);
+        }
+
+
         /* Wait a brief time to see if the window manager decided to let this happen.
            If the window changes at all, even to an unexpected value, we break out. */
         X11_XSync(display, False);
@@ -1385,7 +1412,6 @@ static void X11_SetWindowFullscreenViaWM(_THIS, SDL_Window *window, SDL_VideoDis
 
         timeout = SDL_GetTicks64() + 100;
         while (SDL_TRUE) {
-            int x, y;
 
             caught_x11_error = SDL_FALSE;
             X11_XSync(display, False);
@@ -1394,18 +1420,20 @@ static void X11_SetWindowFullscreenViaWM(_THIS, SDL_Window *window, SDL_VideoDis
                                       attrs.x, attrs.y, &x, &y, &childReturn);
 
             if (!caught_x11_error) {
-                SDL_bool window_changed = SDL_FALSE;
                 if ((x != orig_x) || (y != orig_y)) {
-                    SDL_SendWindowEvent(data->window, SDL_WINDOWEVENT_MOVED, x, y);
+                    orig_x = x;
-                    window_changed = SDL_TRUE;
+                    orig_y = y;
+                    window_position_changed += 1;
                 }
 
                 if ((attrs.width != orig_w) || (attrs.height != orig_h)) {
-                    SDL_SendWindowEvent(data->window, SDL_WINDOWEVENT_RESIZED, attrs.width, attrs.height);
+                    orig_w = attrs.width;
-                    window_changed = SDL_TRUE;
+                    orig_h = attrs.height;
+                    window_size_changed = SDL_TRUE;
                 }
 
-                if (window_changed) {
+                /* Wait for at least 2 moves + 1 size changed to have valid values */
+                if (window_position_changed >= 2 && window_size_changed) {
                     break; /* window changed, time to go. */
                 }
             }
@@ -1417,6 +1445,11 @@ static void X11_SetWindowFullscreenViaWM(_THIS, SDL_Window *window, SDL_VideoDis
             SDL_Delay(10);
         }
 
+        if (!caught_x11_error) {
+            SDL_SendWindowEvent(window, SDL_WINDOWEVENT_MOVED, x, y);
+            SDL_SendWindowEvent(window, SDL_WINDOWEVENT_RESIZED, attrs.width, attrs.height);
+        }
+
         X11_XSetErrorHandler(prev_handler);
         caught_x11_error = SDL_FALSE;
     } else {

test/testautomation.c

@@ -45,6 +45,9 @@ int main(int argc, char *argv[])
         return 1;
     }
 
+    /* No need of windows (or update testautomation_mouse.c:mouse_getMouseFocus() */
+    state->num_windows = 0;
+
     /* Parse commandline */
     for (i = 1; i < argc;) {
         int consumed;

test/testautomation_video.c

@@ -1044,6 +1044,37 @@ int video_getWindowPixelFormat(void *arg)
     return TEST_COMPLETED;
 }
 
+
+static SDL_bool getPositionFromEvent(int *x, int *y)
+{
+    SDL_bool ret = SDL_FALSE;
+    SDL_Event evt;
+    SDL_zero(evt);
+    while (SDL_PollEvent(&evt)) {
+        if (evt.type == SDL_WINDOWEVENT && evt.window.event == SDL_WINDOWEVENT_MOVED) {
+            *x = evt.window.data1;
+            *y = evt.window.data2;
+            ret = SDL_TRUE;
+        }
+    }
+    return ret;
+}
+
+static SDL_bool getSizeFromEvent(int *w, int *h)
+{
+    SDL_bool ret = SDL_FALSE;
+    SDL_Event evt;
+    SDL_zero(evt);
+    while (SDL_PollEvent(&evt)) {
+        if (evt.type == SDL_WINDOWEVENT && evt.window.event == SDL_WINDOWEVENT_RESIZED) {
+            *w = evt.window.data1;
+            *h = evt.window.data2;
+            ret = SDL_TRUE;
+        }
+    }
+    return ret;
+}
+
 /**
  * @brief Tests call to SDL_GetWindowPosition and SDL_SetWindowPosition
  *
@@ -1116,8 +1147,23 @@ int video_getSetWindowPosition(void *arg)
             currentY = desiredY + 1;
             SDL_GetWindowPosition(window, &currentX, &currentY);
             SDLTest_AssertPass("Call to SDL_GetWindowPosition()");
-            SDLTest_AssertCheck(desiredX == currentX, "Verify returned X position; expected: %d, got: %d", desiredX, currentX);
+
-            SDLTest_AssertCheck(desiredY == currentY, "Verify returned Y position; expected: %d, got: %d", desiredY, currentY);
+            if (desiredX == currentX && desiredY == currentY) {
+                SDLTest_AssertCheck(desiredX == currentX, "Verify returned X position; expected: %d, got: %d", desiredX, currentX);
+                SDLTest_AssertCheck(desiredY == currentY, "Verify returned Y position; expected: %d, got: %d", desiredY, currentY);
+            } else {
+                SDL_bool hasEvent;
+                /* SDL_SetWindowPosition() and SDL_SetWindowSize() will make requests of the window manager and set the internal position and size,
+                 * and then we get events signaling what actually happened, and they get passed on to the application if they're not what we expect. */
+                desiredX = currentX + 1;
+                desiredY = currentY + 1;
+                hasEvent = getPositionFromEvent(&desiredX, &desiredY);
+                SDLTest_AssertCheck(hasEvent == SDL_TRUE, "Changing position was not honored by WM, checking present of SDL_WINDOWEVENT_MOVED");
+                if (hasEvent) {
+                    SDLTest_AssertCheck(desiredX == currentX, "Verify returned X position is the position from SDL event; expected: %d, got: %d", desiredX, currentX);
+                    SDLTest_AssertCheck(desiredY == currentY, "Verify returned Y position is the position from SDL event; expected: %d, got: %d", desiredY, currentY);
+                }
+            }
 
             /* Get position X */
             currentX = desiredX + 1;
@@ -1291,8 +1337,24 @@ int video_getSetWindowSize(void *arg)
             currentH = desiredH + 1;
             SDL_GetWindowSize(window, &currentW, &currentH);
             SDLTest_AssertPass("Call to SDL_GetWindowSize()");
-            SDLTest_AssertCheck(desiredW == currentW, "Verify returned width; expected: %d, got: %d", desiredW, currentW);
+
-            SDLTest_AssertCheck(desiredH == currentH, "Verify returned height; expected: %d, got: %d", desiredH, currentH);
+            if (desiredW == currentW && desiredH == currentH) {
+                SDLTest_AssertCheck(desiredW == currentW, "Verify returned width; expected: %d, got: %d", desiredW, currentW);
+                SDLTest_AssertCheck(desiredH == currentH, "Verify returned height; expected: %d, got: %d", desiredH, currentH);
+            } else {
+                SDL_bool hasEvent;
+                /* SDL_SetWindowPosition() and SDL_SetWindowSize() will make requests of the window manager and set the internal position and size,
+                 * and then we get events signaling what actually happened, and they get passed on to the application if they're not what we expect. */
+                desiredW = currentW + 1;
+                desiredH = currentH + 1;
+                hasEvent = getSizeFromEvent(&desiredW, &desiredH);
+                SDLTest_AssertCheck(hasEvent == SDL_TRUE, "Changing size was not honored by WM, checking presence of SDL_WINDOWEVENT_RESIZED");
+                if (hasEvent) {
+                    SDLTest_AssertCheck(desiredW == currentW, "Verify returned width is the one from SDL event; expected: %d, got: %d", desiredW, currentW);
+                    SDLTest_AssertCheck(desiredH == currentH, "Verify returned height is the one from SDL event; expected: %d, got: %d", desiredH, currentH);
+                }
+            }
+
 
             /* Get just width */
             currentW = desiredW + 1;