[1/2] ui/cocoa: Fix absolute input device grabbing issue on Mojave

Commit Message

Zhijian Li (Fujitsu)" via June 4, 2019, 9:36 a.m. UTC

On Mojave, absolute input device, i.e. tablet, had trouble re-grabbing
the cursor in re-entry into the virtual screen area. In some cases,
the `window` property of NSEvent object was nil after cursor exiting from
window, hinting that the `-locationInWindow` method would return value in 
screen coordinates. The current implementation used raw locations from 
NSEvent without considering whether the value was for the window coordinates
or the macOS screen coordinates, nor the zooming factor for Zoom-to-Fit in
fullscreen mode.

In fullscreen mode, the fullscreen cocoa window might not be the key
window, therefore the location of event in virtual coordinates should
suffice.

This patches fixed boundary check methods for cursor in normal
and fullscreen with/without Zoom-to-Fit in Mojave.

Note: CGRect, -convertRectToScreen: and -convertRectFromScreen: were
used in coordinates conversion for compatibility reason.

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

Comments

Peter Maydell June 7, 2019, 5:04 p.m. UTC | #1

On Tue, 4 Jun 2019 at 10:36, Chen Zhang <[e-mail]> wrote:
>
> On Mojave, absolute input device, i.e. tablet, had trouble re-grabbing
> the cursor in re-entry into the virtual screen area. In some cases,
> the `window` property of NSEvent object was nil after cursor exiting from
> window, hinting that the `-locationInWindow` method would return value in
> screen coordinates. The current implementation used raw locations from
> NSEvent without considering whether the value was for the window coordinates
> or the macOS screen coordinates, nor the zooming factor for Zoom-to-Fit in
> fullscreen mode.
>
> In fullscreen mode, the fullscreen cocoa window might not be the key
> window, therefore the location of event in virtual coordinates should
> suffice.
>
> This patches fixed boundary check methods for cursor in normal
> and fullscreen with/without Zoom-to-Fit in Mojave.
>
> Note: CGRect, -convertRectToScreen: and -convertRectFromScreen: were
> used in coordinates conversion for compatibility reason.
>
> Signed-off-by: Chen Zhang <[e-mail]>
> ---
>  ui/cocoa.m | 43 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 420b2411c1..474d44cb9f 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -405,6 +405,41 @@ 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];
> +    // XXX: Use CGRect and -convertRectFromScreen: to support macOS 10.10

The "XXX" prefix usually indicates something in the code that
needs to be fixed, but in this case the code is already using
these methods, so it's OK I think?

(I hope to have some time to test these patches this weekend
or next week.)

thanks
-- PMM

Patch

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 420b2411c1..474d44cb9f 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -405,6 +405,41 @@  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];
+    // XXX: Use CGRect and -convertRectFromScreen: to support macOS 10.10
+    CGRect r = CGRectZero;
+    r.origin = [ev locationInWindow];
+    if (!eventWindow) {
+        if (!isFullscreen) {
+            return [[self window] convertRectFromScreen:r].origin;
+        } else {
+            CGPoint locationInSelfWindow = [[self window] convertRectFromScreen:r].origin;
+            CGPoint loc = [self convertPoint:locationInSelfWindow fromView:nil];
+            if (stretch_video) {
+                loc.x /= cdx;
+                loc.y /= cdy;
+            }
+            return loc;
+        }
+    } else if ([[self window] isEqual:eventWindow]) {
+        if (!isFullscreen) {
+            return r.origin;
+        } else {
+            CGPoint loc = [self convertPoint:r.origin fromView:nil];
+            if (stretch_video) {
+                loc.x /= cdx;
+                loc.y /= cdy;
+            }
+            return loc;
+        }
+    } else {
+        return [[self window] convertRectFromScreen:[eventWindow convertRectToScreen:r]].origin;
+    }
+}
+
 - (void) hideCursor
 {
     if (!cursor_hide) {
@@ -704,7 +739,8 @@  QemuCocoaView *cocoaView;
     int keycode = 0;
     bool mouse_event = false;
     static bool switched_to_fullscreen = false;
-    NSPoint p = [event locationInWindow];
+    // Location of event in virtual screen coordinates
+    NSPoint p = [self screenLocationOfEvent:event];
 
     switch ([event type]) {
         case NSEventTypeFlagsChanged:
@@ -815,7 +851,10 @@  QemuCocoaView *cocoaView;
             break;
         case NSEventTypeMouseMoved:
             if (isAbsoluteEnabled) {
-                if (![self screenContainsPoint:p] || ![[self window] isKeyWindow]) {
+                // Cursor re-entered into a window might generate events bound to screen coordinates
+                // and `nil` window property, and in full screen mode, current window might not be
+                // key window, where event location alone should suffice.
+                if (![self screenContainsPoint:p] || !([[self window] isKeyWindow] || isFullscreen)) {
                     if (isMouseGrabbed) {
                         [self ungrabMouse];
                     }