Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wlroots backend #2088

Merged
merged 43 commits into from
Aug 27, 2024
Merged

Wlroots backend #2088

merged 43 commits into from
Aug 27, 2024

Conversation

marcusbritanicus
Copy link
Contributor

@marcusbritanicus marcusbritanicus commented Aug 26, 2024

Requires

  1. Initial Window Manager backend plugin infrastructure #2075
  2. Wayland taskbar support v2 #2043

New Features

Two new features have been implemented (a) backend chooser (b) wlroots plugin

Backend chooser

Based on XDG_CURRENT_DESKTOP and XDG_SESSION_TYPE decides the best plugin to use. The selection is done based on the backend score returned by each plugin. If the user has a preference for particular plugin for a particular compositor, that needs to be defined in panel.conf, under [General] as

[General]
preferred_backend=wayfire:wfipc,kwin_wayland:libkwin_plugin.so

Notes:

  • If, for example, wfipc plugin is not loadable/does not exist, the panel will attempt to load a suitable plugin (Log entry will be made).
  • When specifying the plugin as <platform>:<plugin>, the panel searches for libwmbackend_<plugin>.so file in LXQTPANEL_PLUGIN_PATH. If the filename is different, specify the name only: <platform>:libmy_amazing_plugin.so

wlroots backend

This is the plugin for all compositors that support wlr_foreign_toplevel_management protocol. The backend score for <platform> == wlroots is 50, where as, the backend score for all platforms that support this protocol (wayfire, sway, hyprland, labwc and river) is 30. The reason for lowered score is so that platform-specific plugins can be developed for these, which provide a much better support.
Currently, wlroot backend supports the following "features":

  1. View mapped (E)
  2. View closed (E|R)
  3. View active state changed (E|R)
  4. View (un)maximized (E|R)
  5. View (un)minimized (E|R)
  6. View (un)fullscreened (E|R)
    E -> Event, R -> Request

Both wayland and wlroots do not provide native (via protocols) support for workspaces.

gfgit and others added 18 commits July 9, 2024 16:42
- Fix X11 backend to return zero score on non-X11 platforms
If preferred backend is set try it first.
Do not set preferred backend automatically. It must be user choice.
- Split XDG_CURRENT_DESKTOP
- Skip LXQTPANEL_PLUGIN_PATH if empty
libwmbackend_<platform>.so
NOTE: works only on KWin

- Choose backend at runtime
- Windows filter logic is re-evaluated on window property changes

LXQtTaskBarPlasmaWindowManagment: implement showDesktop()

LXQtTaskbarWaylandBackend: do not show transient windows

LXQtTaskBarPlasmaWindowManagment: fix destructor TODO

TODO: is this correct?
Seems to call wl_proxy_destroy underneath

LXQtPanel: basic virtual desktop support on Plasma Wayland

Add desktop file to be recognized by KWin Wayland

NOTE: absolute path is needed inside .desktop file for this to work
      use CMake to get it.

- Prevent double dekstop file installed in autostart

LXQtTaskbarWaylandBackend: return only accepted windows

- reloadWindows() force removal and readding of windows

This fixes changing windows grouping settings and adding taskbar plugin
AFTER panel is started.
Both situations resulted in empty taskbar previously

LXQtTaskbarWaylandBackend: fix workspace logic

LXQtTaskbarWaylandBackend: fix workspace removal logic

LXQtTaskbarWaylandBackend: implement moving window to virtual desktop
workspace

LXQtPlasmaWaylandWorkspaceInfo: fix signedness comparison

CMake: move panel WM backends to separate libraries

LXQtTaskbarWaylandBackend: possibly fix crash on showDesktop for non-
KWin

Update license headers

LXQtTaskbarWaylandBackend: add dummy setDesktopLayout()

Implement LXQtWMBackendKWinWaylandLibrary

- Add Desktop Environment detection
TODO: show error message when not supported
- Add NoDisplay=true to .desktop file

CMake: rename autostart desktop variable
Copy link
Member

@stefonarch stefonarch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't find any issue in any compositor, including X11.

But I tested preferred_backend=kwin_wayland:libkwin_plugin.so and kwin_wayland:libwmbackend_kwin_wayland.so and on startlxqtwayland I get always "no backend specified, choosing drm`. Kwin works fine though.

EDIT: looks like this message isn't from the panel at all (quite obvious), now I started it from terminal and the text is different, repeating the entry in panel.conf and if invalid falling back to autodetect.

Panel backend: "libwmbackend_kwin_wayland.so"

Fixes an inconsistency in themes when they expect an expanding size policy,
which is what most plugins have set. Some themes that do this: KDE-Plasma,
Ambiance, Kvantum. Further stylesheet changes in some themes will be needed
to match themes exactly.
@tsujan
Copy link
Member

tsujan commented Aug 26, 2024

Great job!

I can test it only under labwc, wayfire and kwin_wayland — and currently I'm using it under labwc. It works as well as your previous PR here.

The rest of this comment isn't related to the whole work but three specific issues that I encountered with both old and new PRs. I emphasize that I'm talking about lxqt-panel under labwc.

First, when I minimize a single window by pressing its minimize button, its task button remains activated. I "fixed" that by adding m_pendingState.activated = false; to case ZWLR_FOREIGN_TOPLEVEL_HANDLE_V1_STATE_MINIMIZED: inside LXQtTaskbarWlrootsWindow::zwlr_foreign_toplevel_handle_v1_state.

Second, unlike in KWin, under labwc we have two separate task buttons for a window and its child dialog. That causes some inconsistensies. For example, if a single window and its child dialog is opened, opening a second window will correctly make their task buttons inactive, but if the second window disappears, the task button of the child dialog of the first window might not be activated, i.e., we might have the logically impossible case of an active window with no active task button.

I "fixed" the second problem by removing QHash<WId, WId> transients; and its related codes. In other words, I removed the concept of "transiency".

There is also an issue about multiple activated task buttons. I circumvented it by a workaround in most cases, but there are still rare cases of it. Sorry, I forgot how I reproduced it in the first place.

Copy link
Member

@stefonarch stefonarch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As colorpicker plugin doesn't work yet I'd say to leave this for a separate PR.

In kwin_backend this is missing as initial step to allow minimize|restore on left click. There is still an issue which can be chased after IMO.

The "move to prevoius monitor" items still need greeding out in wlroots.

The last 2 are just reminders which can be done also after merging for me.

@tsujan
Copy link
Member

tsujan commented Aug 26, 2024

As colorpicker plugin doesn't work yet I'd say to leave this for a separate PR.

Oh, certainly! Even if it worked, it should be moved to a separate PR.

@tsujan
Copy link
Member

tsujan commented Aug 26, 2024

If the changes to colorpicker are reversed, we could merge this. Then, fixes and other probable changes could be made in separate PRs.

@stefonarch, @marcusbritanicus, @gfgit
Do you agree with a single merge, or do you prefer three separate commits for #2043, #2075 and a rebased version of this PR?

gfgit and others added 20 commits August 27, 2024 14:51
- Fix X11 backend to return zero score on non-X11 platforms
If preferred backend is set try it first.
Do not set preferred backend automatically. It must be user choice.
- Split XDG_CURRENT_DESKTOP
- Skip LXQTPANEL_PLUGIN_PATH if empty
libwmbackend_<platform>.so
NOTE: works only on KWin

- Choose backend at runtime
- Windows filter logic is re-evaluated on window property changes

LXQtTaskBarPlasmaWindowManagment: implement showDesktop()

LXQtTaskbarWaylandBackend: do not show transient windows

LXQtTaskBarPlasmaWindowManagment: fix destructor TODO

TODO: is this correct?
Seems to call wl_proxy_destroy underneath

LXQtPanel: basic virtual desktop support on Plasma Wayland

Add desktop file to be recognized by KWin Wayland

NOTE: absolute path is needed inside .desktop file for this to work
      use CMake to get it.

- Prevent double dekstop file installed in autostart

LXQtTaskbarWaylandBackend: return only accepted windows

- reloadWindows() force removal and readding of windows

This fixes changing windows grouping settings and adding taskbar plugin
AFTER panel is started.
Both situations resulted in empty taskbar previously

LXQtTaskbarWaylandBackend: fix workspace logic

LXQtTaskbarWaylandBackend: fix workspace removal logic

LXQtTaskbarWaylandBackend: implement moving window to virtual desktop
workspace

LXQtPlasmaWaylandWorkspaceInfo: fix signedness comparison

CMake: move panel WM backends to separate libraries

LXQtTaskbarWaylandBackend: possibly fix crash on showDesktop for non-
KWin

Update license headers

LXQtTaskbarWaylandBackend: add dummy setDesktopLayout()

Implement LXQtWMBackendKWinWaylandLibrary

- Add Desktop Environment detection
TODO: show error message when not supported
- Add NoDisplay=true to .desktop file

CMake: rename autostart desktop variable
@marcusbritanicus
Copy link
Contributor Author

marcusbritanicus commented Aug 27, 2024

@tsujan Please check if I have done it correctly.

Edit: After struggling with it a couple of minutes, I just copied the contents of colorpicker.cpp/colorpicker.h from lxqt-panel/master and committed those changes.

@stefonarch
Copy link
Member

I noticed only now that "minimize-restore" is broken here on all wayland compositors.

I see no problem in labwc.

Sure? I mean the item in the button menu in the
taskbar. I rechecked the several packages I've and in wlroots it's working only in the very first package "wlroots-taskbar", while in kwin there are some more versions where it's working.

@marcusbritanicus
Copy link
Contributor Author

I just tested with both Wayfire and labwc. I have noticed problems on both.

@tsujan
Copy link
Member

tsujan commented Aug 27, 2024

I just tested with both Wayfire and labwc. I have noticed problems on both.

The problem @stefonarch described? Why don't I see it here then?!

EDIT: There's a small possibility that one of my personal workarounds has "fixed" it (→ #2088 (comment))

@tsujan
Copy link
Member

tsujan commented Aug 27, 2024

@tsujan Please check if I have done it correctly.

I will. Thanks!

@tsujan
Copy link
Member

tsujan commented Aug 27, 2024

Let's not complicate it more than it is. I'm going to merge this, and if it changes anything that it shouldn't touch in the master branch, I'll fix that as soon as possible. Then problems in the new features could be reported and fixed in separate PRs.

Any objection?

@tsujan tsujan merged commit 246b33f into lxqt:master Aug 27, 2024
@tsujan
Copy link
Member

tsujan commented Aug 27, 2024

OK, I did my checks. Everything is fine: the master branch isn't touched where it shouldn't :)

@gfgit, @marcusbritanicus, @stefonarch
Thank you so much for doing the hard jobs of coding and testing!

Now we can proceed on a solid basis, fix the issues in the new features, and probably add more features. I hope more git users will test it under Wayland.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants