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 initial support for horizontal / vertical tiling #16

Closed
wants to merge 10 commits into from

Conversation

codic12
Copy link
Contributor

@codic12 codic12 commented Jan 4, 2022

Description

This PR adds basic support for horizontal tiling to Budgie. It looks like this:
image
We add a new option to the dconf schema com.solus-project.budgie.wm.gschema.xml: toggle-layout, which is the keybinding to toggle layout. Currently the default is <Super>T.

In the future support will be added for vertical tiling in a follow-up PR if this one gets accepted.

The only downside here is that size hints are respected, which is great for a floating WM but not so great for a tiling one (obviously not what Mutter developers had in mind). If anyone knows any way to override size hints beyond using manual X11 calls or simply removing the size hints, please let me know. For now I've just used move_resize_frame, which respects hints, but it's not that horrible.

I'm new to Mutter, Budgie, and Vala, so please tell me if there's any improvements I can make! Code reviews and some testing would be much appreciated.

Also as for multi-monitor support, in the future this needs to be more discussed. Right now it uses the primary monitor reported by gdk, whatever that is, and tiles all windows on just that monitor.

Submitter Checklist

  • Squashed commits with git rebase -i (if needed)
  • Built budgie-desktop and verified that the patch worked (if needed)

@Jacob-Vlijm
Copy link
Contributor

Jacob-Vlijm commented Jan 4, 2022

For my understanding, what should the shortcut exactly do, tile three windows like in the image? What is exactly the issue? Other question: are you aware of Window Shuffler on Ubuntu Budgie? Seems the work is all done already and features like this are available from either a shortcut a hotcorner or whatever you prefer.

@codic12
Copy link
Contributor Author

codic12 commented Jan 4, 2022

For my understanding, what should the shortcut exactly do, tile three windows like in the image?

No, it can tile any arbitrary number of windows that are on the current workspace.
Yes I'm aware of Window Shuffler, but this provides something different. Instead of manual tiling, this automatically tiles windows whenever you open them. For example:

Press Super+T
Launch window: takes up whole screen
Launch another: each takes 1/2 height, full width
Launch another: each takes 1/3 height, full width
And so on. This is similar behavior to what you'll find in standalone tiling window managers like dwm or i3. Window shuffler just lets you select a layout by dragging the cursor every time you press the hotkey/hotcorner, then tiles those windows. You have to do this every time you open a new window if you want similar behavior to this PR.

This also solves solus-project/budgie-desktop#2183

@Jacob-Vlijm
Copy link
Contributor

Jacob-Vlijm commented Jan 4, 2022

Ah, it does a lot more today, but deliberately not like i3wm and such. So the issue is you run into minsize issues? I also see a slightly off-position code- window. You need any help with that?

@codic12
Copy link
Contributor Author

codic12 commented Jan 4, 2022

Well, it's not that the window is off-position. It's that the terminal window on top is not the size i've given it, because Mutter's Meta.Window#move_resize_frame function does respect size hints. The gap is not big enough to fit another row of characters, so the terminal window keeps it smaller. I need help with that, yeah, but I'm not sure how to do it or if it is even possible with Mutter's API in the first place.

@Jacob-Vlijm
Copy link
Contributor

Jacob-Vlijm commented Jan 4, 2022

Personally, I use Wnck to position windows, which works pretty much exactly. Occasionally, a window only accepts resize steps in e.g. 2px or something.
What you do need to take into account though is that if a window has the property NET_FRAME_EXTENTS, we need to also correct the window's position (y-wise) with its y-extent, otherwise it will slightly land off (y-) position.

@codic12
Copy link
Contributor Author

codic12 commented Jan 5, 2022

Probably the _NET_FRAME_EXTENTS thing can be done in the next PR, but I'm still not sure what you mean on that. There isn't any y-extent in _NET_FRAME_EXTENTS, just top/bottom/left/right.

@Jacob-Vlijm
Copy link
Contributor

Ah, sorry, should have been clearer, it's the top extent.
Other thing is, no matter if you succeed or not and succeed to polish behavior (and if it would survive extensive testing), my guess is that an essential behavior change like this will never ever get accepted. Automatic tiling + rearranging is pop-os -ish, which is totally different from what users are familiar with, and probably wouldn't want.
I'd advise you to create your own repo, work out some kind of an add-on module/daemon, playing with the window manager "from outside". Even then, it would take a long time to create a mature thing though. If you do, I'd be happy to exchange thoughts and ideas. You could even work with existing modules of Shuffler. They are made for a variety of applications.

@codic12
Copy link
Contributor Author

codic12 commented Jan 5, 2022

I see.

I don't really have dedication to maintain a fork or separate plugin because I don't use Budgie too often, so I guess I'll just close this PR.

@codic12 codic12 closed this Jan 5, 2022
@JoshStrobl
Copy link
Member

I would like to emphasize that @Jacob-Vlijm does not speak on behalf of the project and shouldn't have discouraged you from continuing this work.

@codic12
Copy link
Contributor Author

codic12 commented Jan 5, 2022

Fair, if there is a possibility that it could still be accepted in the future then I'm happy to keep working on it.

@codic12 codic12 reopened this Jan 5, 2022
@JoshStrobl
Copy link
Member

In terms of accepting the work, I'd really like to see an option for automatic Tile Vertically to compliment the Tile Horizontally. I think this could be easily accomplished by having a dialog pop up in the center on pressing Super+T and present you with the shapes for tiling, and could be expanded in the future to also grid layout and more.

Is this work you are willing to get involved in?

@codic12
Copy link
Contributor Author

codic12 commented Jan 5, 2022

Sure, that's a good idea. My original plan was to just have Super+T cycle through the available layouts but that seems like a better option, although it will need me to learn a bit about Gtk.

@JoshStrobl
Copy link
Member

@codic12 Well I would like to stress that there is no rush for landing this. You're not on some sort of timeline and I'm always happy to keep this PR open and you contribute more commits as the work evolves. Having Super+T cycle through is a good idea, however I'm just trying to make it more interactive and straight-forward for folks. Having Super+T instantly tile would probably freak the user out at first, whereas it prompting them with an opportunity to tile or simply cancel the operation (cancel button or pressing Esc) would probably feel more intuitive.

Just my two cents of course.

@codic12
Copy link
Contributor Author

codic12 commented Jan 5, 2022

Right now I'm working on vertical tiling, but after that I'm thinking of how to implement the popup dialog. The tile_windows function is in src/wm/wm.vala, however we would probably have to put the popup in another file. Since the wm code seems to be independent of other code, how would that access tile_windows()?

Or can we just create a GTK window in wm.vala inside the on_layout_toggle function?

@JoshStrobl
Copy link
Member

how would that access tile_windows()?

We don't need it to access tile_windows, we can have a popup in a separate file, construct / destroy when needed (or just have it always exist but hide it when we don't need it to reduce the delay) and have the button clicks call signals we set up in the window, which are handled (with .connect) in the WM code. Those signals would dictate what function in the WM we call.

@codic12
Copy link
Contributor Author

codic12 commented Jan 5, 2022

Thanks. I'm still not sure how to go around that but I'll look at the other dialogs when I get there.

@Jacob-Vlijm
Copy link
Contributor

Jacob-Vlijm commented Jan 5, 2022

@JoshStrobl For the record, I never suggested I spoke on behalf of the project, nor do I believe @codic12 did think. At the same time, I can't believe you would implement a popos-ish procedure into Budgie, but apparently I was wrong. Just wanted to be helpful here, since there was no one to react on it.
I also can't remember discouraging him, on the contrary, just wanted to make sure there is time to mature things. I have the impression a PR is not the place for that.

@codic12
Copy link
Contributor Author

codic12 commented Jan 5, 2022

image

It doesn't yet play well with the panel, but otherwise I've pretty much finished vertical tiling.
And yes, @Jacob-Vlijm I was aware, but I thought your point was valid that it would likely not be accepted.

have the impression a PR is not the place for that.

why not, though? it is just a fork which can be merged at any time with the click of a button.

@Jacob-Vlijm
Copy link
Contributor

I'll leave the speech to @JoshStrobl , but certainly a PR can be polished. Not the place for development though, but Josh might have another opinion.

@JoshStrobl
Copy link
Member

JoshStrobl commented Jan 5, 2022

It's fine here, in Budgie and in a separate branch + PR process. Stop it please.

@codic12
Copy link
Contributor Author

codic12 commented Jan 5, 2022

i've finished vertical tiling. Now Super+T will cycle through the list: [Floating, Horizontal Tiling, Vertical Tiling].
i want to start work on the popup but I honestly have no idea where to begin, do you have some guidance?

also here is a video of how it looks. the overlap of the terminal and chromium in the end is because chromium has a minimum size and it doesn't allow going lower than it, and that minimum size is reached in this small Xephyr screen.

simplescreenrecorder-2022-01-05_11.09.00.mp4

@codic12 codic12 changed the title Add horizontal tiling Add initial support for horizontal / vertical tiling Jan 5, 2022
@fossfreedom
Copy link
Contributor

it's fine here, in Budgie and in a separate branch + PR process.

This is a different style of working that some folks are used to - collaborative working sometimes is difficult on text only github style comments. Lets not use that as an excuse to prevent innovation. I think Jacob and Codic can craft something great here that will be useful to the wider community.

Stop it please

The phrase "Stop it" though does not contribute to discussion however misunderstood things are. A formal code of conduct will need to be introduced for the organisation and contributors need to fully understand it and abide by it. No specific person should talk down or be perceived talking down to anyone. That is bordering on the unacceptable. Lets not fall into this sort of way of talking to each other in the future.

@JoshStrobl
Copy link
Member

Lets not use that as an excuse to prevent innovation.

I agree, which is why I was disappointed to see the focus being on not submitting it until it's more mature and the development happening in an environment that doesn't encourage an immediate feedback loop, which is what this was intended for. I understand that folks are coming from a wide background and all have different experiences and expectations on development practices, so cultivating an understanding of that going forward is ideal.

A focus on a future CoC (similar to the one I had originally proposed for Solus to evolve its community engagement) will be engaging in constructive discussions and I will be creating a framework that establishes ideal feedback loops and processes for development, including but not limited to: RFCs, discussions, corresponding issues, preference of any branches and reducing the number of forks if possible, PRs and discussions around that.

I believe that should offer clarify and reduce frustrations. My intent here was to eliminate any cause for concern by the submitter and dissuade any language of otherwise, having experience circumstances where some may continue to assert their way of doing things (not claiming that was being done here, but avoiding it is paramount). I recognize it may not have been the intent of Jacob to dissuade anyone and I could have done better in communicating my request and clarity that this specific method for developing is acceptable (so I do apologize to @Jacob-Vlijm for that), that was simply the impression given and the end result was the closure of the PR, so clearer communication going forward is always ideal on all parts.

In regards to everyone getting engaged in finding a workable solution, as well as @codic12's ask for guidance, I certainly believe that can be provided in the current framework of this pull request.


Onto the topic of the PR itself and technicalities around implementation of the switch, @codic12 I would highly encourage you to take a look at the alt+tab switcher code available in src/daemon/tabswitcher, particularly the TabSwitcherWindow. This should provide some insight on implementation, such as:

  • Defining the window type (POPUP)
  • Setting the window position to ensure it is centered
  • Various signals such as hide and connecting to them
  • Skipping decorations and taskbar hinting so it doesn't show up in the IconTasklist

And so forth.

I believe a couple GtkButtons in the switcher, both with icons (Gtk.Image) that communicate vertical v.s. horizontal options, would likely be sufficient. If you want, you can also use a Gtk.Box set to vertical orientation, have the flowbox pushed to that, and have a dedicated "Cancel" button below it (or an explicit "close" icon above the flowbox), which can be handled with pack_end or pack_start respective on the box.

If @Jacob-Vlijm, @fossfreedom , @serebit, etc. have other ideas on how this switcher can be overlaid and presentable, certainly this is the place for this discussion.

@codic12
Copy link
Contributor Author

codic12 commented Jan 5, 2022

Thanks, I appreciate your points.

Various signals such as hide and connecting to them

This is the one point I don't get even after reading src/daemon/tabswitcher.vala. My only guess would be that it's communicating via DBus?

Also, for the initial implementation, would simply textual buttons saying "Vertical", "Horizontal", "Floating" etc do? With images we'd need to create assets (ideally scalable SVGs) first.

@JoshStrobl
Copy link
Member

My only guess would be that it's communicating via DBus?

If the WM didn't have a reference to it then yea, the best way of accomplishing it would be over D-Bus. In that file, there are various consts, like:

public const string SWITCHER_DBUS_NAME = "org.budgie_desktop.TabSwitcher";

and

public const string SWITCHER_DBUS_OBJECT_PATH = "/org/budgie_desktop/TabSwitcher";

In the TabSwitcherWindow, we set up various functions, such as setup_dbus starting on line 248, that attempt to "own" the tab switcher and register our connection. This is called from the WM itself and we pass info the switcher via asynchronous functions. This interface is defined in the window manager:

/**
* Allows us to display the tab switcher without Gtk
*/
[DBus (name="org.budgie_desktop.TabSwitcher")]
public interface Switcher: GLib.Object {
  public abstract async void PassItem(uint32 xid, uint32 timestamp) throws Error;
  public abstract async void ShowSwitcher(uint32 curr_xid) throws Error;
  public abstract async void StopSwitcher() throws Error;
}

This is, in my opinion, one of the better ways of passing information in a non-blocking way, to that window. To get info, we would create a couple signals, such as:

  • public signal void closed()
  • public signal void layout_selected(string layout)

We would connect in the wm.vala code to the closed signal so we are informed on closure (not an absolute necessity to do, we don't for TabSwitcher, but doesn't hurt to at least add the signal even if there are no consumers).

Similarly in the wm.vala code, we would have a function that connects to the layout_selected signal (via layout_selected.connect(ourfunctionname). This ourfunctionname would take one argument, the string of the layout. This can just as easily be an ENUM like LAYOUT_HORIZONTAL and LAYOUT_GRID, and the wm.vala would handle that and apply the appropriate logic.

By having that layout in the window manager, we may even be able to extend it to grid, or provide more functionality, possibly something that Budgie Shuffler can hook into down the line.

@guillotjulien
Copy link
Contributor

@codic12 @JoshStrobl would this be fit for the buttons? Made those myself so feel free to use them if they fit.

image

SVG are here: https://drive.google.com/drive/folders/1odvRuHOkxx0haFaGR1Wh2Nj6C7ezkO8V?usp=sharing

@codic12
Copy link
Contributor Author

codic12 commented Jan 5, 2022

@JoshStrobl thanks for the information, I think I have an idea of where to start now.

This can just as easily be an ENUM like LAYOUT_HORIZONTAL and LAYOUT_GRID

Indeed, it's already an enum:

	private enum Layout {
		FLOATING, // The default, traditional layout.
		TILING_HORIZ, // tiling with windows having 100% width and screen size / clients height
		TILING_VERT, // the opposite of the TILING_HORIZ layout.
	}

@codic12
Copy link
Contributor Author

codic12 commented Jan 5, 2022

@guillotjulien those buttons look great! would be happy to use them. Maybe make either all the windows filled or all of them empty, though; right now only the floating one doesn't have filled windows so it looks a bit inconsistent.

@JoshStrobl
Copy link
Member

@codic12 @JoshStrobl would this be fit for the buttons? Made those myself so feel free to use them if they fit.

image

SVG are here: https://drive.google.com/drive/folders/1odvRuHOkxx0haFaGR1Wh2Nj6C7ezkO8V?usp=sharing

That looks perfect.

@EbonJaeger
Copy link
Member

With regard to the icons, what about light theme users?

@codic12
Copy link
Contributor Author

codic12 commented Jan 5, 2022

They could be recolored to be black. Idk how we'd detect if the current theme is light or dark though.

@Jacob-Vlijm
Copy link
Contributor

Jacob-Vlijm commented Jan 5, 2022

By having that layout in the window manager, we may even be able to extend it to grid, or provide more functionality, possibly something that Budgie Shuffler can hook into down the line.

Just thinking out loud, but the whole procedure could be hooked up to the automatic layouts as done by shuffler applet:

image

Each layout is defined in its own string in gsettings, but could as well (in any layout arrangement) be stored in an array of layouts. Then, on new-window or window-closed signal (shuffler has a set-action option on both), windows would be rearranged into the next/previous layout. The advantage of this would be that you have full control, and a choice to either rearrange (only) vertically, horizontally or in any complex "next" order.

So indeed: one window -> full size, two windows -> 1/2 + 1/2? or 1/3 + 2/3 or 1/5 + 4/5? Whatever you like.
That would be under the hood quite a different story though, but relatively simple to create on a high level.

My personal opinion is that you shouldn't put this sort of stuff into the windowmanager, but play with from outside. It' s very interesting, but quite different from standard q/h tiling, and something probably only a selective group of users will appreciate (popos- ish).

@codic12
Copy link
Contributor Author

codic12 commented Jan 6, 2022

I'm having a lot of trouble trying to create a gtk popup, always running into issues - if someone else wants to do the basics of that, I'd be very welcome, otherwise I'll just let it be for the time being and look into it later.

My personal opinion is that you shouldn't put this sort of stuff into the windowmanager, but play with from outside. It' s very interesting, but quite different from standard q/h tiling, and something probably only a selective group of users will appreciate (popos- ish).

I'm also a fan of modularity. I would happily put this in an applet, a much more appealing choice, but I don't think this is possible. The thing is we need to intercept MapRequest and DestroyNotify events (which mutter does and we override functions map and destroy in wm.vala) in order to retile when windows open and close, we need to add an extra hook here. Is that possible with an applet? If so, I'll move all the code to an applet (of course, it could even be included into the inbuilt list of applets if it's good enough); just tell me the way!

@JoshStrobl
Copy link
Member

Would like to emphasize that this is something I do want baked into Budgie and exposed through the suggested UI and not just part of an applet. Having more advanced functionality in an applet I'm open to (even merging in Budgie Shuffler itself), but not having that as the sole solution.

@Jacob-Vlijm
Copy link
Contributor

Jacob-Vlijm commented Jan 6, 2022

Would like to emphasize that this is something I do want baked into Budgie and exposed through the suggested UI and not just part of an applet.

I am afraid I am misunderstood.
It is actually the other way around, I am definitely not suggesting making it part of an applet (wouldn't like that either), the image was just to show what functionality the applet performs that shuffler executes, in this case triggered by the applet; arranging either one, or a group of windows into a (any) layout. This could be automated on window create/destroy signal, calling a variety of layouts, depending on n-windows. This signal, including the data about "normal" windows, is already handed over by the daemon.
As I said, just thinking aloud.

@guillotjulien
Copy link
Contributor

image

@codic12 I agree, doesn't really make sense for only one icon to not be filled. Here is another version more consistent with the other two icons (floating_fill.svg).

@EbonJaeger I believe SVG can be recolored on the fly by themes. Not sure how it plays out with existing themes, maybe we'll have to set a special class? I'll have to check to be sure but it's definitely a valid concern.

@serebit serebit added the enhancement New feature or request label May 23, 2022
@JoshStrobl JoshStrobl closed this Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants