search

bk138 / multivnc

MultiVNC is a cross-platform Multicast-enabled VNC viewer based on LibVNCClient. The desktop version runs on Unix, Mac OS X and Windows. There also is an Android version.
GNU General Public License v3.0
447 stars 65 forks source link

Android: Handle middle mouse button #190

Closed gujjwal00 closed 2 years ago

gujjwal00 commented 2 years ago

Fixes #189

Middle button is sent to the server, but currently two middle-clicks are sent for single mouse button click. I am about 80% sure this is caused by way GenericMotion events are translated to normal Touch events and processed in processPointerEvent().

My question is why are these events handled this way, instead of directly invoking sendPointerEvent() like scroll events?

bk138 commented 2 years ago

Good question. I've got no good answer though since the code has come a long way and passed through several hands. Reminds me of #162 a bit...

gujjwal00 commented 2 years ago

You were right! Pointer button handling feels like a deep rabbit hole. I will sidestep other parts, and focus on mouse button for this PR.

Can we require API-23 for full mouse support? Dedicated button press & release actions in MotionEvent are available since 23.

bk138 commented 2 years ago

Can we require API-23 for full mouse support? Dedicated button press & release actions in MotionEvent are available since 23.

I guess so. If it's not too hard, you might want to add extra cases for SDK level 23 in the code, but if this gets too messy, let's bump the requiremt up to 23 (which is reasonably old IMO).

gujjwal00 commented 2 years ago

I guess so. If it's not too hard, you might want to add extra cases for SDK level 23 in the code, but if this gets too messy, let's bump the requiremt up to 23 (which is reasonably old IMO).

I was referring only to full mouse support. Existing logic (without middle button support) will continue to work on API <23. On API 23 and above, all mouse buttons will work correctly, assuming device manufacturers don't mess with events. We could probably make middle button work on API 21, but as you said, I doubt anyone will benefit from the effort.

This is ready for review/testing.

Summary: Instead of going through processPointerEvent() and messing with MotionEvent, different mouse actions are now directly handled in onGenericMotionEvent() and converted to corresponding VNC actions. These actions are then passed to a new method processMouseEvent(), which updates the button mask and sends the event to server.

bk138 commented 2 years ago

Thanks, will have a look ASAP!

bk138 commented 2 years ago

Finally found some time to revisit this again. I'll get a three-button Bluetooth mouse and test.

bk138 commented 2 years ago

@gujjwal00 Finally could test this and while the middle mouse button works well, dragging (at least w/ button 1) is now broken. Any quick idea from your side on why this may happen?

gujjwal00 commented 2 years ago

Fixed the drag issue. I missed the ACTION_MOVE handling in original patch.

One interesting thing to note here is that ACTION_MOVE events from mouse are reported via onTouchEvent() instead of onGenericMotionEvent().

bk138 commented 2 years ago

Fixed the drag issue. I missed the ACTION_MOVE handling in original patch.

One interesting thing to note here is that ACTION_MOVE events from mouse are reported via onTouchEvent() instead of onGenericMotionEvent().

Thanks so much! And yes, it really seems Android input handling is a deep rabbit hole :-/