[Qemu-devel] [PATCH 0/4] ui/cocoa: Fix absolute positioning and other bugs

[Qemu-devel] [PATCH 0/4] ui/cocoa: Fix absolute positioning and other bugs

[Peter Maydell]

This set of cocoa UI patches:
 * fixes the completely broken handling of absolute positioning
   (tablet-style) input devices
 * fixes a bug where if the first surface created was the same 640x480
   as the initial window we'd never actually draw it
 * implements support for the -show-cursor command line option

The GTK and SDL UI frontends don't seem to be consistent about how they
handle mousegrab for absolute-position devices; I followed SDL on the
basis that it was the older and more established UI. (GTK doesn't
implement -show-cursor at all, incidentally.)

Peter Maydell (4):
  ui/cocoa: Cope with first surface being same as initial window size
  ui/cocoa: Add utility method to check if point is within window
  ui/cocoa: Fix handling of absolute positioning devices
  ui/cocoa: Honour -show-cursor command line option

 ui/cocoa.m | 108 ++++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 75 insertions(+), 33 deletions(-)

-- 
1.8.5.4

[Qemu-devel] [PATCH 1/4] ui/cocoa: Cope with first surface being same as initial window size

[Peter Maydell]

Do the recalculation of the content dimensions in switchSurface if the
current cdx is zero as well as if the new surface is a different size to
the current window. This catches the case where the first surface registered
happens to be 640x480 (our current window size), and fixes a bug where we
would always display a black screen until the first surface of a different
size was registered.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 ui/cocoa.m | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index f20fd1f..a270a46 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -404,7 +404,11 @@ QemuCocoaView *cocoaView;
 
     int w = surface_width(surface);
     int h = surface_height(surface);
-    bool isResize = (w != screen.width || h != screen.height);
+    /* cdx == 0 means this is our very first surface, in which case we need
+     * to recalculate the content dimensions even if it happens to be the size
+     * of the initial empty window.
+     */
+    bool isResize = (w != screen.width || h != screen.height || cdx == 0.0);
 
     int oldh = screen.height;
     if (isResize) {
-- 
1.8.5.4

[Qemu-devel] [PATCH 2/4] ui/cocoa: Add utility method to check if point is within window

[Peter Maydell]

Add a utility method to check whether a point is within the current window
bounds, and use it in the various places in the mouse handling code that
were opencoding the check.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 ui/cocoa.m | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index a270a46..1a626e2 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -305,6 +305,11 @@ QemuCocoaView *cocoaView;
     return YES;
 }
 
+- (BOOL) screenContainsPoint:(NSPoint) p
+{
+    return (p.x > -1 && p.x < screen.width && p.y > -1 && p.y < screen.height);
+}
+
 - (void) drawRect:(NSRect) rect
 {
     COCOA_DEBUG("QemuCocoaView: drawRect\n");
@@ -607,7 +612,7 @@ QemuCocoaView *cocoaView;
             break;
         case NSMouseMoved:
             if (isAbsoluteEnabled) {
-                if (p.x < 0 || p.x > screen.width || p.y < 0 || p.y > screen.height || ![[self window] isKeyWindow]) {
+                if (![self screenContainsPoint:p] || ![[self window] isKeyWindow]) {
                     if (isTabletEnabled) { // if we leave the window, deactivate the tablet
                         [NSCursor unhide];
                         isTabletEnabled = FALSE;
@@ -657,7 +662,7 @@ QemuCocoaView *cocoaView;
             if (isTabletEnabled) {
                     mouse_event = true;
             } else if (!isMouseGrabbed) {
-                if (p.x > -1 && p.x < screen.width && p.y > -1 && p.y < screen.height) {
+                if ([self screenContainsPoint:p]) {
                     [self grabMouse];
                 } else {
                     [NSApp sendEvent:event];
-- 
1.8.5.4

[Qemu-devel] [PATCH 3/4] ui/cocoa: Fix handling of absolute positioning devices

[Peter Maydell]

Fix handling of absolute positioning devices, which were basically
unusable for two separate reasons:
 (1) as soon as you pressed the left mouse button we would call
     CGAssociateMouseAndMouseCursorPosition(FALSE), which means that
     the absolute coordinates of the mouse events are never updated
 (2) we didn't account for MacOSX coordinate origin being bottom left
     rather than top right, and so all the Y values sent to the guest
     were inverted

We fix (1) by aligning our behaviour with the SDL UI backend for
absolute devices:
 * when the mouse moves into the window we do a grab (which means
   hiding the host cursor and sending special keys to the guest)
 * when the mouse moves out of the window we un-grab
and fix (2) by doing the correct transformation in the call to
qemu_input_queue_abs().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
(Note that the GTK UI backend behaves differently from SDL; I picked
the SDL one to follow as it is older and less experimental, and also
because the GTK behaviour of "grab-on-click and then never let go
until ungrab keycombo" is problematic for Cocoa because AFAICT there's
no way to get absolute mouse coordinates and still trap the mouse within
our window.)
---
 ui/cocoa.m | 75 ++++++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 46 insertions(+), 29 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 1a626e2..06951d0 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -256,7 +256,7 @@ static int cocoa_keycode_to_qemu(int keycode)
     BOOL isMouseGrabbed;
     BOOL isFullscreen;
     BOOL isAbsoluteEnabled;
-    BOOL isTabletEnabled;
+    BOOL isMouseDeassociated;
 }
 - (void) switchSurface:(DisplaySurface *)surface;
 - (void) grabMouse;
@@ -264,8 +264,21 @@ static int cocoa_keycode_to_qemu(int keycode)
 - (void) toggleFullScreen:(id)sender;
 - (void) handleEvent:(NSEvent *)event;
 - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
+/* The state surrounding mouse grabbing is potentially confusing.
+ * isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated
+ *   pointing device an absolute-position one?"], but is only updated on
+ *   next refresh.
+ * isMouseGrabbed tracks whether GUI events are directed to the guest;
+ *   it controls whether special keys like Cmd get sent to the guest,
+ *   and whether we capture the mouse when in non-absolute mode.
+ * isMouseDeassociated tracks whether we've told MacOSX to disassociate
+ *   the mouse and mouse cursor position by calling
+ *   CGAssociateMouseAndMouseCursorPosition(FALSE)
+ *   (which basically happens if we grab in non-absolute mode).
+ */
 - (BOOL) isMouseGrabbed;
 - (BOOL) isAbsoluteEnabled;
+- (BOOL) isMouseDeassociated;
 - (float) cdx;
 - (float) cdy;
 - (QEMUScreen) gscreen;
@@ -613,14 +626,12 @@ QemuCocoaView *cocoaView;
         case NSMouseMoved:
             if (isAbsoluteEnabled) {
                 if (![self screenContainsPoint:p] || ![[self window] isKeyWindow]) {
-                    if (isTabletEnabled) { // if we leave the window, deactivate the tablet
-                        [NSCursor unhide];
-                        isTabletEnabled = FALSE;
+                    if (isMouseGrabbed) {
+                        [self ungrabMouse];
                     }
                 } else {
-                    if (!isTabletEnabled) { // if we enter the window, activate the tablet
-                        [NSCursor hide];
-                        isTabletEnabled = TRUE;
+                    if (!isMouseGrabbed) {
+                        [self grabMouse];
                     }
                 }
             }
@@ -659,16 +670,9 @@ QemuCocoaView *cocoaView;
             mouse_event = true;
             break;
         case NSLeftMouseUp:
-            if (isTabletEnabled) {
-                    mouse_event = true;
-            } else if (!isMouseGrabbed) {
-                if ([self screenContainsPoint:p]) {
-                    [self grabMouse];
-                } else {
-                    [NSApp sendEvent:event];
-                }
-            } else {
-                mouse_event = true;
+            mouse_event = true;
+            if (!isMouseGrabbed && [self screenContainsPoint:p]) {
+                [self grabMouse];
             }
             break;
         case NSRightMouseUp:
@@ -678,13 +682,11 @@ QemuCocoaView *cocoaView;
             mouse_event = true;
             break;
         case NSScrollWheel:
-            if (isTabletEnabled || isMouseGrabbed) {
+            if (isMouseGrabbed) {
                 buttons |= ([event deltaY] < 0) ?
                     MOUSE_EVENT_WHEELUP : MOUSE_EVENT_WHEELDN;
-                mouse_event = true;
-            } else {
-                [NSApp sendEvent:event];
             }
+            mouse_event = true;
             break;
         default:
             [NSApp sendEvent:event];
@@ -702,12 +704,20 @@ QemuCocoaView *cocoaView;
             qemu_input_update_buttons(dcl->con, bmap, last_buttons, buttons);
             last_buttons = buttons;
         }
-        if (isTabletEnabled) {
-            qemu_input_queue_abs(dcl->con, INPUT_AXIS_X, p.x, screen.width);
-            qemu_input_queue_abs(dcl->con, INPUT_AXIS_Y, p.y, screen.height);
-        } else if (isMouseGrabbed) {
-            qemu_input_queue_rel(dcl->con, INPUT_AXIS_X, (int)[event deltaX]);
-            qemu_input_queue_rel(dcl->con, INPUT_AXIS_Y, (int)[event deltaY]);
+        if (isMouseGrabbed) {
+            if (isAbsoluteEnabled) {
+                /* Note that the origin for Cocoa mouse coords is bottom left, not top left.
+                 * The check on screenContainsPoint is to avoid sending out of range values for
+                 * clicks in the titlebar.
+                 */
+                if ([self screenContainsPoint:p]) {
+                    qemu_input_queue_abs(dcl->con, INPUT_AXIS_X, p.x, screen.width);
+                    qemu_input_queue_abs(dcl->con, INPUT_AXIS_Y, screen.height - p.y, screen.height);
+                }
+            } else {
+                qemu_input_queue_rel(dcl->con, INPUT_AXIS_X, (int)[event deltaX]);
+                qemu_input_queue_rel(dcl->con, INPUT_AXIS_Y, (int)[event deltaY]);
+            }
         } else {
             [NSApp sendEvent:event];
         }
@@ -726,7 +736,10 @@ QemuCocoaView *cocoaView;
             [normalWindow setTitle:@"QEMU - (Press ctrl + alt to release Mouse)"];
     }
     [NSCursor hide];
-    CGAssociateMouseAndMouseCursorPosition(FALSE);
+    if (!isAbsoluteEnabled) {
+        isMouseDeassociated = TRUE;
+        CGAssociateMouseAndMouseCursorPosition(FALSE);
+    }
     isMouseGrabbed = TRUE; // while isMouseGrabbed = TRUE, QemuCocoaApp sends all events to [cocoaView handleEvent:]
 }
 
@@ -741,13 +754,17 @@ QemuCocoaView *cocoaView;
             [normalWindow setTitle:@"QEMU"];
     }
     [NSCursor unhide];
-    CGAssociateMouseAndMouseCursorPosition(TRUE);
+    if (isMouseDeassociated) {
+        CGAssociateMouseAndMouseCursorPosition(TRUE);
+        isMouseDeassociated = FALSE;
+    }
     isMouseGrabbed = FALSE;
 }
 
 - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {isAbsoluteEnabled = tIsAbsoluteEnabled;}
 - (BOOL) isMouseGrabbed {return isMouseGrabbed;}
 - (BOOL) isAbsoluteEnabled {return isAbsoluteEnabled;}
+- (BOOL) isMouseDeassociated {return isMouseDeassociated;}
 - (float) cdx {return cdx;}
 - (float) cdy {return cdy;}
 - (QEMUScreen) gscreen {return screen;}
-- 
1.8.5.4

[Qemu-devel] [PATCH 4/4] ui/cocoa: Honour -show-cursor command line option

[Peter Maydell]

Honour the -show-cursor command line option (which forces the mouse pointer
to always be displayed even when input is grabbed) in the Cocoa UI backend.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 ui/cocoa.m | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 06951d0..3147178 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -323,6 +323,22 @@ QemuCocoaView *cocoaView;
     return (p.x > -1 && p.x < screen.width && p.y > -1 && p.y < screen.height);
 }
 
+- (void) hideCursor
+{
+    if (!cursor_hide) {
+        return;
+    }
+    [NSCursor hide];
+}
+
+- (void) unhideCursor
+{
+    if (!cursor_hide) {
+        return;
+    }
+    [NSCursor unhide];
+}
+
 - (void) drawRect:(NSRect) rect
 {
     COCOA_DEBUG("QemuCocoaView: drawRect\n");
@@ -735,7 +751,7 @@ QemuCocoaView *cocoaView;
         else
             [normalWindow setTitle:@"QEMU - (Press ctrl + alt to release Mouse)"];
     }
-    [NSCursor hide];
+    [self hideCursor];
     if (!isAbsoluteEnabled) {
         isMouseDeassociated = TRUE;
         CGAssociateMouseAndMouseCursorPosition(FALSE);
@@ -753,7 +769,7 @@ QemuCocoaView *cocoaView;
         else
             [normalWindow setTitle:@"QEMU"];
     }
-    [NSCursor unhide];
+    [self unhideCursor];
     if (isMouseDeassociated) {
         CGAssociateMouseAndMouseCursorPosition(TRUE);
         isMouseDeassociated = FALSE;
-- 
1.8.5.4

Re: [Qemu-devel] [PATCH 0/4] ui/cocoa: Fix absolute positioning and other bugs

[Peter Maydell]

On 23 June 2014 10:35, Peter Maydell <peter.maydell@linaro.org> wrote:
> This set of cocoa UI patches:
>  * fixes the completely broken handling of absolute positioning
>    (tablet-style) input devices
>  * fixes a bug where if the first surface created was the same 640x480
>    as the initial window we'd never actually draw it
>  * implements support for the -show-cursor command line option
>
> The GTK and SDL UI frontends don't seem to be consistent about how they
> handle mousegrab for absolute-position devices; I followed SDL on the
> basis that it was the older and more established UI. (GTK doesn't
> implement -show-cursor at all, incidentally.)
>
> Peter Maydell (4):
>   ui/cocoa: Cope with first surface being same as initial window size
>   ui/cocoa: Add utility method to check if point is within window
>   ui/cocoa: Fix handling of absolute positioning devices
>   ui/cocoa: Honour -show-cursor command line option
>
>  ui/cocoa.m | 108 ++++++++++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 75 insertions(+), 33 deletions(-)

Git branch here if you wish to test:

git://git.linaro.org/people/pmaydell/qemu-arm.git cocoa-abs

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/4] ui/cocoa: Fix absolute positioning and other bugs

[Peter Maydell]

On 23 June 2014 10:35, Peter Maydell <peter.maydell@linaro.org> wrote:
> This set of cocoa UI patches:
>  * fixes the completely broken handling of absolute positioning
>    (tablet-style) input devices
>  * fixes a bug where if the first surface created was the same 640x480
>    as the initial window we'd never actually draw it
>  * implements support for the -show-cursor command line option
>
> The GTK and SDL UI frontends don't seem to be consistent about how they
> handle mousegrab for absolute-position devices; I followed SDL on the
> basis that it was the older and more established UI. (GTK doesn't
> implement -show-cursor at all, incidentally.)
>
> Peter Maydell (4):
>   ui/cocoa: Cope with first surface being same as initial window size
>   ui/cocoa: Add utility method to check if point is within window
>   ui/cocoa: Fix handling of absolute positioning devices
>   ui/cocoa: Honour -show-cursor command line option

I double-checked and none of these patches add new calls
to MacOSX functions (they're just fixing/rearranging logic) so
I don't expect this to break older versions (tested on 10.9).
I'm planning to send a cocoa.next pullreq with these in on Monday.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/4] ui/cocoa: Fix absolute positioning and other bugs

[Gerd Hoffmann]

On Mo, 2014-06-23 at 10:35 +0100, Peter Maydell wrote:
> This set of cocoa UI patches:
>  * fixes the completely broken handling of absolute positioning
>    (tablet-style) input devices
>  * fixes a bug where if the first surface created was the same 640x480
>    as the initial window we'd never actually draw it
>  * implements support for the -show-cursor command line option
> 
> The GTK and SDL UI frontends don't seem to be consistent about how they
> handle mousegrab for absolute-position devices; I followed SDL on the
> basis that it was the older and more established UI. (GTK doesn't
> implement -show-cursor at all, incidentally.)

Series looks sane to me, although I can't claim I fully understand patch
#3, especially the macos x mouse handling internals involved there ...

cheers,
  Gerd