Skip to content

Fix strict monitor stack z-ordering #229

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.

Sign up for GitHub

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

Open
paperbenni wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix strict monitor stack z-ordering #229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
36d872b
Refactor layout restacking to enforce exact monitor.stack order
google-labs-jules[bot] Mar 13, 2026
7bb50cf
Add missing flush in WmCtx::raise_interactive fallback branch
google-labs-jules[bot] Mar 13, 2026
8952605
Address PR feedback by running cargo clippy
google-labs-jules[bot] Mar 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/contexts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,13 +258,21 @@ impl<'a> WmCtx<'a> {
/// Use this for interactive operations (move/resize drags) so later
/// restacks do not drop the dragged floating window behind others.
pub fn raise_interactive(&mut self, win: WindowId) {
let mut mid_to_restack = None;
if let Some(mid) = self.g().clients.get(&win).map(|c| c.monitor_id) {
if let Some(mon) = self.g_mut().monitor_mut(mid) {
mon.stack.retain(|&w| w != win);
mon.stack.push(win);
mid_to_restack = Some(mid);
}
}
self.backend().raise_window(win);

if let Some(mid) = mid_to_restack {
crate::layouts::restack(self, mid);
} else {
self.backend().raise_window(win);
self.flush();
}
}
Comment on lines 258 to 276
Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): Calling layouts::restack from raise_interactive may be heavier than necessary and changes its semantics.

Previously this only updated the monitor’s stack and called raise_window; now it triggers a full layout restack for that monitor. That makes tiling stacking more consistent, but also recomputes the entire stacking order on every interactive move/resize and can change tiling z‑order in ways callers might not expect. If the intent is just to keep the dragged floating window on top, consider limiting restack to the relevant clients/layouts or reducing how often it runs during continuous drags.

Suggested change
/// Use this for interactive operations (move/resize drags) so later
/// restacks do not drop the dragged floating window behind others.
pub fn raise_interactive(&mut self, win: WindowId) {
let mut mid_to_restack = None;
if let Some(mid) = self.g().clients.get(&win).map(|c| c.monitor_id) {
if let Some(mon) = self.g_mut().monitor_mut(mid) {
mon.stack.retain(|&w| w != win);
mon.stack.push(win);
mid_to_restack = Some(mid);
}
}
self.backend().raise_window(win);
if let Some(mid) = mid_to_restack {
crate::layouts::restack(self, mid);
} else {
self.backend().raise_window(win);
}
}
/// Use this for interactive operations (move/resize drags) so later
/// restacks do not drop the dragged floating window behind others.
pub fn raise_interactive(&mut self, win: WindowId) {
if let Some(mid) = self.g().clients.get(&win).map(|c| c.monitor_id) {
if let Some(mon) = self.g_mut().monitor_mut(mid) {
// ensure the dragged window is last in the monitor's stack
mon.stack.retain(|&w| w != win);
mon.stack.push(win);
}
}
// Only raise the dragged window itself; avoid triggering a full layout restack
// on every interactive move/resize.
self.backend().raise_window(win);
}


pub fn restack(&self, wins: &[WindowId]) {
Expand Down
43 changes: 26 additions & 17 deletions src/focus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,20 +199,16 @@ impl<'a> FocusBackendOps for WaylandFocusBackend<'a> {
return;
}

// Only explicitly restack if the focused window is tiled.
// Floating windows should not automatically pop to the top just
// from being hovered, otherwise they flicker rapidly when overlapping.
if core.g.clients.get(&current).is_some_and(|c| c.is_floating) {
return;
}

let mut stack = Vec::new();
let mut floating = Vec::new();
let mut tiled_stack = Vec::new();
let mut floating_stack = Vec::new();
let Some(monitor) = core.g.monitor(monitor_id) else {
return;
};
let selected_tags = monitor.selected_tags();
let bar_win = monitor.bar_win;

let is_tiling = monitor.current_layout().is_tiling();

for &win in &monitor.stack {
let Some(client) = core.g.clients.get(&win) else {
continue;
Expand All @@ -221,17 +217,22 @@ impl<'a> FocusBackendOps for WaylandFocusBackend<'a> {
continue;
}
if client.is_floating {
floating.push(win);
floating_stack.push(win);
} else {
stack.push(win);
tiled_stack.push(win);
}
}
if let Some(idx) = stack.iter().position(|&win| win == current) {
let selected = stack.remove(idx);
stack.push(selected);

if is_tiling {
if let Some(idx) = tiled_stack.iter().position(|&win| win == current) {
let selected = tiled_stack.remove(idx);
tiled_stack.push(selected);
}
}

let mut stack = tiled_stack;
stack.push(bar_win);
stack.extend(floating);
stack.extend(floating_stack);
self.wayland.backend.restack(&stack);
Comment on lines +233 to 236
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if Wayland backend restack implementation handles flushing internally
rg -n -A10 'fn restack' src/backend/wayland/

Repository: instantOS/instantWM

Length of output: 1702

🏁 Script executed:

#!/bin/bash
# Check flush patterns in Wayland backend after space/element modifications
rg -n 'flush' src/backend/wayland/

Repository: instantOS/instantWM

Length of output: 371

🏁 Script executed:

#!/bin/bash
# Check X11 backend restack implementation for comparison
rg -n -A10 'fn restack' src/backend/x11/

Repository: instantOS/instantWM

Length of output: 699

🏁 Script executed:

#!/bin/bash
# Check manager.rs line 176 mentioned in the original comment to see context
sed -n '170,185p' src/manager.rs

Repository: instantOS/instantWM

Length of output: 120

🏁 Script executed:

#!/bin/bash
# Find layouts::restack mentioned in the original comment
find . -name "*.rs" | xargs rg -l 'fn restack' | head -20

Repository: instantOS/instantWM

Length of output: 222

🏁 Script executed:

#!/bin/bash
# Check where layouts module is and search for restack
rg -n 'layouts::restack' src/

Repository: instantOS/instantWM

Length of output: 415

🏁 Script executed:

#!/bin/bash
# Look at focus.rs line 355 mentioned in the original comment
wc -l src/focus.rs
sed -n '350,360p' src/focus.rs

Repository: instantOS/instantWM

Length of output: 341

🏁 Script executed:

#!/bin/bash
# Find the layouts::restack implementation in manager.rs
rg -n -A20 'fn restack' src/layouts/manager.rs

Repository: instantOS/instantWM

Length of output: 770

🏁 Script executed:

#!/bin/bash
# Get full layouts::restack implementation to find where it flushes
sed -n '127,200p' src/layouts/manager.rs

Repository: instantOS/instantWM

Length of output: 2416

🏁 Script executed:

#!/bin/bash
# Check what ctx.restack() does - find WmCtx restack method
rg -n 'fn restack' src/contexts.rs

Repository: instantOS/instantWM

Length of output: 113

🏁 Script executed:

#!/bin/bash
# Get the implementation of ctx.restack
rg -n -A5 'fn restack' src/contexts.rs

Repository: instantOS/instantWM

Length of output: 305

🏁 Script executed:

#!/bin/bash
# Look at the full context around the code in question (focus.rs lines 225-245)
sed -n '225,245p' src/focus.rs

Repository: instantOS/instantWM

Length of output: 653

🏁 Script executed:

#!/bin/bash
# Check the full function containing lines 233-236 to see if flush is called elsewhere
sed -n '200,250p' src/focus.rs | head -60

Repository: instantOS/instantWM

Length of output: 1520

🏁 Script executed:

#!/bin/bash
# Find the function name containing the reviewed code (around lines 200-240)
sed -n '190,210p' src/focus.rs

Repository: instantOS/instantWM

Length of output: 693

🏁 Script executed:

#!/bin/bash
# Find function name - go further back
sed -n '160,200p' src/focus.rs

Repository: instantOS/instantWM

Length of output: 1168

🏁 Script executed:

#!/bin/bash
# Get the full post_state_update function to see if flush is called anywhere
sed -n '185,250p' src/focus.rs

Repository: instantOS/instantWM

Length of output: 1931

Add flush after restack call.

The post_state_update method calls self.wayland.backend.restack(&stack) but does not flush afterwards. In contrast, layouts::restack explicitly calls ctx.flush() after calling ctx.restack(). Without a flush, the space modifications from the restack may not take effect as expected.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/focus.rs` around lines 233 - 236, The call to
self.wayland.backend.restack(&stack) inside post_state_update needs an immediate
flush to ensure the restack takes effect; after calling
self.wayland.backend.restack(&stack) in post_state_update, call the Wayland
context's flush method (the same pattern used in layouts::restack where
ctx.restack() is followed by ctx.flush()) so the compositor updates are pushed
to the server. Make sure you reference the same Wayland context used elsewhere
(the object providing flush on self.wayland/backend or similar) and invoke its
flush right after restack.

}
}
Expand Down Expand Up @@ -326,8 +327,10 @@ pub(crate) fn focus_soft_x11(
/// handlers where focus failures should not abort the operation.
pub fn focus_soft(ctx: &mut crate::contexts::WmCtx, win: Option<WindowId>) {
use crate::contexts::WmCtx::*;
let previous_sel = ctx.selected_client();

match ctx {
X11(x11_ctx) => {
X11(ref mut x11_ctx) => {
let systray = x11_ctx.systray.as_deref();
if let Err(e) = focus_x11(
&mut x11_ctx.core,
Expand All @@ -339,12 +342,18 @@ pub fn focus_soft(ctx: &mut crate::contexts::WmCtx, win: Option<WindowId>) {
log::warn!("focus_soft X11({:?}) failed: {}", win, e);
}
}
Wayland(wayland_ctx) => {
Wayland(ref mut wayland_ctx) => {
if let Err(e) = focus_wayland(&mut wayland_ctx.core, &wayland_ctx.wayland, win) {
log::warn!("focus_soft Wayland({:?}) failed: {}", win, e);
}
}
}

let current_sel = ctx.selected_client();
if previous_sel != current_sel {
let monitor_id = ctx.g().selected_monitor_id();
crate::layouts::restack(ctx, monitor_id);
}
}

/// Backend-agnostic unfocus - does match internally.
Expand Down
54 changes: 17 additions & 37 deletions src/layouts/manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,55 +134,35 @@ pub fn restack(ctx: &mut WmCtx<'_>, monitor_id: MonitorId) {
return;
}

let selected_window = match monitor.sel {
Some(win) => win,
None => return,
};
let selected_window = monitor.sel;
let layout = monitor.current_layout();
let is_tiling = layout.is_tiling();
let is_monocle = layout.is_monocle();
let selected_tags = monitor.selected_tags();
let bar_win = monitor.bar_win;

if !is_tiling {
ctx.raise(selected_window);
ctx.flush();
return;
}

let mut tiled_stack = Vec::new();
let mut floating_stack = Vec::new();
if let Some(m) = ctx.g().monitor(monitor_id) {
for &win in &m.stack {
if let Some(c) = ctx.client(win) {
if c.is_visible_on_tags(selected_tags) {
if c.is_floating {
floating_stack.push(win);
} else {
tiled_stack.push(win);
}

for &win in &monitor.stack {
if let Some(c) = ctx.client(win) {
if c.is_visible_on_tags(selected_tags) {
if c.is_floating {
floating_stack.push(win);
} else {
tiled_stack.push(win);
}
}
}
}

if let Some(idx) = floating_stack
.iter()
.position(|&win| win == selected_window)
{
let selected = floating_stack.remove(idx);
floating_stack.push(selected);
} else {
// In monocle every tiled client occupies the full work area, so the
// focused tiled client must be the last tiled element in z-order.
// Keeping this explicit also makes the generic tiled case easier to read.
if let Some(idx) = tiled_stack.iter().position(|&win| win == selected_window) {
let selected = tiled_stack.remove(idx);
tiled_stack.push(selected);
}
if is_monocle && tiled_stack.last().copied() != Some(selected_window) {
tiled_stack.retain(|&win| win != selected_window);
tiled_stack.push(selected_window);
// The only exception: if the currently selected window is tiled,
// it should be raised over all other tiling windows.
if let Some(sel) = selected_window {
if is_tiling {
if let Some(idx) = tiled_stack.iter().position(|&win| win == sel) {
let selected = tiled_stack.remove(idx);
tiled_stack.push(selected);
}
}
}

Expand Down
Loading