Add support for notifying clients about pointer movements #1198

lhchavez · 2021-02-08T00:55:43Z

This change adds support for the VMware Mouse Position
pseudo-encoding[1], which is used to notify VNC clients when X11 clients
call XWarpPointer()[2]. This function is called by SDL[3] (and other
similar libraries) when they detect that the server does not support
native relative motion, like some RFB clients.

With this, RFB clients can choose to adjust the local cursor position
under certain circumstances to match what the server has set. For
instance, if pointer lock has been enabled on the client's machine and
the cursor is not being drawn locally, the local position of the cursor
is irrelevant, so the RFB client can use what the server sends as the
canonical absolute position of the cursor. This ultimately enables the
possibility of games (especially FPS games) to behave how users expect
(if the clients implement the corresponding change).

You can try this out end-to-end with noVNC with
novnc/noVNC#1520 applied!

Part of: #619

1: https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#vmware-cursor-position-pseudo-encoding
2: https://tronche.com/gui/x/xlib/input/XWarpPointer.html
3: https://hg.libsdl.org/SDL/file/28e3b60e2131/src/events/SDL_mouse.c#l804

lhchavez · 2021-02-08T01:11:24Z

The failures were caused by Docker authentication limits. I swear I did not break the build ^^;;

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

CendioOssman · 2021-02-08T10:26:42Z

Nice. I'll try to have a look and comment.

Have you also looked at adding support to our viewer? We prefer if features are fully implemented in both client and server so we can test properly without external projects.

lhchavez · 2021-02-08T12:35:39Z

Nice. I'll try to have a look and comment.

Have you also looked at adding support to our viewer? We prefer if features are fully implemented in both client and server so we can test properly without external projects.

not originally, but let me see how hard would it be.

lhchavez · 2021-02-08T15:11:59Z

Nice. I'll try to have a look and comment.
Have you also looked at adding support to our viewer? We prefer if features are fully implemented in both client and server so we can test properly without external projects.

not originally, but let me see how hard would it be.

Turns out most of the functionality was already there! Used this little snippet of code[1] because fltk does not support relative mouse events and the cursor eventually hit a wall at the edges of the screen and broke everything unless we also mirrored the cursor-warping that was done remotely.

1: https://www.mail-archive.com/[email protected]/msg00023.html

CendioOssman

This looks really nice. Good work!

Please also merge the two fix commits in too the others so we just have the two original commits. You can rebase and force push to fix up this.

How is this best tested? And what other servers and clients implement this?

CendioOssman · 2021-03-01T12:29:50Z

vncviewer/DesktopWindow.cxx

+  Window rootwindow = DefaultRootWindow(fl_display);
+  XWarpPointer(fl_display, rootwindow, rootwindow, 0, 0, 0, 0,
+               pos.x, pos.y);
+#endif


The coordinates here look wrong. The VNC screen might not be at coordinates 0,0, so everything here needs to be adjusted by an offset.

haha, this is my first change of a client application, so i basically stumbled upon a solution that worked for me in this one case (translation: please suggest what offset should be added).

i kinda guess that the viewport->x/y() is what we want, but have no way of confirming since i don't know any situations other than the one i can test locally.

I see two test scenarios:

Viewport doesn't match window. Just disable "Resize remote session..." in the options and make sure the session is larger or smaller than your client window

Client is on different monitors. You need two monitors for this, and disable "Enable full-screen mode over all monitors"

I would suspect you need both viewport->x/y() and window()->x/y() to properly get screen coordinates.

both cases seemed to work with a combination of x_root() + viewport->x(). thanks!

lhchavez · 2021-03-01T13:40:09Z

This looks really nice. Good work!

Please also merge the two fix commits in too the others so we just have the two original commits. You can rebase and force push to fix up this.

Done

How is this best tested? And what other servers and clients implement this?

So far only noVNC and this are the only open source server/clients that will implement this (after a quick Sourcegraph search). To test, run any SDL game that captures the cursor. In a pinch, running https://gist.github.com/lhchavez/50f4c31fc3faf811c3a572148479ef1b (requires Python3 and pygame) should do the trick.

CendioOssman · 2021-03-02T07:33:41Z

vncviewer/DesktopWindow.cxx

+  if (!mouseGrabbed) {
+    // Do nothing if we do not have the mouse captured.
+    return;
+  }


We probably shouldn't do this if we aren't focused as well.

wouldn't that make it hard to exit the client? in any case, moved to #1212.

How so? The warping should generally be in the way in such cases, not helping?

CendioOssman · 2021-03-02T07:33:57Z

vncviewer/DesktopWindow.cxx

+  Window rootwindow = DefaultRootWindow(fl_display);
+  XWarpPointer(fl_display, rootwindow, rootwindow, 0, 0, 0, 0,
+               pos.x + x_root() + viewport->x(),
+               pos.y + y_root() + viewport->y());


Have you tried this with Wayland? It doesn't seem to work for me here. :/

no, and i don't have a wayland environment :( is there a way to detect Wayland and not do this there? (in any case, moved to #1212).

CendioOssman · 2021-03-02T07:34:50Z

vncviewer/DesktopWindow.cxx

@@ -51,6 +51,8 @@

 #ifdef __APPLE__
 #include "cocoa.h"
+#include <CoreGraphics/CGGeometry.h>
+#include <CoreGraphics/CGRemoteOperation.h>


The compiler can't find these here. I'm using an older SDK and gcc instead of clang. Hower #include <Carbon/Carbon.h> works for me. Does that work for you?

i don't have a macOS device ^^;; i was just fiddling until i got it working on CI. what's preferable in these cases? to drop support for macOS altogether? (in any case, moved to #1212).

It works for me with those changes, so go with that. :)

CendioOssman · 2021-03-02T08:37:20Z

Doesn't seem to work on Windows either. macOS kind-a works, but there seems to be something generally wrong there unrelated to your changes.

lhchavez · 2021-03-02T13:15:26Z

Doesn't seem to work on Windows either. macOS kind-a works, but there seems to be something generally wrong there unrelated to your changes.

splitting this change into server-side (the one i really care about) and client-side, since it seems that the client-side will take more time.

CendioOssman · 2021-03-02T13:30:19Z

We don't want you disappearing on us once the server is merged though, so I prefer the one PR. :)

CendioOssman · 2021-03-02T13:31:02Z

The fix for macOS seems to be this at least:

diff --git a/vncviewer/DesktopWindow.cxx b/vncviewer/DesktopWindow.cxx
index bb51019e..385a9e88 100644
--- a/vncviewer/DesktopWindow.cxx
+++ b/vncviewer/DesktopWindow.cxx
@@ -188,6 +187,14 @@ DesktopWindow::DesktopWindow(int w, int h, const char *name,
 
   // Show hint about menu key
   Fl::add_timeout(0.5, menuOverlay, this);
+
+  // By default we get a slight delay when we warp the pointer, something
+  // we don't want or we'll get jerky movement
+#ifdef __APPLE__
+  CGEventSourceRef event = CGEventSourceCreate(kCGEventSourceStateCombinedSessionState);
+  CGEventSourceSetLocalEventsSuppressionInterval(event, 0);
+  CFRelease(event);
+#endif
 }

lhchavez · 2021-03-02T13:54:51Z

We don't want you disappearing on us once the server is merged though, so I prefer the one PR. :)

don't worry about that, i promise right here and now to finish the whole thing. disappearing once the server is merged through will contribute to building a bad relationship with you, and I already have a lot of PRs open in repositories you own, so it's in my best interest to see through all of them (within reason: i'm probably not going to go buy a Mac just to test these changes ^^;; ).

but on the other hand, throw a man a bone! this is the part of the change that i have a vested interest in (and ultimately what motivates me to do the other parts). i see the vncviewer part as "building goodwill" (other less charitable folks call it the "open source contributor tax").

lhchavez · 2021-03-02T15:03:48Z

i have no idea why Travis is complaining about HOWTO.md O_O

CendioOssman · 2021-03-02T15:51:00Z

i have no idea why Travis is complaining about HOWTO.md O_O

Unrelated and fixed on master now. :)

This change adds support for the VMware Mouse Position pseudo-encoding[1], which is used to notify VNC clients when X11 clients call `XWarpPointer()`[2]. This function is called by SDL (and other similar libraries) when they detect that the server does not support native relative motion, like some RFB clients. With this, RFB clients can choose to adjust the local cursor position under certain circumstances to match what the server has set. For instance, if pointer lock has been enabled on the client's machine and the cursor is not being drawn locally, the local position of the cursor is irrelevant, so the RFB client can use what the server sends as the canonical absolute position of the cursor. This ultimately enables the possibility of games (especially FPS games) to behave how users expect (if the clients implement the corresponding change). Part of: TigerVNC#619 1: https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#vmware-cursor-position-pseudo-encoding 2: https://tronche.com/gui/x/xlib/input/XWarpPointer.html 3: https://hg.libsdl.org/SDL/file/28e3b60e2131/src/events/SDL_mouse.c#l804

lhchavez · 2021-03-02T16:41:46Z

i have no idea why Travis is complaining about HOWTO.md O_O

Unrelated and fixed on master now. :)

yay, thanks! i was stumped for a bit there as to how the other PR wasn't getting in the same situation.

lhchavez · 2021-03-11T14:26:15Z

is this change also good to merge?

CendioOssman

Yup. The only notes where about the client side of things.

samhed · 2021-12-01T12:15:51Z

For reference, the vncviewer side of this: #1212

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

lhchavez mentioned this pull request Feb 8, 2021

Support grabbing the pointer with the Pointer Lock API novnc/noVNC#1520

Open

samhed added the enhancement New feature or request label Feb 8, 2021

lhchavez force-pushed the vmware-cursor-position branch from 6d1de4f to 0029a49 Compare February 8, 2021 17:09

CendioOssman requested changes Mar 1, 2021

View reviewed changes

lhchavez force-pushed the vmware-cursor-position branch from 491daa9 to ee0e0ef Compare March 1, 2021 13:35

lhchavez force-pushed the vmware-cursor-position branch from ee0e0ef to 536b679 Compare March 1, 2021 15:16

CendioOssman requested changes Mar 2, 2021

View reviewed changes

lhchavez force-pushed the vmware-cursor-position branch from 536b679 to b151bbd Compare March 2, 2021 13:14

lhchavez force-pushed the vmware-cursor-position branch from b151bbd to 75d36b7 Compare March 2, 2021 13:23

lhchavez force-pushed the vmware-cursor-position branch from 75d36b7 to cb8629a Compare March 2, 2021 16:37

CendioOssman approved these changes Mar 11, 2021

View reviewed changes

CendioOssman merged commit 41bf43b into TigerVNC:master Mar 11, 2021

lhchavez deleted the vmware-cursor-position branch March 18, 2021 23:57

kasmtech mentioned this pull request Apr 9, 2021

Support grabbing the pointer with the Pointer Lock API kasmtech/noVNC#2

Open

CendioOssman mentioned this pull request Apr 9, 2021

[REQUEST] Mouse synchronization/grabbing/capture/wrapping #619

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for notifying clients about pointer movements #1198

Add support for notifying clients about pointer movements #1198

lhchavez commented Feb 8, 2021 •

edited

Loading

lhchavez commented Feb 8, 2021

CendioOssman commented Feb 8, 2021

lhchavez commented Feb 8, 2021

lhchavez commented Feb 8, 2021

CendioOssman left a comment

CendioOssman Mar 1, 2021

lhchavez Mar 1, 2021

CendioOssman Mar 1, 2021

lhchavez Mar 1, 2021

lhchavez commented Mar 1, 2021

CendioOssman Mar 2, 2021

lhchavez Mar 2, 2021

CendioOssman Mar 2, 2021

CendioOssman Mar 2, 2021

lhchavez Mar 2, 2021 •

edited

Loading

CendioOssman Mar 2, 2021

lhchavez Mar 2, 2021

CendioOssman Mar 2, 2021

CendioOssman commented Mar 2, 2021

lhchavez commented Mar 2, 2021

CendioOssman commented Mar 2, 2021

CendioOssman commented Mar 2, 2021

lhchavez commented Mar 2, 2021 •

edited

Loading

lhchavez commented Mar 2, 2021

CendioOssman commented Mar 2, 2021

lhchavez commented Mar 2, 2021

lhchavez commented Mar 11, 2021

CendioOssman left a comment

samhed commented Dec 1, 2021

Add support for notifying clients about pointer movements #1198

Add support for notifying clients about pointer movements #1198

Conversation

lhchavez commented Feb 8, 2021 • edited Loading

lhchavez commented Feb 8, 2021

CendioOssman commented Feb 8, 2021

lhchavez commented Feb 8, 2021

lhchavez commented Feb 8, 2021

CendioOssman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lhchavez commented Mar 1, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lhchavez Mar 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CendioOssman commented Mar 2, 2021

lhchavez commented Mar 2, 2021

CendioOssman commented Mar 2, 2021

CendioOssman commented Mar 2, 2021

lhchavez commented Mar 2, 2021 • edited Loading

lhchavez commented Mar 2, 2021

CendioOssman commented Mar 2, 2021

lhchavez commented Mar 2, 2021

lhchavez commented Mar 11, 2021

CendioOssman left a comment

Choose a reason for hiding this comment

samhed commented Dec 1, 2021

lhchavez commented Feb 8, 2021 •

edited

Loading

lhchavez Mar 2, 2021 •

edited

Loading

lhchavez commented Mar 2, 2021 •

edited

Loading