From 9197db186b65e1da2d59c60e103f7bf92353ab27 Mon Sep 17 00:00:00 2001 From: Alex Tanskanen Date: Wed, 22 Apr 2020 12:15:06 +0200 Subject: [PATCH] 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. --- tests/unit/emulatemb.cxx | 62 ++++++++++++++++++++++++++++++++-- vncviewer/EmulateMB.cxx | 72 +++++++++++++++++++++++++++++++++------- vncviewer/EmulateMB.h | 4 ++- 3 files changed, 123 insertions(+), 15 deletions(-) diff --git a/tests/unit/emulatemb.cxx b/tests/unit/emulatemb.cxx index b04b647b..3d95beea 100644 --- 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 22332745..3cd3fea6 100644 --- 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 e2a70d8e..132f44fe 100644 --- 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; }; -- 2.39.5