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

hyprland/workspaces improvements #2316

Merged
merged 19 commits into from
Jul 24, 2023
Merged

hyprland/workspaces improvements #2316

merged 19 commits into from
Jul 24, 2023

Conversation

MightyPlaza
Copy link
Contributor

Some improvements to the hyprland/workspaces modules

adds workspace clicking support (except for the non named special workspace which seems to be an issue on hyprland's side)

adds "all-ouptuts" and "show-special" to config

listens to "focusedmon" and "moveworkspace" events

actually implements "{name}" format which was present on the wiki but wasn't implemented (opposite of "{id}")

fixes issues with previous event implementation on named workspaces

adds more variables to the workspaces

sorts workspaces in the order normal -> named -> special -> named special, instead of id

didn't find any other issues/bugs

will also ping @Anakael @zjeffer in case I accidentally broke something

@zjeffer
Copy link
Contributor

zjeffer commented Jul 15, 2023

Great, thanks for this! I'll do some tests with this tomorrow with multiple monitors.

I just compiled and ran it on a single monitor, and I had a segfault after switching to the fourth workspace, when it creates the new workspace button. I can't seem recreate it again, so I'll do some more testing.

include/modules/hyprland/workspaces.hpp Outdated Show resolved Hide resolved
src/modules/hyprland/workspaces.cpp Outdated Show resolved Hide resolved
man/waybar-hyprland-workspaces.5.scd Show resolved Hide resolved
include/modules/hyprland/workspaces.hpp Show resolved Hide resolved
include/modules/hyprland/workspaces.hpp Show resolved Hide resolved
include/modules/hyprland/workspaces.hpp Show resolved Hide resolved
include/modules/hyprland/workspaces.hpp Outdated Show resolved Hide resolved
include/modules/hyprland/workspaces.hpp Outdated Show resolved Hide resolved
src/modules/hyprland/workspaces.cpp Outdated Show resolved Hide resolved
src/modules/hyprland/workspaces.cpp Outdated Show resolved Hide resolved
src/modules/hyprland/workspaces.cpp Outdated Show resolved Hide resolved
src/modules/hyprland/workspaces.cpp Outdated Show resolved Hide resolved
@Anakael
Copy link
Contributor

Anakael commented Jul 15, 2023

Can you please explain what named, special and named special workspaces are?

@Anakael
Copy link
Contributor

Anakael commented Jul 15, 2023

Can you please explain what named, special and named special workspaces are?

I've found it in wiki.

@MightyPlaza
Copy link
Contributor Author

there seems to be an issue with dispatcher that makes giving no args use the dispatcher as arg, that's why clicking the non named special workspace doesn't work.
currently trying to fix it and other issues on hyprland's side

@zjeffer
Copy link
Contributor

zjeffer commented Jul 16, 2023

This might not be a big problem but I notice the workspace creation is much slower than the wlr/workspaces implementation. Here's a video showing my bar with wlr/workspaces at the top, and your implementation on the bottom:

2023-07-16.10-24-52.mp4

I quickly scroll through the workspaces, and have to wait every once in a while to let the bottom bar update.

I built the latest master and noticed it doesn't have this problem, although it has another problem where it sometimes doesn't destroy the workspaces, resulting in something like this:

2023-07-16_10-32


EDIT: It's probably due to calling hyprctl workspaces to get the workspace json data, so I guess not something that can be improved much. Nevermind.

@MightyPlaza
Copy link
Contributor Author

MightyPlaza commented Jul 16, 2023

tested spamming the mouse wheel and it seems to update at the same speed as wlr. Also seems to be updating at the same speed as the IPC socket.
The duplicated issue, tried to spam the mouse wheel and hyprctl but can't repro.
Probably caused by existing a remove and a create workspaces shared vector.
To fix it, something else needs to be used, maybe a combined list of the workspaces that need updating, what do you guys think?

@MightyPlaza
Copy link
Contributor Author

MightyPlaza commented Jul 16, 2023

fixed one of the issues for clicking the main special workspace on hyprland's side.
should be in the main branch soon
Edit: also had to change previous waybar code for the intended implementation
clicking on all workspaces should be fully working now

src/modules/hyprland/workspaces.cpp Outdated Show resolved Hide resolved
src/modules/hyprland/workspaces.cpp Outdated Show resolved Hide resolved
@MightyPlaza
Copy link
Contributor Author

I suggest this gets merged now, since the only issue is the non removed workspaces which only happen on spamming the mouse wheel.
That should be fixed, in my opinion, by using a vector of combined DTO to create/destroy, which will likely be a major rewrite, so will take some time.
What do you guys think?

@zjeffer
Copy link
Contributor

zjeffer commented Jul 18, 2023

since the only issue is the non removed workspaces which only happen on spamming the mouse wheel.

To clarify: this problem only occured for me on the master branch, not on this PR's branch. On this PR's branch, it's just a little slower compared to wlr/workspaces when creating & destroying hundreds of workspaces per second. Not a big deal.

I agree it can be merged now.

@zjeffer
Copy link
Contributor

zjeffer commented Jul 24, 2023

@Alexays can this be merged? I added persistent workspaces on my own fork and would like to open a separate PR for this.

@Alexays
Copy link
Owner

Alexays commented Jul 24, 2023

Yes, LGTM

@Alexays Alexays merged commit c087d8c into Alexays:master Jul 24, 2023
@MightyPlaza MightyPlaza deleted the workspaces branch July 25, 2023 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants