Hi, First of all, thank you all so much for providing this useful window manager! This is a reproduction of the question that I had asked in the forum https://forum.xfce.org/viewtopic.php?pid=37633#p37633. Currently the hotkeys tile-left and tile-right work only within a single display. I am currently using a script that a user has described here to move them across the "monitor barrier" http://makandracards.com/makandra/12447-how-to-move-a-window-to-the-next-monitor-on-xfce-xubuntu Given that 4.12 moved towards more multi-monitor support, I hoped to find something like that within XFCE now. I browsed the settings for a while but didn't discover anything. I am wondering if XFCE could natively support something like that by adding a 'move to other monitor' button to be defined as tile-left/right or by allowing tile-left to cycle through the monitors (as Win 7 does it). Best regards, Matthias Kauer
Could you describe the feature in more details? What is this supposed to do exactly?
Basically, I want to get an application window from monitor A to monitor B without removing my hands from the keyboard. This could be integrated with tiling, e.g., hitting tile left on window that was already tiled left on monitor B could move it to monitor A (this is Win 7 behavior). There could also just be another binding (as with the script I use now) and I can tile it once it is one the other monitor somehow.
I have written a patch to have better tiling support with multiple monitors, please see attached. The patch only works for TILE_LEFT and TILE_RIGHT. At the moment if you e.g. TILE_LEFT and the window is already tiled left, the position is again set to TILE_LEFT (although nothing actually changed). For these cases my patch first checks if there is a previous (or next) screen available. The comment in myScreenRebuildMonitorIndex made me believe that the screen_info->monitors_index is already sorted from top-left to bottom-right. Therefor I check the myScreenFindMonitorAtPoint method for the current monitor rectangle and loop num_monitors to check if "i - 1" (or "i + 1" for TILE_RIGHT) is available. If it is available, the patch does a TILE_RIGHT on the monitor to the left (or a TILE_LEFT on the monitor to the right). Although this means that myScreenFindMonitorAtPoint is actually called twice in these cases, both times it should always return on the '/* Cache system */' because a tiled window is on one monitor only. This implementation could be shorter if we would add an additional variable screen_info->cache_monitor_index so I would know what i is and dont have to loop num_monitors. Please comment on what you prefer. My patch also includes an implementation for https://bugzilla.xfce.org/show_bug.cgi?id=8768.
Created attachment 6895 patch
Created attachment 6915 0001-Improved-tile-support-with-multiple-monitors.patch My old implementation was bugged with windows that were resized by character and not pixel. This new implementation should fix that. Now when you TILE_LEFT or TILE_RIGHT, the client primitive maximized_vert is set. For TILE_UP and TILE_DOWN the maximized_horiz is set. This way we can check if we are tiling a window that is already tiled. The tiling behaviour is as follows: - TILE_LEFT (1st) -> window is tiled left - TILE_LEFT (2nd) -> window is tiled right on first monitor to the left of the current monitor - TILE_RIGHT (1st) -> window is tiled right - TILE_RIGHT (2nd) -> window is tiled left on first monitor to the right of the current monitor - TILE_UP (1st) -> window tiled up - TILE_UP (2nd) -> window is maximised - TILE_DOWN (1st) -> window is tiled down - TILE_DOWN (2nd) -> window is restored to original position For left & right, tiling loops continiously along all monitors. So if you TILE_LEFT for the second time on the first monitor, you will TILE_RIGHT on the far right monitor. Should this behaviour be configurable?
Comment on attachment 6915 0001-Improved-tile-support-with-multiple-monitors.patch Unfortunately that patch breaks un-tiling on single monitor. Pressing the same tiling key shortcut multiple times alterntes between left/right tiling instead of restoring the pre-tiled state. >From c240ee2176f1dafdf80340a0466d539a96f1109f Mon Sep 17 00:00:00 2001 >From: "P. Pronk" <xfce@pronk.nl> >Date: Thu, 1 Dec 2016 20:04:24 +0100 >Subject: [PATCH] Improved tile support with multiple monitors I would prefer a more detailed commit message mentioning the bug and what it does, something like: Subject: [PATCH 3/4] tiling: Improved tiling with multiple monitors Bug: 11936 Allows to switch back from tiled state using the same keyboard shortcut and improves tiling on multiple monitors. >--- > src/client.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----- > src/screen.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++- > src/screen.h | 11 ++++++++++ > 3 files changed, 139 insertions(+), 6 deletions(-) > >diff --git a/src/client.c b/src/client.c >index 668c11b..2bf15b6 100644 >--- a/src/client.c >+++ b/src/client.c >@@ -3453,6 +3453,7 @@ clientTile (Client *c, gint cx, gint cy, tilePositionType tile, gboolean send_co [...] >+ switch (tile) >+ { >+ case TILE_UP: >+ case TILE_DOWN: >+ if ((c->x == wc.x) && (c->y == wc.y) && Trailing whitespace here ^^^ >+ FLAG_TEST (old_flags, CLIENT_FLAG_MAXIMIZED_HORIZ)) [...] >+ case TILE_LEFT: >+ case TILE_RIGHT: >+ if ((c->x == wc.x) && (c->y == wc.y) && Trailing whitespace here ^^^ >+ FLAG_TEST (old_flags, CLIENT_FLAG_MAXIMIZED_VERT)) >+ { >+ if ((tile == TILE_LEFT) && Trailing whitespace here ^^^ >+ myScreenFindNeighbourMonitorAtPoint (screen_info, cx, cy, &rect, SCREEN_NEIGHBOUR_LEFT, TRUE)) [...] >+ { >+ restore_window = TRUE; >+ } >+ } >+ Whitespaces ^^^ >+ if (!restore_window) >+ { >+ FLAG_SET (c->flags, CLIENT_FLAG_MAXIMIZED_VERT | CLIENT_FLAG_RESTORE_SIZE_POS); >+ } >+ default: >+ FLAG_SET (c->flags, CLIENT_FLAG_RESTORE_SIZE_POS); >+ break; >+ } >+ Whitespaces ^^^ [...] >diff --git a/src/screen.c b/src/screen.c >index d5c7266..cf64172 100644 >--- a/src/screen.c >+++ b/src/screen.c >@@ -752,6 +752,66 @@ myScreenInvalidateMonitorCache (ScreenInfo *screen_info) > screen_info->cache_monitor.y = -1; > screen_info->cache_monitor.width = 0; > screen_info->cache_monitor.height = 0; >+ screen_info->cache_monitor_index = -1; >+} >+ >+gboolean >+myScreenFindNeighbourMonitorAtPoint(ScreenInfo *screen_info, gint x, gint y, GdkRectangle *rect, int direction, gboolean continious) ^^^ Typo And long line should be wrapped: myScreenFindNeighbourMonitorAtPoint (ScreenInfo *screen_info, gint x, gint y, GdkRectangle *rect, int direction, gboolean continuous) >+{ >+ gint num_monitors, monitor_index, gdk_monitor_index; >+ GdkRectangle monitor = { G_MAXINT, G_MAXINT, 0, 0 }; >+ >+ g_return_if_fail (screen_info != NULL); >+ g_return_if_fail (rect != NULL); >+ g_return_if_fail (GDK_IS_SCREEN (screen_info->gscr)); Needs a return value for all 3 cases above g_return_val_if_fail (screen_info != NULL, FALSE); g_return_val_if_fail (rect != NULL, FALSE); g_return_val_if_fail (GDK_IS_SCREEN (screen_info->gscr), FALSE); >+ TRACE ("entering myScreenFindNeighbourMonitorAtPoint"); [...] >+ else if (direction == SCREEN_NEIGHBOUR_RIGHT) >+ { >+ monitor_index = screen_info->cache_monitor_index + 1; >+ } >+ Whitespaces ^^^ >+ if (monitor_index < 0) [...] >+ else if (monitor_index > (num_monitors - 1)) >+ { >+ if (continious) >+ { >+ monitor_index = 0; >+ } >+ else >+ { >+ return FALSE; >+ } >+ Empty line ^^^ >+ } >+ >+ gdk_monitor_index = myScreenGetMonitorIndex (screen_info, monitor_index); >+ gdk_screen_get_monitor_geometry (screen_info->gscr, gdk_monitor_index, &monitor); >+ *rect = monitor; >+ Whitespaces ^^^ >+ return TRUE; >+ } >+ Whitespaces ^^^ >+ return FALSE; > } > > /* >@@ -787,11 +847,12 @@ myScreenFindMonitorAtPoint (ScreenInfo *screen_info, gint x, gint y, GdkRectangl > > monitor_index = myScreenGetMonitorIndex (screen_info, i); > gdk_screen_get_monitor_geometry (screen_info->gscr, monitor_index, &monitor); >- >+ Whitespaces ^^^ [...] >diff --git a/src/screen.h b/src/screen.h >index 2f19057..5fab43d 100644 >--- a/src/screen.h >+++ b/src/screen.h >@@ -127,6 +127,7 @@ struct _ScreenInfo > > /* Monitor search caching */ > GdkRectangle cache_monitor; >+ gint cache_monitor_index; > gint num_monitors; > GArray *monitors_index; > >@@ -230,6 +231,10 @@ struct _ScreenInfo > #endif /* HAVE_COMPOSITOR */ > }; > >+#define SCREEN_NEIGHBOUR_LEFT (1L<<0) >+#define SCREEN_NEIGHBOUR_RIGHT (1L<<1) What about horizontal tiling between multiple monitors stacked vertically?
Thank you for taking the time to look at the patch, I will clean up the code and resubmit the patch. It seems the patch currently also generates some compiler warnings, will have a look at them too. Restoring behaviour on a single monitor can be fixed by adding a check on screen_info->cache_monitor_index to make sure that myScreenFindNeighbourMonitorAtPoint returns a different monitor. (In reply to Olivier Fourdan from comment #6) > What about horizontal tiling between multiple monitors stacked vertically? In my opinion this would add too much complexity in the code to check which monitor is (for the most part) above which without giving a lot of benefits. And although this is maybe a personal preference caused by using Microsoft Windows for too long, I strongly prefer the maximising behaviour of TILE_UP. For me an use-case of TILING would be that I am searching (Google/StackOverflow) for an issue on my main monitor, then when I found what I am looking for I will TILE that window to a side monitor to start implementing that solution. Having the ability to maximise the window (so instead of using the upper half of the monitor only) by using the same keys benefits me greatly. If you compare the tiling behaviour after applying this patch with e.g. tiling in Windows, you will notice that: - Windows doesnt TILE to a corner - Windows doesnt TILE horizontically between multiple vertically stacked monitors, it also cycles through them from top,left to bottom,right - Windows doesnt TILE_UP or TILE_DOWN to just a half of the screen. Windows maximises on TILE_UP and on TILE_DOWN it first restores then minimises the window - Tiling on multiple monitors on Windows is mon1(LEFT -> original size and relative position -> RIGHT)->mon2(LEFT-> original size and relative position -> RIGHT)->Etc. If you wish we could mimic this behaviour but again I think it will add quite some complexity when using different size monitors but without a real benefit. Personally I just dont see the use-case of moving a window with the original size but relative position to another monitor. I'd guess the only use-case would be when you dont care about that window anymore and just want it removed from the main screen, but then it actually doesnt matter what you do with it that much and the LEFT or RIGHT behaviour suffices as well. What do you think?
Tile uo shortcut should definitely not maximize, there is a dedicated shortcut for maximizing. Besides, now that xfwm4 maximizes instead of tiling horizontally when dragging a window toward the top, the only way to achieve horizontal tiling is by using the keyboard shortcut, so it has to remain...
Created attachment 7416 Updated patch
Hi, I have (finally) cleaned up the patch and based it on the latest branch. It includes your previous remarks, let me know if there are still any issues :)
Comment on attachment 7416 Updated patch >From e14765a1ec66c87ad28a95be64fdf7a1ea70cbac Mon Sep 17 00:00:00 2001 >From: "P. Pronk" <xfce@pronk.nl> >Date: Sat, 4 Nov 2017 17:52:57 +0100 >Subject: [PATCH] tiling: Improved tiling with multiple monitors > >Bug: 11936 > >Allows to switch back from tiled state using the same keyboard shortcut >and improves tiling on multiple monitors. >--- > src/client.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----- > src/screen.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > src/screen.h | 10 +++++++++ > 3 files changed, 138 insertions(+), 5 deletions(-) > >diff --git a/src/client.c b/src/client.c >index 4085e75..26df8ab 100644 >--- a/src/client.c >+++ b/src/client.c >+ if (screen_info->cache_monitor_index > -1) Why that cached index? >+ { >+ if (direction == SCREEN_NEIGHBOUR_LEFT) >+ { >+ monitor_index = screen_info->cache_monitor_index - 1; >+ } >+ else if (direction == SCREEN_NEIGHBOUR_RIGHT) >+ { >+ monitor_index = screen_info->cache_monitor_index + 1; >+ } >+ >+ if (monitor_index < 0) >+ { >+ if (continious) spelling: "continuous" >diff --git a/src/screen.h b/src/screen.h >index e29f4e9..8e32146 100644 >--- a/src/screen.h >+++ b/src/screen.h >@@ -129,6 +129,7 @@ struct _ScreenInfo > GdkRectangle cache_monitor; > gint num_monitors; > GArray *monitors_index; >+ gint cache_monitor_index; > > /* Workspace definitions */ > guint workspace_count; >@@ -227,6 +228,9 @@ struct _ScreenInfo > #endif /* HAVE_COMPOSITOR */ > }; > >+#define SCREEN_NEIGHBOUR_LEFT (1L<<0) >+#define SCREEN_NEIGHBOUR_RIGHT (1L<<1) An enum would be preferable. Why bitwise values, you can't search for both directions at once...
-- GitLab Migration Automatic Message -- This bug has been migrated to xfce.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.xfce.org/xfce/xfwm4/-/issues/181. Please create an account or use an existing account on one of our supported OAuth providers. If you want to fork to submit patches and merge requests please continue reading here: https://docs.xfce.org/contribute/dev/git/start#gitlab_forks_and_merge_requests Also feel free to reach out to us on the mailing list https://mail.xfce.org/mailman/listinfo/xfce4-dev