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

Add depth limit as argument for how often to autotile #41

Merged
merged 3 commits into from
Oct 31, 2022

Conversation

Syphdias
Copy link
Contributor

@Syphdias Syphdias commented Oct 27, 2022

Currently the script will autotile (vsplit/hsplit) every window focused on with no limit.

At a certain depth this behaviour is not desirably to me and I want to add more windows to the same container. Especially for terminals I prefer to have more space horizontally than vertically to see the entire line if possible.

To achieve this, I add a optional parameter to prevent further autotiling after the specified limit. For example with a 16:9 display and a depth limit of 1, it will split horizontally first and then only split vertically but in the same container.

 ________________
|        |       |
|--------|       |
|        |-------|
|--------|       |
|________|_______|

As a side effect, this enables stacking and tabbed layouts after reaching the limit.
The default behavior is still the same with an infinite limit for how deep to autotile.

Edit: I mainly made this for me. Not sure if you are interested in this.

Currently the script will autotile (vsplit/hsplit) every window focused
on with no limit.

At a certain depth this behaviour is not desirably to me and I want to
add more windows to the same container. Especially for terminals I
prefer to have more space horizontally than vertically to see the entire
line if possible.

To achieve this, I add a optional parameter to prevent further
autotiling after the specified limit. For example with a 16:9 display
and a depth limit of 1, it will split horizontally first and then only
split vertically but in the same container.

```
 ________________
|        |       |
|--------|       |
|        |-------|
|--------|       |
|________|_______|
```

As a side effect, this enables stacking and tabbed layouts after
reaching the limit.
The default behavior is still the same with an infinite limit for how
deep to autotile.
@nwg-piotr
Copy link
Owner

Well, I'm not sure. Need to give it a try and use for some time.

@Syphdias
Copy link
Contributor Author

I unfortunately found a bug. It is caused by i3 not "merging" containers when not used.

What I mean by that is: If you open a window, vsplit, open a window, vsplit, etc. and then close everything but the last window, you end up with nested single vsplit containers.
image
I closed the upper ones and left the last one open. This results in a structure that looks like this:

      workspace: 3 (splitv)
        con:  (splitv)
          con:  (splitv)
            con:  (splitv)
              con:  (splitv)
                con: Tilix: ~ (splith)

output is "type: name (layout)"

This looks like depth 0 with only one window in the workspace but it is recognized as depth 4 or 5.

@nwg-piotr
Copy link
Owner

nwg-piotr commented Oct 29, 2022

I don't care about i3 in the nwg-autotiling clone package. And actually I don't test on i3, as X11 sucks on my triple-headed setup with hybrid graphics. I'd gladly use your solution in nwg-shell, but if it comes to the autotiling repo & package, I need to rely on you.

@Syphdias
Copy link
Contributor Author

Gaming keeps me on X11 for now. ;)

Apparently the containers not being flattened is an i3 bug: i3/i3#3503
I might be able to detect if there is only one node in the parent and not count it. Need to play around with it a bit.

How does sway handle the nested containers? Do they get merged/flattened?

@nwg-piotr
Copy link
Owner

Gaming keeps me on X11 for now. ;)

I only have one game installed (Witcher 2), but it behaves perfectly well on sway.

How does sway handle the nested containers? Do they get merged/flattened?

Wish I knew. ;)

@Syphdias
Copy link
Contributor Author

Syphdias commented Oct 29, 2022

How does sway handle the nested containers? Do they get merged/flattened?

Wish I knew. ;)

You could try this #41 (comment) and then run
https://github.com/Syphdias/i3nodes/blob/main/i3nodes.py (need python3.5 i think and i3ipc(docs says it works with sway as well))

@nwg-piotr
Copy link
Owner

$ print-or-nodes
root: root (splith)
  output: __i3 (output)
    workspace: __i3_scratch (splith)
  output: eDP-1 (output)
    workspace: 7 (splith)
      con: Add depth limit as argument for how often to autotile by Syphdias · Pull Request #41 · nwg-piotr/autotiling - Google Chrome (none)
  output: DP-1 (output)
    workspace: 1 (splith)
  output: HDMI-A-1 (output)
    workspace: 4 (splith)
      con:  (splitv)
        con:  (splith)
          con: ~/bin/print-or-nodes - Mousepad (none)
        con:  (splith)
          con: foot (none)
      con:  (splitv)
        con: bin (none)
        con:  (splith)
          con:  (splitv)
            con: foot (none)
          con:  (splitv)
            con: foot (none)
            con:  (splith)
              con: foot (none)

@Syphdias
Copy link
Contributor Author

I closed the upper ones and left the last one open. This results in a structure that looks like this:

You did not close all the upper terminals but yes, looks like sway does not flatten the hierarchy as well. Otherwise there would be no containers with only one other container in it:

        con:  (splith)
          con:  (splitv)
            con: foot (none)

@nwg-piotr
Copy link
Owner

nwg-piotr commented Oct 29, 2022

Tried on HDMI-A-1.

$ print-or-nodes
root: root (splith)
  output: __i3 (output)
    workspace: __i3_scratch (splith)
  output: eDP-1 (output)
    workspace: 7 (splith)
      con: Add depth limit as argument for how often to autotile by Syphdias · Pull Request #41 · nwg-piotr/autotiling - Google Chrome (none)
  output: DP-1 (output)
    workspace: 1 (splitv)
      con:  (splitv)
        con: nwg-shell-config – ui_components.py (none)
  output: HDMI-A-1 (output)
    workspace: 4 (splith)
      con: foot (none)

@nwg-piotr
Copy link
Owner

nwg-piotr commented Oct 29, 2022

You mean this situation, right (HDMI-A-1)?

$ print-or-nodes
root: root (splith)
  output: __i3 (output)
    workspace: __i3_scratch (splith)
  output: eDP-1 (output)
    workspace: 7 (splith)
      con: Add depth limit as argument for how often to autotile by Syphdias · Pull Request #41 · nwg-piotr/autotiling - Google Chrome (none)
  output: DP-1 (output)
    workspace: 1 (splitv)
      con:  (splitv)
        con: nwg-shell-config – ui_components.py (none)
  output: HDMI-A-1 (output)
    workspace: 4 (splith)
      con:  (splitv)
        con: foot (none)

Too bad! I like the behavior at the depth limit 1.

With lots of splits the hierarchy can get deeper than normal, if
containers get closed. i3 does not flatten this hierarchy (atm).

This means that the depth option does needs to ignore containers that
only contain one child/node. This is done only incrementing the current
depth when checking against the limit if the nodes of the parent
container is bigger than 1.
@Syphdias
Copy link
Contributor Author

So I think I fixed it by ignoring containers with only one child container in them and disregard them as depth. Depth changed a bit:

  • depth 0 is unlimited
  • depth 1 is control the first split
  • depth 2 is control until the seconds split (former depth limit of 1)

@nwg-piotr
Copy link
Owner

nwg-piotr commented Oct 30, 2022

Correct me if I'm wrong.

  • -l 0 does nothing visible;
  • -l 1 spoils autotiling;
  • -l 2 does what we want;
  • -l 3 and more causes behavior expected, but probably unwanted by anyone.

Right? If so, shouldn't we just have a boolean flag, e.g. -m for "Mimic master/stack layout" (or -l for "Limit autotiling"), and constant DEPTH_LIMIT = 2?

@Syphdias
Copy link
Contributor Author

Correct me if I'm wrong.

Nope, I totally agree. Unless your aspect ratio is different maybe. I tried flipping my 16:9 monitor to vertical and there 3 might make sense – not to me though.

We could provide both options and imply -l 2 with -m. We'd need to answer the question to what happens to -l 3 -m though. Which one wins or do they conflicz and exit 1?

Do you use a master/stack layout on sway? If yes, is a sway feature or do you use scripts/tooling? I thought about making a script to swap containers in a certain way but that is out of the scope of this repo.

@nwg-piotr
Copy link
Owner

nwg-piotr commented Oct 30, 2022

Do you use a master/stack layout on sway?

Nope, but I liked it on DWL. In contrary to DWL itself. 🤣

I do believe in the KISS rule. Let's not multiply arguments that do the same job. If you find the current -l argument better, I'm all right with it. My nwg-shell config will just disallow values other than 2, exactly as I did it for testing purposes:

image

Whoever uses the original autotiling script, will decide on their own.

@Syphdias
Copy link
Contributor Author

My worry with -m would be that for certain layouts it might not imitate master/stack but still work as expected. For example you could turn your first split manually to be a splitv and autotiling would then splith in both containers (infinity with -l 2, and differently with default). Another example might be #29 where the first two splits are horizotal due to window geometry I assume.

So I'd say -l and the recommendation to try 2 as a value and that 1 does not really make sense.

@nwg-piotr
Copy link
Owner

OK. Could you also update README.md? I'm working on a small fix to another repo at the moment.

@Syphdias
Copy link
Contributor Author

Syphdias commented Oct 30, 2022

Absolutely, I'll get to it tomorrow.

  • updating help
  • updating README

@nwg-piotr
Copy link
Owner

Thanks!

Hope you'll give sway & nwg-shell a chance one day. The project needs valuable contributors.

The help and README did not go into details what exactly the --limit
option does.

Since it can be used to imitate a master-stack layout, and can enable
tabbed or stacking layouts to some degree, this adds the explanation in
the section talking about this limitation.

It softens the limitations of autotiling while still stressing that
tabbed and stacked layouts may behave unexpectedly and are not supported
by autotiling.
@nwg-piotr
Copy link
Owner

Thank you! I'll merge later tonight.

@Syphdias
Copy link
Contributor Author

Hope you'll give sway & nwg-shell a chance one day. The project needs valuable contributors.

This genuinely makes me interested in giving Wayland a go, but if I remember correctly the problem with Wayland was nvidia drivers (bought while my daily driver was still Windows).

@Syphdias
Copy link
Contributor Author

Thank you! I'll merge later tonight.

No rush, I rephrased the section about not working with tabbed/stacked quite a bit. Please nitpick away.

@nwg-piotr
Copy link
Owner

Dunno what the nvidia status nowadays is. My hardware is all-AMD.

@Syphdias
Copy link
Contributor Author

Dunno what the nvidia status nowadays is. My hardware is all-AMD.

Pretty happy with my Ryzen 7 from a few years back. My next GPU will be red (or blue – one can hope). But currently not in need of a better one, (un)fortunately. But I am looking forward to a new one.

Just a quick Google for the state of things in 2022:

As far as the GBM and wlroots support goes, my experience is that it is not up to the point of being properly used. Better than before, but not good enough.
https://blog.devgenius.io/wayland-and-nvidia-in-2022-2f0407fb34f4

They did not even try to game on it and it failed the test.

@nwg-piotr
Copy link
Owner

nwg-piotr commented Oct 31, 2022

Actually my previous laptop, back in 2015-2020, was an Intel/Nvidia hybrid, and sway worked well on it. The best test is what you check yourself.

@nwg-piotr
Copy link
Owner

nwg-piotr commented Oct 31, 2022

About to merge.

Final reflection: for my use case (1 vertical and 2 horizontal displays), using -l 3 would make sense if applied to the vertical display only.

image

Maybe it's worth of further consideration: a possibility to use different depth level on different displays.

@nwg-piotr nwg-piotr merged commit 79f4696 into nwg-piotr:master Oct 31, 2022
@Syphdias
Copy link
Contributor Author

Syphdias commented Nov 1, 2022

I think the easiest might be to add --outputs like --workspaces and you define as many exec_always as you have displays. Not sure if that is compatible with your gtk interface for configuring it.

@Syphdias Syphdias deleted the limit-depth branch November 1, 2022 19:55
@nwg-piotr
Copy link
Owner

nwg-piotr commented Nov 1, 2022

Not sure if that is compatible with your gtk interface for configuring it.

Haha! If I managed to integrate swaync and gtklock, this is going to be be an easy task. ;)

@nwg-piotr
Copy link
Owner

Well, I've just realized that the version I use in nwg-shell allows a single instance only. :/

@nwg-piotr
Copy link
Owner

nwg-piotr commented Nov 3, 2022

For my specific use case it's possible to store the "output-name": limit dictionary in the nwg-shell-config settings file, so I won't need arguments. It could look something like this: https://github.com/nwg-piotr/nwg-shell-config/blob/limit-per-output/nwg_shell_config/autotiling.py

@nwg-piotr
Copy link
Owner

image

@Syphdias
Copy link
Contributor Author

Syphdias commented Nov 8, 2022

Would there be use for something like: limit of 3 on output x with workspace 1 3 5, but limit of 3 on output y with any workspace.
Hm, I don't know. Just trying to add to the idea. It would be hard to do via CLI if you only want one process. Something like xrandr arguments comes to mind where the order matters (if you are not familiar with it: xrandr --output DP-1 --mode 2560x1440 --output DP-2 --mode 1920x1080). Also, probably hard to do with argparse.

It would split the arguments in two groups: one for filtering (output, workspace, maybe more) and one for attributes (limit, ratio, etc.). This would add complexity which I am not a fan of. You can already do all this by running multiple commands.

@nwg-piotr
Copy link
Owner

It's going to look like this on my side:

image

But I use no arguments, just dictionaries stored in a json file.

@Syphdias
Copy link
Contributor Author

Syphdias commented Nov 8, 2022

I know. But if it was possible through arguments you could use the @ symbol.

Off topic:
I tried to rewrite the nested loop and replace it with "gates" also ran black to unify formatting. What do you think?
https://github.com/Syphdias/autotiling/blob/code-style/autotiling/main.py

@nwg-piotr
Copy link
Owner

Well, I use PyCharm, so it will reformat it back on my next Ctrl+Alt+L.

@Syphdias
Copy link
Contributor Author

Syphdias commented Nov 9, 2022

Black runs on save in my vscode 😅
I can leave the formatting alone.

Mainly check out the logic rewrites

@nwg-piotr
Copy link
Owner

Mainly check out the logic rewrites

Looks good while reading. Needs to be tested carefully, anyway.

If it comes to formatting: I'm strongly against reformatting

def switch_splitting(i3, e, debug, outputs, workspaces, depth_limit):

to

def switch_splitting(
        i3, e, debug, outputs, workspaces, depth_limit, splitwidth, splitheight, splitratio
):

It's 1 line -> 3 lines. The more code I see w/o scrolling - the better.

@Syphdias
Copy link
Contributor Author

Syphdias commented Nov 9, 2022

Ignore the formatting. It is done by black which is quite opinionated. The only reason it got touched is the line length. Again, Ignore the formatting. I just didn't turn it off for this.

I'll wait for the open PR to be done. If you like the sieve/gate structure, I can open I PR for the logic changes and migrate them one by one to make it obvious that the behavior is still the same.

@nwg-piotr
Copy link
Owner

I'll wait for the open PR to be done.

Yes, please. There's still a thing I'd like to be considered there.

@HirschBerge
Copy link

HirschBerge commented Nov 13, 2022

Not sure if this is strictly related, but on ultrawide screens, tiles go three across before tiling correctly
but if I force my resolution to 1920x1080 it will tile correctly

@Syphdias
Copy link
Contributor Author

This behaves as expected and what is also reported by #29. autotiling does not alternate between splits, it decides based on the ratio how to split. On your ultrawide after opening two windows the windows will still be wider than tall. This means that the split will will be horizontal.

There is an open PR #44 that will allow you to set the ratio when it will split.

@HirschBerge
Copy link

This behaves as expected and what is also reported by #29. autotiling does not alternate between splits, it decides based on the ratio how to split. On your ultrawide after opening two windows the windows will still be wider than tall. This means that the split will will be horizontal.

There is an open PR #44 that will allow you to set the ratio when it will split.

Thank you!

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.

3 participants