[1/2] Fix absolute input device grabbing issues on Mojave

Commit Message

Gonglei (Arei)" via March 15, 2019, 9:36 a.m. UTC

Signed-off-by: Chen Zhang <[e-mail]>
---
 ui/cocoa.m | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

Comments

Peter Maydell March 18, 2019, 11:52 a.m. UTC | #1

On Fri, 15 Mar 2019 at 09:37, Chen Zhang <[e-mail]> wrote:
>
>
> Signed-off-by: Chen Zhang <[e-mail]>

Hi; thanks for this patch. I think a commit message for
the patch would be useful, containing the rationale for
why we make the change. I know you put that in the cover letter,
but that won't get into the git commit history.

If you know under what circumstances the event's window
pointer is NULL, that would also be useful to record.

> ---
>  ui/cocoa.m | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 420b2411c1..5d0a6599d9 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -405,6 +405,24 @@ QemuCocoaView *cocoaView;
>      return (p.x > -1 && p.x < screen.width && p.y > -1 && p.y < screen.height);
>  }
>
> +/* Get location of event and convert to virtual screen coordinate */
> +- (CGPoint) screenLocationOfEvent:(NSEvent *)ev
> +{
> +    NSWindow *eventWindow = [ev window];
> +    if (!eventWindow) {
> +       return [self.window convertPointFromScreen:[ev locationInWindow]];
> +    } else if ([self.window isEqual:eventWindow]) {
> +        return [ev locationInWindow];
> +    } else {
> +        return [self.window convertPointFromScreen:[eventWindow convertPointToScreen:[ev locationInWindow]]];
> +    }
> +}
> +
> +- (BOOL) screenContainsPointOfEvent:(NSEvent *)ev
> +{
> +    return [self screenContainsPoint:[self screenLocationOfEvent:ev]];
> +}

It looks like we pretty much always call
[self screenLocationOfEvent: event] after we call
[self screenContainsPoint:p], which will do the conversion
all over again. So I think it would be clearer to have
a method which did "return an NSPoint which is the location
of this event in the QEMU display window", and then just
call that where we currently do
 NSPoint p = [event locationInWindow];
Then we wouldn't need to change any of the following code or
the screenContainsPoint calls.

thanks
-- PMM

Patch

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 420b2411c1..5d0a6599d9 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -405,6 +405,24 @@  QemuCocoaView *cocoaView;
     return (p.x > -1 && p.x < screen.width && p.y > -1 && p.y < screen.height);
 }
 
+/* Get location of event and convert to virtual screen coordinate */
+- (CGPoint) screenLocationOfEvent:(NSEvent *)ev
+{
+    NSWindow *eventWindow = [ev window];
+    if (!eventWindow) {
+	return [self.window convertPointFromScreen:[ev locationInWindow]];
+    } else if ([self.window isEqual:eventWindow]) {
+        return [ev locationInWindow];
+    } else {
+        return [self.window convertPointFromScreen:[eventWindow convertPointToScreen:[ev locationInWindow]]];
+    }
+}
+
+- (BOOL) screenContainsPointOfEvent:(NSEvent *)ev
+{
+    return [self screenContainsPoint:[self screenLocationOfEvent:ev]];
+}
+
 - (void) hideCursor
 {
     if (!cursor_hide) {
@@ -815,7 +833,9 @@  QemuCocoaView *cocoaView;
             break;
         case NSEventTypeMouseMoved:
             if (isAbsoluteEnabled) {
-                if (![self screenContainsPoint:p] || ![[self window] isKeyWindow]) {
+                BOOL is_key_window = [[self window] isKeyWindow];
+                BOOL is_in_screen =  [self screenContainsPointOfEvent: event];
+                if (!is_in_screen || !is_key_window) {
                     if (isMouseGrabbed) {
                         [self ungrabMouse];
                     }
@@ -927,9 +947,10 @@  QemuCocoaView *cocoaView;
                  * 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, 0, screen.width);
-                    qemu_input_queue_abs(dcl->con, INPUT_AXIS_Y, screen.height - p.y, 0, screen.height);
+                if ([self screenContainsPointOfEvent:event]) {
+                    CGPoint loc = [self screenLocationOfEvent: event];
+                    qemu_input_queue_abs(dcl->con, INPUT_AXIS_X, loc.x, 0, screen.width);
+                    qemu_input_queue_abs(dcl->con, INPUT_AXIS_Y, screen.height - loc.y, 0, screen.height);
                 }
             } else {
                 qemu_input_queue_rel(dcl->con, INPUT_AXIS_X, (int)[event deltaX]);