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

Streamline dwindle groups #593

Closed
wants to merge 3 commits into from
Closed

Streamline dwindle groups #593

wants to merge 3 commits into from

Conversation

phi-mah
Copy link

@phi-mah phi-mah commented Aug 28, 2022

Definition

I had a look into the implementation of dwindle groups. For a clean design and rigorous implementation, I would propose the following definition:

For each tiling window, we have a terminal node (i.e. no further children) , to which the window is assigned to. A group is a non-terminal node in the dwindle tree, that exposes the window of one of its descendant terminal nodes (selectable), solely, on the entire viewport of the group . Normal non-terminal nodes, in short nodes, don't do that and just distribute the remaining rendering space to its children.

Implementation guidelines

Implementation wise this can be achieved when sticking to the following principles:

  • Always keep the binary tree intact. Even with a group being present, they behave transparent with regard to the tree: Nodes/child nodes are handled (moved, resized, inserted, etc.) completely normal (like in case of a plain node)
  • A grouping action is just a matter of visualization and does not affect the tree structure: If a node turns into a group, it short-cuts the window rendering at this tree position and presents a window of it descendant child nodes.

Development state

In this PR, I implemented the outlined behavior to provide a proof of concept. It was easiest to discard the most recent changes w.r.t groups in the main branch (in particular c319a2a) and give it a fresh start.

Apart from a cleaner implementation, we gain the following:

  • Grouped nodes remember their original split-ratio when being ungrouped again
  • Groups of groups are possible now. Following the definition from above, an outer group still presents all reachable child windows. But if the outer group gets dissolved, the inner group becomes visible again.

Demo:

The following video:

hyprland.mp4

demonstrates the new features. It is driven by the playbook below:

#!/bin/zsh

alias D="hyprctl dispatch"
alias S="sleep"

for i in `seq 3`; do 
	S 0.5; D exec ~/.config/hypr/kitty-with-banner;
done

S 1; D resizeactive 120 120
S 2; D togglegroup 1
S 1; D focuswindow "title:.*001$"
S 1; D togglegroup 1
S 2; D togglegroup 0
S 1; D movewindow l
S 1; D focuswindow "title:.*003$"
S 1; D resizeactive -120 0
S 2; D togglegroup 0
S 1; D resizeactive 0 -120
S 1; D movewindow l

Unresolved issues:

Atm, a few things still need to be done:

  • Fullscreen requests are not handled probably. However, with the concept from above, maximized fullscreen windows can be represented with the top node turned into a group and the view being rotated to the desired window.
  • I did not care about decorations (Don't now how they work). Once this gets fixed, I would suggest to
    • use dedicated decoration colors for fullscreen windows, making them easier to identify

I submit this PR to trigger a discussion and get some feedback although the implementation is not finished and thoroughly tested.
I also do not now how much time I can invest into this within the next week, so help with the remaining issues would be appreciated. But imho, it would be worth it!

phi-mah and others added 3 commits August 25, 2022 01:52
Stick with the original idea of groups being non-terminal nodes that expose windows of child nodes. Keep the binary tree intact all the time.
@vaxerski
Copy link
Member

After some testing - this PR introduces a lot of issues the new rewrite was supposed to get rid of

  • closing the "master" node (the one from which the group was created) will remove the group
  • dragging any window from the group out does not work, dragging the "master" will break the group and cancel the drag
  • of course the issues you mentioned above
  • I dislike groups of groups - they are not intuitive, and like, why? Example: I have 3 windows, two on the right and one on the left. The left one is a group. While focused on the group, i use togglegroup. Should it make a new group, with the 3 windows, or dissolve the focused group? In your PR, it will dissolve, but the first time it's done that it was a bit of a "oh, thought it would make a group, no?"
  • I do agree there can be a problem where this would block the creation of a new group, but that can be solved differently, either by "eating" the subgroup or, after I do Add support for dimming windows #541, a tint-like indicator.

Is all of this really worth it just to enable "remembering the layout"? I think we're missing the point of what most people want when they see groups. They want a group, not a façade.

That was my original thought regarding my group rewrite (c319a2a)

Feel free to prove me wrong, but I feel this is an overkill, and will lead to more issues than it solves. The only requests I've heard from people is #598, being groups of single windows. Never anything else.

Of course this PR also introduces some temporary issues, like the code styling not following the Hyprland code styling, but again, that's just a temporary easy-to-fix thing.

Feel free to elaborate on this PR.

@spikespaz
Copy link
Contributor

Is all of this really worth it just to enable "remembering the layout"?

Worth? IDK. It is a wishlist item though.

@phi-mah
Copy link
Author

phi-mah commented Sep 2, 2022

Ok, thank's again for looking into this and your honest feedback. My reply gonna be a rather longish read (again) and I will discuss things on a more general level. Sorry for it; We also use a different place for further discussions, since this not focused on the initial PR any more:

Let me first explain an example situation, in which I would like to use some kind of grouping feature:

I am scientist and when doing analysis a typical workspace looks like this:

  • One large editor window
  • At least one console window in which I execute the the scripts I am working on
  • (Multiple) result windows product by the scripts

In such a context I would like to group windows mostly for two reasons:

  • Give them all the same size, so that plots and alike can be compared. For this, you can also use floats
  • Temporarily magnify windows: As the grouped windows are displayed sequentially they can take up the entire group area on the screen. If I undo this, I do want my workspace to return to its former layout.

So, yes, I would say "remembering the layout" is important here. In particular a dynamic tiling algorithm should honor the (limited) adjustments that have been made by the user and try to preserve them. For the reset, it should deliver predictable results that follow a logic to which the user can adapt to -- and in my particular use case this is where the current implementation fails (cf the videos):

I made to videos, in which I compare the behavior of the main branch (1st clip) with this branch (2nd clip):

main-220831-1312-03.mp4
branch-screencast-220831-1311-07.mp4

In case of this branch (not possible in the main branch atm) I also regroup a node being already a group. The group eating aspect (as you called it) can become quite handy to enlarge an already existent group. The fact that groups can be nested is not important and could be abandoned. With the decorations being fixed its also clear on which node the togglegroup operation has to be performed: The node not (yet) being a group.


With this in mind I tried to see how far I could get with the facade implementation. It was mostly a poc and atm I do not expect from it to deliver perfect results. But from my point of view this implementation looks cleaner (less if (grouped) {...} else segments scattered over the code, it looks like we could get rid of the dummy nodes being labeled with "massive hack"). So I would expect that the issues you presented could finally be resolved. And let me ask the other way around:
What are the things that would really break with this implementation approach?

However, I do admit that we are not there yet and I cannot guarantee that I can resolve all the issues in the near future (probably not, due to time constraints). I could still try to keep slowly/gradually working on this (so exactly the opposite to the current hyprland dev spirit) but only if has the realistic possibility of being finally merged in.

So, before discussing the details we should reach conclusion on:

I think we're missing the point of what most people want when they see groups. They want a group, not a façade.

Btw, do WM like bspwm or awesome offer a grouping feature for their dwindle layout. How do they handle them?

I would lean toward the facade, but this is the implementation perspective. From the user perspective, I want to have groups that do not deteriorate a window arrangement once they get dissolved again. How this is done behind the scenes is merely an academic discussion, that I have to admit.

@spikespaz
Copy link
Contributor

spikespaz commented Sep 24, 2022

This is how Hyprland should have been designed in the first place. The current implementation displays an abhorable lack of forethought; when writing software it is best to work around the premise of a prototype. You build something twice, expecting to throw it away the first time. This pull request is fundamentally correct in the principle it regards--it deserves attention.

@vaxerski
Copy link
Member

vaxerski commented Sep 24, 2022

That's mostly how it used to work before, and was so difficult to manage it was dropped. It served no purpose whatsoever. Haven't seen a single person outside you two that complained about the way groups work.

The only request atm regarding groups is #516 (well and #520 but that is cosmetic)

@phi-mah
Copy link
Author

phi-mah commented Sep 24, 2022

Believe me, I am still bothered by this almost every day...

I want to have groups that do not deteriorate a window arrangement once they get dissolved again.

Key aspect of this PR was to promote the facade aspect and to demonstrate that you can reach a concise design that delivers all the functionality that we expect + even more.

Also, none of the issues brought up by @vaxerski seem to fundamentally contradict with my approach, though it still needs some development work.

Once it is is finished, I beg to differ here:

and was so difficult to manage

I am quite busy at the moment, but I have not forgotten about this PR and whenever I find the time, I would like to work on it/ bring it forward again

@Dickby
Copy link
Contributor

Dickby commented Sep 24, 2022

This is how Hyprland should have been designed in the first place.

I think this is quite debatable!
Sometimes it's just what you discover on the way, that turns out to be the right solution.
I think if you try to guess beforehand what the best thing is going to be, you might wasting time thinking, when in reality you can't even know at that time.

You build something twice, expecting to throw it away the first time.

It's a luxury be allowed to rewrite and change stuff on the way, when you discover how things actually should work.
A luxury that a big team, possible with a deadline, doesn't always have.
But i think if you can work that way, it often leads to better more original software.

@spikespaz
Copy link
Contributor

A luxury that a big team, possible with a deadline, doesn't always have.
But i think if you can work that way, it often leads to better more original software.

And Hyprland is written by someone for fun so iterative improvement ought not to be an issue.

@Dickby
Copy link
Contributor

Dickby commented Sep 24, 2022

@spikespaz : Your right!
I just wanted to point out, that it's quite easy to state things like:

The current implementation displays an abhorable lack of forethought

at a point where you have already gained information by trying things.

@spikespaz
Copy link
Contributor

@Dickby Apologies. Perhaps this was a bit harsh. I say this because I made my own prototype in OpenGL and came up with the exact solution provided by this PR.

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