Desktop icons moved from the bottom edge after commit #5688856

[L-U-T-i]

Expected behaviour

Desktop icons are aligned down by the desktop bottom edge

Actual behaviour

Desktop icons are about 50px up from the desktop bottom edge

Steps to reproduce the behaviour

Install caja build after git commit #5688856

MATE general version

1.27.0

Package version

Custom build

Linux Distribution

Rocky Linux 9.2

Link to bugreport of your Distribution (requirement)

Bug not reported as it is a custom build

If I apply the following patch (partially reverted commit #5688856):

diff -upr a/libcaja-private/caja-icon-container.c b/libcaja-private/caja-icon-container.c
--- a/libcaja-private/caja-icon-container.c 2023-05-25 20:56:56.000000000 +0200
+++ b/libcaja-private/caja-icon-container.c 2023-06-29 13:43:06.144776032 +0200
@@ -327,6 +327,7 @@ icon_set_position_full (CajaIcon *icon,
         GdkWindow *window;
         GdkMonitor *monitor;
         GdkRectangle workarea = {0};
+        int scale;

         /*  FIXME: This should be:

@@ -345,12 +343,13 @@ icon_set_position_full (CajaIcon *icon,
         window  = gtk_widget_get_window (GTK_WIDGET(container));
         monitor = gdk_display_get_monitor_at_window (gdk_display_get_default(), window);
         gdk_monitor_get_workarea(monitor, &workarea);
+        scale = gtk_widget_get_scale_factor (GTK_WIDGET (container));
         container_x = 0;
         container_y = 0;
-        container_width = workarea.width  - container_x
+        container_width = WidthOfScreen (gdk_x11_screen_get_xscreen (gdk_screen_get_default ())) / scale - container_x
                           - container->details->left_margin
                           - container->details->right_margin;
-        container_height = workarea.height - container_y
+        container_height = HeightOfScreen (gdk_x11_screen_get_xscreen (gdk_screen_get_default ())) / scale - container_y
                            - container->details->top_margin
                            - container->details->bottom_margin;
         pixels_per_unit = EEL_CANVAS (container)->pixels_per_unit;

the issue seems to be resolved (I can align the desktop icons down by the bottom edge again). But probably this is not the right solution (as the code has been replaced by purpose)? It is just a quick test, to find the problematic part of the current code (it seems 'workarea.height' is wrongly calculated at the moment, so it is probably manifested elsewhere now...?).

[lukefromdc]

UPDATE: looks like I didn't leave the margins or the subtracted container_x and container_y variables out, so will split the codepath with X11 selector so x11 uses the old code and only wayland uses the new code.

Will try removing the margins first though, that's quick

[lukefromdc]

A test w the code used in master (same as in my wayland-desktop branch) showed that I could put icons within maybe 10px of the desktop edge (perhaps controlled by the icon window's size including the lable) so long as I did NOT have a panel on the bottom edge. With a panel, the panel struts reserve some space above the panel, and perhaps this is computed differently in the new codepath.

Will cut to the chase and set up separate x11 and wayland code paths

[lukefromdc]

Try https://github.com/mate-desktop/caja/pull/1729

[L-U-T-i]

Tried #1729 - it seems to resolve the issue (the whole area is usable again).

In any case, it seems the new code: workarea.height value is different than the old code: scale = gtk_widget_get_scale_factor (GTK_WIDGET (container)) HeightOfScreen (gdk_x11_screen_get_xscreen (gdk_screen_get_default ())) / scale one (the subtracted container_x and container_y variables are in the old and in the new code...).

I have both the top and the bottom panel (if it matters...).

[lukefromdc]

I tested both ways, and with a bottom panel can now use the space right up to the panel. Didn't catch this at first as I normally leave a substantial margin myself, but this may have played a role in causing icons to move around when switching back and forth between x11 and wayland while testing the wayland-desktop branch.

THANK YOU for testing! We need more of this kind of early testing, it's how bugs are fixed before getting to a final release distros pick up

[L-U-T-i]

Just for your info - 'workarea.height' (new code) and 'HeightOfScreen...' (old code) apper also in:

• eel/eel-gtk-extensions.c
• eel/eel-gtk-extensions.c
• src/caja-window.c

Is there a possibility of some issue due to the different usable screen height (I am not familiar with the purpose of each part of the code...)?

[lukefromdc]

Reusing variable names is not an issue, but using different workarea computations certainly could be.

I looked at this, for x11 we now (with the PR applied) have the same code for drawable_get_adjusted_size and icon_set_position_full

I see though that in sanity_check_window_dimensions (which seems to be for navigation windows) we have use of the root window in wayland where it is invalid though the code somehow still works There too I will restore the old code in x11 and write new code for wayland that does not attempt to use the root window. Nice catch!

[lukefromdc]

Just pushed a second commit making those consistant in both x11 (using the old gdk_x11 code) and in wayland (using the monitor dimensions) across these files

[L-U-T-i]

Reusing variable names is not an issue, but using different workarea computations certainly could be.

Of course I meant different computation... ;-)

And, I see there has been a mistake copying a link above - the same file is listed twice, there should be also: eel/eel-editable-label.c where PANGO_SCALE * (WidthOfScreen (gdk_x11_screen_get_xscreen (gdk_screen_get_default ())) / scale + 1) / 2); has been replaced by PANGO_SCALE * (gdk_screen_width () + 1) / 2); ... if it matters, however.

[lukefromdc]

Thanks, just switched that one over too. Probably no real world effect on behavior and a bit more complex wayland code, but it is now consistant across all of these. Editable label behavior seems same as it always was to me, and in wayland as well as x11. That does NOT mean someone with a different setup or use case would not encounter a new bug if the x11 codepath was changed from what they last used however

[L-U-T-i]

The following:

The work area should be considered when positioning menus and similar popups, to avoid placing them below panels, docks or other desktop components.

from https://docs.gtk.org/gdk3/method.Monitor.get_workarea.html made me think that maybe just the removal of margin deductions in the new code would be "sufficient" (at least for X11, can't say about wayland), in particular considering the dead area below the bottom icons has been just about the height of both top and bottom panel (and, width has not been an issue, since no vertical panels...).

I've therfore rebuilt the current latest git commit without PR #1729, but with the following patch:

diff -up a/libcaja-private/caja-icon-container.c b/libcaja-private/caja-icon-container.c
--- a/libcaja-private/caja-icon-container.c 2023-07-02 11:24:39.938677396 +0200
+++ b/libcaja-private/caja-icon-container.c 2023-07-02 11:24:40.058678197 +0200
@@ -303,6 +303,7 @@ icon_set_position_full (CajaIcon *icon,
     CajaIconContainer *container;
     int x1, x2, y1, y2;
     EelDRect icon_bounds;
+    GdkDisplay *display;

     if (!force && icon->x == x && icon->y == y)
     {
@@ -347,12 +348,21 @@ icon_set_position_full (CajaIcon *icon,
         gdk_monitor_get_workarea(monitor, &workarea);
         container_x = 0;
         container_y = 0;
-        container_width = workarea.width  - container_x
-                          - container->details->left_margin
-                          - container->details->right_margin;
-        container_height = workarea.height - container_y
-                           - container->details->top_margin
-                           - container->details->bottom_margin;
+        display = gdk_screen_get_display (gdk_screen_get_default());
+        if  (GDK_IS_X11_DISPLAY (display))
+        {
+            container_width = workarea.width  - container_x;
+            container_height = workarea.height - container_y;
+        }
+        else
+        {
+            container_width = workarea.width  - container_x
+                              - container->details->left_margin
+                              - container->details->right_margin;
+            container_height = workarea.height - container_y
+                               - container->details->top_margin
+                               - container->details->bottom_margin;
+        }
         pixels_per_unit = EEL_CANVAS (container)->pixels_per_unit;
         /* Clip the position of the icon within our desktop bounds */
         container_left = container_x / pixels_per_unit;

and it seems also to resolve the issue.

Is it possible that gdk_monitor_get_workarea function takes care about margins for panels already (so those shouldn't be deducted once again in that code)?

That would simplify a lot the solution of this issue...

[L-U-T-i]

BTW, what is the purpose of deductions ' - container_x' and ' - container_y', considering they are bot set to 0 just before...

[lukefromdc]

My guess is they are leftovers from the commented out code that did not work in the old days, saved for anyone fixing that. No, they don't do anything but I was just restoring the prior x11 code exactly as ir was

[L-U-T-i]

Is there a possibility that 'screen_width' and 'screen_height' are calculated differently for X11 and Wayland in 'icon_container_set_workarea' function (defined in src/file-manager/fm-desktop-icon-view.c'), considering they are used for 'right' and 'bottom' parameters of 'caja_icon_container_set_margins'?

I mean, that margins are somehow "included" in X11 algorythm already (and, sholdn't be deducted once again later).

So that would actually be the source of this issue...