[PATCH] ui/cocoa: Support hardware cursor interface

Elliot Nunn posted 1 patch 1 year, 8 months ago
Failed in applying to current master (apply log)
ui/cocoa.m | 263 ++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 240 insertions(+), 23 deletions(-)
[PATCH] ui/cocoa: Support hardware cursor interface
Posted by Elliot Nunn 1 year, 8 months ago
Implement dpy_cursor_define() and dpy_mouse_set() on macOS.

The main benefit is from dpy_cursor_define: in absolute pointing mode, the
host can redraw the cursor on the guest's behalf much faster than the guest
can itself.

To provide the programmatic movement expected from a hardware cursor,
dpy_mouse_set is also implemented.

Tricky cases are handled:
- dpy_mouse_set() avoids rounded window corners.
- The sometimes-delay between warping the cursor and an affected mouse-move
  event is accounted for.
- Cursor bitmaps are nearest-neighbor scaled to Retina size.

Signed-off-by: Elliot Nunn <elliot@nunn.io>
---
 ui/cocoa.m | 263 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 240 insertions(+), 23 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 5a8bd5dd84..f9d54448e4 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -85,12 +85,20 @@ static void cocoa_switch(DisplayChangeListener *dcl,
 
 static void cocoa_refresh(DisplayChangeListener *dcl);
 
+static void cocoa_mouse_set(DisplayChangeListener *dcl,
+                            int x, int y, int on);
+
+static void cocoa_cursor_define(DisplayChangeListener *dcl,
+                                QEMUCursor *c);
+
 static NSWindow *normalWindow;
 static const DisplayChangeListenerOps dcl_ops = {
     .dpy_name          = "cocoa",
     .dpy_gfx_update = cocoa_update,
     .dpy_gfx_switch = cocoa_switch,
     .dpy_refresh = cocoa_refresh,
+    .dpy_mouse_set = cocoa_mouse_set,
+    .dpy_cursor_define = cocoa_cursor_define,
 };
 static DisplayChangeListener dcl = {
     .ops = &dcl_ops,
@@ -313,6 +321,13 @@ @interface QemuCocoaView : NSView
     BOOL isFullscreen;
     BOOL isAbsoluteEnabled;
     CFMachPortRef eventsTap;
+    NSCursor *guestCursor;
+    BOOL cursorHiddenByMe;
+    BOOL guestCursorVis;
+    int guestCursorX, guestCursorY;
+    int lastWarpX, lastWarpY;
+    int warpDeltaX, warpDeltaY;
+    BOOL ignoreNextMouseMove;
 }
 - (void) switchSurface:(pixman_image_t *)image;
 - (void) grabMouse;
@@ -323,6 +338,10 @@ - (void) handleMonitorInput:(NSEvent *)event;
 - (bool) handleEvent:(NSEvent *)event;
 - (bool) handleEventLocked:(NSEvent *)event;
 - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
+- (void) cursorDefine:(NSCursor *)cursor;
+- (void) mouseSetX:(int)x Y:(int)y on:(int)on;
+- (void) setCursorAppearance;
+- (void) setCursorPosition;
 /* The state surrounding mouse grabbing is potentially confusing.
  * isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated
  *   pointing device an absolute-position one?"], but is only updated on
@@ -432,22 +451,6 @@ - (CGPoint) screenLocationOfEvent:(NSEvent *)ev
     }
 }
 
-- (void) hideCursor
-{
-    if (!cursor_hide) {
-        return;
-    }
-    [NSCursor hide];
-}
-
-- (void) unhideCursor
-{
-    if (!cursor_hide) {
-        return;
-    }
-    [NSCursor unhide];
-}
-
 - (void) drawRect:(NSRect) rect
 {
     COCOA_DEBUG("QemuCocoaView: drawRect\n");
@@ -635,6 +638,8 @@ - (void) switchSurface:(pixman_image_t *)image
         screen.height = h;
         [self setContentDimensions];
         [self setFrame:NSMakeRect(cx, cy, cw, ch)];
+        [self setCursorAppearance];
+        [self setCursorPosition];
     }
 
     // update screenBuffer
@@ -681,6 +686,7 @@ - (void) toggleFullScreen:(id)sender
             styleMask:NSWindowStyleMaskBorderless
             backing:NSBackingStoreBuffered
             defer:NO];
+        [fullScreenWindow disableCursorRects];
         [fullScreenWindow setAcceptsMouseMovedEvents: YES];
         [fullScreenWindow setHasShadow:NO];
         [fullScreenWindow setBackgroundColor: [NSColor blackColor]];
@@ -812,6 +818,7 @@ - (bool) handleEventLocked:(NSEvent *)event
     int buttons = 0;
     int keycode = 0;
     bool mouse_event = false;
+    bool mousemoved_event = false;
     // Location of event in virtual screen coordinates
     NSPoint p = [self screenLocationOfEvent:event];
     NSUInteger modifiers = [event modifierFlags];
@@ -1023,6 +1030,7 @@ - (bool) handleEventLocked:(NSEvent *)event
                 }
             }
             mouse_event = true;
+            mousemoved_event = true;
             break;
         case NSEventTypeLeftMouseDown:
             buttons |= MOUSE_EVENT_LBUTTON;
@@ -1039,14 +1047,17 @@ - (bool) handleEventLocked:(NSEvent *)event
         case NSEventTypeLeftMouseDragged:
             buttons |= MOUSE_EVENT_LBUTTON;
             mouse_event = true;
+            mousemoved_event = true;
             break;
         case NSEventTypeRightMouseDragged:
             buttons |= MOUSE_EVENT_RBUTTON;
             mouse_event = true;
+            mousemoved_event = true;
             break;
         case NSEventTypeOtherMouseDragged:
             buttons |= MOUSE_EVENT_MBUTTON;
             mouse_event = true;
+            mousemoved_event = true;
             break;
         case NSEventTypeLeftMouseUp:
             mouse_event = true;
@@ -1121,7 +1132,12 @@ - (bool) handleEventLocked:(NSEvent *)event
             qemu_input_update_buttons(dcl.con, bmap, last_buttons, buttons);
             last_buttons = buttons;
         }
-        if (isMouseGrabbed) {
+
+        if (!isMouseGrabbed) {
+            return false;
+        }
+
+        if (mousemoved_event) {
             if (isAbsoluteEnabled) {
                 /* Note that the origin for Cocoa mouse coords is bottom left, not top left.
                  * The check on screenContainsPoint is to avoid sending out of range values for
@@ -1132,11 +1148,38 @@ - (bool) handleEventLocked:(NSEvent *)event
                     qemu_input_queue_abs(dcl.con, INPUT_AXIS_Y, screen.height - p.y, 0, screen.height);
                 }
             } else {
-                qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, (int)[event deltaX]);
-                qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, (int)[event deltaY]);
+                if (ignoreNextMouseMove) {
+                    // Discard the first mouse-move event after a grab, because
+                    // it includes the warp delta from an unknown initial position.
+                    ignoreNextMouseMove = NO;
+                    warpDeltaX = warpDeltaY = 0;
+                } else {
+                    // Correct subsequent events to remove the known warp delta.
+                    // The warp delta is sometimes late to be reported, so never
+                    // allow the delta compensation to alter the direction.
+                    int dX = (int)[event deltaX];
+                    int dY = (int)[event deltaY];
+
+                    if (dX == 0 || (dX ^ (dX - warpDeltaX)) < 0) { // Flipped sign?
+                        warpDeltaX -= dX; // Save excess correction for later
+                        dX = 0;
+                    } else {
+                        dX -= warpDeltaX; // Apply entire correction
+                        warpDeltaX = 0;
+                    }
+
+                    if (dY == 0 || (dY ^ (dY - warpDeltaY)) < 0) {
+                        warpDeltaY -= dY;
+                        dY = 0;
+                    } else {
+                        dY -= warpDeltaY;
+                        warpDeltaY = 0;
+                    }
+
+                    qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, dX);
+                    qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, dY);
+                }
             }
-        } else {
-            return false;
         }
         qemu_input_event_sync();
     }
@@ -1153,9 +1196,15 @@ - (void) grabMouse
         else
             [normalWindow setTitle:@"QEMU - (Press ctrl + alt + g to release Mouse)"];
     }
-    [self hideCursor];
     CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
     isMouseGrabbed = TRUE; // while isMouseGrabbed = TRUE, QemuCocoaApp sends all events to [cocoaView handleEvent:]
+    [self setCursorAppearance];
+    [self setCursorPosition];
+
+    // We took over and warped the mouse, so ignore the next mouse-move
+    if (!isAbsoluteEnabled) {
+        ignoreNextMouseMove = YES;
+    }
 }
 
 - (void) ungrabMouse
@@ -1168,9 +1217,14 @@ - (void) ungrabMouse
         else
             [normalWindow setTitle:@"QEMU"];
     }
-    [self unhideCursor];
     CGAssociateMouseAndMouseCursorPosition(TRUE);
     isMouseGrabbed = FALSE;
+    [self setCursorAppearance];
+
+    if (!isAbsoluteEnabled) {
+        ignoreNextMouseMove = NO;
+        warpDeltaX = warpDeltaY = 0;
+    }
 }
 
 - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
@@ -1179,6 +1233,116 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
         CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
     }
 }
+
+// Indirectly called by dpy_cursor_define() in the virtual GPU
+- (void) cursorDefine:(NSCursor *)cursor {
+    guestCursor = cursor;
+    [self setCursorAppearance];
+}
+
+// Indirectly called by dpy_mouse_set() in the virtual GPU
+- (void) mouseSetX:(int)x Y:(int)y on:(int)on {
+    if (!on != !guestCursorVis) {
+        guestCursorVis = on;
+        [self setCursorAppearance];
+    }
+
+    if (on && (x != guestCursorX || y != guestCursorY)) {
+        guestCursorX = x;
+        guestCursorY = y;
+        [self setCursorPosition];
+    }
+}
+
+// Change the cursor image to the default, the guest cursor bitmap or hidden.
+// Said to be an expensive operation on macOS Monterey, so use sparingly.
+- (void) setCursorAppearance {
+    NSCursor *cursor = NULL; // NULL means hidden
+
+    if (!isMouseGrabbed) {
+        cursor = [NSCursor arrowCursor];
+    } else if (!guestCursor && !cursor_hide) {
+        cursor = [NSCursor arrowCursor];
+    } else if (guestCursorVis && guestCursor) {
+        cursor = guestCursor;
+    } else {
+        cursor = NULL;
+    }
+
+    if (cursor != NULL) {
+        [cursor set];
+
+        if (cursorHiddenByMe) {
+            [NSCursor unhide];
+            cursorHiddenByMe = NO;
+        }
+    } else {
+        if (!cursorHiddenByMe) {
+            [NSCursor hide];
+            cursorHiddenByMe = YES;
+        }
+    }
+}
+
+// Move the cursor within the virtual screen
+- (void) setCursorPosition {
+    // Ignore the guest's request if the cursor belongs to Cocoa
+    if (!isMouseGrabbed || isAbsoluteEnabled) {
+        return;
+    }
+
+    // Get guest screen rect in Cocoa coordinates (bottom-left origin).
+    NSRect virtualScreen = [[self window] convertRectToScreen:[self frame]];
+
+    // Convert to top-left origin.
+    NSInteger hostScreenH = [NSScreen screens][0].frame.size.height;
+    int scrX = virtualScreen.origin.x;
+    int scrY = hostScreenH - virtualScreen.origin.y - virtualScreen.size.height;
+    int scrW = virtualScreen.size.width;
+    int scrH = virtualScreen.size.height;
+
+    int cursX = scrX + guestCursorX;
+    int cursY = scrY + guestCursorY;
+
+    // Clip to edges
+    cursX = MIN(MAX(scrX, cursX), scrX + scrW - 1);
+    cursY = MIN(MAX(scrY, cursY), scrY + scrH - 1);
+
+    // Move diagonally towards the center to avoid rounded window corners.
+    // Limit the number of hit-tests and discard failed attempts.
+    int betterX = cursX, betterY = cursY;
+    for (int i=0; i<16; i++) {
+        if ([NSWindow windowNumberAtPoint:NSMakePoint(betterX, hostScreenH - betterY)
+            belowWindowWithWindowNumber:0] == self.window.windowNumber) {
+            cursX = betterX;
+            cursY = betterY;
+            break;
+        };
+
+        if (betterX < scrX + scrW/2) {
+            betterX++;
+        } else {
+            betterX--;
+        }
+
+        if (betterY < scrY + scrH/2) {
+            betterY++;
+        } else {
+            betterY--;
+        }
+    }
+
+    // Subtract this warp delta from the next NSEventTypeMouseMoved.
+    // These are in down-is-positive coords, same as NSEvent deltaX/deltaY.
+    warpDeltaX += cursX - lastWarpX;
+    warpDeltaY += cursY - lastWarpY;
+
+    CGWarpMouseCursorPosition(NSMakePoint(cursX, cursY));
+
+    lastWarpX = cursX;
+    lastWarpY = cursY;
+}
+
 - (BOOL) isMouseGrabbed {return isMouseGrabbed;}
 - (BOOL) isAbsoluteEnabled {return isAbsoluteEnabled;}
 - (float) cdx {return cdx;}
@@ -1251,6 +1415,7 @@ - (id) init
             error_report("(cocoa) can't create window");
             exit(1);
         }
+        [normalWindow disableCursorRects];
         [normalWindow setAcceptsMouseMovedEvents:YES];
         [normalWindow setTitle:@"QEMU"];
         [normalWindow setContentView:cocoaView];
@@ -2123,6 +2288,58 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
     qemu_clipboard_peer_register(&cbpeer);
 }
 
+static void cocoa_mouse_set(DisplayChangeListener *dcl, int x, int y, int on) {
+    dispatch_async(dispatch_get_main_queue(), ^{
+        [cocoaView mouseSetX:x Y:y on:on];
+    });
+}
+
+// Convert QEMUCursor to NSCursor, then call cursorDefine
+static void cocoa_cursor_define(DisplayChangeListener *dcl, QEMUCursor *cursor) {
+    CFDataRef cfdata = CFDataCreate(
+        /*allocator*/ NULL,
+        /*bytes*/ (void *)cursor->data,
+        /*length*/ sizeof(uint32_t) * cursor->width * cursor->height);
+
+    CGDataProviderRef dataprovider = CGDataProviderCreateWithCFData(cfdata);
+
+    CGImageRef cgimage = CGImageCreate(
+        cursor->width, cursor->height,
+        /*bitsPerComponent*/ 8,
+        /*bitsPerPixel*/ 32,
+        /*bytesPerRow*/ sizeof(uint32_t) * cursor->width,
+        /*colorspace*/ CGColorSpaceCreateWithName(kCGColorSpaceSRGB),
+        /*bitmapInfo*/ kCGBitmapByteOrder32Host | kCGImageAlphaLast,
+        /*provider*/ dataprovider,
+        /*decode*/ NULL,
+        /*shouldInterpolate*/ FALSE,
+        /*intent*/ kCGRenderingIntentDefault);
+
+    NSImage *unscaled = [[NSImage alloc] initWithCGImage:cgimage size:NSZeroSize];
+
+    CFRelease(cfdata);
+    CGDataProviderRelease(dataprovider);
+    CGImageRelease(cgimage);
+
+    // Nearest-neighbor scale to the possibly "Retina" cursor size
+    NSImage *scaled = [NSImage
+        imageWithSize:NSMakeSize(cursor->width, cursor->height)
+        flipped:NO
+        drawingHandler:^BOOL(NSRect dest) {
+            [NSGraphicsContext currentContext].imageInterpolation = NSImageInterpolationNone;
+            [unscaled drawInRect:dest];
+            return YES;
+        }];
+
+    NSCursor *nscursor = [[NSCursor alloc]
+        initWithImage:scaled
+        hotSpot:NSMakePoint(cursor->hot_x, cursor->hot_y)];
+
+    dispatch_async(dispatch_get_main_queue(), ^{
+        [cocoaView cursorDefine:nscursor];
+    });
+}
+
 static QemuDisplay qemu_display_cocoa = {
     .type       = DISPLAY_TYPE_COCOA,
     .init       = cocoa_display_init,
Re: [PATCH] ui/cocoa: Support hardware cursor interface
Posted by Peter Maydell 1 year, 6 months ago
Ccing Akihiko to see if he wants to review this cocoa ui frontend
patch.

also available at:
https://lore.kernel.org/qemu-devel/54930451-d85f-4ce0-9a45-b3478c5a6468@www.fastmail.com/

I can confirm that the patch does build, but I don't have any
interesting graphics-using test images to hand to test with.

thanks
-- PMM

On Thu, 4 Aug 2022 at 07:28, Elliot Nunn <elliot@nunn.io> wrote:
>
> Implement dpy_cursor_define() and dpy_mouse_set() on macOS.
>
> The main benefit is from dpy_cursor_define: in absolute pointing mode, the
> host can redraw the cursor on the guest's behalf much faster than the guest
> can itself.
>
> To provide the programmatic movement expected from a hardware cursor,
> dpy_mouse_set is also implemented.
>
> Tricky cases are handled:
> - dpy_mouse_set() avoids rounded window corners.
> - The sometimes-delay between warping the cursor and an affected mouse-move
>   event is accounted for.
> - Cursor bitmaps are nearest-neighbor scaled to Retina size.
>
> Signed-off-by: Elliot Nunn <elliot@nunn.io>
> ---
>  ui/cocoa.m | 263 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 240 insertions(+), 23 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 5a8bd5dd84..f9d54448e4 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -85,12 +85,20 @@ static void cocoa_switch(DisplayChangeListener *dcl,
>
>  static void cocoa_refresh(DisplayChangeListener *dcl);
>
> +static void cocoa_mouse_set(DisplayChangeListener *dcl,
> +                            int x, int y, int on);
> +
> +static void cocoa_cursor_define(DisplayChangeListener *dcl,
> +                                QEMUCursor *c);
> +
>  static NSWindow *normalWindow;
>  static const DisplayChangeListenerOps dcl_ops = {
>      .dpy_name          = "cocoa",
>      .dpy_gfx_update = cocoa_update,
>      .dpy_gfx_switch = cocoa_switch,
>      .dpy_refresh = cocoa_refresh,
> +    .dpy_mouse_set = cocoa_mouse_set,
> +    .dpy_cursor_define = cocoa_cursor_define,
>  };
>  static DisplayChangeListener dcl = {
>      .ops = &dcl_ops,
> @@ -313,6 +321,13 @@ @interface QemuCocoaView : NSView
>      BOOL isFullscreen;
>      BOOL isAbsoluteEnabled;
>      CFMachPortRef eventsTap;
> +    NSCursor *guestCursor;
> +    BOOL cursorHiddenByMe;
> +    BOOL guestCursorVis;
> +    int guestCursorX, guestCursorY;
> +    int lastWarpX, lastWarpY;
> +    int warpDeltaX, warpDeltaY;
> +    BOOL ignoreNextMouseMove;
>  }
>  - (void) switchSurface:(pixman_image_t *)image;
>  - (void) grabMouse;
> @@ -323,6 +338,10 @@ - (void) handleMonitorInput:(NSEvent *)event;
>  - (bool) handleEvent:(NSEvent *)event;
>  - (bool) handleEventLocked:(NSEvent *)event;
>  - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
> +- (void) cursorDefine:(NSCursor *)cursor;
> +- (void) mouseSetX:(int)x Y:(int)y on:(int)on;
> +- (void) setCursorAppearance;
> +- (void) setCursorPosition;
>  /* The state surrounding mouse grabbing is potentially confusing.
>   * isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated
>   *   pointing device an absolute-position one?"], but is only updated on
> @@ -432,22 +451,6 @@ - (CGPoint) screenLocationOfEvent:(NSEvent *)ev
>      }
>  }
>
> -- (void) hideCursor
> -{
> -    if (!cursor_hide) {
> -        return;
> -    }
> -    [NSCursor hide];
> -}
> -
> -- (void) unhideCursor
> -{
> -    if (!cursor_hide) {
> -        return;
> -    }
> -    [NSCursor unhide];
> -}
> -
>  - (void) drawRect:(NSRect) rect
>  {
>      COCOA_DEBUG("QemuCocoaView: drawRect\n");
> @@ -635,6 +638,8 @@ - (void) switchSurface:(pixman_image_t *)image
>          screen.height = h;
>          [self setContentDimensions];
>          [self setFrame:NSMakeRect(cx, cy, cw, ch)];
> +        [self setCursorAppearance];
> +        [self setCursorPosition];
>      }
>
>      // update screenBuffer
> @@ -681,6 +686,7 @@ - (void) toggleFullScreen:(id)sender
>              styleMask:NSWindowStyleMaskBorderless
>              backing:NSBackingStoreBuffered
>              defer:NO];
> +        [fullScreenWindow disableCursorRects];
>          [fullScreenWindow setAcceptsMouseMovedEvents: YES];
>          [fullScreenWindow setHasShadow:NO];
>          [fullScreenWindow setBackgroundColor: [NSColor blackColor]];
> @@ -812,6 +818,7 @@ - (bool) handleEventLocked:(NSEvent *)event
>      int buttons = 0;
>      int keycode = 0;
>      bool mouse_event = false;
> +    bool mousemoved_event = false;
>      // Location of event in virtual screen coordinates
>      NSPoint p = [self screenLocationOfEvent:event];
>      NSUInteger modifiers = [event modifierFlags];
> @@ -1023,6 +1030,7 @@ - (bool) handleEventLocked:(NSEvent *)event
>                  }
>              }
>              mouse_event = true;
> +            mousemoved_event = true;
>              break;
>          case NSEventTypeLeftMouseDown:
>              buttons |= MOUSE_EVENT_LBUTTON;
> @@ -1039,14 +1047,17 @@ - (bool) handleEventLocked:(NSEvent *)event
>          case NSEventTypeLeftMouseDragged:
>              buttons |= MOUSE_EVENT_LBUTTON;
>              mouse_event = true;
> +            mousemoved_event = true;
>              break;
>          case NSEventTypeRightMouseDragged:
>              buttons |= MOUSE_EVENT_RBUTTON;
>              mouse_event = true;
> +            mousemoved_event = true;
>              break;
>          case NSEventTypeOtherMouseDragged:
>              buttons |= MOUSE_EVENT_MBUTTON;
>              mouse_event = true;
> +            mousemoved_event = true;
>              break;
>          case NSEventTypeLeftMouseUp:
>              mouse_event = true;
> @@ -1121,7 +1132,12 @@ - (bool) handleEventLocked:(NSEvent *)event
>              qemu_input_update_buttons(dcl.con, bmap, last_buttons, buttons);
>              last_buttons = buttons;
>          }
> -        if (isMouseGrabbed) {
> +
> +        if (!isMouseGrabbed) {
> +            return false;
> +        }
> +
> +        if (mousemoved_event) {
>              if (isAbsoluteEnabled) {
>                  /* Note that the origin for Cocoa mouse coords is bottom left, not top left.
>                   * The check on screenContainsPoint is to avoid sending out of range values for
> @@ -1132,11 +1148,38 @@ - (bool) handleEventLocked:(NSEvent *)event
>                      qemu_input_queue_abs(dcl.con, INPUT_AXIS_Y, screen.height - p.y, 0, screen.height);
>                  }
>              } else {
> -                qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, (int)[event deltaX]);
> -                qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, (int)[event deltaY]);
> +                if (ignoreNextMouseMove) {
> +                    // Discard the first mouse-move event after a grab, because
> +                    // it includes the warp delta from an unknown initial position.
> +                    ignoreNextMouseMove = NO;
> +                    warpDeltaX = warpDeltaY = 0;
> +                } else {
> +                    // Correct subsequent events to remove the known warp delta.
> +                    // The warp delta is sometimes late to be reported, so never
> +                    // allow the delta compensation to alter the direction.
> +                    int dX = (int)[event deltaX];
> +                    int dY = (int)[event deltaY];
> +
> +                    if (dX == 0 || (dX ^ (dX - warpDeltaX)) < 0) { // Flipped sign?
> +                        warpDeltaX -= dX; // Save excess correction for later
> +                        dX = 0;
> +                    } else {
> +                        dX -= warpDeltaX; // Apply entire correction
> +                        warpDeltaX = 0;
> +                    }
> +
> +                    if (dY == 0 || (dY ^ (dY - warpDeltaY)) < 0) {
> +                        warpDeltaY -= dY;
> +                        dY = 0;
> +                    } else {
> +                        dY -= warpDeltaY;
> +                        warpDeltaY = 0;
> +                    }
> +
> +                    qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, dX);
> +                    qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, dY);
> +                }
>              }
> -        } else {
> -            return false;
>          }
>          qemu_input_event_sync();
>      }
> @@ -1153,9 +1196,15 @@ - (void) grabMouse
>          else
>              [normalWindow setTitle:@"QEMU - (Press ctrl + alt + g to release Mouse)"];
>      }
> -    [self hideCursor];
>      CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
>      isMouseGrabbed = TRUE; // while isMouseGrabbed = TRUE, QemuCocoaApp sends all events to [cocoaView handleEvent:]
> +    [self setCursorAppearance];
> +    [self setCursorPosition];
> +
> +    // We took over and warped the mouse, so ignore the next mouse-move
> +    if (!isAbsoluteEnabled) {
> +        ignoreNextMouseMove = YES;
> +    }
>  }
>
>  - (void) ungrabMouse
> @@ -1168,9 +1217,14 @@ - (void) ungrabMouse
>          else
>              [normalWindow setTitle:@"QEMU"];
>      }
> -    [self unhideCursor];
>      CGAssociateMouseAndMouseCursorPosition(TRUE);
>      isMouseGrabbed = FALSE;
> +    [self setCursorAppearance];
> +
> +    if (!isAbsoluteEnabled) {
> +        ignoreNextMouseMove = NO;
> +        warpDeltaX = warpDeltaY = 0;
> +    }
>  }
>
>  - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
> @@ -1179,6 +1233,116 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
>          CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
>      }
>  }
> +
> +// Indirectly called by dpy_cursor_define() in the virtual GPU
> +- (void) cursorDefine:(NSCursor *)cursor {
> +    guestCursor = cursor;
> +    [self setCursorAppearance];
> +}
> +
> +// Indirectly called by dpy_mouse_set() in the virtual GPU
> +- (void) mouseSetX:(int)x Y:(int)y on:(int)on {
> +    if (!on != !guestCursorVis) {
> +        guestCursorVis = on;
> +        [self setCursorAppearance];
> +    }
> +
> +    if (on && (x != guestCursorX || y != guestCursorY)) {
> +        guestCursorX = x;
> +        guestCursorY = y;
> +        [self setCursorPosition];
> +    }
> +}
> +
> +// Change the cursor image to the default, the guest cursor bitmap or hidden.
> +// Said to be an expensive operation on macOS Monterey, so use sparingly.
> +- (void) setCursorAppearance {
> +    NSCursor *cursor = NULL; // NULL means hidden
> +
> +    if (!isMouseGrabbed) {
> +        cursor = [NSCursor arrowCursor];
> +    } else if (!guestCursor && !cursor_hide) {
> +        cursor = [NSCursor arrowCursor];
> +    } else if (guestCursorVis && guestCursor) {
> +        cursor = guestCursor;
> +    } else {
> +        cursor = NULL;
> +    }
> +
> +    if (cursor != NULL) {
> +        [cursor set];
> +
> +        if (cursorHiddenByMe) {
> +            [NSCursor unhide];
> +            cursorHiddenByMe = NO;
> +        }
> +    } else {
> +        if (!cursorHiddenByMe) {
> +            [NSCursor hide];
> +            cursorHiddenByMe = YES;
> +        }
> +    }
> +}
> +
> +// Move the cursor within the virtual screen
> +- (void) setCursorPosition {
> +    // Ignore the guest's request if the cursor belongs to Cocoa
> +    if (!isMouseGrabbed || isAbsoluteEnabled) {
> +        return;
> +    }
> +
> +    // Get guest screen rect in Cocoa coordinates (bottom-left origin).
> +    NSRect virtualScreen = [[self window] convertRectToScreen:[self frame]];
> +
> +    // Convert to top-left origin.
> +    NSInteger hostScreenH = [NSScreen screens][0].frame.size.height;
> +    int scrX = virtualScreen.origin.x;
> +    int scrY = hostScreenH - virtualScreen.origin.y - virtualScreen.size.height;
> +    int scrW = virtualScreen.size.width;
> +    int scrH = virtualScreen.size.height;
> +
> +    int cursX = scrX + guestCursorX;
> +    int cursY = scrY + guestCursorY;
> +
> +    // Clip to edges
> +    cursX = MIN(MAX(scrX, cursX), scrX + scrW - 1);
> +    cursY = MIN(MAX(scrY, cursY), scrY + scrH - 1);
> +
> +    // Move diagonally towards the center to avoid rounded window corners.
> +    // Limit the number of hit-tests and discard failed attempts.
> +    int betterX = cursX, betterY = cursY;
> +    for (int i=0; i<16; i++) {
> +        if ([NSWindow windowNumberAtPoint:NSMakePoint(betterX, hostScreenH - betterY)
> +            belowWindowWithWindowNumber:0] == self.window.windowNumber) {
> +            cursX = betterX;
> +            cursY = betterY;
> +            break;
> +        };
> +
> +        if (betterX < scrX + scrW/2) {
> +            betterX++;
> +        } else {
> +            betterX--;
> +        }
> +
> +        if (betterY < scrY + scrH/2) {
> +            betterY++;
> +        } else {
> +            betterY--;
> +        }
> +    }
> +
> +    // Subtract this warp delta from the next NSEventTypeMouseMoved.
> +    // These are in down-is-positive coords, same as NSEvent deltaX/deltaY.
> +    warpDeltaX += cursX - lastWarpX;
> +    warpDeltaY += cursY - lastWarpY;
> +
> +    CGWarpMouseCursorPosition(NSMakePoint(cursX, cursY));
> +
> +    lastWarpX = cursX;
> +    lastWarpY = cursY;
> +}
> +
>  - (BOOL) isMouseGrabbed {return isMouseGrabbed;}
>  - (BOOL) isAbsoluteEnabled {return isAbsoluteEnabled;}
>  - (float) cdx {return cdx;}
> @@ -1251,6 +1415,7 @@ - (id) init
>              error_report("(cocoa) can't create window");
>              exit(1);
>          }
> +        [normalWindow disableCursorRects];
>          [normalWindow setAcceptsMouseMovedEvents:YES];
>          [normalWindow setTitle:@"QEMU"];
>          [normalWindow setContentView:cocoaView];
> @@ -2123,6 +2288,58 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
>      qemu_clipboard_peer_register(&cbpeer);
>  }
>
> +static void cocoa_mouse_set(DisplayChangeListener *dcl, int x, int y, int on) {
> +    dispatch_async(dispatch_get_main_queue(), ^{
> +        [cocoaView mouseSetX:x Y:y on:on];
> +    });
> +}
> +
> +// Convert QEMUCursor to NSCursor, then call cursorDefine
> +static void cocoa_cursor_define(DisplayChangeListener *dcl, QEMUCursor *cursor) {
> +    CFDataRef cfdata = CFDataCreate(
> +        /*allocator*/ NULL,
> +        /*bytes*/ (void *)cursor->data,
> +        /*length*/ sizeof(uint32_t) * cursor->width * cursor->height);
> +
> +    CGDataProviderRef dataprovider = CGDataProviderCreateWithCFData(cfdata);
> +
> +    CGImageRef cgimage = CGImageCreate(
> +        cursor->width, cursor->height,
> +        /*bitsPerComponent*/ 8,
> +        /*bitsPerPixel*/ 32,
> +        /*bytesPerRow*/ sizeof(uint32_t) * cursor->width,
> +        /*colorspace*/ CGColorSpaceCreateWithName(kCGColorSpaceSRGB),
> +        /*bitmapInfo*/ kCGBitmapByteOrder32Host | kCGImageAlphaLast,
> +        /*provider*/ dataprovider,
> +        /*decode*/ NULL,
> +        /*shouldInterpolate*/ FALSE,
> +        /*intent*/ kCGRenderingIntentDefault);
> +
> +    NSImage *unscaled = [[NSImage alloc] initWithCGImage:cgimage size:NSZeroSize];
> +
> +    CFRelease(cfdata);
> +    CGDataProviderRelease(dataprovider);
> +    CGImageRelease(cgimage);
> +
> +    // Nearest-neighbor scale to the possibly "Retina" cursor size
> +    NSImage *scaled = [NSImage
> +        imageWithSize:NSMakeSize(cursor->width, cursor->height)
> +        flipped:NO
> +        drawingHandler:^BOOL(NSRect dest) {
> +            [NSGraphicsContext currentContext].imageInterpolation = NSImageInterpolationNone;
> +            [unscaled drawInRect:dest];
> +            return YES;
> +        }];
> +
> +    NSCursor *nscursor = [[NSCursor alloc]
> +        initWithImage:scaled
> +        hotSpot:NSMakePoint(cursor->hot_x, cursor->hot_y)];
> +
> +    dispatch_async(dispatch_get_main_queue(), ^{
> +        [cocoaView cursorDefine:nscursor];
> +    });
> +}
> +
>  static QemuDisplay qemu_display_cocoa = {
>      .type       = DISPLAY_TYPE_COCOA,
>      .init       = cocoa_display_init,
Re: [PATCH] ui/cocoa: Support hardware cursor interface
Posted by Akihiko Odaki 1 year, 6 months ago
Thanks Peter and Elliot,

Unfortunately Patchew seems to have failed to apply the patch to the
current master. It would be nice if you rebase it to the current
master.

Actually I have a patch to add hardware support to ui/cocoa, but I
have not submitted to the mailing list because it depends on a number
of other patches:
https://github.com/akihikodaki/qemu/commit/34199fcd4080ce8c705b46df26fdf02966b1610c

My patch avoided using CGWarpMouseCursorPosition because of its
quirks. I'd like to test your patch by myself to see if it avoids them
properly for my own workloads.

I have also added some comments to the patch. Please see the below.

Regards,
Akihiko Odaki

On Wed, Oct 5, 2022 at 12:39 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Ccing Akihiko to see if he wants to review this cocoa ui frontend
> patch.
>
> also available at:
> https://lore.kernel.org/qemu-devel/54930451-d85f-4ce0-9a45-b3478c5a6468@www.fastmail.com/
>
> I can confirm that the patch does build, but I don't have any
> interesting graphics-using test images to hand to test with.
>
> thanks
> -- PMM
>
> On Thu, 4 Aug 2022 at 07:28, Elliot Nunn <elliot@nunn.io> wrote:
> >
> > Implement dpy_cursor_define() and dpy_mouse_set() on macOS.
> >
> > The main benefit is from dpy_cursor_define: in absolute pointing mode, the
> > host can redraw the cursor on the guest's behalf much faster than the guest
> > can itself.
> >
> > To provide the programmatic movement expected from a hardware cursor,
> > dpy_mouse_set is also implemented.
> >
> > Tricky cases are handled:
> > - dpy_mouse_set() avoids rounded window corners.
> > - The sometimes-delay between warping the cursor and an affected mouse-move
> >   event is accounted for.
> > - Cursor bitmaps are nearest-neighbor scaled to Retina size.
> >
> > Signed-off-by: Elliot Nunn <elliot@nunn.io>
> > ---
> >  ui/cocoa.m | 263 ++++++++++++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 240 insertions(+), 23 deletions(-)
> >
> > diff --git a/ui/cocoa.m b/ui/cocoa.m
> > index 5a8bd5dd84..f9d54448e4 100644
> > --- a/ui/cocoa.m
> > +++ b/ui/cocoa.m
> > @@ -85,12 +85,20 @@ static void cocoa_switch(DisplayChangeListener *dcl,
> >
> >  static void cocoa_refresh(DisplayChangeListener *dcl);
> >
> > +static void cocoa_mouse_set(DisplayChangeListener *dcl,
> > +                            int x, int y, int on);
> > +
> > +static void cocoa_cursor_define(DisplayChangeListener *dcl,
> > +                                QEMUCursor *c);
> > +
> >  static NSWindow *normalWindow;
> >  static const DisplayChangeListenerOps dcl_ops = {
> >      .dpy_name          = "cocoa",
> >      .dpy_gfx_update = cocoa_update,
> >      .dpy_gfx_switch = cocoa_switch,
> >      .dpy_refresh = cocoa_refresh,
> > +    .dpy_mouse_set = cocoa_mouse_set,
> > +    .dpy_cursor_define = cocoa_cursor_define,
> >  };
> >  static DisplayChangeListener dcl = {
> >      .ops = &dcl_ops,
> > @@ -313,6 +321,13 @@ @interface QemuCocoaView : NSView
> >      BOOL isFullscreen;
> >      BOOL isAbsoluteEnabled;
> >      CFMachPortRef eventsTap;
> > +    NSCursor *guestCursor;
> > +    BOOL cursorHiddenByMe;

Who is "Me" here?

> > +    BOOL guestCursorVis;
> > +    int guestCursorX, guestCursorY;
> > +    int lastWarpX, lastWarpY;
> > +    int warpDeltaX, warpDeltaY;
> > +    BOOL ignoreNextMouseMove;
> >  }
> >  - (void) switchSurface:(pixman_image_t *)image;
> >  - (void) grabMouse;
> > @@ -323,6 +338,10 @@ - (void) handleMonitorInput:(NSEvent *)event;
> >  - (bool) handleEvent:(NSEvent *)event;
> >  - (bool) handleEventLocked:(NSEvent *)event;
> >  - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
> > +- (void) cursorDefine:(NSCursor *)cursor;
> > +- (void) mouseSetX:(int)x Y:(int)y on:(int)on;
> > +- (void) setCursorAppearance;
> > +- (void) setCursorPosition;
> >  /* The state surrounding mouse grabbing is potentially confusing.
> >   * isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated
> >   *   pointing device an absolute-position one?"], but is only updated on
> > @@ -432,22 +451,6 @@ - (CGPoint) screenLocationOfEvent:(NSEvent *)ev
> >      }
> >  }
> >
> > -- (void) hideCursor
> > -{
> > -    if (!cursor_hide) {
> > -        return;
> > -    }
> > -    [NSCursor hide];
> > -}
> > -
> > -- (void) unhideCursor
> > -{
> > -    if (!cursor_hide) {
> > -        return;
> > -    }
> > -    [NSCursor unhide];
> > -}
> > -
> >  - (void) drawRect:(NSRect) rect
> >  {
> >      COCOA_DEBUG("QemuCocoaView: drawRect\n");
> > @@ -635,6 +638,8 @@ - (void) switchSurface:(pixman_image_t *)image
> >          screen.height = h;
> >          [self setContentDimensions];
> >          [self setFrame:NSMakeRect(cx, cy, cw, ch)];
> > +        [self setCursorAppearance];

Why do you need to set cursor appearance when resizing the screen?

> > +        [self setCursorPosition];
> >      }
> >
> >      // update screenBuffer
> > @@ -681,6 +686,7 @@ - (void) toggleFullScreen:(id)sender
> >              styleMask:NSWindowStyleMaskBorderless
> >              backing:NSBackingStoreBuffered
> >              defer:NO];
> > +        [fullScreenWindow disableCursorRects];
> >          [fullScreenWindow setAcceptsMouseMovedEvents: YES];
> >          [fullScreenWindow setHasShadow:NO];
> >          [fullScreenWindow setBackgroundColor: [NSColor blackColor]];
> > @@ -812,6 +818,7 @@ - (bool) handleEventLocked:(NSEvent *)event
> >      int buttons = 0;
> >      int keycode = 0;
> >      bool mouse_event = false;
> > +    bool mousemoved_event = false;
> >      // Location of event in virtual screen coordinates
> >      NSPoint p = [self screenLocationOfEvent:event];
> >      NSUInteger modifiers = [event modifierFlags];
> > @@ -1023,6 +1030,7 @@ - (bool) handleEventLocked:(NSEvent *)event
> >                  }
> >              }
> >              mouse_event = true;
> > +            mousemoved_event = true;
> >              break;
> >          case NSEventTypeLeftMouseDown:
> >              buttons |= MOUSE_EVENT_LBUTTON;
> > @@ -1039,14 +1047,17 @@ - (bool) handleEventLocked:(NSEvent *)event
> >          case NSEventTypeLeftMouseDragged:
> >              buttons |= MOUSE_EVENT_LBUTTON;
> >              mouse_event = true;
> > +            mousemoved_event = true;
> >              break;
> >          case NSEventTypeRightMouseDragged:
> >              buttons |= MOUSE_EVENT_RBUTTON;
> >              mouse_event = true;
> > +            mousemoved_event = true;
> >              break;
> >          case NSEventTypeOtherMouseDragged:
> >              buttons |= MOUSE_EVENT_MBUTTON;
> >              mouse_event = true;
> > +            mousemoved_event = true;
> >              break;
> >          case NSEventTypeLeftMouseUp:
> >              mouse_event = true;
> > @@ -1121,7 +1132,12 @@ - (bool) handleEventLocked:(NSEvent *)event
> >              qemu_input_update_buttons(dcl.con, bmap, last_buttons, buttons);
> >              last_buttons = buttons;
> >          }
> > -        if (isMouseGrabbed) {
> > +
> > +        if (!isMouseGrabbed) {
> > +            return false;
> > +        }
> > +
> > +        if (mousemoved_event) {
> >              if (isAbsoluteEnabled) {
> >                  /* Note that the origin for Cocoa mouse coords is bottom left, not top left.
> >                   * The check on screenContainsPoint is to avoid sending out of range values for
> > @@ -1132,11 +1148,38 @@ - (bool) handleEventLocked:(NSEvent *)event
> >                      qemu_input_queue_abs(dcl.con, INPUT_AXIS_Y, screen.height - p.y, 0, screen.height);
> >                  }
> >              } else {
> > -                qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, (int)[event deltaX]);
> > -                qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, (int)[event deltaY]);
> > +                if (ignoreNextMouseMove) {
> > +                    // Discard the first mouse-move event after a grab, because
> > +                    // it includes the warp delta from an unknown initial position.
> > +                    ignoreNextMouseMove = NO;
> > +                    warpDeltaX = warpDeltaY = 0;
> > +                } else {
> > +                    // Correct subsequent events to remove the known warp delta.
> > +                    // The warp delta is sometimes late to be reported, so never
> > +                    // allow the delta compensation to alter the direction.
> > +                    int dX = (int)[event deltaX];
> > +                    int dY = (int)[event deltaY];
> > +
> > +                    if (dX == 0 || (dX ^ (dX - warpDeltaX)) < 0) { // Flipped sign?

Instead, do: (dx < 0) == (dx - warpDeltaX < 0). The original flipped
sign check is dependent on an implementation-defined behavior, and a
bit difficult to understand. A decent compiler should be able to
optimize it to the bitwise operation.

> > +                        warpDeltaX -= dX; // Save excess correction for later
> > +                        dX = 0;
> > +                    } else {
> > +                        dX -= warpDeltaX; // Apply entire correction
> > +                        warpDeltaX = 0;
> > +                    }
> > +
> > +                    if (dY == 0 || (dY ^ (dY - warpDeltaY)) < 0) {
> > +                        warpDeltaY -= dY;
> > +                        dY = 0;
> > +                    } else {
> > +                        dY -= warpDeltaY;
> > +                        warpDeltaY = 0;
> > +                    }
> > +
> > +                    qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, dX);
> > +                    qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, dY);
> > +                }
> >              }
> > -        } else {
> > -            return false;
> >          }
> >          qemu_input_event_sync();
> >      }
> > @@ -1153,9 +1196,15 @@ - (void) grabMouse
> >          else
> >              [normalWindow setTitle:@"QEMU - (Press ctrl + alt + g to release Mouse)"];
> >      }
> > -    [self hideCursor];
> >      CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
> >      isMouseGrabbed = TRUE; // while isMouseGrabbed = TRUE, QemuCocoaApp sends all events to [cocoaView handleEvent:]
> > +    [self setCursorAppearance];
> > +    [self setCursorPosition];
> > +
> > +    // We took over and warped the mouse, so ignore the next mouse-move
> > +    if (!isAbsoluteEnabled) {
> > +        ignoreNextMouseMove = YES;
> > +    }

It shouldn't warp the mouse when the pointing device is absolute. An
absolute pointing device, especially vdagent, is often used to
seamlessly integrate the guest and host cursors.

> >  }
> >
> >  - (void) ungrabMouse
> > @@ -1168,9 +1217,14 @@ - (void) ungrabMouse
> >          else
> >              [normalWindow setTitle:@"QEMU"];
> >      }
> > -    [self unhideCursor];
> >      CGAssociateMouseAndMouseCursorPosition(TRUE);
> >      isMouseGrabbed = FALSE;
> > +    [self setCursorAppearance];
> > +
> > +    if (!isAbsoluteEnabled) {
> > +        ignoreNextMouseMove = NO;
> > +        warpDeltaX = warpDeltaY = 0;
> > +    }
> >  }
> >
> >  - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
> > @@ -1179,6 +1233,116 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
> >          CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
> >      }
> >  }
> > +
> > +// Indirectly called by dpy_cursor_define() in the virtual GPU
> > +- (void) cursorDefine:(NSCursor *)cursor {
> > +    guestCursor = cursor;

The old cursor is leaked here. Note that ARC is not enabled on QEMU,
unfortunately.

> > +    [self setCursorAppearance];
> > +}
> > +
> > +// Indirectly called by dpy_mouse_set() in the virtual GPU
> > +- (void) mouseSetX:(int)x Y:(int)y on:(int)on {
> > +    if (!on != !guestCursorVis) {
> > +        guestCursorVis = on;
> > +        [self setCursorAppearance];
> > +    }
> > +
> > +    if (on && (x != guestCursorX || y != guestCursorY)) {
> > +        guestCursorX = x;
> > +        guestCursorY = y;
> > +        [self setCursorPosition];
> > +    }
> > +}
> > +
> > +// Change the cursor image to the default, the guest cursor bitmap or hidden.
> > +// Said to be an expensive operation on macOS Monterey, so use sparingly.
> > +- (void) setCursorAppearance {
> > +    NSCursor *cursor = NULL; // NULL means hidden
> > +
> > +    if (!isMouseGrabbed) {
> > +        cursor = [NSCursor arrowCursor];
> > +    } else if (!guestCursor && !cursor_hide) {
> > +        cursor = [NSCursor arrowCursor];
> > +    } else if (guestCursorVis && guestCursor) {
> > +        cursor = guestCursor;
> > +    } else {
> > +        cursor = NULL;
> > +    }
> > +
> > +    if (cursor != NULL) {
> > +        [cursor set];
> > +
> > +        if (cursorHiddenByMe) {
> > +            [NSCursor unhide];
> > +            cursorHiddenByMe = NO;
> > +        }
> > +    } else {
> > +        if (!cursorHiddenByMe) {
> > +            [NSCursor hide];
> > +            cursorHiddenByMe = YES;
> > +        }
> > +    }
> > +}
> > +
> > +// Move the cursor within the virtual screen
> > +- (void) setCursorPosition {
> > +    // Ignore the guest's request if the cursor belongs to Cocoa
> > +    if (!isMouseGrabbed || isAbsoluteEnabled) {
> > +        return;
> > +    }
> > +
> > +    // Get guest screen rect in Cocoa coordinates (bottom-left origin).
> > +    NSRect virtualScreen = [[self window] convertRectToScreen:[self frame]];
> > +
> > +    // Convert to top-left origin.
> > +    NSInteger hostScreenH = [NSScreen screens][0].frame.size.height;
> > +    int scrX = virtualScreen.origin.x;
> > +    int scrY = hostScreenH - virtualScreen.origin.y - virtualScreen.size.height;
> > +    int scrW = virtualScreen.size.width;
> > +    int scrH = virtualScreen.size.height;
> > +
> > +    int cursX = scrX + guestCursorX;
> > +    int cursY = scrY + guestCursorY;
> > +
> > +    // Clip to edges
> > +    cursX = MIN(MAX(scrX, cursX), scrX + scrW - 1);
> > +    cursY = MIN(MAX(scrY, cursY), scrY + scrH - 1);
> > +
> > +    // Move diagonally towards the center to avoid rounded window corners.
> > +    // Limit the number of hit-tests and discard failed attempts.
> > +    int betterX = cursX, betterY = cursY;
> > +    for (int i=0; i<16; i++) {
> > +        if ([NSWindow windowNumberAtPoint:NSMakePoint(betterX, hostScreenH - betterY)
> > +            belowWindowWithWindowNumber:0] == self.window.windowNumber) {
> > +            cursX = betterX;
> > +            cursY = betterY;
> > +            break;
> > +        };
> > +
> > +        if (betterX < scrX + scrW/2) {
> > +            betterX++;
> > +        } else {
> > +            betterX--;
> > +        }
> > +
> > +        if (betterY < scrY + scrH/2) {
> > +            betterY++;
> > +        } else {
> > +            betterY--;
> > +        }
> > +    }
> > +
> > +    // Subtract this warp delta from the next NSEventTypeMouseMoved.
> > +    // These are in down-is-positive coords, same as NSEvent deltaX/deltaY.
> > +    warpDeltaX += cursX - lastWarpX;
> > +    warpDeltaY += cursY - lastWarpY;
> > +
> > +    CGWarpMouseCursorPosition(NSMakePoint(cursX, cursY));
> > +
> > +    lastWarpX = cursX;
> > +    lastWarpY = cursY;
> > +}
> > +
> >  - (BOOL) isMouseGrabbed {return isMouseGrabbed;}
> >  - (BOOL) isAbsoluteEnabled {return isAbsoluteEnabled;}
> >  - (float) cdx {return cdx;}
> > @@ -1251,6 +1415,7 @@ - (id) init
> >              error_report("(cocoa) can't create window");
> >              exit(1);
> >          }
> > +        [normalWindow disableCursorRects];
> >          [normalWindow setAcceptsMouseMovedEvents:YES];
> >          [normalWindow setTitle:@"QEMU"];
> >          [normalWindow setContentView:cocoaView];
> > @@ -2123,6 +2288,58 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
> >      qemu_clipboard_peer_register(&cbpeer);
> >  }
> >
> > +static void cocoa_mouse_set(DisplayChangeListener *dcl, int x, int y, int on) {

Put { for a function in a new line. See docs/devel/style.rst.

> > +    dispatch_async(dispatch_get_main_queue(), ^{
> > +        [cocoaView mouseSetX:x Y:y on:on];
> > +    });
> > +}
> > +
> > +// Convert QEMUCursor to NSCursor, then call cursorDefine
> > +static void cocoa_cursor_define(DisplayChangeListener *dcl, QEMUCursor *cursor) {
> > +    CFDataRef cfdata = CFDataCreate(
> > +        /*allocator*/ NULL,
> > +        /*bytes*/ (void *)cursor->data,
> > +        /*length*/ sizeof(uint32_t) * cursor->width * cursor->height);
> > +
> > +    CGDataProviderRef dataprovider = CGDataProviderCreateWithCFData(cfdata);
> > +
> > +    CGImageRef cgimage = CGImageCreate(
> > +        cursor->width, cursor->height,
> > +        /*bitsPerComponent*/ 8,
> > +        /*bitsPerPixel*/ 32,
> > +        /*bytesPerRow*/ sizeof(uint32_t) * cursor->width,
> > +        /*colorspace*/ CGColorSpaceCreateWithName(kCGColorSpaceSRGB),
> > +        /*bitmapInfo*/ kCGBitmapByteOrder32Host | kCGImageAlphaLast,
> > +        /*provider*/ dataprovider,
> > +        /*decode*/ NULL,
> > +        /*shouldInterpolate*/ FALSE,
> > +        /*intent*/ kCGRenderingIntentDefault);
> > +
> > +    NSImage *unscaled = [[NSImage alloc] initWithCGImage:cgimage size:NSZeroSize];
> > +
> > +    CFRelease(cfdata);
> > +    CGDataProviderRelease(dataprovider);
> > +    CGImageRelease(cgimage);
> > +
> > +    // Nearest-neighbor scale to the possibly "Retina" cursor size
> > +    NSImage *scaled = [NSImage
> > +        imageWithSize:NSMakeSize(cursor->width, cursor->height)
> > +        flipped:NO
> > +        drawingHandler:^BOOL(NSRect dest) {
> > +            [NSGraphicsContext currentContext].imageInterpolation = NSImageInterpolationNone;
> > +            [unscaled drawInRect:dest];
> > +            return YES;
> > +        }];

unscaled and scaled are leaked.

> > +
> > +    NSCursor *nscursor = [[NSCursor alloc]
> > +        initWithImage:scaled
> > +        hotSpot:NSMakePoint(cursor->hot_x, cursor->hot_y)];
> > +
> > +    dispatch_async(dispatch_get_main_queue(), ^{
> > +        [cocoaView cursorDefine:nscursor];
> > +    });
> > +}
> > +
> >  static QemuDisplay qemu_display_cocoa = {
> >      .type       = DISPLAY_TYPE_COCOA,
> >      .init       = cocoa_display_init,
Re: [PATCH] ui/cocoa: Support hardware cursor interface
Posted by Elliot Nunn 1 year, 5 months ago
Akihiko,

Thanks very much for reviewing my patch.

I think that you were right to use the sprite-within-a-window approach,
and avoid warping the OS cursor. I tried to compensate for the error
that cursor warping causes in the subsequent mouse event, but there is
still some error getting through that makes the cursor feel "janky".

But in absolute pointing mode, will it be possible to remove the guest's
code path from visual updates of the cursor? I find that under Mac OS 9,
this provides better responsiveness. I can think of two methods:

1. In absolute pointing mode, re-enable Cocoa's cursor and let the host
OS move it according to user input.

2. Keep the cursor sprite, but move it according to Cocoa's mouse
movement events instead of dpy_mouse_set events.

I prefer option 2. What do you think?

And I didn't realise that you had added VirGL support to cocoa.m. Well
done! Is it on track for release?

My patch should be withdrawn from consideration, in favour of a future
solution that does not use cursor warping.

Many thanks,

Elliot

> On 6 Oct 2022, at 8:15 pm, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> 
> Thanks Peter and Elliot,
> 
> Unfortunately Patchew seems to have failed to apply the patch to the
> current master. It would be nice if you rebase it to the current
> master.
> 
> Actually I have a patch to add hardware support to ui/cocoa, but I
> have not submitted to the mailing list because it depends on a number
> of other patches:
> https://github.com/akihikodaki/qemu/commit/34199fcd4080ce8c705b46df26fdf02966b1610c
> 
> My patch avoided using CGWarpMouseCursorPosition because of its
> quirks. I'd like to test your patch by myself to see if it avoids them
> properly for my own workloads.
> 
> I have also added some comments to the patch. Please see the below.
> 
> Regards,
> Akihiko Odaki
> 
> On Wed, Oct 5, 2022 at 12:39 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>> 
>> Ccing Akihiko to see if he wants to review this cocoa ui frontend
>> patch.
>> 
>> also available at:
>> https://lore.kernel.org/qemu-devel/54930451-d85f-4ce0-9a45-b3478c5a6468@www.fastmail.com/
>> 
>> I can confirm that the patch does build, but I don't have any
>> interesting graphics-using test images to hand to test with.
>> 
>> thanks
>> -- PMM
>> 
>> On Thu, 4 Aug 2022 at 07:28, Elliot Nunn <elliot@nunn.io> wrote:
>>> 
>>> Implement dpy_cursor_define() and dpy_mouse_set() on macOS.
>>> 
>>> The main benefit is from dpy_cursor_define: in absolute pointing mode, the
>>> host can redraw the cursor on the guest's behalf much faster than the guest
>>> can itself.
>>> 
>>> To provide the programmatic movement expected from a hardware cursor,
>>> dpy_mouse_set is also implemented.
>>> 
>>> Tricky cases are handled:
>>> - dpy_mouse_set() avoids rounded window corners.
>>> - The sometimes-delay between warping the cursor and an affected mouse-move
>>>  event is accounted for.
>>> - Cursor bitmaps are nearest-neighbor scaled to Retina size.
>>> 
>>> Signed-off-by: Elliot Nunn <elliot@nunn.io>
>>> ---
>>> ui/cocoa.m | 263 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>>> 1 file changed, 240 insertions(+), 23 deletions(-)
>>> 
>>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>>> index 5a8bd5dd84..f9d54448e4 100644
>>> --- a/ui/cocoa.m
>>> +++ b/ui/cocoa.m
>>> @@ -85,12 +85,20 @@ static void cocoa_switch(DisplayChangeListener *dcl,
>>> 
>>> static void cocoa_refresh(DisplayChangeListener *dcl);
>>> 
>>> +static void cocoa_mouse_set(DisplayChangeListener *dcl,
>>> +                            int x, int y, int on);
>>> +
>>> +static void cocoa_cursor_define(DisplayChangeListener *dcl,
>>> +                                QEMUCursor *c);
>>> +
>>> static NSWindow *normalWindow;
>>> static const DisplayChangeListenerOps dcl_ops = {
>>>     .dpy_name          = "cocoa",
>>>     .dpy_gfx_update = cocoa_update,
>>>     .dpy_gfx_switch = cocoa_switch,
>>>     .dpy_refresh = cocoa_refresh,
>>> +    .dpy_mouse_set = cocoa_mouse_set,
>>> +    .dpy_cursor_define = cocoa_cursor_define,
>>> };
>>> static DisplayChangeListener dcl = {
>>>     .ops = &dcl_ops,
>>> @@ -313,6 +321,13 @@ @interface QemuCocoaView : NSView
>>>     BOOL isFullscreen;
>>>     BOOL isAbsoluteEnabled;
>>>     CFMachPortRef eventsTap;
>>> +    NSCursor *guestCursor;
>>> +    BOOL cursorHiddenByMe;
> 
> Who is "Me" here?
> 
>>> +    BOOL guestCursorVis;
>>> +    int guestCursorX, guestCursorY;
>>> +    int lastWarpX, lastWarpY;
>>> +    int warpDeltaX, warpDeltaY;
>>> +    BOOL ignoreNextMouseMove;
>>> }
>>> - (void) switchSurface:(pixman_image_t *)image;
>>> - (void) grabMouse;
>>> @@ -323,6 +338,10 @@ - (void) handleMonitorInput:(NSEvent *)event;
>>> - (bool) handleEvent:(NSEvent *)event;
>>> - (bool) handleEventLocked:(NSEvent *)event;
>>> - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
>>> +- (void) cursorDefine:(NSCursor *)cursor;
>>> +- (void) mouseSetX:(int)x Y:(int)y on:(int)on;
>>> +- (void) setCursorAppearance;
>>> +- (void) setCursorPosition;
>>> /* The state surrounding mouse grabbing is potentially confusing.
>>>  * isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated
>>>  *   pointing device an absolute-position one?"], but is only updated on
>>> @@ -432,22 +451,6 @@ - (CGPoint) screenLocationOfEvent:(NSEvent *)ev
>>>     }
>>> }
>>> 
>>> -- (void) hideCursor
>>> -{
>>> -    if (!cursor_hide) {
>>> -        return;
>>> -    }
>>> -    [NSCursor hide];
>>> -}
>>> -
>>> -- (void) unhideCursor
>>> -{
>>> -    if (!cursor_hide) {
>>> -        return;
>>> -    }
>>> -    [NSCursor unhide];
>>> -}
>>> -
>>> - (void) drawRect:(NSRect) rect
>>> {
>>>     COCOA_DEBUG("QemuCocoaView: drawRect\n");
>>> @@ -635,6 +638,8 @@ - (void) switchSurface:(pixman_image_t *)image
>>>         screen.height = h;
>>>         [self setContentDimensions];
>>>         [self setFrame:NSMakeRect(cx, cy, cw, ch)];
>>> +        [self setCursorAppearance];
> 
> Why do you need to set cursor appearance when resizing the screen?
> 
>>> +        [self setCursorPosition];
>>>     }
>>> 
>>>     // update screenBuffer
>>> @@ -681,6 +686,7 @@ - (void) toggleFullScreen:(id)sender
>>>             styleMask:NSWindowStyleMaskBorderless
>>>             backing:NSBackingStoreBuffered
>>>             defer:NO];
>>> +        [fullScreenWindow disableCursorRects];
>>>         [fullScreenWindow setAcceptsMouseMovedEvents: YES];
>>>         [fullScreenWindow setHasShadow:NO];
>>>         [fullScreenWindow setBackgroundColor: [NSColor blackColor]];
>>> @@ -812,6 +818,7 @@ - (bool) handleEventLocked:(NSEvent *)event
>>>     int buttons = 0;
>>>     int keycode = 0;
>>>     bool mouse_event = false;
>>> +    bool mousemoved_event = false;
>>>     // Location of event in virtual screen coordinates
>>>     NSPoint p = [self screenLocationOfEvent:event];
>>>     NSUInteger modifiers = [event modifierFlags];
>>> @@ -1023,6 +1030,7 @@ - (bool) handleEventLocked:(NSEvent *)event
>>>                 }
>>>             }
>>>             mouse_event = true;
>>> +            mousemoved_event = true;
>>>             break;
>>>         case NSEventTypeLeftMouseDown:
>>>             buttons |= MOUSE_EVENT_LBUTTON;
>>> @@ -1039,14 +1047,17 @@ - (bool) handleEventLocked:(NSEvent *)event
>>>         case NSEventTypeLeftMouseDragged:
>>>             buttons |= MOUSE_EVENT_LBUTTON;
>>>             mouse_event = true;
>>> +            mousemoved_event = true;
>>>             break;
>>>         case NSEventTypeRightMouseDragged:
>>>             buttons |= MOUSE_EVENT_RBUTTON;
>>>             mouse_event = true;
>>> +            mousemoved_event = true;
>>>             break;
>>>         case NSEventTypeOtherMouseDragged:
>>>             buttons |= MOUSE_EVENT_MBUTTON;
>>>             mouse_event = true;
>>> +            mousemoved_event = true;
>>>             break;
>>>         case NSEventTypeLeftMouseUp:
>>>             mouse_event = true;
>>> @@ -1121,7 +1132,12 @@ - (bool) handleEventLocked:(NSEvent *)event
>>>             qemu_input_update_buttons(dcl.con, bmap, last_buttons, buttons);
>>>             last_buttons = buttons;
>>>         }
>>> -        if (isMouseGrabbed) {
>>> +
>>> +        if (!isMouseGrabbed) {
>>> +            return false;
>>> +        }
>>> +
>>> +        if (mousemoved_event) {
>>>             if (isAbsoluteEnabled) {
>>>                 /* Note that the origin for Cocoa mouse coords is bottom left, not top left.
>>>                  * The check on screenContainsPoint is to avoid sending out of range values for
>>> @@ -1132,11 +1148,38 @@ - (bool) handleEventLocked:(NSEvent *)event
>>>                     qemu_input_queue_abs(dcl.con, INPUT_AXIS_Y, screen.height - p.y, 0, screen.height);
>>>                 }
>>>             } else {
>>> -                qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, (int)[event deltaX]);
>>> -                qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, (int)[event deltaY]);
>>> +                if (ignoreNextMouseMove) {
>>> +                    // Discard the first mouse-move event after a grab, because
>>> +                    // it includes the warp delta from an unknown initial position.
>>> +                    ignoreNextMouseMove = NO;
>>> +                    warpDeltaX = warpDeltaY = 0;
>>> +                } else {
>>> +                    // Correct subsequent events to remove the known warp delta.
>>> +                    // The warp delta is sometimes late to be reported, so never
>>> +                    // allow the delta compensation to alter the direction.
>>> +                    int dX = (int)[event deltaX];
>>> +                    int dY = (int)[event deltaY];
>>> +
>>> +                    if (dX == 0 || (dX ^ (dX - warpDeltaX)) < 0) { // Flipped sign?
> 
> Instead, do: (dx < 0) == (dx - warpDeltaX < 0). The original flipped
> sign check is dependent on an implementation-defined behavior, and a
> bit difficult to understand. A decent compiler should be able to
> optimize it to the bitwise operation.
> 
>>> +                        warpDeltaX -= dX; // Save excess correction for later
>>> +                        dX = 0;
>>> +                    } else {
>>> +                        dX -= warpDeltaX; // Apply entire correction
>>> +                        warpDeltaX = 0;
>>> +                    }
>>> +
>>> +                    if (dY == 0 || (dY ^ (dY - warpDeltaY)) < 0) {
>>> +                        warpDeltaY -= dY;
>>> +                        dY = 0;
>>> +                    } else {
>>> +                        dY -= warpDeltaY;
>>> +                        warpDeltaY = 0;
>>> +                    }
>>> +
>>> +                    qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, dX);
>>> +                    qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, dY);
>>> +                }
>>>             }
>>> -        } else {
>>> -            return false;
>>>         }
>>>         qemu_input_event_sync();
>>>     }
>>> @@ -1153,9 +1196,15 @@ - (void) grabMouse
>>>         else
>>>             [normalWindow setTitle:@"QEMU - (Press ctrl + alt + g to release Mouse)"];
>>>     }
>>> -    [self hideCursor];
>>>     CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
>>>     isMouseGrabbed = TRUE; // while isMouseGrabbed = TRUE, QemuCocoaApp sends all events to [cocoaView handleEvent:]
>>> +    [self setCursorAppearance];
>>> +    [self setCursorPosition];
>>> +
>>> +    // We took over and warped the mouse, so ignore the next mouse-move
>>> +    if (!isAbsoluteEnabled) {
>>> +        ignoreNextMouseMove = YES;
>>> +    }
> 
> It shouldn't warp the mouse when the pointing device is absolute. An
> absolute pointing device, especially vdagent, is often used to
> seamlessly integrate the guest and host cursors.
> 
>>> }
>>> 
>>> - (void) ungrabMouse
>>> @@ -1168,9 +1217,14 @@ - (void) ungrabMouse
>>>         else
>>>             [normalWindow setTitle:@"QEMU"];
>>>     }
>>> -    [self unhideCursor];
>>>     CGAssociateMouseAndMouseCursorPosition(TRUE);
>>>     isMouseGrabbed = FALSE;
>>> +    [self setCursorAppearance];
>>> +
>>> +    if (!isAbsoluteEnabled) {
>>> +        ignoreNextMouseMove = NO;
>>> +        warpDeltaX = warpDeltaY = 0;
>>> +    }
>>> }
>>> 
>>> - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
>>> @@ -1179,6 +1233,116 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
>>>         CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
>>>     }
>>> }
>>> +
>>> +// Indirectly called by dpy_cursor_define() in the virtual GPU
>>> +- (void) cursorDefine:(NSCursor *)cursor {
>>> +    guestCursor = cursor;
> 
> The old cursor is leaked here. Note that ARC is not enabled on QEMU,
> unfortunately.
> 
>>> +    [self setCursorAppearance];
>>> +}
>>> +
>>> +// Indirectly called by dpy_mouse_set() in the virtual GPU
>>> +- (void) mouseSetX:(int)x Y:(int)y on:(int)on {
>>> +    if (!on != !guestCursorVis) {
>>> +        guestCursorVis = on;
>>> +        [self setCursorAppearance];
>>> +    }
>>> +
>>> +    if (on && (x != guestCursorX || y != guestCursorY)) {
>>> +        guestCursorX = x;
>>> +        guestCursorY = y;
>>> +        [self setCursorPosition];
>>> +    }
>>> +}
>>> +
>>> +// Change the cursor image to the default, the guest cursor bitmap or hidden.
>>> +// Said to be an expensive operation on macOS Monterey, so use sparingly.
>>> +- (void) setCursorAppearance {
>>> +    NSCursor *cursor = NULL; // NULL means hidden
>>> +
>>> +    if (!isMouseGrabbed) {
>>> +        cursor = [NSCursor arrowCursor];
>>> +    } else if (!guestCursor && !cursor_hide) {
>>> +        cursor = [NSCursor arrowCursor];
>>> +    } else if (guestCursorVis && guestCursor) {
>>> +        cursor = guestCursor;
>>> +    } else {
>>> +        cursor = NULL;
>>> +    }
>>> +
>>> +    if (cursor != NULL) {
>>> +        [cursor set];
>>> +
>>> +        if (cursorHiddenByMe) {
>>> +            [NSCursor unhide];
>>> +            cursorHiddenByMe = NO;
>>> +        }
>>> +    } else {
>>> +        if (!cursorHiddenByMe) {
>>> +            [NSCursor hide];
>>> +            cursorHiddenByMe = YES;
>>> +        }
>>> +    }
>>> +}
>>> +
>>> +// Move the cursor within the virtual screen
>>> +- (void) setCursorPosition {
>>> +    // Ignore the guest's request if the cursor belongs to Cocoa
>>> +    if (!isMouseGrabbed || isAbsoluteEnabled) {
>>> +        return;
>>> +    }
>>> +
>>> +    // Get guest screen rect in Cocoa coordinates (bottom-left origin).
>>> +    NSRect virtualScreen = [[self window] convertRectToScreen:[self frame]];
>>> +
>>> +    // Convert to top-left origin.
>>> +    NSInteger hostScreenH = [NSScreen screens][0].frame.size.height;
>>> +    int scrX = virtualScreen.origin.x;
>>> +    int scrY = hostScreenH - virtualScreen.origin.y - virtualScreen.size.height;
>>> +    int scrW = virtualScreen.size.width;
>>> +    int scrH = virtualScreen.size.height;
>>> +
>>> +    int cursX = scrX + guestCursorX;
>>> +    int cursY = scrY + guestCursorY;
>>> +
>>> +    // Clip to edges
>>> +    cursX = MIN(MAX(scrX, cursX), scrX + scrW - 1);
>>> +    cursY = MIN(MAX(scrY, cursY), scrY + scrH - 1);
>>> +
>>> +    // Move diagonally towards the center to avoid rounded window corners.
>>> +    // Limit the number of hit-tests and discard failed attempts.
>>> +    int betterX = cursX, betterY = cursY;
>>> +    for (int i=0; i<16; i++) {
>>> +        if ([NSWindow windowNumberAtPoint:NSMakePoint(betterX, hostScreenH - betterY)
>>> +            belowWindowWithWindowNumber:0] == self.window.windowNumber) {
>>> +            cursX = betterX;
>>> +            cursY = betterY;
>>> +            break;
>>> +        };
>>> +
>>> +        if (betterX < scrX + scrW/2) {
>>> +            betterX++;
>>> +        } else {
>>> +            betterX--;
>>> +        }
>>> +
>>> +        if (betterY < scrY + scrH/2) {
>>> +            betterY++;
>>> +        } else {
>>> +            betterY--;
>>> +        }
>>> +    }
>>> +
>>> +    // Subtract this warp delta from the next NSEventTypeMouseMoved.
>>> +    // These are in down-is-positive coords, same as NSEvent deltaX/deltaY.
>>> +    warpDeltaX += cursX - lastWarpX;
>>> +    warpDeltaY += cursY - lastWarpY;
>>> +
>>> +    CGWarpMouseCursorPosition(NSMakePoint(cursX, cursY));
>>> +
>>> +    lastWarpX = cursX;
>>> +    lastWarpY = cursY;
>>> +}
>>> +
>>> - (BOOL) isMouseGrabbed {return isMouseGrabbed;}
>>> - (BOOL) isAbsoluteEnabled {return isAbsoluteEnabled;}
>>> - (float) cdx {return cdx;}
>>> @@ -1251,6 +1415,7 @@ - (id) init
>>>             error_report("(cocoa) can't create window");
>>>             exit(1);
>>>         }
>>> +        [normalWindow disableCursorRects];
>>>         [normalWindow setAcceptsMouseMovedEvents:YES];
>>>         [normalWindow setTitle:@"QEMU"];
>>>         [normalWindow setContentView:cocoaView];
>>> @@ -2123,6 +2288,58 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
>>>     qemu_clipboard_peer_register(&cbpeer);
>>> }
>>> 
>>> +static void cocoa_mouse_set(DisplayChangeListener *dcl, int x, int y, int on) {
> 
> Put { for a function in a new line. See docs/devel/style.rst.
> 
>>> +    dispatch_async(dispatch_get_main_queue(), ^{
>>> +        [cocoaView mouseSetX:x Y:y on:on];
>>> +    });
>>> +}
>>> +
>>> +// Convert QEMUCursor to NSCursor, then call cursorDefine
>>> +static void cocoa_cursor_define(DisplayChangeListener *dcl, QEMUCursor *cursor) {
>>> +    CFDataRef cfdata = CFDataCreate(
>>> +        /*allocator*/ NULL,
>>> +        /*bytes*/ (void *)cursor->data,
>>> +        /*length*/ sizeof(uint32_t) * cursor->width * cursor->height);
>>> +
>>> +    CGDataProviderRef dataprovider = CGDataProviderCreateWithCFData(cfdata);
>>> +
>>> +    CGImageRef cgimage = CGImageCreate(
>>> +        cursor->width, cursor->height,
>>> +        /*bitsPerComponent*/ 8,
>>> +        /*bitsPerPixel*/ 32,
>>> +        /*bytesPerRow*/ sizeof(uint32_t) * cursor->width,
>>> +        /*colorspace*/ CGColorSpaceCreateWithName(kCGColorSpaceSRGB),
>>> +        /*bitmapInfo*/ kCGBitmapByteOrder32Host | kCGImageAlphaLast,
>>> +        /*provider*/ dataprovider,
>>> +        /*decode*/ NULL,
>>> +        /*shouldInterpolate*/ FALSE,
>>> +        /*intent*/ kCGRenderingIntentDefault);
>>> +
>>> +    NSImage *unscaled = [[NSImage alloc] initWithCGImage:cgimage size:NSZeroSize];
>>> +
>>> +    CFRelease(cfdata);
>>> +    CGDataProviderRelease(dataprovider);
>>> +    CGImageRelease(cgimage);
>>> +
>>> +    // Nearest-neighbor scale to the possibly "Retina" cursor size
>>> +    NSImage *scaled = [NSImage
>>> +        imageWithSize:NSMakeSize(cursor->width, cursor->height)
>>> +        flipped:NO
>>> +        drawingHandler:^BOOL(NSRect dest) {
>>> +            [NSGraphicsContext currentContext].imageInterpolation = NSImageInterpolationNone;
>>> +            [unscaled drawInRect:dest];
>>> +            return YES;
>>> +        }];
> 
> unscaled and scaled are leaked.
> 
>>> +
>>> +    NSCursor *nscursor = [[NSCursor alloc]
>>> +        initWithImage:scaled
>>> +        hotSpot:NSMakePoint(cursor->hot_x, cursor->hot_y)];
>>> +
>>> +    dispatch_async(dispatch_get_main_queue(), ^{
>>> +        [cocoaView cursorDefine:nscursor];
>>> +    });
>>> +}
>>> +
>>> static QemuDisplay qemu_display_cocoa = {
>>>     .type       = DISPLAY_TYPE_COCOA,
>>>     .init       = cocoa_display_init,
Re: [PATCH] ui/cocoa: Support hardware cursor interface
Posted by Akihiko Odaki 1 year, 5 months ago
Hi,

On 2022/10/30 14:20, Elliot Nunn wrote:
> Akihiko,
> 
> Thanks very much for reviewing my patch.
> 
> I think that you were right to use the sprite-within-a-window approach,
> and avoid warping the OS cursor. I tried to compensate for the error
> that cursor warping causes in the subsequent mouse event, but there is
> still some error getting through that makes the cursor feel "janky".
> 
> But in absolute pointing mode, will it be possible to remove the guest's
> code path from visual updates of the cursor? I find that under Mac OS 9,
> this provides better responsiveness. I can think of two methods:
> 
> 1. In absolute pointing mode, re-enable Cocoa's cursor and let the host
> OS move it according to user input.
> 
> 2. Keep the cursor sprite, but move it according to Cocoa's mouse
> movement events instead of dpy_mouse_set events.
> 
> I prefer option 2. What do you think?

My patch has been only tested with recent Linux, but it certainly should 
be ensured that it works well for old systems when upstreaming.

First I'd like to know what display device you use. It looks like 
dpy_mouse_set is used only by ati-vga, vhost-user-gpu, virtio-gpu, and 
vmware.

Also, can you give reasoning while 2 is preferred? 1 would allow to 
exploit the hardware's feature for cursor composition, resulting in 
smoother experience and a bit less power consumption. But there may be 
complications it can cause so I have not decided which one is the better 
yet.

> 
> And I didn't realise that you had added VirGL support to cocoa.m. Well
> done! Is it on track for release?
> 
> My patch should be withdrawn from consideration, in favour of a future
> solution that does not use cursor warping.

I'm not really pushing my changes hard so it's kind of stale. Perhaps it 
is better to rewrite the cursor composition patch in a way that does not 
depend on the Virgl patch. I'm also aware that the cursor composition 
using Core Graphics is somewhat laggy so it may be better to rewrite it 
using subview, Core Animation, Metal, or something else. But I have not 
done that yet.

Regards,
Akihiko Odaki

> 
> Many thanks,
> 
> Elliot
> 
>> On 6 Oct 2022, at 8:15 pm, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
>>
>> Thanks Peter and Elliot,
>>
>> Unfortunately Patchew seems to have failed to apply the patch to the
>> current master. It would be nice if you rebase it to the current
>> master.
>>
>> Actually I have a patch to add hardware support to ui/cocoa, but I
>> have not submitted to the mailing list because it depends on a number
>> of other patches:
>> https://github.com/akihikodaki/qemu/commit/34199fcd4080ce8c705b46df26fdf02966b1610c
>>
>> My patch avoided using CGWarpMouseCursorPosition because of its
>> quirks. I'd like to test your patch by myself to see if it avoids them
>> properly for my own workloads.
>>
>> I have also added some comments to the patch. Please see the below.
>>
>> Regards,
>> Akihiko Odaki
>>
>> On Wed, Oct 5, 2022 at 12:39 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>>>
>>> Ccing Akihiko to see if he wants to review this cocoa ui frontend
>>> patch.
>>>
>>> also available at:
>>> https://lore.kernel.org/qemu-devel/54930451-d85f-4ce0-9a45-b3478c5a6468@www.fastmail.com/
>>>
>>> I can confirm that the patch does build, but I don't have any
>>> interesting graphics-using test images to hand to test with.
>>>
>>> thanks
>>> -- PMM
>>>
>>> On Thu, 4 Aug 2022 at 07:28, Elliot Nunn <elliot@nunn.io> wrote:
>>>>
>>>> Implement dpy_cursor_define() and dpy_mouse_set() on macOS.
>>>>
>>>> The main benefit is from dpy_cursor_define: in absolute pointing mode, the
>>>> host can redraw the cursor on the guest's behalf much faster than the guest
>>>> can itself.
>>>>
>>>> To provide the programmatic movement expected from a hardware cursor,
>>>> dpy_mouse_set is also implemented.
>>>>
>>>> Tricky cases are handled:
>>>> - dpy_mouse_set() avoids rounded window corners.
>>>> - The sometimes-delay between warping the cursor and an affected mouse-move
>>>>   event is accounted for.
>>>> - Cursor bitmaps are nearest-neighbor scaled to Retina size.
>>>>
>>>> Signed-off-by: Elliot Nunn <elliot@nunn.io>
>>>> ---
>>>> ui/cocoa.m | 263 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>>>> 1 file changed, 240 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>>>> index 5a8bd5dd84..f9d54448e4 100644
>>>> --- a/ui/cocoa.m
>>>> +++ b/ui/cocoa.m
>>>> @@ -85,12 +85,20 @@ static void cocoa_switch(DisplayChangeListener *dcl,
>>>>
>>>> static void cocoa_refresh(DisplayChangeListener *dcl);
>>>>
>>>> +static void cocoa_mouse_set(DisplayChangeListener *dcl,
>>>> +                            int x, int y, int on);
>>>> +
>>>> +static void cocoa_cursor_define(DisplayChangeListener *dcl,
>>>> +                                QEMUCursor *c);
>>>> +
>>>> static NSWindow *normalWindow;
>>>> static const DisplayChangeListenerOps dcl_ops = {
>>>>      .dpy_name          = "cocoa",
>>>>      .dpy_gfx_update = cocoa_update,
>>>>      .dpy_gfx_switch = cocoa_switch,
>>>>      .dpy_refresh = cocoa_refresh,
>>>> +    .dpy_mouse_set = cocoa_mouse_set,
>>>> +    .dpy_cursor_define = cocoa_cursor_define,
>>>> };
>>>> static DisplayChangeListener dcl = {
>>>>      .ops = &dcl_ops,
>>>> @@ -313,6 +321,13 @@ @interface QemuCocoaView : NSView
>>>>      BOOL isFullscreen;
>>>>      BOOL isAbsoluteEnabled;
>>>>      CFMachPortRef eventsTap;
>>>> +    NSCursor *guestCursor;
>>>> +    BOOL cursorHiddenByMe;
>>
>> Who is "Me" here?
>>
>>>> +    BOOL guestCursorVis;
>>>> +    int guestCursorX, guestCursorY;
>>>> +    int lastWarpX, lastWarpY;
>>>> +    int warpDeltaX, warpDeltaY;
>>>> +    BOOL ignoreNextMouseMove;
>>>> }
>>>> - (void) switchSurface:(pixman_image_t *)image;
>>>> - (void) grabMouse;
>>>> @@ -323,6 +338,10 @@ - (void) handleMonitorInput:(NSEvent *)event;
>>>> - (bool) handleEvent:(NSEvent *)event;
>>>> - (bool) handleEventLocked:(NSEvent *)event;
>>>> - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
>>>> +- (void) cursorDefine:(NSCursor *)cursor;
>>>> +- (void) mouseSetX:(int)x Y:(int)y on:(int)on;
>>>> +- (void) setCursorAppearance;
>>>> +- (void) setCursorPosition;
>>>> /* The state surrounding mouse grabbing is potentially confusing.
>>>>   * isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated
>>>>   *   pointing device an absolute-position one?"], but is only updated on
>>>> @@ -432,22 +451,6 @@ - (CGPoint) screenLocationOfEvent:(NSEvent *)ev
>>>>      }
>>>> }
>>>>
>>>> -- (void) hideCursor
>>>> -{
>>>> -    if (!cursor_hide) {
>>>> -        return;
>>>> -    }
>>>> -    [NSCursor hide];
>>>> -}
>>>> -
>>>> -- (void) unhideCursor
>>>> -{
>>>> -    if (!cursor_hide) {
>>>> -        return;
>>>> -    }
>>>> -    [NSCursor unhide];
>>>> -}
>>>> -
>>>> - (void) drawRect:(NSRect) rect
>>>> {
>>>>      COCOA_DEBUG("QemuCocoaView: drawRect\n");
>>>> @@ -635,6 +638,8 @@ - (void) switchSurface:(pixman_image_t *)image
>>>>          screen.height = h;
>>>>          [self setContentDimensions];
>>>>          [self setFrame:NSMakeRect(cx, cy, cw, ch)];
>>>> +        [self setCursorAppearance];
>>
>> Why do you need to set cursor appearance when resizing the screen?
>>
>>>> +        [self setCursorPosition];
>>>>      }
>>>>
>>>>      // update screenBuffer
>>>> @@ -681,6 +686,7 @@ - (void) toggleFullScreen:(id)sender
>>>>              styleMask:NSWindowStyleMaskBorderless
>>>>              backing:NSBackingStoreBuffered
>>>>              defer:NO];
>>>> +        [fullScreenWindow disableCursorRects];
>>>>          [fullScreenWindow setAcceptsMouseMovedEvents: YES];
>>>>          [fullScreenWindow setHasShadow:NO];
>>>>          [fullScreenWindow setBackgroundColor: [NSColor blackColor]];
>>>> @@ -812,6 +818,7 @@ - (bool) handleEventLocked:(NSEvent *)event
>>>>      int buttons = 0;
>>>>      int keycode = 0;
>>>>      bool mouse_event = false;
>>>> +    bool mousemoved_event = false;
>>>>      // Location of event in virtual screen coordinates
>>>>      NSPoint p = [self screenLocationOfEvent:event];
>>>>      NSUInteger modifiers = [event modifierFlags];
>>>> @@ -1023,6 +1030,7 @@ - (bool) handleEventLocked:(NSEvent *)event
>>>>                  }
>>>>              }
>>>>              mouse_event = true;
>>>> +            mousemoved_event = true;
>>>>              break;
>>>>          case NSEventTypeLeftMouseDown:
>>>>              buttons |= MOUSE_EVENT_LBUTTON;
>>>> @@ -1039,14 +1047,17 @@ - (bool) handleEventLocked:(NSEvent *)event
>>>>          case NSEventTypeLeftMouseDragged:
>>>>              buttons |= MOUSE_EVENT_LBUTTON;
>>>>              mouse_event = true;
>>>> +            mousemoved_event = true;
>>>>              break;
>>>>          case NSEventTypeRightMouseDragged:
>>>>              buttons |= MOUSE_EVENT_RBUTTON;
>>>>              mouse_event = true;
>>>> +            mousemoved_event = true;
>>>>              break;
>>>>          case NSEventTypeOtherMouseDragged:
>>>>              buttons |= MOUSE_EVENT_MBUTTON;
>>>>              mouse_event = true;
>>>> +            mousemoved_event = true;
>>>>              break;
>>>>          case NSEventTypeLeftMouseUp:
>>>>              mouse_event = true;
>>>> @@ -1121,7 +1132,12 @@ - (bool) handleEventLocked:(NSEvent *)event
>>>>              qemu_input_update_buttons(dcl.con, bmap, last_buttons, buttons);
>>>>              last_buttons = buttons;
>>>>          }
>>>> -        if (isMouseGrabbed) {
>>>> +
>>>> +        if (!isMouseGrabbed) {
>>>> +            return false;
>>>> +        }
>>>> +
>>>> +        if (mousemoved_event) {
>>>>              if (isAbsoluteEnabled) {
>>>>                  /* Note that the origin for Cocoa mouse coords is bottom left, not top left.
>>>>                   * The check on screenContainsPoint is to avoid sending out of range values for
>>>> @@ -1132,11 +1148,38 @@ - (bool) handleEventLocked:(NSEvent *)event
>>>>                      qemu_input_queue_abs(dcl.con, INPUT_AXIS_Y, screen.height - p.y, 0, screen.height);
>>>>                  }
>>>>              } else {
>>>> -                qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, (int)[event deltaX]);
>>>> -                qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, (int)[event deltaY]);
>>>> +                if (ignoreNextMouseMove) {
>>>> +                    // Discard the first mouse-move event after a grab, because
>>>> +                    // it includes the warp delta from an unknown initial position.
>>>> +                    ignoreNextMouseMove = NO;
>>>> +                    warpDeltaX = warpDeltaY = 0;
>>>> +                } else {
>>>> +                    // Correct subsequent events to remove the known warp delta.
>>>> +                    // The warp delta is sometimes late to be reported, so never
>>>> +                    // allow the delta compensation to alter the direction.
>>>> +                    int dX = (int)[event deltaX];
>>>> +                    int dY = (int)[event deltaY];
>>>> +
>>>> +                    if (dX == 0 || (dX ^ (dX - warpDeltaX)) < 0) { // Flipped sign?
>>
>> Instead, do: (dx < 0) == (dx - warpDeltaX < 0). The original flipped
>> sign check is dependent on an implementation-defined behavior, and a
>> bit difficult to understand. A decent compiler should be able to
>> optimize it to the bitwise operation.
>>
>>>> +                        warpDeltaX -= dX; // Save excess correction for later
>>>> +                        dX = 0;
>>>> +                    } else {
>>>> +                        dX -= warpDeltaX; // Apply entire correction
>>>> +                        warpDeltaX = 0;
>>>> +                    }
>>>> +
>>>> +                    if (dY == 0 || (dY ^ (dY - warpDeltaY)) < 0) {
>>>> +                        warpDeltaY -= dY;
>>>> +                        dY = 0;
>>>> +                    } else {
>>>> +                        dY -= warpDeltaY;
>>>> +                        warpDeltaY = 0;
>>>> +                    }
>>>> +
>>>> +                    qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, dX);
>>>> +                    qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, dY);
>>>> +                }
>>>>              }
>>>> -        } else {
>>>> -            return false;
>>>>          }
>>>>          qemu_input_event_sync();
>>>>      }
>>>> @@ -1153,9 +1196,15 @@ - (void) grabMouse
>>>>          else
>>>>              [normalWindow setTitle:@"QEMU - (Press ctrl + alt + g to release Mouse)"];
>>>>      }
>>>> -    [self hideCursor];
>>>>      CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
>>>>      isMouseGrabbed = TRUE; // while isMouseGrabbed = TRUE, QemuCocoaApp sends all events to [cocoaView handleEvent:]
>>>> +    [self setCursorAppearance];
>>>> +    [self setCursorPosition];
>>>> +
>>>> +    // We took over and warped the mouse, so ignore the next mouse-move
>>>> +    if (!isAbsoluteEnabled) {
>>>> +        ignoreNextMouseMove = YES;
>>>> +    }
>>
>> It shouldn't warp the mouse when the pointing device is absolute. An
>> absolute pointing device, especially vdagent, is often used to
>> seamlessly integrate the guest and host cursors.
>>
>>>> }
>>>>
>>>> - (void) ungrabMouse
>>>> @@ -1168,9 +1217,14 @@ - (void) ungrabMouse
>>>>          else
>>>>              [normalWindow setTitle:@"QEMU"];
>>>>      }
>>>> -    [self unhideCursor];
>>>>      CGAssociateMouseAndMouseCursorPosition(TRUE);
>>>>      isMouseGrabbed = FALSE;
>>>> +    [self setCursorAppearance];
>>>> +
>>>> +    if (!isAbsoluteEnabled) {
>>>> +        ignoreNextMouseMove = NO;
>>>> +        warpDeltaX = warpDeltaY = 0;
>>>> +    }
>>>> }
>>>>
>>>> - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
>>>> @@ -1179,6 +1233,116 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
>>>>          CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
>>>>      }
>>>> }
>>>> +
>>>> +// Indirectly called by dpy_cursor_define() in the virtual GPU
>>>> +- (void) cursorDefine:(NSCursor *)cursor {
>>>> +    guestCursor = cursor;
>>
>> The old cursor is leaked here. Note that ARC is not enabled on QEMU,
>> unfortunately.
>>
>>>> +    [self setCursorAppearance];
>>>> +}
>>>> +
>>>> +// Indirectly called by dpy_mouse_set() in the virtual GPU
>>>> +- (void) mouseSetX:(int)x Y:(int)y on:(int)on {
>>>> +    if (!on != !guestCursorVis) {
>>>> +        guestCursorVis = on;
>>>> +        [self setCursorAppearance];
>>>> +    }
>>>> +
>>>> +    if (on && (x != guestCursorX || y != guestCursorY)) {
>>>> +        guestCursorX = x;
>>>> +        guestCursorY = y;
>>>> +        [self setCursorPosition];
>>>> +    }
>>>> +}
>>>> +
>>>> +// Change the cursor image to the default, the guest cursor bitmap or hidden.
>>>> +// Said to be an expensive operation on macOS Monterey, so use sparingly.
>>>> +- (void) setCursorAppearance {
>>>> +    NSCursor *cursor = NULL; // NULL means hidden
>>>> +
>>>> +    if (!isMouseGrabbed) {
>>>> +        cursor = [NSCursor arrowCursor];
>>>> +    } else if (!guestCursor && !cursor_hide) {
>>>> +        cursor = [NSCursor arrowCursor];
>>>> +    } else if (guestCursorVis && guestCursor) {
>>>> +        cursor = guestCursor;
>>>> +    } else {
>>>> +        cursor = NULL;
>>>> +    }
>>>> +
>>>> +    if (cursor != NULL) {
>>>> +        [cursor set];
>>>> +
>>>> +        if (cursorHiddenByMe) {
>>>> +            [NSCursor unhide];
>>>> +            cursorHiddenByMe = NO;
>>>> +        }
>>>> +    } else {
>>>> +        if (!cursorHiddenByMe) {
>>>> +            [NSCursor hide];
>>>> +            cursorHiddenByMe = YES;
>>>> +        }
>>>> +    }
>>>> +}
>>>> +
>>>> +// Move the cursor within the virtual screen
>>>> +- (void) setCursorPosition {
>>>> +    // Ignore the guest's request if the cursor belongs to Cocoa
>>>> +    if (!isMouseGrabbed || isAbsoluteEnabled) {
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    // Get guest screen rect in Cocoa coordinates (bottom-left origin).
>>>> +    NSRect virtualScreen = [[self window] convertRectToScreen:[self frame]];
>>>> +
>>>> +    // Convert to top-left origin.
>>>> +    NSInteger hostScreenH = [NSScreen screens][0].frame.size.height;
>>>> +    int scrX = virtualScreen.origin.x;
>>>> +    int scrY = hostScreenH - virtualScreen.origin.y - virtualScreen.size.height;
>>>> +    int scrW = virtualScreen.size.width;
>>>> +    int scrH = virtualScreen.size.height;
>>>> +
>>>> +    int cursX = scrX + guestCursorX;
>>>> +    int cursY = scrY + guestCursorY;
>>>> +
>>>> +    // Clip to edges
>>>> +    cursX = MIN(MAX(scrX, cursX), scrX + scrW - 1);
>>>> +    cursY = MIN(MAX(scrY, cursY), scrY + scrH - 1);
>>>> +
>>>> +    // Move diagonally towards the center to avoid rounded window corners.
>>>> +    // Limit the number of hit-tests and discard failed attempts.
>>>> +    int betterX = cursX, betterY = cursY;
>>>> +    for (int i=0; i<16; i++) {
>>>> +        if ([NSWindow windowNumberAtPoint:NSMakePoint(betterX, hostScreenH - betterY)
>>>> +            belowWindowWithWindowNumber:0] == self.window.windowNumber) {
>>>> +            cursX = betterX;
>>>> +            cursY = betterY;
>>>> +            break;
>>>> +        };
>>>> +
>>>> +        if (betterX < scrX + scrW/2) {
>>>> +            betterX++;
>>>> +        } else {
>>>> +            betterX--;
>>>> +        }
>>>> +
>>>> +        if (betterY < scrY + scrH/2) {
>>>> +            betterY++;
>>>> +        } else {
>>>> +            betterY--;
>>>> +        }
>>>> +    }
>>>> +
>>>> +    // Subtract this warp delta from the next NSEventTypeMouseMoved.
>>>> +    // These are in down-is-positive coords, same as NSEvent deltaX/deltaY.
>>>> +    warpDeltaX += cursX - lastWarpX;
>>>> +    warpDeltaY += cursY - lastWarpY;
>>>> +
>>>> +    CGWarpMouseCursorPosition(NSMakePoint(cursX, cursY));
>>>> +
>>>> +    lastWarpX = cursX;
>>>> +    lastWarpY = cursY;
>>>> +}
>>>> +
>>>> - (BOOL) isMouseGrabbed {return isMouseGrabbed;}
>>>> - (BOOL) isAbsoluteEnabled {return isAbsoluteEnabled;}
>>>> - (float) cdx {return cdx;}
>>>> @@ -1251,6 +1415,7 @@ - (id) init
>>>>              error_report("(cocoa) can't create window");
>>>>              exit(1);
>>>>          }
>>>> +        [normalWindow disableCursorRects];
>>>>          [normalWindow setAcceptsMouseMovedEvents:YES];
>>>>          [normalWindow setTitle:@"QEMU"];
>>>>          [normalWindow setContentView:cocoaView];
>>>> @@ -2123,6 +2288,58 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
>>>>      qemu_clipboard_peer_register(&cbpeer);
>>>> }
>>>>
>>>> +static void cocoa_mouse_set(DisplayChangeListener *dcl, int x, int y, int on) {
>>
>> Put { for a function in a new line. See docs/devel/style.rst.
>>
>>>> +    dispatch_async(dispatch_get_main_queue(), ^{
>>>> +        [cocoaView mouseSetX:x Y:y on:on];
>>>> +    });
>>>> +}
>>>> +
>>>> +// Convert QEMUCursor to NSCursor, then call cursorDefine
>>>> +static void cocoa_cursor_define(DisplayChangeListener *dcl, QEMUCursor *cursor) {
>>>> +    CFDataRef cfdata = CFDataCreate(
>>>> +        /*allocator*/ NULL,
>>>> +        /*bytes*/ (void *)cursor->data,
>>>> +        /*length*/ sizeof(uint32_t) * cursor->width * cursor->height);
>>>> +
>>>> +    CGDataProviderRef dataprovider = CGDataProviderCreateWithCFData(cfdata);
>>>> +
>>>> +    CGImageRef cgimage = CGImageCreate(
>>>> +        cursor->width, cursor->height,
>>>> +        /*bitsPerComponent*/ 8,
>>>> +        /*bitsPerPixel*/ 32,
>>>> +        /*bytesPerRow*/ sizeof(uint32_t) * cursor->width,
>>>> +        /*colorspace*/ CGColorSpaceCreateWithName(kCGColorSpaceSRGB),
>>>> +        /*bitmapInfo*/ kCGBitmapByteOrder32Host | kCGImageAlphaLast,
>>>> +        /*provider*/ dataprovider,
>>>> +        /*decode*/ NULL,
>>>> +        /*shouldInterpolate*/ FALSE,
>>>> +        /*intent*/ kCGRenderingIntentDefault);
>>>> +
>>>> +    NSImage *unscaled = [[NSImage alloc] initWithCGImage:cgimage size:NSZeroSize];
>>>> +
>>>> +    CFRelease(cfdata);
>>>> +    CGDataProviderRelease(dataprovider);
>>>> +    CGImageRelease(cgimage);
>>>> +
>>>> +    // Nearest-neighbor scale to the possibly "Retina" cursor size
>>>> +    NSImage *scaled = [NSImage
>>>> +        imageWithSize:NSMakeSize(cursor->width, cursor->height)
>>>> +        flipped:NO
>>>> +        drawingHandler:^BOOL(NSRect dest) {
>>>> +            [NSGraphicsContext currentContext].imageInterpolation = NSImageInterpolationNone;
>>>> +            [unscaled drawInRect:dest];
>>>> +            return YES;
>>>> +        }];
>>
>> unscaled and scaled are leaked.
>>
>>>> +
>>>> +    NSCursor *nscursor = [[NSCursor alloc]
>>>> +        initWithImage:scaled
>>>> +        hotSpot:NSMakePoint(cursor->hot_x, cursor->hot_y)];
>>>> +
>>>> +    dispatch_async(dispatch_get_main_queue(), ^{
>>>> +        [cocoaView cursorDefine:nscursor];
>>>> +    });
>>>> +}
>>>> +
>>>> static QemuDisplay qemu_display_cocoa = {
>>>>      .type       = DISPLAY_TYPE_COCOA,
>>>>      .init       = cocoa_display_init,
>
Re: [PATCH] ui/cocoa: Support hardware cursor interface
Posted by BALATON Zoltan 1 year, 5 months ago
On Sun, 30 Oct 2022, Akihiko Odaki wrote:
> On 2022/10/30 14:20, Elliot Nunn wrote:
>> Akihiko,
>> 
>> Thanks very much for reviewing my patch.
>> 
>> I think that you were right to use the sprite-within-a-window approach,
>> and avoid warping the OS cursor. I tried to compensate for the error
>> that cursor warping causes in the subsequent mouse event, but there is
>> still some error getting through that makes the cursor feel "janky".
>> 
>> But in absolute pointing mode, will it be possible to remove the guest's
>> code path from visual updates of the cursor? I find that under Mac OS 9,
>> this provides better responsiveness. I can think of two methods:
>> 
>> 1. In absolute pointing mode, re-enable Cocoa's cursor and let the host
>> OS move it according to user input.
>> 
>> 2. Keep the cursor sprite, but move it according to Cocoa's mouse
>> movement events instead of dpy_mouse_set events.
>> 
>> I prefer option 2. What do you think?
>
> My patch has been only tested with recent Linux, but it certainly should be 
> ensured that it works well for old systems when upstreaming.
>
> First I'd like to know what display device you use. It looks like 
> dpy_mouse_set is used only by ati-vga, vhost-user-gpu, virtio-gpu, and 
> vmware.
>
> Also, can you give reasoning while 2 is preferred? 1 would allow to exploit 
> the hardware's feature for cursor composition, resulting in smoother 
> experience and a bit less power consumption. But there may be complications 
> it can cause so I have not decided which one is the better yet.

Maybe cc-ing Gerd as the UI maintainer is a good idea in case he can add 
some insight too.

I'm not sure about this and may be wrong but I have a theory that the 
problems with mouse tracking are caused by not taking mouse ponter 
acceleration into account correctly. I did not check it and can't prove it 
but I think the guest and host cursor get out of sync because QEMU does 
not know about pointer acceleration methods and parameters which are 
different for each OS (and also user settable) so it's hard to reproduce 
it in QEMU so it just assumes linear movement and tracks guest cursor 
assuming that. Then it has to correct it when mouse is moved faster and 
the pointer ends up at a different place than expected. Is this a 
plausible theory? If so then we may need to take pointer acceleration into 
account or move the guest pointer based on host values or maybe somehow 
change guest pointer acceleration values to match the host's but that 
seems to be quite difficult because of the number of different guests. But 
it you're writing a guest driver this may be an option too at least for 
that guest.

Maybe it would help if there was a clearer picture of how all this works. 
We have the mouse connected to the host that sends relative move events. 
Then this is forwarded to the guest which moves the pointer based on accel 
setting of its own while the host moves its pointer with a different accel 
setting. Did I get that right? Then we either need to sync these pointer 
accel settings or pick one and use that for both. To let the host draw the 
pointer we should change the guest accel settings to match the host's or 
somehow change it for the QEMU window on the host to match the guest's. 
Neither seems to be easy and probably needs something in the guest to do 
it so I'm lost at this point and it's just a vague idea that could be all 
wrong but maybe thinking on this path could lead somewhere.

Regards,
BALATON Zoltan
Re: [PATCH] ui/cocoa: Support hardware cursor interface
Posted by Peter Maydell 1 year, 5 months ago
On Sun, 30 Oct 2022 at 11:20, BALATON Zoltan <balaton@eik.bme.hu> wrote:
> I'm not sure about this and may be wrong but I have a theory that the
> problems with mouse tracking are caused by not taking mouse ponter
> acceleration into account correctly. I did not check it and can't prove it
> but I think the guest and host cursor get out of sync because QEMU does
> not know about pointer acceleration methods and parameters which are
> different for each OS (and also user settable) so it's hard to reproduce
> it in QEMU so it just assumes linear movement and tracks guest cursor
> assuming that. Then it has to correct it when mouse is moved faster and
> the pointer ends up at a different place than expected. Is this a
> plausible theory?

Yes, this is a common problem. The traditional solution is to
make QEMU emulate an absolute-pointing device, usually the USB
tablet. That way we can always provide the guest with the actual
position of the host cursor.

-- PMM
Re: [PATCH] ui/cocoa: Support hardware cursor interface
Posted by Elliot Nunn 1 year, 5 months ago
Akihiko,

Sounds like you've done a lot of work on ui/cocoa, with the goal of
improving the experience with modern Linux guests. My goal is to improve
the experience with antiquated Mac OS 9 guests.

> My patch has been only tested with recent Linux, but it certainly should
> be ensured that it works well for old systems when upstreaming.
> 
> First I'd like to know what display device you use. It looks like
> dpy_mouse_set is used only by ati-vga, vhost-user-gpu, virtio-gpu, and
> vmware.

I was using my own hardware cursor patches to the VGA device, but now I am
using virtio-gpu. My Mac OS 9 driver for virtio-gpu is in progress.

>> 1. In absolute pointing mode, re-enable Cocoa's cursor and let the host
>> OS move it according to user input.
>> 2. Keep the cursor sprite, but move it according to Cocoa's mouse
>> movement events instead of dpy_mouse_set events.
> 
> Also, can you give reasoning while 2 is preferred? 1 would allow to
> exploit the hardware's feature for cursor composition, resulting in
> smoother experience and a bit less power consumption. But there may be
> complications it can cause so I have not decided which one is the better
> yet.

Mainly that it would simplify the code. OTOH, if we expect the guest to
use the hardware cursor facility, then it's only fair that the host does
the same. I'm open to either option. We should probably try both.

>> And I didn't realise that you had added VirGL support to cocoa.m. Well
>> done! Is it on track for release?
>> My patch should be withdrawn from consideration, in favour of a future
>> solution that does not use cursor warping.
> 
> I'm not really pushing my changes hard so it's kind of stale. Perhaps it
> is better to rewrite the cursor composition patch in a way that does not
> depend on the Virgl patch. I'm also aware that the cursor composition
> using Core Graphics is somewhat laggy so it may be better to rewrite it
> using subview, Core Animation, Metal, or something else. But I have not
> done that yet.

Is there some Cocoa-native way of compositing within the window, that
will work with or without a GL surface? Subviews sound appropriate.

Not that I have any influence, but I think your virgl patch is an
excellent contribution and should go upstream.

Thanks again,

Elliot
Re: [PATCH] ui/cocoa: Support hardware cursor interface
Posted by Akihiko Odaki 1 year, 5 months ago
On 2022/10/30 19:12, Elliot Nunn wrote:
> Akihiko,
> 
> Sounds like you've done a lot of work on ui/cocoa, with the goal of
> improving the experience with modern Linux guests. My goal is to improve
> the experience with antiquated Mac OS 9 guests.
> 
>> My patch has been only tested with recent Linux, but it certainly should
>> be ensured that it works well for old systems when upstreaming.
>>
>> First I'd like to know what display device you use. It looks like
>> dpy_mouse_set is used only by ati-vga, vhost-user-gpu, virtio-gpu, and
>> vmware.
> 
> I was using my own hardware cursor patches to the VGA device, but now I am
> using virtio-gpu. My Mac OS 9 driver for virtio-gpu is in progress.

Interesting, but I'm worried that your driver may be not performant 
enough to drive dpy_mouse_set. Does your driver provide hardware cursor 
as smooth as software cursor? If not, the proper way to fix the problem 
is to fix your driver. Strictly speaking, ignoring dpy_mouse_set and 
using the input information directly violates the semantics and should 
be avoided if possible. That said, if your driver already does the best 
to the extent what Mac OS 9 allows and you want even better, it can be a 
worthwhile option.

> 
>>> 1. In absolute pointing mode, re-enable Cocoa's cursor and let the host
>>> OS move it according to user input.
>>> 2. Keep the cursor sprite, but move it according to Cocoa's mouse
>>> movement events instead of dpy_mouse_set events.
>>
>> Also, can you give reasoning while 2 is preferred? 1 would allow to
>> exploit the hardware's feature for cursor composition, resulting in
>> smoother experience and a bit less power consumption. But there may be
>> complications it can cause so I have not decided which one is the better
>> yet.
> 
> Mainly that it would simplify the code. OTOH, if we expect the guest to
> use the hardware cursor facility, then it's only fair that the host does
> the same. I'm open to either option. We should probably try both.

Regarding the code complexity, option  2 can be still the better option 
because option 1 requires additional code to pass the input information 
to the cursor composition code. But it is just a possibility and I guess 
we will not know which is the better unless we implement them as you say.

> 
>>> And I didn't realise that you had added VirGL support to cocoa.m. Well
>>> done! Is it on track for release?
>>> My patch should be withdrawn from consideration, in favour of a future
>>> solution that does not use cursor warping.
>>
>> I'm not really pushing my changes hard so it's kind of stale. Perhaps it
>> is better to rewrite the cursor composition patch in a way that does not
>> depend on the Virgl patch. I'm also aware that the cursor composition
>> using Core Graphics is somewhat laggy so it may be better to rewrite it
>> using subview, Core Animation, Metal, or something else. But I have not
>> done that yet.
> 
> Is there some Cocoa-native way of compositing within the window, that
> will work with or without a GL surface? Subviews sound appropriate.

I'm for subview because we can retain the current implementation using 
[-NSView drawRect:] for the screen output in this way. The current 
implementation using Core Graphics or OpenGL for cursor composition 
should be avoided as Core Graphics cannot accelerate it with GPU and 
OpenGL requires the deprecated CGL or an external library like ANGLE.

Regards,
Akihiko Odaki

> 
> Not that I have any influence, but I think your virgl patch is an
> excellent contribution and should go upstream.
> 
> Thanks again,
> 
> Elliot
Re: [PATCH] ui/cocoa: Support hardware cursor interface
Posted by Peter Maydell 1 year, 6 months ago
On Thu, 6 Oct 2022 at 13:16, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
>
> Thanks Peter and Elliot,
>
> Unfortunately Patchew seems to have failed to apply the patch to the
> current master. It would be nice if you rebase it to the current
> master.

I think this is probably mostly because the patch got sent in
quoted-printable or something similar. I found that grabbing the
mbox from lore.kernel.org gave me something I could apply. But
it would probably be helpful if Elliot could rebase and resend making
sure it's in plain text without linewraps or quoted-printable or other
encoding.

> Actually I have a patch to add hardware support to ui/cocoa, but I
> have not submitted to the mailing list because it depends on a number
> of other patches:
> https://github.com/akihikodaki/qemu/commit/34199fcd4080ce8c705b46df26fdf02966b1610c
>
> My patch avoided using CGWarpMouseCursorPosition because of its
> quirks. I'd like to test your patch by myself to see if it avoids them
> properly for my own workloads.
>
> I have also added some comments to the patch. Please see the below.

Thanks for taking a look at the patch.

-- PMM
Re: [PATCH] ui/cocoa: Support hardware cursor interface
Posted by Elliot Nunn 1 year, 8 months ago
Resending this patch now that 7.1 is released (well done).

> On 4 Aug 2022, at 2:27 pm, Elliot Nunn <elliot@nunn.io> wrote:
> 
> Implement dpy_cursor_define() and dpy_mouse_set() on macOS.
> 
> The main benefit is from dpy_cursor_define: in absolute pointing mode, the
> host can redraw the cursor on the guest's behalf much faster than the guest
> can itself.
> 
> To provide the programmatic movement expected from a hardware cursor,
> dpy_mouse_set is also implemented.
> 
> Tricky cases are handled:
> - dpy_mouse_set() avoids rounded window corners.
> - The sometimes-delay between warping the cursor and an affected mouse-move
>  event is accounted for.
> - Cursor bitmaps are nearest-neighbor scaled to Retina size.
> 
> Signed-off-by: Elliot Nunn <elliot@nunn.io>
> ---
> ui/cocoa.m | 263 ++++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 240 insertions(+), 23 deletions(-)
> 
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 5a8bd5dd84..f9d54448e4 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -85,12 +85,20 @@ static void cocoa_switch(DisplayChangeListener *dcl,
> 
> static void cocoa_refresh(DisplayChangeListener *dcl);
> 
> +static void cocoa_mouse_set(DisplayChangeListener *dcl,
> +                            int x, int y, int on);
> +
> +static void cocoa_cursor_define(DisplayChangeListener *dcl,
> +                                QEMUCursor *c);
> +
> static NSWindow *normalWindow;
> static const DisplayChangeListenerOps dcl_ops = {
>     .dpy_name          = "cocoa",
>     .dpy_gfx_update = cocoa_update,
>     .dpy_gfx_switch = cocoa_switch,
>     .dpy_refresh = cocoa_refresh,
> +    .dpy_mouse_set = cocoa_mouse_set,
> +    .dpy_cursor_define = cocoa_cursor_define,
> };
> static DisplayChangeListener dcl = {
>     .ops = &dcl_ops,
> @@ -313,6 +321,13 @@ @interface QemuCocoaView : NSView
>     BOOL isFullscreen;
>     BOOL isAbsoluteEnabled;
>     CFMachPortRef eventsTap;
> +    NSCursor *guestCursor;
> +    BOOL cursorHiddenByMe;
> +    BOOL guestCursorVis;
> +    int guestCursorX, guestCursorY;
> +    int lastWarpX, lastWarpY;
> +    int warpDeltaX, warpDeltaY;
> +    BOOL ignoreNextMouseMove;
> }
> - (void) switchSurface:(pixman_image_t *)image;
> - (void) grabMouse;
> @@ -323,6 +338,10 @@ - (void) handleMonitorInput:(NSEvent *)event;
> - (bool) handleEvent:(NSEvent *)event;
> - (bool) handleEventLocked:(NSEvent *)event;
> - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
> +- (void) cursorDefine:(NSCursor *)cursor;
> +- (void) mouseSetX:(int)x Y:(int)y on:(int)on;
> +- (void) setCursorAppearance;
> +- (void) setCursorPosition;
> /* The state surrounding mouse grabbing is potentially confusing.
>  * isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated
>  *   pointing device an absolute-position one?"], but is only updated on
> @@ -432,22 +451,6 @@ - (CGPoint) screenLocationOfEvent:(NSEvent *)ev
>     }
> }
> 
> -- (void) hideCursor
> -{
> -    if (!cursor_hide) {
> -        return;
> -    }
> -    [NSCursor hide];
> -}
> -
> -- (void) unhideCursor
> -{
> -    if (!cursor_hide) {
> -        return;
> -    }
> -    [NSCursor unhide];
> -}
> -
> - (void) drawRect:(NSRect) rect
> {
>     COCOA_DEBUG("QemuCocoaView: drawRect\n");
> @@ -635,6 +638,8 @@ - (void) switchSurface:(pixman_image_t *)image
>         screen.height = h;
>         [self setContentDimensions];
>         [self setFrame:NSMakeRect(cx, cy, cw, ch)];
> +        [self setCursorAppearance];
> +        [self setCursorPosition];
>     }
> 
>     // update screenBuffer
> @@ -681,6 +686,7 @@ - (void) toggleFullScreen:(id)sender
>             styleMask:NSWindowStyleMaskBorderless
>             backing:NSBackingStoreBuffered
>             defer:NO];
> +        [fullScreenWindow disableCursorRects];
>         [fullScreenWindow setAcceptsMouseMovedEvents: YES];
>         [fullScreenWindow setHasShadow:NO];
>         [fullScreenWindow setBackgroundColor: [NSColor blackColor]];
> @@ -812,6 +818,7 @@ - (bool) handleEventLocked:(NSEvent *)event
>     int buttons = 0;
>     int keycode = 0;
>     bool mouse_event = false;
> +    bool mousemoved_event = false;
>     // Location of event in virtual screen coordinates
>     NSPoint p = [self screenLocationOfEvent:event];
>     NSUInteger modifiers = [event modifierFlags];
> @@ -1023,6 +1030,7 @@ - (bool) handleEventLocked:(NSEvent *)event
>                 }
>             }
>             mouse_event = true;
> +            mousemoved_event = true;
>             break;
>         case NSEventTypeLeftMouseDown:
>             buttons |= MOUSE_EVENT_LBUTTON;
> @@ -1039,14 +1047,17 @@ - (bool) handleEventLocked:(NSEvent *)event
>         case NSEventTypeLeftMouseDragged:
>             buttons |= MOUSE_EVENT_LBUTTON;
>             mouse_event = true;
> +            mousemoved_event = true;
>             break;
>         case NSEventTypeRightMouseDragged:
>             buttons |= MOUSE_EVENT_RBUTTON;
>             mouse_event = true;
> +            mousemoved_event = true;
>             break;
>         case NSEventTypeOtherMouseDragged:
>             buttons |= MOUSE_EVENT_MBUTTON;
>             mouse_event = true;
> +            mousemoved_event = true;
>             break;
>         case NSEventTypeLeftMouseUp:
>             mouse_event = true;
> @@ -1121,7 +1132,12 @@ - (bool) handleEventLocked:(NSEvent *)event
>             qemu_input_update_buttons(dcl.con, bmap, last_buttons, buttons);
>             last_buttons = buttons;
>         }
> -        if (isMouseGrabbed) {
> +
> +        if (!isMouseGrabbed) {
> +            return false;
> +        }
> +
> +        if (mousemoved_event) {
>             if (isAbsoluteEnabled) {
>                 /* Note that the origin for Cocoa mouse coords is bottom left, not top left.
>                  * The check on screenContainsPoint is to avoid sending out of range values for
> @@ -1132,11 +1148,38 @@ - (bool) handleEventLocked:(NSEvent *)event
>                     qemu_input_queue_abs(dcl.con, INPUT_AXIS_Y, screen.height - p.y, 0, screen.height);
>                 }
>             } else {
> -                qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, (int)[event deltaX]);
> -                qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, (int)[event deltaY]);
> +                if (ignoreNextMouseMove) {
> +                    // Discard the first mouse-move event after a grab, because
> +                    // it includes the warp delta from an unknown initial position.
> +                    ignoreNextMouseMove = NO;
> +                    warpDeltaX = warpDeltaY = 0;
> +                } else {
> +                    // Correct subsequent events to remove the known warp delta.
> +                    // The warp delta is sometimes late to be reported, so never
> +                    // allow the delta compensation to alter the direction.
> +                    int dX = (int)[event deltaX];
> +                    int dY = (int)[event deltaY];
> +
> +                    if (dX == 0 || (dX ^ (dX - warpDeltaX)) < 0) { // Flipped sign?
> +                        warpDeltaX -= dX; // Save excess correction for later
> +                        dX = 0;
> +                    } else {
> +                        dX -= warpDeltaX; // Apply entire correction
> +                        warpDeltaX = 0;
> +                    }
> +
> +                    if (dY == 0 || (dY ^ (dY - warpDeltaY)) < 0) {
> +                        warpDeltaY -= dY;
> +                        dY = 0;
> +                    } else {
> +                        dY -= warpDeltaY;
> +                        warpDeltaY = 0;
> +                    }
> +
> +                    qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, dX);
> +                    qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, dY);
> +                }
>             }
> -        } else {
> -            return false;
>         }
>         qemu_input_event_sync();
>     }
> @@ -1153,9 +1196,15 @@ - (void) grabMouse
>         else
>             [normalWindow setTitle:@"QEMU - (Press ctrl + alt + g to release Mouse)"];
>     }
> -    [self hideCursor];
>     CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
>     isMouseGrabbed = TRUE; // while isMouseGrabbed = TRUE, QemuCocoaApp sends all events to [cocoaView handleEvent:]
> +    [self setCursorAppearance];
> +    [self setCursorPosition];
> +
> +    // We took over and warped the mouse, so ignore the next mouse-move
> +    if (!isAbsoluteEnabled) {
> +        ignoreNextMouseMove = YES;
> +    }
> }
> 
> - (void) ungrabMouse
> @@ -1168,9 +1217,14 @@ - (void) ungrabMouse
>         else
>             [normalWindow setTitle:@"QEMU"];
>     }
> -    [self unhideCursor];
>     CGAssociateMouseAndMouseCursorPosition(TRUE);
>     isMouseGrabbed = FALSE;
> +    [self setCursorAppearance];
> +
> +    if (!isAbsoluteEnabled) {
> +        ignoreNextMouseMove = NO;
> +        warpDeltaX = warpDeltaY = 0;
> +    }
> }
> 
> - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
> @@ -1179,6 +1233,116 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
>         CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
>     }
> }
> +
> +// Indirectly called by dpy_cursor_define() in the virtual GPU
> +- (void) cursorDefine:(NSCursor *)cursor {
> +    guestCursor = cursor;
> +    [self setCursorAppearance];
> +}
> +
> +// Indirectly called by dpy_mouse_set() in the virtual GPU
> +- (void) mouseSetX:(int)x Y:(int)y on:(int)on {
> +    if (!on != !guestCursorVis) {
> +        guestCursorVis = on;
> +        [self setCursorAppearance];
> +    }
> +
> +    if (on && (x != guestCursorX || y != guestCursorY)) {
> +        guestCursorX = x;
> +        guestCursorY = y;
> +        [self setCursorPosition];
> +    }
> +}
> +
> +// Change the cursor image to the default, the guest cursor bitmap or hidden.
> +// Said to be an expensive operation on macOS Monterey, so use sparingly.
> +- (void) setCursorAppearance {
> +    NSCursor *cursor = NULL; // NULL means hidden
> +
> +    if (!isMouseGrabbed) {
> +        cursor = [NSCursor arrowCursor];
> +    } else if (!guestCursor && !cursor_hide) {
> +        cursor = [NSCursor arrowCursor];
> +    } else if (guestCursorVis && guestCursor) {
> +        cursor = guestCursor;
> +    } else {
> +        cursor = NULL;
> +    }
> +
> +    if (cursor != NULL) {
> +        [cursor set];
> +
> +        if (cursorHiddenByMe) {
> +            [NSCursor unhide];
> +            cursorHiddenByMe = NO;
> +        }
> +    } else {
> +        if (!cursorHiddenByMe) {
> +            [NSCursor hide];
> +            cursorHiddenByMe = YES;
> +        }
> +    }
> +}
> +
> +// Move the cursor within the virtual screen
> +- (void) setCursorPosition {
> +    // Ignore the guest's request if the cursor belongs to Cocoa
> +    if (!isMouseGrabbed || isAbsoluteEnabled) {
> +        return;
> +    }
> +
> +    // Get guest screen rect in Cocoa coordinates (bottom-left origin).
> +    NSRect virtualScreen = [[self window] convertRectToScreen:[self frame]];
> +
> +    // Convert to top-left origin.
> +    NSInteger hostScreenH = [NSScreen screens][0].frame.size.height;
> +    int scrX = virtualScreen.origin.x;
> +    int scrY = hostScreenH - virtualScreen.origin.y - virtualScreen.size.height;
> +    int scrW = virtualScreen.size.width;
> +    int scrH = virtualScreen.size.height;
> +
> +    int cursX = scrX + guestCursorX;
> +    int cursY = scrY + guestCursorY;
> +
> +    // Clip to edges
> +    cursX = MIN(MAX(scrX, cursX), scrX + scrW - 1);
> +    cursY = MIN(MAX(scrY, cursY), scrY + scrH - 1);
> +
> +    // Move diagonally towards the center to avoid rounded window corners.
> +    // Limit the number of hit-tests and discard failed attempts.
> +    int betterX = cursX, betterY = cursY;
> +    for (int i=0; i<16; i++) {
> +        if ([NSWindow windowNumberAtPoint:NSMakePoint(betterX, hostScreenH - betterY)
> +            belowWindowWithWindowNumber:0] == self.window.windowNumber) {
> +            cursX = betterX;
> +            cursY = betterY;
> +            break;
> +        };
> +
> +        if (betterX < scrX + scrW/2) {
> +            betterX++;
> +        } else {
> +            betterX--;
> +        }
> +
> +        if (betterY < scrY + scrH/2) {
> +            betterY++;
> +        } else {
> +            betterY--;
> +        }
> +    }
> +
> +    // Subtract this warp delta from the next NSEventTypeMouseMoved.
> +    // These are in down-is-positive coords, same as NSEvent deltaX/deltaY.
> +    warpDeltaX += cursX - lastWarpX;
> +    warpDeltaY += cursY - lastWarpY;
> +
> +    CGWarpMouseCursorPosition(NSMakePoint(cursX, cursY));
> +
> +    lastWarpX = cursX;
> +    lastWarpY = cursY;
> +}
> +
> - (BOOL) isMouseGrabbed {return isMouseGrabbed;}
> - (BOOL) isAbsoluteEnabled {return isAbsoluteEnabled;}
> - (float) cdx {return cdx;}
> @@ -1251,6 +1415,7 @@ - (id) init
>             error_report("(cocoa) can't create window");
>             exit(1);
>         }
> +        [normalWindow disableCursorRects];
>         [normalWindow setAcceptsMouseMovedEvents:YES];
>         [normalWindow setTitle:@"QEMU"];
>         [normalWindow setContentView:cocoaView];
> @@ -2123,6 +2288,58 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
>     qemu_clipboard_peer_register(&cbpeer);
> }
> 
> +static void cocoa_mouse_set(DisplayChangeListener *dcl, int x, int y, int on) {
> +    dispatch_async(dispatch_get_main_queue(), ^{
> +        [cocoaView mouseSetX:x Y:y on:on];
> +    });
> +}
> +
> +// Convert QEMUCursor to NSCursor, then call cursorDefine
> +static void cocoa_cursor_define(DisplayChangeListener *dcl, QEMUCursor *cursor) {
> +    CFDataRef cfdata = CFDataCreate(
> +        /*allocator*/ NULL,
> +        /*bytes*/ (void *)cursor->data,
> +        /*length*/ sizeof(uint32_t) * cursor->width * cursor->height);
> +
> +    CGDataProviderRef dataprovider = CGDataProviderCreateWithCFData(cfdata);
> +
> +    CGImageRef cgimage = CGImageCreate(
> +        cursor->width, cursor->height,
> +        /*bitsPerComponent*/ 8,
> +        /*bitsPerPixel*/ 32,
> +        /*bytesPerRow*/ sizeof(uint32_t) * cursor->width,
> +        /*colorspace*/ CGColorSpaceCreateWithName(kCGColorSpaceSRGB),
> +        /*bitmapInfo*/ kCGBitmapByteOrder32Host | kCGImageAlphaLast,
> +        /*provider*/ dataprovider,
> +        /*decode*/ NULL,
> +        /*shouldInterpolate*/ FALSE,
> +        /*intent*/ kCGRenderingIntentDefault);
> +
> +    NSImage *unscaled = [[NSImage alloc] initWithCGImage:cgimage size:NSZeroSize];
> +
> +    CFRelease(cfdata);
> +    CGDataProviderRelease(dataprovider);
> +    CGImageRelease(cgimage);
> +
> +    // Nearest-neighbor scale to the possibly "Retina" cursor size
> +    NSImage *scaled = [NSImage
> +        imageWithSize:NSMakeSize(cursor->width, cursor->height)
> +        flipped:NO
> +        drawingHandler:^BOOL(NSRect dest) {
> +            [NSGraphicsContext currentContext].imageInterpolation = NSImageInterpolationNone;
> +            [unscaled drawInRect:dest];
> +            return YES;
> +        }];
> +
> +    NSCursor *nscursor = [[NSCursor alloc]
> +        initWithImage:scaled
> +        hotSpot:NSMakePoint(cursor->hot_x, cursor->hot_y)];
> +
> +    dispatch_async(dispatch_get_main_queue(), ^{
> +        [cocoaView cursorDefine:nscursor];
> +    });
> +}
> +
> static QemuDisplay qemu_display_cocoa = {
>     .type       = DISPLAY_TYPE_COCOA,
>     .init       = cocoa_display_init,