WLR Workspace manager implementation #805

Anakael · 2020-08-07T21:17:52Z

This is implementation of wlr_workspaces protocol from https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/40

Sway compositor implementation: swaywm/sway#5597
Wayfile compositor implementation: WayfireWM/wayfire#647

Many options comparing sway/workspaces was omitted like disable-*.
Let me know if they was usefull. By the rest it is drop-in replacement of sway/workspaces.

Scrolling will be introduced in next PRs.

P.S.
Not sure if it works correctly with multiple outputs.

man/waybar-wlr-workspaces.5.scd

src/modules/wlr/workspace_manager.cpp

l3nkz · 2020-08-10T02:57:35Z

src/modules/wlr/workspace_manager.cpp

+auto WorkspaceGroup::handle_output_enter(wl_output *output) -> void {
+  spdlog::debug("Output {} assigned to {} group", (void *)output, id_);
+  for (auto &workspace : workspaces_) {
+    workspace->show();
+  }
+  output_ = output;
+}
+
+auto WorkspaceGroup::handle_output_leave() -> void {
+  spdlog::debug("Output {} remove from {} group", (void *)output_, id_);
+  output_ = nullptr;
+  for (auto &workspace : workspaces_) {
+    workspace->hide();
+  }
+}


If you have multiple outputs you will get this message multiple times. You probably want to do this differently here. You can have a look in the taskbar module, how I handled output_{enter,leave} there. Something similar might also make sense for the workspaces (in combination with an option for all_outputs).

Done.
Check only equality with the current output since compositor assign outputs to group by self. So all_output parameter isn't actual for wlr_workspaces.

src/modules/wlr/workspace_manager.cpp

Add removing buttons Adjust handling multiple outputs.

Anakael · 2020-08-22T20:07:54Z

@l3nkz
Can you review this pull request?

l3nkz · 2020-08-23T20:03:23Z

Sorry! I already looked through your other changes when you pushed the commit and they look good to me now. I have nothing to complain. Your work is a good basis to continue with further enhancements. However, I couldn't test it yet. I am currently not running a self-built wayfire on my machine. I will test it as soon as possible, if @Alexays considers it necessary.

Alexays · 2021-11-12T08:07:12Z

Code LGTM, once conflicts have been resolved.
@l3nkz can you test to make sure it's work fine :)
Then wel'll merge the PR!

l3nkz · 2021-11-13T20:43:02Z

I will try to make some tests in the next days. Unfortunately not before Monday I think.

l3nkz · 2021-11-16T08:26:28Z

I tested the module yesterday evening with my setup at home (single monitor) and today with my setup at work (multi-monitor) and it looks good to me so far. Four things that I recognized, but which are not a show-killer right now:

My WM (wayfire with the wlr-workspaces PR merged) announces the protocol at a different name (zext_workspace_manager instead of zwlr_workspace_manager). But this will probably be sorted out once the protocol is officially merged upstream.
Especially in the multi-monitor setup, the workspace bar has a lot of elements. Adding an option to only show the workspace of the current output is probably worth the effort.
I would also appreciate an option to only show the (name of the) currently focused workspace.
I also in favor of using the same nomenclature for focused in the taskbar and workspacebar. In the taskbar it is called active whereas in the workspacebar it is called focused. Maybe we can change this so that both use the same word to describe the currently used element. I don't mind changing the taskbar accordingly, if we consider focused the better term.

In total, I think the module is great! I am looking forward to use this in my production system!

…pace-manager-implementation

Anakael · 2021-11-16T20:36:22Z

@l3nkz
Thank you for feedback!

Adding an option to only show the workspace of the current output is probably worth the effort
I need some time to update patched sway and wlroots to make it works again and test in multi-monitor setup. About option: the protocol introduces "workspaces group" which can be bounded to specific output (or outputs can share the same workspaces), so it should be controlled by compositors (I'm not sure if it implemented in wayfire).
I would also appreciate an option to only show the (name of the) currently focused workspace.
I'll try to implement it in about next two weeks.
In the taskbar it is called active whereas in the workspacebar it is called focused.
Workspace class names was copied from sway/workspaces module in order to be "drop-in replacement". Changing taskbar naming will break existing configurations. So I think dropping "drop-in replacement" concept is better in order to unify names. I'll do that.

@Alexays

Then wel'll merge the PR!
Protocol itself and its implementation for sway still are in MR state. Also, this client implementation here can be outdated from current version of protocol. Should I update this PR now or we can just wait for merging and update it after?

Alexays · 2021-11-18T22:17:39Z

Then wel'll merge the PR!
Protocol itself and its implementation for sway still are in MR state. Also, this client implementation here can be outdated from current version of protocol. Should I update this PR now or we can just wait for merging and update it after?

I think you can update the PR and merge, in that way we'll avoid merge conflicts in the future.

Anakael · 2021-11-23T00:27:28Z

I've updated PR accordingly to latest version of protocol.
What updated:

Protocol name
Class naming

What fixed:

Now in one group per output configuration workspaces are only shown only on output they belongs to (unless all-outputs option is set to true)
Fix critical error in logs
Fix invalid readings in bindings

What added:

New states: urgent and hidden (from protocol)
all-outputs option
active-only option
configuration for actions on buttons
remove action

As a next step I'll update sway PR with implementation, but this module can be considered as completed.

Alexays · 2021-11-23T12:52:23Z

Thanks a lot!

Anakael · 2021-11-23T19:43:27Z

I've also added documentation on wiki: https://github.com/Alexays/Waybar/wiki/Module:-Workspaces

emersion · 2021-11-24T07:33:06Z

You shouldn't merge this PR at this point, because the protocol hasn't been finalized yet. Waybar may break because of a protocol adjustment.

https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/40#note_1167835

l3nkz · 2021-11-24T08:09:42Z

I think all of us, that worked on the module (@Alexays, @Anakael and me) are aware of the fact, that the protocol isn't stable yet and that we have to adapt the implementation ones the protocol gets finalized. Maybe we can mark the module as experimental, might not work with your WM in the wiki and in the manpages?
@emersion Can we somehow assist you with the protocoel merge request? I am personally very interested on getting this protocol finalized as well as go even further and implement an extension for the foreign_toplevel_manager protocol that we discussed here #762.

Anakael · 2021-11-24T08:16:21Z

You shouldn't merge this PR at this point, because the protocol hasn't been finalized yet. Waybar may break because of a protocol adjustment.

https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/40#note_1167835

It won't break the whole Waybar. It will break wlr/workspaces module.
Waybar use own copy of protocol, so breaking changes won't break this module immediately.
There were added 2 requests (and only one can be used here) and 2 states in protocol for last 11 months. And it took me less than 40 minutes to adjust implementation to it.
Now, when it merged some of us can use this module (and protocol) every day and it allow us to understand what we need from it.

Anakael · 2021-11-24T08:18:09Z

Maybe we can mark the module as experimental, might not work with your WM in the wiki and in the manpages?

I've added note about it on page.

emersion · 2021-11-24T08:18:34Z

It will break the whole Waybar, and it will disconnect the whole Wayland connection with a protocol error. The copy of the protocol doesn't prevent that.

You can daily use this WIP work by building the unmerged PR.

Alexays · 2021-11-24T08:31:08Z

@Anakael We should add an experimental flag to enable some modules on Waybar, e.g. this one.

Anakael · 2021-11-24T08:51:59Z

Ok, I will add.

AGCaesar · 2022-01-10T09:24:47Z

Great to see you are still working on this and finished it for the waybar side! Am I right to assume this won't work on sway at the moment because sway is missing the implementation?

Anakael · 2022-01-10T13:30:05Z

Am I right to assume this won't work on sway at the moment because sway is missing the implementation?

Yes, master is missing of this implementation because protocol is not merged.
If you really want to try it, you can use my WIP branch from here: swaywm/sway#5597

tamirzb · 2022-01-26T17:46:58Z

Hey, sorry to bother you with a noob question, but can you maybe explain why is this needed and what is the different between this and waybar-sway-workspaces? And why would I want to use this and not the sway workspaces module?

Anakael · 2022-01-26T17:59:49Z

Hey, sorry to bother you with a noob question, but can you maybe explain why is this needed and what is the different between this and waybar-sway-workspaces? And why would I want to use this and not the sway workspaces module?

This one is sway-independent
When it will be fully merged and implemented, it probably be more supported and featured because
a) more developers can be involved who use other compositors
b) it will be driven by protocol, not by sway ipc

But as for now this module should be used only for testing purposes.

Anakael added 6 commits August 5, 2020 02:17

Base wlr impl (Manager, Group)

2b11b7e

Finish implement wlr

edd4d8e

Add base name representation

7638f5c

Add formatting and states handling

4c25157

Finish base implementation

0ad29a5

Add docs and adjust sorting

42b6c08

Anakael mentioned this pull request Aug 7, 2020

[Taskbar] Add option "all_workspaces" #762

Open

l3nkz reviewed Aug 10, 2020

View reviewed changes

Fix docs typos

22409d2

Add removing buttons Adjust handling multiple outputs.

Merge branch 'master' of https://github.com/Alexays/Waybar into works…

15761ef

…pace-manager-implementation

Anakael added 4 commits November 21, 2021 15:01

Update names

ffeecf6

Update to proto. Fix displaying. Rename classes.

ef4c6a9

Finish

23991b6

Fix sort

98f7a10

Merge branch 'master' into workspace-manager-implementation

0a48413

Alexays merged commit 977d21b into Alexays:master Nov 23, 2021

Anakael mentioned this pull request Nov 27, 2021

build: Add experimental build option #1328

Merged

Consolatis mentioned this pull request Sep 22, 2022

Hyprland Minimize, Maximize, Focus not working LBCrion/sfwbar#51

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WLR Workspace manager implementation #805

WLR Workspace manager implementation #805

Anakael commented Aug 7, 2020

l3nkz Aug 10, 2020

Anakael Aug 10, 2020

Anakael commented Aug 22, 2020

l3nkz commented Aug 23, 2020 •

edited

Loading

Alexays commented Nov 12, 2021

l3nkz commented Nov 13, 2021 •

edited

Loading

l3nkz commented Nov 16, 2021 •

edited

Loading

Anakael commented Nov 16, 2021

Alexays commented Nov 18, 2021

Anakael commented Nov 23, 2021

Alexays commented Nov 23, 2021

Anakael commented Nov 23, 2021 •

edited

Loading

emersion commented Nov 24, 2021

l3nkz commented Nov 24, 2021

Anakael commented Nov 24, 2021

Anakael commented Nov 24, 2021 •

edited

Loading

emersion commented Nov 24, 2021

Alexays commented Nov 24, 2021

Anakael commented Nov 24, 2021

AGCaesar commented Jan 10, 2022

Anakael commented Jan 10, 2022

tamirzb commented Jan 26, 2022

Anakael commented Jan 26, 2022

WLR Workspace manager implementation #805

WLR Workspace manager implementation #805

Conversation

Anakael commented Aug 7, 2020

l3nkz Aug 10, 2020

Choose a reason for hiding this comment

Anakael Aug 10, 2020

Choose a reason for hiding this comment

Anakael commented Aug 22, 2020

l3nkz commented Aug 23, 2020 • edited Loading

Alexays commented Nov 12, 2021

l3nkz commented Nov 13, 2021 • edited Loading

l3nkz commented Nov 16, 2021 • edited Loading

Anakael commented Nov 16, 2021

Alexays commented Nov 18, 2021

Anakael commented Nov 23, 2021

Alexays commented Nov 23, 2021

Anakael commented Nov 23, 2021 • edited Loading

emersion commented Nov 24, 2021

l3nkz commented Nov 24, 2021

Anakael commented Nov 24, 2021

Anakael commented Nov 24, 2021 • edited Loading

emersion commented Nov 24, 2021

Alexays commented Nov 24, 2021

Anakael commented Nov 24, 2021

AGCaesar commented Jan 10, 2022

Anakael commented Jan 10, 2022

tamirzb commented Jan 26, 2022

Anakael commented Jan 26, 2022

l3nkz commented Aug 23, 2020 •

edited

Loading

l3nkz commented Nov 13, 2021 •

edited

Loading

l3nkz commented Nov 16, 2021 •

edited

Loading

Anakael commented Nov 23, 2021 •

edited

Loading

Anakael commented Nov 24, 2021 •

edited

Loading