mate-desktop / caja

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

wayland: initial support for showing the desktop #1722

Closed lukefromdc closed 1 year ago

lukefromdc commented 1 year ago

Uses gtk-layer-shell to keep the desktop always on the bottom Uses gtk-layer-shell to keep the desktop on all workpaces Depends on two changes in mate-desktop package We can later make wayland support optional here due to new dependency on gtk-layer-shell

*Depends on https://github.com/mate-desktop/mate-desktop/pull/558

lukefromdc commented 1 year ago

Out of time tonight but took a copy of the travis errors. Builds finished here fine but travis found an issue in eel-canvas.c that should predate this PR as that file was not modified.

Note that with this and it's matching mate-desktop PR, plus current panel and the applets I am working on, I now have a mate-wayfire session that is becoming increasingly difficult to tell from my x11 session! This was supposed to be the hardest part, and I have working desktop icons in wayland, with the desktop window staying under the others and rendering on all workspaces. Interaction with it seems normal.

This will need to be checked over to make sure NO changes to x11 behavior occur and I've made as few x11 codepath changes as possible for that reason. I have to move a function though to avoid a segfault on trying to detect the monitor too soon. The desktop code is a bit racy, and that caused problems in two areas

lukefromdc commented 1 year ago

travis build failures came from an inabiity to find gtk-layer-shell. If this is a problem in distros wayland support will have to be made optional for those who have this available

lukefromdc commented 1 year ago

Mate-Wayland_6--20-2023

This is caja managing a wayland desktop. Note that mate-panel can be made the DEFAULT panel in wayfire

raveit65 commented 1 year ago

Build dependency gtk-layer-shell is missing in CI. https://app.travis-ci.com/github/mate-desktop/caja/jobs/604390722#L951

configure: error: Package requirements (

    gdk-pixbuf-2.0 >= 2.36.5

    glib-2.0 >= 2.58.1

    mate-desktop-2.0 >= 1.17.3

    gthread-2.0

    gio-unix-2.0

    gio-2.0 >= 2.50.0

    pango >= 1.1.2

    gtk+-3.0 >= 3.22.0

    gtk-layer-shell-0 >= gtk-layer-shell-0_minver

    libnotify

    libxml-2.0 >= 2.4.7

    gail-3.0 >= 3.0.0

) were not met:

Package 'gtk-layer-shell-0', required by 'virtual:world', not found

Take a look a mate-panel how it looks. For example debian https://github.com/mate-desktop/mate-panel/blob/master/.build.yml#L44

lukefromdc commented 1 year ago

I just noticed gtk3-devel also missing from Fedora build of caja but present in mate-panel. Will add that too, we can remove it if it's not in fact necessary

lukefromdc commented 1 year ago

Looks like all the Travis builds now worked

lukefromdc commented 1 year ago

Just added code to use gtk-layer-shell to always anchor the desktop to all four edges, a simple approach to ensuring the desktop always follows any screen resolution changes.

Also make wayland support optional, so as not to force a gtk-layer-shell dependency on builds not intended for use with wayland. Note that the earlier code to put "x-isms" behind x11-only selectors allows navigation windows to run in wayland without any wayland libs, and using the desktop on wayland (as it is outside a mate-session) requires passing --force-desktop to caja on invocation

lukefromdc commented 1 year ago

Just ran some tests with wlr-randr which does in wlroots based compositors what xrandr does in x11. Always the desktop spanned the monitor, and even fading to the new and correct background image size worked.

lukefromdc commented 1 year ago

Just rebased after https://github.com/mate-desktop/caja/pull/1724 merged

lukefromdc commented 1 year ago

I had to do some fiddling to get a rational multimonitor setup in wayfire but once I did, caja would render a desktop on the leftmost monitor (my largest and set with wlr-randr to position 0,0) just fine

lukefromdc commented 1 year ago

Rebased after https://github.com/mate-desktop/caja/commit/126a4a2a3ad3d93088644406936a2499f7be18be

lukefromdc commented 1 year ago

No it is NOT,, that looks like it wil cause translation problems!

On 7/2/2023 at 12:51 PM, "raveit65" @.***> wrote:

@raveit65 commented on this pull request.

@@ -28,8 +28,10 @@

include <gdk/gdkx.h>

include <gtk/gtk.h>

include <gio/gio.h>

-#include <glib/gi18n.h>

Is it correct to remove glib?

-- Reply to this email directly or view it on GitHub: https://github.com/mate-desktop/caja/pull/1722#pullrequestreview- 1509634099 You are receiving this because you authored the thread.

Message ID: <mate- @.***>

lukefromdc commented 1 year ago

Probably bad cut and paste while bringing the code over from mate-panel thanks for catching it.

On 7/2/2023 at 12:47 PM, "raveit65" @.***> wrote:

@raveit65 commented on this pull request.

@@ -78,7 +79,7 @@ PKG_CHECK_MODULES(ALL, [ gio-unix-2.0 gio-2.0 >= gio_minver pango >= pango_minver

  • gtk+-3.0 >= gtk_minver
  • gtk-layer-shell-0 >= gtk-layer-shell-0_minver

Why gtk+-3.0 dependency is removed?

-- Reply to this email directly or view it on GitHub: https://github.com/mate-desktop/caja/pull/1722#pullrequestreview- 1509633683 You are receiving this because you authored the thread.

Message ID: <mate- @.***>

lukefromdc commented 1 year ago

brought back #include <glib/gi18n.h>

raveit65 commented 1 year ago

With a quick test this works for x11 and wayland. My wayfire desktop: screenshot-2023-07-02-20:05:37 Seems caja don't handle the desktop from my second monitor.

Sadly, i lost always my scaling factor1.7 when i change something in wayfire.ini or with wayfire-config-tool, which is annoying. Also with session start the scaling factor will be ignored.

But for a first raw implementation it looks good. Code review i will do the next week. @mate-desktop/core-team Your reviews are wanted!

lukefromdc commented 1 year ago

I'm not getting icons on a second monitor with any recent version of caja, including from before I started the wayland work.

raveit65 commented 1 year ago

Really, this was always working with x11 and marco.

lukefromdc commented 1 year ago

Might be a compiz issue in my case, I will check that out when I get home. Also I have a ton of old caja debs and might be able to find out when it started acting up if it's not compiz in some way. Background renders just fine, I just can't move icons to the 2nd window x11 or Wayland and rolling back to an early 2023 version didn't fix it

raveit65 commented 1 year ago

I will check again code-format when all code changes are done. During testing i found an issue with caja. open-in-terminal command from content menu crashes caja in wayfire. From desktop or inside the browser window. Edit: Can be related to the extension package.

lukefromdc commented 1 year ago

I was able to successfully open terminals using the open-in-terminal menu item

lukefromdc commented 1 year ago

Changes pushed and ready for testing

raveit65 commented 1 year ago

offtop on Caja crashes for me when using open-terminal extension in wayland. I mean this menu entry. (sorry german,but it should be clear, screenshot from X11) caja_open_terminal Still wondering that it works for you. But i have a stacktrace, starts with libxcb ! https://www.dropbox.com/s/z089gb5kadj0cni/backtrace_caja_wayland_open_terminal?dl=0 Maybe you guys could take a look at it? Should i open a report for it? offtopic off

Sunderland93 commented 1 year ago

Should i open a report for it?

I can confirm same crash on Debian 12 on Wayland (caja built from current git master)

cwendling commented 1 year ago

Looks a bit like libstartup-notification is choosing an X backend and crashes somehow… I can't tell much more like that, but it's a bit weird… not saying it should work, but XWayland should probably kick in and pretend everything's alright?

cwendling commented 1 year ago

And yeah, that should probably be another report, it looks like an issue either in libstartup-notification or in libmate-desktop, but doesn't seem like a bug in Caja itself

lukefromdc commented 1 year ago

At leaat in Debian, wayfire is being shipped with xwayland disabled for some reason.To switch my whole session over will require a local builf if this does not change, so as to allow kdenlive and audacious to run.

A dev build of GIMP using gtk3 would presumably work

lukefromdc commented 1 year ago

About the terminal extension: xcb should not be in the wayland codepath at all. String searchws for "xcb" and "x11" in caja-extensions/open-terminal find nothing. In caja/libcaja-extension I found it only inCORE_LIBS in the finished Makefile and nowhere else, as an included library.

c->has_error from the stacktrace doesn't turn up in either source directory either, so the question is why someone else gets a crash caused by invoking something from xcb and I do not.

A fix for this will probably be for another PR

lukefromdc commented 1 year ago

Next round of changes pushed and ready for testing

raveit65 commented 1 year ago

Did test it again today and crash in X11 is gone. I suggest to wait some days before merging, so that PR can be better tested in daily use on my system or by other testers.

In general i don't think that wayfire compositor is a stable platform for MATE on wayland. There config system based on a text file doesn't work very well here. I don't think that all settings in wayfire.ini will be applied with starting the session. For example the scaling factor for HIDPI is still ignored here or i did mistakes in my setup. Also, i don't think that all there effects like the ugly wobbly windows or the cube are really needed for a clean and silent production system ;) When i found some time i will take a look at mir or sway. Anyway, nice job with this PR It's a good starting point for caja on wayland.

lukefromdc commented 1 year ago

Thanks, I think wayfire will be rather like compiz was in the 2008 era: not the baseline compositor for the reasons you describe but one that can be substituted.

Note that the wlr-randr package can change the monitor resolution at runtime, and it would not need to autostart anything started by a MATE-wayland session, so it should be drop-in compatable with any other wlroots-based compositor. GNOME 2 and MATE therafter on x11 have always been modular in this way.

No problem with running this a while before merge, so as to find hidden problems

lukefromdc commented 1 year ago

Found and fixed that last style issue, thanks

raveit65 commented 1 year ago

Looks a bit like libstartup-notification is choosing an X backend and crashes somehow… I can't tell much more like that, but it's a bit weird… not saying it should work, but XWayland should probably kick in and pretend everything's alright?

Open report https://github.com/mate-desktop/caja/issues/1731

lukefromdc commented 1 year ago

For some reason at least Debian is shipping wayfire without xwayland support

L-U-T-i commented 1 year ago

PR #1730 cannot be applied after this PR - the problematic part of this PR is:.

From 2d1c397a4ccd161b255282a908b2aa0dbb9d4317 Mon Sep 17 00:00:00 2001
From: lukefromdc <lukefromdc@hushmail.com>
Date: Wed, 5 Jul 2023 14:53:08 -0400
Subject: [PATCH 2/4] eel-editable-label-c: remove unused variable

This was leftover from earlier code compatable only with x11
---
 eel/eel-editable-label.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/eel/eel-editable-label.c b/eel/eel-editable-label.c
index 92ce70154..7c4bcb38f 100644
--- a/eel/eel-editable-label.c
+++ b/eel/eel-editable-label.c
@@ -1028,7 +1028,6 @@ eel_editable_label_ensure_layout (EelEditableLabel *label,
             else
             {
                 gint wrap_width;
-                gint scale;

                 pango_layout_set_width (label->layout, -1);
                 pango_layout_get_extents (label->layout, NULL, &logical_rect);
@@ -1039,7 +1038,6 @@ eel_editable_label_ensure_layout (EelEditableLabel *label,
                 longest_paragraph = width;

                 wrap_width = get_label_wrap_width (label);
-                scale = gtk_widget_get_scale_factor (widget);
                 width = MIN (width, wrap_width);
                 width = MIN (width,
                 PANGO_SCALE * (gdk_screen_width () + 1) / 2);

Is it OK that we remove 'scale' here? I believe 'scale' is still needed, considering we use it in X11 code...

lukefromdc commented 1 year ago

I can manually remake https://github.com/mate-desktop/caja/pull/1730 after this, no problem. Scale in that case was being flagged as unused, the code path there is same for x11 and wayland and the PANGO_SCALE line seems to just plain work

L-U-T-i commented 1 year ago

But if you apply #1730 after that, it needs scale (so it is not unused anymore...).

L-U-T-i commented 1 year ago

I've manually modified this patch to keep scale (so not to modify eel/eel-editable-label.c at all) and still had to modify #1730 as well (adapt for a modified eel/eel-gtk-extensions.c). Build went well after that.

raveit65 commented 1 year ago

GDK knows about scaling. Only using Width/HeightOfScreen, which are xlib macros, needs the scale variable. The goal should be to avoid re-adding Width/HeightOfScreen and replacing gdk stuff, which can be use for x11 and wayland. So current state of https://github.com/mate-desktop/caja/pull/1730 is a bit weird, ihmo.

lukefromdc commented 1 year ago

Closed https://github.com/mate-desktop/caja/pull/1730 based on discussions elsewhere-and the likelihood that the windowing system agnostic gdk code may be better maintained going forward

Sunderland93 commented 1 year ago

After applying this patch and run Caja with caja --force-desktop -n wallpaper is turned black. When I tried to set another desktop wallpaper, it would appear for a couple of seconds and turn back to black. "Change desktop background" option in right-clicking context menu also doesn't work. Am I miss something?

raveit65 commented 1 year ago

Actual only the wayfire background modul is working. In wayfire.ini you need: background = wf-background My autostart section:

[autostart]
a1 = /usr/libexec/mate-settings-daemon
a6 = caja --force-desktop -n
a7 = nm-applet --indicator
autostart_wf_shell = false
background = wf-background
idle = swayidle before-sleep swaylock
panel = mate-panel
portal = /usr/libexec/xdg-desktop-portal

The background modul of mate-settings-daemon needs a complete re-work for wayland, and......and......a lot of ands..... :)

Sunderland93 commented 1 year ago

Ah, I see. Well, I use LabWC, so my option is swaybg :) Anyway, thank you! Everything else works well.

Sunderland93 commented 1 year ago

[autostart] a1 = /usr/libexec/mate-settings-daemon

On my setup mate-settings-daemon doesn't work at all, it crashes after startup with

invalid cast from 'GdkWaylandDisplay' to 'GdkX11Display'
Segmentation fault
raveit65 commented 1 year ago

Only a few modules of m-s-d are working. 3 or 4..... You can disable the plugins which do not work with dconf-editor. First disable all and than enable each plugin and see if it crash or not.

Sunderland93 commented 1 year ago

Thanks, I've disabled modules that causes crash. Now it works.

lukefromdc commented 1 year ago

Is this good to go for merging to master (which seems to get more 3ed party testing) or do you all want to wait longer for anyone else on the team to review it?