Re: [Qemu-devel] [PATCH] ui/cocoa: Adding cursor support

[BALATON Zoltan]

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 <address@hidden> 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