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

[Chen Zhang]

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