[Qemu-devel] [PATCH] ui/cocoa.m: fix absolute mode

[Qemu-devel] [PATCH] ui/cocoa.m: fix absolute mode

[John Arbuckle]

Fix the cocoa front-end to correctly be able to use absolute mode.

Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
---
 ui/cocoa.m | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 2991ed4f19..dda99ad638 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -350,9 +350,20 @@ - (BOOL) isOpaque
     return YES;
 }
 
-- (BOOL) screenContainsPoint:(NSPoint) p
+/* Returns YES if the host mouse cursor is in the QEMU window, NO otherwise */
+- (BOOL) mouseInWindow
 {
-    return (p.x > -1 && p.x < screen.width && p.y > -1 && p.y < screen.height);
+    NSPoint p = [NSEvent mouseLocation];
+    BOOL return_value = NO;
+    float x, y, width, height;
+    x = [normalWindow frame].origin.x;
+    y = [normalWindow frame].origin.y;
+    width = [[normalWindow contentView] frame].size.width;
+    height = [[normalWindow contentView] frame].size.height;
+    if (p.x >= x && p.y >= y && p.x <= (x + width) && p.y <= (y + height)) {
+        return_value = YES;
+    }
+    return return_value;
 }
 
 - (void) hideCursor
@@ -637,7 +648,6 @@ - (void) handleEvent:(NSEvent *)event
     int buttons = 0;
     int keycode = 0;
     bool mouse_event = false;
-    NSPoint p = [event locationInWindow];
 
     switch ([event type]) {
         case NSEventTypeFlagsChanged:
@@ -738,17 +748,22 @@ - (void) handleEvent:(NSEvent *)event
             break;
         case NSEventTypeMouseMoved:
             if (isAbsoluteEnabled) {
-                if (![self screenContainsPoint:p] || ![[self window] isKeyWindow]) {
+                if ([self mouseInWindow]) {
+                    mouse_event = true;
+                }
+
+                if (![self mouseInWindow] || ![[self window] isKeyWindow]) {
                     if (isMouseGrabbed) {
                         [self ungrabMouse];
                     }
                 } else {
-                    if (!isMouseGrabbed) {
+                    if (!isMouseGrabbed && [self mouseInWindow]) {
                         [self grabMouse];
                     }
                 }
+            } else {
+                mouse_event = true;
             }
-            mouse_event = true;
             break;
         case NSEventTypeLeftMouseDown:
             if ([event modifierFlags] & NSEventModifierFlagCommand) {
@@ -784,7 +799,7 @@ - (void) handleEvent:(NSEvent *)event
             break;
         case NSEventTypeLeftMouseUp:
             mouse_event = true;
-            if (!isMouseGrabbed && [self screenContainsPoint:p]) {
+            if (!isMouseGrabbed && [self mouseInWindow]) {
                 if([[self window] isKeyWindow]) {
                     [self grabMouse];
                 }
@@ -844,7 +859,8 @@ - (void) handleEvent:(NSEvent *)event
                  * The check on screenContainsPoint is to avoid sending out of range values for
                  * clicks in the titlebar.
                  */
-                if ([self screenContainsPoint:p]) {
+                if ([self mouseInWindow]) {
+                    NSPoint p = [event locationInWindow];
                     qemu_input_queue_abs(dcl->con, INPUT_AXIS_X, p.x, 0, screen.width);
                     qemu_input_queue_abs(dcl->con, INPUT_AXIS_Y, screen.height - p.y, 0, screen.height);
                 }
-- 
2.14.3 (Apple Git-98)

Re: [Qemu-devel] [PATCH] ui/cocoa.m: fix absolute mode

[Peter Maydell]

On 8 June 2018 at 21:01, John Arbuckle <programmingkidx@gmail.com> wrote:
> Fix the cocoa front-end to correctly be able to use absolute mode.
>
> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>

This commit message is insufficiently detailed, which makes
it hard to review the patch. When you send patches, please
can you provide commit messages which describe what the
problem is, where the current code gets things wrong and
why the changes fix the issue? It is painful to have to
reverse-engineer your design from your code changes.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] ui/cocoa.m: fix absolute mode

[Programmingkid]

On Jun 18, 2018, at 7:18 AM, Peter Maydell wrote:

> On 8 June 2018 at 21:01, John Arbuckle <programmingkidx@gmail.com> wrote:
>> Fix the cocoa front-end to correctly be able to use absolute mode.
>> 
>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
> 
> This commit message is insufficiently detailed, which makes
> it hard to review the patch. When you send patches, please
> can you provide commit messages which describe what the
> problem is, where the current code gets things wrong and
> why the changes fix the issue? It is painful to have to
> reverse-engineer your design from your code changes.

Sorry I will correct this problem now.

This should have been the commit message:

When using an usb-tablet device in QEMU, the Cocoa UI uses absolute mode code to handle mouse events. The absolute mode code does not correctly determine where QEMU's window is located. Tests indicate the current code thinks the window is located in the bottom left of the screen. This makes grabbing and ungrabbing happen at the wrong place. This patch fixes this problem by making mouse grabs happen only in the window and mouse ungrabs happen only when the mouse is moved outside the window.

Re: [Qemu-devel] [PATCH] ui/cocoa.m: fix absolute mode

[Peter Maydell]

On 18 June 2018 at 23:24, Programmingkid <programmingkidx@gmail.com> wrote:
> When using an usb-tablet device in QEMU, the Cocoa UI uses absolute
>mode code to handle mouse events. The absolute mode code does not
>correctly determine where QEMU's window is located. Tests indicate
>the current code thinks the window is located in the bottom left
>of the screen.

I guess this is happening because of the note in the
NSEvent locationInWindow method docs that the event
can have a nil window, in which case it gives you the
event location in screen coordinates:
https://developer.apple.com/documentation/appkit/nsevent/1529068-locationinwindow

Those docs also say that the common way to deal with this
is to use the NSView convert method to get the window-relative
coordinates that you want. Maybe we just need to do that?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] ui/cocoa.m: fix absolute mode

[Peter Maydell]

On 8 June 2018 at 21:01, John Arbuckle <programmingkidx@gmail.com> wrote:
> Fix the cocoa front-end to correctly be able to use absolute mode.
>
> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>

>          case NSEventTypeMouseMoved:
>              if (isAbsoluteEnabled) {
> -                if (![self screenContainsPoint:p] || ![[self window] isKeyWindow]) {
> +                if ([self mouseInWindow]) {
> +                    mouse_event = true;
> +                }
> +
> +                if (![self mouseInWindow] || ![[self window] isKeyWindow]) {
>                      if (isMouseGrabbed) {
>                          [self ungrabMouse];
>                      }
>                  } else {
> -                    if (!isMouseGrabbed) {
> +                    if (!isMouseGrabbed && [self mouseInWindow]) {
>                          [self grabMouse];
>                      }

We may well not need this part of the patch, but:
this change of condition from "!isMouseGrabbed" to
"!isMouseGrabbed && [self mouseInWindow]" is unnecessary.
This code is inside the "else" block for the preceding
"if (![self mouseInWindow] || ![[self window] isKeyWindow])",
so it is not possible to reach it if "[self mouseInWindow]"
returns false.

thanks
-- PMM