[Spice-devel] [PATCH vdagent-win] vdagent: fix cursor position sync on multimon rhbz#757819

[Alon Levy]

On Mon, Jan 23, 2012 at 12:59:22PM +0200, Arnon Gilboa wrote:
> On any change of the display settings, driven by client message or guest user,
> normalize all display positions to non-negative coordinates, and update total
> width and height of the virtual desktop.
> 
> The bug was due to wrong handling of (non-primary) displays positioned on
> negative coordinates.

Looks good, but can you give a better example of how something goes
wrong in the patch comment? 

ACK

> ---
>  vdagent/desktop_layout.cpp |   52 +++++++++++++++++++++++++++----------------
>  vdagent/desktop_layout.h   |    1 +
>  2 files changed, 34 insertions(+), 19 deletions(-)
> 
> diff --git a/vdagent/desktop_layout.cpp b/vdagent/desktop_layout.cpp
> index 0eada52..0ba7248 100644
> --- a/vdagent/desktop_layout.cpp
> +++ b/vdagent/desktop_layout.cpp
> @@ -44,10 +44,6 @@ void DesktopLayout::get_displays()
>      DEVMODE mode;
>      DWORD display_id;
>      DWORD dev_id = 0;
> -    LONG min_x = 0;
> -    LONG min_y = 0;
> -    LONG max_x = 0;
> -    LONG max_y = 0;
>      bool attached;
>  
>      lock();
> @@ -81,22 +77,10 @@ void DesktopLayout::get_displays()
>          attached = !!(dev_info.StateFlags & DISPLAY_DEVICE_ATTACHED_TO_DESKTOP);
>          EnumDisplaySettings(dev_info.DeviceName, ENUM_CURRENT_SETTINGS, &mode);
>          _displays[display_id] = new DisplayMode(mode.dmPosition.x, mode.dmPosition.y,
> -                                            mode.dmPelsWidth, mode.dmPelsHeight,
> -                                            mode.dmBitsPerPel, attached);
> -        if (attached) {
> -            min_x = min(min_x, mode.dmPosition.x);
> -            min_y = min(min_y, mode.dmPosition.y);
> -            max_x = max(max_x, mode.dmPosition.x + (LONG)mode.dmPelsWidth);
> -            max_y = max(max_y, mode.dmPosition.y + (LONG)mode.dmPelsHeight);
> -        }
> -    }
> -    if (min_x || min_y) {
> -        for (Displays::iterator iter = _displays.begin(); iter != _displays.end(); iter++) {
> -            (*iter)->move_pos(-min_x, -min_y);
> -        }
> +                                                mode.dmPelsWidth, mode.dmPelsHeight,
> +                                                mode.dmBitsPerPel, attached);
>      }
> -    _total_width = max_x - min_x;
> -    _total_height = max_y - min_y;
> +    normalize_displays_pos();
>      unlock();
>  }
>  
> @@ -147,10 +131,40 @@ void DesktopLayout::set_displays()
>      }
>      if (dev_sets) {
>          ChangeDisplaySettingsEx(NULL, NULL, NULL, 0, NULL);
> +        normalize_displays_pos();
>      }
>      unlock();
>  }
>  
> +// Normalize all display positions to non-negative coordinates and update total width and height of
> +// the virtual desktop. Caller is responsible to lock() & unlock().
> +void DesktopLayout::normalize_displays_pos()
> +{
> +    Displays::iterator iter;
> +    DisplayMode* mode;
> +    LONG min_x = 0;
> +    LONG min_y = 0;
> +    LONG max_x = 0;
> +    LONG max_y = 0;
> +
> +    for (iter = _displays.begin(); iter != _displays.end(); iter++) {
> +        mode = *iter;
> +        if (mode->_attached) {
> +            min_x = min(min_x, mode->_pos_x);
> +            min_y = min(min_y, mode->_pos_y);
> +            max_x = max(max_x, mode->_pos_x + (LONG)mode->_width);
> +            max_y = max(max_y, mode->_pos_y + (LONG)mode->_height);
> +        }
> +    }
> +    if (min_x || min_y) {
> +        for (iter = _displays.begin(); iter != _displays.end(); iter++) {
> +            (*iter)->move_pos(-min_x, -min_y);
> +        }
> +    }
> +    _total_width = max_x - min_x;
> +    _total_height = max_y - min_y;
> +}
> +
>  bool DesktopLayout::consistent_displays()
>  {
>      DISPLAY_DEVICE dev_info;
> diff --git a/vdagent/desktop_layout.h b/vdagent/desktop_layout.h
> index caab84f..c43ff03 100644
> --- a/vdagent/desktop_layout.h
> +++ b/vdagent/desktop_layout.h
> @@ -73,6 +73,7 @@ public:
>  
>  private:
>      void clean_displays();
> +    void normalize_displays_pos();
>      static bool consistent_displays();
>      static bool is_attached(LPCTSTR dev_name);
>      static bool get_qxl_device_id(WCHAR* device_key, DWORD* device_id);
> -- 
> 1.7.4.1
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel