[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] ui/cocoa: Adding cursor support
From: |
Chen Zhang |
Subject: |
Re: [Qemu-devel] [PATCH] ui/cocoa: Adding cursor support |
Date: |
Thu, 14 Mar 2019 16:47:40 +0800 |
> On Mar 14, 2019, at 1:35 AM, BALATON Zoltan <address@hidden> 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 <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