Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix x11 taskbar icon/title #8132

Closed
wants to merge 1 commit into from
Closed

Fix x11 taskbar icon/title #8132

wants to merge 1 commit into from

Conversation

broughtong
Copy link
Contributor

Currently on linux, the "taskbar window title" aka the X11 class property (WM_CLASS) is pulled from the executable name - which is reasonable but there is no way for clients to change it.

Moreover, when making SDL calls from another language, for example Python, the python interpreter itself gets detected as the executable. This means that when opening a window in SDL called through Python, the window title will be according to the SDL_CreateWindow command, but the taskbar title will just be the Python interpreter.

This also stops calls to set window icon from propagating correctly to the taskbar. No matter how the window icon changes, the taskbar icon will always be the python logo.

Proposed change is whenever the window title is set, to also change the WM_CLASS, updating the taskbar title to the current window title.

Description

Whenever the window title is set/changed, the x11 WM_CLASS is also changed to match.

Existing Issue(s)

py-sdl/py-sdl2#268

@broughtong
Copy link
Contributor Author

For clarity, here's an example image:

image

The window icon has been change and the title to "some title". But the executable name "Python (v3.10)" still gets thrown to the top and the icon here wont change.

The window itself is fine.

Commit means the icon here is the same as the window, and "some title" here becomes the main title instead of the parent/child name layout.

Also, I would really appreciate someone double checking the logic here.

@icculus
Copy link
Collaborator

icculus commented Aug 19, 2023

Are there any limitations on WM_CLASS? Like, must it only be low-ASCII characters or something?

@broughtong
Copy link
Contributor Author

It supports an X11 string so latin-1, but not UTF-8.
If you put an emoji in your window title, the window itself will display correctly, but the taskbar not.
Although this is the same behaviour I'm getting when using xdotool.

I suppose the commit can just use the title variable directly though.

There is some talk about the same issue in SMFL.

The thing is though that I've realised the same issue exist on Windows.
Would an SDL hint work better?
Or perhaps some basic taskbar API to control the group of windows, set the icon etc?

@Kontrabant
Copy link
Contributor

SDL3 has the X11 WM_CLASS and Wayland app id rolled into the SDL_APP_ID hint, which can be set internally and is inherited by windows at creation time.

The class/app id aren't supposed to match the window title though; it's used by desktops for grouping windows together (e.g. all Firefox/Chrome windows appear under the same alt-tab icon because they have the same class/id). Additionally, while it defaults to the executable name if undefined, it really should be a string in reverse-DNS notation (com.my_company.my_app) with an accompanying com.my_company.my_app.desktop file that has your application icon and info contained within. This is especially important when packaging your application in a container such as Flatpak, where the class/app id strings have to match the container name.

@icculus
Copy link
Collaborator

icculus commented Sep 4, 2023

This is especially important when packaging your application in a container such as Flatpak, where the class/app id strings have to match the container name.

Uh, do we need to handle this better in SDL3?

@broughtong
Copy link
Contributor Author

SDL3 has the X11 WM_CLASS and Wayland app id rolled into the SDL_APP_ID hint, which can be set internally and is inherited by windows at creation time.

The class/app id aren't supposed to match the window title though; it's used by desktops for grouping windows together (e.g. all Firefox/Chrome windows appear under the same alt-tab icon because they have the same class/id). Additionally, while it defaults to the executable name if undefined, it really should be a string in reverse-DNS notation (com.my_company.my_app) with an accompanying com.my_company.my_app.desktop file that has your application icon and info contained within. This is especially important when packaging your application in a container such as Flatpak, where the class/app id strings have to match the container name.

The problem is that the WM_CLASS should be a two-part string. Currently the first part is always pulled from the current exe name (SDL_GetExeName) and the second part is pulled from SDL_GetAppID. (SDL_x11window.c line 631)

The second part can be modified using the hint, but as far as I can see, the first part will always just taken from the exe name.

In the case of calling SDL from a python interpreter, the interpreter is picked up as the executable. This means that any SDL windows are just grouped with every other python window on the desktop, related or unrelated to the current program. They all just take the python interpreter name and logo.

As you say, simply pushing the window title to the taskbar is not an ideal solution. But it does break the link and make the current programs windows independent of some other python application on the machine.

Is there a way to set this I'm not aware of? Else, some way to set this programmatically would be desirable.

@Kontrabant
Copy link
Contributor

Kontrabant commented Sep 13, 2023

If I try your example in the linked issue and set the envvar SDL_VIDEO_X11_WMCLASS="firefox" (the envvar for changing it on SDL2), it shows up in the taskbar and alt-tab menu with the Firefox title string and icon, as it is pulling it from the .desktop file of the same name, so overriding the class name seems to work for setting the icon and title on Python apps. Note that on SDL2, this envvar must be set before initializing the video system, as it is cached at init time and remains static from then on. Only SDL3 allows this to be set per-window (change SDL_HINT_APP_ID before window creation).

The second part can be modified using the hint, but as far as I can see, the first part will always just taken from the exe name.

The X11 res_name always being the executable name doesn't matter, as windows are grouped and load desktop resources according to the res_class string. I can start two instances of the same application with the same res_name but different res_class strings, and the windows won't be grouped by the desktop.

Anyway, setting the class to always match the window title will break packaging for many existing apps, so this pull is a no-go.

@Kontrabant Kontrabant closed this Sep 13, 2023
@broughtong
Copy link
Contributor Author

If I try your example in the linked issue and set the envvar SDL_VIDEO_X11_WMCLASS="firefox" (the envvar for changing it on SDL2), it shows up in the taskbar and alt-tab menu with the Firefox title string and icon, as it is pulling it from the .desktop file of the same name, so overriding the class name seems to work for setting the icon and title on Python apps. Note that on SDL2, this envvar must be set before initializing the video system, as it is cached at init time and remains static from then on.

Yes I see that, working now in SDL2 👍

Only SDL3 allows this to be set per-window (change SDL_HINT_APP_ID before window creation).

Sorry to drag this on but can you confirm how this works in SDL3? I tried just setting this hint from Python and it didn't see to work as expected. There's some sample code here which I used:

import ctypes
import time

lib = ctypes.cdll.LoadLibrary("libSDL3.so")

setHintF = lib.SDL_SetHint
setHintF.argtypes = ctypes.c_char_p, ctypes.c_char_p
setHintF.restype = ctypes.c_int
print(setHintF(b"SDL_HINT_APP_ID", b"test"))
print(setHintF(b"SDL_APP_ID", b"test"))

initF = lib.SDL_Init
initF.argtype = ctypes.c_int
initF.restype = ctypes.c_int
initF(32)

createWindowF = lib.SDL_CreateWindowWithPosition
createWindowF.argtypes = ctypes.c_char_p, ctypes.c_int, ctypes.c_int, ctypes.c_int, ctypes.c_int, ctypes.c_int
createWindowF.restype = ctypes.c_int
window = createWindowF(b"initial title", 0, 0, 400, 400, 0)

changeTitleF = lib.SDL_SetWindowTitle
changeTitleF.argtypes = ctypes.c_int, ctypes.c_char_p
changeTitleF.restype = ctypes.c_int
changeTitleF(window, b"new title")

loadIconF = lib.SDL_LoadBMP
loadIconF.argtype = ctypes.c_char_p
loadIconF.restype = ctypes.c_voidp
icon = loadIconF(b"test.bmp")

setIconF = lib.SDL_SetWindowIcon
setIconF.argtype = ctypes.c_voidp, ctypes.c_voidp
setIconF.restype = ctypes.c_int
setIconF(window, icon)

time.sleep(10)

If I run xprop on the window, I get WM_CLASS(STRING) = "python3.8", "test" (ubuntu 20.04)
Which I suppose causes the system (ubuntu 20.04) to match it with the python .desktop and load the icons, description etc for that.
By contrast, your approach for SDL2 results in: WM_CLASS(STRING) = "test", "test", which means the window's icon get's pulled in to the taskbar.

Anyway, setting the class to always match the window title will break packaging for many existing apps, so this pull is a no-go.

Agreed :)

@Kontrabant
Copy link
Contributor

The example code has some bugs: window handles are pointers, not ints, and the parameter code for setting the icon was incorrect (it should be argtypes, plural). It works as expected for me with these fixes. I'm surprised it ran for you at all, as it was just crashing for me.

import ctypes
import time

lib = ctypes.cdll.LoadLibrary("libSDL3.so")

setHintF = lib.SDL_SetHint
setHintF.argtypes = ctypes.c_char_p, ctypes.c_char_p
setHintF.restype = ctypes.c_int
print(setHintF(b"SDL_HINT_APP_ID", b"test"))
print(setHintF(b"SDL_APP_ID", b"test"))

initF = lib.SDL_Init
initF.argtype = ctypes.c_int
initF.restype = ctypes.c_int
initF(32)

createWindowF = lib.SDL_CreateWindowWithPosition
createWindowF.argtypes = ctypes.c_char_p, ctypes.c_int, ctypes.c_int, ctypes.c_int, ctypes.c_int, ctypes.c_int
createWindowF.restype = ctypes.c_void_p
window = createWindowF(b"initial title", 0, 0, 400, 400, 0)

changeTitleF = lib.SDL_SetWindowTitle
changeTitleF.argtypes = ctypes.c_void_p, ctypes.c_char_p
changeTitleF.restype = ctypes.c_int
changeTitleF(window, b"new title")

loadIconF = lib.SDL_LoadBMP
loadIconF.argtype = ctypes.c_char_p
loadIconF.restype = ctypes.c_void_p
icon = loadIconF(b"test.bmp")

setIconF = lib.SDL_SetWindowIcon
setIconF.argtypes = ctypes.c_void_p, ctypes.c_void_p
setIconF.restype = ctypes.c_int
setIconF(window, icon)

time.sleep(10)

@broughtong
Copy link
Contributor Author

Ah that's so stupid. Thanks. Confirm it's working 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants