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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
