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

Get current layout from workspace instead of g_pLayoutManager #6544

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

UjinT34
Copy link
Contributor

@UjinT34 UjinT34 commented Jun 16, 2024

Describe your PR, what does it fix/add?

  • added CWorkspace::getCurrentLayout
  • replaced g_pLayoutManager->getCurrentLayout with CWorkspace::getCurrentLayout
  • renamed CLayoutManager::getCurrentLayout to CLayoutManager::getCurrentGlobalLayout
  • removed calls to getCurrentLayout from layouts

Initial work on per workspace layouts. For now CWorkspace::getCurrentLayout just returns g_pLayoutManager->getCurrentGlobalLayout(). So any existing logic shouldn't be affected by these changes.
Renamed CLayoutManager::getCurrentLayout to break the api and get conflicts with other PRs which may add new calls to g_pLayoutManager->getCurrentLayout.
Also removed calls to getCurrentLayout from layouts since it should point to this.

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

This code relies on CWindow::m_pWorkspace and CMonitor::activeWorkspace being set before the calls to getCurrentLayout.
In some cases getCurrentLayout might be called on an incorrect workspace. This should not be an issue for now since all workspaces will return the same layout.
When a window is moved to another workspace there are no checks that both workspaces have the same layout and only one of them gets updated. Again not an issue for now.

Is it ready for merging, or does it need work?

Ready for merging

@vaxerski
Copy link
Member

Makes no sense to split that though, as per-workspace layouts by the pure design of layouts cannot work properly. Am I missing something?

@UjinT34
Copy link
Contributor Author

UjinT34 commented Jun 16, 2024

Well, have to start with something. Too many changes to do it in one go. And it won't be easy to review it in one go...
I can continue to work on this as a single PR. I am just not sure if what I am doing is right. Is it the right way forward?

@vaxerski
Copy link
Member

my numba 1 question would be: why? It adds complexity for no reason, tbh. Currently Hyprland is designed so that each "layout" is like a standalone window manager - so it's easy to swap them out and also write new ones using plugins.

@UjinT34
Copy link
Contributor Author

UjinT34 commented Jun 16, 2024

This will implement #622
A plugin for this will have to duplicate the existing layouts and any new ones. I don't understand why a layout should care about what happens offscreen on other workspaces. Any layout work on active workspace doesn't depend on the state of other workspaces and doesn't affect the state of other workspaces. Am I missing some features which require simultaneous access to nodes across several workspaces? From a glance there are bunch of helper functions that limit the work to the given workspace id.
If you don't have any plans to change current design and support per-monitor or per-workspace layouts within hyprland itself than I won't pursue this any further.

@zakk4223
Copy link
Contributor

I've had a plugin that does this for a while now. It does work and it's nice for people with multiple monitors that may have different sizes and/or orientations. Or just people who want different layouts in workspaces based on workflows/content etc.

It works by adding a layout that just proxies layout calls to the "real" layout for a given workspace, and adding/removing windows from the 'real' layouts so a given window is never tracked by more than one layout.

I think if the only layouts possible in hyprland were the two default ones, having native per-workspace layouts might seem a bit overkill. With plugins able to add as many layouts as users want, it might be nice to allow that extra flexibility.

@vaxerski
Copy link
Member

swapwindow between two workspaces will break this, for example.

@zakk4223
Copy link
Contributor

If my hacky proxy layout can deal with swapwindow across workspace/layout a native implementation of it can too.

Copy link

@vignesh1507 vignesh1507 left a comment

Choose a reason for hiding this comment

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

it it necessary to update the code, i think the original code was perfect.

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

Successfully merging this pull request may close these issues.

4 participants