Consolidate the X11 WM_CLASS and Wayland app ID envvars

Consolidate the X11_WMCLASS and WAYLAND_WMCLASS envvars into one SDL_HINT_APP_ID hint. This hint serves the same purpose on both windowing systems to allow desktop compositors to identify and group windows together, as well as associate applications with their desktop settings and icons. The common code for retrieving the value is now consolidated under core/unix/SDL_appid.c as it's common to *nix platforms, and the value is now retrieved at window creation time instead of being cached by the video driver at startup so that changes to the hint after video initialization and before window creation will be seen, as well as to accommodate cases where applications want to use different values for different windows.
1bsyl · May 28, 2023 · 2f75596 · 2f75596
1 parent 85f33fe
commit 2f75596
Show file tree

Hide file tree

Showing 12 changed files with 167 additions and 114 deletions.
diff --git a/docs/README-migration.md b/docs/README-migration.md
@@ -397,6 +397,7 @@ The following hints have been removed:
 
 * Renamed hints SDL_HINT_VIDEODRIVER and SDL_HINT_AUDIODRIVER to SDL_HINT_VIDEO_DRIVER and SDL_HINT_AUDIO_DRIVER
 * Renamed environment variables SDL_VIDEODRIVER and SDL_AUDIODRIVER to SDL_VIDEO_DRIVER and SDL_AUDIO_DRIVER
+* The environment variables SDL_VIDEO_X11_WMCLASS and SDL_VIDEO_WAYLAND_WMCLASS have been removed and replaced with the unified hint SDL_HINT_APP_ID
 
 ## SDL_init.h
 

diff --git a/include/SDL3/SDL_hints.h b/include/SDL3/SDL_hints.h
@@ -158,6 +158,53 @@ extern "C" {
  */
 #define SDL_HINT_ANDROID_ALLOW_RECREATE_ACTIVITY "SDL_ANDROID_ALLOW_RECREATE_ACTIVITY"
 
+/**
+ *  \brief  A variable setting the app ID string.
+ *          This string is used by desktop compositors to identify and group windows
+ *          together, as well as match applications with associated desktop settings
+ *          and icons.
+ *
+ *          On Wayland this corresponds to the "app ID" window property and on X11 this
+ *          corresponds to the WM_CLASS property. Windows inherit the value of this hint
+ *          at creation time. Changing this hint after a window has been created will not
+ *          change the app ID or class of existing windows.
+ *
+ *          For *nix platforms, this string should be formatted in reverse-DNS notation
+ *          and follow some basic rules to be valid:
+ *
+ *          - The application ID must be composed of two or more elements separated by a
+ *            period (‘.’) character.
+ *
+ *          - Each element must contain one or more of the alphanumeric characters
+ *            (A-Z, a-z, 0-9) plus underscore (‘_’) and hyphen (‘-’) and must not start
+ *            with a digit. Note that hyphens, while technically allowed, should not be
+ *            used if possible, as they are not supported by all components that use the ID,
+ *            such as D-Bus. For maximum compatability, replace hyphens with an underscore.
+ *
+ *          - The empty string is not a valid element (ie: your application ID may not
+ *            start or end with a period and it is not valid to have two periods in a row).
+ *
+ *          - The entire ID must be less than 255 characters in length.
+ *
+ *          Examples of valid app ID strings:
+ *
+ *          - org.MyOrg.MyApp
+ *          - com.your_company.your_app
+ *
+ *          Desktops such as GNOME and KDE require that the app ID string matches your
+ *          application's .desktop file name (e.g. if the app ID string is 'org.MyOrg.MyApp',
+ *          your application's .desktop file should be named 'org.MyOrg.MyApp.desktop').
+ *
+ *          If you plan to package your application in a container such as Flatpak, the
+ *          app ID should match the name of your Flatpak container as well.
+ *
+ *  If not set, SDL will attempt to use the application executable name.
+ *  If the executable name cannot be retrieved, the generic string "SDL_App" will be used.
+ *
+ *  On targets where this is not supported, this hint does nothing.
+ */
+#define SDL_HINT_APP_ID      "SDL_APP_ID"
+
 /**
  *  \brief Specify an application name.
  *

diff --git a/src/core/unix/SDL_appid.c b/src/core/unix/SDL_appid.c
@@ -0,0 +1,76 @@
+/*
+  Simple DirectMedia Layer
+  Copyright (C) 1997-2023 Sam Lantinga <slouken@libsdl.org>
+
+  This software is provided 'as-is', without any express or implied
+  warranty.  In no event will the authors be held liable for any damages
+  arising from the use of this software.
+
+  Permission is granted to anyone to use this software for any purpose,
+  including commercial applications, and to alter it and redistribute it
+  freely, subject to the following restrictions:
+
+  1. The origin of this software must not be misrepresented; you must not
+     claim that you wrote the original software. If you use this software
+     in a product, an acknowledgment in the product documentation would be
+     appreciated but is not required.
+  2. Altered source versions must be plainly marked as such, and must not be
+     misrepresented as being the original software.
+  3. This notice may not be removed or altered from any source distribution.
+*/
+
+#include "SDL_internal.h"
+
+#include "SDL_appid.h"
+#include <unistd.h>
+
+const char *SDL_GetExeName()
+{
+    static const char *proc_name = NULL;
+
+    /* TODO: Use a fallback if BSD has no mounted procfs (OpenBSD has no procfs at all) */
+    if (!proc_name) {
+#if defined(__LINUX__) || defined(__FREEBSD__) || defined (__NETBSD__)
+        static char linkfile[1024];
+        int linksize;
+
+#if defined(__LINUX__)
+        const char *proc_path = "/proc/self/exe";
+#elif defined(__FREEBSD__)
+        const char *proc_path = "/proc/curproc/file";
+#elif defined(__NETBSD__)
+        const char *proc_path = "/proc/curproc/exe";
+#endif
+        linksize = readlink(proc_path, linkfile, sizeof(linkfile) - 1);
+        if (linksize > 0) {
+            linkfile[linksize] = '\0';
+            proc_name = SDL_strrchr(linkfile, '/');
+            if (proc_name) {
+                ++proc_name;
+            } else {
+                proc_name = linkfile;
+            }
+        }
+#endif
+    }
+
+    return proc_name;
+}
+
+const char *SDL_GetAppID()
+{
+    /* Always check the hint, as it may have changed */
+    const char *id_str = SDL_GetHint(SDL_HINT_APP_ID);
+
+    if (!id_str) {
+        /* If the hint isn't set, try to use the application's executable name */
+        id_str = SDL_GetExeName();
+    }
+
+    if (!id_str) {
+        /* Finally, use the default we've used forever */
+        id_str = "SDL_App";
+    }
+
+    return id_str;
+}
diff --git a/src/core/unix/SDL_appid.h b/src/core/unix/SDL_appid.h
@@ -0,0 +1,30 @@
+/*
+Simple DirectMedia Layer
+Copyright (C) 1997-2023 Sam Lantinga <slouken@libsdl.org>
+
+This software is provided 'as-is', without any express or implied
+warranty.  In no event will the authors be held liable for any damages
+arising from the use of this software.
+
+Permission is granted to anyone to use this software for any purpose,
+including commercial applications, and to alter it and redistribute it
+freely, subject to the following restrictions:
+
+1. The origin of this software must not be misrepresented; you must not
+   claim that you wrote the original software. If you use this software
+   in a product, an acknowledgment in the product documentation would be
+   appreciated but is not required.
+2. Altered source versions must be plainly marked as such, and must not be
+   misrepresented as being the original software.
+3. This notice may not be removed or altered from any source distribution.
+*/
+
+#include "SDL_internal.h"
+
+#ifndef SDL_appid_h_
+#define SDL_appid_h_
+
+extern const char *SDL_GetExeName();
+extern const char *SDL_GetAppID();
+
+#endif /* SDL_appid_h_ */
diff --git a/src/video/wayland/SDL_waylandvideo.c b/src/video/wayland/SDL_waylandvideo.c
@@ -82,60 +82,6 @@ static int Wayland_VideoInit(SDL_VideoDevice *_this);
 static int Wayland_GetDisplayBounds(SDL_VideoDevice *_this, SDL_VideoDisplay *display, SDL_Rect *rect);
 static void Wayland_VideoQuit(SDL_VideoDevice *_this);
 
-/* Find out what class name we should use
- * Based on src/video/x11/SDL_x11video.c */
-static char *get_classname(void)
-{
-    /* !!! FIXME: this is probably wrong, albeit harmless in many common cases. From protocol spec:
-        "The surface class identifies the general class of applications
-        to which the surface belongs. A common convention is to use the
-        file name (or the full path if it is a non-standard location) of
-        the application's .desktop file as the class." */
-
-    char *spot;
-#if defined(__LINUX__) || defined(__FREEBSD__)
-    char procfile[1024];
-    char linkfile[1024];
-    int linksize;
-#endif
-
-    /* First allow environment variable override */
-    spot = SDL_getenv("SDL_VIDEO_WAYLAND_WMCLASS");
-    if (spot) {
-        return SDL_strdup(spot);
-    } else {
-        /* Fallback to the "old" envvar */
-        spot = SDL_getenv("SDL_VIDEO_X11_WMCLASS");
-        if (spot) {
-            return SDL_strdup(spot);
-        }
-    }
-
-    /* Next look at the application's executable name */
-#if defined(__LINUX__) || defined(__FREEBSD__)
-#ifdef __LINUX__
-    (void)SDL_snprintf(procfile, SDL_arraysize(procfile), "/proc/%d/exe", getpid());
-#elif defined(__FREEBSD__)
-    (void)SDL_snprintf(procfile, SDL_arraysize(procfile), "/proc/%d/file", getpid());
-#else
-#error Where can we find the executable name?
-#endif
-    linksize = readlink(procfile, linkfile, sizeof(linkfile) - 1);
-    if (linksize > 0) {
-        linkfile[linksize] = '\0';
-        spot = SDL_strrchr(linkfile, '/');
-        if (spot) {
-            return SDL_strdup(spot + 1);
-        } else {
-            return SDL_strdup(linkfile);
-        }
-    }
-#endif /* __LINUX__ || __FREEBSD__ */
-
-    /* Finally use the default we've used forever */
-    return SDL_strdup("SDL_App");
-}
-
 static const char *SDL_WAYLAND_surface_tag = "sdl-window";
 static const char *SDL_WAYLAND_output_tag = "sdl-output";
 
@@ -945,9 +891,6 @@ int Wayland_VideoInit(SDL_VideoDevice *_this)
 
     Wayland_InitMouse();
 
-    /* Get the surface class name, usually the name of the application */
-    data->classname = get_classname();
-
     WAYLAND_wl_display_flush(data->display);
 
     Wayland_InitKeyboard(_this);
@@ -1144,18 +1087,15 @@ SDL_bool Wayland_VideoReconnect(SDL_VideoDevice *_this)
 
 void Wayland_VideoQuit(SDL_VideoDevice *_this)
 {
-    SDL_VideoData *data = _this->driverdata;
-
     Wayland_VideoCleanup(_this);
 
 #ifdef HAVE_LIBDECOR_H
+    SDL_VideoData *data = _this->driverdata;
     if (data->shell.libdecor) {
         libdecor_unref(data->shell.libdecor);
         data->shell.libdecor = NULL;
     }
 #endif
-
-    SDL_free(data->classname);
 }
 
 #endif /* SDL_VIDEO_DRIVER_WAYLAND */
diff --git a/src/video/wayland/SDL_waylandvideo.h b/src/video/wayland/SDL_waylandvideo.h
@@ -90,8 +90,6 @@ struct SDL_VideoData
     struct qt_windowmanager *windowmanager;
 #endif /* SDL_VIDEO_DRIVER_WAYLAND_QT_TOUCH */
 
-    char *classname;
-
     int relative_mouse_mode;
 };
 

diff --git a/src/video/wayland/SDL_waylandwindow.c b/src/video/wayland/SDL_waylandwindow.c
@@ -25,6 +25,7 @@
 
 #include "../SDL_sysvideo.h"
 #include "../../events/SDL_events_c.h"
+#include "../../core/unix/SDL_appid.h"
 #include "../SDL_egl_c.h"
 #include "SDL_waylandevents_c.h"
 #include "SDL_waylandwindow.h"
@@ -1328,7 +1329,7 @@ void Wayland_ShowWindow(SDL_VideoDevice *_this, SDL_Window *window)
         if (data->shell_surface.libdecor.frame == NULL) {
             SDL_LogError(SDL_LOG_CATEGORY_VIDEO, "Failed to create libdecor frame!");
         } else {
-            libdecor_frame_set_app_id(data->shell_surface.libdecor.frame, c->classname);
+            libdecor_frame_set_app_id(data->shell_surface.libdecor.frame, data->app_id);
             libdecor_frame_map(data->shell_surface.libdecor.frame);
         }
     } else
@@ -1390,7 +1391,7 @@ void Wayland_ShowWindow(SDL_VideoDevice *_this, SDL_Window *window)
             }
         } else {
             data->shell_surface.xdg.roleobj.toplevel = xdg_surface_get_toplevel(data->shell_surface.xdg.surface);
-            xdg_toplevel_set_app_id(data->shell_surface.xdg.roleobj.toplevel, c->classname);
+            xdg_toplevel_set_app_id(data->shell_surface.xdg.roleobj.toplevel, data->app_id);
             xdg_toplevel_add_listener(data->shell_surface.xdg.roleobj.toplevel, &toplevel_listener_xdg, data);
         }
     }
@@ -2047,6 +2048,9 @@ int Wayland_CreateWindow(SDL_VideoDevice *_this, SDL_Window *window)
     data->outputs = NULL;
     data->num_outputs = 0;
 
+    /* Cache the app_id at creation time, as it may change before the window is mapped. */
+    data->app_id = SDL_strdup(SDL_GetAppID());
+
     data->requested_window_width = window->w;
     data->requested_window_height = window->h;
     data->floating_width = window->windowed.w;
@@ -2311,6 +2315,7 @@ void Wayland_DestroyWindow(SDL_VideoDevice *_this, SDL_Window *window)
         }
 
         SDL_free(wind->outputs);
+        SDL_free(wind->app_id);
 
         if (wind->gles_swap_frame_callback) {
             wl_callback_destroy(wind->gles_swap_frame_callback);

diff --git a/src/video/wayland/SDL_waylandwindow.h b/src/video/wayland/SDL_waylandwindow.h
@@ -111,6 +111,7 @@ struct SDL_WindowData
 
     SDL_Window *keyboard_focus;
 
+    char *app_id;
     float windowed_scale_factor;
     float pointer_scale_x;
     float pointer_scale_y;

diff --git a/src/video/x11/SDL_x11keyboard.c b/src/video/x11/SDL_x11keyboard.c
@@ -203,7 +203,7 @@ int X11_InitKeyboard(SDL_VideoDevice *_this)
         (void)setlocale(LC_ALL, "");
         X11_XSetLocaleModifiers(new_xmods);
 
-        data->im = X11_XOpenIM(data->display, NULL, data->classname, data->classname);
+        data->im = X11_XOpenIM(data->display, NULL, NULL, NULL);
 
         /* Reset the locale + X locale modifiers back to how they were,
            locale first because the X locale modifiers depend on it. */

diff --git a/src/video/x11/SDL_x11video.c b/src/video/x11/SDL_x11video.c
@@ -45,47 +45,6 @@
 static int X11_VideoInit(SDL_VideoDevice *_this);
 static void X11_VideoQuit(SDL_VideoDevice *_this);
 
-/* Find out what class name we should use */
-static char *get_classname(void)
-{
-    char *spot;
-#if defined(__LINUX__) || defined(__FREEBSD__)
-    char procfile[1024];
-    char linkfile[1024];
-    int linksize;
-#endif
-
-    /* First allow environment variable override */
-    spot = SDL_getenv("SDL_VIDEO_X11_WMCLASS");
-    if (spot) {
-        return SDL_strdup(spot);
-    }
-
-    /* Next look at the application's executable name */
-#if defined(__LINUX__) || defined(__FREEBSD__)
-#ifdef __LINUX__
-    (void)SDL_snprintf(procfile, SDL_arraysize(procfile), "/proc/%d/exe", getpid());
-#elif defined(__FREEBSD__)
-    (void)SDL_snprintf(procfile, SDL_arraysize(procfile), "/proc/%d/file", getpid());
-#else
-#error Where can we find the executable name?
-#endif
-    linksize = readlink(procfile, linkfile, sizeof(linkfile) - 1);
-    if (linksize > 0) {
-        linkfile[linksize] = '\0';
-        spot = SDL_strrchr(linkfile, '/');
-        if (spot) {
-            return SDL_strdup(spot + 1);
-        } else {
-            return SDL_strdup(linkfile);
-        }
-    }
-#endif /* __LINUX__ || __FREEBSD__ */
-
-    /* Finally use the default we've used forever */
-    return SDL_strdup("SDL_App");
-}
-
 /* X11 driver bootstrap functions */
 
 static int (*orig_x11_errhandler)(Display *, XErrorEvent *) = NULL;
@@ -409,9 +368,6 @@ int X11_VideoInit(SDL_VideoDevice *_this)
 {
     SDL_VideoData *data = _this->driverdata;
 
-    /* Get the window class name, usually the name of the application */
-    data->classname = get_classname();
-
     /* Get the process PID to be associated to the window */
     data->pid = getpid();
 
@@ -491,7 +447,6 @@ void X11_VideoQuit(SDL_VideoDevice *_this)
         X11_XDestroyWindow(data->display, data->clipboard_window);
     }
 
-    SDL_free(data->classname);
 #ifdef X_HAVE_UTF8_STRING
     if (data->im) {
         X11_XCloseIM(data->im);