Ivan Krylov
2021-Mar-23 10:18 UTC
[Rd] Possible x11 window manager window aggregation under one icon?
On Mon, 22 Mar 2021 16:57:48 -0500 Dirk Eddelbuettel <edd at debian.org> wrote:> Do you want to send a proper patch to bugzilla?Would be glad to, especially if we manage to solve that problem you uncovered while I was asleep. On Mon, 22 Mar 2021 22:23:47 -0500 Dirk Eddelbuettel <edd at debian.org> wrote:> Close, close, close but no cigar yet: For a given R process, x11() > windows group for a that process. But we often run multiple R > processes. Have you seen anything for grouping under the > "program" (in some sense) but not the concrete process from it?Do windows from different Emacs processes group together the way you want them to group? What other applications group together for you despite running from different processes? Do they have the same window id # of group leader in `xprop WM_HINTS`? I checked Firefox, but its windows all seem to have the same _NET_WM_PID. I decided to copy the way GVim sets its group leader ID (because I know the windows are different processes _and_ that they group in Xfce) and spent a while chasing this red herring before realising that (1) on my PC, different x11() windows are still grouped together, even from different R processes, even without the patch (I never used the "group windows" option in xfce4-panel before) and (2) different GVim windows actually have different group leader XIDs in their WM_HINTS properties. Oops. Apparently Xfce uses libwnck [*] which groups windows by WM_CLASS in addition to WM_HINTS (as far as understand the code). Here is what GNOME Shell does [**] besides looking at WM_HINTS.window_group: - looks up the window's WM_CLASS in .desktop files known to it - looks up the window's _NET_WM_PID among running applications (?) - looks for an XDG startup notification matching the window - checks other things not likely applicable to R, such as sandbox IDs and GApplication IDs Adding StartupWMClass=R_x11 to R.desktop (not part of R sources, but part of the .deb package, I believe) should help GNOME Shell match all x11() windows to a single application without any changes to devX11.c, but I don't have GNOME installed to check it. Alternatively, we can also add a _NET_WM_PID property to x11() windows (in the hope that GNOME Shell matches the PIDs to the same binary), but then we'd have to add the WM_CLIENT_MACHINE property too [***], which is way more hacky than I would prefer it to be: -----------------------------------8<----------------------------------- Index: src/modules/X11/devX11.c ==================================================================--- src/modules/X11/devX11.c (revision 80104) +++ src/modules/X11/devX11.c (working copy) @@ -52,6 +52,8 @@ #endif #include <X11/keysymdef.h> +#include <sys/utsname.h> /* for uname -> WM_CLIENT_MACHINE -> _NET_WM_PID */ +#include <unistd.h> /* getpid -> _NET_WM_PID */ #define R_USE_PROTOTYPES 1 #include <R_ext/GraphicsEngine.h> @@ -105,7 +107,7 @@ static Display *display; /* Display */ static char dspname[101]=""; static int screen; /* Screen */ -static Window rootwin; /* Root Window */ +static Window rootwin, group_leader; /* Root Window */ static Visual *visual; /* Visual */ static int depth; /* Pixmap depth */ static int Vclass; /* Visual class */ @@ -1617,6 +1619,39 @@ PropModeReplace, (const unsigned char*) rlogo_icon, 2 + 99*77); + /* set the window group leader */ + XWMHints * hints; + hints = XAllocWMHints(); + if (hints) { + hints->window_group = group_leader; + hints->flags |= WindowGroupHint; + XSetWMHints(display, xd->window, hints); + XFree(hints); + } + + /* Provide WM_CLIENT_MACHINE to set a valid _NET_WM_PID */ + struct utsname unm; + if (uname(&unm)) goto no_wm_pid; + char * nodename = &unm.nodename[0]; + XTextProperty hostname = {0}; /* initialise the value pointer */ + if (Success != XmbTextListToTextProperty( + display, &nodename, 1, XStringStyle, &hostname + )) { + if (hostname.value) XFree(hostname.value); + goto no_wm_pid; + } + XSetWMClientMachine(display, xd->window, &hostname); + XFree(hostname.value); + + /* set _NET_WM_PID */ + uint32_t mypid = (uint32_t)getpid(); /* must be CARDINAL(32) */ + XChangeProperty(display, xd->window, + XInternAtom(display, "_NET_WM_PID", False), + XInternAtom(display, "CARDINAL", False), 32, + PropModeReplace, + (const unsigned char*) &mypid, 1); + no_wm_pid: + /* set up protocols so that window manager sends */ /* me an event when user "destroys" window */ _XA_WM_PROTOCOLS = XInternAtom(display, "WM_PROTOCOLS", 0); @@ -2109,6 +2144,7 @@ if (numX11Devices == 0) { int fd = ConnectionNumber(display); /* Free Resources Here */ + XDestroyWindow(display, group_leader); while (nfonts--) R_XFreeFont(display, fontcache[nfonts].font); nfonts = 0; @@ -3133,6 +3169,9 @@ #endif screen = DefaultScreen(display); rootwin = DefaultRootWindow(display); + group_leader = XCreateSimpleWindow( + display, rootwin, 0, 0, 1, 1, 0, 0, 0 + ); depth = DefaultDepth(display, screen); visual = DefaultVisual(display, screen); colormap = DefaultColormap(display, screen); -----------------------------------8<----------------------------------- -- Best regards, Ivan [*] https://gitlab.gnome.org/GNOME/libwnck/-/blob/master/libwnck/class-group.c https://gitlab.gnome.org/GNOME/libwnck/-/blob/master/libwnck/window.c [**] https://gitlab.gnome.org/GNOME/gnome-shell/-/blob/4dfc53ca/src/shell-window-tracker.c#L371 [***] https://specifications.freedesktop.org/wm-spec/1.3/ar01s05.html#idm45225103239616
Dirk Eddelbuettel
2021-Mar-23 12:44 UTC
[Rd] Possible x11 window manager window aggregation under one icon?
On 23 March 2021 at 13:18, Ivan Krylov wrote: | On Mon, 22 Mar 2021 16:57:48 -0500 | Dirk Eddelbuettel <edd at debian.org> wrote: | | > Do you want to send a proper patch to bugzilla? | | Would be glad to, especially if we manage to solve that problem you | uncovered while I was asleep. | | On Mon, 22 Mar 2021 22:23:47 -0500 | Dirk Eddelbuettel <edd at debian.org> wrote: | | > Close, close, close but no cigar yet: For a given R process, x11() | > windows group for a that process. But we often run multiple R | > processes. Have you seen anything for grouping under the | > "program" (in some sense) but not the concrete process from it? | | Do windows from different Emacs processes group together the way you | want them to group? What other applications group together for you | despite running from different processes? Do they have the same window | id # of group leader in `xprop WM_HINTS`? I checked Firefox, but its | windows all seem to have the same _NET_WM_PID. "All of them, but R". Right now (under unity) I have four for Gnome Terminal (clearly distinct processes), two for Chrome (plus more temporarily), and two for emacs (two windows from same process, I checked that launching a new one clearly aggregates within). But R is different, and I see this as a bug. How "grave" it is is open for debate, but the application behaves differently under the window manager. The overall behaviour is consistent, yet R sticks out. I think it shouldn't. | I decided to copy the way GVim sets its group leader ID (because I know | the windows are different processes _and_ that they group in Xfce) and | spent a while chasing this red herring before realising that (1) on my | PC, different x11() windows are still grouped together, even from | different R processes, even without the patch (I never used the "group | windows" option in xfce4-panel before) and (2) different GVim windows | actually have different group leader XIDs in their WM_HINTS properties. | Oops. Hah! | Apparently Xfce uses libwnck [*] which groups windows by WM_CLASS in | addition to WM_HINTS (as far as understand the code). | | Here is what GNOME Shell does [**] besides looking at | WM_HINTS.window_group: | | - looks up the window's WM_CLASS in .desktop files known to it | - looks up the window's _NET_WM_PID among running applications (?) | - looks for an XDG startup notification matching the window | - checks other things not likely applicable to R, such as sandbox IDs | and GApplication IDs | | Adding StartupWMClass=R_x11 to R.desktop (not part of R sources, but | part of the .deb package, I believe) should help GNOME Shell match all | x11() windows to a single application without any changes to devX11.c, | but I don't have GNOME installed to check it. Easy enough for me to check, but I won't get to it for a bit. | Alternatively, we can also add a _NET_WM_PID property to x11() windows | (in the hope that GNOME Shell matches the PIDs to the same binary), but | then we'd have to add the WM_CLIENT_MACHINE property too [***], which | is way more hacky than I would prefer it to be: [proposed change set omitted] Ok. You did amazing. I had poked around a little in one or two apps but not made any progress. Dirk -- https://dirk.eddelbuettel.com | @eddelbuettel | edd at debian.org
Duncan Murdoch
2021-Mar-23 15:41 UTC
[Rd] Possible x11 window manager window aggregation under one icon?
On 23/03/2021 6:18 a.m., Ivan Krylov wrote:> On Mon, 22 Mar 2021 16:57:48 -0500 > Dirk Eddelbuettel <edd at debian.org> wrote: > >> Do you want to send a proper patch to bugzilla? > > Would be glad to, especially if we manage to solve that problem you > uncovered while I was asleep. > > On Mon, 22 Mar 2021 22:23:47 -0500 > Dirk Eddelbuettel <edd at debian.org> wrote: > >> Close, close, close but no cigar yet: For a given R process, x11() >> windows group for a that process. But we often run multiple R >> processes. Have you seen anything for grouping under the >> "program" (in some sense) but not the concrete process from it? > > Do windows from different Emacs processes group together the way you > want them to group? What other applications group together for you > despite running from different processes? Do they have the same window > id # of group leader in `xprop WM_HINTS`? I checked Firefox, but its > windows all seem to have the same _NET_WM_PID.In Ubuntu 18.04, all terminal windows display the same _NET_WM_PID, and that PID corresponds to gnome-terminal-server. So I think it's not going to be possible to do what Dirk wants without really major changes to the way different R processes create graphics windows. On the other hand, R doesn't set the _NET_WM_PID value. I've put a version of your code into rgl, and it does what you'd expect: it groups all rgl windows from the same R process together, but different R processes get different groups. It would probably be nice to have rgl windows and other R graphics windows in the same group, but I don't see a way for rgl to know the group_leader that R is using (and it's probably not worth adding this to the API to be able to request it). Am I missing an easier solution? Duncan Murdoch> > I decided to copy the way GVim sets its group leader ID (because I know > the windows are different processes _and_ that they group in Xfce) and > spent a while chasing this red herring before realising that (1) on my > PC, different x11() windows are still grouped together, even from > different R processes, even without the patch (I never used the "group > windows" option in xfce4-panel before) and (2) different GVim windows > actually have different group leader XIDs in their WM_HINTS properties. > Oops. > > Apparently Xfce uses libwnck [*] which groups windows by WM_CLASS in > addition to WM_HINTS (as far as understand the code). > > Here is what GNOME Shell does [**] besides looking at > WM_HINTS.window_group: > > - looks up the window's WM_CLASS in .desktop files known to it > - looks up the window's _NET_WM_PID among running applications (?) > - looks for an XDG startup notification matching the window > - checks other things not likely applicable to R, such as sandbox IDs > and GApplication IDs > > Adding StartupWMClass=R_x11 to R.desktop (not part of R sources, but > part of the .deb package, I believe) should help GNOME Shell match all > x11() windows to a single application without any changes to devX11.c, > but I don't have GNOME installed to check it. > > Alternatively, we can also add a _NET_WM_PID property to x11() windows > (in the hope that GNOME Shell matches the PIDs to the same binary), but > then we'd have to add the WM_CLIENT_MACHINE property too [***], which > is way more hacky than I would prefer it to be: > > -----------------------------------8<----------------------------------- > Index: src/modules/X11/devX11.c > ==================================================================> --- src/modules/X11/devX11.c (revision 80104) > +++ src/modules/X11/devX11.c (working copy) > @@ -52,6 +52,8 @@ > #endif > #include <X11/keysymdef.h> > > +#include <sys/utsname.h> /* for uname -> WM_CLIENT_MACHINE -> _NET_WM_PID */ > +#include <unistd.h> /* getpid -> _NET_WM_PID */ > > #define R_USE_PROTOTYPES 1 > #include <R_ext/GraphicsEngine.h> > @@ -105,7 +107,7 @@ > static Display *display; /* Display */ > static char dspname[101]=""; > static int screen; /* Screen */ > -static Window rootwin; /* Root Window */ > +static Window rootwin, group_leader; /* Root Window */ > static Visual *visual; /* Visual */ > static int depth; /* Pixmap depth */ > static int Vclass; /* Visual class */ > @@ -1617,6 +1619,39 @@ > PropModeReplace, > (const unsigned char*) rlogo_icon, 2 + 99*77); > > + /* set the window group leader */ > + XWMHints * hints; > + hints = XAllocWMHints(); > + if (hints) { > + hints->window_group = group_leader; > + hints->flags |= WindowGroupHint; > + XSetWMHints(display, xd->window, hints); > + XFree(hints); > + } > + > + /* Provide WM_CLIENT_MACHINE to set a valid _NET_WM_PID */ > + struct utsname unm; > + if (uname(&unm)) goto no_wm_pid; > + char * nodename = &unm.nodename[0]; > + XTextProperty hostname = {0}; /* initialise the value pointer */ > + if (Success != XmbTextListToTextProperty( > + display, &nodename, 1, XStringStyle, &hostname > + )) { > + if (hostname.value) XFree(hostname.value); > + goto no_wm_pid; > + } > + XSetWMClientMachine(display, xd->window, &hostname); > + XFree(hostname.value); > + > + /* set _NET_WM_PID */ > + uint32_t mypid = (uint32_t)getpid(); /* must be CARDINAL(32) */ > + XChangeProperty(display, xd->window, > + XInternAtom(display, "_NET_WM_PID", False), > + XInternAtom(display, "CARDINAL", False), 32, > + PropModeReplace, > + (const unsigned char*) &mypid, 1); > + no_wm_pid: > + > /* set up protocols so that window manager sends */ > /* me an event when user "destroys" window */ > _XA_WM_PROTOCOLS = XInternAtom(display, "WM_PROTOCOLS", 0); > @@ -2109,6 +2144,7 @@ > if (numX11Devices == 0) { > int fd = ConnectionNumber(display); > /* Free Resources Here */ > + XDestroyWindow(display, group_leader); > while (nfonts--) > R_XFreeFont(display, fontcache[nfonts].font); > nfonts = 0; > @@ -3133,6 +3169,9 @@ > #endif > screen = DefaultScreen(display); > rootwin = DefaultRootWindow(display); > + group_leader = XCreateSimpleWindow( > + display, rootwin, 0, 0, 1, 1, 0, 0, 0 > + ); > depth = DefaultDepth(display, screen); > visual = DefaultVisual(display, screen); > colormap = DefaultColormap(display, screen); > -----------------------------------8<----------------------------------- >