mate-desktop / caja

Caja, the file manager for the MATE desktop
https://mate-desktop.org/
Other
265 stars 143 forks source link

ensure all of the desktop is usable in x11 #1729

Closed lukefromdc closed 1 year ago

lukefromdc commented 1 year ago

Fix https://github.com/mate-desktop/caja/issues/1728 (area near bottom of screen unusable for icon placement)

revert x11 icon position handling to the older and well tested code use the newer code only in wayland where all the screen can already be used

lukefromdc commented 1 year ago

I put in an email request for more Travis credits since the wayland testing is using a lot

L-U-T-i commented 1 year ago

Thank you for a quick feedback / solution.

raveit65 commented 1 year ago

@lukefromdc i have a wayland session running but not full setup. Give me some time over the weekend .Than i can test all related PR's on wayfire and x11.

lukefromdc commented 1 year ago

2nd commit keeps desktop area computation consistant across all the files where it was changed for wayland, and in all cases restoring the prior gtk_x11 code for the x11 case and using monitor dimensions in wayland. Computing this different ways in different functions is asking for bugs it seems

L-U-T-i commented 1 year ago

This patch (as-is at the moment) shall be slightly modified to be applied after #1722, from:

--- eel/eel-gtk-extensions.c
+++ eel/eel-gtk-extensions.c
@@ -80,36 +80,49 @@ eel_gtk_window_get_geometry_string (GtkWindow *window)
 static void
 sanity_check_window_position (int *left, int *top)
 {
+    GdkScreen *screen;
+    gint scale;
+    GdkDisplay *display;
+
     g_assert (left != NULL);
     g_assert (top != NULL);
-    GdkDisplay *display;
-    GdkWindow *root_window;
-    GdkRectangle workarea = {0};
-
-    /* Make sure the top of the window is on screen, for
-     * draggability (might not be necessary with all window managers,
-     * but seems reasonable anyway). Make sure the top of the window
-     * isn't off the bottom of the screen, or so close to the bottom
-     * that it might be obscured by the panel.
-     *
-     */
-    root_window = gdk_screen_get_root_window (gdk_screen_get_default());
-    display = gdk_display_get_default ();
-    gdk_monitor_get_workarea(gdk_display_get_monitor_at_window (display, root_window), &workarea);
-    *top = CLAMP (*top, 0, workarea.height - MINIMUM_ON_SCREEN_HEIGHT);
-
-    /* FIXME bugzilla.eazel.com 669:
-     * If window has negative left coordinate, set_uposition sends it
-     * somewhere else entirely. Not sure what level contains this bug (XWindows?).
-     * Hacked around by pinning the left edge to zero, which just means you
-     * can't set a window to be partly off the left of the screen using
-     * this routine.
-     */
-    /* Make sure the left edge of the window isn't off the right edge of
-     * the screen, or so close to the right edge that it might be
-     * obscured by the panel.
-     */
-    *left = CLAMP (*left, 0, workarea.width - MINIMUM_ON_SCREEN_WIDTH);
+
+    screen = gdk_screen_get_default ();
+    display = gdk_screen_get_display (screen);
+    if  (GDK_IS_X11_DISPLAY (display))
+    {
+        scale = gdk_window_get_scale_factor (gdk_screen_get_root_window (screen));
+
+        /* Make sure the top of the window is on screen, for
+         * draggability (might not be necessary with all window managers,
+         * but seems reasonable anyway). Make sure the top of the window
+         * isn't off the bottom of the screen, or so close to the bottom
+         * that it might be obscured by the panel.
+         */
+        *top = CLAMP (*top, 0, HeightOfScreen (gdk_x11_screen_get_xscreen (screen)) / scale - MINIMUM_ON_SCREEN_HEIGHT);
+
+        /* FIXME bugzilla.eazel.com 669:
+         * If window has negative left coordinate, set_uposition sends it
+         * somewhere else entirely. Not sure what level contains this bug (XWindows?).
+         * Hacked around by pinning the left edge to zero, which just means you
+         * can't set a window to be partly off the left of the screen using
+         * this routine.
+         */
+        /* Make sure the left edge of the window isn't off the right edge of
+         * the screen, or so close to the right edge that it might be
+         * obscured by the panel.
+         */
+        *left = CLAMP (*left, 0, WidthOfScreen (gdk_x11_screen_get_xscreen (screen)) / scale - MINIMUM_ON_SCREEN_WIDTH);
+    }
+    else
+    {   /*Not running in x11, don't use x11 specific code*/
+        GdkRectangle geometry = {0};
+        GdkMonitor *monitor;
+        monitor = gdk_display_get_monitor (display, 0);
+        gdk_monitor_get_geometry(monitor, &geometry);
+        *top = CLAMP (*top, 0, geometry.height - MINIMUM_ON_SCREEN_HEIGHT);
+        *left = CLAMP (*left, 0, geometry.width - MINIMUM_ON_SCREEN_WIDTH);
+    }
 }

 static void

to:

--- a/eel/eel-gtk-extensions.c
+++ b/eel/eel-gtk-extensions.c
@@ -80,26 +80,26 @@ eel_gtk_window_get_geometry_string (GtkW
 static void
 sanity_check_window_position (int *left, int *top)
 {
+    GdkScreen *screen;
+    gint scale;
+    GdkDisplay *display;
+
     g_assert (left != NULL);
     g_assert (top != NULL);
-    GdkDisplay *display;
-    GdkWindow *root_window;
-    GdkRectangle workarea = {0};

-    /* Make sure the top of the window is on screen, for
-     * draggability (might not be necessary with all window managers,
-     * but seems reasonable anyway). Make sure the top of the window
-     * isn't off the bottom of the screen, or so close to the bottom
-     * that it might be obscured by the panel.
-     *
-     */
-    display = gdk_display_get_default ();
-    /*This is x11 only, there is no root window in wayland*/
+    screen = gdk_screen_get_default ();
+    display = gdk_screen_get_display (screen);
     if  (GDK_IS_X11_DISPLAY (display))
     {
-        root_window = gdk_screen_get_root_window (gdk_screen_get_default());
-        gdk_monitor_get_workarea(gdk_display_get_monitor_at_window (display, root_window), &workarea);
-        *top = CLAMP (*top, 0, workarea.height - MINIMUM_ON_SCREEN_HEIGHT);
+        scale = gdk_window_get_scale_factor (gdk_screen_get_root_window (screen));
+
+        /* Make sure the top of the window is on screen, for
+         * draggability (might not be necessary with all window managers,
+         * but seems reasonable anyway). Make sure the top of the window
+         * isn't off the bottom of the screen, or so close to the bottom
+         * that it might be obscured by the panel.
+         */
+        *top = CLAMP (*top, 0, HeightOfScreen (gdk_x11_screen_get_xscreen (screen)) / scale - MINIMUM_ON_SCREEN_HEIGHT);

         /* FIXME bugzilla.eazel.com 669:
          * If window has negative left coordinate, set_uposition sends it

After this modification, caja with the latest patch builds and seems to run as expected.

lukefromdc commented 1 year ago

I'm fine with applying it either before or after the wayland desktop wirk. I have it applied in my local copy in fact for testing

raveit65 commented 1 year ago

Can second commit be done in another PR, please? Because this isn't needed to to fix icon issue from https://github.com/mate-desktop/caja/issues/1728 . IHMO, this needs extra testing.

lukefromdc commented 1 year ago

Not a problem, can split them when I get back from the road trip. Won't conflict as they are differenr filea.

On 7/2/2023 at 6:56 AM, "raveit65" @.***> wrote:

Can second commit be done in another PR, please? Because this isn't needed to to fix icon issue from https://github.com/mate-desktop/caja/issues/1728 . IHMO, this needs extra testing.

-- Reply to this email directly or view it on GitHub: https://github.com/mate-desktop/caja/pull/1729#issuecomment- 1616599191 You are receiving this because you were mentioned.

Message ID: @.***>

lukefromdc commented 1 year ago

Also note that while a lot of the wayland code is usable in x11, we don't have enough testers to avoid the risk of new bugs if the x11 code changes. I am OK wirh either approach but suspect being conservative w changes on x11 is going to make the least problems down the road.

Wayland we cannot avoid that, but that no doubt be cinsidered experimenral for a while. Should attract substantial interest and hopefully testers once it is released though

raveit65 commented 1 year ago

@lukefromdc Is it normal that i can't move a caja browser window in wayfire? And are you using stock or gtk-shell under workarounds in wayfire-config-manager? offtopic off

lukefromdc commented 1 year ago

With wayfire's default CSD decoration for some reason caja's windows can only be moved with the (works for all windows all the time) super-left mouse button to move, super-right mouse button to resise.

If you go to ~/.config/wayfire.ini though you can replace CSD with SSD and windows move normally. I've been tryijng to find out what the hell makes caja different than pluma and mate-terminal on this without success.

A MATE-wayland session could of course specify SSD but wayfire does not yet support theming the decorations that way, probably will doiwn the road. Will also publish my wayfire.ini file

lukefromdc commented 1 year ago

Here is my wayfire.ini contents:


# Default config for Wayfire
#
# Copy this to ~/.config/wayfire.ini and edit it to your liking.
#
# Take the tutorial to get started.
# https://github.com/WayfireWM/wayfire/wiki/Tutorial
#
# Read the Configuration document for a complete reference.
# https://github.com/WayfireWM/wayfire/wiki/Configuration

# Input configuration ──────────────────────────────────────────────────────────

# Example configuration:
#
# [input]
# xkb_layout = us,fr
# xkb_variant = dvorak,bepo
#
# See Input options for a complete reference.
# https://github.com/WayfireWM/wayfire/wiki/Configuration#input

# Output configuration ─────────────────────────────────────────────────────────

# Example configuration:
#
[output:DP-1]
mode = 3840x2160@60000
position = 0,0
transform = normal
scale = 2.000000
#
# You can get the names of your outputs with wlr-randr.
# https://github.com/emersion/wlr-randr
#
# See also kanshi for configuring your outputs automatically.
# https://wayland.emersion.fr/kanshi/
#
# See Output options for a complete reference.
# https://github.com/WayfireWM/wayfire/wiki/Configuration#output

# Core options ─────────────────────────────────────────────────────────────────

[core]

# List of plugins to be enabled.
# See the Configuration document for a complete list.
plugins = \
  alpha \
  animate \
  autostart \
  command \
  cube \
  decoration \
  expo \
  fast-switcher \
  fisheye \
  foreign-toplevel \
  grid \
  gtk-shell \
  idle \
  invert \
  move \
  oswitch \
  place \
  resize \
  switcher \
  vswitch \
  window-rules \
  wm-actions \
  wrot \
  zoom

#  wobbly \
# Note: [blur] is not enabled by default, because it can be resource-intensive.
# Feel free to add it to the list if you want it.
# You can find its documentation here:
# https://github.com/WayfireWM/wayfire/wiki/Configuration#blur

# Close focused window.
close_top_view = <super> KEY_Q | <alt> KEY_F4

# Workspaces arranged into a grid: 3 × 3.
vwidth = 4
vheight = 1

# Prefer client-side decoration or server-side decoration
preferred_decoration_mode = server

xwayland = true

# Mouse bindings ───────────────────────────────────────────────────────────────

# Drag windows by holding down Super and left mouse button.
[move]
activate = <super> BTN_LEFT

# Resize them with right mouse button + Super.
[resize]
activate = <super> BTN_RIGHT

# Zoom in the desktop by scrolling + Super.
[zoom]
modifier = <super>

# Change opacity by scrolling with Super + Alt.
[alpha]
modifier = <super> <alt>

# Rotate windows with the mouse.
[wrot]
activate = <super> <ctrl> BTN_RIGHT

# Fisheye effect.
[fisheye]
toggle = <super> <ctrl> KEY_F

# Startup commands ─────────────────────────────────────────────────────────────

[autostart]

# Automatically start background and panel.
# Set to false if you want to override the default clients.
autostart_wf_shell = false

# Set the wallpaper, start a panel and dock if you want one.
# https://github.com/WayfireWM/wf-shell
#
# These are started by the autostart_wf_shell option above.
#
background = wf-background
panel = mate-panel
# dock = wf-dock

# Output configuration
# https://wayland.emersion.fr/kanshi/
#outputs = kanshi

# Notifications
# https://wayland.emersion.fr/mako/
#notifications = mako

# Screen color temperature
# https://sr.ht/~kennylevinsen/wlsunset/
#gamma = wlsunset

# Idle configuration
# https://github.com/swaywm/swayidle
# https://github.com/swaywm/swaylock
# idle = swayidle before-sleep swaylock

XDG desktop portal
# Needed by some GTK applications
portal = /usr/libexec/xdg-desktop-portal

# Example configuration:
#
# [idle]
# toggle = <super> KEY_Z
# screensaver_timeout = 300
# dpms_timeout = 600
#
# Disables the compositor going idle with Super + z.
# This will lock your screen after 300 seconds of inactivity, then turn off
# your displays after another 300 seconds.

# Applications ─────────────────────────────────────────────────────────────────

[command]

# Start a terminal
# https://github.com/alacritty/alacritty
binding_terminal = <super> KEY_ENTER
command_terminal = mate-terminal

# Start your launcher
# https://hg.sr.ht/~scoopta/wofi
# Note: Add mode=run or mode=drun to ~/.config/wofi/config.
# You can also specify the mode with --show option.
#binding_launcher = <super> <shift> KEY_ENTER
#command_launcher = wofi

# Screen locker
# https://github.com/swaywm/swaylock
#binding_lock = <super> <shift> KEY_ESC
#command_lock = swaylock

# Logout
# https://github.com/ArtsyMacaw/wlogout
binding_logout = <super> KEY_ESC
command_logout = wlogout

# Screenshots
# https://wayland.emersion.fr/grim/
# https://wayland.emersion.fr/slurp/
binding_screenshot = KEY_PRINT
command_screenshot = grim $(date '+%F_%T').webp
binding_screenshot_interactive = <shift> KEY_PRINT
command_screenshot_interactive = slurp | grim -g - $(date '+%F_%T').webp

# Volume controls
# https://alsa-project.org
repeatable_binding_volume_up = KEY_VOLUMEUP
command_volume_up = amixer set Master 5%+
repeatable_binding_volume_down = KEY_VOLUMEDOWN
command_volume_down = amixer set Master 5%-
binding_mute = KEY_MUTE
command_mute = amixer set Master toggle

# Screen brightness
# https://haikarainen.github.io/light/
repeatable_binding_light_up = KEY_BRIGHTNESSUP
command_light_up = light -A 5
repeatable_binding_light_down = KEY_BRIGHTNESSDOWN
command_light_down = light -U 5

# Windows ──────────────────────────────────────────────────────────────────────

# Actions related to window management functionalities.
#
# Example configuration:
#
[wm-actions]
# toggle_fullscreen = <super> KEY_F
# toggle_always_on_top = <super> KEY_X
toggle_sticky = <super> KEY_X

# Position the windows in certain regions of the output.
[grid]
#
# ⇱ ↑ ⇲   │ 7 8 9
# ← f →   │ 4 5 6
# ⇱ ↓ ⇲ d │ 1 2 3 0
# ‾   ‾
slot_bl = <super> KEY_KP1
slot_b = <super> KEY_KP2
slot_br = <super> KEY_KP3
slot_l = <super> KEY_LEFT | <super> KEY_KP4
slot_c = <super> KEY_UP | <super> KEY_KP5
slot_r = <super> KEY_RIGHT | <super> KEY_KP6
slot_tl = <super> KEY_KP7
slot_t = <super> KEY_KP8
slot_tr = <super> KEY_KP9
# Restore default.
restore = <super> KEY_DOWN | <super> KEY_KP0

# Change active window with an animation.
[switcher]
next_view = <alt> KEY_TAB
prev_view = <alt> <shift> KEY_TAB

# Simple active window switcher.
[fast-switcher]
activate = <alt> KEY_ESC

# Workspaces ───────────────────────────────────────────────────────────────────

# Switch to workspace.
[vswitch]
binding_left = <ctrl> <super> KEY_LEFT
binding_down = <ctrl> <super> KEY_DOWN
binding_up = <ctrl> <super> KEY_UP
binding_right = <ctrl> <super> KEY_RIGHT
# Move the focused window with the same key-bindings, but add Shift.
with_win_left = <ctrl> <super> <shift> KEY_LEFT
with_win_down = <ctrl> <super> <shift> KEY_DOWN
with_win_up = <ctrl> <super> <shift> KEY_UP
with_win_right = <ctrl> <super> <shift> KEY_RIGHT

# Show the current workspace row as a cube.
[cube]
activate = <ctrl> <alt> BTN_LEFT
# Switch to the next or previous workspace.
rotate_left =  <ctrl> <alt> KEY_LEFT
rotate_right =  <ctrl> <alt> KEY_RIGHT

# Show an overview of all workspaces.
[expo]
toggle = <super>
# Select a workspace.
# Workspaces are arranged into a grid of 3 × 3.
# The numbering is left to right, line by line.
#
# ⇱ k ⇲
# h ⏎ l
# ⇱ j ⇲
# ‾   ‾
# See core.vwidth and core.vheight for configuring the grid.
select_workspace_1 = KEY_1
select_workspace_2 = KEY_2
select_workspace_3 = KEY_3
select_workspace_4 = KEY_4
select_workspace_5 = KEY_5
select_workspace_6 = KEY_6
select_workspace_7 = KEY_7
select_workspace_8 = KEY_8
select_workspace_9 = KEY_9

# Outputs ──────────────────────────────────────────────────────────────────────

# Change focused output.
[oswitch]
# Switch to the next output.
next_output = <super> KEY_O
# Same with the window.
next_output_with_win = <super> <shift> KEY_O

# Invert the colors of the whole output.
[invert]
toggle = <super> KEY_I

# Rules ────────────────────────────────────────────────────────────────────────

# Example configuration:
#
[window-rules]
#caja = on created if title is "desktop" then send_to_back
desktop1 on created if app_id is "caja" then sticky
#
# You can get the properties of your applications with the following command:
# $ WAYLAND_DEBUG=1 alacritty 2>&1 | kak
#
# See Window rules for a complete reference.
# https://github.com/WayfireWM/wayfire/wiki/Configuration#window-rules
lukefromdc commented 1 year ago

Done, the othe commits are now split out into https://github.com/mate-desktop/caja/pull/1730

raveit65 commented 1 year ago

@mate-desktop/core-team Review please?

lukefromdc commented 1 year ago

In x11 using the code we've used for years avoids new bugs. In wayland we need new code, and for the desktop must remember the root window does not exist in wayland

lukefromdc commented 1 year ago

Is anyone else willing to test and review this?

zhuyaliang commented 1 year ago

I don't know much about wayland, let someone else review the code

lukefromdc commented 1 year ago

This is mostly about preserving the original x11 behavior

raveit65 commented 1 year ago

It's only a restore of the old code, so let's merging.