-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Support grabbing the pointer with the Pointer Lock API #1520
base: master
Are you sure you want to change the base?
Conversation
ec708a1
to
0113f7f
Compare
This change adds the following: a) A new button on the UI to enter full pointer lock mode, which invokes the Pointer Lock API[1] on the canvas, which hides the cursor and makes mouse events provide relative motion from the previous event (through `movementX` and `movementY`). These can be added to the previously-known mouse position to convert it back to an absolute position. b) Adds support for the VMware Cursor Position pseudo-encoding[2], which servers can use when they make cursor position changes themselves. This is done by some APIs like SDL, when they detect that the client does not support relative mouse movement[3] and then "warp"[4] the cursor to the center of the window, to calculate the relative mouse motion themselves. c) When the canvas is in pointer lock mode and the cursor is not being locally displayed, it updates the cursor position with the information that the server sends, since the actual position of the cursor does not matter locally anymore, since it's not visible. d) Adds some tests for the above. You can try this out end-to-end with TigerVNC with TigerVNC/tigervnc#1198 applied! Fixes: novnc#1493 under some circumstances (at least all SDL games would now work). 1: https://developer.mozilla.org/en-US/docs/Web/API/Pointer_Lock_API 2: https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#vmware-cursor-position-pseudo-encoding 3: https://hg.libsdl.org/SDL/file/28e3b60e2131/src/events/SDL_mouse.c#l804 4: https://tronche.com/gui/x/xlib/input/XWarpPointer.html
0113f7f
to
30f9d4e
Compare
@lhchavez your timing on working on this is hilarious, because I am in the middle of trying to add game support to this project right now and just jumped into the same issue that I'm trying to solve with the Pointer Lock API. Happy to help out in any way I can or be an extra tester for you. I immediately ran into the issue of noVNC not picking up pointer events when locked to the canvas, but your PR seems to address this. If I am understanding correctly, you aim to make it possible to call |
This makes it clear that people can call these methods.
errr... forgot to update the API docs ^^;; yes, I have now made it clear about the fact that folks can call |
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.
Thanks. This would be nice to get working. I do have some reservations though:
- Cursor isn't shown when this is active. This seems to be how the browsers implement things, so I guess we need to enable the cursor emulation when this is active.
- I'd really like to avoid adding more stuff to the toolbar and make this more seamless. And 99% of users won't use this so let's see if we can make this discrete for them. E.g. showing some icon to click when this is requested by the server?
- Related, I'd like to be very cautious about changing the API. It is forever, so adding things should be a last resort. And if we do, we should think about possible future steps as well. E.g. keyboard grabbing?
Let me look into this. (Pointers as to how to achieve that are welcome).
I couldn't find a way to detect when this was requested by the server :( Otherwise, the QEMU Pointer Motion Change might have been a better solution for that.
Yeah. What's the concrete suggestion in this case? Convert it into "input grabbing" and grab everything? (Want to avoid trying to guess a direction that will ultimately not get accepted.) |
Actually I'll just chime in and offer a counter-point to that. I feel in most contexts where you'd want to lock your pointer to the canvas, it is because the application being used either shows its own cursor, or the window moves in a way to reflect what your cursor is doing. You may not necessarily always want to see the emulated cursor in those cases. I think if anything an opt-in to the emulation might be better. |
You probably need to dig around in
When we get the warp request?
Probably. I haven't given it much thought yet. Ideally we find a solution where we don't need to change the API and this becomes a non-issue. :)
In such cases the application needs to disable its cursor, so that will continue to work fine. |
that doesn't work :( browsers require pointer-grabbing to occur on the same stack as a user event handler (e.g. a click event on a button). |
Right, so that's when we would show something for the user to click. Like how the browsers pop up something saying that this page requires feature X. E.g. a blinking icon in the corner. And how much have you experimented with this requirement from the browsers? Is it on every request? Or just the first? |
every one in the version of Chrome i tried. otherwise it's a potential attack vector for tricking people into clicking unrelated things. |
This avoids the user having to guess where their pointer is, since the browsers will hide the cursor with no option to unhide it.
neat, this is a much better experience! thanks for the help.
sounds good. any preference as to how a clickable element (see below for the rationale) would be rendered in vnc.html/ui.js? (i couldn't find anything that could be used as a reference, and want to avoid guessing). that also sounds like it would require another API change to be able to notify ui.js when warp requests are sent (or at least that's the way i would imagine implementing it).
changing the API probably is unavoidable due to browser restrictions. there needs to be something clickable/tappable (per https://w3c.github.io/pointerlock/#dfn-engagement-gesture). and as an embedder, i would like to be able to control how that element gets rendered / interacted with on our side. As for the API change, how about
After making the cursor render locally when pointer grab is enabled, things seem to work as expected: the applications typically require hiding the cursor in order to be able to enter cursor grab mode (SDL, FLTK, and the browsers all do this). and you still get to see a cursor otherwise. |
This new API can now be used to support [keyboard lock](https://web.dev/keyboard-lock/), although support for that is limited to Chrome only at the moment.
This change is a compromise to de-clutter the navbar by only showing the pointer capture button when fullscreen is enabled. There is no strong requirement (from the browser side) to be in fullscreen to acquire a pointer lock.
This is now addressed.
There is a compromise of not showing the button unless fullscreen is enabled.
There's no way around this :( so made it such that it can also grab the keyboard once more browsers support it (currently only Chrome does: https://caniuse.com/mdn-api_keyboard_lock) |
I finally got around to do a test here and the basics seem to work nicely in Firefox at least. I noticed one bug right away though: the cursor position doesn't account for where the canvas is. It might not be at the top left of the viewport. I'd also still like to see if we can do this without new API as such things are always costly. Let me experiment a bit and see what can be done. |
awesome, thanks! |
So here is a rough idea how it could work without any GUI or API changes. This works on Firefox at least, so hopefully on the other browsers as well. diff --git a/core/rfb.js b/core/rfb.js
index 79a3fd8..387c959 100644
--- a/core/rfb.js
+++ b/core/rfb.js
@@ -33,6 +33,12 @@ import HextileDecoder from "./decoders/hextile.js";
import TightDecoder from "./decoders/tight.js";
import TightPNGDecoder from "./decoders/tightpng.js";
+// Events that user interaction and hence permit certain operations:
+// https://html.spec.whatwg.org/multipage/interaction.html#user-activation-processing-model
+const ACTIVATION_EVENT_TYPES = [ 'change', 'click', 'contextmenu',
+ 'dblclick', 'mouseup', 'pointerup',
+ 'reset', 'submit', 'touchend' ]
+
// How many seconds to wait for a disconnect to finish
const DISCONNECT_TIMEOUT = 3;
const DEFAULT_BACKGROUND = 'rgb(40, 40, 40)';
@@ -157,6 +163,7 @@ export default class RFB extends EventTargetMixin {
this._mouseButtonMask = 0;
this._mouseLastMoveTime = 0;
this._pointerLock = false;
+ this._pendingPointerLock = false;
this._viewportDragging = false;
this._viewportDragPos = {};
this._viewportHasMoved = false;
@@ -176,6 +183,7 @@ export default class RFB extends EventTargetMixin {
handleMouse: this._handleMouse.bind(this),
handlePointerLockChange: this._handlePointerLockChange.bind(this),
handlePointerLockError: this._handlePointerLockError.bind(this),
+ checkPointerLock: this._checkPointerLock.bind(this),
handleWheel: this._handleWheel.bind(this),
handleGesture: this._handleGesture.bind(this),
};
@@ -442,14 +450,7 @@ export default class RFB extends EventTargetMixin {
requestInputLock(locks) {
if (locks.pointer) {
- if (this._canvas.requestPointerLock) {
- this._canvas.requestPointerLock();
- return;
- }
- if (this._canvas.mozRequestPointerLock) {
- this._canvas.mozRequestPointerLock();
- return;
- }
+ this._requestPointerLock();
}
// If we were not able to request any lock, still let the user know
// about the result.
@@ -530,9 +531,15 @@ export default class RFB extends EventTargetMixin {
if (document.onpointerlockchange !== undefined) {
document.addEventListener('pointerlockchange', this._eventHandlers.handlePointerLockChange, false);
document.addEventListener('pointerlockerror', this._eventHandlers.handlePointerLockError, false);
+ for (let type of ACTIVATION_EVENT_TYPES) {
+ this._canvas.addEventListener(type, this._eventHandlers.checkPointerLock)
+ }
} else if (document.onmozpointerlockchange !== undefined) {
document.addEventListener('mozpointerlockchange', this._eventHandlers.handlePointerLockChange, false);
document.addEventListener('mozpointerlockerror', this._eventHandlers.handlePointerLockError, false);
+ for (let type of ACTIVATION_EVENT_TYPES) {
+ this._canvas.addEventListener(type, this._eventHandlers.checkPointerLock)
+ }
}
// Wheel events
@@ -561,9 +568,15 @@ export default class RFB extends EventTargetMixin {
if (document.onpointerlockchange !== undefined) {
document.removeEventListener('pointerlockchange', this._eventHandlers.handlePointerLockChange);
document.removeEventListener('pointerlockerror', this._eventHandlers.handlePointerLockError);
+ for (let type of ACTIVATION_EVENT_TYPES) {
+ this._canvas.removeEventListener(type, this._eventHandlers.checkPointerLock)
+ }
} else if (document.onmozpointerlockchange !== undefined) {
document.removeEventListener('mozpointerlockchange', this._eventHandlers.handlePointerLockChange);
document.removeEventListener('mozpointerlockerror', this._eventHandlers.handlePointerLockError);
+ for (let type of ACTIVATION_EVENT_TYPES) {
+ this._canvas.removeEventListener(type, this._eventHandlers.checkPointerLock)
+ }
}
this._canvas.removeEventListener("mousedown", this._eventHandlers.focusCanvas);
this._canvas.removeEventListener("touchstart", this._eventHandlers.focusCanvas);
@@ -1061,11 +1074,13 @@ export default class RFB extends EventTargetMixin {
}
_handlePointerLockChange() {
+ console.log("Lock change", document.pointerLockElement, document.mozPointerLockElement);
if (
document.pointerLockElement === this._canvas ||
document.mozPointerLockElement === this._canvas
) {
this._pointerLock = true;
+ this._pendingPointerLock = false;
this._cursor.setEmulateCursor(true);
} else {
this._pointerLock = false;
@@ -1077,11 +1092,47 @@ export default class RFB extends EventTargetMixin {
}
_handlePointerLockError() {
+ console.log("Lock error");
this.dispatchEvent(new CustomEvent(
"inputlock",
{ detail: { pointer: this._pointerLock }, }));
}
+ _checkPointerLock() {
+ if (!this._pendingPointerLock) {
+ return;
+ }
+
+ console.log("Delayed attempt to get pointer lock");
+
+ this._requestPointerLock();
+ }
+
+ _requestPointerLock() {
+ // We don't want to grab the cursor from the user unexpectedly
+ // so only do it when we are focused and fullscreen
+ if (document.activeElement != this._canvas) {
+ return;
+ }
+ if (!document.fullscreenElement &&
+ !document.mozFullScreenElement &&
+ !document.webkitFullscreenElement &&
+ !document.msFullscreenElement) {
+ return;
+ }
+
+ this._pendingPointerLock = true;
+
+ if (this._canvas.requestPointerLock) {
+ this._canvas.requestPointerLock();
+ return;
+ }
+ if (this._canvas.mozRequestPointerLock) {
+ this._canvas.mozRequestPointerLock();
+ return;
+ }
+ }
+
_sendMouse(x, y, mask) {
if (this._rfbConnectionState !== 'connected') { return; }
if (this._viewOnly) { return; } // View only, skip mouse events
@@ -2420,6 +2471,9 @@ export default class RFB extends EventTargetMixin {
// Only attempt to match the server's pointer position if we are in
// pointer lock mode.
this._mousePos = { x: x, y: y };
+ } else {
+ console.log("Server wants pointer lock");
+ this._requestPointerLock();
}
return true; |
one thing that as a library consume would like to do is to be able to engage pointer lock without the requirement to be in fullscreen, which is the reason why it was done in that way to begin with. the use case is developing your app on replit.com, where it's beneficial to be able to view the code and/or logs while you interact with the app. is there any way that you could be persuaded to support this use case? |
The use case is perfectly reasonable, so no issues there. It does require more work though. My suggested approach pesters the browser until we're able to get the lock. However this might take a while, and during that time the application might no longer be interested in locking the pointer. And this VNC extension doesn't have a clear signal for that. So we'd need some heuristic to determine when to stop nagging the browser. Xwayland should have the same issue so it could be interesting to see what heuristic it implements for this. I seem to recall one parameter is that the cursor has to be blank. |
even if it requires more work, it's the only way it can be achieved :( (with the constraint of not requiring fullscreen). is there any chance that we can keep that constraint as a requirement? it's really important for us. |
Sure, but someone needs to get that heuristic in place. I'm afraid I haven't had any more time to play around with this so it might take a while if no one else has a look. |
ok, so how does this sound:
|
The point was to avoid adding new APIs, so the missing heuristic is for when it should do grabs when not in fullscreen. If you want to minimise the diff you have in your version we could do things in steps though. We could finish up and merge a version that only works in fullscreen. You would then have to keep a patch that gives you the extra API you need. Then, at a later time, we could get non-fullscreen working here and you can start using an unmodified noVNC again. |
i don't think it's feasible to add heuristics for the not-in-fullscreen case: we need an API. can we please add one? |
or rather, since a non-API, non-fullscreen world would rely on heuristics, there will be cases where users would want to have the cursor grabbed where the heuristics fail (and viceversa too). with fullscreen, the user's intent is pretty clear and non-ambiguous, so that case is completely fine. |
We want our APIs to be stable and permanent, so adding more should be a last resort as they can be a hindrance in the future. And I'm not convinced that we are at our last resort here. Xwayland has managed to figure this out, so we should be able to as well. With that said, we do try to make the |
Xwayland doesn't have the same non-fullscreen problem that we have :( wayland has complete control of the users' input and can use other signals into the decision whether to grab the cursor lock. the problems that are being solved are slightly different. in a browser, users sometimes want to not have their cursors grabbed from them even if the window appears as if it were.
is there any chance of adding more extensibility points so that clients have more flexibility in how they can do integrations? |
I'm not sure I see a meaningful difference? Wayland doesn't have pointer warping. So Xwayland needs to translate the pointer warping it gets from X11 to relative pointer mode and pointer lock on the Wayland side. Which sounds exactly like the problem we're facing. Xwayland has some back doors in to the compositor, but in most cases it has very little control.
As I mentioned, we are very cautious about adding new API. So that would be on a case by case basis depending on what possible limitations such hooks would impose. Feel free to discuss ideas on the mailing list/group and we can see what can be done. |
@lhchavez I have attempted to use this and I can see the icon, however the mouse does not actually lock. Running chrome on chrome OS. Tested using https://mdn.github.io/dom-examples/pointer-lock/ which works directly in the browser but not on a virtual machine using tigervnc and novnc. Feel free to reproduce this in gitpod: here Why isn't it working? How are you actually supposed to use the button? When do you press it? |
Trying out this branch because I need this functionality, clicking the pointer lock button messes up the cursor position. The cursor shown on the screen (which is not my browser cursor; it's the server's X cursor) is not aligned with what the pointer actually is over. The borders are misaligned, too; I cannot move the visible cursor past about halfway horizontally, but it is still possible to hover over things past that boundary (because the visible cursor and the actual cursor are misaligned). I suspect that is the root cause. I'm on Chrome OS 109. Edit: Figured out how to get a screenshot: Everything works fine when the pointer is not locked. This occurs not only in Minecraft but desktop applications too. |
I just resolved the conflicts because I was bored. Link: https://github.com/happylabdab2/noVNC |
Any updates here? I could really use this. |
when will this be released? |
This change adds the following:
a) A new button on the UI to enter full pointer lock mode, which invokes
the Pointer Lock API[1] on the canvas, which hides the cursor and
makes mouse events provide relative motion from the previous event
(through
movementX
andmovementY
). These can be added to thepreviously-known mouse position to convert it back to an absolute
position.
b) Adds support for the VMware Cursor Position pseudo-encoding[2], which
servers can use when they make cursor position changes themselves.
This is done by some APIs like SDL, when they detect that the client
does not support relative mouse movement[3] and then "warp"[4] the
cursor to the center of the window, to calculate the relative mouse
motion themselves.
c) When the canvas is in pointer lock mode and the cursor is not being
locally displayed, it updates the cursor position with the
information that the server sends, since the actual position of the
cursor does not matter locally anymore, since it's not visible.
d) Adds some tests for the above.
You can try this out end-to-end with TigerVNC with
TigerVNC/tigervnc#1198 applied!
Fixes: #1493 under some circumstances (at least all SDL games would now
work).
1: https://developer.mozilla.org/en-US/docs/Web/API/Pointer_Lock_API
2: https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#vmware-cursor-position-pseudo-encoding
3: https://hg.libsdl.org/SDL/file/28e3b60e2131/src/events/SDL_mouse.c#l804
4: https://tronche.com/gui/x/xlib/input/XWarpPointer.html