This change implements the callbacks dpy_cursor_define and dpy_mouse_set
for the Cocoa UI. The incoming mouse cursor image is converted into an
NSCursor object, allowing the guest mouse cursor to be rendered as the
host's native OS cursor on macOS.

This is straightforward in absolute pointing mode, but rather trickier
with a relative pointing device:

 1. The cursor position in Qemu's coordinate system must be translated
    and converted into macOS's Core Graphics/Quartz coordinates when
    positioning the cursor. Additionally, the position already includes
    the hotspot offset; we'd prefer to use the host OS's hotspot support
    so we need subtract the hotspot vector off again.
 2. Setting the cursor position programmatically on macOS biases the
    next mouse movement event by the amount the cursor was shifted.
    If we didn't reverse that bias when forwarding the next event
    back into Qemu's input stack, this would create a feedback loop.
    (The behaviour of affecting mouse events makes sense for e.g.
    setting the cursor position in a remote access system.)

This change slightly improves the user experience when using virtual
display adapter implementations which check for UI back-end cursor
support, and fixes the issue of no visible mouse cursor when using one
which does not. (Such as virtio-vga)

Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
---
 ui/cocoa.m      | 167 +++++++++++++++++++++++++++++++++++++++++++++++-
 ui/trace-events |   7 ++
 2 files changed, 171 insertions(+), 3 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 981615a8b9..0c71533835 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -49,6 +49,7 @@
 #include "qemu/error-report.h"
 #include <Carbon/Carbon.h>
 #include "hw/core/cpu.h"
+#include "trace.h"
 
 #ifndef MAC_OS_VERSION_14_0
 #define MAC_OS_VERSION_14_0 140000
@@ -80,11 +81,17 @@ static void cocoa_switch(DisplayChangeListener *dcl,
 
 static void cocoa_refresh(DisplayChangeListener *dcl);
 
+static void cocoa_cursor_define(DisplayChangeListener *dcl, QEMUCursor *cursor);
+
+static void cocoa_mouse_set(DisplayChangeListener *dcl, int x, int y, int on);
+
 static const DisplayChangeListenerOps dcl_ops = {
     .dpy_name          = "cocoa",
     .dpy_gfx_update = cocoa_update,
     .dpy_gfx_switch = cocoa_switch,
     .dpy_refresh = cocoa_refresh,
+    .dpy_cursor_define = cocoa_cursor_define,
+    .dpy_mouse_set = cocoa_mouse_set,
 };
 static DisplayChangeListener dcl = {
     .ops = &dcl_ops,
@@ -299,6 +306,11 @@ @interface QemuCocoaView : NSView
     BOOL isMouseGrabbed;
     BOOL isAbsoluteEnabled;
     CFMachPortRef eventsTap;
+    NSCursor *current_cursor;
+    int cursor_hot_x;
+    int cursor_hot_y;
+    int offset_delta_x;
+    int offset_delta_y;
 }
 - (void) switchSurface:(pixman_image_t *)image;
 - (void) grabMouse;
@@ -320,6 +332,9 @@ - (BOOL) isMouseGrabbed;
 - (BOOL) isAbsoluteEnabled;
 - (QEMUScreen) gscreen;
 - (void) raiseAllKeys;
+- (void) setCursor:(NSCursor*)newCursor hotspotX:(int)hotX y:(int)hotY;
+- (void) setMouseX:(int)x y:(int)y showCursor:(BOOL)showCursor;
+
 @end
 
 QemuCocoaView *cocoaView;
@@ -376,6 +391,9 @@ - (void) dealloc
         pixman_image_unref(pixman_image);
     }
 
+    [self->current_cursor release];
+    self->current_cursor = nil;
+
     if (eventsTap) {
         CFRelease(eventsTap);
     }
@@ -407,6 +425,68 @@ - (void) selectConsoleLocked:(unsigned int)index
     [self updateUIInfo];
 }
 
+- (void) setCursor:(NSCursor*)newCursor hotspotX:(int)hotX y:(int)hotY
+{
+    [self->current_cursor release];
+    [newCursor retain];
+    self->current_cursor = newCursor;
+
+    cocoaView->cursor_hot_x = hotX;
+    cocoaView->cursor_hot_y = hotY;
+
+    [self.window invalidateCursorRectsForView:self];
+}
+
+- (void) resetCursorRects
+{
+    if (self->current_cursor == nil) {
+        [super resetCursorRects];
+    } else {
+        [self addCursorRect:NSMakeRect(0.0, 0.0, self->screen.width, self->screen.height) cursor:self->current_cursor];
+    }
+}
+
+- (void) setMouseX:(int)x y:(int)y showCursor:(BOOL)showCursor
+{
+    if (isAbsoluteEnabled) {
+        offset_delta_x = 0;
+        offset_delta_y = 0;
+        return;
+    } else if (!isMouseGrabbed) {
+        return;
+    }
+
+    NSWindow* window = [self window];
+
+    /* Coordinates seem to come in already offset by hotspot; undo that. Also
+     * avoid out-of-window coordinates. */
+    x += cursor_hot_x;
+    y += cursor_hot_y;
+    x = int_clamp(x, 0, screen.width);
+    y = int_clamp(y, 0, screen.height);
+    /* Flip coordinates so origin is bottom left (Cocoa), not top left (Qemu),
+     * before translating into window and then desktop coordinate systems. */
+    y = screen.height - y;
+
+    NSPoint new_pos_window = [self convertPoint:NSMakePoint(x, y) toView:nil];
+    NSPoint prev_pos_window = window.mouseLocationOutsideOfEventStream;
+
+    CGPoint screen_pos = [window convertPointToScreen:new_pos_window];
+
+    /* Translate from Cocoa (origin: main screen bottom left, +y up)
+     * to Quartz (origin: top left, +y down) coordinate system. */
+    screen_pos.y = NSScreen.mainScreen.frame.size.height - screen_pos.y;
+
+    /* Warp moves the host cursor to the new position. This doesn't generate a
+     * spurious move event, but it does offset the delta for next genuine event,
+     * which we need to take into account when that event comes in. */
+    CGWarpMouseCursorPosition(screen_pos);
+
+    offset_delta_x += (prev_pos_window.x - new_pos_window.x);
+    /* -ve due to Cocoa -> Qemu/Quartz Y axis conversion: */
+    offset_delta_y -= (prev_pos_window.y - new_pos_window.y);
+}
+
 - (void) hideCursor
 {
     if (!cursor_hide) {
@@ -1005,9 +1085,21 @@ - (void) handleMouseEvent:(NSEvent *)event
             /* Note that the origin for Cocoa mouse coords is bottom left, not top left. */
             qemu_input_queue_abs(dcl.con, INPUT_AXIS_X, p.x * d, 0, screen.width);
             qemu_input_queue_abs(dcl.con, INPUT_AXIS_Y, screen.height - p.y * d, 0, screen.height);
+            trace_cocoa_handle_mouse_event_absolute(p.x, p.y, screen.width, screen.height, p.x * d, screen.height - p.y * d);
         } else {
-            qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, [event deltaX]);
-            qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, [event deltaY]);
+            /* Programmatically moving the Cocoa mouse cursor also offsets the
+             * next mouse event, so we offset by the amount we moved the cursor
+             * to avoid a feedback loop. */
+            int delta_x = [event deltaX] + offset_delta_x;
+            int delta_y = [event deltaY] + offset_delta_y;
+            trace_cocoa_handle_mouse_event_relative(
+                [event deltaX], [event deltaY],
+                offset_delta_x, offset_delta_y,
+                delta_x, delta_y);
+            qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, /*[event deltaX]/*/delta_x/**/);
+            qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, /*[event deltaY]/*/delta_y/**/);
+            offset_delta_x = 0;
+            offset_delta_y = 0;
         }
 
         qemu_input_event_sync();
@@ -1084,19 +1176,26 @@ - (void) otherMouseUp:(NSEvent *)event
 
 - (void) grabMouse
 {
+    trace_cocoa_grab_mouse();
     COCOA_DEBUG("QemuCocoaView: grabMouse\n");
 
     if (qemu_name)
         [[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];
+
+    if (current_cursor == nil) {
+        [self hideCursor];
+    }
     CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
     isMouseGrabbed = TRUE; // while isMouseGrabbed = TRUE, QemuCocoaApp sends all events to [cocoaView handleEvent:]
+    offset_delta_x = 0;
+    offset_delta_y = 0;
 }
 
 - (void) ungrabMouse
 {
+    trace_cocoa_ungrab_mouse();
     COCOA_DEBUG("QemuCocoaView: ungrabMouse\n");
 
     if (qemu_name)
@@ -1104,6 +1203,11 @@ - (void) ungrabMouse
     else
         [[self window] setTitle:@"QEMU"];
     [self unhideCursor];
+
+    if (current_cursor == nil) {
+        [self unhideCursor];
+    }
+
     CGAssociateMouseAndMouseCursorPosition(TRUE);
     isMouseGrabbed = FALSE;
     [self raiseAllButtons];
@@ -2000,6 +2104,63 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
     [pool release];
 }
 
+static NSImage *cocoa_create_image_argb32(size_t width, size_t height, const void *pixel_data)
+{
+    size_t buffer_size = width * height * 4lu;
+    CGDataProviderRef provider = CGDataProviderCreateWithData(
+        NULL, pixel_data, buffer_size, NULL);
+    size_t bpc = 8;
+    size_t bpp = 32;
+    size_t bytes_per_row = 4u * width;
+    CGColorSpaceRef color_space = CGColorSpaceCreateDeviceRGB();
+    CGBitmapInfo bitmap_info =
+        kCGBitmapByteOrder32Little | kCGImageAlphaFirst;
+    CGColorRenderingIntent intent = kCGRenderingIntentDefault;
+
+    CGImageRef cg_image = CGImageCreate(
+        width,
+        height,
+        bpc,
+        bpp,
+        bytes_per_row,
+        color_space,
+        bitmap_info,
+        provider,
+        NULL,       // decode
+        YES,        // should interpolate
+        intent);
+
+    NSImage *image = [[NSImage alloc] initWithCGImage:cg_image size:NSMakeSize(width, height)];
+    CGImageRelease(cg_image);
+    CGColorSpaceRelease(color_space);
+    CGDataProviderRelease(provider);
+    return image;
+}
+
+static void cocoa_cursor_define(DisplayChangeListener *dcl, QEMUCursor *cursor)
+{
+    NSImage *cursor_image = nil;
+    NSPoint hotspot = { cursor->hot_x, cursor->hot_y };
+    trace_cocoa_cursor_define(cursor->hot_x, cursor->hot_y,
+                              cursor->width, cursor->height);
+    if (cursor == NULL || cursor->width <= 0 || cursor->height <= 0) {
+        cursor_image = [[NSImage alloc] initWithSize:NSMakeSize(1.0, 1.0)];
+    } else {
+        cursor_image = cocoa_create_image_argb32(cursor->width, cursor->height,
+                                                 cursor->data);
+    }
+    NSCursor *cocoa_cursor =
+        [[NSCursor alloc] initWithImage:cursor_image hotSpot:hotspot];
+    [cursor_image release];
+    [cocoaView setCursor:cocoa_cursor hotspotX:cursor->hot_x y:cursor->hot_y];
+    [cocoa_cursor release];
+}
+
+static void cocoa_mouse_set(DisplayChangeListener *dcl, int x, int y, int on)
+{
+    [cocoaView setMouseX:x y:y showCursor:(on != 0)];
+}
+
 static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
 {
     NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
diff --git a/ui/trace-events b/ui/trace-events
index 69ff22955d..e53601693d 100644
--- a/ui/trace-events
+++ b/ui/trace-events
@@ -166,3 +166,10 @@ dbus_filter(unsigned int serial, unsigned int filter) "serial=%u (<= %u)"
 
 # egl-helpers.c
 egl_init_d3d11_device(void *p) "d3d device: %p"
+
+# cocoa.m
+cocoa_cursor_define(int hot_x, int hot_y, uint16_t width, uint16_t height) "Cursor hotspot: (%d, %d), size: %" PRIu16 "x%" PRIu16
+cocoa_grab_mouse(void) ""
+cocoa_ungrab_mouse(void) ""
+cocoa_handle_mouse_event_relative(int raw_dx, int raw_dy, int offset_dx, int offset_dy, int net_dx, int net_dy) "raw delta: %d, %d; offset for delta: %d, %d; net delta: %d, %d"
+cocoa_handle_mouse_event_absolute(int event_x, int event_y, int screen_width, int screen_height, int window_x, int window_y) "event: %d, %d; screen: %d x %d; position: %d, %d"
-- 
2.36.1

On 2024/06/09 5:20, Phil Dennis-Jordan wrote:
> This change implements the callbacks dpy_cursor_define and dpy_mouse_set
> for the Cocoa UI. The incoming mouse cursor image is converted into an
> NSCursor object, allowing the guest mouse cursor to be rendered as the
> host's native OS cursor on macOS.
> 
> This is straightforward in absolute pointing mode, but rather trickier
> with a relative pointing device:
> 
>   1. The cursor position in Qemu's coordinate system must be translated
>      and converted into macOS's Core Graphics/Quartz coordinates when
>      positioning the cursor. Additionally, the position already includes
>      the hotspot offset; we'd prefer to use the host OS's hotspot support
>      so we need subtract the hotspot vector off again.
>   2. Setting the cursor position programmatically on macOS biases the
>      next mouse movement event by the amount the cursor was shifted.
>      If we didn't reverse that bias when forwarding the next event
>      back into Qemu's input stack, this would create a feedback loop.
>      (The behaviour of affecting mouse events makes sense for e.g.
>      setting the cursor position in a remote access system.)
> 
> This change slightly improves the user experience when using virtual
> display adapter implementations which check for UI back-end cursor
> support, and fixes the issue of no visible mouse cursor when using one
> which does not. (Such as virtio-vga)

Thanks for working on ui/cocoa, but I already have submitted a patch for 
this particular problem:
https://patchew.org/QEMU/20240318-cursor-v1-0-0bbe6c382217@daynix.com/

The difference between these patches is that my patch does not use 
warping at all. I thought reversing the mouse movement bias is a fragile 
approach that depends on the details of how Quartz works.

On Sun, 9 Jun 2024 at 11:06, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:

> Thanks for working on ui/cocoa, but I already have submitted a patch for
> this particular problem:
> https://patchew.org/QEMU/20240318-cursor-v1-0-0bbe6c382217@daynix.com/
>

Sorry, I missed this patch set - thanks for bringing it to my attention.


> The difference between these patches is that my patch does not use
> warping at all. I thought reversing the mouse movement bias is a fragile
> approach that depends on the details of how Quartz works.
>

Hmm. So, I agree that the relative cursor implementation with NSCursor is
somewhat awkward. I'm not sure it's as fragile as you make out as the
behaviour of the APIs used hasn't changed in decades and has plenty of
existing software depending on it. Still, it might behave awkwardly in the
context of other apps warping the cursor at the same time. I also
definitely think host cursor integration is useful and valuable, at least
in absolute pointing mode - for example, when the host system is itself
being remote controlled, and also to avoid the cursor being cropped near
the edges of the guest viewport.

The CALayer based rendering makes sense to me in relative mode though. For
one, it avoids the complicated event offsets. The cursor cropping actually
makes sense as a visual cue when the cursor is actually constrained to the
guest viewport while mouse input is grabbed. And because the guest cursor
is decoupled from the host cursor even after ungrabbing, it makes sense to
continue rendering it even when Qemu has relinquished the host cursor.

I've therefore reworked my NSCursor code on top of your CALayer cursor and
change notifier work so that the NSCursor is visible and updated with the
guest's cursor image when in absolute mode, and the CALayer draws the
cursor in relative mode. Because of the chain of patch dependencies I've
staged it on gitlab here for initial feedback/testing/review:
https://gitlab.com/pmdj/qemu-upstreaming/-/compare/master...m-cocoa-cursors-2.0?from_project_id=53960510&straight=false

Let me know what you think. If we decide to go with this approach we can
post our respective patches as a combined v2 patchset to the list.

Incidentally, that version of my NSCursor patch includes a few
refinements/fixes to the CALayer patch which I'll tease out into a separate
commit, and which I'd recommend applying even if consensus settles on a
CALayer-only approach. (setCursor: was rather long and messy; it also
leaked a colour space object; layer and cursor objects would leak if the
view was hypothetically dealloc'd)

Thanks,
Phil

On 2024/06/10 23:00, Phil Dennis-Jordan wrote:
> On Sun, 9 Jun 2024 at 11:06, Akihiko Odaki <akihiko.odaki@daynix.com 
> <mailto:akihiko.odaki@daynix.com>> wrote:
> 
>     Thanks for working on ui/cocoa, but I already have submitted a patch
>     for
>     this particular problem:
>     https://patchew.org/QEMU/20240318-cursor-v1-0-0bbe6c382217@daynix.com/ <https://patchew.org/QEMU/20240318-cursor-v1-0-0bbe6c382217@daynix.com/>
> 
> 
> Sorry, I missed this patch set - thanks for bringing it to my attention.
> 
>     The difference between these patches is that my patch does not use
>     warping at all. I thought reversing the mouse movement bias is a
>     fragile
>     approach that depends on the details of how Quartz works.
> 
> 
> Hmm. So, I agree that the relative cursor implementation with NSCursor 
> is somewhat awkward. I'm not sure it's as fragile as you make out as the 
> behaviour of the APIs used hasn't changed in decades and has plenty of 
> existing software depending on it. Still, it might behave awkwardly in 
> the context of other apps warping the cursor at the same time. I also 
> definitely think host cursor integration is useful and valuable, at 
> least in absolute pointing mode - for example, when the host system is 
> itself being remote controlled, and also to avoid the cursor being 
> cropped near the edges of the guest viewport.

Can you elaborate more about the remote control scenario? I don't think 
having extra code just to fix cropped cursor is not worthwhile (I even 
feel a bit awkward to make the cursor overflow.)

> 
> The CALayer based rendering makes sense to me in relative mode though. 
> For one, it avoids the complicated event offsets. The cursor cropping 
> actually makes sense as a visual cue when the cursor is actually 
> constrained to the guest viewport while mouse input is grabbed. And 
> because the guest cursor is decoupled from the host cursor even after 
> ungrabbing, it makes sense to continue rendering it even when Qemu has 
> relinquished the host cursor.
> 
> I've therefore reworked my NSCursor code on top of your CALayer cursor 
> and change notifier work so that the NSCursor is visible and updated 
> with the guest's cursor image when in absolute mode, and the CALayer 
> draws the cursor in relative mode. Because of the chain of patch 
> dependencies I've staged it on gitlab here for initial 
> feedback/testing/review:

You can add Based-on: to the cover letter to clarify the dependency, and 
add "RFC" to the subject if the code is not ready to pull. Please look 
at docs/devel/submitting-a-patch.rst for more info.

Regards,
Akihiko Odaki

On Tue, 11 Jun 2024 at 09:36, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> > […] I also
> > definitely think host cursor integration is useful and valuable, at
> > least in absolute pointing mode - for example, when the host system is
> > itself being remote controlled, and also to avoid the cursor being
> > cropped near the edges of the guest viewport.
>
> Can you elaborate more about the remote control scenario? I don't think
> having extra code just to fix cropped cursor is not worthwhile (I even
> feel a bit awkward to make the cursor overflow.)

If you're remote-controlling the host Mac, many VNC/RDP/whatever
clients will use a local cursor and simply request cursor image
updates from the server and apply them to the local native cursor.
That way, there's no lag when moving the cursor even on slower
connections, which you'd otherwise get if you had to wait for the
regular screen image update, and which can make precise positioning
difficult.

At any rate, most other Qemu UI frontends likewise implement guest
cursors by setting the guest-supplied cursor image on the host's
native windowing system's cursor, e.g. gdk_window_set_cursor(). I
don't really see a good reason why macOS should be different? Qemu
would also hardly be the first VMM on the Mac to pass guest pointers
through as NSPointers - Parallels Desktop appears to do the same, for
example.

> You can add Based-on: to the cover letter to clarify the dependency, and
> add "RFC" to the subject if the code is not ready to pull. Please look
> at docs/devel/submitting-a-patch.rst for more info.

I've done that now, thanks.
https://patchew.org/QEMU/20240625134931.92279-1-phil@philjordan.eu/

Phil