Conversation
Contributor
Author
|
@akurtakov in case you have time, please review, its in two commits to separate the two different changes, the first one is a protection against bad callers, the second the fix for the issue. |
There was a problem hiding this comment.
Pull request overview
Anchors GTK3 Wayland coordinates to monitor origins and prevents invalid negative shell allocations.
Changes:
- Aligns shell, control, cursor, and menu coordinates with monitor geometry.
- Converts display coordinates back before native positioning.
- Clamps shell content allocations to non-negative sizes.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
Shell.java |
Anchors shell coordinates and hardens allocation sizing. |
Menu.java |
Converts popup positions to shell-relative coordinates. |
Display.java |
Anchors cursor coordinates to a monitor. |
Control.java |
Adds monitor-aware coordinate conversion. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
vogella
force-pushed
the
wayland-monitor-coordinates
branch
from
August 26, 2026 16:59
fdfe5fb to
11e33fc
Compare
Contributor
vogella
force-pushed
the
wayland-monitor-coordinates
branch
from
August 26, 2026 18:22
c9af240 to
cae94f1
Compare
akurtakov
force-pushed
the
wayland-monitor-coordinates
branch
from
August 31, 2026 21:07
cae94f1 to
14721d1
Compare
vogella
force-pushed
the
wayland-monitor-coordinates
branch
2 times, most recently
from
September 15, 2026 13:27
785e864 to
02d3bf8
Compare
Member
|
Needs manual rebase. |
Wayland never tells a client where its window is, so gdk_window_get_origin returns window relative values while Monitor geometry is global. Code that clips a location against a monitor then mixes two coordinate spaces. The JFace information controls do exactly that. With a monitor layout whose client areas do not start near y=0, every anchor is rejected: hovers come up empty and the content assist javadoc popup lands on a different monitor. Wayland does report which monitor a surface is on. Anchor display coordinates to that origin, and remove it again when positioning a window or a menu. Display.getCursorLocation() and the MenuDetect event get the same treatment, since callers compare them against Control.toDisplay() and pass them to Menu.setLocation(). A shell tree shares one anchor, the monitor of its root toplevel: child shells are positioned relative to their parent, and a popup that is hidden or straddles a monitor edge reports no or a different monitor of its own. The window's position within its monitor is still unknown. Popups are placed relative to their parent and unaffected by that remaining error. X11 and GTK4 are unchanged. The two tests pass on a single monitor whose origin is 0,0, so they only bite on a multi-monitor Wayland session, but test_popupLocationRoundTrip covers the reuse cycle that JFace performs, which is where a wrong anchor shows up. Both are GTK3 only, since GTK4 toDisplay() is still shell relative. Fixes eclipse-platform#3539 Fixes eclipse-platform#538 Assisted-by: multiple AI agents and layers of automated tooling 🤖
vogella
force-pushed
the
wayland-monitor-coordinates
branch
from
September 21, 2026 07:55
02d3bf8 to
4b6d43a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On Wayland a client is never told where its window is, so SWT returned window relative display coordinates while
Monitorgeometry stayed global. Anything that clips a location against a monitor was therefore mixing two coordinate spaces, which the JFace information controls do: on a multi-monitor layout whose client areas do not start near y=0, every candidate position is rejected, so javadoc hovers came up empty and the content assist javadoc popup opened on a different monitor.Wayland does tell us which monitor a surface is on, so display coordinates are now anchored to that monitor origin and the offset is removed again when a window or menu is positioned. A whole shell tree uses the monitor of its root toplevel as the one anchor, since child shells are positioned relative to their parent and a hidden or edge-straddling popup reports no or a different monitor of its own.
Display.getCursorLocation()is anchored the same way, since callers compare it againstControl.toDisplay().Fixes #3539
Fixes #538
The reporter of #3539 confirmed both the empty hovers and the misplaced content assist popup are gone on their three monitor setup. #538 is the same root cause reported from several setups, all of them with a second monitor whose top edge is not aligned with the primary, and all of them worked around by aligning the monitors. The window position within its monitor is still unknown, but popups are placed relative to their parent and unaffected by that. X11 and GTK4 are unchanged.
The first commit is the independent size clamp from #3590 and goes away once that is merged.