Skip to content

Commit

Permalink
fix(wm): correct use of z-order flags
Browse files Browse the repository at this point in the history
We had been setting managed windows to HWND_TOPMOST which is a sticky
and viral parameter. This was also the cause of the border window ending
up behind other windows in an undesirable fashion, as even though it was
marked WS_EX_TOPMOST, we were then having to mark it HWND_NOTOPTMOST
when raising it to avoid it ending up drawing over other windows.

Since we've fixed the border window to no longer be visible when
unmanaged windows are focused, we can now set the border window to
HWND_TOP when we reposition, which will ensure it's drawing in the order
that we want.

Now we also set managed windows only to HWND_TOP, rather than
HWND_TOPMOST which stops us from incorrectly reordering internal
concerns vs. child windows and owned windows that we're not managing.
Windows are still brought to the foreground as expected/desired, but
they're no longer 'sticking' there, nor are they drawing over the border
window.

This change does have a slight transition behavior as it initially rolls
out, as prior versions of the Komorebi have been setting HWND_TOPMOST,
which as a sticky parameter won't be cleared until the application or
host system removes that flag. This means that the final z-order
behavior will come good eventually.

To immediately see the correct results, restarting affected apps or
logging out / in will do. Unfortuantely we can't just set
HWND_NOTTOPMOST, as similarly to setting HWND_TOPMOST, this can cause
issues with an applications intended owned-window Z-Ordering - mostly
affecting toolwindows and child windows, such as file dialogs, toolbars
and so on, most of which we do not manage.
  • Loading branch information
raggi authored and LGUG2Z committed Apr 14, 2024
1 parent 732aca7 commit 28b46c5
Showing 1 changed file with 22 additions and 6 deletions.
28 changes: 22 additions & 6 deletions komorebi/src/windows_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ use windows::Win32::UI::WindowsAndMessaging::GWL_EXSTYLE;
use windows::Win32::UI::WindowsAndMessaging::GWL_STYLE;
use windows::Win32::UI::WindowsAndMessaging::GW_HWNDNEXT;
use windows::Win32::UI::WindowsAndMessaging::HWND_BOTTOM;
use windows::Win32::UI::WindowsAndMessaging::HWND_NOTOPMOST;
use windows::Win32::UI::WindowsAndMessaging::HWND_TOP;
use windows::Win32::UI::WindowsAndMessaging::LWA_ALPHA;
use windows::Win32::UI::WindowsAndMessaging::LWA_COLORKEY;
Expand Down Expand Up @@ -348,11 +347,18 @@ impl WindowsApi {
/// the layout to account for any window shadow borders (the window painted
/// region will match layout on completion).
pub fn position_window(hwnd: HWND, layout: &Rect, top: bool) -> Result<()> {
let flags = SetWindowPosition::NO_ACTIVATE
let mut flags = SetWindowPosition::NO_ACTIVATE
| SetWindowPosition::NO_SEND_CHANGING
| SetWindowPosition::NO_COPY_BITS
| SetWindowPosition::FRAME_CHANGED;

// If the request is to place the window on top, then HWND_TOP will take
// effect, otherwise pass NO_Z_ORDER that will cause set_window_pos to
// ignore the z-order paramter.
if !top {
flags |= SetWindowPosition::NO_Z_ORDER;
}

let shadow_rect = Self::shadow_rect(hwnd)?;
let rect = Rect {
left: layout.left + shadow_rect.left,
Expand All @@ -361,8 +367,19 @@ impl WindowsApi {
bottom: layout.bottom + shadow_rect.bottom,
};

let position = if top { HWND_TOP } else { HWND_NOTOPMOST };
Self::set_window_pos(hwnd, &rect, position, flags.bits())
// Note: earlier code had set HWND_TOPMOST here, but we should not do
// that. HWND_TOPMOST is a sticky z-order change, rather than a regular
// z-order reordering. Programs will use TOPMOST themselves to do things
// such as making sure that their tool windows or dialog pop-ups are
// above their main window. If any such windows are unmanaged, they must
// still remian topmost, so we set HWND_TOP here, which will cause the
// managed window to come to the front, but if the managed window has a
// child that is TOPMOST it will still be rendered above, in the proper
// order expected by the application. It's also important to understand
// that TOPMOST is somewhat viral, in that when you set a window to
// TOPMOST all of its owned windows are also made TOPMOST.
// See https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-setwindowpos#remarks
Self::set_window_pos(hwnd, &rect, HWND_TOP, flags.bits())
}

pub fn bring_window_to_top(hwnd: HWND) -> Result<()> {
Expand Down Expand Up @@ -397,8 +414,7 @@ impl WindowsApi {
// top of other pop-up dialogs such as a file picker dialog from
// Firefox. When adjusting this in the future, it's important to check
// those dialog cases.
let position = HWND_NOTOPMOST;
Self::set_window_pos(hwnd, layout, position, flags.bits())
Self::set_window_pos(hwnd, layout, HWND_TOP, flags.bits())
}

pub fn hide_border_window(hwnd: HWND) -> Result<()> {
Expand Down

0 comments on commit 28b46c5

Please sign in to comment.