diff mbox series

ui/cocoa: Adding cursor support

Message ID BE04B88B-9AAC-4348-A165-E115CBE54077@me.com
State New
Headers show
Series ui/cocoa: Adding cursor support | expand

Commit Message

Cameron Esfahani via March 12, 2019, 1:47 a.m. UTC
This patch added cocoa_mouse_set() and cocoa_cursor_define(),
supporting virtio-gpu simple rendering on macOS host.

Image content, rect and visibility of the cursor buffer were added as
ivars in QemuCocoaView class. Corresponding accessors were added.

Note that the rect of the cursor was in coordinates of the QEMU
screen, not the NSScreen or the NSView, for convenience of blending.

Signed-off-by: Chen Zhang <tgfbeta@me.com>
---
 ui/cocoa.m | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

Comments

no-reply@patchew.org March 12, 2019, 1:58 a.m. UTC | #1
Patchew URL: https://patchew.org/QEMU/BE04B88B-9AAC-4348-A165-E115CBE54077@me.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: BE04B88B-9AAC-4348-A165-E115CBE54077@me.com
Subject: [Qemu-devel] [PATCH] ui/cocoa: Adding cursor support

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/BE04B88B-9AAC-4348-A165-E115CBE54077@me.com -> patchew/BE04B88B-9AAC-4348-A165-E115CBE54077@me.com
Switched to a new branch 'test'
0fa666fc9e ui/cocoa: Adding cursor support

=== OUTPUT BEGIN ===
ERROR: Author email address is mangled by the mailing list
#2: 
Author: Chen Zhang via Qemu-devel <qemu-devel@nongnu.org>

total: 1 errors, 0 warnings, 121 lines checked

Commit 0fa666fc9e29 (ui/cocoa: Adding cursor support) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/BE04B88B-9AAC-4348-A165-E115CBE54077@me.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Gerd Hoffmann March 12, 2019, 7:26 a.m. UTC | #2
Hi,

> +        if (cursorVisible && cursorImage && NSIntersectsRect(rect, cursorRect)) {
> +            CGContextDrawImage (viewContextRef, cursorRect, cursorImage);

So you are rendering the cursor to the window.

Better approach would be to just set the cursor of the host window,
like the gtk UI does, using gdk_window_set_cursor().

cheers,
  Gerd
Cameron Esfahani via March 12, 2019, 11:45 a.m. UTC | #3
Hi,

I did try to utilize NSCursor and CGWarpMouseCursorPosition API before this compromise. In cocoa_mouse_set, the position of cursor should to be modified, but the bottom half that called it was not scheduled on main thread. UI operations have to be queued on main thread asynchronously thereafter. This introduced troubles.

One issue was that once CGWarpMouseCursorPosition(), which should not trigger any mouse events, was called from cocoa_mouse_set(), the cursor position accelerated in a positive feedback manner. This was independent from the association state between mouse movement and cursor.

Another issue was that the cursor moved several steps later than the Cocoa mouse events.

All these phenomena made me wonder if I was messing up with the host input source, runloop, the bottom halves and the asynchronous changes made to cursor positions.

On the other hand, rendering the cursor in the window frame buffer only introduce a few more dirty rectangles per second and this does not seem to add up any significant amount of overhead. At least it keeps the troubles above away.

Best regards,

> On Mar 12, 2019, at 3:26 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
>  Hi,
> 
>> +        if (cursorVisible && cursorImage && NSIntersectsRect(rect, cursorRect)) {
>> +            CGContextDrawImage (viewContextRef, cursorRect, cursorImage);
> 
> So you are rendering the cursor to the window.
> 
> Better approach would be to just set the cursor of the host window,
> like the gtk UI does, using gdk_window_set_cursor().
> 
> cheers,
>  Gerd
BALATON Zoltan March 12, 2019, 12:32 p.m. UTC | #4
On Tue, 12 Mar 2019, Chen Zhang via Qemu-devel wrote:
> Hi,
>
> I did try to utilize NSCursor and CGWarpMouseCursorPosition API before 
> this compromise. In cocoa_mouse_set, the position of cursor should to be 
> modified, but the bottom half that called it was not scheduled on main 
> thread. UI operations have to be queued on main thread asynchronously 
> thereafter. This introduced troubles.
>
> One issue was that once CGWarpMouseCursorPosition(), which should not 
> trigger any mouse events, was called from cocoa_mouse_set(), the cursor 
> position accelerated in a positive feedback manner. This was independent 
> from the association state between mouse movement and cursor.
>
> Another issue was that the cursor moved several steps later than the 
> Cocoa mouse events.
>
> All these phenomena made me wonder if I was messing up with the host 
> input source, runloop, the bottom halves and the asynchronous changes 
> made to cursor positions.
>
> On the other hand, rendering the cursor in the window frame buffer only 
> introduce a few more dirty rectangles per second and this does not seem 
> to add up any significant amount of overhead. At least it keeps the 
> troubles above away.

We've also found similar mouse jumping problems with the ati-vga after 
using define_cursor and host side cursor but also another problem where 
cursor turned into a black square sometimes. I think the latter happens 
when guest sets the memory offset to the cursor (which is when I call 
cursor_define) but only writes cursor data there later. This works if 
cursor data is read only when rendering the cursor but fails in QEMU with 
cursor_define. Problem does not happen with guest_hwcursor option of 
ati-vga that uses the cirrus callbacks that read cursor data when 
rendering but that needs another display surface to render cursor into. I 
cannot fix this by calling define_cursor on all register writes because 
guest writes offset on every mouse move even if it's not changed and 
calling cursor_define causes flickering of the pointer.

So I wonder if this defining cursor in the host is a good idea after all 
given all the above problems with it. (It might be wanted for remote 
displays as an optimisation with tradeoffs for accuracy but not as a 
default on local displays.) Could it be modified to be called from screen 
update at least instead of being completely async which might fix some of 
the problems. (I can't do that from ati-vga itself because I don't want to 
have custom screen refresh function just for this which would need 
reimplementing all of vga.) Or maybe the cirrus callbacks could be fixed 
to render cursor in a similar way like this patch and then use that?

I think my point is that there seems to be a problem with host side cursor 
which would need to be solved more generally within QEMU instead of 
inventing workarounds individually within device models and ui backends.

Regards,
BALATON Zoltan

>
> Best regards,
>
>> On Mar 12, 2019, at 3:26 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>
>>  Hi,
>>
>>> +        if (cursorVisible && cursorImage && NSIntersectsRect(rect, cursorRect)) {
>>> +            CGContextDrawImage (viewContextRef, cursorRect, cursorImage);
>>
>> So you are rendering the cursor to the window.
>>
>> Better approach would be to just set the cursor of the host window,
>> like the gtk UI does, using gdk_window_set_cursor().
>>
>> cheers,
>>  Gerd
>
>
Cameron Esfahani via March 13, 2019, 5:07 a.m. UTC | #5
Hi,

I sympathize with your situation, but the things on macOS seems a little different.

The QEMU Cocoa UI starts in the `main` thread and detach a `qemu_main` thread which runs stuff in vl.c etc. On the contrary, QEMU with gtk and other UIs just scheduled its event loop in the qemu_main thread without detaching a dedicated thread. These two kinds of event loop(, the cocoa one and the glib one,) do not mix well. 

A second problem is that recently Cocoa UI event handling routine is locking io threads with qemu_mutex_lock_iothread(), and this may delay bottom halves and subsequently the dpy_mouse_set calls, as glib_pollfds_poll() is also protected by qemu_mutex_lock_iothread().

My preliminary guess is that all mouse NSEvents must be queued and processed in a way not breaking any causal order.  Maybe the next NSEvent shall not be processed before the corresponding dpy_mouse_set call.

> On Mar 12, 2019, at 8:32 PM, BALATON Zoltan <balaton@eik.bme.hu> wrote:
> 
> On Tue, 12 Mar 2019, Chen Zhang via Qemu-devel wrote:
>> Hi,
>> 
>> I did try to utilize NSCursor and CGWarpMouseCursorPosition API before this compromise. In cocoa_mouse_set, the position of cursor should to be modified, but the bottom half that called it was not scheduled on main thread. UI operations have to be queued on main thread asynchronously thereafter. This introduced troubles.
>> 
>> One issue was that once CGWarpMouseCursorPosition(), which should not trigger any mouse events, was called from cocoa_mouse_set(), the cursor position accelerated in a positive feedback manner. This was independent from the association state between mouse movement and cursor.
>> 
>> Another issue was that the cursor moved several steps later than the Cocoa mouse events.
>> 
>> All these phenomena made me wonder if I was messing up with the host input source, runloop, the bottom halves and the asynchronous changes made to cursor positions.
>> 
>> On the other hand, rendering the cursor in the window frame buffer only introduce a few more dirty rectangles per second and this does not seem to add up any significant amount of overhead. At least it keeps the troubles above away.
> 
> We've also found similar mouse jumping problems with the ati-vga after using define_cursor and host side cursor but also another problem where cursor turned into a black square sometimes. I think the latter happens when guest sets the memory offset to the cursor (which is when I call cursor_define) but only writes cursor data there later. This works if cursor data is read only when rendering the cursor but fails in QEMU with cursor_define. Problem does not happen with guest_hwcursor option of ati-vga that uses the cirrus callbacks that read cursor data when rendering but that needs another display surface to render cursor into. I cannot fix this by calling define_cursor on all register writes because guest writes offset on every mouse move even if it's not changed and calling cursor_define causes flickering of the pointer.
Is it possible to compare the cursor pixel data before calling define_cursor and avoid some redundant calls?
> 
> So I wonder if this defining cursor in the host is a good idea after all given all the above problems with it. (It might be wanted for remote displays as an optimisation with tradeoffs for accuracy but not as a default on local displays.) Could it be modified to be called from screen update at least instead of being completely async which might fix some of the problems. (I can't do that from ati-vga itself because I don't want to have custom screen refresh function just for this which would need reimplementing all of vga.) Or maybe the cirrus callbacks could be fixed to render cursor in a similar way like this patch and then use that?
> 
> I think my point is that there seems to be a problem with host side cursor which would need to be solved more generally within QEMU instead of inventing workarounds individually within device models and ui backends.
> 
> Regards,
> BALATON Zoltan
> 
>> 
>> Best regards,
>> 
>>> On Mar 12, 2019, at 3:26 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>> 
>>> Hi,
>>> 
>>>> +        if (cursorVisible && cursorImage && NSIntersectsRect(rect, cursorRect)) {
>>>> +            CGContextDrawImage (viewContextRef, cursorRect, cursorImage);
>>> 
>>> So you are rendering the cursor to the window.
>>> 
>>> Better approach would be to just set the cursor of the host window,
>>> like the gtk UI does, using gdk_window_set_cursor().
>>> 
>>> cheers,
>>> Gerd
BALATON Zoltan March 13, 2019, 5:35 p.m. UTC | #6
On Wed, 13 Mar 2019, Chen Zhang wrote:
> I sympathize with your situation, but the things on macOS seems a little different.
>
> The QEMU Cocoa UI starts in the `main` thread and detach a `qemu_main` 
> thread which runs stuff in vl.c etc. On the contrary, QEMU with gtk and 
> other UIs just scheduled its event loop in the qemu_main thread without 
> detaching a dedicated thread. These two kinds of event loop(, the cocoa 
> one and the glib one,) do not mix well.

I was following the recent ui/cocoa.m changes so I'm aware of this.

> A second problem is that recently Cocoa UI event handling routine is 
> locking io threads with qemu_mutex_lock_iothread(), and this may delay 
> bottom halves and subsequently the dpy_mouse_set calls, as 
> glib_pollfds_poll() is also protected by qemu_mutex_lock_iothread().
>
> My preliminary guess is that all mouse NSEvents must be queued and 
> processed in a way not breaking any causal order.  Maybe the next 
> NSEvent shall not be processed before the corresponding dpy_mouse_set 
> call.

Sorry if this was not clear but the cursor change to black square problem 
is not cocoa UI related and we've actually seen it on Linux with gtk (I 
think, it was reported to me, not reproduced by me). Maybe I should not 
have mixed that in here and report independently.

Based on my experience with the ati-vga model where I've implemented 
hardware cursor both with dpy_cursor_define()/dpy_mouse_set() and with 
cursor_invalidate()/cursor_draw_line() callbacks I've found the latter to 
work more smoothly. I don't know why jumpy mouse pointer happens with the 
dpy_mouse_set() way but that's what I see. With mouse drawn from screen 
refresh the pointer may stop when guest is busy but it won't jump around 
unlike when it's updated on the host side on register writes not tied to 
screen refresh. I don't know how much does it relate to your case with 
Cocoa UI but there may be some similarities that's why I've brought this 
up here. If it's not related just ignore my comment.

Regards,
BALATON Zoltan

>
>> On Mar 12, 2019, at 8:32 PM, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>
>> On Tue, 12 Mar 2019, Chen Zhang via Qemu-devel wrote:
>>> Hi,
>>>
>>> I did try to utilize NSCursor and CGWarpMouseCursorPosition API before this compromise. In cocoa_mouse_set, the position of cursor should to be modified, but the bottom half that called it was not scheduled on main thread. UI operations have to be queued on main thread asynchronously thereafter. This introduced troubles.
>>>
>>> One issue was that once CGWarpMouseCursorPosition(), which should not trigger any mouse events, was called from cocoa_mouse_set(), the cursor position accelerated in a positive feedback manner. This was independent from the association state between mouse movement and cursor.
>>>
>>> Another issue was that the cursor moved several steps later than the Cocoa mouse events.
>>>
>>> All these phenomena made me wonder if I was messing up with the host input source, runloop, the bottom halves and the asynchronous changes made to cursor positions.
>>>
>>> On the other hand, rendering the cursor in the window frame buffer only introduce a few more dirty rectangles per second and this does not seem to add up any significant amount of overhead. At least it keeps the troubles above away.
>>
>> We've also found similar mouse jumping problems with the ati-vga after using define_cursor and host side cursor but also another problem where cursor turned into a black square sometimes. I think the latter happens when guest sets the memory offset to the cursor (which is when I call cursor_define) but only writes cursor data there later. This works if cursor data is read only when rendering the cursor but fails in QEMU with cursor_define. Problem does not happen with guest_hwcursor option of ati-vga that uses the cirrus callbacks that read cursor data when rendering but that needs another display surface to render cursor into. I cannot fix this by calling define_cursor on all register writes because guest writes offset on every mouse move even if it's not changed and calling cursor_define causes flickering of the pointer.
> Is it possible to compare the cursor pixel data before calling define_cursor and avoid some redundant calls?
>>
>> So I wonder if this defining cursor in the host is a good idea after all given all the above problems with it. (It might be wanted for remote displays as an optimisation with tradeoffs for accuracy but not as a default on local displays.) Could it be modified to be called from screen update at least instead of being completely async which might fix some of the problems. (I can't do that from ati-vga itself because I don't want to have custom screen refresh function just for this which would need reimplementing all of vga.) Or maybe the cirrus callbacks could be fixed to render cursor in a similar way like this patch and then use that?
>>
>> I think my point is that there seems to be a problem with host side cursor which would need to be solved more generally within QEMU instead of inventing workarounds individually within device models and ui backends.
>>
>> Regards,
>> BALATON Zoltan
>>
>>>
>>> Best regards,
>>>
>>>> On Mar 12, 2019, at 3:26 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>>> +        if (cursorVisible && cursorImage && NSIntersectsRect(rect, cursorRect)) {
>>>>> +            CGContextDrawImage (viewContextRef, cursorRect, cursorImage);
>>>>
>>>> So you are rendering the cursor to the window.
>>>>
>>>> Better approach would be to just set the cursor of the host window,
>>>> like the gtk UI does, using gdk_window_set_cursor().
>>>>
>>>> cheers,
>>>> Gerd
>
>
>
Cameron Esfahani via March 14, 2019, 8:47 a.m. UTC | #7
> On Mar 14, 2019, at 1:35 AM, BALATON Zoltan <balaton@eik.bme.hu> wrote:
> 
> On Wed, 13 Mar 2019, Chen Zhang wrote:
>> I sympathize with your situation, but the things on macOS seems a little different.
>> 
>> The QEMU Cocoa UI starts in the `main` thread and detach a `qemu_main` thread which runs stuff in vl.c etc. On the contrary, QEMU with gtk and other UIs just scheduled its event loop in the qemu_main thread without detaching a dedicated thread. These two kinds of event loop(, the cocoa one and the glib one,) do not mix well.
> 
> I was following the recent ui/cocoa.m changes so I'm aware of this.
> 
>> A second problem is that recently Cocoa UI event handling routine is locking io threads with qemu_mutex_lock_iothread(), and this may delay bottom halves and subsequently the dpy_mouse_set calls, as glib_pollfds_poll() is also protected by qemu_mutex_lock_iothread().
>> 
>> My preliminary guess is that all mouse NSEvents must be queued and processed in a way not breaking any causal order.  Maybe the next NSEvent shall not be processed before the corresponding dpy_mouse_set call.
> 
> Sorry if this was not clear but the cursor change to black square problem is not cocoa UI related and we've actually seen it on Linux with gtk (I think, it was reported to me, not reproduced by me). Maybe I should not have mixed that in here and report independently.
> 
> Based on my experience with the ati-vga model where I've implemented hardware cursor both with dpy_cursor_define()/dpy_mouse_set() and with cursor_invalidate()/cursor_draw_line() callbacks I've found the latter to work more smoothly. I don't know why jumpy mouse pointer happens with the dpy_mouse_set() way but that's what I see. With mouse drawn from screen refresh the pointer may stop when guest is busy but it won't jump around unlike when it's updated on the host side on register writes not tied to screen refresh. I don't know how much does it relate to your case with Cocoa UI but there may be some similarities that's why I've brought this up here. If it's not related just ignore my comment.
For the Cocoa UI, I tried to isolate the cursor movement issue after calling CGWarpMouseCursorPosition(). 

In an empty Cocoa window app with a minimal overridden -sendEvent:, a minimal -handleEvent:, calling CGWarpMouseCursorPosition calls upon mouse-moved event showed the similar cursor movement issue as in QEMU.

It was suggested that Apple throttled how frequently the cursor could be moved in earlier version of macOS. [1][2] However none of these workaround methods I found worked on Mojave so far.

Best Regards,

[1] https://stackoverflow.com/questions/10196603/using-cgeventsourcesetlocaleventssuppressioninterval-instead-of-the-deprecated

[2] https://lists.apple.com/archives/cocoa-dev/2012/Feb/msg00372.html
> 
> Regards,
> BALATON Zoltan
> 
>> 
>>> On Mar 12, 2019, at 8:32 PM, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>> 
>>> On Tue, 12 Mar 2019, Chen Zhang via Qemu-devel wrote:
>>>> Hi,
>>>> 
>>>> I did try to utilize NSCursor and CGWarpMouseCursorPosition API before this compromise. In cocoa_mouse_set, the position of cursor should to be modified, but the bottom half that called it was not scheduled on main thread. UI operations have to be queued on main thread asynchronously thereafter. This introduced troubles.
>>>> 
>>>> One issue was that once CGWarpMouseCursorPosition(), which should not trigger any mouse events, was called from cocoa_mouse_set(), the cursor position accelerated in a positive feedback manner. This was independent from the association state between mouse movement and cursor.
>>>> 
>>>> Another issue was that the cursor moved several steps later than the Cocoa mouse events.
>>>> 
>>>> All these phenomena made me wonder if I was messing up with the host input source, runloop, the bottom halves and the asynchronous changes made to cursor positions.
>>>> 
>>>> On the other hand, rendering the cursor in the window frame buffer only introduce a few more dirty rectangles per second and this does not seem to add up any significant amount of overhead. At least it keeps the troubles above away.
>>> 
>>> We've also found similar mouse jumping problems with the ati-vga after using define_cursor and host side cursor but also another problem where cursor turned into a black square sometimes. I think the latter happens when guest sets the memory offset to the cursor (which is when I call cursor_define) but only writes cursor data there later. This works if cursor data is read only when rendering the cursor but fails in QEMU with cursor_define. Problem does not happen with guest_hwcursor option of ati-vga that uses the cirrus callbacks that read cursor data when rendering but that needs another display surface to render cursor into. I cannot fix this by calling define_cursor on all register writes because guest writes offset on every mouse move even if it's not changed and calling cursor_define causes flickering of the pointer.
>> Is it possible to compare the cursor pixel data before calling define_cursor and avoid some redundant calls?
>>> 
>>> So I wonder if this defining cursor in the host is a good idea after all given all the above problems with it. (It might be wanted for remote displays as an optimisation with tradeoffs for accuracy but not as a default on local displays.) Could it be modified to be called from screen update at least instead of being completely async which might fix some of the problems. (I can't do that from ati-vga itself because I don't want to have custom screen refresh function just for this which would need reimplementing all of vga.) Or maybe the cirrus callbacks could be fixed to render cursor in a similar way like this patch and then use that?
>>> 
>>> I think my point is that there seems to be a problem with host side cursor which would need to be solved more generally within QEMU instead of inventing workarounds individually within device models and ui backends.
>>> 
>>> Regards,
>>> BALATON Zoltan
>>> 
>>>> 
>>>> Best regards,
>>>> 
>>>>> On Mar 12, 2019, at 3:26 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>>>> 
>>>>> Hi,
>>>>> 
>>>>>> +        if (cursorVisible && cursorImage && NSIntersectsRect(rect, cursorRect)) {
>>>>>> +            CGContextDrawImage (viewContextRef, cursorRect, cursorImage);
>>>>> 
>>>>> So you are rendering the cursor to the window.
>>>>> 
>>>>> Better approach would be to just set the cursor of the host window,
>>>>> like the gtk UI does, using gdk_window_set_cursor().
>>>>> 
>>>>> cheers,
>>>>> Gerd
diff mbox series

Patch

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 420b2411c1..8beed6e514 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -334,6 +334,9 @@  static void handleAnyDeviceErrors(Error * err)
     BOOL isFullscreen;
     BOOL isAbsoluteEnabled;
     BOOL isMouseDeassociated;
+    CGRect cursorRect;
+    CGImageRef cursorImage;
+    BOOL cursorVisible;
 }
 - (void) switchSurface:(pixman_image_t *)image;
 - (void) grabMouse;
@@ -362,6 +365,12 @@  static void handleAnyDeviceErrors(Error * err)
 - (float) cdy;
 - (QEMUScreen) gscreen;
 - (void) raiseAllKeys;
+- (CGRect) cursorRect;
+- (void) setCursorRect:(CGRect)rect;
+- (CGImageRef) cursorImage;
+- (void) setCursorImage:(CGImageRef)image;
+- (BOOL) isCursorVisible;
+- (void) setCursorVisible:(BOOL)visible;
 @end
 
 QemuCocoaView *cocoaView;
@@ -392,6 +401,10 @@  QemuCocoaView *cocoaView;
         pixman_image_unref(pixman_image);
     }
 
+    if (cursorImage) {
+        CGImageRelease(cursorImage);
+    }
+
     [super dealloc];
 }
 
@@ -479,6 +492,10 @@  QemuCocoaView *cocoaView;
                                                         );
             CGContextDrawImage (viewContextRef, cgrect(rectList[i]), clipImageRef);
             CGImageRelease (clipImageRef);
+
+        }
+        if (cursorVisible && cursorImage && NSIntersectsRect(rect, cursorRect)) {
+            CGContextDrawImage (viewContextRef, cursorRect, cursorImage);
         }
         CGImageRelease (imageRef);
     }
@@ -987,6 +1004,19 @@  QemuCocoaView *cocoaView;
 - (float) cdy {return cdy;}
 - (QEMUScreen) gscreen {return screen;}
 
+- (CGRect) cursorRect {return cursorRect;}
+- (void) setCursorRect:(CGRect)rect {cursorRect = rect;}
+- (CGImageRef) cursorImage {return cursorImage;}
+- (void) setCursorImage:(CGImageRef)image
+{
+    if (cursorImage && cursorImage != image) {
+        CGImageRelease(cursorImage);
+    }
+    cursorImage = image;
+}
+- (BOOL) isCursorVisible {return cursorVisible;}
+- (void) setCursorVisible:(BOOL)visible {cursorVisible = visible;}
+
 /*
  * Makes the target think all down keys are being released.
  * This prevents a stuck key problem, since we will not see
@@ -1830,6 +1860,53 @@  static void cocoa_refresh(DisplayChangeListener *dcl)
     [pool release];
 }
 
+static void cocoa_cursor_define(DisplayChangeListener *dcl, QEMUCursor *c)
+{
+    NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+    int bitsPerComponent = [cocoaView gscreen].bitsPerComponent;
+    int bitsPerPixel = [cocoaView gscreen].bitsPerPixel;
+    CGDataProviderRef provider = CGDataProviderCreateWithData(NULL, c->data, c->width * 4 * c->height, NULL);
+
+    CGImageRef img = CGImageCreate(c->width, c->height,
+                                   bitsPerComponent, bitsPerPixel, c->width * bitsPerComponent / 2,
+#ifdef __LITTLE_ENDIAN__
+                                   CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), //colorspace for OS X >= 10.4
+                                   kCGBitmapByteOrder32Little | kCGImageAlphaFirst,
+#else
+                                   CGColorSpaceCreateDeviceRGB(), //colorspace for OS X < 10.4 (actually ppc)
+                                   kCGImageAlphaFirst, //bitmapInfo
+#endif
+                                   provider, NULL, 0, kCGRenderingIntentDefault);
+
+    CGDataProviderRelease(provider);
+    CGFloat width = c->width;
+    CGFloat height = c->height;
+    dispatch_async(dispatch_get_main_queue(), ^{
+        [cocoaView setCursorImage:img];
+        CGRect rect = [cocoaView cursorRect];
+        rect.size = CGSizeMake(width, height);
+        [cocoaView setCursorRect:rect];
+    });
+    [pool release];
+}
+static void cocoa_mouse_set(DisplayChangeListener *dcl,
+                            int x, int y, int visible)
+{
+    NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+    dispatch_async(dispatch_get_main_queue(), ^{
+        QEMUScreen screen = [cocoaView gscreen];
+        CGRect rect = [cocoaView cursorRect];
+        // Mark old cursor rect as dirty
+        [cocoaView setNeedsDisplayInRect:rect];
+        rect.origin = CGPointMake(x, screen.height - (y + rect.size.height));
+        [cocoaView setCursorRect:rect];
+        [cocoaView setCursorVisible:visible ? YES : NO];
+        // Mark new cursor rect as dirty
+        [cocoaView setNeedsDisplayInRect:rect];
+    });
+    [pool release];
+}
+
 static void cocoa_cleanup(void)
 {
     COCOA_DEBUG("qemu_cocoa: cocoa_cleanup\n");
@@ -1841,6 +1918,8 @@  static const DisplayChangeListenerOps dcl_ops = {
     .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 void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)