panel: Use the wlroots backend with unknown Wayland compositors by Conan-Kudo · Pull Request #2161 · lxqt/lxqt-panel

Conan-Kudo · 2024-11-03T11:50:45Z

The description of the LXQtPanelApplicationPrivate::loadBackend() method already states that if we cannot identify the correct backend for a Wayland session, we should fall back to the wlroots backend module. This is reasonable given how broadly the wlr protocols are implemented. However, this was not happening due to a small error where we checked for the wlroots string in XDG_CURRENT_DESKTOP, which is not a valid option.

This change fixes this by checking XDG_SESSION_TYPE for "wayland" instead. As this is fallback logic, this is safe as the preferred backend logic using XDG_CURRENT_DESKTOP still wins over this.

The description of the LXQtPanelApplicationPrivate::loadBackend() method already states that if we cannot identify the correct backend for a Wayland session, we should fall back to the wlroots backend module. This is reasonable given how broadly the wlr protocols are implemented. However, this was not happening due to a small error where we checked for the wlroots string in XDG_CURRENT_DESKTOP, which is not a valid option. This change fixes this by checking XDG_SESSION_TYPE for "wayland" instead. As this is fallback logic, this is safe as the preferred backend logic using XDG_CURRENT_DESKTOP still wins over this.

tsujan · 2024-11-03T12:53:00Z

    }

-    if ( preferredBackend.isEmpty() && xdgCurrentDesktops.contains( QStringLiteral("wlroots") ) )
+    if ( preferredBackend.isEmpty() && xdgSessionType == QStringLiteral("wayland") ) )


The wlroots backend should be used only if wlroots is included. That inclusion means, "we know that it works with this Wayland compositor." The dummy backed should be used with unsupported compositors.

The wlroots backend should be used only if wlroots is included. That inclusion means, "we know that it works with this Wayland compositor." The dummy backed should be used with unsupported compositors.

Well, "unsupported" here means we ship no configuration for it and session exit etc may not work, but I think we can still load the wlroots backend for them. If an user starts an unsupported compositor by its own it's up to him.

The wlroots backend is what its name says; it's not the fallback. If we know that a compositor works with it — even when it isn't based on wlroots — we should include it in this category explicitly.

This PR is wrong and can create a mess.

tsujan · 2024-11-03T12:59:51Z

As this is fallback logic

Only for compositors that are compatible with wlroots, as far as lxqt-panel is concerned. Otherwise:

lxqt-panel/panel/lxqtpanelapplication.cpp

Lines 306 to 313 in 5ce16ac

    
           // If no backend can be found fall back to dummy backend 
        
           loader.unload(); 
        
           mWMBackend = new LXQtDummyWMBackend; 
        
           qWarning() << "\n" 
        
                      << "ERROR: Could not create a backend for window managment operations.\n" 
        
                      << "Falling back to dummy backend. Some functions will not be available.\n" 
        
                      << "\n";

Conan-Kudo · 2024-11-03T13:16:57Z

That's not what the method description says:

lxqt-panel/panel/lxqtpanelapplication.cpp

Lines 193 to 203 in 246b33f

    
               /** 
        
                * 1. Get the XDG_CURRENT_DESKTOP. It's a colon separate list. 
        
                * 2. Get the preferredBackend. It's a comma separated list. 
        
                * 3. First attempt to match some value in XDG_CURRENT_DESKTOP with any value in preferredBackend. 
        
                * 4. If it matches, end of story. Else, we attempt to deduce the backend based on XDG_CURRENT_DESKTOP: 
        
                *    a. X11 -> xcb 
        
                *    b. kwin_wayland -> plasma 
        
                *    c. wayfire -> wayfire 
        
                *    d. wayland -> wlroots 
        
                *    e. other -> dummy 
        
                */

tsujan · 2024-11-03T13:36:10Z

It's a code comment :) What exists other than X11 and Wayland?

The code isn't final of course — more generic backends might be added in future — but for now, several good compositors are compatible with wlroots (except for kwin_wayland, which has its own backend; I like it very much).

Conan-Kudo · 2024-11-03T13:40:50Z

Yeah. I like KWin too. 😄

But yeah, I think it's reasonable to use the wlroots backend as a wayland fallback because outside of KWin; Mutter; and Weston, pretty much all compositors offer the wlr protocols that the backend depends on.

And we already have a KWin backend, if someone actually wants to use Mutter or Weston, then backends can be made for them. 😂

tsujan · 2024-11-03T13:48:20Z

it's reasonable to use the wlroots backend as a wayland fallback

Until the current disarray comes to an end in the Wayland world, we should thread carefully, IMO.

Two years ago we didn't imagine that we could support 7 well-known Wayland compositors so soon. Yes, the situation of Wayland has got better but not completely.

Also see how "wlroots" is handled in another part of the code:

lxqt-panel/panel/backends/wayland/wlroots/lxqtwmbackend_wlr.cpp

Lines 609 to 643 in 5ce16ac

    
           int LXQtWMBackendWlrootsLibrary::getBackendScore(const QString& key) const 
        
           { 
        
           	if (key == QStringLiteral("wlroots")) 
        
           	{ 
        
           		return 50; 
        
           	} 
        
           	else if (key == QStringLiteral("wayfire")) 
        
           	{ 
        
           		return 30; 
        
           	} 
        
           	else if (key == QStringLiteral("sway")) 
        
           	{ 
        
           		return 30; 
        
           	} 
        
           	else if (key == QStringLiteral("hyprland")) 
        
           	{ 
        
           		return 30; 
        
           	} 
        
           	else if (key == QStringLiteral("labwc")) 
        
           	{ 
        
           		return 30; 
        
           	} 
        
           	else if (key == QStringLiteral("river")) 
        
           	{ 
        
           		return 30; 
        
           	} 
        
           	// Unsupported 
        
           	return 0; 
        
           }

Conan-Kudo · 2024-11-03T13:59:27Z

Why does this method exist? We already select to load it from the panel frontend side?

tsujan · 2024-11-03T14:26:58Z

The backend is chosen in two ways: by checking the key preferred_backend in panel's config file (its value is a string list), or if it doesn't exist, by checking the scores of what is included in XDG_CURRENT_DESKTOP. This logic will work fine if more backends are added.

BTW, we need to add a dedicated backend for labwc because it supports cosmic-workspace-unstable-v1 now.

Conan-Kudo · 2024-11-03T14:51:29Z

Sure, but does that mean that the wlroots backend isn't going to let itself be used as the least-common-denominator panel backend?

tsujan · 2024-11-03T15:31:09Z

As the lowest common denominator for the compositors that are announced to be compatible with wlroots by lxqt-wayland-session — without raising false hopes.

Conan-Kudo · 2024-11-03T15:35:52Z

Then we should use a separate variable for this, because XDG_CURRENT_DESKTOP isn't the right place for it.

tsujan · 2024-11-03T15:53:55Z

This logic was a result of our developers' hard works and long discussions. It's proven to work fine and flexibly.

That being said, it isn't written in stone. If another method is shown to be better in practice, it could be replaced.

Anyway, LXQt 2.1.0 will be released with it the day after tomorrow.

stefonarch · 2024-11-04T06:30:29Z

Then we should use a separate variable for this, because XDG_CURRENT_DESKTOP isn't the right place for it.

We could just read the compositor in use as in startlxqtwayland.

tsujan · 2024-11-04T07:53:26Z

We could just read the compositor in use as in startlxqtwayland.

No, it's not that simple. We've used XDG_CURRENT_DESKTOP in other parts of the code because we agreed on it. For example:

lxqt-panel/panel/lxqtpanel.cpp

Lines 262 to 270 in 5ce16ac

    
           #if (QT_VERSION >= QT_VERSION_CHECK(6,8,0)) 
        
                       // WARNING: Only the following desktops are known to give the focus to child popups 
        
                       // when the panel does not accept focus. 
        
                       const QRegularExpression desktops(QStringLiteral("(?i)(kde|kwin|wayfire|hyprland)")); 
        
                       if (desktops.match(qEnvironmentVariable("XDG_CURRENT_DESKTOP")).hasMatch()) 
        
                           mLayerWindow->setKeyboardInteractivity(LayerShellQt::Window::KeyboardInteractivityNone); 
        
                       else 
        
           #endif

tsujan · 2024-11-04T08:22:52Z

Using XDG_CURRENT_DESKTOP is necessary because it isn't limited to compositors. For example, one may use LXQt panel inside Plasma Wayland. See

lxqt-panel/panel/backends/wayland/kwin_wayland/lxqtwmbackend_kwinwayland.cpp

Lines 800 to 803 in 5ce16ac

    
           if(key == QLatin1String("KDE") || key == QLatin1String("KWIN")) 
        
           { 
        
               return 100; 
        
           }

Moreover, a Wayland compositor is a DE too.

EDIT:
Please remind me to remove the case-sensitivity after 2.1.0 if I forget it, although it isn't a problem for us.

Conan-Kudo · 2025-02-21T13:34:22Z

Then we should use a separate variable for this, because XDG_CURRENT_DESKTOP isn't the right place for it.

We could just read the compositor in use as in startlxqtwayland.

Something like LXQT_PANEL_WAYLAND_BACKEND as a selection variable would make sense instead of overloading XDG_CURRENT_DESKTOP.

This was referenced Nov 3, 2024

startlxqtwayland: Correctly identify Niri and Hyprland lxqt/lxqt-wayland-session#39

Open

Add initial support for Miriway lxqt/lxqt-wayland-session#40

Open

tsujan requested changes Nov 3, 2024

View reviewed changes

Conan-Kudo mentioned this pull request Jan 16, 2025

Add option for Miriway lxqt/lxqt-session#571

Open

Uh oh!

Conversation

Conan-Kudo commented Nov 3, 2024

Uh oh!

tsujan Nov 3, 2024

Choose a reason for hiding this comment

Uh oh!

stefonarch Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tsujan Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

tsujan commented Nov 3, 2024

Uh oh!

Conan-Kudo commented Nov 3, 2024

Uh oh!

tsujan commented Nov 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Conan-Kudo commented Nov 3, 2024

Uh oh!

tsujan commented Nov 3, 2024

Uh oh!

Conan-Kudo commented Nov 3, 2024

Uh oh!

tsujan commented Nov 3, 2024

Uh oh!

Conan-Kudo commented Nov 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tsujan commented Nov 3, 2024

Uh oh!

Conan-Kudo commented Nov 3, 2024

Uh oh!

tsujan commented Nov 3, 2024

Uh oh!

stefonarch commented Nov 4, 2024

Uh oh!

tsujan commented Nov 4, 2024

Uh oh!

tsujan commented Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Conan-Kudo commented Feb 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

stefonarch Jan 17, 2025 •

edited

Loading

tsujan commented Nov 3, 2024 •

edited

Loading

Conan-Kudo commented Nov 3, 2024 •

edited

Loading

tsujan commented Nov 4, 2024 •

edited

Loading

Conan-Kudo commented Feb 21, 2025 •

edited

Loading