[v2,2/2] ui/cocoa: Adds NSCursor absolute pointer support

Commit Message

Phil Dennis-Jordan June 25, 2024, 1:49 p.m. UTC

When pointer input is absolute, use the native macOS host’s Cocoa
NSCursor to render the guest’s cursor. The rendered cursor is no longer
cropped to the guest viewport, and the correct cursor image is passed to
anything tapping into the host system’s native cursor. (such as remote
access)

The CALayer is retained for rendering the cursor in relative pointer
input mode. Cropping the cursor here gives a visual indication of the
captured pointer (the mouse must be explicitly ungrabbed before allowing
the cursor to leave the Qemu window), and teleporting the host cursor
when its position is changed by the guest causes a feedback loop in
input events.

Signed-off-by: Phil Dennis-Jordan <[e-mail]>
---
 ui/cocoa.m | 82 +++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 65 insertions(+), 17 deletions(-)

Comments

Akihiko Odaki June 27, 2024, 11:48 a.m. UTC | #1

Hi,

Thanks for fixing my patch and adding this follow-up.

I incorporated your fix with some change with v2 so please review it and 
rebase this patch to it.

On 2024/06/25 22:49, Phil Dennis-Jordan wrote:
> When pointer input is absolute, use the native macOS host’s Cocoa
> NSCursor to render the guest’s cursor. The rendered cursor is no longer
> cropped to the guest viewport, and the correct cursor image is passed to
> anything tapping into the host system’s native cursor. (such as remote
> access)
> 
> The CALayer is retained for rendering the cursor in relative pointer
> input mode. Cropping the cursor here gives a visual indication of the
> captured pointer (the mouse must be explicitly ungrabbed before allowing
> the cursor to leave the Qemu window), and teleporting the host cursor
> when its position is changed by the guest causes a feedback loop in
> input events. >
> Signed-off-by: Phil Dennis-Jordan <[e-mail]>
> ---
>   ui/cocoa.m | 82 +++++++++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 65 insertions(+), 17 deletions(-)
> 
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index cca987eac7..131c442e16 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -314,6 +314,7 @@ @interface QemuCocoaView : NSView
>       CFMachPortRef eventsTap;
>       CALayer *cursorLayer;
>       QEMUCursor *cursor;
> +    NSCursor *cocoaCursor;
>       int mouseX;
>       int mouseY;
>       int mouseOn;
> @@ -402,6 +403,9 @@ - (void) dealloc
>   
>       [cursorLayer release];
>       cursorLayer = nil;
> +    [cocoaCursor release];
> +    cocoaCursor = nil;
> +
>       [super dealloc];
>   }
>   
> @@ -460,27 +464,14 @@ - (void)setMouseX:(int)x y:(int)y on:(int)on
>       [CATransaction begin];
>       [CATransaction setDisableActions:YES];
>       [cursorLayer setPosition:position];
> -    [cursorLayer setHidden:!mouseOn];
> +    [cursorLayer setHidden:!mouseOn || isAbsoluteEnabled];
>       [CATransaction commit];
>   }
>   
> -- (void)setCursor:(QEMUCursor *)given_cursor
> +static CGImageRef cursor_cgimage_create(QEMUCursor *cursor)

Don't add C functions in middle of Objective-C definition.

>   {
>       CGDataProviderRef provider;
>       CGImageRef image;
> -    CGRect bounds = CGRectZero;
> -
> -    cursor_unref(cursor);
> -    cursor = given_cursor;
> -
> -    if (!cursor) {
> -        return;
> -    }
> -
> -    cursor_ref(cursor);
> -
> -    bounds.size.width = cursor->width;
> -    bounds.size.height = cursor->height;
>       CGColorSpaceRef color_space = CGColorSpaceCreateWithName(kCGColorSpaceSRGB);
>   
>       provider = CGDataProviderCreateWithData(
> @@ -506,6 +497,43 @@ - (void)setCursor:(QEMUCursor *)given_cursor
>   
>       CGDataProviderRelease(provider);
>       CGColorSpaceRelease(color_space);
> +    return image;
> +}
> +
> +static NSCursor *cocoa_cursor_create(QEMUCursor *cursor, CGImageRef image)
> +{
> +    NSPoint hotspot = { cursor->hot_x, cursor->hot_y };

Use NSMakePoint() for the consistency with the next line (among other 
similar constructs).

> +    NSSize size = NSMakeSize(cursor->width, cursor->height);
> +    NSImage *cursor_image = [[NSImage alloc] initWithCGImage:image size:size];
> +    NSCursor *cocoa_cursor =
> +        [[NSCursor alloc] initWithImage:cursor_image hotSpot:hotspot];
> +    [cursor_image release];
> +    return cocoa_cursor;
> +}
> +
> +- (void)setCursor:(QEMUCursor *)given_cursor
> +{
> +    CGImageRef image;
> +    NSImage *cursor_nsimage = nil;
> +    CGRect bounds = CGRectZero;
> +
> +    cursor_unref(cursor);
> +    cursor = given_cursor;
> +
> +    if (!cursor) {
> +        return;
> +    }
> +
> +    cursor_ref(cursor);
> +
> +    bounds.size.width = cursor->width;
> +    bounds.size.height = cursor->height;
> +
> +    image = cursor_cgimage_create(cursor);
> +    [cocoaCursor release];
> +    cocoaCursor = cocoa_cursor_create(cursor, image);
> +    [self.window invalidateCursorRectsForView:self];
> +
>       [CATransaction begin];
>       [CATransaction setDisableActions:YES];
>       [cursorLayer setBounds:bounds];
> @@ -514,6 +542,16 @@ - (void)setCursor:(QEMUCursor *)given_cursor
>       CGImageRelease(image);
>   }
>   
> +- (void) resetCursorRects
> +{
> +    if (self->cocoaCursor == nil) {

For consistency, just do: if (!cocoaCursor)

> +        [super resetCursorRects];
> +    } else {
> +        NSRect guest_area = {{ 0.0, 0.0 }, { screen.width, screen.height }};
> +        [self addCursorRect:guest_area cursor:cocoaCursor];
> +    }
> +}
> +
>   - (void) drawRect:(NSRect) rect
>   {
>       COCOA_DEBUG("QemuCocoaView: drawRect\n");
> @@ -1181,7 +1219,12 @@ - (void) grabMouse
>           [[self window] setTitle:[NSString stringWithFormat:@"QEMU %s - (Press  " UC_CTRL_KEY " " UC_ALT_KEY " G  to release Mouse)", qemu_name]];
>       else
>           [[self window] setTitle:@"QEMU - (Press  " UC_CTRL_KEY " " UC_ALT_KEY " G  to release Mouse)"];
> -    [self hideCursor];
> +
> +    [cursorLayer setHidden:!mouseOn || isAbsoluteEnabled];
> +    if (!isAbsoluteEnabled) {
> +        [self hideCursor];
> +    }

[self hideCursor] should also be called for an absolute pointer device 
if the guest does not set the cursor. See ui/gtk.c and ui/sdl2.c to know 
how the show-cursor option should behave.

Regards,
Akihiko Odaki

Patch

diff --git a/ui/cocoa.m b/ui/cocoa.m
index cca987eac7..131c442e16 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -314,6 +314,7 @@  @interface QemuCocoaView : NSView
     CFMachPortRef eventsTap;
     CALayer *cursorLayer;
     QEMUCursor *cursor;
+    NSCursor *cocoaCursor;
     int mouseX;
     int mouseY;
     int mouseOn;
@@ -402,6 +403,9 @@  - (void) dealloc
 
     [cursorLayer release];
     cursorLayer = nil;
+    [cocoaCursor release];
+    cocoaCursor = nil;
+
     [super dealloc];
 }
 
@@ -460,27 +464,14 @@  - (void)setMouseX:(int)x y:(int)y on:(int)on
     [CATransaction begin];
     [CATransaction setDisableActions:YES];
     [cursorLayer setPosition:position];
-    [cursorLayer setHidden:!mouseOn];
+    [cursorLayer setHidden:!mouseOn || isAbsoluteEnabled];
     [CATransaction commit];
 }
 
-- (void)setCursor:(QEMUCursor *)given_cursor
+static CGImageRef cursor_cgimage_create(QEMUCursor *cursor)
 {
     CGDataProviderRef provider;
     CGImageRef image;
-    CGRect bounds = CGRectZero;
-
-    cursor_unref(cursor);
-    cursor = given_cursor;
-
-    if (!cursor) {
-        return;
-    }
-
-    cursor_ref(cursor);
-
-    bounds.size.width = cursor->width;
-    bounds.size.height = cursor->height;
     CGColorSpaceRef color_space = CGColorSpaceCreateWithName(kCGColorSpaceSRGB);
 
     provider = CGDataProviderCreateWithData(
@@ -506,6 +497,43 @@  - (void)setCursor:(QEMUCursor *)given_cursor
 
     CGDataProviderRelease(provider);
     CGColorSpaceRelease(color_space);
+    return image;
+}
+
+static NSCursor *cocoa_cursor_create(QEMUCursor *cursor, CGImageRef image)
+{
+    NSPoint hotspot = { cursor->hot_x, cursor->hot_y };
+    NSSize size = NSMakeSize(cursor->width, cursor->height);
+    NSImage *cursor_image = [[NSImage alloc] initWithCGImage:image size:size];
+    NSCursor *cocoa_cursor =
+        [[NSCursor alloc] initWithImage:cursor_image hotSpot:hotspot];
+    [cursor_image release];
+    return cocoa_cursor;
+}
+
+- (void)setCursor:(QEMUCursor *)given_cursor
+{
+    CGImageRef image;
+    NSImage *cursor_nsimage = nil;
+    CGRect bounds = CGRectZero;
+
+    cursor_unref(cursor);
+    cursor = given_cursor;
+
+    if (!cursor) {
+        return;
+    }
+
+    cursor_ref(cursor);
+
+    bounds.size.width = cursor->width;
+    bounds.size.height = cursor->height;
+
+    image = cursor_cgimage_create(cursor);
+    [cocoaCursor release];
+    cocoaCursor = cocoa_cursor_create(cursor, image);
+    [self.window invalidateCursorRectsForView:self];
+
     [CATransaction begin];
     [CATransaction setDisableActions:YES];
     [cursorLayer setBounds:bounds];
@@ -514,6 +542,16 @@  - (void)setCursor:(QEMUCursor *)given_cursor
     CGImageRelease(image);
 }
 
+- (void) resetCursorRects
+{
+    if (self->cocoaCursor == nil) {
+        [super resetCursorRects];
+    } else {
+        NSRect guest_area = {{ 0.0, 0.0 }, { screen.width, screen.height }};
+        [self addCursorRect:guest_area cursor:cocoaCursor];
+    }
+}
+
 - (void) drawRect:(NSRect) rect
 {
     COCOA_DEBUG("QemuCocoaView: drawRect\n");
@@ -1181,7 +1219,12 @@  - (void) grabMouse
         [[self window] setTitle:[NSString stringWithFormat:@"QEMU %s - (Press  " UC_CTRL_KEY " " UC_ALT_KEY " G  to release Mouse)", qemu_name]];
     else
         [[self window] setTitle:@"QEMU - (Press  " UC_CTRL_KEY " " UC_ALT_KEY " G  to release Mouse)"];
-    [self hideCursor];
+
+    [cursorLayer setHidden:!mouseOn || isAbsoluteEnabled];
+    if (!isAbsoluteEnabled) {
+        [self hideCursor];
+    }
+
     CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
     isMouseGrabbed = TRUE; // while isMouseGrabbed = TRUE, QemuCocoaApp sends all events to [cocoaView handleEvent:]
 }
@@ -1194,7 +1237,11 @@  - (void) ungrabMouse
         [[self window] setTitle:[NSString stringWithFormat:@"QEMU %s", qemu_name]];
     else
         [[self window] setTitle:@"QEMU"];
-    [self unhideCursor];
+
+    [cursorLayer setHidden:!mouseOn || isAbsoluteEnabled];
+    if (!isAbsoluteEnabled) {
+        [self unhideCursor];
+    }
     CGAssociateMouseAndMouseCursorPosition(TRUE);
     isMouseGrabbed = FALSE;
     [self raiseAllButtons];
@@ -1216,6 +1263,7 @@  - (void) notifyMouseModeChange {
             [self ungrabMouse];
         } else {
             CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
+            [self hideCursor];
         }
     }
 }