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

Fix issue with placement in per-monitor configuration. #1076

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

somiaj
Copy link
Collaborator

@somiaj somiaj commented Oct 20, 2024

When two monitors are on different desks, ensure that the correct monitor is used when placing windows. Otherwise placement policies can use the wrong desk when computing where to place a window. This is done two fold, first initialize windows with a NULL monitor so current monitor is used in cases a monitor is not specified for the window to start on, and second delay updating the window's monitor until after the window has been placed, so the correct monitor is found based on the windows location.

This fixes #1043.

When two monitors are on different desks, ensure that the correct
monitor is used when placing windows. Otherwise placement policies
can use the wrong desk when computing where to place a window.
This is done two fold, first initialize windows with a NULL monitor
so current monitor is used in cases a monitor is not specified
for the window to start on, and second delay updating the window's
monitor until after the window has been placed, so the correct
monitor is found based on the windows location.

This fixes #1043.
@ThomasAdam ThomasAdam added type:bug Something's broken! relates:placement Issue is in window placement code labels Oct 22, 2024
@ThomasAdam ThomasAdam added this to the 1.1.1 milestone Oct 22, 2024
@ThomasAdam ThomasAdam merged commit 72df98b into main Oct 22, 2024
12 checks passed
@ThomasAdam ThomasAdam deleted the js/placement-fix branch October 22, 2024 15:03
somiaj added a commit that referenced this pull request Nov 5, 2024
It is possible that the FvwmWindow monitor is NULL, that can cause
a crash when broadcasting the window's configuration to packet modules
by referencing a NULL pointer to send the monitor's ID. This now
happens with StartsOnPage due to #1076 setting the monitor to NULL
initially.

This fix ensures that the monitor is always defined after processing
any StartsOnScreen settings, by setting it to the current monitor if
it is still NULL. Also ensuring the monitor is defined here simplifies
some future checks that were doing the same thing by falling back to
the current monitor if it was undefined.

This fixes the crash reported in #1100.
somiaj added a commit that referenced this pull request Nov 6, 2024
It is possible that the FvwmWindow monitor is NULL, that can cause
a crash when broadcasting the window's configuration to packet modules
by referencing a NULL pointer to send the monitor's ID. This now
happens with StartsOnPage due to #1076 setting the monitor to NULL
initially.

This fix ensures that the monitor is always defined after processing
any StartsOnScreen settings, by setting it to the current monitor if
it is still NULL. Also ensuring the monitor is defined here simplifies
some future checks that were doing the same thing by falling back to
the current monitor if it was undefined.

This fixes the crash reported in #1100.
ThomasAdam pushed a commit that referenced this pull request Nov 7, 2024
It is possible that the FvwmWindow monitor is NULL, that can cause
a crash when broadcasting the window's configuration to packet modules
by referencing a NULL pointer to send the monitor's ID. This now
happens with StartsOnPage due to #1076 setting the monitor to NULL
initially.

This fix ensures that the monitor is always defined after processing
any StartsOnScreen settings, by setting it to the current monitor if
it is still NULL. Also ensuring the monitor is defined here simplifies
some future checks that were doing the same thing by falling back to
the current monitor if it was undefined.

This fixes the crash reported in #1100.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relates:placement Issue is in window placement code type:bug Something's broken!
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

TileManualPlacement doesn't apply to second monitor/desk
2 participants