gujjwal00 / avnc

VNC Client for Android
GNU General Public License v3.0
602 stars 57 forks source link

Two finger tap gesture unresponsive #180

Closed domportera closed 3 months ago

domportera commented 1 year ago

I recorded a video of the behavior. Sorry for the sound, forgot to mute my mic 😅

But basically two finger tapping doesn't seem to work reliably, at least i haven't figured out a way to do it reliably. I have it bound to bring up my keyboard, but I've tried it with other things like right click as well with the same effect. It'd be wonderful if you could look into this.

Thanks ! :)

Device info:

Android 13 Lg V60

gujjwal00 commented 1 year ago

Hi @domportera , thanks for taking the time to record the issue.

The root cause for this behavior is that AVNC is trying to differentiate between two-finger-tap and two-finger-pinch gestures. If the fingers move more than a certain threshold after touching the screen, it will be treated as the start of pinch/scroll gesture, and two-finger-tap detection will be cancelled.

I know this is not perfect, but two-finger-tap was a relatively newer addition, I tried to not affect other gestures. I am still looking to improve this situation.

PS: sorry for late response.

domportera commented 1 year ago

No worries! you are not beholden to my needs, you got a whole ass life to live

Yeah gestures are tricky and they all tend to affect each other - I've developed a multitouch library for Godot that has had to deal with similar issues when I was trying to make my own VNC application 😅 It's super challenging to start, let alone adding support for additional gestures after the fact. Maybe my repo-in-progress could provide some insights, though I doubt it is in any way comparable to what you have going on here. Where do you keep your code for gesture detection?

gujjwal00 commented 1 year ago

All incoming touch events are processed in TouchHandler, and gesture detection is here.

gujjwal00 commented 1 year ago

Your library is doing all event processing and gesture detection itself, but AVNC builds on top of GestureDetector.

domportera commented 1 year ago

I see, so yeah the logic most certainly wouldn't be transferable in any significant way. I'm hella busy lately but I'll see if I can take a look one day

shunf4 commented 3 months ago

A quick and dirty fix could be:

diff --git a/app/src/main/java/com/gaurav/avnc/ui/vnc/TouchHandler.kt b/app/src/main/java/com/gaurav/avnc/ui/vnc/TouchHandler.kt
index 8e45b87..caa1009 100644
--- a/app/src/main/java/com/gaurav/avnc/ui/vnc/TouchHandler.kt
+++ b/app/src/main/java/com/gaurav/avnc/ui/vnc/TouchHandler.kt
@@ -329,12 +329,15 @@ class TouchHandler(private val viewModel: VncViewModel, private val dispatcher:
         private var longPressDetected = false
         private var doubleTapDetected = false
         private var scrolling = false
+        private var cumulatedX = 0f
+        private var cumulatedY = 0f
         private var maxFingerDown = 0
         private var currentDownEvent: MotionEvent? = null

         private inner class InnerListener1 : SimpleOnGestureListener() {
             override fun onSingleTapConfirmed(e: MotionEvent): Boolean {
+//                Log.i("TouchHandler", "onSingleTapConfirmed")
                 listener.onSingleTapConfirmed(e)
                 return true
             }
@@ -379,6 +382,8 @@ class TouchHandler(private val viewModel: VncViewModel, private val dispatcher:
             }

             callOnScroll(e1, e2, -dx, -dy)
+            cumulatedX += dx
+            cumulatedY += dy
             return true
         }

@@ -417,7 +422,8 @@ class TouchHandler(private val viewModel: VncViewModel, private val dispatcher:
                             listener.onDoubleTapConfirmed(downEvent)

                         val gestureDuration = (e.eventTime - downEvent.eventTime)
-                        if (maxFingerDown > 1 && !scrolling && gestureDuration < ViewConfiguration.getDoubleTapTimeout())
+//                        Log.i("TouchHandler", "maxFingerDown " + maxFingerDown + " scrolling " + scrolling + " gestureDuration " + gestureDuration);
+                        if (maxFingerDown > 1 && (!scrolling || cumulatedX * cumulatedX + cumulatedY * cumulatedY < 900) && gestureDuration < ViewConfiguration.getDoubleTapTimeout())
                             listener.onMultiFingerTap(downEvent, maxFingerDown)
                     }

@@ -437,6 +443,8 @@ class TouchHandler(private val viewModel: VncViewModel, private val dispatcher:
             maxFingerDown = 0
             currentDownEvent?.recycle()
             currentDownEvent = null
+            cumulatedX = 0f
+            cumulatedY = 0f
         }
     }
gujjwal00 commented 3 months ago

Thanks @shunf4 , I have pushed a commit with these changes: https://github.com/gujjwal00/avnc/commit/645b6b73f0afc7bae53c114d5969874272a052a1

There are couple of points worth noting:

That said, lets see if this change improves user experience. @domportera can you please test this APK to see if things improve: https://github.com/gujjwal00/avnc/actions/runs/9352489686/artifacts/1563121815

domportera commented 3 months ago

Thanks @shunf4 , I have pushed a commit with these changes: https://github.com/gujjwal00/avnc/commit/645b6b73f0afc7bae53c114d5969874272a052a1

There are couple of points worth noting:

  • It is now possible to trigger Two-finger tap while zooming in some edge cases. This was the primary reason for not implementing this earlier.
  • The value 900 may need to be dynamic based on screen density

That said, lets see if this change improves user experience. @domportera can you please test this APK to see if things improve: https://github.com/gujjwal00/avnc/actions/runs/9352489686/artifacts/1563121815

Nice work!! I think this improves user experience for sure. It's not perfect as the zoom does trigger it sometimes, but i definitely prefer this version as it's less annoying for that to happen occasionally than to struggle to two-finger tap in my particular use case. For reference my DPI is 411/420 and i feel it's def a worthy tradeoff at its current state

gujjwal00 commented 3 months ago

v2.5 is now available.