Skip to content

Commit

Permalink
Fix X11_SetWindowPosition() edgecase
Browse files Browse the repository at this point in the history
Currently it is not possible to move window with `SDL_WINDOW_BORDERLESS` flag outside of screen bounds using `SDL_SetWindowPosition()` (#3813). Suppose we move such window at the bottom of the screen, and it's bottom border is stops at bottom border of the screen. If we proceed to move such window further out of the screen bounds (still using `SDL_SetWindowPosition()`), window position keeps getting updated in `SDL_SetWindowPosition()`, but position we requested is outside of screen bounds and therefore is not valid. WM forces our window to stay in the same position. When `X11_SetWindowPosition()` executes following happens:
1. `X11_XTranslateCoordinates(&orig_x, &orig_y);` - return position clamped within screen bounds. This is not what we requested in `X11_SetWindowPosition()` and not what `window->x`/`window->y` have currently set.
2. `X11_XMoveWindow();` - move to requested coordinates. However nothing happens because WM prevents moving outside of screen bounds. Window remains in the same location.
3. `X11_XTranslateCoordinates(&x, &y);` - checking if window moved. This call returns same coordinates as in step 1.
4. `if (SDL_TICKS_PASSED(SDL_GetTicks(), timeout))` - eventually check passes and we break out of the loop.

So `SDL_SetWindowPosition()` updated `window->x`/`window->y`, then we failed to move window and kept our requested window position set in `SDL_Window`, making subsequent `SDL_GetWindowPosition()` calls return incorrect results.

At the same time we get 100ms stalls when WM prevents window movement.

Solution: unconditionally update window struct with real window position while removing loop. This change may result in `SDL_GetWindowPosition()` returning different than requested coordinates if called right after `SDL_SetWindowPosition()`, however this is OK. We have no guarantees `SDL_SetWindowPosition()` would place a window at requested position anyhow. If WM continues to move window after `SDL_SetWindowPosition()` - these updates will be taken into account by `ConfigureNotify` event handler.
  • Loading branch information
rokups committed Jun 11, 2021
1 parent e13b43a commit eb0c751
Showing 1 changed file with 11 additions and 30 deletions.
41 changes: 11 additions & 30 deletions src/video/x11/SDL_x11window.c
Original file line number Diff line number Diff line change
Expand Up @@ -796,42 +796,23 @@ X11_SetWindowPosition(_THIS, SDL_Window * window)
Window childReturn, root, parent;
Window* children;
XWindowAttributes attrs;
int orig_x, orig_y;
Uint32 timeout;
int x, y;

/*Attempt to move the window*/
X11_XMoveWindow(display, data->xwindow, window->x - data->border_left, window->y - data->border_top);

/* WM may interfere with window positioning. Obtain actual current position and keep it. We may not get a final
window position here. If window position changes further after this function returns - it will be updated by
ConfigureNotify event handler.
*/
X11_XSync(display, False);
X11_XQueryTree(display, data->xwindow, &root, &parent, &children, &childCount);
X11_XGetWindowAttributes(display, data->xwindow, &attrs);
X11_XTranslateCoordinates(display, parent, DefaultRootWindow(display),
attrs.x, attrs.y, &orig_x, &orig_y, &childReturn);

/*Attempt to move the window*/
X11_XMoveWindow(display, data->xwindow, window->x - data->border_left, window->y - data->border_top);

/* Wait a brief time to see if the window manager decided to let this move happen.
If the window changes at all, even to an unexpected value, we break out. */
timeout = SDL_GetTicks() + 100;
while (SDL_TRUE) {
int x, y;
X11_XSync(display, False);
X11_XGetWindowAttributes(display, data->xwindow, &attrs);
X11_XTranslateCoordinates(display, parent, DefaultRootWindow(display),
attrs.x, attrs.y, &x, &y, &childReturn);

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. */
}
attrs.x, attrs.y, &x, &y, &childReturn);

if (SDL_TICKS_PASSED(SDL_GetTicks(), timeout)) {
break;
}

SDL_Delay(10);
}
window->x = x;
window->y = y;
}

void
Expand Down

0 comments on commit eb0c751

Please sign in to comment.