Re: [Spice-devel] [PATCH vdagent-win] vdagent: fix loss of mouse movement events
Hi, On Wed, May 30, 2018 at 08:04:21AM -0400, Frediano Ziglio wrote: > > Hi, > > > > On Fri, Feb 16, 2018 at 03:05:39PM +0300, free.user.name wrote: > > > 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 > > > > Would it be possible to use a real name? > > Unfortunately this e-mail was not replied. A signed off with a > dummy name and e-mail does not make sense. I would replace with > a "Patch sent my anonymous with a dummy Signed-off" or > something like that. I would just remove the signed-off but keep the author name. > The patch is solving a real problem, if you increase > VD_INPUT_INTERVAL_MS value (like 1000) is easy to see, move > events are kind of lost. Right, that could be a good addition to the commit log "An easy way to reproduce the problem is by ..." > > > 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. > > > > How easy is that to reproduce? I couldn't notice any lag but > > maybe it is just me and/or my environment. > > > > With the original interval value is hard to spot, see above. > Surely there must be a reason why was easy for the author to > reproduce without changing that value. > > > > > > > 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)); > > Maybe would be time to update these style? > > > > @@ -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
Re: [Spice-devel] [PATCH vdagent-win] vdagent: fix loss of mouse movement events
> > Hi, > > On Fri, Feb 16, 2018 at 03:05:39PM +0300, free.user.name wrote: > > 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 > > Would it be possible to use a real name? > Unfortunately this e-mail was not replied. A signed off with a dummy name and e-mail does not make sense. I would replace with a "Patch sent my anonymous with a dummy Signed-off" or something like that. The patch is solving a real problem, if you increase VD_INPUT_INTERVAL_MS value (like 1000) is easy to see, move events are kind of lost. > > --- > > 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. > > How easy is that to reproduce? I couldn't notice any lag but > maybe it is just me and/or my environment. > With the original interval value is hard to spot, see above. Surely there must be a reason why was easy for the author to reproduce without changing that value. > > > > 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)); Maybe would be time to update these style? > > @@ -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
Re: [Spice-devel] [PATCH vdagent-win] vdagent: fix loss of mouse movement events
Hi, On Fri, Feb 16, 2018 at 03:05:39PM +0300, free.user.name wrote: > 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.nameWould it be possible to use a real name? > --- > 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. How easy is that to reproduce? I couldn't notice any lag but maybe it is just me and/or my environment. > > 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(); Did not follow this change. > 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
Re: [Spice-devel] [PATCH vdagent-win] vdagent: fix loss of mouse movement events
On Fri, Feb 16, 2018 at 03:05:39PM +0300, free.user.name wrote: > 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. Ping. Is Windows vdagent still maintained? ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel