[PR #44] [MERGED] fix(compositor): pointer focus for newly mapped windows — bbox refresh, map-time enter, and a get_pointer-race guard #45

New issue

Closed

opened 2026-10-15 12:47:51 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-15 12:47:51 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/games-on-whales/gst-wayland-display/pull/44
Author: @salty2011
Created: 7/26/2026
Status: ✅ Merged
Merged: 9/7/2026
Merged by: @ABeltramo

Base: stable ← Head: fix/initial-pointer-focus

📝 Commits (3)

6dd6719 fix(compositor): refresh a newly mapped window's bbox so pointer focus resolves
c327a99 fix(compositor): resolve pointer focus for a newly mapped toplevel
6db7979 fix(compositor): re-emit wl_pointer.enter for clients that lose the map race

📊 Changes

5 files changed (+323 additions, -2 deletions)

View changed files

📝 wayland-display-core/src/comp/input.rs (+56 -2)
📝 wayland-display-core/src/comp/mod.rs (+5 -0)
📝 wayland-display-core/src/tests/client.rs (+16 -0)
📝 wayland-display-core/src/tests/test_pointer.rs (+209 -0)
📝 wayland-display-core/src/wayland/handlers/compositor.rs (+37 -0)

📄 Description

Window::bbox() stays (0,0) until on_commit() recomputes it from the surface tree, and the per-commit on_commit() at the top of commit() only runs for surfaces already in the space. A client whose root toplevel is mapped by this commit and never re-committed afterwards therefore keeps an empty bbox forever, so Space::element_under() never resolves it and wl_pointer focus is never assigned. Keyboard focus is assigned directly at map, which is what makes this awkward to diagnose: typing works, the pointer is dead.

This PR originally carried only the bbox refresh, and its description said of the map-time-enter follow-up: "That is a protocol-visible behaviour change rather than a bug fix, so it seemed wrong to bundle it in." We have since shipped that follow-up in production, hit the failure mode it introduces, and root-caused it — so the PR now carries the complete, hardened change as two further commits, each independently green including the test updates it necessitates. Field evidence below.

The three commits

on_commit() after map_element() (unchanged from the original PR): recomputes the bbox from the mapping commit's buffer so element_under() can resolve the window. The in-tree fixture.rs workaround ("I've spent hours trying to debug why the window size was kept at (0,0)") is this same call done by hand.
Synthetic zero-delta pointer_motion() at map: delivers wl_pointer.enter to the newly mapped toplevel immediately and runs constraint activation, so a pointer lock/confine requested before the surface became mappable (nested gamescope with --force-grab-cursor) is applied the moment the surface is focusable. Pointer focus thereby follows the newest toplevel exactly like keyboard focus. move_mouse is updated in the same commit for the earlier enter — the behaviour change the original description predicted — by draining the map-time Enter; its motion-delivery assertions are unchanged.
An edge-triggered refocus guarding the race that (2) introduces. smithay emits enter only on a focus transition, broadcasts it to the wl_pointer resources existing at that instant, and records the new focus unconditionally (GetPointer sends no retroactive enter). So when (2) resolves focus at map time, a client that calls wl_seat.get_pointer after that instant loses permanently: the enter reached zero resources, and every subsequent motion takes smithay's same-target arm — wl_pointer.motion only, no enter, forever. The client never learns it has pointer focus and discards all pointer input.

This is not theoretical. Rootful Xwayland (Xwayland :0, one surface for the whole X screen, mapped exactly once) loses the race deterministically: the cursor moves (compositor-drawn) but every X ButtonPress lands at one frozen stale coordinate — "mouse moves, nothing responds to clicks" on an XFCE-in-Xwayland desktop. gamescope loses it intermittently and self-heals on its next surface re-map, which presents as "input dead at launch, starts working later".

The fix: the map handler arms pending_pointer_refocus after its synthetic motion (order matters — that motion is itself a pointer_motion() call and would consume the flag inside the race window). The next real motion, guarded on surface-present (else retained), pointer-not-grabbed (else retained — a forced leave mid-click-drag would break the drag), and no-active-constraint (else cleared — a constraint can only activate on an entered surface, so the client demonstrably has its enter), forces the focus to None so the following motion takes the (focus, None) arm and re-emits a genuine enter. The re-emitted enter's serial is allocated after the forced leave's so it is the newer one, since clients echo enter serials (set_cursor, constraint activation).

Two tests assert a real fixture client receives the re-emitted Enter, and that the flag survives a motion with nothing under the pointer. (The fixture client binds its pointer at seat-bind time so it cannot lose the race naturally; the tests arm the flag directly.)

Field verification (Quasar, production fork)

Debugging chain on a live deployment, each hop instrumented: evdev capture proved motion+buttons reached the compositor's input devices; WAYLAND_DEBUG=1 inside the client container showed wl_pointer.motion and wl_pointer.button delivered with zero wl_pointer.enter in the whole trace (while wl_keyboard.enter arrived fine); xinput test-xi2 --root inside Xwayland showed Motion=0, RawMotion=0, ButtonPress frozen at one coordinate. With (3) applied: X Motion events flow and ButtonPress tracks the cursor; Steam/gamescope validated over repeated cold starts with no dead-input window; both confirmed by a human tester.

The deeper root cause — no retroactive enter for a wl_pointer created while its surface already holds focus — arguably belongs in smithay's GetPointer handling; we intend to raise it there. This PR's (3) is the compositor-side guard that makes map-time focus safe regardless.

Verification

cargo test -p wayland-display-core --lib in a container with the build deps, at every commit (the series is bisect-safe — each commit carries the test updates it necessitates):

	result
base (`stable` + the bbox commit)	18 passed, 1 failed (`utils::allocator::tests::test_dmabuf` — needs a real render node, fails in a deviceless container regardless)
+ map-time enter	18 passed, 1 failed — `move_mouse` green under the new behaviour
+ refocus guard (tip)	21 passed, 1 failed — the three refocus tests green (`pending_refocus_re_emits_enter` asserts a real fixture client receives the re-emitted `Enter`; `..._retained_without_surface` and `..._retained_while_grabbed` cover the retain guards)

cargo fmt --all --check clean and clippy zero-new at both commits.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/games-on-whales/gst-wayland-display/pull/44 **Author:** [@salty2011](https://github.com/salty2011) **Created:** 7/26/2026 **Status:** ✅ Merged **Merged:** 9/7/2026 **Merged by:** [@ABeltramo](https://github.com/ABeltramo) **Base:** `stable` ← **Head:** `fix/initial-pointer-focus` --- ### 📝 Commits (3) - [`6dd6719`](https://github.com/games-on-whales/gst-wayland-display/commit/6dd67191685f8e63db45c5bc81d7b9fc02cd3ffc) fix(compositor): refresh a newly mapped window's bbox so pointer focus resolves - [`c327a99`](https://github.com/games-on-whales/gst-wayland-display/commit/c327a995c43a6dad35c9945ae56cc7881ade75b0) fix(compositor): resolve pointer focus for a newly mapped toplevel - [`6db7979`](https://github.com/games-on-whales/gst-wayland-display/commit/6db7979683154f6e381b68a539aa3bd67e9ad0b1) fix(compositor): re-emit wl_pointer.enter for clients that lose the map race ### 📊 Changes **5 files changed** (+323 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `wayland-display-core/src/comp/input.rs` (+56 -2) 📝 `wayland-display-core/src/comp/mod.rs` (+5 -0) 📝 `wayland-display-core/src/tests/client.rs` (+16 -0) 📝 `wayland-display-core/src/tests/test_pointer.rs` (+209 -0) 📝 `wayland-display-core/src/wayland/handlers/compositor.rs` (+37 -0) </details> ### 📄 Description `Window::bbox()` stays `(0,0)` until `on_commit()` recomputes it from the surface tree, and the per-commit `on_commit()` at the top of `commit()` only runs for surfaces **already in the space**. A client whose root toplevel is mapped by this commit and never re-committed afterwards therefore keeps an empty bbox forever, so `Space::element_under()` never resolves it and `wl_pointer` focus is never assigned. Keyboard focus is assigned directly at map, which is what makes this awkward to diagnose: typing works, the pointer is dead. This PR originally carried only the bbox refresh, and its description said of the map-time-enter follow-up: *"That is a protocol-visible behaviour change rather than a bug fix, so it seemed wrong to bundle it in."* We have since shipped that follow-up in production, hit the failure mode it introduces, and root-caused it — so the PR now carries the complete, hardened change as two further commits, each independently green including the test updates it necessitates. Field evidence below. ### The three commits 1. **`on_commit()` after `map_element()`** (unchanged from the original PR): recomputes the bbox from the mapping commit's buffer so `element_under()` can resolve the window. The in-tree `fixture.rs` workaround ("I've spent hours trying to debug why the window size was kept at (0,0)") is this same call done by hand. 2. **Synthetic zero-delta `pointer_motion()` at map**: delivers `wl_pointer.enter` to the newly mapped toplevel immediately and runs constraint activation, so a pointer lock/confine requested *before* the surface became mappable (nested gamescope with `--force-grab-cursor`) is applied the moment the surface is focusable. Pointer focus thereby follows the newest toplevel exactly like keyboard focus. `move_mouse` is updated in the same commit for the earlier enter — the behaviour change the original description predicted — by draining the map-time `Enter`; its motion-delivery assertions are unchanged. 3. **An edge-triggered refocus guarding the race that (2) introduces.** smithay emits `enter` only on a focus *transition*, broadcasts it to the `wl_pointer` resources existing at that instant, and records the new focus unconditionally (`GetPointer` sends no retroactive enter). So when (2) resolves focus at map time, a client that calls `wl_seat.get_pointer` *after* that instant loses permanently: the enter reached zero resources, and every subsequent motion takes smithay's same-target arm — `wl_pointer.motion` only, no enter, forever. The client never learns it has pointer focus and discards all pointer input. **This is not theoretical.** Rootful Xwayland (`Xwayland :0`, one surface for the whole X screen, mapped exactly once) loses the race deterministically: the cursor moves (compositor-drawn) but every X `ButtonPress` lands at one frozen stale coordinate — "mouse moves, nothing responds to clicks" on an XFCE-in-Xwayland desktop. gamescope loses it intermittently and self-heals on its next surface re-map, which presents as "input dead at launch, starts working later". The fix: the map handler arms `pending_pointer_refocus` *after* its synthetic motion (order matters — that motion is itself a `pointer_motion()` call and would consume the flag inside the race window). The next real motion, guarded on surface-present (else retained), pointer-not-grabbed (else retained — a forced leave mid-click-drag would break the drag), and no-active-constraint (else cleared — a constraint can only activate on an entered surface, so the client demonstrably has its enter), forces the focus to `None` so the following motion takes the `(focus, None)` arm and re-emits a genuine `enter`. The re-emitted enter's serial is allocated after the forced leave's so it is the newer one, since clients echo enter serials (`set_cursor`, constraint activation). Two tests assert a real fixture client receives the re-emitted `Enter`, and that the flag survives a motion with nothing under the pointer. (The fixture client binds its pointer at seat-bind time so it cannot lose the race naturally; the tests arm the flag directly.) ### Field verification (Quasar, production fork) Debugging chain on a live deployment, each hop instrumented: evdev capture proved motion+buttons reached the compositor's input devices; `WAYLAND_DEBUG=1` inside the client container showed `wl_pointer.motion` and `wl_pointer.button` delivered with **zero `wl_pointer.enter`** in the whole trace (while `wl_keyboard.enter` arrived fine); `xinput test-xi2 --root` inside Xwayland showed `Motion=0, RawMotion=0`, ButtonPress frozen at one coordinate. With (3) applied: X `Motion` events flow and ButtonPress tracks the cursor; Steam/gamescope validated over repeated cold starts with no dead-input window; both confirmed by a human tester. The deeper root cause — no retroactive `enter` for a `wl_pointer` created while its surface already holds focus — arguably belongs in smithay's `GetPointer` handling; we intend to raise it there. This PR's (3) is the compositor-side guard that makes map-time focus safe regardless. ### Verification `cargo test -p wayland-display-core --lib` in a container with the build deps, **at every commit** (the series is bisect-safe — each commit carries the test updates it necessitates): | | result | |---|---| | base (`stable` + the bbox commit) | 18 passed, 1 failed (`utils::allocator::tests::test_dmabuf` — needs a real render node, fails in a deviceless container regardless) | | + map-time enter | 18 passed, 1 failed — `move_mouse` green under the new behaviour | | + refocus guard (tip) | **21 passed**, 1 failed — the three refocus tests green (`pending_refocus_re_emits_enter` asserts a real fixture client receives the re-emitted `Enter`; `..._retained_without_surface` and `..._retained_while_grabbed` cover the retain guards) | `cargo fmt --all --check` clean and clippy zero-new at both commits. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-15 12:47:51 +00:00