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

Implement dynamic layouts, tabbing and more #644

Open
wants to merge 30 commits into
base: master
Choose a base branch
from

Conversation

Elv13
Copy link
Member

@Elv13 Elv13 commented Jan 19, 2016

This is the whole dynamic layout system. I stopped doing small PR with this because it became untestable/useless without the other parts. I still plan to split a couple of PRs out of this before calling this non-experimental.

I run this code since January 2016. For me, it is trouble free.

Use cases:

  • Dynamic layouts: Create custom split and tabbed area at runtime in any layout at any spliting point.
  • Session support: Save the current screen and combine startup notification based spawn and the manual layout/tags to save and restore the layout as they are.
  • Custom layout: Make it trivial to create complex layouts that were previously a pain to write.
  • *Merge the widget and client layout: A single concept for both type of layouts will make the API nicer and more coherent
  • i3/Ion/BSP cast-away: Other tiled WMs use this type of structure, so it will be easier for their users to transition to Awesome.
  • Scene Graph: Along with virtual screen support, client frame hierarchy and wibox hierarchy, Awesome would finally have a DOM/SceneGraph with everything in it. This will help extensions hook into user configs.
  • Better documentation: This system heavily uses the doc examples to document everything. The older layout system is almost fully undocumented (aka: read the code)

However, I am aware of a few major issues:

  • Size honoring is not supported, it just create weird resizing bugs. I know how to fix it, I just want feedback first. [DONE]
  • 2 layout suits are broken, corner and treesome (i3 layout). I don't use them, they are easy to fix, it is not a priority for now.
  • There is a few "off by one" bugs leaving 1px gaps between clients, I solve one every now and then, it is not that bad anymore. [DONE (I think)]

Minor issues:

  • One of the fair layout has the wrong layout name and no icon

Features not fully implemented:

  • Random splitters: It works fine, but I don't like the API, it will use the newly merged awful.placement API, but this is not done yet, it still have a somewhat hardcoded API. [DONE]
  • minimap: The "pager" widgets of other WMs. My collision module has a working implementation, but this PR only have a skeleton. It is not a priority.
  • documentation: The suits are very well documented, but nothing else is. [DONE]

API changes to come: (updated 2017)

  • Add a standard way to resize layouts elements instead of the current "check if its a ratio and hardcode the changes"
  • Merge all layout description into a single documentation guide/page DONE
  • Find a way to "monkey test" repeated random changes
  • Implement :reset() for all layouts
  • the "multiple resize mode" for the ratio layouts are still unmerged [DONE]
  • tabbing needs to be able to only show the topmost sublayout (or client)
  • merge the grid layout with resize support DONE
  • add offset to the stack layout
  • the ratio layout should read the children ratio property (once, then update it when changed)
  • make it trivial to use different layouts depending on the client count (necessary for the corner layout)
  • add client placeholder for SNID
  • add minimized placeholder
  • add a generic way to give focus to widgets and use it for the tree layout
  • enable "garbage" layout collection in base_layout and base_stack (can be disabled) [DONE]
  • Implement request::geometry with {slave=true} or {master = true} as hint (and use it in awful.client and awful.ewmh)
  • Rename base_stack base and tabbed with their own module namespace
  • Add a floating base layout
  • Implement z-index support (per tag) in the core

It has tests now, I found only 3 very minor bugs when writing them. It doesn't have full coverage, but it is quite close. Some tests fail due to the off-by-one pixel bugs mentioned above. It also has "some" (read: many) luacheck warnings. I will fix that soon-ish, none are really hitting critical paths, most of them are unused loop variables.

Missing tests:

  • Integration tests
    • SNID
    • Fullscreen
    • Maximization
    • Minimization
    • Sticky
    • Tagging and untagging a client
    • Size hints
    • Each layouts in a for-loop
    • Change tags (see if the tab wiboxes are hidden)
    • Floating clients are removed on floating state change
    • Newly tiled (from floating) are correctly inserted.
  • Merge the example tests using new templates, 2k lines are currently copy-pasted
    • Client count scaling
    • Padding
    • Gap
    • mwfact
    • Columns/masters count
    • Manual layouts

RFC:

Widget resize API

function w:relative_resize(x, y, recursive, args) return did_x, did_y end

Where args has a direction, direction_x, direction_y "official" arguments.

@Elv13 Elv13 force-pushed the upstream_dynamic_p7 branch from eaa8bd2 to 4ec3059 Compare February 6, 2016 08:35
@Elv13
Copy link
Member Author

Elv13 commented Feb 6, 2016

I improved the resize commit. I now use the "live" resize mode by default (same as the current stateless system) as wibox.layout.ratio has been fixed, it no longer explode. I left the other commit untouched not to lose @psychon comments, I only fixed half of them so far (I just restarted working on this now that I don't have to spend an hour rebasing them everyday)

@Elv13
Copy link
Member Author

Elv13 commented Feb 10, 2016

The way you check if a tag is selected (tag.selected(tag.getscreen(t)) == t) is still broken when multiple tags are selected.

Actually, no, I think it is correct. If there is multiple selected tags and it isn't the first, then the layout in suspended. As with the current system, when multiple tags are selected, the first tag layout is used. I did some tests. It seem to work

@Elv13 Elv13 force-pushed the upstream_dynamic_p7 branch from 4ec3059 to 908b4bd Compare February 10, 2016 07:16
@Elv13
Copy link
Member Author

Elv13 commented Feb 10, 2016

Rebased and updated

@Elv13 Elv13 force-pushed the upstream_dynamic_p7 branch 2 times, most recently from 0cfa30d to 48fb69f Compare February 13, 2016 07:02
@Elv13
Copy link
Member Author

Elv13 commented Feb 13, 2016

Updated, ready for review

@psychon I implemented a different solution from the one you proposed. Using a parallel implementation of hierarchy traversing wasn't really working. Many widgets expect a certain sequence, so making it work was almost re-implementing hierarchy. The solution proposed in the commit is a little hacky, but cleaner than the other (IMO).

@psychon
Copy link
Member

psychon commented Feb 13, 2016

Here are some selected (I removed things like "unused loop variable) warnings from Luacheck:

Checking lib/awful/layout/dynamic/base.lua

lib/awful/layout/dynamic/base.lua:13:7: unused variable util
lib/awful/layout/dynamic/base.lua:139:28: unused argument self
lib/awful/layout/dynamic/base.lua:164:36: unused argument self
lib/awful/layout/dynamic/base.lua:184:23: unused argument a
lib/awful/layout/dynamic/base.lua:186:15: unused variable cr
lib/awful/layout/dynamic/base.lua:186:34: accessing undefined variable img
lib/awful/layout/dynamic/base.lua:300:27: accessing undefined variable unpack

Checking lib/awful/layout/dynamic/resize.lua

lib/awful/layout/dynamic/resize.lua:12:7: unused variable screen
lib/awful/layout/dynamic/resize.lua:99:31: unused argument offset
lib/awful/layout/dynamic/resize.lua:139:54: unused argument corner
lib/awful/layout/dynamic/resize.lua:139:62: unused argument x
lib/awful/layout/dynamic/resize.lua:139:65: unused argument y
lib/awful/layout/dynamic/resize.lua:145:26: accessing undefined variable geo
lib/awful/layout/dynamic/resize.lua:238:39: unused argument c

Checking lib/awful/layout/dynamic/wrapper.lua

lib/awful/layout/dynamic/wrapper.lua:13:7: unused variable client
lib/awful/layout/dynamic/wrapper.lua:138:34: unused argument c

function matrix:set_mutating(mutate)
rawset(self, "_automerge", mutate)
end

Copy link
Member

Choose a reason for hiding this comment

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

I don't like all of this change to gears.matrix. I made matrices immutable on purpose so that e.g. I don't have to return a copy from functions in wibox.hierarchy, but can just return the internal matrix. If a matrix is immutable, you don't need to save/restore it, but can just keep the old instance around (in a stack?).

Let's see what you are using this for in the other commits...

@psychon
Copy link
Member

psychon commented Feb 13, 2016

error while running function
stack traceback:
    ./awesomerc.lua:15: in main chunk
error: ./awesomerc.lua:15: attempt to index field 'dynamic' (a nil value)

Apparently I have to require this separately, require("awful") is not enough. That should change.

Edit:

error: ./awesomerc.lua:15: module 'awful.layout.dynamic' not found:

I can't even require it, I have to add .base....

Edit: Which means that the example that I copied from somewhere is wrong

Edit:

error while running function
stack traceback:
    [C]: in function 'require'
    ...s/awesome/build/lib/awful/layout/dynamic/wrapper.lua:12: in main chunk
    [C]: in function 'require'
    ...ects/awesome/build/lib/awful/layout/dynamic/base.lua:19: in main chunk
    [C]: in function 'require'
    ./awesomerc.lua:15: in main chunk
error: ...s/awesome/build/lib/awful/layout/dynamic/wrapper.lua:12: module 'awful.layout.dynamic.base_layout' not found:

I give up. This is just plain broken.

@psychon
Copy link
Member

psychon commented Feb 13, 2016

This is so broken, you may only reopen this once you added unit tests to this which cover at least 50% of the new code. Meh.

@psychon psychon closed this Feb 13, 2016
@psychon
Copy link
Member

psychon commented Feb 15, 2016

Or can you at least tell me how to test this since apparently I didn't do the right thing from looking at the docs?

@psychon
Copy link
Member

psychon commented Feb 16, 2016

I'm sorry I were so harsh and will try to be less angry when things break.

@psychon psychon reopened this Feb 16, 2016
@Elv13
Copy link
Member Author

Elv13 commented Feb 17, 2016

how to test this

Better take the update_dynamic_complete branch of my fork. It has the missing init.lua and modified awesomerc.lua. It also has the huge "work in progress" commit, ignore it

I'm sorry I were so harsh

Thanks, it was indeed a little harsh. Ready for review ~= ready to merge, it just imply I fixed the issues you pointed in the last round. You are also right this badly need tests. I will write them.

It is not as broken as you think it is. Considering only the tile layout is properly implemented (the others are there, but incomplete and somewhat broken). The bugs are:

  • There is a rounding error leaving a 1px gap between some clients
  • Tabbing implementation is a little cheap. It "work" fine, but if you play with it for too long it will eventually do weird things.
  • Resize only works when there is 1 layer of X and Y ratio layouts. This isn't a design issue, I just need to add "ratio strategies" to wibox.layout.ratio as one size wont fit all for such resizing. Note that the current layouts do not support this kind of resizing at all for this very reason, so this is not a regression.
  • There is some minor itches, I use this code without problem, but my workflow may not hit all corner cases (also, my config may hide some).
  • The clients that require size_honoring are a little broken (nothing major, it just need some more work in resize.wrapper_geometry)

Note that the "dynamic" branch of my collision module need to be up-to-date. Resizing can be done using the usual keybindings, but the "better" version is mod4+alt+arrows to increase size and mod4+alt+shift+arrows to reduce it. If you get any errors, forward them here. also note that the mouse resize implemented by this PR is more advanced than the current one (and the code is also simpler, as long as the reader did some matrix math before). It support "side" resize, like left/right/up/down instead of corner only like the old implementation. It also support proper keyboard resize, the old implementation did not. It is also layout agnostic, unlike the previous code.

@Elv13 Elv13 force-pushed the upstream_dynamic_p7 branch from 48fb69f to 49bb36c Compare April 3, 2016 08:35
@Elv13 Elv13 changed the title Add the wibox.layout compatibility code Implement dynamic layouts, tabbing and more Apr 3, 2016
@Elv13 Elv13 force-pushed the upstream_dynamic_p7 branch 2 times, most recently from a73db8c to 58eb3ac Compare April 12, 2016 04:30
@Elv13
Copy link
Member Author

Elv13 commented Apr 12, 2016

Rebased on top of the new APIs

It hasn't changed much in 2 months. The few issues it had back then are still there. I am still waiting for a review before spending more time on this.

The requested coverage is implemented since a month or so.

widgetlayout5

@Elv13 Elv13 force-pushed the upstream_dynamic_p7 branch 5 times, most recently from 2425727 to c19c3f4 Compare April 19, 2016 08:24
@Elv13
Copy link
Member Author

Elv13 commented Apr 19, 2016

Updated. I removed the fake context, then it still didn't always work (the same corner cases as last time, no surprises here). It also added 5 new "standard" method to the widget API for almost no reason, which is ugly and confusing.

After that I reverted that commit and created a new "domain specific" context, but with @psychon handle_hierarchy code as backend. So the current solution is an hybrid of mine and psychon. It can still use unmodified normal widgets[1] while not having to shim the whole clip part of the context (that was called, but had its value unused).

[1] including those who alter the CR matrix in the before__/after__, but not the clip (previously, the code would not cause error, but would do nothing). So not "all" widgets/layouts/containers are compatible, a little regression compared to my previous cairo context shim. I also force the translation back, I will have to re-implement more of wibox.hierarchy.draw to avoid doing this.

@psychon
Copy link
Member

psychon commented Apr 19, 2016

If you really want this to use a cairo context, use an unbounded cairo recording surface as the target of the cairo context. Just, please, don't try to create a fake cairo context.
(Hm, widgets altering the transformation in their before/after draw methods... That's bad, I think. That can easily break redraws badly.)

@Sinvers
Copy link

Sinvers commented Jun 10, 2018

That's a really good work that you have done here. It was the only thing that was really missing from Awesomewm.
But I faced a problem, is this a known bug that the focused client is never taken in account while using the awful.layout.dynamic.suit.treesome ? Or am I doing domething wrong ? I built the project from this pull request. Everything looks fine, except that I just can't always get to split the right client. The only way I manage to make it works is maximizing the client, then open an other client and unmaximize the first client. And then, the split happened at the right place.
Anyway, great job, I'm looking forward to the project's progress.

@Elv13
Copy link
Member Author

Elv13 commented Jul 5, 2018

@Sinvers sorry for the delay, it has been "I will find time tomorrow" for a month now.

Or am I doing domething wrong

No, you are doing it right. The commit message has [BROKEN] in its name for that reason ;). I am very surprised this suit works at all without error. It's the first one I wrote, it never worked properly and I never ever updated it since I wrote it.

The issue is that by the time the client is added to the layout, it already has focus and as it can't split itself, it falls back to the largest remaining client. So the fallback code is the only one that can be reached unless you add some awful.rules to disable autofocus. At that time there was no clean way to fox this. That's why I submitted that pull request 2 years ago #1031

However I never patched the treesome layout to use it. Until now that is! I wont update this pull request with the fix. I will probably never update it again as I broke the API in the second generation of this module. You can get it in my fork doc_tests_and_notif branch under the lib/dynamite directory. It is a temporary placeholder name until I find a better one for the module. Otherwise it is a much cleaner version of the PR. It is 99% complete and has addressed much of the TODO in the list above and much of @psychon complaints (beside the fake cairo context, that's unfixable with the current wibox API). Only the serialization part isn't final. See doc at https://elv13.github.io/libraries/dynamite.html . As always with that branch, it is my fork "main branch" and has a megaton of incomplete stuff and I break the API all the time.

I will close this PR and open a new once I found a better name and added regression tests for minimization, maximization and master_fill_policy. @blueyed I can no longer reproduce the bug you had last year, mind trying again with the version from my doc_tests_and_notif branch?

@Sinvers
Copy link

Sinvers commented Jul 19, 2018

@Elv13 Thanks for the precise answer, I'll check this out !

@GrayJack
Copy link

GrayJack commented Nov 7, 2018

There is any ETA for this PR to be merged?

@actionless
Copy link
Member

@GrayJack depends on user feedback

is it working fine for you?

@GrayJack
Copy link

GrayJack commented Nov 7, 2018

I don't use it, since I don't know witch branch to clone to compile it

What is the branch, cause I really want to test it!!

Or a easy way to get a .patch file so can git the master and patch it before compiling

@Elv13
Copy link
Member Author

Elv13 commented Nov 7, 2018

Elv13 upstream_dynamic_p7 is the branch for this PR, there is a more advanced one, but it has a couple minor regressions regarding floating clients (but has more testable code, which isn't a factor for people who want to try it)

The doc is here https://elv13.github.io/libraries/dynamite.html

If you want to help, resolve all code comments here:
#2244 I have zero time and I am not working on getting this merged. The few hours I have for Awesome go toward getting the other big project merged (notification rewrite).

@GrayJack
Copy link

GrayJack commented Nov 7, 2018

And where I can find a example of rc.lua

@Elv13
Copy link
Member Author

Elv13 commented Nov 7, 2018

The rc.lua is the same, you just need to include awful.layout.dynamic for this branch or dynamite for the newer one. It adds zero features by default. If you want to use tabs or dynamic split, either use the treesome layout or add some shortcuts to emulate i3. My config have the elaborate set of widgets with the arrows and tag stack preview, but that's not part of this pull request. This pull request replaces the way layout works to allow these features to exist. My rc.lua is very large and isn't intended to be re-used by others. You will spend more time learning how to use it than add the keybindings yourself.

@GrayJack
Copy link

GrayJack commented Nov 7, 2018

What is the regression for floating? if is something minor, I don't mind using the more updated branch 😄

@Elv13
Copy link
Member Author

Elv13 commented Nov 7, 2018

Try the doc_tests_and_notif branch of my fork, which has all the commits from all my ongoing projects. If you can't find bugs, then was there a regression in the first place? A word of warning, this branch has a lot of stuff in it, so random other things may have regressed. The notification maybe? It should not be that bad and you can email me bugs, but this is a very experimental branch.

@GrayJack
Copy link

GrayJack commented Nov 7, 2018

I want to do something like i3, dynamic split + both types of stack.

One thing I don't understand about your documentation I where I define my layouts, is it inside the awful.layout.layouts or is somewhere else?

@Elv13
Copy link
Member Author

Elv13 commented Nov 7, 2018

I want to do something like i3, dynamic split + both types of stack.

It has all the feature to do that, for now it's a bit low level and non-trivial to use them, but it's definitely there.

One thing I don't understand about your documentation I where I define my layouts, is it inside the awful.layout.layouts or is somewhere else?

By default it uses the same layouts as Awesome. If you want to add new layouts, you can add them there is directly set them on the tag (see my tyrannical module).

In the near term, this pull request was recently merged #2360 and will make this easier to manage going forward.

@GrayJack
Copy link

GrayJack commented Nov 7, 2018

Ok, i put dynamite.suit.treesome, on the list, but I get an error in the line that I put it saying
attempt to index a nil value (field suit)
Did I put the name wrong?

@Elv13
Copy link
Member Author

Elv13 commented Nov 7, 2018

You are probably missing a , at the end of the previous line. For further issue, email me at elv1313 at gmail. That will prevent flooding this pull request with support questions.

If the error isn't a missing , at the previous line, please send the complete error message (and use Xephyr to debug).

@skrzyp
Copy link

skrzyp commented Apr 12, 2019

What's the current situation of this?

@Elv13
Copy link
Member Author

Elv13 commented Apr 16, 2019

What's the current situation of this?

Unchanged. It works fine and is usable in production. There's another, newer, version that has some improvements regarding unit testing, but has some regressions. This project is too impractical to merge during Awesome 4.x lifetime so is maintained out-of-tree. Some parts get merged as time pass, but I have very limited time to work on these things, so other AwesomeWM related project get more attention than this. Currently finishing the notification rewrite and getting v4.4 into shape is the priority.

@intrntbrn
Copy link

intrntbrn commented Apr 24, 2019

Idea: There should be a possibility of enforcing tabs via rules, so that e.g. every new instance of wm_class gets added as a tab by default.

//Edit: I've been using the doc_tests_and_notif brach for a few days now and treesome is working fine. I've only noticed minor resize issues. However tabs are horribly broken. I havn't tested the other features yet.

@m-kru
Copy link

m-kru commented May 7, 2020

@Elv13 any status update? When is it planed to be merged into master?

@nova-nowiz
Copy link

Time for the yearly status update questoom x)
Is there any plans to get this merged or should someone interested in having tabbed and stacked layouts for screen real estate just stick to i3 for now?
In my research, it seems like i3 is the only tiling window manager to have tabbed and stacked layouts (If I'm wrong, that would be awesome)

@Elv13
Copy link
Member Author

Elv13 commented Jun 20, 2021

For now there are:

Getting this PR merged cannot happen during the 4.x lifecycle and has to wait for 5.0. The code here still works and still does what it claims to do. As of now 4.4 is getting close. It was "supposed" to be the last 4.x, but I don't have enough time/energy to finish all the features I originally planned, so it looks like there is still room for a 4.5 before getting to the point where bumping the default API level to 5 becomes worthwhile.

As for "why can't you just merge it and leave it disabled by default", this PR is missing an important chunk. Right now, AwesomeWM has some "layers" for clients like ontop/below/above and a :raise() and :lower() method to move objects in those stacks. That's not enough to make tabbed layouts non-buggy. Switching tags currently messes with the tab order and it's annoying. A large refactor needs to happen in the C code and it will change some behavior, thus is hard to integrate in a minor release.

@nova-nowiz
Copy link

Woaw thank you for the great explanation and the links!
I'll check back when 5.0 is released then 😄
I'm super hyped that another window manager will support these modes natively in the future, awesome!

@LeducH

This comment was marked as off-topic.

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.