Skip to content

fix(darwin): use primary screen height for SetPosition Y conversion#5304

Merged
leaanthony merged 5 commits into
masterfrom
agent/beta-release-manager/90319db7
May 12, 2026
Merged

fix(darwin): use primary screen height for SetPosition Y conversion#5304
leaanthony merged 5 commits into
masterfrom
agent/beta-release-manager/90319db7

Conversation

@leaanthony

@leaanthony leaanthony commented May 2, 2026

Copy link
Copy Markdown
Member

Summary

  • SetPosition() on macOS was calculating Y using the current window's screen height as the reference. This is wrong when the target position is on a monitor at a different vertical offset from the primary screen.
  • In macOS NSWindow coordinates, Y=0 is at the bottom-left of the primary screen ([[NSScreen screens] firstObject], always at origin (0,0)). The primary screen's height is therefore the correct global Y baseline.
  • Both windowGetPosition and windowSetPosition now use [[[NSScreen screens] firstObject] frame].size.height for the Y conversion, making Position() and SetPosition() consistent inverses across all monitor layouts.

Fixes #5117.

Test plan

  • Single monitor: call Position(), then SetPosition() with the returned values — window must end up at the same location
  • Two monitors side by side (same Y level): move window to second monitor with SetPosition(x, y) where x falls on the second monitor — window must appear at the correct Y position
  • Two monitors vertically stacked (different Y origins): move window to the upper/lower monitor — this was the broken case; window must now land at the correct position
  • Verify on mac-node1 / macbook-node1 with a two-screen macOS setup

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • BREAKING (macOS): Normalized macOS coordinates to primary-screen logical points with Y-down; window Position/SetPosition use logical points (no per-screen DPI scaling) and preserve round-tripping, fixing cross-screen Y placement.
    • Linux: Disable DMA-BUF renderer when NVIDIA GPUs are detected.
    • Windows: Fixed systray menu crash/leaks during menu rebuilds.
  • Documentation

    • Corrected the git PR template feedback URL.

Review Change Stack

When calling SetPosition on macOS, the Y coordinate was calculated
using the current window's screen height as the reference, which
produces the wrong NSWindow frame.origin.y when the target position
is on a monitor at a different vertical offset from the primary
screen.

macOS NSWindow coordinates have Y=0 at the bottom-left of the primary
screen (always [[NSScreen screens] firstObject], which is at origin
(0,0)). Both windowGetPosition and windowSetPosition now use the
primary screen's height as the global Y baseline, making get and set
consistent inverses regardless of which screen the window currently
lives on.

Fixes #5117

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
Copilot AI review requested due to automatic review settings May 2, 2026 12:15
@coderabbitai

coderabbitai Bot commented May 2, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Normalize macOS coordinates: screens and window Position/SetPosition now use a primary-screen–anchored logical point space with Y-down and (0,0) at the primary top-left; window position conversions no longer apply per-screen DPI scaling and use the primary screen height for Y flips.

Changes

macOS Cross-Screen Window Positioning Fix

Layer / File(s) Summary
Screen processing / Flip Y
v3/pkg/application/screen_darwin.go
processScreen flips screen Y coordinates using the primary screen height (with [NSScreen mainScreen] fallback) so Bounds.Y/WorkArea.Y use a Y-down primary-anchored logical space.
Window position conversions (get/set)
v3/pkg/application/webview_window_darwin.go
windowGetPosition and windowSetPosition now compute X directly from frame.origin.x and Y as primaryHeight - frame.origin.y - frame.size.height (and inverse for set) using the primary screen; per-screen backingScaleFactor is no longer applied in these conversions.
Changelog
v3/UNRELEASED_CHANGELOG.md
Adds a BREAKING (macOS) entry describing the normalized coordinate system and adds ## Fixed bullets including the macOS SetPosition cross-screen Y conversion fix and other platform fixes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

  • wailsapp/wails#5168 — Related macOS screen coordinate handling changes in screen_darwin.go.
  • wailsapp/wails#4818 — Prior work on Position()/SetPosition() coordinate consistency on macOS.
  • wailsapp/wails#3885 — Related changes to macOS window positioning in webview_window_darwin.go.

Suggested labels

Bug, MacOS

Suggested reviewers

  • atterpac

Poem

🐰
I hopped across each pixel plain,
I flipped the Y and left no stain.
Now windows leap to screens afar,
Anchored by the primary star. 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: using primary screen height for Y conversion in SetPosition on macOS, which directly addresses issue #5117.
Description check ✅ Passed The description provides clear context (problem, solution, rationale), links the issue (#5117), and outlines a comprehensive test plan. Most template sections are addressed or appropriately omitted.
Linked Issues check ✅ Passed The code changes directly address issue #5117 by using the primary screen height for Y conversion in both windowGetPosition and windowSetPosition, making cross-screen position moves consistent with the desired behavior.
Out of Scope Changes check ✅ Passed All changes (UNRELEASED_CHANGELOG.md documentation, webview_window_darwin.go, and screen_darwin.go) are directly related to normalizing macOS coordinate systems and fixing the SetPosition cross-screen issue, with no extraneous modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/beta-release-manager/90319db7

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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 the current code and only fix it if needed.

Inline comments:
In `@v3/pkg/application/webview_window_darwin.go`:
- Around line 621-627: windowGetPosition and windowSetPosition currently mix the
window's screen backingScaleFactor with the primary screen's height, causing
drift on mixed-DPI setups; fix by obtaining the primary screen's
backingScaleFactor (e.g. from [[[NSScreen screens] firstObject]
backingScaleFactor]) and use that same primary scale for both X and Y
conversions in windowGetPosition (where *x and *y are set) and in
windowSetPosition (where you convert desired coords back to frame.origin),
replacing any use of the current window screen's backingScaleFactor so both axes
share the global primary-screen scale baseline.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 616c5920-52e6-4184-864a-a2ce2698afc1

📥 Commits

Reviewing files that changed from the base of the PR and between b55cd96 and 79703db.

📒 Files selected for processing (2)
  • v3/UNRELEASED_CHANGELOG.md
  • v3/pkg/application/webview_window_darwin.go

Comment thread v3/pkg/application/webview_window_darwin.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts macOS window position Y-coordinate conversion to use the primary screen’s height as the global reference, aiming to make Position() and SetPosition() behave consistently across multi-monitor layouts with vertical offsets.

Changes:

  • Update windowGetPosition to compute Y using the primary screen height baseline.
  • Update windowSetPosition to compute Y using the primary screen height baseline.
  • Add an unreleased changelog entry for the macOS cross-screen SetPosition fix.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
v3/pkg/application/webview_window_darwin.go Switches Y conversion baseline from the current window’s screen height to the primary screen height for absolute positioning.
v3/UNRELEASED_CHANGELOG.md Documents the macOS SetPosition cross-screen Y conversion fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread v3/pkg/application/webview_window_darwin.go Outdated
Comment thread v3/pkg/application/webview_window_darwin.go Outdated
leaanthony and others added 2 commits May 2, 2026 22:25
…ed-DPI

Address CodeRabbit and Copilot review: windowGetPosition and
windowSetPosition were using the current window screen's
backingScaleFactor while relying on the primary screen's height for Y.
On mixed-DPI setups this mixes units — e.g. primary at 2x height 800pt
with secondary at 1x produces incorrect scaled values.

Both functions now obtain scale from the primary screen (firstObject)
to match its height reference, making get/set strict inverses in a
consistent "primary-screen physical pixel" coordinate space regardless
of which monitor the window occupies.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
v3/UNRELEASED_CHANGELOG.md (2)

29-29: ⚡ Quick win

Add PR link and author credit.

This entry is missing a PR reference and author credit, making it inconsistent with other entries in the changelog (lines 26, 28).

Additionally, the static analysis hint about "systray" is a false positive—it's a valid technical abbreviation for "system tray" commonly used in programming contexts.

As per coding guidelines: "Reference issue/PR numbers when applicable" to maintain changelog traceability and consistency.

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

In `@v3/UNRELEASED_CHANGELOG.md` at line 29, The changelog entry starting "Fix a
family of Windows systray `SetMenu` crashes..." is missing a PR reference and
author credit; update that entry to match the style of the neighboring entries
by appending the PR number/link and the author name (e.g., "(`#1234` by
`@username`)") in the same format used on lines 26 and 28, and leave the term
"systray" as-is (it's an accepted abbreviation), ensuring punctuation and
spacing align with other entries.

27-27: ⚡ Quick win

Add PR link and author credit for consistency.

This entry should follow the same format as the surrounding entries (lines 26, 28) by including a link to the PR and crediting the author.

📝 Proposed fix for consistent formatting
-- Fix `SetPosition` cross-screen Y conversion on macOS: use primary screen height as global reference so windows land at the correct position on monitors that are vertically offset from the primary display in [`#5117`](https://github.com/wailsapp/wails/issues/5117)
+- Fix `SetPosition` cross-screen Y conversion on macOS: use primary screen height as global reference so windows land at the correct position on monitors that are vertically offset from the primary display in [`#5117`](https://github.com/wailsapp/wails/issues/5117) / [PR](https://github.com/wailsapp/wails/pull/5304) by `@leaanthony`

As per coding guidelines: "Reference issue/PR numbers when applicable" and maintain consistency with other changelog entries.

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

In `@v3/UNRELEASED_CHANGELOG.md` at line 27, The changelog entry for the Fix to
SetPosition on macOS is missing the PR link and author credit; update the line
mentioning SetPosition so it matches surrounding entries by appending the PR
link and author credit in the same format (e.g., include "in
[`#5117`](https://github.com/wailsapp/wails/pull/5117) by `@author`"), and ensure it
references SetPosition and "primary screen height" as the fix description.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@v3/UNRELEASED_CHANGELOG.md`:
- Line 29: The changelog entry starting "Fix a family of Windows systray
`SetMenu` crashes..." is missing a PR reference and author credit; update that
entry to match the style of the neighboring entries by appending the PR
number/link and the author name (e.g., "(`#1234` by `@username`)") in the same
format used on lines 26 and 28, and leave the term "systray" as-is (it's an
accepted abbreviation), ensuring punctuation and spacing align with other
entries.
- Line 27: The changelog entry for the Fix to SetPosition on macOS is missing
the PR link and author credit; update the line mentioning SetPosition so it
matches surrounding entries by appending the PR link and author credit in the
same format (e.g., include "in
[`#5117`](https://github.com/wailsapp/wails/pull/5117) by `@author`"), and ensure it
references SetPosition and "primary screen height" as the fix description.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5cca8d78-63cc-4582-9c61-7e3b9b2a96a2

📥 Commits

Reviewing files that changed from the base of the PR and between 06370e1 and 6509edf.

📒 Files selected for processing (1)
  • v3/UNRELEASED_CHANGELOG.md

@leaanthony

Copy link
Copy Markdown
Member Author

This PR has the "awaiting review 🕝" label. Please confirm that you have tested this on a multi-monitor setup, especially with vertically stacked monitors at different Y offsets, as mentioned in the test plan.

The fix addresses window positioning across monitors on macOS by using the primary screen height as the reference for Y coordinate conversion. Please verify:

  • Single monitor: Position() → SetPosition() returns to same location
  • Side-by-side monitors: Window appears at correct Y on second monitor
  • Vertically stacked monitors: Window lands at correct position on upper/lower monitor

@flofreud

Copy link
Copy Markdown

The feedback on #5117 (comment) should be probably be addressed before merging.

And this change will break existing usages working around the issue - so maybe should be flagged in the release logs.

leaanthony added a commit that referenced this pull request May 11, 2026
…cal points, Y-down, top-left)

flofreud's review of #5304 (#5117 (comment))
flagged that macOS still exposed three distinct coordinate spaces:

  * GetScreens()      -> NSScreen logical points, Y-up,   bottom-left of primary
  * Position()/Set    -> NSScreen logical points * pSF,   Y-down, top-left of primary
  * RelativePosition  -> NSScreen logical points,         Y-down, top-left of *current* screen

This commit collapses GetScreens() and Position()/SetPosition() into a single
canonical space: logical points, Y-down, with (0,0) at the top-left of the
primary screen. This is the same convention exposed publicly by Electron's
Display.bounds, Tauri's Monitor::position, Win32 MONITORINFO, GTK and the web
(window.screenY / Screen.availTop). Windows + Linux paths in Wails already
expose this space, so the macOS normalisation also makes the three platforms
match each other.

Changes in webview_window_darwin.go:

  * windowGetPosition / windowSetPosition drop the `* primaryScale` /
    `/ primaryScale` multiplications. Values are now logical points, not
    "points x primaryScale". The Y flip against primary height stays.

Changes in screen_darwin.go:

  * processScreen flips Y on both `frame` and `visibleFrame` against the
    primary screen's height, so Bounds.Y is the screen's top edge and
    screens above the primary have negative Y. screenmanager's
    getScreenAlignment (which already assumed Y-down) becomes correct for
    macOS.

After this change `SetPosition(s.Bounds.X + dx, s.Bounds.Y + dy)` lands at
the expected offset on any screen, with no scale dance required.

BREAKING: callers that hand-compensated with `* primaryScale` or that
flipped Y against a screen height (e.g. the workaround in #5117) must drop
those conversions. Position()->SetPosition() round-trips are preserved.

A separate, pre-existing inconsistency in RelativePosition() (Get returns
absolute X, Set treats X as screen-relative) was also flagged in the same
review and should be addressed in its own issue/PR.

Refs #5117

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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 `@v3/pkg/application/screen_darwin.go`:
- Around line 47-51: GetPrimaryScreen() currently uses [NSScreen mainScreen]
which can differ from the fixed primary/menu-bar display used by processScreen()
(which normalizes Y against [[NSScreen screens] firstObject]); change
GetPrimaryScreen() to use [[NSScreen screens] firstObject] instead of [NSScreen
mainScreen] so it returns the same primary screen as processScreen(), and make
the same replacement in the other occurrence (the block around the second
NSScreen retrieval) to ensure Bounds and window position normalization use a
consistent primary display.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 83c18806-4268-461f-82b2-3f92956afc80

📥 Commits

Reviewing files that changed from the base of the PR and between 6509edf and a87758c.

📒 Files selected for processing (3)
  • v3/UNRELEASED_CHANGELOG.md
  • v3/pkg/application/screen_darwin.go
  • v3/pkg/application/webview_window_darwin.go
✅ Files skipped from review due to trivial changes (1)
  • v3/UNRELEASED_CHANGELOG.md

Comment thread v3/pkg/application/screen_darwin.go
…cal points, Y-down, top-left)

flofreud's review of #5304 (#5117 (comment))
flagged that macOS still exposed three distinct coordinate spaces:

  * GetScreens()      -> NSScreen logical points, Y-up,   bottom-left of primary
  * Position()/Set    -> NSScreen logical points * pSF,   Y-down, top-left of primary
  * RelativePosition  -> NSScreen logical points,         Y-down, top-left of *current* screen

This commit collapses GetScreens() and Position()/SetPosition() into a single
canonical space: logical points, Y-down, with (0,0) at the top-left of the
primary screen. This is the same convention exposed publicly by Electron's
Display.bounds, Win32 MONITORINFO, GTK and the web (window.screenY /
Screen.availTop). Windows + Linux paths in Wails already expose this space,
so the macOS normalisation also makes the three platforms match each other.

Changes in webview_window_darwin.go:

  * windowGetPosition / windowSetPosition drop the `* primaryScale` /
    `/ primaryScale` multiplications. Values are now logical points, not
    "points x primaryScale". The Y flip against primary height stays.

Changes in screen_darwin.go:

  * processScreen flips Y on both `frame` and `visibleFrame` against the
    primary screen's height, so Bounds.Y is the screen's top edge and
    screens above the primary have negative Y. screenmanager's
    getScreenAlignment (which already assumed Y-down) becomes correct for
    macOS.

After this change `SetPosition(s.Bounds.X + dx, s.Bounds.Y + dy)` lands at
the expected offset on any screen, with no scale dance required.

BREAKING: callers that hand-compensated with `* primaryScale` or that
flipped Y against a screen height (e.g. the workaround in #5117) must drop
those conversions. Position()->SetPosition() round-trips are preserved.

A separate, pre-existing inconsistency in RelativePosition() (Get returns
absolute X, Set treats X as screen-relative) was filed as #5408; the
mixed-DPI Bounds.X/Y scale-factor issue on non-primary screens was filed
as #5409.

Refs #5117
@leaanthony leaanthony force-pushed the agent/beta-release-manager/90319db7 branch from a87758c to 6975c21 Compare May 11, 2026 12:06
@leaanthony leaanthony merged commit c0706ac into master May 12, 2026
12 of 16 checks passed
@leaanthony leaanthony deleted the agent/beta-release-manager/90319db7 branch May 12, 2026 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[v3, macOS] SetPosition() cannot move windows across screens — Y conversion uses current screen instead of target

3 participants