-
Notifications
You must be signed in to change notification settings - Fork 119
Fix pointer not properly grabbed on X11 apps #4596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@copilot Can you retry this work? |
When X11 applications (like games) go fullscreen and have focus, automatically lock the pointer to enable proper relative motion input. This fixes the issue where games like Counter-Strike 2 running under XWayland couldn't properly capture mouse movement for camera control. The solution: - Add update_pointer_confinement() method to XWaylandSurface - Call it when focus or window state changes - Automatically apply mir_pointer_locked_persistent when: - Surface is in fullscreen mode AND - Surface has focus - Remove pointer lock when either condition is no longer met This matches the behavior of other Wayland compositors like Weston. Co-authored-by: mattkae <25062299+mattkae@users.noreply.github.com>
Done! I've implemented automatic pointer locking for fullscreen XWayland surfaces. When X11 games go fullscreen and have focus, the pointer is now automatically locked using Changes in commit c45c83a:
This should fix the issue with Counter-Strike 2 and other X11 games not receiving proper mouse input. |
|
|
||
| void mf::XWaylandSurface::update_pointer_confinement() | ||
| { | ||
| std::lock_guard lock{mutex}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot this will hold the mutex when calling shell->modify_surface() - that may result in callbacks (via XWaylandSurfaceObserver) and a deadlock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit e2b43d6. The mutex is now released before calling shell->modify_surface(). The method now follows the same pattern as request_scene_surface_state() - extracting necessary values while holding the lock, then releasing it before the modify_surface call.
Release mutex before calling shell->modify_surface() to avoid potential deadlock from callbacks via XWaylandSurfaceObserver. This follows the same pattern used in request_scene_surface_state() and other methods in the same file. The fix: - Move all mutex-protected operations into a scoped block - Extract necessary values (scene_surface, desired_state, should_update) - Release the lock before calling shell->modify_surface() This addresses the concern raised by @AlanGriffiths about holding the mutex during shell->modify_surface() which could result in callbacks and deadlock. Co-authored-by: AlanGriffiths <9048879+AlanGriffiths@users.noreply.github.com>
mattkae
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixed the issue 🎉
|
@AlanGriffiths Would you like to give your opinion too? This is working for me |
|
@copilot Would this not break other fullscreen surfaces that do not need the cursor to be confined? For example, if I'm running a browser under XWayland, and I fullscreen it, would that not confine the cursor? EDIT: Just confirmed it locally. Launching google-chrome and pressing F11 to fullscreen it locks the cursor to the center of the screen. @copilot Is it possible to listen to XWayland clients specifically requesting the cursor to be locked and confining the cursor based on that instead of confining the cursor based on fullscreen and focus status? |
Change the heuristic to only lock pointer for: 1. Fullscreen + focused + decorations disabled (games) 2. OR override_redirect + focused (special game windows) This prevents locking the pointer for normal fullscreen applications like browsers (which keep decorations in fullscreen) while still working for games that disable decorations via motif hints. Addresses the issue where Chrome and other browsers had their pointer locked when going fullscreen (F11), which is not the desired behavior. Games like CS2 disable decorations and will still get pointer locking. Co-authored-by: tarek-y-ismail <171774743+tarek-y-ismail@users.noreply.github.com>
Fixed in commit 8678263. The heuristic now only locks pointer for:
This prevents browsers from getting pointer locked in fullscreen while keeping games working. Games typically disable window decorations via motif hints, while browsers keep them even in fullscreen mode. Ideally XWayland clients should use the Wayland |
tarek-y-ismail
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now works with google-chrome. @mattkae Can you check with CS2?
6749333 to
8041fca
Compare
| // Lock pointer for focused fullscreen windows that have decorations disabled. | ||
| // This heuristic captures games (which disable decorations) but not browsers | ||
| // (which keep decorations even in fullscreen). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would we lock the pointer based on these criteria? This seems like a very fragile heuristic: e.g. what about a game that isn't fullscreen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the X11 client be requesting a pointer grab from Xwayland? I'm not sure how that looks from the Mir side, but that has nothing to do with the above.
TICS Quality Gate❌ Failedmir
❌ Condition “No new Coding Standard Violations for level 1, 2, 3 with respect to Previous analysis” failed 1 time.
|
||||||||
| File | Issues | |
|---|---|---|
| 🪲 Total | ❌ Blocking | |
| 9 | +1 | |
See the results in the TICS Viewer
The following files have been checked for this project
- src/server/frontend_xwayland/xwayland_surface.cpp
- src/server/frontend_xwayland/xwayland_surface.h
I am concerned that this heuristic is going to create problems for applications that have no requirement for locking the cursor. There are games that may be fullscreen with no decorations and do not want the cursor locked. E.g. minesweeper or chess. And there are non-games that fullscreen without decorations. |
What's new?
Implements automatic pointer locking for fullscreen XWayland surfaces to fix pointer grab issues in X11 applications like Counter-Strike 2 running under XWayland.
When X11 games go fullscreen and have focus, the pointer is now automatically locked using
mir_pointer_locked_persistent, enabling proper relative motion input for camera control. The lock is automatically removed when the surface loses focus or exits fullscreen.Implementation Details
update_pointer_confinement()method toXWaylandSurfaceclassscene_surface_focus_set) and window state change (scene_surface_state_set) handlerszwp_pointer_constraints_v1shell->modify_surface()to prevent potential deadlock from callbacks viaXWaylandSurfaceObserverFiles Modified
src/server/frontend_xwayland/xwayland_surface.cpp- Implementation of pointer locking logicsrc/server/frontend_xwayland/xwayland_surface.h- Method declarationThis approach uses heuristics to distinguish between games (which need pointer locking) and regular desktop applications (which don't).
How to test
Games (should lock pointer):
Browsers (should NOT lock pointer):
Debugging:
MIR_SERVER_ENABLE_X11_VERBOSE_LOGGING=1, check logs for pointer confinement state changesChecklist
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.