Fix position for click and drag with EmulateMB

If you have the setting "Emulate middle mouse button" turned on, a click
and drag can fail if it is done very quickly. The position of the
initial click will be incorrect in such a case because the timeout will
delay events.

diff --git a/tests/unit/emulatemb.cxx b/tests/unit/emulatemb.cxx
index b04b647b31c7fd0c402ea773bf9cc4838db695aa..3d95beead11a2ac911f06b151f7df29eccbc2bf8 100644 (file)
--- a/tests/unit/emulatemb.cxx
+++ b/tests/unit/emulatemb.cxx
@@ -372,7 +372,7 @@ void testBothPressAfterRightTimeout()
   printf("OK\n");
 }
 
-void testTimeoutDuringDrag()
+void testTimeoutAndDrag()
 {
   TestClass test;
 
@@ -401,6 +401,62 @@ void testTimeoutDuringDrag()
   printf("OK\n");
 }
 
+void testDragAndTimeout()
+{
+  TestClass test;
+
+  printf("%s: ", __func__);
+
+  emulateMiddleButton.setParam(true);
+  test.filterPointerEvent(rfb::Point(10, 10), left);
+  test.filterPointerEvent(rfb::Point(30, 30), left);
+  usleep(100000); //0.1s
+  rfb::Timer::checkTimeouts();
+
+  ASSERT_EQ(test.results.size(), 3);
+
+  ASSERT_EQ(test.results[0].pos.x, 10);
+  ASSERT_EQ(test.results[0].pos.y, 10);
+  ASSERT_EQ(test.results[0].mask, empty);
+
+  ASSERT_EQ(test.results[1].pos.x, 10);
+  ASSERT_EQ(test.results[1].pos.y, 10);
+  ASSERT_EQ(test.results[1].mask, left);
+
+  ASSERT_EQ(test.results[2].pos.x, 30);
+  ASSERT_EQ(test.results[2].pos.y, 30);
+  ASSERT_EQ(test.results[2].mask, left);
+
+  printf("OK\n");
+}
+
+void testDragAndRelease()
+{
+  TestClass test;
+
+  printf("%s: ", __func__);
+
+  emulateMiddleButton.setParam(true);
+  test.filterPointerEvent(rfb::Point(10, 10), left);
+  test.filterPointerEvent(rfb::Point(20, 20), empty);
+
+  ASSERT_EQ(test.results.size(), 3);
+
+  ASSERT_EQ(test.results[0].pos.x, 10);
+  ASSERT_EQ(test.results[0].pos.y, 10);
+  ASSERT_EQ(test.results[0].mask, empty);
+
+  ASSERT_EQ(test.results[1].pos.x, 10);
+  ASSERT_EQ(test.results[1].pos.y, 10);
+  ASSERT_EQ(test.results[1].mask, left);
+
+  ASSERT_EQ(test.results[2].pos.x, 20);
+  ASSERT_EQ(test.results[2].pos.y, 20);
+  ASSERT_EQ(test.results[2].mask, empty);
+
+  printf("OK\n");
+}
+
 int main(int argc, char** argv)
 {
   testDisabledOption();
@@ -422,8 +478,10 @@ int main(int argc, char** argv)
   testBothPressAfterLeftTimeout();
   testBothPressAfterRightTimeout();
 
-  testTimeoutDuringDrag();
+  testTimeoutAndDrag();
 
+  testDragAndTimeout();
+  testDragAndRelease();
 
   return 0;
 }

diff --git a/vncviewer/EmulateMB.cxx b/vncviewer/EmulateMB.cxx
index 22332745d0364192c9de9abfc0355cb4ee99e141..3cd3fea617d4fb7632b11f8597eba56eb2dc5588 100644 (file)
--- a/vncviewer/EmulateMB.cxx
+++ b/vncviewer/EmulateMB.cxx
@@ -205,6 +205,7 @@ void EmulateMB::filterPointerEvent(const rfb::Point& pos, int buttonMask)
   int action1, action2;
   int lastState;
 
+  // Just pass through events if the emulate setting is disabled
   if (!emulateMiddleButton) {
      sendPointerEvent(pos, buttonMask);
      return;
@@ -225,16 +226,39 @@ void EmulateMB::filterPointerEvent(const rfb::Point& pos, int buttonMask)
     throw rfb::Exception(_("Invalid state for 3 button emulation"));
 
   action1 = stateTab[state][btstate][0];
-  if (action1 != 0)
-    sendAction(pos, buttonMask, action1);
+
+  if (action1 != 0) {
+    // Some presses are delayed, that means we have to check if that's
+    // the case and send the position corresponding to where the event
+    // first was initiated
+    if ((stateTab[state][4][2] >= 0) && action1 > 0)
+      // We have a timeout state and a button press (a delayed press),
+      // always use the original position when leaving a timeout state,
+      // whether the timeout was triggered or not
+      sendAction(origPos, buttonMask, action1);
+    else
+      // Normal non-delayed event
+      sendAction(pos, buttonMask, action1);
+  }
 
   action2 = stateTab[state][btstate][1];
-  if (action2 != 0)
-    sendAction(pos, buttonMask, action2);
 
-  if ((action1 == 0) && (action2 == 0)) {
-    buttonMask &= ~0x5;
-    buttonMask |= emulatedButtonMask;
+  // In our case with the state machine, action2 always occurs during a button
+  // release but if this change we need handle action2 accordingly
+  if (action2 != 0) {
+    if ((stateTab[state][4][2] >= 0) && action2 > 0)
+      sendAction(origPos, buttonMask, action2);
+    else
+      // Normal non-delayed event
+      sendAction(pos, buttonMask, action2);
+  }
+
+  // Still send a pointer move event even if there are no actions.
+  // However if the timer is running then we are supressing _all_
+  // events, even movement. The pointer's actual position will be
+  // sent once the timer fires or is abandoned.
+  if ((action1 == 0) && (action2 == 0) && !timer.isStarted()) {
+    buttonMask = createButtonMask(buttonMask);
     sendPointerEvent(pos, buttonMask);
   }
 
@@ -244,14 +268,19 @@ void EmulateMB::filterPointerEvent(const rfb::Point& pos, int buttonMask)
   if (lastState != state) {
     timer.stop();
 
-    if (stateTab[state][4][2] >= 0)
+    if (stateTab[state][4][2] >= 0) {
+      // We need to save the original position so that
+      // drags start from the correct position
+      origPos = pos;
       timer.start(50);
+    }
   }
 }
 
 bool EmulateMB::handleTimeout(rfb::Timer *t)
 {
   int action1, action2;
+  int buttonMask;
 
   if (&timer != t)
     return false;
@@ -259,15 +288,26 @@ bool EmulateMB::handleTimeout(rfb::Timer *t)
   if ((state > 10) || (state < 0))
     throw rfb::Exception(_("Invalid state for 3 button emulation"));
 
+  // Timeout shouldn't trigger when there's no timeout action
   assert(stateTab[state][4][2] >= 0);
 
   action1 = stateTab[state][4][0];
   if (action1 != 0)
-    sendAction(lastPos, lastButtonMask, action1);
+    sendAction(origPos, lastButtonMask, action1);
 
   action2 = stateTab[state][4][1];
   if (action2 != 0)
-    sendAction(lastPos, lastButtonMask, action2);
+    sendAction(origPos, lastButtonMask, action2);
+
+  buttonMask = lastButtonMask;
+
+  // Pointer move events are not sent when waiting for the timeout.
+  // However, we can't let the position get out of sync so when
+  // the pointer has moved we have to send the latest position here.
+  if (!origPos.equals(lastPos)) {
+    buttonMask = createButtonMask(buttonMask);
+    sendPointerEvent(lastPos, buttonMask);
+  }
 
   state = stateTab[state][4][2];
 
@@ -283,7 +323,15 @@ void EmulateMB::sendAction(const rfb::Point& pos, int buttonMask, int action)
   else
     emulatedButtonMask |= (1 << (action - 1));
 
-  buttonMask &= ~0x5;
-  buttonMask |= emulatedButtonMask;
+  buttonMask = createButtonMask(buttonMask);
   sendPointerEvent(pos, buttonMask);
 }
+
+int EmulateMB::createButtonMask(int buttonMask)
+{
+  // Unset left and right buttons in the mask
+  buttonMask &= ~0x5;
+
+  // Set the left and right buttons according to the action
+  return buttonMask |= emulatedButtonMask;
+}
\ No newline at end of file

diff --git a/vncviewer/EmulateMB.h b/vncviewer/EmulateMB.h
index e2a70d8e4a21838a0070595d08143637bb4bfb3a..132f44fe29dc12b3591575840d60bb38fe59ac00 100644 (file)
--- a/vncviewer/EmulateMB.h
+++ b/vncviewer/EmulateMB.h
@@ -36,11 +36,13 @@ protected:
 private:
   void sendAction(const rfb::Point& pos, int buttonMask, int action);
 
+  int createButtonMask(int buttonMask);
+
 private:
   int state;
   int emulatedButtonMask;
   int lastButtonMask;
-  rfb::Point lastPos;
+  rfb::Point lastPos, origPos;
   rfb::Timer timer;
 };