Skip to content
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

Add native touch gestures support with UltraVNC #1908

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

EvilAngel00
Copy link

@EvilAngel00 EvilAngel00 commented Oct 24, 2024

This PR aims to start a discussion about alternative ways of managing gestures.

It is an implementation of a gesture handler that sends touches to an UltraVNC server (Windows only) via the GII pseudo-encoding. It allows for multi-touch, zooming, panning and scrolling while using the native Windows touch events instead of sending mouse events. At the moment, it can be activated via a checkbox in the noVNC settings before establishing a connection and during the handshake, the GII pseudo-encoding is added to the list of encodings sent to the server. Touches are then managed internally with an array that keeps track of the touch status (touchstart, touchmove or touchend/touchcancel) while sending GII messages every time there is a change. Touch events contain an identifier to determine which pointer is being moved, this identifier is given by the browser and is handled differently on Chromium based browsers (Edge, Chrome) and Firefox. Chromium browsers assign the smallest possible ID while Firefox keeps assigning incrementally i.e. if I place three fingers down one after the other, lift the second one and place it back I would have an array of IDs:

[0, 1, 2] ->[0, 2] -> [0, 2, 1] on Chromium
[0, 1, 2] -> [0, 2] -> [0, 2, 3] on Firefox

This caused issues when sending higher ID numbers to UltraVNC and we have to manage those IDs manually instead.

It could be interesting to have a framework in place to allow people to have their own implementation of touch events and gestures depending on the support from the VNC server being used. I do not intend to expand this at the moment, this is more of a starting point for the potential support for different types of gestures.

In any case, thank you for work on this library!

@CendioOssman
Copy link
Member

Thanks for your contribution! You haven't gone unnoticed. We just haven't had time to look at this PR yet.

CendioOssman and others added 10 commits November 20, 2024 10:46
It returns an object with details, not just a simple boolean.
Avoid duplicating this logic in multiple places.
So that we can use await at module top level.
Firefox is buggy and reports support for H.264 but then throws errors
once we actually try to decode things. Detect this early by doing a
quick test decode of a single frame.
These are not supposed to happen according to the specification, but
Firefox has some bug and throws them anyway.
The Firefox H.264 decoder on Windows might simply just refuse to deliver
any finished frames. It also doesn't deliver any errors.

Detect this early by expecting a frame after flush() has completed.
Try to be more consistent in how we capitalize things. Both the "Title
Case" and "Sentence case" styles are popular, so either would work.
Google and Mozilla both prefer "Sentence case", so let's follow them.
This would have resolved itself automatically on the next translation
update, but this commit will reduce unnecessary noise in that change.
Copy link
Member

@CendioOssman CendioOssman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not had time to look at this in detail yet, but a casual look shows that everything is labeled "ultravnc" even though it implements the GII extension.

Could you go over things and try to follow a naming that matches whats documented in rfbproto.

There were also a bunch of magical constants. We should create constants for these so things are more readable, just like we have for other protocol stuff at the top of rfb.js.

@EvilAngel00
Copy link
Author

Hi Pierre, thanks for looking into the PR!

The reason why everything is labeled like that is because this GII implementation is specific to UltraVNC. I added the GII pseudo-encoding to the list of sent encodings when the option is set but the rest (setting up the GII device, sending the touches, ...) are based on what UltraVNC does with their own viewer.
What I would suggest then, instead of having a checkbox in the UI, is to have a dropdown with the choice to use the noVNC implementation of gestures or this UltraVNC specific one. This option would then be passed to the RFB object and the constructor would select the appropriate gesture handler. Like that, it can then be extended as people could just add their own implementation of gestures depending on their needs. What do you think?

I will also solve the merge conflicts and set the constants as you suggested.

@EvilAngel00
Copy link
Author

I made the changes but rebasing the main branch into mine didn't work very well with the PR 😖

I can make a new PR for this if you'd prefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants