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

[Arnon Gilboa]

Thanks Alon, adding these lines to the comment:

In VDAgent::handle_mouse_event(), mouse event is generated by scaling 
the received (x,y, display_id) to normalized absolute coordinates 
(0..0xffff) on the entire virtual desktop which contains all the 
displays. Keeping negative display coordinates, as received from the 
client, was mistakenly handled and generated wrong (sometimes even 
negative) coordinates, see:
_input.mi.dx = (mode->get_pos_x() + _mouse_x) * 0xffff / 
_desktop_layout->get_total_width();
_input.mi.dy = (mode->get_pos_y() + _mouse_y) * 0xffff / 
_desktop_layout->get_total_height();

Is it clear now?

Alon Levy wrote:
> 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
>>