[PATCH v2 3/4] ui/cocoa: Add cursor composition

Akihiko Odaki posted 4 patches 5 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Peter Maydell <peter.maydell@linaro.org>, Akihiko Odaki <akihiko.odaki@daynix.com>
[PATCH v2 3/4] ui/cocoa: Add cursor composition
Posted by Akihiko Odaki 5 months ago
Add accelerated cursor composition to ui/cocoa. This does not only
improve performance for display devices that exposes the capability to
the guest according to dpy_cursor_define_supported(), but fixes the
cursor display for devices that unconditionally expects the availability
of the capability (e.g., virtio-gpu).

The common pattern to implement accelerated cursor composition is to
replace the cursor and warp it so that the replaced cursor is shown at
the correct position on the guest display for relative pointer devices.
Unfortunately, ui/cocoa cannot do the same because warping the cursor
position interfers with the mouse input so it uses CALayer instead;
although it is not specialized for cursor composition, it still can
compose images with hardware acceleration.

Co-authored-by: Phil Dennis-Jordan <phil@philjordan.eu>
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 meson.build |  3 +-
 ui/cocoa.m  | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 638660714455..59cc1fb8bcfc 100644
--- a/meson.build
+++ b/meson.build
@@ -1041,7 +1041,8 @@ if get_option('attr').allowed()
   endif
 endif
 
-cocoa = dependency('appleframeworks', modules: ['Cocoa', 'CoreVideo'],
+cocoa = dependency('appleframeworks',
+                   modules: ['Cocoa', 'CoreVideo', 'QuartzCore'],
                    required: get_option('cocoa'))
 
 vmnet = dependency('appleframeworks', modules: 'vmnet', required: get_option('vmnet'))
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 908454a434c5..0dcd1923bfb3 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -25,6 +25,7 @@
 #include "qemu/osdep.h"
 
 #import <Cocoa/Cocoa.h>
+#import <QuartzCore/QuartzCore.h>
 #include <crt_externs.h>
 
 #include "qemu/help-texts.h"
@@ -79,12 +80,16 @@ static void cocoa_switch(DisplayChangeListener *dcl,
                          DisplaySurface *surface);
 
 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 *cursor);
 
 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,
@@ -300,6 +305,11 @@ @interface QemuCocoaView : NSView
     BOOL isAbsoluteEnabled;
     CFMachPortRef eventsTap;
     CGColorSpaceRef colorspace;
+    CALayer *cursorLayer;
+    QEMUCursor *cursor;
+    int mouseX;
+    int mouseY;
+    bool mouseOn;
 }
 - (void) switchSurface:(pixman_image_t *)image;
 - (void) grabMouse;
@@ -365,6 +375,12 @@ - (id)initWithFrame:(NSRect)frameRect
 #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_VERSION_14_0
         [self setClipsToBounds:YES];
 #endif
+        [self setWantsLayer:YES];
+        cursorLayer = [[CALayer alloc] init];
+        [cursorLayer setAnchorPoint:CGPointMake(0, 1)];
+        [cursorLayer setAutoresizingMask:kCALayerMaxXMargin |
+                                         kCALayerMinYMargin];
+        [[self layer] addSublayer:cursorLayer];
 
     }
     return self;
@@ -383,6 +399,8 @@ - (void) dealloc
     }
 
     CGColorSpaceRelease(colorspace);
+    [cursorLayer release];
+    cursor_unref(cursor);
     [super dealloc];
 }
 
@@ -426,6 +444,72 @@ - (void) unhideCursor
     [NSCursor unhide];
 }
 
+- (void)setMouseX:(int)x y:(int)y on:(bool)on
+{
+    CGPoint position;
+
+    mouseX = x;
+    mouseY = y;
+    mouseOn = on;
+
+    position.x = mouseX;
+    position.y = screen.height - mouseY;
+
+    [CATransaction begin];
+    [CATransaction setDisableActions:YES];
+    [cursorLayer setPosition:position];
+    [cursorLayer setHidden:!mouseOn];
+    [CATransaction commit];
+}
+
+- (void)setCursor:(QEMUCursor *)given_cursor
+{
+    CGDataProviderRef provider;
+    CGImageRef image;
+    CGRect bounds = CGRectZero;
+
+    cursor_unref(cursor);
+    cursor = given_cursor;
+
+    if (!cursor) {
+        return;
+    }
+
+    cursor_ref(cursor);
+
+    bounds.size.width = cursor->width;
+    bounds.size.height = cursor->height;
+
+    provider = CGDataProviderCreateWithData(
+        NULL,
+        cursor->data,
+        cursor->width * cursor->height * 4,
+        NULL
+    );
+
+    image = CGImageCreate(
+        cursor->width, //width
+        cursor->height, //height
+        8, //bitsPerComponent
+        32, //bitsPerPixel
+        cursor->width * 4, //bytesPerRow
+        colorspace, //colorspace
+        kCGBitmapByteOrder32Little | kCGImageAlphaFirst, //bitmapInfo
+        provider, //provider
+        NULL, //decode
+        0, //interpolate
+        kCGRenderingIntentDefault //intent
+    );
+
+    CGDataProviderRelease(provider);
+    [CATransaction begin];
+    [CATransaction setDisableActions:YES];
+    [cursorLayer setBounds:bounds];
+    [cursorLayer setContents:(id)image];
+    [CATransaction commit];
+    CGImageRelease(image);
+}
+
 - (void) drawRect:(NSRect) rect
 {
     COCOA_DEBUG("QemuCocoaView: drawRect\n");
@@ -2003,6 +2087,21 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
     [pool release];
 }
 
+static void cocoa_mouse_set(DisplayChangeListener *dcl, int x, int y, bool on)
+{
+    dispatch_async(dispatch_get_main_queue(), ^{
+        [cocoaView setMouseX:x y:y on:on];
+    });
+}
+
+static void cocoa_cursor_define(DisplayChangeListener *dcl, QEMUCursor *cursor)
+{
+    dispatch_async(dispatch_get_main_queue(), ^{
+        BQL_LOCK_GUARD();
+        [cocoaView setCursor:qemu_console_get_cursor(dcl->con)];
+    });
+}
+
 static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
 {
     NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];

-- 
2.45.2
Re: [PATCH v2 3/4] ui/cocoa: Add cursor composition
Posted by Phil Dennis-Jordan 4 months, 3 weeks ago
I've just tried to rebase my own patches on top of this work and noticed
the following typo:

On Thu, 27 Jun 2024 at 13:17, Akihiko Odaki <akihiko.odaki@daynix.com>
wrote:

>  static void cocoa_refresh(DisplayChangeListener *dcl);
> +static void cocoa_mouse_set(DisplayChangeListener *dcl, int x, int y, int
> on);


The above prototype does not match the below definition - note the type
mismatch on the last parameter:

+static void cocoa_mouse_set(DisplayChangeListener *dcl, int x, int y, bool
> on)
> +{
> +    dispatch_async(dispatch_get_main_queue(), ^{
> +        [cocoaView setMouseX:x y:y on:on];
> +    });
> +}
>
Re: [PATCH v2 3/4] ui/cocoa: Add cursor composition
Posted by Phil Dennis-Jordan 4 months, 3 weeks ago
On Thu, 27 Jun 2024 at 13:17, Akihiko Odaki <akihiko.odaki@daynix.com>
wrote:

> Co-authored-by: Phil Dennis-Jordan <phil@philjordan.eu>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>
> +- (void)setCursor:(QEMUCursor *)given_cursor
> +{
> +    […]
> +
> +    provider = CGDataProviderCreateWithData(
> +        NULL,
> +        cursor->data,
> +        cursor->width * cursor->height * 4,
> +        NULL
> +    );
> +
> +    image = CGImageCreate(
> +        cursor->width, //width
> +        cursor->height, //height
> +        8, //bitsPerComponent
> +        32, //bitsPerPixel
> +        cursor->width * 4, //bytesPerRow
> +        colorspace, //colorspace
> +        kCGBitmapByteOrder32Little | kCGImageAlphaFirst, //bitmapInfo
> +        provider, //provider
> +        NULL, //decode
> +        0, //interpolate
> +        kCGRenderingIntentDefault //intent
> +    );
>

I still think this is an awkward amount of boilerplate that could be
outsourced to a helper function - especially as you've now reminded me in
patch 1/4 that drawRect: does essentially the same thing and could probably
share that helper.

I'm still keen on NSCursor support for absolute pointing mode though, so I
can experiment with doing a better job of cleaning it up as part v3 of that
patch series. So:

Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
Re: [PATCH v2 3/4] ui/cocoa: Add cursor composition
Posted by Philippe Mathieu-Daudé 4 months, 3 weeks ago
Hi Phil,

On 2/7/24 15:19, Phil Dennis-Jordan wrote:

> I'm still keen on NSCursor support for absolute pointing mode though, so 
> I can experiment with doing a better job of cleaning it up as part v3 of 
> that patch series.

Do we need a v3, or can you clean on top?

Regards,

Phil :)
Re: [PATCH v2 3/4] ui/cocoa: Add cursor composition
Posted by Phil Dennis-Jordan 4 months, 3 weeks ago
On Tue, 2 Jul 2024 at 16:20, Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> Hi Phil,
>
> On 2/7/24 15:19, Phil Dennis-Jordan wrote:
>
> > I'm still keen on NSCursor support for absolute pointing mode though, so
> > I can experiment with doing a better job of cleaning it up as part v3 of
> > that patch series.
>
> Do we need a v3, or can you clean on top?
>

Sorry, I meant v3 of my NSCursor patch, not this series.

https://patchew.org/QEMU/20240625134931.92279-1-phil@philjordan.eu/

(That in turn uses the CGImage for the NSCursor as well)

So from my point of view, this series can be merged, and I'll submit v3 of
my series, which will aim to clean up the CGImage code as well as
implementing NSCursor support in absolute pointing mode.

Thanks!
Phil
Re: [PATCH v2 3/4] ui/cocoa: Add cursor composition
Posted by Philippe Mathieu-Daudé 4 months, 3 weeks ago
On 2/7/24 16:25, Phil Dennis-Jordan wrote:
> 
> 
> On Tue, 2 Jul 2024 at 16:20, Philippe Mathieu-Daudé <philmd@linaro.org 
> <mailto:philmd@linaro.org>> wrote:
> 
>     Hi Phil,
> 
>     On 2/7/24 15:19, Phil Dennis-Jordan wrote:
> 
>      > I'm still keen on NSCursor support for absolute pointing mode
>     though, so
>      > I can experiment with doing a better job of cleaning it up as
>     part v3 of
>      > that patch series.
> 
>     Do we need a v3, or can you clean on top?
> 
> 
> Sorry, I meant v3 of my NSCursor patch, not this series.
> 
> https://patchew.org/QEMU/20240625134931.92279-1-phil@philjordan.eu/ 
> <https://patchew.org/QEMU/20240625134931.92279-1-phil@philjordan.eu/>
> 
> (That in turn uses the CGImage for the NSCursor as well)

Ah OK, thanks for clarifying :)

> So from my point of view, this series can be merged, and I'll submit v3 
> of my series, which will aim to clean up the CGImage code as well as 
> implementing NSCursor support in absolute pointing mode.
> 
> Thanks!
> Phil
>