On Wed, Feb 01, 2012 at 04:38:44PM +0200, Arnon Gilboa wrote:
> Alon Levy wrote:
> >On Wed, Feb 01, 2012 at 10:48:29AM +0200, Arnon Gilboa wrote:
> >>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?
> >
> >That wasn't the point. I wasn't talking about what goes wrong with the
> >computation, I was talking about what sequence of events leads to the
> >client sending monitors locations that are negatively offset to the origin.
> >
> >Anyway, if you already wrote it it wouldn't hurt to be in the commit
> >message I guess :)
> >
> Primary monitor is always located (in windows, what about linux?) in
> (0,0). When you position a secondary monitor above/to the left of
> the primary, it will have negative coordinate. This can be the case
> when guest display settings are changed either manually on the
> guest, or due to a message (auto-conf's VD_AGENT_MONITORS_CONFIG)
> from the client to the agent.
>
> Does it answer your question? :/
Yes, it does. Why the sour face?
>
> >>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@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
