[PATCH vdagent-win] vdagent: fix loss of mouse movement events

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



send_input() may not be immediately called from handle_mouse_event() on
movement. INPUT structure is generated and stored and a timer may be set
instead. If subsequent call to handle_mouse_event() occurs before timer
expires, prepared INPUT structure gets overwritten and MOUSEEVENTF_MOVE
bit is lost. Windows doesn't see updated mouse position as the result.

Make handle_mouse_event() merely store the new mouse state, and move
INPUT structure generation to send_input(). Shuffle new mouse state to
previous only after mouse events are submitted to SendInput() Windows
API function.

Signed-off-by: free.user.name <free.user.name@xxxxx>
---
This fixes a very annoying mouse input bug in Windows 7/10 guests for
me. Whenever vdagent-win is handling mouse input, mouse pointer position
seen by the guest sometimes lags behind the cursor drawn on screen,
which makes it impossible to reliably click on anything.

 vdagent/vdagent.cpp | 132 +++++++++++++++++++++++++---------------------------
 1 file changed, 63 insertions(+), 69 deletions(-)

diff --git a/vdagent/vdagent.cpp b/vdagent/vdagent.cpp
index f00fbf5..89b3c6a 100644
--- a/vdagent/vdagent.cpp
+++ b/vdagent/vdagent.cpp
@@ -89,8 +89,7 @@ private:
     void on_clipboard_grab();
     void on_clipboard_request(UINT format);
     void on_clipboard_release();
-    DWORD get_buttons_change(DWORD last_buttons_state, DWORD new_buttons_state,
-                             DWORD mask, DWORD down_flag, DWORD up_flag);
+    DWORD get_buttons_change(DWORD mask, DWORD down_flag, DWORD up_flag);
     static HGLOBAL utf8_alloc(LPCSTR data, int size);
     static LRESULT CALLBACK wnd_proc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam);
     static DWORD WINAPI event_thread_proc(LPVOID param);
@@ -131,10 +130,8 @@ private:
     int _system_version;
     int _clipboard_owner;
     DWORD _clipboard_tick;
-    DWORD _buttons_state;
-    ULONG _mouse_x;
-    ULONG _mouse_y;
-    INPUT _input;
+    VDAgentMouseState _new_mouse;
+    VDAgentMouseState _last_mouse;
     DWORD _input_time;
     HANDLE _control_event;
     HANDLE _stop_event;
@@ -191,9 +188,6 @@ VDAgent::VDAgent()
     , _remove_clipboard_listener (NULL)
     , _clipboard_owner (owner_none)
     , _clipboard_tick (0)
-    , _buttons_state (0)
-    , _mouse_x (0)
-    , _mouse_y (0)
     , _input_time (0)
     , _control_event (NULL)
     , _stop_event (NULL)
@@ -221,7 +215,8 @@ VDAgent::VDAgent()
         swprintf_s(log_path, MAX_PATH, VD_AGENT_LOG_PATH, temp_path);
         _log = VDLog::get(log_path);
     }
-    ZeroMemory(&_input, sizeof(_input));
+    ZeroMemory(&_new_mouse, sizeof(_new_mouse));
+    ZeroMemory(&_last_mouse, sizeof(_last_mouse));
     ZeroMemory(&_read_overlapped, sizeof(_read_overlapped));
     ZeroMemory(&_write_overlapped, sizeof(_write_overlapped));
     ZeroMemory(_read_buf, sizeof(_read_buf));
@@ -522,13 +517,12 @@ void VDAgent::event_dispatcher(DWORD timeout, DWORD wake_mask)
     }
 }
 
-DWORD VDAgent::get_buttons_change(DWORD last_buttons_state, DWORD new_buttons_state,
-                                  DWORD mask, DWORD down_flag, DWORD up_flag)
+DWORD VDAgent::get_buttons_change(DWORD mask, DWORD down_flag, DWORD up_flag)
 {
     DWORD ret = 0;
-    if (!(last_buttons_state & mask) && (new_buttons_state & mask)) {
+    if (!(_last_mouse.buttons & mask) && (_new_mouse.buttons & mask)) {
         ret = down_flag;
-    } else if ((last_buttons_state & mask) && !(new_buttons_state & mask)) {
+    } else if ((_last_mouse.buttons & mask) && !(_new_mouse.buttons & mask)) {
         ret = up_flag;
     }
     return ret;
@@ -536,101 +530,101 @@ DWORD VDAgent::get_buttons_change(DWORD last_buttons_state, DWORD new_buttons_st
 
 bool VDAgent::send_input()
 {
+    DisplayMode* mode = NULL;
+    DWORD mouse_move = 0;
+    DWORD buttons_change = 0;
+    DWORD mouse_wheel = 0;
     bool ret = true;
-    _desktop_layout->lock();
+    INPUT input;
+
     if (_pending_input) {
         if (KillTimer(_hwnd, VD_TIMER_ID)) {
             _pending_input = false;
         } else {
             vd_printf("KillTimer failed: %lu", GetLastError());
             _running = false;
-            _desktop_layout->unlock();
             return false;
         }
     }
-    if (!SendInput(1, &_input, sizeof(INPUT))) {
-        DWORD err = GetLastError();
-        // Don't stop agent due to UIPI blocking, which is usually only for specific windows
-        // of system security applications (anti-viruses etc.)
-        if (err != ERROR_SUCCESS && err != ERROR_ACCESS_DENIED) {
-            vd_printf("SendInput failed: %lu", err);
-            ret = _running = false;
-        }
-    }
-    _input_time = GetTickCount();
-    _desktop_layout->unlock();
-    return ret;
-}
-
-bool VDAgent::handle_mouse_event(VDAgentMouseState* state)
-{
-    DisplayMode* mode = NULL;
-    DWORD mouse_move = 0;
-    DWORD buttons_change = 0;
-    DWORD mouse_wheel = 0;
-    bool ret = true;
 
     ASSERT(_desktop_layout);
     _desktop_layout->lock();
-    if (state->display_id < _desktop_layout->get_display_count()) {
-        mode = _desktop_layout->get_display(state->display_id);
+    if (_new_mouse.display_id < _desktop_layout->get_display_count()) {
+        mode = _desktop_layout->get_display(_new_mouse.display_id);
     }
     if (!mode || !mode->get_attached()) {
         _desktop_layout->unlock();
         return true;
     }
-    ZeroMemory(&_input, sizeof(INPUT));
-    _input.type = INPUT_MOUSE;
-    if (state->x != _mouse_x || state->y != _mouse_y) {
+    ZeroMemory(&input, sizeof(INPUT));
+    input.type = INPUT_MOUSE;
+    if (_new_mouse.x != _last_mouse.x || _new_mouse.y != _last_mouse.y) {
         DWORD w = _desktop_layout->get_total_width();
         DWORD h = _desktop_layout->get_total_height();
         w = (w > 1) ? w-1 : 1; /* coordinates are 0..w-1, protect w==0 */
         h = (h > 1) ? h-1 : 1; /* coordinates are 0..h-1, protect h==0 */
-        _mouse_x = state->x;
-        _mouse_y = state->y;
         mouse_move = MOUSEEVENTF_MOVE;
-        _input.mi.dx = (mode->get_pos_x() + _mouse_x) * 0xffff / w;
-        _input.mi.dy = (mode->get_pos_y() + _mouse_y) * 0xffff / h;
+        input.mi.dx = (mode->get_pos_x() + _new_mouse.x) * 0xffff / w;
+        input.mi.dy = (mode->get_pos_y() + _new_mouse.y) * 0xffff / h;
     }
-    if (state->buttons != _buttons_state) {
-        buttons_change = get_buttons_change(_buttons_state, state->buttons, VD_AGENT_LBUTTON_MASK,
+    if (_new_mouse.buttons != _last_mouse.buttons) {
+        buttons_change = get_buttons_change(VD_AGENT_LBUTTON_MASK,
                                             MOUSEEVENTF_LEFTDOWN, MOUSEEVENTF_LEFTUP) |
-                         get_buttons_change(_buttons_state, state->buttons, VD_AGENT_MBUTTON_MASK,
+                         get_buttons_change(VD_AGENT_MBUTTON_MASK,
                                             MOUSEEVENTF_MIDDLEDOWN, MOUSEEVENTF_MIDDLEUP) |
-                         get_buttons_change(_buttons_state, state->buttons, VD_AGENT_RBUTTON_MASK,
+                         get_buttons_change(VD_AGENT_RBUTTON_MASK,
                                             MOUSEEVENTF_RIGHTDOWN, MOUSEEVENTF_RIGHTUP);
-        mouse_wheel = get_buttons_change(_buttons_state, state->buttons,
-                                         VD_AGENT_UBUTTON_MASK | VD_AGENT_DBUTTON_MASK,
+        mouse_wheel = get_buttons_change(VD_AGENT_UBUTTON_MASK | VD_AGENT_DBUTTON_MASK,
                                          MOUSEEVENTF_WHEEL, 0);
         if (mouse_wheel) {
-            if (state->buttons & VD_AGENT_UBUTTON_MASK) {
-                _input.mi.mouseData = WHEEL_DELTA;
-            } else if (state->buttons & VD_AGENT_DBUTTON_MASK) {
-                _input.mi.mouseData = (DWORD)(-WHEEL_DELTA);
+            if (_new_mouse.buttons & VD_AGENT_UBUTTON_MASK) {
+                input.mi.mouseData = WHEEL_DELTA;
+            } else if (_new_mouse.buttons & VD_AGENT_DBUTTON_MASK) {
+                input.mi.mouseData = (DWORD)(-WHEEL_DELTA);
             }
         }
-        _buttons_state = state->buttons;
     }
 
-    _input.mi.dwFlags = MOUSEEVENTF_ABSOLUTE | MOUSEEVENTF_VIRTUALDESK | mouse_move |
-                        mouse_wheel | buttons_change;
+    input.mi.dwFlags = MOUSEEVENTF_ABSOLUTE | MOUSEEVENTF_VIRTUALDESK | mouse_move |
+                       mouse_wheel | buttons_change;
 
-    if ((mouse_move && GetTickCount() - _input_time > VD_INPUT_INTERVAL_MS) || buttons_change ||
-                                                                                     mouse_wheel) {
-        ret = send_input();
-    } else if (!_pending_input) {
-        if (SetTimer(_hwnd, VD_TIMER_ID, VD_INPUT_INTERVAL_MS, NULL)) {
-            _pending_input = true;
-        } else {
-            vd_printf("SetTimer failed: %lu", GetLastError());
-            _running = false;
-            ret = false;
+    if (!SendInput(1, &input, sizeof(INPUT))) {
+        DWORD err = GetLastError();
+        // Don't stop agent due to UIPI blocking, which is usually only for specific windows
+        // of system security applications (anti-viruses etc.)
+        if (err != ERROR_SUCCESS && err != ERROR_ACCESS_DENIED) {
+            vd_printf("SendInput failed: %lu", err);
+            ret = _running = false;
         }
+    } else {
+        _last_mouse = _new_mouse;
     }
+    _input_time = GetTickCount();
     _desktop_layout->unlock();
     return ret;
 }
 
+bool VDAgent::handle_mouse_event(VDAgentMouseState* state)
+{
+    _new_mouse = *state;
+    if (_new_mouse.buttons != _last_mouse.buttons) {
+        return send_input();
+    } else if (_new_mouse.x != _last_mouse.x || _new_mouse.y != _last_mouse.y) {
+        if (GetTickCount() - _input_time > VD_INPUT_INTERVAL_MS) {
+            return send_input();
+        } else if (!_pending_input) {
+            if (SetTimer(_hwnd, VD_TIMER_ID, VD_INPUT_INTERVAL_MS, NULL)) {
+                _pending_input = true;
+            } else {
+                vd_printf("SetTimer failed: %lu", GetLastError());
+                _running = false;
+                return false;
+            }
+        }
+    }
+    return true;
+}
+
 bool VDAgent::handle_mon_config(VDAgentMonitorsConfig* mon_config, uint32_t port)
 {
     VDIChunk* reply_chunk;
-- 
2.15.1

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]