implement locked cursor position hints #685
Conversation
|
Haven't read through this yet, but FYI I'll be merging this Smithay update PR: #230 |
That's okay, this can be rebased once that is merged. |
sodiboo
force-pushed
the
position-hint
branch
2 times, most recently
from
6739cb3 to
3aaab48
Compare
|
The Smithay PR was merged. I've updated this branch to no longer use my fork of Smithay. As far as i'm concerned, this can be merged as-is, and i'm already running this branch on my main system (precisely to fix the annoyance with the client i'm working on shown in the videos above), feel free to review it or block merging prior to the other PR and i'll rebase this to include just the third commit |
sodiboo
changed the title
sodiboo
changed the title
|
Since the Smithay changes are now merged separately into the main branch, i've dropped that first commit from this PR. I've kept the commit fixing a lint. The next commit implements cursor position hints naively, in such a way that a window on an output edge may cause the cursor to cross a monitor boundary or even end up out of bounds. This makes no sense for niri, because cursor position hints are surface-relative, and in niri, surfaces are positioned within each output and not in a global coordinate space. The newest commit fixes that issue, by constraining the cursor position hint to the current monitor's bounds. Previously, if the cursor went outside of all monitor boundaries (which is easy if the surface extends past the output edge), the cursor would just teleport back to the middle of the layout. Now, it works seamlessly, and the cursor is just snapped back to the same edge of a screen that it left through, once the client releases their pointer lock (or it otherwise is inactivated, such as by sending a position hint outside of their surface, which is blindly respected, but it can no longer cross a monitor boundary) It would be really cool to have some kind of scrolling mechanism in niri to make sure the cursor is still visible even if it is semantically off-screen[1] (just because we clip the cursor position hint doesn't mean the client cares, it might render a cursor off-screen, which is unintuitive), i have not implemented such a thing as it would require a deep dive into the layout code that i don't want to deal with. I'm also not going to implement such a mechanism for this PR, and i don't currently have plans to do anything further on this PR. This PR is ready to merge, as far as i'm concerned. |
(sidenote i've had this extremely cursed idea where the cursor always remains in the center of the monitor, and moving the mouse moves the whole rendering around it) |
I've had the same cursed idea, actually, but I I'm pretty sure I decided to keep my mouth shut once I realized that this would just be extremely disorienting and painful to use. I think it's more reasonable and much less motion-sickness-inducing if we enforce this constraint but with the horizontal struts instead of a line down the center of the output. |
|
unrelated, but it was brought up in the Smithay matrix that my implementation for Anvil (which was based on the one in niri) doesn't work for fractional scaling. and yeah, that seems reasonable. this PR probably needs to account for fractional scaling, and it should be tested prior to merging. also note that my Wayland backend (referenced above as my personal motivation for implementing this in the first place) does not currently do fractional scaling, so go find some other client to test with. |
There was a problem hiding this comment.
I applied some fixes, including the things I commented on. This should be good to merge. Could you test it once more?
unrelated, but it was brought up in the Smithay matrix that my implementation for Anvil (which was based on the one in niri) doesn't work for fractional scaling
Really? Seems fine here. I tested it and it seems to work fine too.
src/handlers/mod.rs
Outdated
| }); | ||
| pointer.set_location(target); | ||
| } else { | ||
| error!("cursor_position_hint called on a surface that is not the focused surface, but the constraint is active. this should be impossible."); |
There was a problem hiding this comment.
For future reference, rustfmt doesn't break apart long string literals on its own, so this might just cause it to prevent formatting for the whole block. There's probably an unstable flag for that but I always kinda did it by hand.
src/handlers/mod.rs
Outdated
| output_geometry.size -= (1, 1).into(); | ||
| (origin + location).constrain(output_geometry.to_f64()) | ||
| }); | ||
| pointer.set_location(target); |
There was a problem hiding this comment.
What if the window moves between set_cursor_position_hint and unlocking? Blender sets the position hint right in front of unlocking, but I don't see why that should always be the case. Either way, I guess it's not a big deal.
src/handlers/mod.rs
Outdated
| if focused_surface == surface { | ||
| let target = self | ||
| .niri | ||
| .output_for_root(surface) |
There was a problem hiding this comment.
This can be a subsurface, so this needs to get the root surface first.
src/handlers/mod.rs
Outdated
| error!("cursor_position_hint called on a surface that is not the focused surface, but the constraint is active. this should be impossible."); | ||
| } | ||
| } else { | ||
| error!("cursor_position_hint called with no focused surface, but the constraint is active. this should be impossible."); |
There was a problem hiding this comment.
These two error conditions are actually possible. At the moment, the niri pointer focus does not take pointer grabs into account (this pointer focus business is Complicated, fairly sure last time I looked into it I figured I need API in Smithay to get the active pointer grab data, but I never got around to adding it yet). Which means that you can start middle dragging in blender, then touchpad-swipe the window away, and the niri pointer focus will point at either nothing or another window, but the real Smithay pointer focus will still be on Blender (due to a click pointer grab).
src/handlers/mod.rs
Outdated
| output_geometry.size -= (1, 1).into(); | ||
| (origin + location).constrain(output_geometry.to_f64()) | ||
| }); | ||
| pointer.set_location(target); |
There was a problem hiding this comment.
Needs a queue redraw if the cursor was visible.
Hm, yeah. It does in fact seem to work just fine here. Note that i hadn't tested it with any scaling at all (only It's worth noting that the complaint i saw was about an entirely different compositor; but it was also with a very similar implementation to what i did here, and given that i don't mention scaling at all in this PR diff, i did expect the issue to be present in both, if any.
Yeah. It seems to work great! LGTM. |
|
Thanks |
* implement cursor position hints * Remove redundant fully qualified path * Find root surface * Convert nesting to if-return * Manually wrap error messages * Remove error!() prints * Add queue redraw --------- Co-authored-by: Ivan Molodetskikh <yalterz@gmail.com>
Firstly, this PR updates Smithay to latest master fixing stuff along the way(superseded by eb190e3) and also fixes a clippy lint in thesecondfirst commit.The
thirdsecond commit implements cursor position hints for locked pointers.Clients which hold a locked pointer may send a zwp_locked_pointer_v1::set_cursor_position_hint which we currently ignore. This leads to annoying behaviour, detailed below.
This is based off a Smithay PR, and is going to be a draft until that PR is merged into Smithay:
I ran into this while working on an improved Wayland backend to fix some annoyances with the winit backend. One such thing is more seamless pointer handling; to properly support features such as
warp-mouse-to-focusand internal pointer constraints, i must use the pointer constraints protocol in that backend. Since niri already has cursor compositing functionality baked in, it only makes sense to use this while niri has cursor focus. However, when the niri wayland backend loses cursor focus, the parent compositor (which is also niri, in my case) should know where the cursor actually is, because otherwise it can be really disorienting for a user.Let's introduce some names to these branches of niri, and how to tell them apart in the videos i'm about to show you:
The Wayland backend, aka the client shown in these videos, is correctly reporting its cursor position hint. It is the same client in both videos, and sending the same requests in both. The only difference is what the compositor (Yeah! I know the client is a compositor too, but to be clear i mean the outermost one) does with the position hint. It's worth noting that this cursor lock is held to allow the client to deal with its own internal pointer constraints smoothly. To ensure a seamless windowing experience, however, the lock is automatically released when the cursor touches the window edge. The intended result is that it's not obvious there is a cursor lock to begin with, but features like
warp-mouse-to-focusshould still "just work". Currently, in the winit backend, that doesn't work at all.The latest commit of niri's main branch (shown in orange) simply ignores it. This leads to a disorienting mess where the cursor seems to jump around erratically. When the pointer lock is deactivated, the pointer is returned to its initial position. Because of how this client in particular behaves, you need to go several roundtrips over a window before you can pass through it, but in other clients there may not even be a guarantee that the cursor can make progress. Because the compositor ignores cursor position hints, this leads to worst-case possibility that a surface can (unintentionally!) become an impenetrable barrier for the cursor. That's not very cool.
no-pointer-location-hint.mp4
However, the position hint variant from this PR (shown in blue) properly updates the pointer location hint whenever it receives a cursor position hint. Below is shown the same client; which hides the cursor and renders its own, and sends cursor position hints. The client is running the exact same commit of my Wayland backend for niri, but with the compositor supporting cursor position hints, it is now much more seamless. If you weren't paying attention really, you might think that the nested niri actually doesn't at all render a cursor, and this is just the outer cursor moving unimpeded across the surface. You'd be wrong.
with-pointer-location-hint.mp4