From ad813a65e791d295351e70b2d1c36b06f3853999 Mon Sep 17 00:00:00 2001
From: Frank Praznik <[EMAIL REDACTED]>
Date: Thu, 11 Apr 2024 16:42:51 -0400
Subject: [PATCH] x11: Fix initial window positioning
Some window managers can send garbage values during the initial mapping of a window, and need the position set again after mapping to ensure proper placement. Position requests sent before mapping can otherwise end up ignored. Ignore initial configure events when initially showing the window, and make sure that the position is set after the window is mapped, either when the window borders appear, or after the initial configure events in the case of borderless windows.
This also eliminates sending excessive/redundant move requests, which can cause strange behavior on some window managers, particularly if done before the window is actually mapped.
Fixes cases of incorrect initial window placement on GNOME + XWayland.
---
src/video/SDL_sysvideo.h | 1 +
src/video/SDL_video.c | 2 +-
src/video/x11/SDL_x11events.c | 65 ++++++++++++++++++-----------------
src/video/x11/SDL_x11window.c | 40 +++++++++++++++++----
4 files changed, 69 insertions(+), 39 deletions(-)
diff --git a/src/video/SDL_sysvideo.h b/src/video/SDL_sysvideo.h
index 36512b8be519e..82b18bbd045bf 100644
--- a/src/video/SDL_sysvideo.h
+++ b/src/video/SDL_sysvideo.h
@@ -498,6 +498,7 @@ extern void SDL_SetDisplayContentScale(SDL_VideoDisplay *display, float scale);
extern void SDL_SetDisplayHDRProperties(SDL_VideoDisplay *display, const SDL_HDRDisplayProperties *HDR);
extern int SDL_SetDisplayModeForDisplay(SDL_VideoDisplay *display, SDL_DisplayMode *mode);
extern SDL_VideoDisplay *SDL_GetVideoDisplay(SDL_DisplayID display);
+extern SDL_DisplayID SDL_GetDisplayForWindowPosition(SDL_Window *window);
extern SDL_VideoDisplay *SDL_GetVideoDisplayForWindow(SDL_Window *window);
extern SDL_VideoDisplay *SDL_GetVideoDisplayForFullscreenWindow(SDL_Window *window);
extern int SDL_GetDisplayIndex(SDL_DisplayID displayID);
diff --git a/src/video/SDL_video.c b/src/video/SDL_video.c
index 6b31345ea735e..8c1503a4fc178 100644
--- a/src/video/SDL_video.c
+++ b/src/video/SDL_video.c
@@ -1388,7 +1388,7 @@ SDL_DisplayID SDL_GetDisplayForRect(const SDL_Rect *rect)
return GetDisplayForRect(rect->x, rect->y, rect->w, rect->h);
}
-static SDL_DisplayID SDL_GetDisplayForWindowPosition(SDL_Window *window)
+SDL_DisplayID SDL_GetDisplayForWindowPosition(SDL_Window *window)
{
int x, y;
SDL_DisplayID displayID = 0;
diff --git a/src/video/x11/SDL_x11events.c b/src/video/x11/SDL_x11events.c
index 6a90d96e39bab..437ac67ea7486 100644
--- a/src/video/x11/SDL_x11events.c
+++ b/src/video/x11/SDL_x11events.c
@@ -1687,8 +1687,7 @@ static void X11_DispatchEvent(SDL_VideoDevice *_this, XEvent *xevent)
if (flags & SDL_WINDOW_FULLSCREEN) {
if (!(flags & SDL_WINDOW_MINIMIZED)) {
- const SDL_bool commit = data->requested_fullscreen_mode.displayID == 0 ||
- SDL_memcmp(&data->window->current_fullscreen_mode, &data->requested_fullscreen_mode, sizeof(SDL_DisplayMode)) != 0;
+ const SDL_bool commit = SDL_memcmp(&data->window->current_fullscreen_mode, &data->requested_fullscreen_mode, sizeof(SDL_DisplayMode)) != 0;
SDL_SendWindowEvent(data->window, SDL_EVENT_WINDOW_ENTER_FULLSCREEN, 0, 0);
if (commit) {
@@ -1705,6 +1704,8 @@ static void X11_DispatchEvent(SDL_VideoDevice *_this, XEvent *xevent)
SDL_SendWindowEvent(data->window, SDL_EVENT_WINDOW_LEAVE_FULLSCREEN, 0, 0);
SDL_UpdateFullscreenMode(data->window, SDL_FALSE, SDL_FALSE);
+ SDL_zero(data->requested_fullscreen_mode);
+
/* Need to restore or update any limits changed while the window was fullscreen. */
X11_SetWindowMinMax(data->window, !!(flags & SDL_WINDOW_MAXIMIZED));
}
@@ -1753,17 +1754,17 @@ static void X11_DispatchEvent(SDL_VideoDevice *_this, XEvent *xevent)
}
if (!(flags & (SDL_WINDOW_MAXIMIZED | SDL_WINDOW_MINIMIZED))) {
data->pending_operation &= ~X11_PENDING_OP_RESTORE;
- SDL_SendWindowEvent(data->window, SDL_EVENT_WINDOW_RESTORED, 0, 0);
-
- /* 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->border_left;
- data->expected.y = data->window->floating.y - data->border_top;
- data->expected.w = data->window->floating.w;
- data->expected.h = data->window->floating.h;
- X11_XMoveWindow(display, data->xwindow, data->window->floating.x - data->border_left, data->window->floating.y - data->border_top);
- X11_XResizeWindow(display, data->xwindow, data->window->floating.w, data->window->floating.h);
+ if (SDL_SendWindowEvent(data->window, SDL_EVENT_WINDOW_RESTORED, 0, 0)) {
+ /* 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->border_left;
+ data->expected.y = data->window->floating.y - data->border_top;
+ data->expected.w = data->window->floating.w;
+ data->expected.h = data->window->floating.h;
+ X11_XMoveWindow(display, data->xwindow, data->window->floating.x - data->border_left, data->window->floating.y - data->border_top);
+ X11_XResizeWindow(display, data->xwindow, data->window->floating.w, data->window->floating.h);
+ }
}
}
if ((flags & SDL_WINDOW_INPUT_FOCUS)) {
@@ -1785,24 +1786,26 @@ static void X11_DispatchEvent(SDL_VideoDevice *_this, XEvent *xevent)
right approach, but it seems to work. */
X11_UpdateKeymap(_this, SDL_TRUE);
} else if (xevent->xproperty.atom == videodata->_NET_FRAME_EXTENTS) {
- /* Re-enable size events if they were turned off waiting for the borders to come back
- * when leaving fullscreen.
- */
- data->disable_size_position_events = SDL_FALSE;
- X11_GetBorderValues(data);
- if (data->border_top != 0 || data->border_left != 0 || data->border_right != 0 || data->border_bottom != 0) {
- /* Adjust if the window size/position 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 | X11_PENDING_OP_MOVE;
- data->expected.w = data->window->floating.w;
- data->expected.h = data->window->floating.h;
- X11_XMoveWindow(display, data->xwindow, data->window->floating.x - data->border_left, data->window->floating.y - data->border_top);
- X11_XResizeWindow(display, data->xwindow, data->window->floating.w, data->window->floating.h);
+ if (data->disable_size_position_events) {
+ /* Re-enable size events if they were turned off waiting for the borders to come back
+ * when leaving fullscreen.
+ */
+ data->disable_size_position_events = SDL_FALSE;
+ X11_GetBorderValues(data);
+ if (data->border_top != 0 || data->border_left != 0 || data->border_right != 0 || data->border_bottom != 0) {
+ /* Adjust if the window size/position 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 | X11_PENDING_OP_MOVE;
+ data->expected.w = data->window->floating.w;
+ data->expected.h = data->window->floating.h;
+ X11_XMoveWindow(display, data->xwindow, data->window->floating.x - data->border_left, data->window->floating.y - data->border_top);
+ X11_XResizeWindow(display, data->xwindow, data->window->floating.w, data->window->floating.h);
+ }
}
}
if (!(data->window->flags & SDL_WINDOW_FULLSCREEN) && data->toggle_borders) {
diff --git a/src/video/x11/SDL_x11window.c b/src/video/x11/SDL_x11window.c
index 8eb0487b00ed3..45436bc42c563 100644
--- a/src/video/x11/SDL_x11window.c
+++ b/src/video/x11/SDL_x11window.c
@@ -895,7 +895,7 @@ static int X11_SyncWindowTimeout(SDL_VideoDevice *_this, SDL_Window *window, int
X11_XSync(display, False);
X11_PumpEvents(_this);
- if ((data->pending_operation & X11_PENDING_OP_MOVE) && (window->x == data->expected.x && window->y == data->expected.y)) {
+ if ((data->pending_operation & X11_PENDING_OP_MOVE) && (window->x == data->expected.x + data->border_left && window->y == data->expected.y + data->border_top)) {
data->pending_operation &= ~X11_PENDING_OP_MOVE;
}
if ((data->pending_operation & X11_PENDING_OP_RESIZE) && (window->w == data->expected.w && window->h == data->expected.h)) {
@@ -904,7 +904,7 @@ static int X11_SyncWindowTimeout(SDL_VideoDevice *_this, SDL_Window *window, int
if (data->pending_operation == X11_PENDING_OP_NONE) {
if (force_exit ||
- (window->x == data->expected.x && window->y == data->expected.y &&
+ (window->x == data->expected.x + data->border_left && window->y == data->expected.y + data->border_top &&
window->w == data->expected.w && window->h == data->expected.h)) {
/* The window is in the expected state and nothing is pending. Done. */
break;
@@ -1358,6 +1358,29 @@ void X11_ShowWindow(SDL_VideoDevice *_this, SDL_Window *window)
if (data->border_left == 0 && data->border_right == 0 && data->border_top == 0 && data->border_bottom == 0) {
X11_GetBorderValues(data);
}
+
+ /* Some window managers can send garbage coordinates while mapping the window, and need the position sent again
+ * after mapping or the window may not be positioned properly.
+ *
+ * Don't emit size and position events during the initial configure events, they will be sent afterwards, when the
+ * final coordinates are available to avoid sending garbage values.
+ */
+ data->disable_size_position_events = SDL_TRUE;
+ X11_XSync(display, False);
+ X11_PumpEvents(_this);
+
+ int x = data->last_xconfigure.x;
+ int y = data->last_xconfigure.y;
+ SDL_GlobalToRelativeForWindow(data->window, x, y, &x, &y);
+
+ /* If the borders appeared, this happened automatically in the event system, otherwise, set the position now. */
+ if (data->disable_size_position_events && (window->x != x || window->y != y)) {
+ data->pending_operation = X11_PENDING_OP_MOVE;
+ X11_XMoveWindow(display, data->xwindow, window->x, window->y);
+ }
+ data->disable_size_position_events = SDL_FALSE;
+ SDL_SendWindowEvent(window, SDL_EVENT_WINDOW_RESIZED, data->last_xconfigure.width, data->last_xconfigure.height);
+ SDL_SendWindowEvent(window, SDL_EVENT_WINDOW_MOVED, x, y);
}
void X11_HideWindow(SDL_VideoDevice *_this, SDL_Window *window)
@@ -1551,7 +1574,7 @@ static int X11_SetWindowFullscreenViaWM(SDL_VideoDevice *_this, SDL_Window *wind
XEvent e;
/* Flush any pending fullscreen events. */
- if (data->pending_operation & (X11_PENDING_OP_FULLSCREEN | X11_PENDING_OP_MAXIMIZE)) {
+ if (data->pending_operation & (X11_PENDING_OP_FULLSCREEN | X11_PENDING_OP_MAXIMIZE | X11_PENDING_OP_MOVE)) {
X11_SyncWindow(_this, window);
}
@@ -1591,10 +1614,8 @@ static int X11_SetWindowFullscreenViaWM(SDL_VideoDevice *_this, SDL_Window *wind
/* Set the position so the window will be on the target display */
if (fullscreen) {
+ SDL_DisplayID current = SDL_GetDisplayForWindowPosition(window);
SDL_copyp(&data->requested_fullscreen_mode, &window->current_fullscreen_mode);
- if (data->requested_fullscreen_mode.displayID == 0) {
- data->requested_fullscreen_mode.displayID = _display->id;
- }
if (fullscreen != !!(window->flags & SDL_WINDOW_FULLSCREEN)) {
data->window_was_maximized = !!(window->flags & SDL_WINDOW_MAXIMIZED);
}
@@ -1602,7 +1623,12 @@ static int X11_SetWindowFullscreenViaWM(SDL_VideoDevice *_this, SDL_Window *wind
data->expected.y = displaydata->y;
data->expected.w = _display->current_mode->w;
data->expected.h = _display->current_mode->h;
- X11_XMoveWindow(display, data->xwindow, displaydata->x, displaydata->y);
+
+ /* Only move the window if it isn't fullscreen or already on the target display. */
+ if (!(window->flags & SDL_WINDOW_FULLSCREEN) || (!current || current != _display->id)) {
+ X11_XMoveWindow(display, data->xwindow, displaydata->x, displaydata->y);
+ data->pending_operation |= X11_PENDING_OP_MOVE;
+ }
} else {
SDL_zero(data->requested_fullscreen_mode);