Add support for forward and back mouse buttons

[CendioHalim]

This is a continuation of the work initially done by manny33 in #1711:

• Added the abillity for vnc to use 9 mouse buttons instead of 7. #1711

This PR intends to add support for forward and back mouse buttons by extending RFB with the pseudo-encoding ExtendedMouseButtons, which is IANA registered and has an open draft PR on rfbproto:

• Document ExtendedMouseButtons pseudo-encoding rfbproto/rfbproto#57

In its current state, this PR has a working implementation that works on Windows, Mac and Linux (with some issues that are being worked on).

I'm quite content with the RFB implementation on the VNC side, but the client-side implementation in Viewport.cxx is currently being re-written quite extensively so that all mouse events are handled with native code instead of using a combination of FLTK and native code.

This is still a work in progress, and commits will be cleaned up / rebased.

[Neustradamus]

To follow this new PR :)

[CendioHalim]

I've now updated Viewport.cxx so that all* mouse handling is done using native platform code: 30e4e9e

(*) There is still a known bug where FL_ENTER and FL_LEAVE in Viewport::handle() are still reached. I think that these should be removed completely, as they are pointer events that should be handled entirely in handleSystemPointerEvent().

[CendioHalim]

Seems like the close button is not respected on macOS (minimize/maximize works), will have to do some digging to find out why that is.

[CendioHalim]

I've fixed the issued and updated my branch. Most of the work was done in cfe40cc, which has a very hacky solution to fix resizing on macOS.

[CendioHalim]

There appears to still be some issues:

When I start vncviewer on Linux, the cursor is invisible until I enter and leave the window once.

On Windows, the window buttons (minimize, maximize, close) don't work (we should return 0 instead of consuming the events). Also, the viewer doesn't detect WM_MOUSELEAVE / mouse-enter unless I lose focus and get window focus again.

On macOS, the resizing is still a bit buggy and can leave the window in a broken state where the viewer can't be resized / loses input.

[CendioHalim]

There is also a bug on all platforms where pressing the forward button while moving the mouse will trigger a push/release/push instantly. Haven't looked into why this happens, but I've seen it on all platforms with three different mice.

[CendioHalim]

On Windows, the window buttons (minimize, maximize, close) don't work (we should return 0 instead of consuming the events). Also, the viewer doesn't detect WM_MOUSELEAVE / mouse-enter unless I lose focus and get window focus again.

Fixed in 2f8fe06 and f82a074.

[CendioHalim]

I've opened a PR to FLTK to fix forward/back mouse button support for macOS and Windows:

• Add support for forward and back mouse buttons on macOS and Windows fltk/fltk#1068

With this fix in FLTK, the TigerVNC code can be immensely simplified. I've updated my branch to reflect the changes done in the PR.

[CendioOssman]

Looks really nice! Great work!

There are some minor tweaks, but we are pretty much done.

Please also organise the commit history to something more final.

[CendioHalim]

I've addressed most of the comments and rebased/cleaned up my commits.

[CendioOssman]

For the commit message; full URLs are preferred to references that only work in GitHub. Or perhaps omit that reference to the old PR and just like it here in the new PR?

Same with using GitHub usernames. If it's a commit message, then a name (and possibly email) is best. Just like for git's author/committer.

[CendioOssman]

Indentation is off here.

[CendioHalim]

Whoops, looks like the file uses tabs and not spaces.

[CendioOssman]

Does this implicitly also make unknown bits ignored? Or are they just unlabeled and perhaps wrong?

[CendioHalim]

We've explicitly labelled and initialized 9 buttons. We then loop through the mask, bits set in the higher range are ignored:

void vncPointerButtonAction(int buttonMask)
{
	int i;
	ValuatorMask mask;

	for (i = 0; i < BUTTONS; i++) {
		if ((buttonMask ^ oldButtonMask) & (1 << i)) {
			int action = (buttonMask & (1<<i)) ?
				     ButtonPress : ButtonRelease;
			valuator_mask_set_range(&mask, 0, 0, NULL);
			QueuePointerEvents(vncPointerDev, action, i + 1,
					   POINTER_RELATIVE, &mask);
		}
	}

	oldButtonMask = buttonMask;
}

[CendioOssman]

Almost complete. x0vncserver doesn't work correctly for some reason. Only the back button works, not the forward button.

[CendioHalim]

Almost complete. x0vncserver doesn't work correctly for some reason. Only the back button works, not the forward button.

1c2f07f fixes this, but this would have to be updated if we want to add support for more buttons in the future.

[CendioOssman]

Seems to work correctly now. Good work!

But I think a tweak is needed on which change is in which commit.

[CendioOssman]

Looks good!

When can we expect fltk/fltk#1083 to be merged? I don't want to risk the API changing.

And @bphinz, I assume you can easily add that PR to your FLTK build?

[CendioHalim]

When can we expect fltk/fltk#1083 to be merged? I don't want to risk the API changing.

The FLTK fix has already been merged into master: fltk/fltk#1081 and should be part of the upcoming 1.4.0 API.

Regarding the backport to 1.3 , it looks like it might be a while: fltk/fltk#1083 (comment)

[CendioOssman]

We are targeting FLTK 1.3, not 1.4, so we need to have some confirmation that we can start relying on this API. Otherwise we might end up in a mess where there are several version with subtle differences that we somehow need to handle.

[Albrecht-S]

We are targeting FLTK 1.3, not 1.4, so we need to have some confirmation that we can start relying on this API. Otherwise we might end up in a mess where there are several version with subtle differences that we somehow need to handle.

Sorry, our release process for 1.4.0 will take a while, depending on the number of RC's we need to publish. RC2 will certainly follow because we do already have one essential fix. Each RC is scheduled one week after the previous one and the final release will typically be two weeks after the last RC, i.e. the final release will be published if we don't have any issues for two weeks.

I can't backport the changes to 1.3.10 (i.e. merge the existing PR) before 1.4.0 has been released which will not be before about 3 weeks from now (or later if more RC's are required, see above).

[CendioOssman]

We don't have to wait for a 1.3.10 release. We can apply a patch to our builds until then. But we need to know that you are happy with the API, and we won't have to change things once 1.3.10 is out.

[Albrecht-S]

we need to know that you are happy with the API,

I am happy with it, as it stands now, but ...

and we won't have to change things once 1.3.10 is out.

As you certainly know the future is hard to predict. Therefore I can't give any guarantees that the API doesn't have to be changed later, but I'm pretty confident that this won't happen.

Once 1.4.0 is finally released chances are even better that the API won't be changed in incompatible ways and I'll begin to verify and merge the open PR to 1.3.10. That's all I can say for sure. Sorry.