From 31ba7efa487450ac4d8f003dbc1bdd2acef06b1c Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Wed, 6 Aug 2025 09:22:15 -0700 Subject: [PATCH] x11: Refactor dpi hooks, removing GTK dependency and fixing XSettings watcher - Removed GTK signal handler in x11settings. XSettings events are now properly dispatched to X11_XsettingsNotify. Previously events were not being passed to xsettings-client as no SDL xsettings_window was created. Now all events are filtered through xsettings_client_process_event allowing it to process the external window events that are selected. Global content scale is updated for changes to any recognized dpi settings. - X11_GetGlobalContent now reads the current RESOURCE_MANAGER prop off of the root window to ensure it sees the current value. XResourceManagerString is now only used if getting the current prop fails as it caches the current resource manager value per-display connection. - Clean up some warnings in SDL_gtk. --- src/core/unix/SDL_gtk.c | 3 +- src/core/unix/SDL_gtk.h | 2 +- src/events/SDL_events.c | 1 - src/tray/unix/SDL_tray.c | 4 +- src/video/x11/SDL_x11events.c | 17 +---- src/video/x11/SDL_x11modes.c | 117 +++++++++++++++++++------------- src/video/x11/SDL_x11settings.c | 50 +++----------- src/video/x11/SDL_x11settings.h | 6 +- src/video/x11/SDL_x11video.c | 1 + src/video/x11/SDL_x11video.h | 1 + 10 files changed, 87 insertions(+), 115 deletions(-) diff --git a/src/core/unix/SDL_gtk.c b/src/core/unix/SDL_gtk.c index 53af04b21c..d5ba754f67 100644 --- a/src/core/unix/SDL_gtk.c +++ b/src/core/unix/SDL_gtk.c @@ -27,7 +27,8 @@ ctx.sub.fn = (void *)SDL_LoadFunction(lib, #sym) #define SDL_GTK_SYM2(ctx, lib, sub, fn, sym) \ - if (!(ctx.sub.fn = (void *)SDL_LoadFunction(lib, #sym))) { \ + SDL_GTK_SYM2_OPTIONAL(ctx, lib, sub, fn, sym); \ + if (!ctx.sub.fn) { \ return SDL_SetError("Could not load GTK functions"); \ } diff --git a/src/core/unix/SDL_gtk.h b/src/core/unix/SDL_gtk.h index 2ec3ea0b09..73fef5a820 100644 --- a/src/core/unix/SDL_gtk.h +++ b/src/core/unix/SDL_gtk.h @@ -118,7 +118,7 @@ typedef struct SDL_GtkContext extern bool SDL_Gtk_Init(void); extern void SDL_Gtk_Quit(void); extern SDL_GtkContext *SDL_Gtk_EnterContext(void); -extern void SDL_Gtk_ExitContext(SDL_GtkContext *gtk); +extern void SDL_Gtk_ExitContext(SDL_GtkContext *ctx); extern void SDL_UpdateGtk(void); #endif // SDL_gtk_h_ diff --git a/src/events/SDL_events.c b/src/events/SDL_events.c index 19d3e37b47..c04d52b282 100644 --- a/src/events/SDL_events.c +++ b/src/events/SDL_events.c @@ -1445,7 +1445,6 @@ void SDL_PumpEventMaintenance(void) } #endif - // SDL_UpdateTrays will also pump GTK events if needed SDL_UpdateTrays(); SDL_SendPendingSignalEvents(); // in case we had a signal handler fire, etc. diff --git a/src/tray/unix/SDL_tray.c b/src/tray/unix/SDL_tray.c index a9863932b1..874df8b15d 100644 --- a/src/tray/unix/SDL_tray.c +++ b/src/tray/unix/SDL_tray.c @@ -233,7 +233,9 @@ static void DestroySDLMenu(SDL_TrayMenu *menu) void SDL_UpdateTrays(void) { - SDL_UpdateGtk(); + if (SDL_HasActiveTrays()) { + SDL_UpdateGtk(); + } } SDL_Tray *SDL_CreateTray(SDL_Surface *icon, const char *tooltip) diff --git a/src/video/x11/SDL_x11events.c b/src/video/x11/SDL_x11events.c index 4d585d3388..110da587a6 100644 --- a/src/video/x11/SDL_x11events.c +++ b/src/video/x11/SDL_x11events.c @@ -850,16 +850,6 @@ static void X11_HandleClipboardEvent(SDL_VideoDevice *_this, const XEvent *xeven } } -static void X11_HandleSettingsEvent(SDL_VideoDevice *_this, const XEvent *xevent) -{ - SDL_VideoData *videodata = _this->internal; - - SDL_assert(videodata->xsettings_window != None); - SDL_assert(xevent->xany.window == videodata->xsettings_window); - - X11_HandleXsettings(_this, xevent); -} - static Bool isMapNotify(Display *display, XEvent *ev, XPointer arg) { XUnmapEvent *unmap; @@ -1228,11 +1218,8 @@ static void X11_DispatchEvent(SDL_VideoDevice *_this, XEvent *xevent) return; } - if ((videodata->xsettings_window != None) && - (videodata->xsettings_window == xevent->xany.window)) { - X11_HandleSettingsEvent(_this, xevent); - return; - } + // xsettings internally filters events for the windows it watches + X11_HandleXsettingsEvent(_this, xevent); data = X11_FindWindow(_this, xevent->xany.window); diff --git a/src/video/x11/SDL_x11modes.c b/src/video/x11/SDL_x11modes.c index cb6e4386b7..e8cd6d6193 100644 --- a/src/video/x11/SDL_x11modes.c +++ b/src/video/x11/SDL_x11modes.c @@ -50,74 +50,93 @@ float X11_GetGlobalContentScale(SDL_VideoDevice *_this) { - static double scale_factor = 0.0; + double scale_factor = 0.0; - if (scale_factor <= 0.0) { + // First use the forced scaling factor specified by the app/user + const char *hint = SDL_GetHint(SDL_HINT_VIDEO_X11_SCALING_FACTOR); + if (hint && *hint) { + double value = SDL_atof(hint); + if (value >= 1.0f && value <= 10.0f) { + scale_factor = value; + } + } - // First use the forced scaling factor specified by the app/user - const char *hint = SDL_GetHint(SDL_HINT_VIDEO_X11_SCALING_FACTOR); - if (hint && *hint) { - double value = SDL_atof(hint); - if (value >= 1.0f && value <= 10.0f) { - scale_factor = value; - } + // If that failed, try "Xft.dpi" from the XResourcesDatabase... + // We attempt to read this directly to get the live value, XResourceManagerString + // is cached per display connection. + if (scale_factor <= 0.0) + { + SDL_VideoData *data = _this->internal; + Display *display = data->display; + int status, real_format; + Atom real_type; + unsigned long items_read, items_left; + char *resource_manager; + bool owns_resource_manager = false; + + X11_XrmInitialize(); + status = X11_XGetWindowProperty(display, RootWindow(display, DefaultScreen(display)), + data->atoms.RESOURCE_MANAGER, 0L, 8192L, False, XA_STRING, + &real_type, &real_format, &items_read, &items_left, + (unsigned char **)&resource_manager); + + if (status == Success && resource_manager) { + owns_resource_manager = true; + } else { + // Fall back to XResourceManagerString. This will not be updated if the + // dpi value is later changed but should allow getting the initial value. + resource_manager = X11_XResourceManagerString(display); } - // If that failed, try "Xft.dpi" from the XResourcesDatabase... - if (scale_factor <= 0.0) - { - SDL_VideoData *data = _this->internal; - Display *display = data->display; - char *resource_manager; + if (resource_manager) { XrmDatabase db; XrmValue value; char *type; - X11_XrmInitialize(); + db = X11_XrmGetStringDatabase(resource_manager); - resource_manager = X11_XResourceManagerString(display); - if (resource_manager) { - db = X11_XrmGetStringDatabase(resource_manager); - - // Get the value of Xft.dpi from the Database - if (X11_XrmGetResource(db, "Xft.dpi", "String", &type, &value)) { - if (value.addr && type && SDL_strcmp(type, "String") == 0) { - int dpi = SDL_atoi(value.addr); - scale_factor = dpi / 96.0; - } - } - X11_XrmDestroyDatabase(db); - } - } - - // If that failed, try the XSETTINGS keys... - if (scale_factor <= 0.0) { - scale_factor = X11_GetXsettingsIntKey(_this, "Gdk/WindowScalingFactor", -1); - - // The Xft/DPI key is stored in increments of 1024th - if (scale_factor <= 0.0) { - int dpi = X11_GetXsettingsIntKey(_this, "Xft/DPI", -1); - if (dpi > 0) { - scale_factor = (double) dpi / 1024.0; - scale_factor /= 96.0; + // Get the value of Xft.dpi from the Database + if (X11_XrmGetResource(db, "Xft.dpi", "String", &type, &value)) { + if (value.addr && type && SDL_strcmp(type, "String") == 0) { + int dpi = SDL_atoi(value.addr); + scale_factor = dpi / 96.0; } } - } + X11_XrmDestroyDatabase(db); - // If that failed, try the GDK_SCALE envvar... - if (scale_factor <= 0.0) { - const char *scale_str = SDL_getenv("GDK_SCALE"); - if (scale_str) { - scale_factor = SDL_atoi(scale_str); + if (owns_resource_manager) { + X11_XFree(resource_manager); } } + } - // Nothing or a bad value, just fall back to 1.0 + // If that failed, try the XSETTINGS keys... + if (scale_factor <= 0.0) { + scale_factor = X11_GetXsettingsIntKey(_this, "Gdk/WindowScalingFactor", -1); + + // The Xft/DPI key is stored in increments of 1024th if (scale_factor <= 0.0) { - scale_factor = 1.0; + int dpi = X11_GetXsettingsIntKey(_this, "Xft/DPI", -1); + if (dpi > 0) { + scale_factor = (double) dpi / 1024.0; + scale_factor /= 96.0; + } } } + // If that failed, try the GDK_SCALE envvar... + if (scale_factor <= 0.0) { + const char *scale_str = SDL_getenv("GDK_SCALE"); + if (scale_str) { + scale_factor = SDL_atoi(scale_str); + } + } + + // Nothing or a bad value, just fall back to 1.0 + if (scale_factor <= 0.0) { + scale_factor = 1.0; + } + return (float)scale_factor; } diff --git a/src/video/x11/SDL_x11settings.c b/src/video/x11/SDL_x11settings.c index 661d66db83..9ade109991 100644 --- a/src/video/x11/SDL_x11settings.c +++ b/src/video/x11/SDL_x11settings.c @@ -26,9 +26,8 @@ #include "SDL_x11video.h" #include "SDL_x11settings.h" -#include "core/unix/SDL_gtk.h" - #define SDL_XSETTINGS_GDK_WINDOW_SCALING_FACTOR "Gdk/WindowScalingFactor" +#define SDL_XSETTINGS_GDK_UNSCALED_DPI "Gdk/UnscaledDPI" #define SDL_XSETTINGS_XFT_DPI "Xft/DPI" static void UpdateContentScale(SDL_VideoDevice *_this) @@ -44,13 +43,12 @@ static void UpdateContentScale(SDL_VideoDevice *_this) static void X11_XsettingsNotify(const char *name, XSettingsAction action, XSettingsSetting *setting, void *data) { SDL_VideoDevice *_this = data; - UpdateContentScale(_this); -} -static void OnGtkXftDpi(GtkSettings *settings, GParamSpec *pspec, gpointer ptr) -{ - SDL_VideoDevice *_this = (SDL_VideoDevice *)ptr; - UpdateContentScale(_this); + if (SDL_strcmp(name, SDL_XSETTINGS_GDK_WINDOW_SCALING_FACTOR) == 0 || + SDL_strcmp(name, SDL_XSETTINGS_GDK_UNSCALED_DPI) == 0 || + SDL_strcmp(name, SDL_XSETTINGS_XFT_DPI) == 0) { + UpdateContentScale(_this); + } } void X11_InitXsettings(SDL_VideoDevice *_this) @@ -58,25 +56,6 @@ void X11_InitXsettings(SDL_VideoDevice *_this) SDL_VideoData *data = _this->internal; SDLX11_SettingsData *xsettings_data = &data->xsettings_data; - GtkSettings *gtksettings = NULL; - guint xft_dpi_signal_handler_id = 0; - - SDL_GtkContext *gtk = SDL_Gtk_EnterContext(); - if (gtk) { - // Prefer to listen for DPI changes from gtk. In XWayland this is necessary as XSettings - // are not updated dynamically. - gtksettings = gtk->gtk.settings_get_default(); - if (gtksettings) { - xft_dpi_signal_handler_id = gtk->g.signal_connect(gtksettings, "notify::gtk-xft-dpi", &OnGtkXftDpi, _this); - } - SDL_Gtk_ExitContext(gtk); - } - - if (gtksettings && xft_dpi_signal_handler_id) { - xsettings_data->gtksettings = gtksettings; - xsettings_data->xft_dpi_signal_handler_id = xft_dpi_signal_handler_id; - } - xsettings_data->xsettings = xsettings_client_new(data->display, DefaultScreen(data->display), X11_XsettingsNotify, NULL, _this); } @@ -89,28 +68,15 @@ void X11_QuitXsettings(SDL_VideoDevice *_this) if (xsettings_data->xsettings) { xsettings_client_destroy(xsettings_data->xsettings); } - - SDL_GtkContext *gtk = SDL_Gtk_EnterContext(); - if (gtk) { - if (xsettings_data->gtksettings && xsettings_data->xft_dpi_signal_handler_id) { - gtk->g.signal_handler_disconnect(xsettings_data->gtksettings, xsettings_data->xft_dpi_signal_handler_id); - } - SDL_Gtk_ExitContext(gtk); - } - - SDL_zero(xsettings_data); } -void X11_HandleXsettings(SDL_VideoDevice *_this, const XEvent *xevent) +void X11_HandleXsettingsEvent(SDL_VideoDevice *_this, const XEvent *xevent) { SDL_VideoData *data = _this->internal; SDLX11_SettingsData *xsettings_data = &data->xsettings_data; if (xsettings_data->xsettings) { - if (!xsettings_client_process_event(xsettings_data->xsettings, xevent)) { - xsettings_client_destroy(xsettings_data->xsettings); - xsettings_data->xsettings = NULL; - } + xsettings_client_process_event(xsettings_data->xsettings, xevent); } } diff --git a/src/video/x11/SDL_x11settings.h b/src/video/x11/SDL_x11settings.h index 39926b66c6..f91ed70ac7 100644 --- a/src/video/x11/SDL_x11settings.h +++ b/src/video/x11/SDL_x11settings.h @@ -27,17 +27,13 @@ #include #include "xsettings-client.h" -#include "core/unix/SDL_gtk.h" - typedef struct X11_SettingsData { XSettingsClient *xsettings; - GtkSettings *gtksettings; - guint xft_dpi_signal_handler_id; } SDLX11_SettingsData; extern void X11_InitXsettings(SDL_VideoDevice *_this); extern void X11_QuitXsettings(SDL_VideoDevice *_this); -extern void X11_HandleXsettings(SDL_VideoDevice *_this, const XEvent *xevent); +extern void X11_HandleXsettingsEvent(SDL_VideoDevice *_this, const XEvent *xevent); extern int X11_GetXsettingsIntKey(SDL_VideoDevice *_this, const char *key, int fallback_value); #endif // SDL_x11settings_h_ diff --git a/src/video/x11/SDL_x11video.c b/src/video/x11/SDL_x11video.c index 8b23186e1a..ce34f9e6e9 100644 --- a/src/video/x11/SDL_x11video.c +++ b/src/video/x11/SDL_x11video.c @@ -395,6 +395,7 @@ static bool X11_VideoInit(SDL_VideoDevice *_this) GET_ATOM(SDL_SELECTION); GET_ATOM(TARGETS); GET_ATOM(SDL_FORMATS); + GET_ATOM(RESOURCE_MANAGER); GET_ATOM(XdndAware); GET_ATOM(XdndEnter); GET_ATOM(XdndLeave); diff --git a/src/video/x11/SDL_x11video.h b/src/video/x11/SDL_x11video.h index a336a800f5..8d89bf5b76 100644 --- a/src/video/x11/SDL_x11video.h +++ b/src/video/x11/SDL_x11video.h @@ -103,6 +103,7 @@ struct SDL_VideoData Atom SDL_SELECTION; Atom TARGETS; Atom SDL_FORMATS; + Atom RESOURCE_MANAGER; Atom XdndAware; Atom XdndEnter; Atom XdndLeave;