-
-
Notifications
You must be signed in to change notification settings - Fork 114
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 interactive move #547
base: main
Are you sure you want to change the base?
Conversation
Cool! But just FYI, I was going to get this done as part of adding floating windows, which will need lots of refactoring. |
(I am postponing it until then specifically because it will influence the architecture and design of moving windows) |
@YaLTeR If you are on it I can drop it. I wanted to try and get a feel for the code before committing to something publicly but do understand that it can mean it doesn't at all fit with your plans or is already started in some other way. |
I am not "on it" (busy with uni atm, don't want to take on big tasks for the time being), but it will likely lead to different design and code compared to implementing move without a floating layer. For example, I thought that moving a window out of a column would just put it on the floating layer for the duration of the move (and then whether it stays there or goes back into a column depends on where you drop it). Which is obviously not possible without a floating layer. Or, how to handle moves across monitors, which will also be just transparently handled by the floating layer, etc. |
That was badly worded. Having some thoughts about the implementation definately counts in what I inteded with "on it" even if not actively or having started coding. And you should ofc. prio uni. I'm thinking maybe I could take a shot at adding a "floating layer" that to start with will only be used for holding the window under drag and then full floating support can be added to it later? However that would immediately raise at the very least one architectural question that maybe you don't want to nail down atm. Is the floating layer bound to a workspace (so floating windows are hidden when you switch workspace/possibly other floating windows from the new workspace are shown) or is the floating layer above the workspace so the same floating windows are shown over all workspaces? Or is there anything else I can look at that could help bring floating windows closer while still being a limited scope? |
I think floating should be global across monitors (use the same coordinate system as the mouse pointer essentially) and, for the first impl, always on top (no workspaces). This means, among other things, moving monitor position tracking into the layout.
Well, I really don't want to tell people to drop something, especially when I haven't started working on it myself. I took a very brief glance at the contents of this PR and it seems to change all the expected places, which is good. But it's just, with larger refactors like this, it may turn out that reviewing someone else's work and shaping it to my vision takes way more effort for both parties than if I eventually just do a several day long dive myself. |
Totally agree and why not tackling floating layout directly and also why I'm happy to drop this work if you think it's too close to it. I'll start looking into shaping this into something that could work well with the floating layout you describe and see if it's possible while keeping it small. |
I've reworked it to remove the window from the workspace and store it on layout during the move instead. So while no floating layer atm. I think it should work pretty well with one, with some code just getting moved into it instead. So far it has been very kind to the existing structure but I do start to need to extract some stuff from Tile into some kind of TopLevel structure that can be used by both Tile and the moving window (as well as floating windows in the future). For example borders and the rendering logic. Still very wip and not ready for review but I'm at a point where its very usable for myself at least. |
That is what the Tile already is. Window with borders and other stuff. Don't mind that it's called Tile. |
I think this is working quite well now.
|
I'll try to take a look over the following days. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks very solid already!
These clients are very useful for testing: https://gitlab.freedesktop.org/emersion/wleird
Especially wleird-resize-loop. I also found useful these patches to it:
diff --git a/resize-loop.c b/resize-loop.c
index c1171a5..200908f 100644
--- a/resize-loop.c
+++ b/resize-loop.c
@@ -6,7 +6,7 @@
#define MIN 2
#define MAX 512
-#define SPEED 10
+#define SPEED 3
static int size = MIN;
static struct wleird_toplevel toplevel = {0};
@@ -27,6 +27,8 @@ static void callback_handle_done(void *data, struct wl_callback *callback,
toplevel.surface.width = abs(size);
toplevel.surface.height = abs(size);
+ xdg_toplevel_set_min_size(toplevel.xdg_toplevel, abs(size), abs(size));
+ xdg_toplevel_set_max_size(toplevel.xdg_toplevel, abs(size), abs(size));
surface_render(&toplevel.surface);
size += SPEED;
@@ -41,11 +41,13 @@ use smithay::backend::renderer::element::Id; | |||
use smithay::backend::renderer::gles::{GlesRenderer, GlesTexture}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a workspace switches, the new workspace should get an insert hint right away. Currently it doesn't until a mouse movement.
src/layout/workspace.rs
Outdated
let size = Size::from((self.data.get(column_index)?.width, 300.)); | ||
let loc = Point::from(( | ||
self.column_x(column_index), | ||
self.columns.get(column_index)?.tile_offset(tile_index).y - size.h / 2., |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't take topmost/bottommost gap into account, so the hint extends up/down there. I feel like it should end on the same pixel the window would end.
true | ||
} | ||
|
||
pub fn interactive_move_update( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When in the middle of a workspace switch, you can't interactive-move across the two workspaces. Up to you if you want to fix this because this is a bit of an edge case.
2215b11
to
7b5c01d
Compare
This now needs to be rebased to remove the gradient changes that were force-pushed to main. |
Do you mind if I finish this PR myself? |
Sorry that I've been so slow in picking this up again. Of course feel free to finish it! |
Alright, no worries! I'll try to get to it sometime in the next days. |
Right, sorry, enabled now |
7b5c01d
to
ef7b095
Compare
Thanks. Pushed a rebase with no further changes yet |
Fascinating. Seems like our Thankfully, they have a feature to work around the problem. |
34f82b6
to
69b270b
Compare
ee833f9
to
89582e4
Compare
89582e4
to
d9bb3d7
Compare
I've started working on interactive move support as I eventually want #122 but it's a little too big to start with. Interactive move feels smaller but still like a step towards it.
Currently implemented: