This set of patches rearranges how we handle events on
the OSX Cocoa UI so that we use the main thread to run
the OSX event loop, and we don't do a long blocking
operation from the applicationDidFinishLaunching callback.
Instead we create a second thread which runs qemu_main()
and becomes the QEMU main-loop thread. The callbacks from
QEMU into the cocoa code asynchronously dispatch their
work to the main thread, and the main thread takes the
iothread lock before calling into QEMU code.

This code all works (though I have only smoke-tested it),
but it is marked RFC because it seems to be unclear whether
the current code in git master is really problematic for
Mojave. If it's not actually solving a problem then it's
a bit tricky to justify this rework, though it does mean
we're doing a less "weird" set of things and behaving a bit
more like how OSX expects apps to behave, so it might in
theory mean less chance of future breakage.

NB: the code to asynchronously run code blocks on the
main thread uses dispatch_get_main_queue(), which is a
10.10-or-later function. If we're going to go with this
refactoring I think the benefit in clarity-of-code is a
worthwhile gain for dropping support for ancient OSX versions.

Patchset structure:
 * patch 1 does the "make sure we have the iothread lock for
   calls into QEMU" (which is effectively a no-op initially
   since we'll already be holding that lock when our refresh
   etc callbacks are called)
 * patch 2 makes switchSurface directly take the pixman image
   (which is refcounted) rather than the DisplaySurface (which
   is not), so we can make the calls to it asynchronous later
 * patches 3 and 4 are just trivial code motion
 * patch 5 does the bulk of the work (and can't really be split
   further without the UI being broken at the intermediate point)

thanks
-- PMM

Peter Maydell (5):
  ui/cocoa: Ensure we have the iothread lock when calling into QEMU
  ui/cocoa: Use the pixman image directly in switchSurface
  ui/cocoa: Factor out initial menu creation
  ui/cocoa: Move console/device menu creation code up in file
  ui/cocoa: Perform UI operations only on the main thread

 ui/cocoa.m | 441 +++++++++++++++++++++++++++++++----------------------
 1 file changed, 258 insertions(+), 183 deletions(-)

-- 
2.19.2

On 12/1/18 6:30 AM, Peter Maydell wrote:
> Patchset structure:
>  * patch 1 does the "make sure we have the iothread lock for
>    calls into QEMU" (which is effectively a no-op initially
>    since we'll already be holding that lock when our refresh
>    etc callbacks are called)
>  * patch 2 makes switchSurface directly take the pixman image
>    (which is refcounted) rather than the DisplaySurface (which
>    is not), so we can make the calls to it asynchronous later
>  * patches 3 and 4 are just trivial code motion
>  * patch 5 does the bulk of the work (and can't really be split
>    further without the UI being broken at the intermediate point)

FWIW, this makes sense and is relatively easy to follow.
That said, I've never touched OSX at all, so can't even test.


r~

  Hi,

> NB: the code to asynchronously run code blocks on the
> main thread uses dispatch_get_main_queue(), which is a
> 10.10-or-later function. If we're going to go with this
> refactoring I think the benefit in clarity-of-code is a
> worthwhile gain for dropping support for ancient OSX versions.
> 
> Patchset structure:
>  * patch 1 does the "make sure we have the iothread lock for
>    calls into QEMU" (which is effectively a no-op initially
>    since we'll already be holding that lock when our refresh
>    etc callbacks are called)
>  * patch 2 makes switchSurface directly take the pixman image
>    (which is refcounted) rather than the DisplaySurface (which
>    is not), so we can make the calls to it asynchronous later
>  * patches 3 and 4 are just trivial code motion
>  * patch 5 does the bulk of the work (and can't really be split
>    further without the UI being broken at the intermediate point)

DisplaySurface / pixman changes look sane to me.
Can't really comment on the cocoa changes.

cheers,
  Gerd