Remote cursor support (tested against UltraVNC and TightVNC so far)#1964
Remote cursor support (tested against UltraVNC and TightVNC so far)#1964jkanefendt wants to merge 2 commits intonovnc:masterfrom
Conversation
CendioOssman
left a comment
CendioOssman
left a comment
There was a problem hiding this comment.
Thanks for your submission!
For new extensions, the first thing is that we want to make sure that it is documented here:
https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst
Which it unfortunately isn't yet. So please, try to write down what the behaviour you've found in TightVNC and UltraVNC and we can start with getting the documentation done.
| const x = this._FBU.x; | ||
| const y = this._FBU.y; | ||
|
|
||
| this._cursor.moveRemote(x, y, this._display.scale); |
There was a problem hiding this comment.
Are you sure that this is what this extension means? Not that the client is expected to really move the actual position of the cursor, e.g. as used in games?
There was a problem hiding this comment.
It's up to the client of how to deal with remote pointer events. The suggested implementation is drawing the remote cursor either if it's in view-only mode or if the local cursor is outside the canvas, otherwise such events are ignored.
There was a problem hiding this comment.
The server needs to know what to expect, though. So the behaviour needs to be specified.
The UltraVNC code you linked treats it as just a display thing, which matches your PR here.
I also had a look at the original TightVNC code. It mostly does the same thing, with the exception of the X11 client. In there, it actually does XWarpCursor().
But since that isn't consistent, the server can't rely on it. So I'm willing to treat that as an odd exception and we can go with this just being for rendering.
As for when this needs to be done, it should be whenever needed to give the user the correct feedback. So it should follow the same logic the server uses without this extension.
In TigerVNC (which is based on the original RealVNC code), this is whenever the local cursor position and server cursor position differ, and some time has passed (to deal with various latencies). The TigerVNC code is rather crude, but in practice the time should be 500 ms on average.
That logic should hopefully handle view-only implicitly, as there will never be a valid local cursor position in that case.
| `showRemoteCursor` | ||
| - Is a `boolean` indicating whether the remote cursor position should | ||
| be tracked. The server must support the respective pseudo encoding. | ||
|
|
There was a problem hiding this comment.
If this is truly just a rendering optimization, then we don't need a setting. Just have it enabled.
…ncoding constants.
The pointer position encoding does not seem to be officially documented. It may be reconstructed from the UltraVNC viewer implementation: The remote pointer position is represented in the x and y coordinates of the framebuffer update message, there is no further payload. |
|
We'd like the documentation in place before we merge this. So please see if you could have a look at writing a draft for that. |
|
@RudiDeVos: Have you seen this noVNC PR? |
@CendioOssman You mean the protocol documentation of the pointer position pseudo encoding or a description of how the implementation actually deals with this information? |
|
I'd say those are the same thing. The protocol information should include both how things are encoded, and the semantics around how it should be used. |
3 participants
Adds support for decoding an displaying the remote cursor position.