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

Adjust windows to fit working area after move. #1067

Merged
merged 2 commits into from
Oct 16, 2024

Conversation

somiaj
Copy link
Collaborator

@somiaj somiaj commented Oct 13, 2024

When moving windows, it made sense to consider the working area base struts before the move when the base struts were on the global view port. Now that the base struts are per monitor this no longer makes sense, and caused strange behavior when computing where to move a window.

Instead the window's position should be adjusted to fit inside the working area after the move is done. This way the position of the window is always computed relative to the global screen (unless the 'screen RANDR_NAME' option is provided).

Notes, this is a breaking change as many multiple monitor configs have updated to work around this behavior. This should also fix #1033 and other issues where computations are done relative to the global screen but incorrectly adjusted when ran on monitors not located at (0,0).

This should also be tested, I've only done limited testing so far, but all my tests have been favorable. Hopefully anything this breaks is due to having to work around the old incorrect behavior.

@somiaj somiaj force-pushed the js/fix-move-working-area branch 4 times, most recently from 48e2a03 to 7d7ca04 Compare October 13, 2024 01:22
@somiaj
Copy link
Collaborator Author

somiaj commented Oct 13, 2024

This is another example of issues that should be fixed by this PR.

https://fvwmforums.org/t/fvwm3-setting-up-one-desktop-for-three-monitors/4734/5

@somiaj
Copy link
Collaborator Author

somiaj commented Oct 13, 2024

When working with the previous patch, I noticed that the Move shuffle commands didn't take into consideration any window hints when computing the working area. Added a patch to fix that issue as well.

@somiaj somiaj force-pushed the js/fix-move-working-area branch 3 times, most recently from 85836a0 to 739001a Compare October 13, 2024 05:14
@somiaj
Copy link
Collaborator Author

somiaj commented Oct 13, 2024

A few more notes, move coordinates are always done relative to the global screen (or specified screen) and are never relative to the working area (as it was before, so that is the breaking change). So users who want to move relative to the working area have to take that into account with their move commands. Instead the window's position is adjusted to ensure it is inside the working area.

Second, gravity isn't taken into account (probably it should be), and it is always the top left corner that determines what screen the window is on, and if a window is bigger than the working area, the position is adjusted so the top/left corner is at the edge of the working area.

@somiaj
Copy link
Collaborator Author

somiaj commented Oct 13, 2024

Updated so the center of the window (not the top/left corner) is used to determine which monitor the window is on. This way if a move command moves the top/left corner slightly off a monitor's boundary, it will still be adjusted to be on the monitor with the most overlap.

When computing the working area for 'Move shuffle' commands, only
the fvwm base struts were taken into consideration. This updates
it so the base struts hints from any running applications are also
taken into consideration when computing the working area.
When moving windows, it made sense to consider the working area
base struts before the move when the base struts were on the
global view port. Now that the base struts are per monitor this
no longer makes sense, and caused strange behavior since the
coordinates were relative to the current monitor when computing
where to move a window.

Instead the window's position is adjusted to fit inside the working
area of the monitor the window is mostly on (determined by the center
of the window) after the move is done. This way the position of the
window is always computed relative to the global screen (or the monitor
specified via the 'screen RANDR_NAME' option). This also ensures that
the window is placed on a valid page.

If the ewmhiwa option is provided, any adjustment to the window
is skipped, and this allows users to place the window anywhere
they want. This option should now be used more often in cases like
auto hiding a panel just off screen.
@jnse
Copy link

jnse commented Oct 16, 2024

I can confirm that using this branch, FvwmRearrange no longer moves windows off screen. There is still a few oddities, probably more to do with the module itself than this change:

  • 'FvwmRearrange -tile -h' seems to be adding unexpected padding between the windows.
  • 'FvwmRearrange -tile -desk' moves all windows to the current desk rather than tiling the windows on the desk they were originally on (which is what I'd expect it to do, although I'm not sure if it behaved like that in the past)

@somiaj
Copy link
Collaborator Author

somiaj commented Oct 16, 2024

@jnse thanks, those issues are going to be addressed in a future pull request, as this is only updating how fvwm Move command works internally (which FvwmRearrange) uses. I am working on improving the logic of FvwmRearrange to know things what the base struts are, which desk you are on, which page, and so on.

@ThomasAdam I think this is probably ready to go, as it fixed the primary issue with modules moving windows relative to base struts in multimonitor situation.

@ThomasAdam ThomasAdam added relates:desktop Issues is in desktop management code relates:ewmh Issues is in EWMH/ICCCM2 handling code labels Oct 16, 2024
@ThomasAdam ThomasAdam self-assigned this Oct 16, 2024
@ThomasAdam ThomasAdam added this to the 1.1.1 milestone Oct 16, 2024
@ThomasAdam ThomasAdam merged commit 2ff7452 into main Oct 16, 2024
5 checks passed
@ThomasAdam ThomasAdam deleted the js/fix-move-working-area branch October 16, 2024 21:40
@ThomasAdam ThomasAdam added the type:breaking Issue is not backwards-compatible and will break configs/build label Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relates:desktop Issues is in desktop management code relates:ewmh Issues is in EWMH/ICCCM2 handling code type:breaking Issue is not backwards-compatible and will break configs/build
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

FvwmRearrange broken on multi-monitor layout
3 participants