Skip to content

Fix windows overlapping across monitors#364

Open
biswadip-paul wants to merge 2 commits into
BarutSRB:mainfrom
biswadip-paul:fix/clamp-window-frames-to-monitor-bounds
Open

Fix windows overlapping across monitors#364
biswadip-paul wants to merge 2 commits into
BarutSRB:mainfrom
biswadip-paul:fix/clamp-window-frames-to-monitor-bounds

Conversation

@biswadip-paul
Copy link
Copy Markdown

@biswadip-paul biswadip-paul commented May 27, 2026

Summary

  • Clamps visible window frames to monitor bounds after layout calculation in calculateCombinedLayoutUsingPools()
  • Prevents tiled window frames from bleeding into adjacent monitors' screen space
  • Hidden windows (already tracked in hiddenPool) are excluded from clamping since their positioning is handled separately by HiddenWindowPlacementResolver

Details

The Niri layout engine's calculateLayoutInto() produces window frames based on container positions and scroll offsets without enforcing monitor boundary constraints. When a visible container partially extends beyond the monitor's frame rect, individual window frames within it can bleed into neighboring monitor space.

The fix adds a post-layout clamping pass that intersects each visible window's frame with monitor.frame, removing frames that fall entirely outside the monitor bounds.

Test plan

  • Open windows across two monitors — no window frame should extend beyond its assigned monitor
  • Scroll through columns near monitor edges — partial columns should be clipped at the boundary
  • Verify hidden windows still animate/position correctly (not affected by clamping)
  • Existing centeredColumnsDoNotEmit*WorkspaceFramesAcross*MonitorBoundary tests continue to pass

Fixes #349

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Fixed a window layout issue where frames could extend beyond monitor boundaries. Visible windows are now clamped to their assigned monitor’s display area so they stay fully on-screen.
    • Removed or hidden any windows with no overlap after clamping, preventing orphaned off-screen entries and improving layout consistency across multi-monitor setups.

Review Change Stack

…ames to monitor bounds Prevents tiled window frames from bleeding across monitor boundaries by clamping rendered frames to the owning monitor's frame rect after layout calculation. Hidden windows (already in hiddenPool) are excluded from clamping since their positioning is handled separately. Fixes BarutSRB#349 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a90a772a-f8db-4105-ac67-a118b2843be7

📥 Commits

Reviewing files that changed from the base of the PR and between 47866ca and c489737.

📒 Files selected for processing (1)
  • Sources/OmniWM/Core/Layout/Niri/NiriLayoutEngine+Animation.swift

📝 Walkthrough

Walkthrough

Window layout calculation in NiriLayoutEngine+Animation.swift now clamps visible window frames to monitor bounds before returning layout results. A new helper method intersects non-hidden window frames with the monitor bounds, removing frames outside bounds and updating frames that partially extend beyond.

Changes

Window Bounds Clamping

Layer / File(s) Summary
Monitor bounds clamping for visible windows
Sources/OmniWM/Core/Layout/Niri/NiriLayoutEngine+Animation.swift
clampVisibleFramesToMonitorBounds(_:) intersects each visible window frame with monitor bounds, removes null intersections, and updates frames that changed. The method is invoked from calculateCombinedLayoutUsingPools after building framePool/hiddenPool to restrict frames before returning results.

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~15 minutes

Poem

🐰 Frames that drift beyond the screen so wide,
Now clipped and bounded, safely tucked inside,
Each visible window meets the monitor line,
No stray corners peeking where they don't belong,
A tidy desktop hops along in time!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix windows overlapping across monitors' directly and clearly summarizes the main change: addressing window overlap issues in multi-monitor setups, which is the core objective of the PR.
Linked Issues check ✅ Passed The PR implements clamping of visible window frames to monitor bounds to prevent windows from overlapping across monitors, directly addressing the multi-monitor overlap issue reported in #349.
Out of Scope Changes check ✅ Passed All changes are focused on fixing window overlap by clamping frames to monitor bounds; no unrelated modifications or out-of-scope work is present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Sources/OmniWM/Core/Layout/Niri/NiriLayoutEngine`+Animation.swift:
- Around line 470-476: The current for-in loop over framePool mutates framePool
(removing/updating entries) which is unsafe; instead, iterate framePool without
mutating it by collecting changes first: create a removal list (e.g.,
removeTokens) and an updates dictionary (e.g., updatedFrames), loop over
framePool where hiddenPool[token] == nil computing clamped, append token to
removeTokens if clamped.isNull, or record updatedFrames[token] = clamped if
clamped != frame, then after the loop apply removals
(framePool.removeValue(forKey:)) and updates (framePool[token] = clamped) to
framePool. Ensure you reference the same symbols: framePool, hiddenPool,
monitorBounds, token, frame, clamped, and perform all mutations only after the
iteration completes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d55519b1-da37-4bad-ac22-bdbfadf09a3a

📥 Commits

Reviewing files that changed from the base of the PR and between 917e270 and 47866ca.

📒 Files selected for processing (1)
  • Sources/OmniWM/Core/Layout/Niri/NiriLayoutEngine+Animation.swift

Comment thread Sources/OmniWM/Core/Layout/Niri/NiriLayoutEngine+Animation.swift
…l dictionary during iteration Use two-pass approach: collect removals and updates first, then apply them after iteration completes. Swift dictionaries don't support safe mutation during for-in traversal. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@biswadip-paul
Copy link
Copy Markdown
Author

CodeRabbit's inline feedback on the unsafe dictionary mutation during iteration has been addressed in commit c489737 with the two-phase collect-then-apply approach. The follow-up review confirms no remaining actionable issues. LGTM — ready for maintainer review.

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.

Windows overlapping in multi monitor setup

2 participants