Skip to content

fix: prevent window oscillation after display topology change#342

Merged
acsandmann merged 4 commits into
acsandmann:mainfrom
strayer:fix/window-oscillation-cross-display
Apr 27, 2026
Merged

fix: prevent window oscillation after display topology change#342
acsandmann merged 4 commits into
acsandmann:mainfrom
strayer:fix/window-oscillation-cross-display

Conversation

@strayer
Copy link
Copy Markdown
Contributor

@strayer strayer commented Apr 18, 2026

Summary

Fixes a bug where windows oscillate ~30 times/second between two display layout trees after a lid open/close cycle with an external monitor.

Root cause: sync_tiled_windows_for_app left windows in two space layout trees simultaneously after a cross-display move. Both spaces issued conflicting SetWindowFrame calls that fed back into each other indefinitely.

Two bugs in engine.rs conspired to keep the window in the source space's tree even after the VWM had moved it to the destination:

  1. Loop re-add bug: The loop that re-adds known VWM windows to desired did not check whether the VWM still assigned each window to this space. A moved window was re-added to the source tree from a stale VWM snapshot.

  2. Guard bug: The "AX login-screen guard" (if desired.is_empty() && total_tiled_count == 0) only checked has_windows_for_app, not whether those tree windows had been moved to another space. It skipped removal when it should have allowed it.

Fix (engine.rs):

  • Loop: skip re-adding any window whose VWM assignment for this space is gone (workspace_for_window returns None).
  • Guard: check if the windows in the layout tree have been moved away in the VWM; if so, allow removal to proceed. The login-screen/AX-failure case is still preserved: when the VWM still lists the window here (nothing moved it), the guard continues to skip removal.

Fix (window_discovery.rs):

  • Add a pre-pass in emit_layout_events that updates the VWM for all claimed windows before sending any per-space WindowsOnScreenUpdated events. This makes the engine fix order-independent regardless of which space's event fires first.

Tests:

  • window_removed_from_source_space_when_dest_claims_it_first — Case 1: destination space fires first (engine guard fix)
  • window_removed_from_source_space_when_source_empty_event_fires_first — Case 2: source empty event fires first (loop fix + pre-pass)
  • window_preserved_in_space_on_empty_discovery_without_cross_space_move — regression guard for login-screen / AX-failure scenario
  • discovery_after_display_change_places_window_on_correct_display — end-to-end integration test through the full WindowsDiscovered → emit_layout_events path

Note: discovery_after_display_change_places_window_on_correct_display calls simulate_until_quiet, which loops forever when the bug is present (faithfully reproducing the oscillation). With the fix applied it terminates normally. This is intentional — the infinite loop is the bug — but be aware that running this test without the fix will hang.

Disclaimer: The root cause analysis was performed by Claude Opus 4.6 (via DeepWiki analysis). The fix and tests were implemented by Claude Sonnet 4.6 (Claude Code). Guided and verified by the PR author.

Testing notice: I was unable to consistently reproduce the original oscillation issue in manual testing after applying the fix, but I haven't been able to test this exhaustively. Given that this fix involves AI-generated changes to core layout logic and could introduce subtle edge cases, I'd strongly appreciate careful review and real-world testing from others before merging.

Test plan

  • cargo test passes (all tests including 4 new regression tests)
  • New unit tests fail without the fix, pass with it
  • End-to-end test terminates with fix applied (would loop forever without it)
  • Manual testing: lid open/close with external monitor no longer causes window oscillation

strayer and others added 4 commits April 18, 2026 21:55
After a lid open/close cycle with an external monitor, windows could
enter a ~30 Hz oscillation between two display layout trees. The root
cause was a desync between the VirtualWorkspaceManager (VWM) and the
per-space layout trees in sync_tiled_windows_for_app.

When a window moves to a new display/space:
- The VWM is updated eagerly (remove from old space, add to new)
- The layout trees are updated lazily via WindowsOnScreenUpdated events

Two bugs conspired to keep the window in both trees:

1. The loop that re-adds known VWM windows to `desired` did not check
   whether the VWM still assigned each window to *this* space. A window
   moved to space2 was still re-added to space1's `desired` list from
   the VWM snapshot.

2. The "AX login-screen guard" (if desired.is_empty() && total_tiled_count == 0)
   only checked has_windows_for_app, not whether those tree windows had
   actually been moved to another space. It skipped removal when it
   should have allowed it.

Fix in engine.rs:
- Loop: skip re-adding any window whose VWM assignment for this space
  is gone (workspace_for_window returns None).
- Guard: additionally check if the windows still in the layout tree have
  been moved away in the VWM; if so, allow removal to proceed.

Fix in window_discovery.rs (emit_layout_events):
- Add a pre-pass that updates the VWM for all claimed windows before
  sending any per-space WindowsOnScreenUpdated events. This makes the
  engine fix order-independent: regardless of which space's event fires
  first, the VWM already reflects the final assignment.
# Conflicts:
#	src/actor/reactor/tests.rs
@acsandmann acsandmann merged commit 35af8fc into acsandmann:main Apr 27, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants