subject:"Re\: \[Spice\-devel\] \[PATCH vdagent\-win\] vdagent\: fix loss of mouse movement events"

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

2018-05-30 Thread Victor Toso

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

2018-05-30 Thread Frediano Ziglio

> 
> 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

2018-03-05 Thread Victor Toso

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?

> ---
> 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

2018-02-23 Thread free.user.name

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

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

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

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

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

4 matches

Site Navigation

Mail list logo

Footer information