wnck-pager: update workspace switcher aspect ratio #1417

raveit65 · 2023-11-15T10:33:41Z

This fixes #1230
and is needed because of an update in libwnck
https://gitlab.gnome.org/GNOME/libwnck/-/commit/3456b747b6381f17d48629dd8fdd4d511e739b10

In fedora i could workaround this issue with reverting the libwnck commit.
But it seems that xfce users had problems with the xfce-panel because of reverted libwnck commit. See https://gitlab.xfce.org/xfce/xfce4-panel/-/issues/630#note_80841 and https://bugzilla.redhat.com/show_bug.cgi?id=2242944
I offered a test RPM without the mentioned patch to confirm the xfce issue.
But we should really try to fix the issue with wnck-pager from our side after 2 years. It is urgent!

As a downside this PR overwrites #806 which fixed 2 issues.
I can't reproduce #799 with PR anymore, but unfortunately #797 still exists with PR.

@mate-desktop/core-team
Please help to fix #797 on top of this PR
If we find no solution the only option is to use PR plus disable applet in-process build for fedora and all other distributions which use libwnck >= 40.

raveit65 · 2023-11-15T15:53:03Z

I built 1.26 branch with this PR and i got same results when testing previous issues.
I am unable to reproduce #799 with un-expanded panel (tested bottom panel), but windows-list applet issue with un-expanded panel still exists #797.

raveit65 · 2023-11-15T16:36:46Z

When building the panel out-of-process issue #797 doesn't exists \o/
If i have to remove https://src.fedoraproject.org/rpms/libwnck3/blob/rawhide/f/libwnck_0001-Revert-pager-do-not-change-workspace-size-from-size_.patch from fedora libwnck3 RPM i will switch mate-panel back to out-of-process build, ..............good bye wayland.

lukefromdc

I tested this in-process only, got these results:

This does solve the switcher aspect ratio problem but on a nonexpanded panel makes the window list disappear entirely, and changing themes didn't fix that. Space is allocated for normally for each button but they are not rendered and that space is treated as bare panel when clicking on it.

If I put the window list on a nonexpanded panel and look at it with GtkInspector, the widgets for the window list are all there, they are just not shown. Wonder if we are just not showing them when not expanded due to a codepath missing something like gtk_widget_show_all()?

lukefromdc · 2023-11-16T01:35:18Z

Using
return GTK_SIZE_REQUEST_CONSTANT_SIZE;
unconditionally in
mate_panel_applet_get_request_mode
makes the unexpanded panel work again, but brings back the switcher aspect ratio problem. If we could look for MATE_PANEL_APPLET_EXPAND_MAJOR (used only by the window list) and apply that for that condition, we should have a fix however hacky

lukefromdc · 2023-11-16T01:46:28Z

Also, on restarting the panel, the window list on a nonexpanded in-process panel appears for a fraction of a second before disappearing when the panel is rerendered, possibly for the first time.

cwendling · 2023-11-16T12:22:58Z

This only fixes the workspace selector aspect ration in-process, right?

For the non-expanded issue, there seem to be a problem in the panel->packed path of panel_widget_size_allocate(). If you comment this out:

diff --git a/mate-panel/panel-widget.c b/mate-panel/panel-widget.c
index 665fdfab..95016be1 100644
--- a/mate-panel/panel-widget.c
+++ b/mate-panel/panel-widget.c
@@ -1514,7 +1514,7 @@ panel_widget_size_allocate(GtkWidget *widget, GtkAllocation *allocation)
 	else
 		panel->size = allocation->height;
 
-	if (panel->packed) {
+	if (FALSE /*panel->packed*/) {
 		/* we're assuming the order is the same as the one that was
 		 * in size_request() */
 		int applet_using_hint_index = 0;

then it mostly works -- yet I don't know the actual implications. But it's clearly not perfect, as it'll allocate take the expected room for the applets, not taking into account their position: e.g. if you put the tasklist on the far right of an unexpanded panel, the panel will be sized as if it was visible, but it's actually basically invisible because it has 0 room before the end of the panel.

I expect the actual issue being a problem computing the minimal size given to the applet when allocating the child, although it seems to be computing the size correctly when it sizes the panel.

If we could look for MATE_PANEL_APPLET_EXPAND_MAJOR (used only by the window list) and apply that for that condition, we should have a fix however hacky

That would indeed be a gross hack, but well, I don't think we're pass that just yet. Unfortunately, it doesn't seem to work; if I apply the following it doesn't help (possibly it's not set early enough):

diff --git a/libmate-panel-applet/mate-panel-applet.c b/libmate-panel-applet/mate-panel-applet.c
index 0830569f..5c604fe6 100644
--- a/libmate-panel-applet/mate-panel-applet.c
+++ b/libmate-panel-applet/mate-panel-applet.c
@@ -1138,7 +1138,7 @@ mate_panel_applet_get_request_mode (GtkWidget *widget)
 
 	priv = mate_panel_applet_get_instance_private (applet);
 
-	if (priv->out_of_process)
+	if (priv->out_of_process || priv->flags & MATE_PANEL_APPLET_EXPAND_MAJOR)
 		return GTK_SIZE_REQUEST_CONSTANT_SIZE;
 
 	orientation = mate_panel_applet_get_orient (applet);

Another workaround could be try the intermediate wrapper for the desktop switcher that Alberts pasted somewhere in one of those reference issues. I didn't try it, but if he says it works, I'm pretty sure it does; and it's a more localized hack.

Of course, the best fix would be to figure out where the layout code is messing up… but that code is far from trivial.

raveit65 · 2023-11-16T12:43:57Z

This only fixes the workspace selector aspect ration in-process, right?

Yes, this is your code from mate-desktop-legacy-archive/libmatewnck#4 (comment)

lukefromdc · 2023-11-17T04:35:04Z

Just tested using

-	if (panel->packed) {
+	if (FALSE /*panel->packed*/) {

and yes, that gives me both a (semi-) functional window list on a non-expanded panel (looking at it right now) and the proper workspace aspect ratio. unfortunately, if I started opening a lot of new windows to use more space after putting the classic menu to the right of the window list to use up some space, the space needed for the buttons was allocated right of the classic menu applet, and the entire window list moved right too, compressing existing buttons and pulling the whole panel end to the right too.

I doublechecked that in the nonexpanded panel, the "packed" case is what's normally used by removing that block entirely, again getting the above results with only the "not packed" case available in the code. This confirms that whatever is allocating zero space to show the buttons after expanding the panel to take them is in or affected by that block of code.

Note the comment in the code:

		/* we're assuming the order is the same as the one that was
		 * in size_request() */

lukefromdc · 2023-11-17T04:57:06Z

This block starting at line 1537 in panel-widget.c I just proved to be the problem

				if (ad->expand_major && ad->size_hints) {
					int width = panel->applets_using_hint[applet_using_hint_index].size;
					applet_using_hint_index++;
					challoc.width = MIN (width, allocation->width - i);
				}

challoc.width is somehow coming up as zero, if I replace MIN (width, allocation->width - i); with something like 700, I get quite normal window list behavior on that nonexpanded panel, other widgets don't get pushed off it, and the window buttons compress when space becomes limited for them.

EDIT: I have proven width to be the offending variable, for some reason
int width = panel->applets_using_hint[applet_using_hint_index].size; is returning zero
if I manually set width to 750 for test purposes only, again the buttons behave normally: getting allocated 750 px, but since space is limited the window list applet shrinks them down to fit.

If I replace MIN with MAX I get huge window buttons, if I use challoc.width = width the buttons disappear
if I use challoc.width = (allocation->width - i); I again get overexpanded buttons pushing all else off the panel

This is the horizontal panel case only of course, and is restricted to the window list on a packed panel. We need to find out why

lukefromdc · 2023-11-17T04:58:25Z

Also note that you left the additional g_print debugging statements in this PR.

These are good for debugging but log spam for users if they get to git master or to a release.

I used a g_message statement to prove that width is coming up zero but had to remove the g_print statements to be able to find the output when running mate-panel --replace in terminal

lukefromdc · 2023-11-17T05:07:53Z

I added my own debugging code to the block in question to see what we are getting back. Making the block in question

				if (ad->expand_major && ad->size_hints) {
					int width = panel->applets_using_hint[applet_using_hint_index].size;
					applet_using_hint_index++;
					challoc.width = MIN (width, allocation->width - i);
					g_message("width =");
					g_message("%i", width);
					g_message("allocation->width - i =");
					g_message("%i", (allocation->width - i));
				}
and running with a second, non-expanded top panel with the window list and the classic menu to the right of it, I got this result w 8 window buttons in the bottom panel's window list (also used in this test):

** Message: 00:04:29.881: width =
** Message: 00:04:29.881: 11
** Message: 00:04:29.881: allocation->width - i =
** Message: 00:04:29.881: 502
** Message: 00:04:29.947: width =
** Message: 00:04:29.947: 11
** Message: 00:04:29.947: allocation->width - i =
** Message: 00:04:29.947: 502
** Message: 00:04:30.102: width =
** Message: 00:04:30.102: 0
** Message: 00:04:30.102: allocation->width - i =
** Message: 00:04:30.102: 502
** Message: 00:04:30.122: width =
** Message: 00:04:30.122: 0
** Message: 00:04:30.122: allocation->width - i =
** Message: 00:04:30.122: 502
** Message: 00:04:30.374: width =
** Message: 00:04:30.374: 1348
** Message: 00:04:30.374: allocation->width - i =
** Message: 00:04:30.374: 1599
** Message: 00:04:30.391: width =
** Message: 00:04:30.391: 0
** Message: 00:04:30.391: allocation->width - i =
** Message: 00:04:30.391: 1599
** Message: 00:04:30.408: width =
** Message: 00:04:30.408: 0
** Message: 00:04:30.408: allocation->width - i =
** Message: 00:04:30.408: 1599
** Message: 00:04:34.675: width =
** Message: 00:04:34.675: 0
** Message: 00:04:34.675: allocation->width - i =
** Message: 00:04:34.675: 1599
** Message: 00:04:34.703: width =
** Message: 00:04:34.703: 0
** Message: 00:04:34.703: allocation->width - i =
** Message: 00:04:34.703: 1599
** Message: 00:05:14.986: width =
** Message: 00:05:14.986: 0
** Message: 00:05:14.986: allocation->width - i =
** Message: 00:05:14.986: 1599
** Message: 00:05:15.012: width =
** Message: 00:05:15.012: 0
** Message: 00:05:15.012: allocation->width - i =
** Message: 00:05:15.012: 1599

raveit65 · 2023-11-17T11:01:42Z

I dropped the debug code blog.

raveit65 · 2023-11-17T14:02:56Z

I found another issue when building mate-panel in-process and wnck-pager commit from PR.
Last icon (ltr) in na-tray is cropped :/
With this commit:

Without this commit:

It doesn't matter which icon is at last position (ltr).
An out-of-process panel build is not affected by this problem.

raveit65 · 2023-11-17T18:04:13Z

Another workaround could be try the intermediate wrapper for the desktop switcher that Alberts pasted somewhere in one of those reference issues. I didn't try it, but if he says it works, I'm pretty sure it does; and it's a more localized hack.

You mean that post?
#1230 (comment)

lukefromdc · 2023-11-17T18:29:33Z

I can confirm the issue with the tray. I looked at the wrapper, only the core of the suggested code is written there. Looks like whatever we do must be limited to the switcher only

lukefromdc · 2023-11-17T19:19:09Z

Some of the g_print debug code is still in the latest commits, line 1453 in panel-widget.c is one example.
Also I am assuming all of applet_debug_name is only used for this

raveit65 · 2023-11-17T19:45:20Z

Done, i removed the rest of the debug code.

fixes #1230 This is needed because of an update in libwnck https://gitlab.gnome.org/GNOME/libwnck/-/commit/3456b747b6381f17d48629dd8fdd4d511e739b10

raveit65 · 2023-11-17T20:33:37Z

And all of applet_debug_name is removed.

lukefromdc · 2023-11-18T01:49:58Z

Thanks,I will be playing with this over the next few days trying to find a fix that works

lukefromdc · 2023-11-20T00:19:50Z

In playing with this, I've noticed something interesting: while the last icon in the tray always gets its right edge cut off, the seemingly random cutting (partial rendering in reduced space) of other icons such as the force-quit and inhibit buttons on restarting the panel is entirely stopped by this. Testing in x11 gives the results decribed above:

Testing in Wayland was interesting: The last icon in the tray (an indicator both here and in x11 on my setup) was cut as in X11, but the unexpanded panel showed the window list buttons! Instead of the panel expanding to take more of them, if more than one button was to show when the panel started text was ellipsized, and the space taken by those initial buttons was all that the applet would ever get, but all of the window list buttons showed up. The backend code is totally different of course, so querying the applet's size gets different results. Also the nonexpanded panel always renders on the left side of a wayland screen

raveit65 · 2023-11-20T09:34:05Z

Well, you changed the code for windows-list applet (wayland) for yourself in summer as far as i can remember.

2 xfce user did confirm that the xfce-panel crash was gone with vanilla libwnck in fedora https://bugzilla.redhat.com/show_bug.cgi?id=2242944#c5
So i have to remove https://src.fedoraproject.org/rpms/libwnck3/blob/rawhide/f/libwnck_0001-Revert-pager-do-not-change-workspace-size-from-size_.patch from fedora rpm. I will switch mate-panel to out-of-process build for the moment.

lukefromdc · 2023-11-20T13:40:22Z

Oh yeah-the wayland window list has to be complteley differenr becsuse libwnck is X11 only. Thankfully wlroots supports gtk-layer-shell which handles passing the window coordinates and name between the compositor and the panel. Some of those icon cutting issues date all the way back to GNOME 2. If necessaey we could see distros building everything out-of-process and specialty repos (e.g old sryle Ubuntu PPA's) offering Wayland builds. The x11 codepath is long enough that this PR mostly working in wayland only narrows things down a little. The tray icon cutting may or may not be the same issue.

fixes #1230 This is needed because of an update in libwnck https://gitlab.gnome.org/GNOME/libwnck/-/commit/3456b747b6381f17d48629dd8fdd4d511e739b10 With that commit applets should be build out-of-process to avoid issues with windows-list and tray applets. See #1417 for more infos. Signed-off-by: raveit65 <[email protected]>

raveit65 · 2023-11-20T17:45:18Z

I can provide an in-process build for wayland testers at fedora coprs cloud https://copr.fedorainfracloud.org/coprs/
But i hope that issues are fixed before a 1.28 release.
@mate-desktop/core-team @cwendling @vkareh @yetist @zhuyaliang
Please help....

lukefromdc · 2023-11-20T17:58:33Z

Given just how long we have had applet cutting issues (and GNOME 2 before us) I am wondering if there is anyone left who fully understands the applet code and these two applets.

This is a dirty hack, but it works both in-process and out-of-process, and doesn't break window list on non-expanded panels. A better solution would be to actually fix the panel's layout code, but nobody seems to have enough time and experience with this to actually do this at the moment. Based off an idea and code from Alberts Muktupāvels[^1], thanks. Fixes #1417. [^1]: #1230 (comment)

cwendling · 2023-11-21T15:33:48Z

@raveit65 @lukefromdc can you give #1420 a try? It's based on the hack suggested by Alberts, and it seems to work a lot better than what we have here, as well as being contained in the pager applet.

It's definitely a hack, but I'm more comfortable with a working, low-impact hack than a proper fix that doesn't really work and has unknown implications 🙂

lukefromdc · 2023-11-21T19:36:50Z

I just confirmed that the changes in mate-panel-applet.c in this are themselves sufficient to kill the window list in a nonexpanded panel, while the changes in panel-widget.c are needed to fix the switcher

lukefromdc · 2023-11-22T00:40:10Z

Better fix merged in
#1421

raveit65 requested review from vkareh, lukefromdc, cwendling and a team November 15, 2023 10:34

lukefromdc reviewed Nov 15, 2023

View reviewed changes

raveit65 force-pushed the wnck-pager branch from 14722c8 to 168e349 Compare November 17, 2023 11:00

raveit65 force-pushed the wnck-pager branch from 168e349 to a03791d Compare November 17, 2023 19:43

wnck-pager: update workspace switcher aspect ratio

5ec70f6

fixes #1230 This is needed because of an update in libwnck https://gitlab.gnome.org/GNOME/libwnck/-/commit/3456b747b6381f17d48629dd8fdd4d511e739b10

raveit65 force-pushed the wnck-pager branch from a03791d to 5ec70f6 Compare November 17, 2023 20:32

raveit65 mentioned this pull request Nov 20, 2023

wnck-pager: update workspace switcher aspect ratio 1.26 #1419

Closed

cwendling mentioned this pull request Nov 21, 2023

wnck-pager: Fix workspace switcher aspect ratio #1420

Closed

lukefromdc closed this Nov 22, 2023

raveit65 deleted the wnck-pager branch February 4, 2024 17:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wnck-pager: update workspace switcher aspect ratio #1417

wnck-pager: update workspace switcher aspect ratio #1417

raveit65 commented Nov 15, 2023 •

edited

Loading

raveit65 commented Nov 15, 2023 •

edited

Loading

raveit65 commented Nov 15, 2023 •

edited

Loading

lukefromdc left a comment

lukefromdc commented Nov 16, 2023

lukefromdc commented Nov 16, 2023

cwendling commented Nov 16, 2023

raveit65 commented Nov 16, 2023

lukefromdc commented Nov 17, 2023

lukefromdc commented Nov 17, 2023 •

edited

Loading

lukefromdc commented Nov 17, 2023 •

edited

Loading

lukefromdc commented Nov 17, 2023

raveit65 commented Nov 17, 2023

raveit65 commented Nov 17, 2023

raveit65 commented Nov 17, 2023

lukefromdc commented Nov 17, 2023 •

edited

Loading

lukefromdc commented Nov 17, 2023 •

edited

Loading

raveit65 commented Nov 17, 2023

raveit65 commented Nov 17, 2023 •

edited

Loading

lukefromdc commented Nov 18, 2023 via email

lukefromdc commented Nov 20, 2023

raveit65 commented Nov 20, 2023 •

edited

Loading

lukefromdc commented Nov 20, 2023 via email

raveit65 commented Nov 20, 2023

lukefromdc commented Nov 20, 2023

cwendling commented Nov 21, 2023

lukefromdc commented Nov 21, 2023

lukefromdc commented Nov 22, 2023

wnck-pager: update workspace switcher aspect ratio #1417

wnck-pager: update workspace switcher aspect ratio #1417

Conversation

raveit65 commented Nov 15, 2023 • edited Loading

raveit65 commented Nov 15, 2023 • edited Loading

raveit65 commented Nov 15, 2023 • edited Loading

lukefromdc left a comment

Choose a reason for hiding this comment

lukefromdc commented Nov 16, 2023

lukefromdc commented Nov 16, 2023

cwendling commented Nov 16, 2023

raveit65 commented Nov 16, 2023

lukefromdc commented Nov 17, 2023

lukefromdc commented Nov 17, 2023 • edited Loading

lukefromdc commented Nov 17, 2023 • edited Loading

lukefromdc commented Nov 17, 2023

raveit65 commented Nov 17, 2023

raveit65 commented Nov 17, 2023

raveit65 commented Nov 17, 2023

lukefromdc commented Nov 17, 2023 • edited Loading

lukefromdc commented Nov 17, 2023 • edited Loading

raveit65 commented Nov 17, 2023

raveit65 commented Nov 17, 2023 • edited Loading

lukefromdc commented Nov 18, 2023 via email

lukefromdc commented Nov 20, 2023

raveit65 commented Nov 20, 2023 • edited Loading

lukefromdc commented Nov 20, 2023 via email

raveit65 commented Nov 20, 2023

lukefromdc commented Nov 20, 2023

cwendling commented Nov 21, 2023

lukefromdc commented Nov 21, 2023

lukefromdc commented Nov 22, 2023

raveit65 commented Nov 15, 2023 •

edited

Loading

raveit65 commented Nov 15, 2023 •

edited

Loading

raveit65 commented Nov 15, 2023 •

edited

Loading

lukefromdc commented Nov 17, 2023 •

edited

Loading

lukefromdc commented Nov 17, 2023 •

edited

Loading

lukefromdc commented Nov 17, 2023 •

edited

Loading

lukefromdc commented Nov 17, 2023 •

edited

Loading

raveit65 commented Nov 17, 2023 •

edited

Loading

raveit65 commented Nov 20, 2023 •

edited

Loading