* [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
@ 2019-02-14 10:28 Peter Maydell
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 1/7] ui/cocoa: Ensure we have the iothread lock when calling into QEMU Peter Maydell
` (15 more replies)
0 siblings, 16 replies; 38+ messages in thread
From: Peter Maydell @ 2019-02-14 10:28 UTC (permalink / raw)
To: qemu-devel
Cc: patches, John Arbuckle, Roman Bolshakov, Berkus Decker,
Gerd Hoffmann, Ben Hekster, BALATON Zoltan
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.
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. I think the benefit in
clarity-of-code is a worthwhile gain for dropping support
for ancient OSX versions.
The change from v1 is that we actually wire up input events
from OSX to the handleEvent method. There are a couple of
plausible ways we could do this:
* most OSX apps do this by implementing event handling
methods on their NSView. This won't work for us, because
these only get called after OSX has dealt with some events
like menu bar accelerator keystrokes, and we would like to
send those to the guest instead in some situations.
* we could use addLocalMonitorForEventsMatchingMask to
add a "local event monitor", which gets to examine/filter
events before the usual OSX event processing. I think this
would work, but it's not what I chose
* we can subclass NSApplication and implement a custom
sendEvent method in our subclass. This gets all events
first and can either handle them or call the superclass
sendEvent to hand them to OSX as usual. This is what I
ended up going for.
* It is also possible to implement sendEvent on NSWindow,
but I think this has the "only called after OSX has done
menu keystroke processing" issue.
I've tested this a bit better than the v1 RFC, and it seems
to work OK for me.
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 (new in v2) restructures handleEvent so it doesn't
directly call sendEvent but instead returns a flag indicating
whether it consumed the event or not
* patch 6 (new in v2) subclasses NSApplication
* patch 7 (the old 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 (7):
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: Don't call NSApp sendEvent directly from handleEvent
ui/cocoa: Subclass NSApplication so we can implement sendEvent
ui/cocoa: Perform UI operations only on the main thread
ui/cocoa.m | 483 ++++++++++++++++++++++++++++++++---------------------
1 file changed, 294 insertions(+), 189 deletions(-)
--
2.17.2 (Apple Git-113)
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 1/7] ui/cocoa: Ensure we have the iothread lock when calling into QEMU
2019-02-14 10:28 [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop Peter Maydell
@ 2019-02-14 10:28 ` Peter Maydell
2019-02-22 15:19 ` Roman Bolshakov
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 2/7] ui/cocoa: Use the pixman image directly in switchSurface Peter Maydell
` (14 subsequent siblings)
15 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2019-02-14 10:28 UTC (permalink / raw)
To: qemu-devel
Cc: patches, John Arbuckle, Roman Bolshakov, Berkus Decker,
Gerd Hoffmann, Ben Hekster, BALATON Zoltan
The Cocoa UI should run on the main thread; this is enforced
in OSX Mojave. In order to be able to run on the main thread,
we need to make sure we hold the iothread lock whenever we
call into various QEMU UI midlayer functions.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
ui/cocoa.m | 83 ++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 59 insertions(+), 24 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index e2567d6946..2931c751fd 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -129,6 +129,21 @@
NSTextField *pauseLabel;
NSArray * supportedImageFileTypes;
+// Utility function to run specified code block with iothread lock held
+typedef void (^CodeBlock)(void);
+
+static void with_iothread_lock(CodeBlock block)
+{
+ bool locked = qemu_mutex_iothread_locked();
+ if (!locked) {
+ qemu_mutex_lock_iothread();
+ }
+ block();
+ if (!locked) {
+ qemu_mutex_unlock_iothread();
+ }
+}
+
// Mac to QKeyCode conversion
const int mac_to_qkeycode_map[] = {
[kVK_ANSI_A] = Q_KEY_CODE_A,
@@ -306,6 +321,7 @@ - (void) ungrabMouse;
- (void) toggleFullScreen:(id)sender;
- (void) handleMonitorInput:(NSEvent *)event;
- (void) handleEvent:(NSEvent *)event;
+- (void) handleEventLocked:(NSEvent *)event;
- (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
/* The state surrounding mouse grabbing is potentially confusing.
* isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated
@@ -649,8 +665,14 @@ - (void) handleMonitorInput:(NSEvent *)event
- (void) handleEvent:(NSEvent *)event
{
- COCOA_DEBUG("QemuCocoaView: handleEvent\n");
+ with_iothread_lock(^{
+ [self handleEventLocked:event];
+ });
+}
+- (void) handleEventLocked:(NSEvent *)event
+{
+ COCOA_DEBUG("QemuCocoaView: handleEvent\n");
int buttons = 0;
int keycode = 0;
bool mouse_event = false;
@@ -945,15 +967,18 @@ - (QEMUScreen) gscreen {return screen;}
*/
- (void) raiseAllKeys
{
- int index;
const int max_index = ARRAY_SIZE(modifiers_state);
- for (index = 0; index < max_index; index++) {
- if (modifiers_state[index]) {
- modifiers_state[index] = 0;
- qemu_input_event_send_key_qcode(dcl->con, index, false);
- }
- }
+ with_iothread_lock(^{
+ int index;
+
+ for (index = 0; index < max_index; index++) {
+ if (modifiers_state[index]) {
+ modifiers_state[index] = 0;
+ qemu_input_event_send_key_qcode(dcl->con, index, false);
+ }
+ }
+ });
}
@end
@@ -1215,13 +1240,17 @@ - (void)removePause
/* Restarts QEMU */
- (void)restartQEMU:(id)sender
{
- qmp_system_reset(NULL);
+ with_iothread_lock(^{
+ qmp_system_reset(NULL);
+ });
}
/* Powers down QEMU */
- (void)powerDownQEMU:(id)sender
{
- qmp_system_powerdown(NULL);
+ with_iothread_lock(^{
+ qmp_system_powerdown(NULL);
+ });
}
/* Ejects the media.
@@ -1237,9 +1266,11 @@ - (void)ejectDeviceMedia:(id)sender
return;
}
- Error *err = NULL;
- qmp_eject(true, [drive cStringUsingEncoding: NSASCIIStringEncoding],
- false, NULL, false, false, &err);
+ __block Error *err = NULL;
+ with_iothread_lock(^{
+ qmp_eject(true, [drive cStringUsingEncoding: NSASCIIStringEncoding],
+ false, NULL, false, false, &err);
+ });
handleAnyDeviceErrors(err);
}
@@ -1271,16 +1302,18 @@ - (void)changeDeviceMedia:(id)sender
return;
}
- Error *err = NULL;
- qmp_blockdev_change_medium(true,
- [drive cStringUsingEncoding:
- NSASCIIStringEncoding],
- false, NULL,
- [file cStringUsingEncoding:
- NSASCIIStringEncoding],
- true, "raw",
- false, 0,
- &err);
+ __block Error *err = NULL;
+ with_iothread_lock(^{
+ qmp_blockdev_change_medium(true,
+ [drive cStringUsingEncoding:
+ NSASCIIStringEncoding],
+ false, NULL,
+ [file cStringUsingEncoding:
+ NSASCIIStringEncoding],
+ true, "raw",
+ false, 0,
+ &err);
+ });
handleAnyDeviceErrors(err);
}
}
@@ -1419,7 +1452,9 @@ - (void)adjustSpeed:(id)sender
// get the throttle percentage
throttle_pct = [sender tag];
- cpu_throttle_set(throttle_pct);
+ with_iothread_lock(^{
+ cpu_throttle_set(throttle_pct);
+ });
COCOA_DEBUG("cpu throttling at %d%c\n", cpu_throttle_get_percentage(), '%');
}
--
2.17.2 (Apple Git-113)
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 2/7] ui/cocoa: Use the pixman image directly in switchSurface
2019-02-14 10:28 [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop Peter Maydell
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 1/7] ui/cocoa: Ensure we have the iothread lock when calling into QEMU Peter Maydell
@ 2019-02-14 10:28 ` Peter Maydell
2019-02-14 17:19 ` BALATON Zoltan
2019-02-22 17:27 ` Roman Bolshakov
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 3/7] ui/cocoa: Factor out initial menu creation Peter Maydell
` (13 subsequent siblings)
15 siblings, 2 replies; 38+ messages in thread
From: Peter Maydell @ 2019-02-14 10:28 UTC (permalink / raw)
To: qemu-devel
Cc: patches, John Arbuckle, Roman Bolshakov, Berkus Decker,
Gerd Hoffmann, Ben Hekster, BALATON Zoltan
Currently the switchSurface method takes a DisplaySurface. We want
to change our DisplayChangeListener's dpy_gfx_switch callback
to do this work asynchronously on a different thread. The caller
of the switch callback will free the old DisplaySurface
immediately the callback returns, so to ensure that the
other thread doesn't access freed data we need to switch
to using the underlying pixman image instead. The pixman
image is reference counted, so we will be able to take
a reference to it to avoid it vanishing too early.
In this commit we only change the switchSurface method
to take a pixman image, and keep the flow of control
synchronous for now.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
ui/cocoa.m | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 2931c751fd..9d23732ff9 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -315,7 +315,7 @@ @interface QemuCocoaView : NSView
BOOL isAbsoluteEnabled;
BOOL isMouseDeassociated;
}
-- (void) switchSurface:(DisplaySurface *)surface;
+- (void) switchSurface:(pixman_image_t *)image;
- (void) grabMouse;
- (void) ungrabMouse;
- (void) toggleFullScreen:(id)sender;
@@ -495,12 +495,13 @@ - (void) setContentDimensions
}
}
-- (void) switchSurface:(DisplaySurface *)surface
+- (void) switchSurface:(pixman_image_t *)image
{
COCOA_DEBUG("QemuCocoaView: switchSurface\n");
- int w = surface_width(surface);
- int h = surface_height(surface);
+ int w = pixman_image_get_width(image);
+ int h = pixman_image_get_height(image);
+ pixman_format_code_t image_format = pixman_image_get_format(image);
/* cdx == 0 means this is our very first surface, in which case we need
* to recalculate the content dimensions even if it happens to be the size
* of the initial empty window.
@@ -522,10 +523,10 @@ - (void) switchSurface:(DisplaySurface *)surface
CGDataProviderRelease(dataProviderRef);
//sync host window color space with guests
- screen.bitsPerPixel = surface_bits_per_pixel(surface);
- screen.bitsPerComponent = surface_bytes_per_pixel(surface) * 2;
+ screen.bitsPerPixel = PIXMAN_FORMAT_BPP(image_format);
+ screen.bitsPerComponent = DIV_ROUND_UP(screen.bitsPerPixel, 8) * 2;
- dataProviderRef = CGDataProviderCreateWithData(NULL, surface_data(surface), w * 4 * h, NULL);
+ dataProviderRef = CGDataProviderCreateWithData(NULL, pixman_image_get_data(image), w * 4 * h, NULL);
// update windows
if (isFullscreen) {
@@ -1625,7 +1626,7 @@ static void cocoa_switch(DisplayChangeListener *dcl,
NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
COCOA_DEBUG("qemu_cocoa: cocoa_switch\n");
- [cocoaView switchSurface:surface];
+ [cocoaView switchSurface:surface->image];
[pool release];
}
--
2.17.2 (Apple Git-113)
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 3/7] ui/cocoa: Factor out initial menu creation
2019-02-14 10:28 [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop Peter Maydell
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 1/7] ui/cocoa: Ensure we have the iothread lock when calling into QEMU Peter Maydell
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 2/7] ui/cocoa: Use the pixman image directly in switchSurface Peter Maydell
@ 2019-02-14 10:28 ` Peter Maydell
2019-02-14 17:11 ` BALATON Zoltan
2019-02-22 17:47 ` Roman Bolshakov
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 4/7] ui/cocoa: Move console/device menu creation code up in file Peter Maydell
` (12 subsequent siblings)
15 siblings, 2 replies; 38+ messages in thread
From: Peter Maydell @ 2019-02-14 10:28 UTC (permalink / raw)
To: qemu-devel
Cc: patches, John Arbuckle, Roman Bolshakov, Berkus Decker,
Gerd Hoffmann, Ben Hekster, BALATON Zoltan
Factor out the long code sequence in main() which creates
the initial set of menus. This will make later patches
which move initialization code around a bit clearer.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
ui/cocoa.m | 78 ++++++++++++++++++++++++++++--------------------------
1 file changed, 41 insertions(+), 37 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 9d23732ff9..0b1cd31543 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1461,43 +1461,8 @@ - (void)adjustSpeed:(id)sender
@end
-
-int main (int argc, const char * argv[]) {
-
- gArgc = argc;
- gArgv = (char **)argv;
- int i;
-
- /* In case we don't need to display a window, let's not do that */
- for (i = 1; i < argc; i++) {
- const char *opt = argv[i];
-
- if (opt[0] == '-') {
- /* Treat --foo the same as -foo. */
- if (opt[1] == '-') {
- opt++;
- }
- if (!strcmp(opt, "-h") || !strcmp(opt, "-help") ||
- !strcmp(opt, "-vnc") ||
- !strcmp(opt, "-nographic") ||
- !strcmp(opt, "-version") ||
- !strcmp(opt, "-curses") ||
- !strcmp(opt, "-display") ||
- !strcmp(opt, "-qtest")) {
- return qemu_main(gArgc, gArgv, *_NSGetEnviron());
- }
- }
- }
-
- NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
-
- // Pull this console process up to being a fully-fledged graphical
- // app with a menubar and Dock icon
- ProcessSerialNumber psn = { 0, kCurrentProcess };
- TransformProcessType(&psn, kProcessTransformToForegroundApplication);
-
- [NSApplication sharedApplication];
-
+static void create_initial_menus(void)
+{
// Add menus
NSMenu *menu;
NSMenuItem *menuItem;
@@ -1581,6 +1546,45 @@ int main (int argc, const char * argv[]) {
menuItem = [[[NSMenuItem alloc] initWithTitle:@"Window" action:nil keyEquivalent:@""] autorelease];
[menuItem setSubmenu:menu];
[[NSApp mainMenu] addItem:menuItem];
+}
+
+int main (int argc, const char * argv[]) {
+
+ gArgc = argc;
+ gArgv = (char **)argv;
+ int i;
+
+ /* In case we don't need to display a window, let's not do that */
+ for (i = 1; i < argc; i++) {
+ const char *opt = argv[i];
+
+ if (opt[0] == '-') {
+ /* Treat --foo the same as -foo. */
+ if (opt[1] == '-') {
+ opt++;
+ }
+ if (!strcmp(opt, "-h") || !strcmp(opt, "-help") ||
+ !strcmp(opt, "-vnc") ||
+ !strcmp(opt, "-nographic") ||
+ !strcmp(opt, "-version") ||
+ !strcmp(opt, "-curses") ||
+ !strcmp(opt, "-display") ||
+ !strcmp(opt, "-qtest")) {
+ return qemu_main(gArgc, gArgv, *_NSGetEnviron());
+ }
+ }
+ }
+
+ NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+
+ // Pull this console process up to being a fully-fledged graphical
+ // app with a menubar and Dock icon
+ ProcessSerialNumber psn = { 0, kCurrentProcess };
+ TransformProcessType(&psn, kProcessTransformToForegroundApplication);
+
+ [NSApplication sharedApplication];
+
+ create_initial_menus();
// Create an Application controller
QemuCocoaAppController *appController = [[QemuCocoaAppController alloc] init];
--
2.17.2 (Apple Git-113)
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 4/7] ui/cocoa: Move console/device menu creation code up in file
2019-02-14 10:28 [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop Peter Maydell
` (2 preceding siblings ...)
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 3/7] ui/cocoa: Factor out initial menu creation Peter Maydell
@ 2019-02-14 10:28 ` Peter Maydell
2019-02-15 0:34 ` BALATON Zoltan
2019-02-22 18:10 ` Roman Bolshakov
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 5/7] ui/cocoa: Don't call NSApp sendEvent directly from handleEvent Peter Maydell
` (11 subsequent siblings)
15 siblings, 2 replies; 38+ messages in thread
From: Peter Maydell @ 2019-02-14 10:28 UTC (permalink / raw)
To: qemu-devel
Cc: patches, John Arbuckle, Roman Bolshakov, Berkus Decker,
Gerd Hoffmann, Ben Hekster, BALATON Zoltan
Move the console/device menu creation code functions
further up in the source file, next to the code which
creates the initial menus. We're going to want to
change the location we call these functions from in
the next patch.
This commit is a pure code move with no other changes.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
ui/cocoa.m | 184 ++++++++++++++++++++++++++---------------------------
1 file changed, 92 insertions(+), 92 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 0b1cd31543..2d943b6e2a 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1548,6 +1548,98 @@ static void create_initial_menus(void)
[[NSApp mainMenu] addItem:menuItem];
}
+/* Returns a name for a given console */
+static NSString * getConsoleName(QemuConsole * console)
+{
+ return [NSString stringWithFormat: @"%s", qemu_console_get_label(console)];
+}
+
+/* Add an entry to the View menu for each console */
+static void add_console_menu_entries(void)
+{
+ NSMenu *menu;
+ NSMenuItem *menuItem;
+ int index = 0;
+
+ menu = [[[NSApp mainMenu] itemWithTitle:@"View"] submenu];
+
+ [menu addItem:[NSMenuItem separatorItem]];
+
+ while (qemu_console_lookup_by_index(index) != NULL) {
+ menuItem = [[[NSMenuItem alloc] initWithTitle: getConsoleName(qemu_console_lookup_by_index(index))
+ action: @selector(displayConsole:) keyEquivalent: @""] autorelease];
+ [menuItem setTag: index];
+ [menu addItem: menuItem];
+ index++;
+ }
+}
+
+/* Make menu items for all removable devices.
+ * Each device is given an 'Eject' and 'Change' menu item.
+ */
+static void addRemovableDevicesMenuItems(void)
+{
+ NSMenu *menu;
+ NSMenuItem *menuItem;
+ BlockInfoList *currentDevice, *pointerToFree;
+ NSString *deviceName;
+
+ currentDevice = qmp_query_block(NULL);
+ pointerToFree = currentDevice;
+ if(currentDevice == NULL) {
+ NSBeep();
+ QEMU_Alert(@"Failed to query for block devices!");
+ return;
+ }
+
+ menu = [[[NSApp mainMenu] itemWithTitle:@"Machine"] submenu];
+
+ // Add a separator between related groups of menu items
+ [menu addItem:[NSMenuItem separatorItem]];
+
+ // Set the attributes to the "Removable Media" menu item
+ NSString *titleString = @"Removable Media";
+ NSMutableAttributedString *attString=[[NSMutableAttributedString alloc] initWithString:titleString];
+ NSColor *newColor = [NSColor blackColor];
+ NSFontManager *fontManager = [NSFontManager sharedFontManager];
+ NSFont *font = [fontManager fontWithFamily:@"Helvetica"
+ traits:NSBoldFontMask|NSItalicFontMask
+ weight:0
+ size:14];
+ [attString addAttribute:NSFontAttributeName value:font range:NSMakeRange(0, [titleString length])];
+ [attString addAttribute:NSForegroundColorAttributeName value:newColor range:NSMakeRange(0, [titleString length])];
+ [attString addAttribute:NSUnderlineStyleAttributeName value:[NSNumber numberWithInt: 1] range:NSMakeRange(0, [titleString length])];
+
+ // Add the "Removable Media" menu item
+ menuItem = [NSMenuItem new];
+ [menuItem setAttributedTitle: attString];
+ [menuItem setEnabled: NO];
+ [menu addItem: menuItem];
+
+ /* Loop through all the block devices in the emulator */
+ while (currentDevice) {
+ deviceName = [[NSString stringWithFormat: @"%s", currentDevice->value->device] retain];
+
+ if(currentDevice->value->removable) {
+ menuItem = [[NSMenuItem alloc] initWithTitle: [NSString stringWithFormat: @"Change %s...", currentDevice->value->device]
+ action: @selector(changeDeviceMedia:)
+ keyEquivalent: @""];
+ [menu addItem: menuItem];
+ [menuItem setRepresentedObject: deviceName];
+ [menuItem autorelease];
+
+ menuItem = [[NSMenuItem alloc] initWithTitle: [NSString stringWithFormat: @"Eject %s", currentDevice->value->device]
+ action: @selector(ejectDeviceMedia:)
+ keyEquivalent: @""];
+ [menu addItem: menuItem];
+ [menuItem setRepresentedObject: deviceName];
+ [menuItem autorelease];
+ }
+ currentDevice = currentDevice->next;
+ }
+ qapi_free_BlockInfoList(pointerToFree);
+}
+
int main (int argc, const char * argv[]) {
gArgc = argc;
@@ -1676,98 +1768,6 @@ static void cocoa_cleanup(void)
.dpy_refresh = cocoa_refresh,
};
-/* Returns a name for a given console */
-static NSString * getConsoleName(QemuConsole * console)
-{
- return [NSString stringWithFormat: @"%s", qemu_console_get_label(console)];
-}
-
-/* Add an entry to the View menu for each console */
-static void add_console_menu_entries(void)
-{
- NSMenu *menu;
- NSMenuItem *menuItem;
- int index = 0;
-
- menu = [[[NSApp mainMenu] itemWithTitle:@"View"] submenu];
-
- [menu addItem:[NSMenuItem separatorItem]];
-
- while (qemu_console_lookup_by_index(index) != NULL) {
- menuItem = [[[NSMenuItem alloc] initWithTitle: getConsoleName(qemu_console_lookup_by_index(index))
- action: @selector(displayConsole:) keyEquivalent: @""] autorelease];
- [menuItem setTag: index];
- [menu addItem: menuItem];
- index++;
- }
-}
-
-/* Make menu items for all removable devices.
- * Each device is given an 'Eject' and 'Change' menu item.
- */
-static void addRemovableDevicesMenuItems(void)
-{
- NSMenu *menu;
- NSMenuItem *menuItem;
- BlockInfoList *currentDevice, *pointerToFree;
- NSString *deviceName;
-
- currentDevice = qmp_query_block(NULL);
- pointerToFree = currentDevice;
- if(currentDevice == NULL) {
- NSBeep();
- QEMU_Alert(@"Failed to query for block devices!");
- return;
- }
-
- menu = [[[NSApp mainMenu] itemWithTitle:@"Machine"] submenu];
-
- // Add a separator between related groups of menu items
- [menu addItem:[NSMenuItem separatorItem]];
-
- // Set the attributes to the "Removable Media" menu item
- NSString *titleString = @"Removable Media";
- NSMutableAttributedString *attString=[[NSMutableAttributedString alloc] initWithString:titleString];
- NSColor *newColor = [NSColor blackColor];
- NSFontManager *fontManager = [NSFontManager sharedFontManager];
- NSFont *font = [fontManager fontWithFamily:@"Helvetica"
- traits:NSBoldFontMask|NSItalicFontMask
- weight:0
- size:14];
- [attString addAttribute:NSFontAttributeName value:font range:NSMakeRange(0, [titleString length])];
- [attString addAttribute:NSForegroundColorAttributeName value:newColor range:NSMakeRange(0, [titleString length])];
- [attString addAttribute:NSUnderlineStyleAttributeName value:[NSNumber numberWithInt: 1] range:NSMakeRange(0, [titleString length])];
-
- // Add the "Removable Media" menu item
- menuItem = [NSMenuItem new];
- [menuItem setAttributedTitle: attString];
- [menuItem setEnabled: NO];
- [menu addItem: menuItem];
-
- /* Loop through all the block devices in the emulator */
- while (currentDevice) {
- deviceName = [[NSString stringWithFormat: @"%s", currentDevice->value->device] retain];
-
- if(currentDevice->value->removable) {
- menuItem = [[NSMenuItem alloc] initWithTitle: [NSString stringWithFormat: @"Change %s...", currentDevice->value->device]
- action: @selector(changeDeviceMedia:)
- keyEquivalent: @""];
- [menu addItem: menuItem];
- [menuItem setRepresentedObject: deviceName];
- [menuItem autorelease];
-
- menuItem = [[NSMenuItem alloc] initWithTitle: [NSString stringWithFormat: @"Eject %s", currentDevice->value->device]
- action: @selector(ejectDeviceMedia:)
- keyEquivalent: @""];
- [menu addItem: menuItem];
- [menuItem setRepresentedObject: deviceName];
- [menuItem autorelease];
- }
- currentDevice = currentDevice->next;
- }
- qapi_free_BlockInfoList(pointerToFree);
-}
-
static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
{
COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n");
--
2.17.2 (Apple Git-113)
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 5/7] ui/cocoa: Don't call NSApp sendEvent directly from handleEvent
2019-02-14 10:28 [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop Peter Maydell
` (3 preceding siblings ...)
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 4/7] ui/cocoa: Move console/device menu creation code up in file Peter Maydell
@ 2019-02-14 10:28 ` Peter Maydell
2019-02-14 17:04 ` BALATON Zoltan
2019-02-22 19:20 ` Roman Bolshakov
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 6/7] ui/cocoa: Subclass NSApplication so we can implement sendEvent Peter Maydell
` (10 subsequent siblings)
15 siblings, 2 replies; 38+ messages in thread
From: Peter Maydell @ 2019-02-14 10:28 UTC (permalink / raw)
To: qemu-devel
Cc: patches, John Arbuckle, Roman Bolshakov, Berkus Decker,
Gerd Hoffmann, Ben Hekster, BALATON Zoltan
Currently the handleEvent method will directly call the NSApp
sendEvent method for any events that we want to let OSX deal
with. When we rearrange the event handling code, the way that
we say "let OSX have this event" is going to change. Prepare
for that by refactoring so that handleEvent returns a flag
indicating whether it consumed the event.
Suggested-by: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
New patch in v2
---
ui/cocoa.m | 49 ++++++++++++++++++++++++++++++++++---------------
1 file changed, 34 insertions(+), 15 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 2d943b6e2a..5a84e1aea7 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -129,8 +129,9 @@
NSTextField *pauseLabel;
NSArray * supportedImageFileTypes;
-// Utility function to run specified code block with iothread lock held
+// Utility functions to run specified code block with iothread lock held
typedef void (^CodeBlock)(void);
+typedef bool (^BoolCodeBlock)(void);
static void with_iothread_lock(CodeBlock block)
{
@@ -144,6 +145,21 @@ static void with_iothread_lock(CodeBlock block)
}
}
+static bool bool_with_iothread_lock(BoolCodeBlock block)
+{
+ bool locked = qemu_mutex_iothread_locked();
+ bool val;
+
+ if (!locked) {
+ qemu_mutex_lock_iothread();
+ }
+ val = block();
+ if (!locked) {
+ qemu_mutex_unlock_iothread();
+ }
+ return val;
+}
+
// Mac to QKeyCode conversion
const int mac_to_qkeycode_map[] = {
[kVK_ANSI_A] = Q_KEY_CODE_A,
@@ -320,8 +336,8 @@ - (void) grabMouse;
- (void) ungrabMouse;
- (void) toggleFullScreen:(id)sender;
- (void) handleMonitorInput:(NSEvent *)event;
-- (void) handleEvent:(NSEvent *)event;
-- (void) handleEventLocked:(NSEvent *)event;
+- (bool) handleEvent:(NSEvent *)event;
+- (bool) handleEventLocked:(NSEvent *)event;
- (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
/* The state surrounding mouse grabbing is potentially confusing.
* isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated
@@ -664,15 +680,16 @@ - (void) handleMonitorInput:(NSEvent *)event
}
}
-- (void) handleEvent:(NSEvent *)event
+- (bool) handleEvent:(NSEvent *)event
{
- with_iothread_lock(^{
- [self handleEventLocked:event];
+ return bool_with_iothread_lock(^{
+ return [self handleEventLocked:event];
});
}
-- (void) handleEventLocked:(NSEvent *)event
+- (bool) handleEventLocked:(NSEvent *)event
{
+ /* Return true if we handled the event, false if it should be given to OSX */
COCOA_DEBUG("QemuCocoaView: handleEvent\n");
int buttons = 0;
int keycode = 0;
@@ -743,8 +760,7 @@ - (void) handleEventLocked:(NSEvent *)event
if (keycode == Q_KEY_CODE_F) {
switched_to_fullscreen = true;
}
- [NSApp sendEvent:event];
- return;
+ return false;
}
// default
@@ -759,12 +775,12 @@ - (void) handleEventLocked:(NSEvent *)event
// enable graphic console
case '1' ... '9':
console_select(key - '0' - 1); /* ascii math */
- return;
+ return true;
// release the mouse grab
case 'g':
[self ungrabMouse];
- return;
+ return true;
}
}
}
@@ -781,7 +797,7 @@ - (void) handleEventLocked:(NSEvent *)event
// don't pass the guest a spurious key-up if we treated this
// command-key combo as a host UI action
if (!isMouseGrabbed && ([event modifierFlags] & NSEventModifierFlagCommand)) {
- return;
+ return true;
}
if (qemu_console_is_graphic(NULL)) {
@@ -875,7 +891,7 @@ - (void) handleEventLocked:(NSEvent *)event
mouse_event = false;
break;
default:
- [NSApp sendEvent:event];
+ return false;
}
if (mouse_event) {
@@ -911,10 +927,11 @@ - (void) handleEventLocked:(NSEvent *)event
qemu_input_queue_rel(dcl->con, INPUT_AXIS_Y, (int)[event deltaY]);
}
} else {
- [NSApp sendEvent:event];
+ return false;
}
qemu_input_event_sync();
}
+ return true;
}
- (void) grabMouse
@@ -1749,7 +1766,9 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
event = [NSApp nextEventMatchingMask:NSEventMaskAny untilDate:distantPast
inMode: NSDefaultRunLoopMode dequeue:YES];
if (event != nil) {
- [cocoaView handleEvent:event];
+ if (![cocoaView handleEvent:event]) {
+ [NSApp sendEvent:event];
+ }
}
} while(event != nil);
[pool release];
--
2.17.2 (Apple Git-113)
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 6/7] ui/cocoa: Subclass NSApplication so we can implement sendEvent
2019-02-14 10:28 [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop Peter Maydell
` (4 preceding siblings ...)
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 5/7] ui/cocoa: Don't call NSApp sendEvent directly from handleEvent Peter Maydell
@ 2019-02-14 10:28 ` Peter Maydell
2019-02-14 17:21 ` BALATON Zoltan
2019-02-22 20:18 ` Roman Bolshakov
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 7/7] ui/cocoa: Perform UI operations only on the main thread Peter Maydell
` (9 subsequent siblings)
15 siblings, 2 replies; 38+ messages in thread
From: Peter Maydell @ 2019-02-14 10:28 UTC (permalink / raw)
To: qemu-devel
Cc: patches, John Arbuckle, Roman Bolshakov, Berkus Decker,
Gerd Hoffmann, Ben Hekster, BALATON Zoltan
When we switch away from our custom event handling, we still want to
be able to have first go at any events our application receives,
because in full-screen mode we want to send key events to the guest,
even if they would be menu item activation events. There are several
ways we could do that, but one simple approach is to subclass
NSApplication so we can implement a custom sendEvent method.
Do that, but for the moment have our sendEvent just invoke the
superclass method.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
New patch in v2
---
ui/cocoa.m | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 5a84e1aea7..184fbd877d 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1478,6 +1478,17 @@ - (void)adjustSpeed:(id)sender
@end
+@interface QemuApplication : NSApplication
+@end
+
+@implementation QemuApplication
+- (void)sendEvent:(NSEvent *)event
+{
+ COCOA_DEBUG("QemuApplication: sendEvent\n");
+ [super sendEvent: event];
+}
+@end
+
static void create_initial_menus(void)
{
// Add menus
@@ -1691,7 +1702,7 @@ int main (int argc, const char * argv[]) {
ProcessSerialNumber psn = { 0, kCurrentProcess };
TransformProcessType(&psn, kProcessTransformToForegroundApplication);
- [NSApplication sharedApplication];
+ [QemuApplication sharedApplication];
create_initial_menus();
--
2.17.2 (Apple Git-113)
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH v2 7/7] ui/cocoa: Perform UI operations only on the main thread
2019-02-14 10:28 [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop Peter Maydell
` (5 preceding siblings ...)
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 6/7] ui/cocoa: Subclass NSApplication so we can implement sendEvent Peter Maydell
@ 2019-02-14 10:28 ` Peter Maydell
2019-02-15 0:55 ` BALATON Zoltan
` (2 more replies)
2019-02-14 11:07 ` [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop no-reply
` (8 subsequent siblings)
15 siblings, 3 replies; 38+ messages in thread
From: Peter Maydell @ 2019-02-14 10:28 UTC (permalink / raw)
To: qemu-devel
Cc: patches, John Arbuckle, Roman Bolshakov, Berkus Decker,
Gerd Hoffmann, Ben Hekster, BALATON Zoltan
The OSX Mojave release is more picky about enforcing the Cocoa API
restriction that only the main thread may perform UI calls. To
accommodate this we need to restructure the Cocoa code:
* the special OSX main() creates a second thread and uses
that to call the vl.c qemu_main(); the original main
thread goes into the OSX event loop
* the refresh, switch and update callbacks asynchronously
tell the main thread to do the necessary work
* the refresh callback no longer does the "get events from the
UI event queue and handle them" loop, since we now use
the stock OSX event loop. Instead our NSApplication sendEvent
method will either deal with them or pass them on to OSX
All these things have to be changed in one commit, to avoid
breaking bisection.
Note that since we use dispatch_get_main_queue(), this bumps
our minimum version requirement to OSX 10.10 Yosemite (released
in 2014, unsupported by Apple since 2017).
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
v2 changes: call handleEvent from sendEvent
---
ui/cocoa.m | 191 +++++++++++++++++++++++++++++++----------------------
1 file changed, 113 insertions(+), 78 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 184fbd877d..201a294ed4 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -129,6 +129,9 @@
NSTextField *pauseLabel;
NSArray * supportedImageFileTypes;
+QemuSemaphore display_init_sem;
+QemuSemaphore app_started_sem;
+
// Utility functions to run specified code block with iothread lock held
typedef void (^CodeBlock)(void);
typedef bool (^BoolCodeBlock)(void);
@@ -325,6 +328,7 @@ @interface QemuCocoaView : NSView
NSWindow *fullScreenWindow;
float cx,cy,cw,ch,cdx,cdy;
CGDataProviderRef dataProviderRef;
+ pixman_image_t *pixman_image;
BOOL modifiers_state[256];
BOOL isMouseGrabbed;
BOOL isFullscreen;
@@ -535,13 +539,16 @@ - (void) switchSurface:(pixman_image_t *)image
}
// update screenBuffer
- if (dataProviderRef)
+ if (dataProviderRef) {
CGDataProviderRelease(dataProviderRef);
+ pixman_image_unref(pixman_image);
+ }
//sync host window color space with guests
screen.bitsPerPixel = PIXMAN_FORMAT_BPP(image_format);
screen.bitsPerComponent = DIV_ROUND_UP(screen.bitsPerPixel, 8) * 2;
+ pixman_image = image;
dataProviderRef = CGDataProviderCreateWithData(NULL, pixman_image_get_data(image), w * 4 * h, NULL);
// update windows
@@ -1013,7 +1020,6 @@ @interface QemuCocoaAppController : NSObject
#endif
{
}
-- (void)startEmulationWithArgc:(int)argc argv:(char**)argv;
- (void)doToggleFullScreen:(id)sender;
- (void)toggleFullScreen:(id)sender;
- (void)showQEMUDoc:(id)sender;
@@ -1101,8 +1107,8 @@ - (void) dealloc
- (void)applicationDidFinishLaunching: (NSNotification *) note
{
COCOA_DEBUG("QemuCocoaAppController: applicationDidFinishLaunching\n");
- // launch QEMU, with the global args
- [self startEmulationWithArgc:gArgc argv:(char **)gArgv];
+ /* Tell cocoa_display_init to proceed */
+ qemu_sem_post(&app_started_sem);
}
- (void)applicationWillTerminate:(NSNotification *)aNotification
@@ -1145,15 +1151,6 @@ - (void) applicationWillResignActive: (NSNotification *)aNotification
[cocoaView raiseAllKeys];
}
-- (void)startEmulationWithArgc:(int)argc argv:(char**)argv
-{
- COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
-
- int status;
- status = qemu_main(argc, argv, *_NSGetEnviron());
- exit(status);
-}
-
/* We abstract the method called by the Enter Fullscreen menu item
* because Mac OS 10.7 and higher disables it. This is because of the
* menu item's old selector's name toggleFullScreen:
@@ -1485,7 +1482,9 @@ @implementation QemuApplication
- (void)sendEvent:(NSEvent *)event
{
COCOA_DEBUG("QemuApplication: sendEvent\n");
- [super sendEvent: event];
+ if (!cocoaView || ![cocoaView handleEvent:event]) {
+ [super sendEvent: event];
+ }
}
@end
@@ -1668,32 +1667,59 @@ static void addRemovableDevicesMenuItems(void)
qapi_free_BlockInfoList(pointerToFree);
}
-int main (int argc, const char * argv[]) {
+/*
+ * The startup process for the OSX/Cocoa UI is complicated, because
+ * OSX insists that the UI runs on the initial main thread, and so we
+ * need to start a second thread which runs the vl.c qemu_main():
+ *
+ * Initial thread: 2nd thread:
+ * in main():
+ * create qemu-main thread
+ * wait on display_init semaphore
+ * call qemu_main()
+ * ...
+ * in cocoa_display_init():
+ * post the display_init semaphore
+ * wait on app_started semaphore
+ * create application, menus, etc
+ * enter OSX run loop
+ * in applicationDidFinishLaunching:
+ * post app_started semaphore
+ * tell main thread to fullscreen if needed
+ * [...]
+ * run qemu main-loop
+ *
+ * We do this in two stages so that we don't do the creation of the
+ * GUI application menus and so on for command line options like --help
+ * where we want to just print text to stdout and exit immediately.
+ */
+static void *call_qemu_main(void *opaque)
+{
+ int status;
+
+ COCOA_DEBUG("Second thread: calling qemu_main()\n");
+ status = qemu_main(gArgc, gArgv, *_NSGetEnviron());
+ COCOA_DEBUG("Second thread: qemu_main() returned, exiting\n");
+ exit(status);
+}
+
+int main (int argc, const char * argv[]) {
+ QemuThread thread;
+
+ COCOA_DEBUG("Entered main()\n");
gArgc = argc;
gArgv = (char **)argv;
- int i;
- /* In case we don't need to display a window, let's not do that */
- for (i = 1; i < argc; i++) {
- const char *opt = argv[i];
+ qemu_sem_init(&display_init_sem, 0);
+ qemu_sem_init(&app_started_sem, 0);
- if (opt[0] == '-') {
- /* Treat --foo the same as -foo. */
- if (opt[1] == '-') {
- opt++;
- }
- if (!strcmp(opt, "-h") || !strcmp(opt, "-help") ||
- !strcmp(opt, "-vnc") ||
- !strcmp(opt, "-nographic") ||
- !strcmp(opt, "-version") ||
- !strcmp(opt, "-curses") ||
- !strcmp(opt, "-display") ||
- !strcmp(opt, "-qtest")) {
- return qemu_main(gArgc, gArgv, *_NSGetEnviron());
- }
- }
- }
+ qemu_thread_create(&thread, "qemu_main", call_qemu_main,
+ NULL, QEMU_THREAD_DETACHED);
+
+ COCOA_DEBUG("Main thread: waiting for display_init_sem\n");
+ qemu_sem_wait(&display_init_sem);
+ COCOA_DEBUG("Main thread: initializing app\n");
NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
@@ -1706,12 +1732,24 @@ int main (int argc, const char * argv[]) {
create_initial_menus();
+ /*
+ * Create the menu entries which depend on QEMU state (for consoles
+ * and removeable devices). These make calls back into QEMU functions,
+ * which is OK because at this point we know that the second thread
+ * holds the iothread lock and is synchronously waiting for us to
+ * finish.
+ */
+ add_console_menu_entries();
+ addRemovableDevicesMenuItems();
+
// Create an Application controller
QemuCocoaAppController *appController = [[QemuCocoaAppController alloc] init];
[NSApp setDelegate:appController];
// Start the main event loop
+ COCOA_DEBUG("Main thread: entering OSX run loop\n");
[NSApp run];
+ COCOA_DEBUG("Main thread: left OSX run loop, exiting\n");
[appController release];
[pool release];
@@ -1729,17 +1767,19 @@ static void cocoa_update(DisplayChangeListener *dcl,
COCOA_DEBUG("qemu_cocoa: cocoa_update\n");
- NSRect rect;
- if ([cocoaView cdx] == 1.0) {
- rect = NSMakeRect(x, [cocoaView gscreen].height - y - h, w, h);
- } else {
- rect = NSMakeRect(
- x * [cocoaView cdx],
- ([cocoaView gscreen].height - y - h) * [cocoaView cdy],
- w * [cocoaView cdx],
- h * [cocoaView cdy]);
- }
- [cocoaView setNeedsDisplayInRect:rect];
+ dispatch_async(dispatch_get_main_queue(), ^{
+ NSRect rect;
+ if ([cocoaView cdx] == 1.0) {
+ rect = NSMakeRect(x, [cocoaView gscreen].height - y - h, w, h);
+ } else {
+ rect = NSMakeRect(
+ x * [cocoaView cdx],
+ ([cocoaView gscreen].height - y - h) * [cocoaView cdy],
+ w * [cocoaView cdx],
+ h * [cocoaView cdy]);
+ }
+ [cocoaView setNeedsDisplayInRect:rect];
+ });
[pool release];
}
@@ -1748,9 +1788,19 @@ static void cocoa_switch(DisplayChangeListener *dcl,
DisplaySurface *surface)
{
NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+ pixman_image_t *image = surface->image;
COCOA_DEBUG("qemu_cocoa: cocoa_switch\n");
- [cocoaView switchSurface:surface->image];
+
+ // The DisplaySurface will be freed as soon as this callback returns.
+ // We take a reference to the underlying pixman image here so it does
+ // not disappear from under our feet; the switchSurface method will
+ // deref the old image when it is done with it.
+ pixman_image_ref(image);
+
+ dispatch_async(dispatch_get_main_queue(), ^{
+ [cocoaView switchSurface:image];
+ });
[pool release];
}
@@ -1762,26 +1812,15 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
graphic_hw_update(NULL);
if (qemu_input_is_absolute()) {
- if (![cocoaView isAbsoluteEnabled]) {
- if ([cocoaView isMouseGrabbed]) {
- [cocoaView ungrabMouse];
+ dispatch_async(dispatch_get_main_queue(), ^{
+ if (![cocoaView isAbsoluteEnabled]) {
+ if ([cocoaView isMouseGrabbed]) {
+ [cocoaView ungrabMouse];
+ }
}
- }
- [cocoaView setAbsoluteEnabled:YES];
+ [cocoaView setAbsoluteEnabled:YES];
+ });
}
-
- NSDate *distantPast;
- NSEvent *event;
- distantPast = [NSDate distantPast];
- do {
- event = [NSApp nextEventMatchingMask:NSEventMaskAny untilDate:distantPast
- inMode: NSDefaultRunLoopMode dequeue:YES];
- if (event != nil) {
- if (![cocoaView handleEvent:event]) {
- [NSApp sendEvent:event];
- }
- }
- } while(event != nil);
[pool release];
}
@@ -1802,10 +1841,17 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
{
COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n");
+ /* Tell main thread to go ahead and create the app and enter the run loop */
+ qemu_sem_post(&display_init_sem);
+ qemu_sem_wait(&app_started_sem);
+ COCOA_DEBUG("cocoa_display_init: app start completed\n");
+
/* if fullscreen mode is to be used */
if (opts->has_full_screen && opts->full_screen) {
- [NSApp activateIgnoringOtherApps: YES];
- [(QemuCocoaAppController *)[[NSApplication sharedApplication] delegate] toggleFullScreen: nil];
+ dispatch_async(dispatch_get_main_queue(), ^{
+ [NSApp activateIgnoringOtherApps: YES];
+ [(QemuCocoaAppController *)[[NSApplication sharedApplication] delegate] toggleFullScreen: nil];
+ });
}
dcl = g_malloc0(sizeof(DisplayChangeListener));
@@ -1816,17 +1862,6 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
// register cleanup function
atexit(cocoa_cleanup);
-
- /* At this point QEMU has created all the consoles, so we can add View
- * menu entries for them.
- */
- add_console_menu_entries();
-
- /* Give all removable devices a menu item.
- * Has to be called after QEMU has started to
- * find out what removable devices it has.
- */
- addRemovableDevicesMenuItems();
}
static QemuDisplay qemu_display_cocoa = {
--
2.17.2 (Apple Git-113)
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
2019-02-14 10:28 [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop Peter Maydell
` (6 preceding siblings ...)
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 7/7] ui/cocoa: Perform UI operations only on the main thread Peter Maydell
@ 2019-02-14 11:07 ` no-reply
2019-02-14 11:11 ` no-reply
` (7 subsequent siblings)
15 siblings, 0 replies; 38+ messages in thread
From: no-reply @ 2019-02-14 11:07 UTC (permalink / raw)
To: peter.maydell
Cc: fam, qemu-devel, patches, programmingkidx, r.bolshakov, kraxel,
ben.hekster, berkus
Patchew URL: https://patchew.org/QEMU/20190214102816.3393-1-peter.maydell@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Message-id: 20190214102816.3393-1-peter.maydell@linaro.org
Subject: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
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
t [tag update] patchew/20190212134758.10514-1-dgilbert@redhat.com -> patchew/20190212134758.10514-1-dgilbert@redhat.com
* [new tag] patchew/20190214102816.3393-1-peter.maydell@linaro.org -> patchew/20190214102816.3393-1-peter.maydell@linaro.org
Switched to a new branch 'test'
b6de88ab3a ui/cocoa: Perform UI operations only on the main thread
1940bfd263 ui/cocoa: Subclass NSApplication so we can implement sendEvent
724525dbfc ui/cocoa: Don't call NSApp sendEvent directly from handleEvent
dcd07aa31c ui/cocoa: Move console/device menu creation code up in file
f0890fa38d ui/cocoa: Factor out initial menu creation
9669817763 ui/cocoa: Use the pixman image directly in switchSurface
bcffb41705 ui/cocoa: Ensure we have the iothread lock when calling into QEMU
=== OUTPUT BEGIN ===
1/7 Checking commit bcffb41705d0 (ui/cocoa: Ensure we have the iothread lock when calling into QEMU)
2/7 Checking commit 96698177637f (ui/cocoa: Use the pixman image directly in switchSurface)
3/7 Checking commit f0890fa38dc1 (ui/cocoa: Factor out initial menu creation)
4/7 Checking commit dcd07aa31c3b (ui/cocoa: Move console/device menu creation code up in file)
5/7 Checking commit 724525dbfc09 (ui/cocoa: Don't call NSApp sendEvent directly from handleEvent)
ERROR: trailing whitespace
#41: FILE: ui/cocoa.m:152:
+ $
total: 1 errors, 0 warnings, 122 lines checked
Patch 5/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/7 Checking commit 1940bfd26326 (ui/cocoa: Subclass NSApplication so we can implement sendEvent)
7/7 Checking commit b6de88ab3a67 (ui/cocoa: Perform UI operations only on the main thread)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190214102816.3393-1-peter.maydell@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
2019-02-14 10:28 [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop Peter Maydell
` (7 preceding siblings ...)
2019-02-14 11:07 ` [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop no-reply
@ 2019-02-14 11:11 ` no-reply
2019-02-14 17:44 ` no-reply
` (6 subsequent siblings)
15 siblings, 0 replies; 38+ messages in thread
From: no-reply @ 2019-02-14 11:11 UTC (permalink / raw)
To: peter.maydell
Cc: fam, qemu-devel, patches, programmingkidx, r.bolshakov, kraxel,
ben.hekster, berkus
Patchew URL: https://patchew.org/QEMU/20190214102816.3393-1-peter.maydell@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
Message-id: 20190214102816.3393-1-peter.maydell@linaro.org
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
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
- [tag update] patchew/20190212074840.13542-1-richard.henderson@linaro.org -> patchew/20190212074840.13542-1-richard.henderson@linaro.org
- [tag update] patchew/20190212134758.10514-1-dgilbert@redhat.com -> patchew/20190212134758.10514-1-dgilbert@redhat.com
- [tag update] patchew/20190214031004.32522-1-stefanha@redhat.com -> patchew/20190214031004.32522-1-stefanha@redhat.com
* [new tag] patchew/20190214084939.20640-1-richardw.yang@linux.intel.com -> patchew/20190214084939.20640-1-richardw.yang@linux.intel.com
* [new tag] patchew/20190214093053.1400-1-armbru@redhat.com -> patchew/20190214093053.1400-1-armbru@redhat.com
* [new tag] patchew/20190214102816.3393-1-peter.maydell@linaro.org -> patchew/20190214102816.3393-1-peter.maydell@linaro.org
Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for path 'capstone'
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) registered for path 'roms/QemuMacDrivers'
Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 'roms/SLOF'
Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 'roms/ipxe'
Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered for path 'roms/openbios'
Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) registered for path 'roms/openhackware'
Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) registered for path 'roms/qemu-palcode'
Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for path 'roms/seabios'
Submodule 'roms/seabios-hppa' (https://github.com/hdeller/seabios-hppa.git) registered for path 'roms/seabios-hppa'
Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for path 'roms/sgabios'
Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for path 'roms/skiboot'
Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for path 'roms/u-boot'
Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) registered for path 'roms/u-boot-sam460ex'
Submodule 'tests/fp/berkeley-softfloat-3' (https://github.com/cota/berkeley-softfloat-3) registered for path 'tests/fp/berkeley-softfloat-3'
Submodule 'tests/fp/berkeley-testfloat-3' (https://github.com/cota/berkeley-testfloat-3) registered for path 'tests/fp/berkeley-testfloat-3'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) registered for path 'ui/keycodemapdb'
Cloning into 'capstone'...
Submodule path 'capstone': checked out '22ead3e0bfdb87516656453336160e0a37b066bf'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Cloning into 'roms/QemuMacDrivers'...
Submodule path 'roms/QemuMacDrivers': checked out '90c488d5f4a407342247b9ea869df1c2d9c8e266'
Cloning into 'roms/SLOF'...
Submodule path 'roms/SLOF': checked out 'a5b428e1c1eae703bdd62a3f527223c291ee3fdc'
Cloning into 'roms/ipxe'...
Submodule path 'roms/ipxe': checked out 'de4565cbe76ea9f7913a01f331be3ee901bb6e17'
Cloning into 'roms/openbios'...
Submodule path 'roms/openbios': checked out '441a84d3a642a10b948369c63f32367e8ff6395b'
Cloning into 'roms/openhackware'...
Submodule path 'roms/openhackware': checked out 'c559da7c8eec5e45ef1f67978827af6f0b9546f5'
Cloning into 'roms/qemu-palcode'...
Submodule path 'roms/qemu-palcode': checked out '51c237d7e20d05100eacadee2f61abc17e6bc097'
Cloning into 'roms/seabios'...
Submodule path 'roms/seabios': checked out 'a698c8995ffb2838296ec284fe3c4ad33dfca307'
Cloning into 'roms/seabios-hppa'...
Submodule path 'roms/seabios-hppa': checked out '1ef99a01572c2581c30e16e6fe69e9ea2ef92ce0'
Cloning into 'roms/sgabios'...
Submodule path 'roms/sgabios': checked out 'cbaee52287e5f32373181cff50a00b6c4ac9015a'
Cloning into 'roms/skiboot'...
Submodule path 'roms/skiboot': checked out 'e0ee24c27a172bcf482f6f2bc905e6211c134bcc'
Cloning into 'roms/u-boot'...
Submodule path 'roms/u-boot': checked out 'd85ca029f257b53a96da6c2fb421e78a003a9943'
Cloning into 'roms/u-boot-sam460ex'...
Submodule path 'roms/u-boot-sam460ex': checked out '60b3916f33e617a815973c5a6df77055b2e3a588'
Cloning into 'tests/fp/berkeley-softfloat-3'...
Submodule path 'tests/fp/berkeley-softfloat-3': checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'tests/fp/berkeley-testfloat-3'...
Submodule path 'tests/fp/berkeley-testfloat-3': checked out '5a59dcec19327396a011a17fd924aed4fec416b3'
Cloning into 'ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
Switched to a new branch 'test'
b6de88a ui/cocoa: Perform UI operations only on the main thread
1940bfd ui/cocoa: Subclass NSApplication so we can implement sendEvent
724525d ui/cocoa: Don't call NSApp sendEvent directly from handleEvent
dcd07aa ui/cocoa: Move console/device menu creation code up in file
f0890fa ui/cocoa: Factor out initial menu creation
96698177 ui/cocoa: Use the pixman image directly in switchSurface
bcffb41 ui/cocoa: Ensure we have the iothread lock when calling into QEMU
=== OUTPUT BEGIN ===
1/7 Checking commit bcffb41705d0 (ui/cocoa: Ensure we have the iothread lock when calling into QEMU)
2/7 Checking commit 96698177637f (ui/cocoa: Use the pixman image directly in switchSurface)
3/7 Checking commit f0890fa38dc1 (ui/cocoa: Factor out initial menu creation)
4/7 Checking commit dcd07aa31c3b (ui/cocoa: Move console/device menu creation code up in file)
5/7 Checking commit 724525dbfc09 (ui/cocoa: Don't call NSApp sendEvent directly from handleEvent)
ERROR: trailing whitespace
#41: FILE: ui/cocoa.m:152:
+ $
total: 1 errors, 0 warnings, 122 lines checked
Patch 5/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/7 Checking commit 1940bfd26326 (ui/cocoa: Subclass NSApplication so we can implement sendEvent)
7/7 Checking commit b6de88ab3a67 (ui/cocoa: Perform UI operations only on the main thread)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190214102816.3393-1-peter.maydell@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/7] ui/cocoa: Don't call NSApp sendEvent directly from handleEvent
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 5/7] ui/cocoa: Don't call NSApp sendEvent directly from handleEvent Peter Maydell
@ 2019-02-14 17:04 ` BALATON Zoltan
2019-02-14 17:41 ` Peter Maydell
2019-02-22 19:20 ` Roman Bolshakov
1 sibling, 1 reply; 38+ messages in thread
From: BALATON Zoltan @ 2019-02-14 17:04 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, patches, John Arbuckle, Roman Bolshakov,
Berkus Decker, Gerd Hoffmann, Ben Hekster
On Thu, 14 Feb 2019, Peter Maydell wrote:
> Currently the handleEvent method will directly call the NSApp
> sendEvent method for any events that we want to let OSX deal
> with. When we rearrange the event handling code, the way that
> we say "let OSX have this event" is going to change. Prepare
> for that by refactoring so that handleEvent returns a flag
> indicating whether it consumed the event.
>
> Suggested-by: BALATON Zoltan <balaton@eik.bme.hu>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> New patch in v2
> ---
> ui/cocoa.m | 49 ++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 34 insertions(+), 15 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 2d943b6e2a..5a84e1aea7 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -129,8 +129,9 @@
> NSTextField *pauseLabel;
> NSArray * supportedImageFileTypes;
>
> -// Utility function to run specified code block with iothread lock held
> +// Utility functions to run specified code block with iothread lock held
> typedef void (^CodeBlock)(void);
> +typedef bool (^BoolCodeBlock)(void);
>
> static void with_iothread_lock(CodeBlock block)
> {
> @@ -144,6 +145,21 @@ static void with_iothread_lock(CodeBlock block)
> }
> }
>
> +static bool bool_with_iothread_lock(BoolCodeBlock block)
> +{
> + bool locked = qemu_mutex_iothread_locked();
> + bool val;
> +
Git complained about extra white space in the end of the empty line above
but not sure if it was added during mailing or you have it in the original
patch.
> + if (!locked) {
> + qemu_mutex_lock_iothread();
> + }
> + val = block();
> + if (!locked) {
> + qemu_mutex_unlock_iothread();
> + }
> + return val;
> +}
> +
> // Mac to QKeyCode conversion
> const int mac_to_qkeycode_map[] = {
> [kVK_ANSI_A] = Q_KEY_CODE_A,
> @@ -320,8 +336,8 @@ - (void) grabMouse;
> - (void) ungrabMouse;
> - (void) toggleFullScreen:(id)sender;
> - (void) handleMonitorInput:(NSEvent *)event;
> -- (void) handleEvent:(NSEvent *)event;
> -- (void) handleEventLocked:(NSEvent *)event;
> +- (bool) handleEvent:(NSEvent *)event;
> +- (bool) handleEventLocked:(NSEvent *)event;
> - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
> /* The state surrounding mouse grabbing is potentially confusing.
> * isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated
> @@ -664,15 +680,16 @@ - (void) handleMonitorInput:(NSEvent *)event
> }
> }
>
> -- (void) handleEvent:(NSEvent *)event
> +- (bool) handleEvent:(NSEvent *)event
> {
> - with_iothread_lock(^{
> - [self handleEventLocked:event];
> + return bool_with_iothread_lock(^{
> + return [self handleEventLocked:event];
> });
> }
If this is only ever used for this one method, wouldn't it be easier to
move locking to the method below (even with some goto after setting a ret
variable to unlock at the end of the method where now it returns in the
middle, but maybe it could even be done without goto as the whole code is
one big switch that can be exited with break and an if that can be skipped
by a flag)? That may be easier to follow than this method within block
within method and then you wouldn't need bool_with_iothread_lock and
neither handleEvent. Unless there's something I'm missing which makes this
convoluted way needed.
Other than that
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Regards,
BALATON Zoltan
> -- (void) handleEventLocked:(NSEvent *)event
> +- (bool) handleEventLocked:(NSEvent *)event
> {
> + /* Return true if we handled the event, false if it should be given to OSX */
> COCOA_DEBUG("QemuCocoaView: handleEvent\n");
> int buttons = 0;
> int keycode = 0;
> @@ -743,8 +760,7 @@ - (void) handleEventLocked:(NSEvent *)event
> if (keycode == Q_KEY_CODE_F) {
> switched_to_fullscreen = true;
> }
> - [NSApp sendEvent:event];
> - return;
> + return false;
> }
>
> // default
> @@ -759,12 +775,12 @@ - (void) handleEventLocked:(NSEvent *)event
> // enable graphic console
> case '1' ... '9':
> console_select(key - '0' - 1); /* ascii math */
> - return;
> + return true;
>
> // release the mouse grab
> case 'g':
> [self ungrabMouse];
> - return;
> + return true;
> }
> }
> }
> @@ -781,7 +797,7 @@ - (void) handleEventLocked:(NSEvent *)event
> // don't pass the guest a spurious key-up if we treated this
> // command-key combo as a host UI action
> if (!isMouseGrabbed && ([event modifierFlags] & NSEventModifierFlagCommand)) {
> - return;
> + return true;
> }
>
> if (qemu_console_is_graphic(NULL)) {
> @@ -875,7 +891,7 @@ - (void) handleEventLocked:(NSEvent *)event
> mouse_event = false;
> break;
> default:
> - [NSApp sendEvent:event];
> + return false;
> }
>
> if (mouse_event) {
> @@ -911,10 +927,11 @@ - (void) handleEventLocked:(NSEvent *)event
> qemu_input_queue_rel(dcl->con, INPUT_AXIS_Y, (int)[event deltaY]);
> }
> } else {
> - [NSApp sendEvent:event];
> + return false;
> }
> qemu_input_event_sync();
> }
> + return true;
> }
>
> - (void) grabMouse
> @@ -1749,7 +1766,9 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
> event = [NSApp nextEventMatchingMask:NSEventMaskAny untilDate:distantPast
> inMode: NSDefaultRunLoopMode dequeue:YES];
> if (event != nil) {
> - [cocoaView handleEvent:event];
> + if (![cocoaView handleEvent:event]) {
> + [NSApp sendEvent:event];
> + }
> }
> } while(event != nil);
> [pool release];
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/7] ui/cocoa: Factor out initial menu creation
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 3/7] ui/cocoa: Factor out initial menu creation Peter Maydell
@ 2019-02-14 17:11 ` BALATON Zoltan
2019-02-22 17:47 ` Roman Bolshakov
1 sibling, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2019-02-14 17:11 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, patches, John Arbuckle, Roman Bolshakov,
Berkus Decker, Gerd Hoffmann, Ben Hekster
On Thu, 14 Feb 2019, Peter Maydell wrote:
> Factor out the long code sequence in main() which creates
> the initial set of menus. This will make later patches
> which move initialization code around a bit clearer.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Regards,
BALATON Zoltan
> ---
> ui/cocoa.m | 78 ++++++++++++++++++++++++++++--------------------------
> 1 file changed, 41 insertions(+), 37 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 9d23732ff9..0b1cd31543 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -1461,43 +1461,8 @@ - (void)adjustSpeed:(id)sender
>
> @end
>
> -
> -int main (int argc, const char * argv[]) {
> -
> - gArgc = argc;
> - gArgv = (char **)argv;
> - int i;
> -
> - /* In case we don't need to display a window, let's not do that */
> - for (i = 1; i < argc; i++) {
> - const char *opt = argv[i];
> -
> - if (opt[0] == '-') {
> - /* Treat --foo the same as -foo. */
> - if (opt[1] == '-') {
> - opt++;
> - }
> - if (!strcmp(opt, "-h") || !strcmp(opt, "-help") ||
> - !strcmp(opt, "-vnc") ||
> - !strcmp(opt, "-nographic") ||
> - !strcmp(opt, "-version") ||
> - !strcmp(opt, "-curses") ||
> - !strcmp(opt, "-display") ||
> - !strcmp(opt, "-qtest")) {
> - return qemu_main(gArgc, gArgv, *_NSGetEnviron());
> - }
> - }
> - }
> -
> - NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
> -
> - // Pull this console process up to being a fully-fledged graphical
> - // app with a menubar and Dock icon
> - ProcessSerialNumber psn = { 0, kCurrentProcess };
> - TransformProcessType(&psn, kProcessTransformToForegroundApplication);
> -
> - [NSApplication sharedApplication];
> -
> +static void create_initial_menus(void)
> +{
> // Add menus
> NSMenu *menu;
> NSMenuItem *menuItem;
> @@ -1581,6 +1546,45 @@ int main (int argc, const char * argv[]) {
> menuItem = [[[NSMenuItem alloc] initWithTitle:@"Window" action:nil keyEquivalent:@""] autorelease];
> [menuItem setSubmenu:menu];
> [[NSApp mainMenu] addItem:menuItem];
> +}
> +
> +int main (int argc, const char * argv[]) {
> +
> + gArgc = argc;
> + gArgv = (char **)argv;
> + int i;
> +
> + /* In case we don't need to display a window, let's not do that */
> + for (i = 1; i < argc; i++) {
> + const char *opt = argv[i];
> +
> + if (opt[0] == '-') {
> + /* Treat --foo the same as -foo. */
> + if (opt[1] == '-') {
> + opt++;
> + }
> + if (!strcmp(opt, "-h") || !strcmp(opt, "-help") ||
> + !strcmp(opt, "-vnc") ||
> + !strcmp(opt, "-nographic") ||
> + !strcmp(opt, "-version") ||
> + !strcmp(opt, "-curses") ||
> + !strcmp(opt, "-display") ||
> + !strcmp(opt, "-qtest")) {
> + return qemu_main(gArgc, gArgv, *_NSGetEnviron());
> + }
> + }
> + }
> +
> + NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
> +
> + // Pull this console process up to being a fully-fledged graphical
> + // app with a menubar and Dock icon
> + ProcessSerialNumber psn = { 0, kCurrentProcess };
> + TransformProcessType(&psn, kProcessTransformToForegroundApplication);
> +
> + [NSApplication sharedApplication];
> +
> + create_initial_menus();
>
> // Create an Application controller
> QemuCocoaAppController *appController = [[QemuCocoaAppController alloc] init];
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/7] ui/cocoa: Use the pixman image directly in switchSurface
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 2/7] ui/cocoa: Use the pixman image directly in switchSurface Peter Maydell
@ 2019-02-14 17:19 ` BALATON Zoltan
2019-02-22 17:27 ` Roman Bolshakov
1 sibling, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2019-02-14 17:19 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, patches, John Arbuckle, Roman Bolshakov,
Berkus Decker, Gerd Hoffmann, Ben Hekster
On Thu, 14 Feb 2019, Peter Maydell wrote:
> Currently the switchSurface method takes a DisplaySurface. We want
> to change our DisplayChangeListener's dpy_gfx_switch callback
> to do this work asynchronously on a different thread. The caller
> of the switch callback will free the old DisplaySurface
> immediately the callback returns, so to ensure that the
> other thread doesn't access freed data we need to switch
> to using the underlying pixman image instead. The pixman
> image is reference counted, so we will be able to take
> a reference to it to avoid it vanishing too early.
>
> In this commit we only change the switchSurface method
> to take a pixman image, and keep the flow of control
> synchronous for now.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Regards,
BALATON Zoltan
> ---
> ui/cocoa.m | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 2931c751fd..9d23732ff9 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -315,7 +315,7 @@ @interface QemuCocoaView : NSView
> BOOL isAbsoluteEnabled;
> BOOL isMouseDeassociated;
> }
> -- (void) switchSurface:(DisplaySurface *)surface;
> +- (void) switchSurface:(pixman_image_t *)image;
> - (void) grabMouse;
> - (void) ungrabMouse;
> - (void) toggleFullScreen:(id)sender;
> @@ -495,12 +495,13 @@ - (void) setContentDimensions
> }
> }
>
> -- (void) switchSurface:(DisplaySurface *)surface
> +- (void) switchSurface:(pixman_image_t *)image
> {
> COCOA_DEBUG("QemuCocoaView: switchSurface\n");
>
> - int w = surface_width(surface);
> - int h = surface_height(surface);
> + int w = pixman_image_get_width(image);
> + int h = pixman_image_get_height(image);
> + pixman_format_code_t image_format = pixman_image_get_format(image);
> /* cdx == 0 means this is our very first surface, in which case we need
> * to recalculate the content dimensions even if it happens to be the size
> * of the initial empty window.
> @@ -522,10 +523,10 @@ - (void) switchSurface:(DisplaySurface *)surface
> CGDataProviderRelease(dataProviderRef);
>
> //sync host window color space with guests
> - screen.bitsPerPixel = surface_bits_per_pixel(surface);
> - screen.bitsPerComponent = surface_bytes_per_pixel(surface) * 2;
> + screen.bitsPerPixel = PIXMAN_FORMAT_BPP(image_format);
> + screen.bitsPerComponent = DIV_ROUND_UP(screen.bitsPerPixel, 8) * 2;
>
> - dataProviderRef = CGDataProviderCreateWithData(NULL, surface_data(surface), w * 4 * h, NULL);
> + dataProviderRef = CGDataProviderCreateWithData(NULL, pixman_image_get_data(image), w * 4 * h, NULL);
>
> // update windows
> if (isFullscreen) {
> @@ -1625,7 +1626,7 @@ static void cocoa_switch(DisplayChangeListener *dcl,
> NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
>
> COCOA_DEBUG("qemu_cocoa: cocoa_switch\n");
> - [cocoaView switchSurface:surface];
> + [cocoaView switchSurface:surface->image];
> [pool release];
> }
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/7] ui/cocoa: Subclass NSApplication so we can implement sendEvent
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 6/7] ui/cocoa: Subclass NSApplication so we can implement sendEvent Peter Maydell
@ 2019-02-14 17:21 ` BALATON Zoltan
2019-02-22 20:18 ` Roman Bolshakov
1 sibling, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2019-02-14 17:21 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, patches, John Arbuckle, Roman Bolshakov,
Berkus Decker, Gerd Hoffmann, Ben Hekster
On Thu, 14 Feb 2019, Peter Maydell wrote:
> When we switch away from our custom event handling, we still want to
> be able to have first go at any events our application receives,
> because in full-screen mode we want to send key events to the guest,
> even if they would be menu item activation events. There are several
> ways we could do that, but one simple approach is to subclass
> NSApplication so we can implement a custom sendEvent method.
> Do that, but for the moment have our sendEvent just invoke the
> superclass method.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Regards,
BALATON Zoltan
> ---
> New patch in v2
> ---
> ui/cocoa.m | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 5a84e1aea7..184fbd877d 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -1478,6 +1478,17 @@ - (void)adjustSpeed:(id)sender
>
> @end
>
> +@interface QemuApplication : NSApplication
> +@end
> +
> +@implementation QemuApplication
> +- (void)sendEvent:(NSEvent *)event
> +{
> + COCOA_DEBUG("QemuApplication: sendEvent\n");
> + [super sendEvent: event];
> +}
> +@end
> +
> static void create_initial_menus(void)
> {
> // Add menus
> @@ -1691,7 +1702,7 @@ int main (int argc, const char * argv[]) {
> ProcessSerialNumber psn = { 0, kCurrentProcess };
> TransformProcessType(&psn, kProcessTransformToForegroundApplication);
>
> - [NSApplication sharedApplication];
> + [QemuApplication sharedApplication];
>
> create_initial_menus();
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/7] ui/cocoa: Don't call NSApp sendEvent directly from handleEvent
2019-02-14 17:04 ` BALATON Zoltan
@ 2019-02-14 17:41 ` Peter Maydell
2019-02-15 0:42 ` BALATON Zoltan
0 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2019-02-14 17:41 UTC (permalink / raw)
To: BALATON Zoltan
Cc: QEMU Developers, patches, John Arbuckle, Roman Bolshakov,
Berkus Decker, Gerd Hoffmann, Ben Hekster
On Thu, 14 Feb 2019 at 17:04, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Thu, 14 Feb 2019, Peter Maydell wrote:
> > Currently the handleEvent method will directly call the NSApp
> > sendEvent method for any events that we want to let OSX deal
> > with. When we rearrange the event handling code, the way that
> > we say "let OSX have this event" is going to change. Prepare
> > for that by refactoring so that handleEvent returns a flag
> > indicating whether it consumed the event.
> >
> > Suggested-by: BALATON Zoltan <balaton@eik.bme.hu>
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > +static bool bool_with_iothread_lock(BoolCodeBlock block)
> > +{
> > + bool locked = qemu_mutex_iothread_locked();
> > + bool val;
> > +
>
> Git complained about extra white space in the end of the empty line above
> but not sure if it was added during mailing or you have it in the original
> patch.
Almost certainly an error in the original patch. I'll fix it.
> > + if (!locked) {
> > + qemu_mutex_lock_iothread();
> > + }
> > + val = block();
> > + if (!locked) {
> > + qemu_mutex_unlock_iothread();
> > + }
> > + return val;
> > +}
> > +
> > // Mac to QKeyCode conversion
> > const int mac_to_qkeycode_map[] = {
> > [kVK_ANSI_A] = Q_KEY_CODE_A,
> > @@ -320,8 +336,8 @@ - (void) grabMouse;
> > - (void) ungrabMouse;
> > - (void) toggleFullScreen:(id)sender;
> > - (void) handleMonitorInput:(NSEvent *)event;
> > -- (void) handleEvent:(NSEvent *)event;
> > -- (void) handleEventLocked:(NSEvent *)event;
> > +- (bool) handleEvent:(NSEvent *)event;
> > +- (bool) handleEventLocked:(NSEvent *)event;
> > - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
> > /* The state surrounding mouse grabbing is potentially confusing.
> > * isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated
> > @@ -664,15 +680,16 @@ - (void) handleMonitorInput:(NSEvent *)event
> > }
> > }
> >
> > -- (void) handleEvent:(NSEvent *)event
> > +- (bool) handleEvent:(NSEvent *)event
> > {
> > - with_iothread_lock(^{
> > - [self handleEventLocked:event];
> > + return bool_with_iothread_lock(^{
> > + return [self handleEventLocked:event];
> > });
> > }
>
> If this is only ever used for this one method, wouldn't it be easier to
> move locking to the method below (even with some goto after setting a ret
> variable to unlock at the end of the method where now it returns in the
> middle, but maybe it could even be done without goto as the whole code is
> one big switch that can be exited with break and an if that can be skipped
> by a flag)? That may be easier to follow than this method within block
> within method and then you wouldn't need bool_with_iothread_lock and
> neither handleEvent. Unless there's something I'm missing which makes this
> convoluted way needed.
The aim was to avoid having to do changes to handleEvent's code flow
in order to do "run it with the lock held"; it also means that the
invariant "we always unlock the lock" is easy to confirm, whereas
if you do the lock/unlock inside a single handleEvent method you have
to look for whether it was done right in early-exit cases. It's
a bit less of a convincing argument than it was in v1 (where we were
making no changes to handleEvent at all other than wrapping it in a
lock), but I think it still makes sense this way.
> Other than that
> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
thanks
-- PMM
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
2019-02-14 10:28 [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop Peter Maydell
` (8 preceding siblings ...)
2019-02-14 11:11 ` no-reply
@ 2019-02-14 17:44 ` no-reply
2019-02-14 17:48 ` no-reply
` (5 subsequent siblings)
15 siblings, 0 replies; 38+ messages in thread
From: no-reply @ 2019-02-14 17:44 UTC (permalink / raw)
To: peter.maydell
Cc: fam, qemu-devel, patches, programmingkidx, r.bolshakov, kraxel,
ben.hekster, berkus
Patchew URL: https://patchew.org/QEMU/20190214102816.3393-1-peter.maydell@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Message-id: 20190214102816.3393-1-peter.maydell@linaro.org
Subject: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
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
Switched to a new branch 'test'
8d76c56d6c ui/cocoa: Perform UI operations only on the main thread
3aee068a7c ui/cocoa: Subclass NSApplication so we can implement sendEvent
1da7a6a150 ui/cocoa: Don't call NSApp sendEvent directly from handleEvent
07e0180720 ui/cocoa: Move console/device menu creation code up in file
ae490d83db ui/cocoa: Factor out initial menu creation
4b0ac50558 ui/cocoa: Use the pixman image directly in switchSurface
c83f0eb31d ui/cocoa: Ensure we have the iothread lock when calling into QEMU
=== OUTPUT BEGIN ===
1/7 Checking commit c83f0eb31d04 (ui/cocoa: Ensure we have the iothread lock when calling into QEMU)
2/7 Checking commit 4b0ac5055867 (ui/cocoa: Use the pixman image directly in switchSurface)
3/7 Checking commit ae490d83dba7 (ui/cocoa: Factor out initial menu creation)
4/7 Checking commit 07e018072093 (ui/cocoa: Move console/device menu creation code up in file)
5/7 Checking commit 1da7a6a15059 (ui/cocoa: Don't call NSApp sendEvent directly from handleEvent)
ERROR: trailing whitespace
#41: FILE: ui/cocoa.m:152:
+ $
total: 1 errors, 0 warnings, 122 lines checked
Patch 5/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/7 Checking commit 3aee068a7c6a (ui/cocoa: Subclass NSApplication so we can implement sendEvent)
7/7 Checking commit 8d76c56d6c33 (ui/cocoa: Perform UI operations only on the main thread)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190214102816.3393-1-peter.maydell@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
2019-02-14 10:28 [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop Peter Maydell
` (9 preceding siblings ...)
2019-02-14 17:44 ` no-reply
@ 2019-02-14 17:48 ` no-reply
2019-02-14 17:52 ` no-reply
` (4 subsequent siblings)
15 siblings, 0 replies; 38+ messages in thread
From: no-reply @ 2019-02-14 17:48 UTC (permalink / raw)
To: peter.maydell
Cc: fam, qemu-devel, patches, programmingkidx, r.bolshakov, kraxel,
ben.hekster, berkus
Patchew URL: https://patchew.org/QEMU/20190214102816.3393-1-peter.maydell@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
Message-id: 20190214102816.3393-1-peter.maydell@linaro.org
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
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
- [tag update] patchew/20190214102816.3393-1-peter.maydell@linaro.org -> patchew/20190214102816.3393-1-peter.maydell@linaro.org
- [tag update] patchew/cover.1549857716.git.balaton@eik.bme.hu -> patchew/cover.1549857716.git.balaton@eik.bme.hu
Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for path 'capstone'
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) registered for path 'roms/QemuMacDrivers'
Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 'roms/SLOF'
Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 'roms/ipxe'
Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered for path 'roms/openbios'
Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) registered for path 'roms/openhackware'
Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) registered for path 'roms/qemu-palcode'
Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for path 'roms/seabios'
Submodule 'roms/seabios-hppa' (https://github.com/hdeller/seabios-hppa.git) registered for path 'roms/seabios-hppa'
Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for path 'roms/sgabios'
Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for path 'roms/skiboot'
Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for path 'roms/u-boot'
Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) registered for path 'roms/u-boot-sam460ex'
Submodule 'tests/fp/berkeley-softfloat-3' (https://github.com/cota/berkeley-softfloat-3) registered for path 'tests/fp/berkeley-softfloat-3'
Submodule 'tests/fp/berkeley-testfloat-3' (https://github.com/cota/berkeley-testfloat-3) registered for path 'tests/fp/berkeley-testfloat-3'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) registered for path 'ui/keycodemapdb'
Cloning into 'capstone'...
Submodule path 'capstone': checked out '22ead3e0bfdb87516656453336160e0a37b066bf'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Cloning into 'roms/QemuMacDrivers'...
Submodule path 'roms/QemuMacDrivers': checked out '90c488d5f4a407342247b9ea869df1c2d9c8e266'
Cloning into 'roms/SLOF'...
Submodule path 'roms/SLOF': checked out 'a5b428e1c1eae703bdd62a3f527223c291ee3fdc'
Cloning into 'roms/ipxe'...
Submodule path 'roms/ipxe': checked out 'de4565cbe76ea9f7913a01f331be3ee901bb6e17'
Cloning into 'roms/openbios'...
Submodule path 'roms/openbios': checked out '441a84d3a642a10b948369c63f32367e8ff6395b'
Cloning into 'roms/openhackware'...
Submodule path 'roms/openhackware': checked out 'c559da7c8eec5e45ef1f67978827af6f0b9546f5'
Cloning into 'roms/qemu-palcode'...
Submodule path 'roms/qemu-palcode': checked out '51c237d7e20d05100eacadee2f61abc17e6bc097'
Cloning into 'roms/seabios'...
Submodule path 'roms/seabios': checked out 'a698c8995ffb2838296ec284fe3c4ad33dfca307'
Cloning into 'roms/seabios-hppa'...
Submodule path 'roms/seabios-hppa': checked out '1ef99a01572c2581c30e16e6fe69e9ea2ef92ce0'
Cloning into 'roms/sgabios'...
Submodule path 'roms/sgabios': checked out 'cbaee52287e5f32373181cff50a00b6c4ac9015a'
Cloning into 'roms/skiboot'...
Submodule path 'roms/skiboot': checked out 'e0ee24c27a172bcf482f6f2bc905e6211c134bcc'
Cloning into 'roms/u-boot'...
Submodule path 'roms/u-boot': checked out 'd85ca029f257b53a96da6c2fb421e78a003a9943'
Cloning into 'roms/u-boot-sam460ex'...
Submodule path 'roms/u-boot-sam460ex': checked out '60b3916f33e617a815973c5a6df77055b2e3a588'
Cloning into 'tests/fp/berkeley-softfloat-3'...
Submodule path 'tests/fp/berkeley-softfloat-3': checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'tests/fp/berkeley-testfloat-3'...
Submodule path 'tests/fp/berkeley-testfloat-3': checked out '5a59dcec19327396a011a17fd924aed4fec416b3'
Cloning into 'ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
Switched to a new branch 'test'
8d76c56 ui/cocoa: Perform UI operations only on the main thread
3aee068 ui/cocoa: Subclass NSApplication so we can implement sendEvent
1da7a6a ui/cocoa: Don't call NSApp sendEvent directly from handleEvent
07e0180 ui/cocoa: Move console/device menu creation code up in file
ae490d8 ui/cocoa: Factor out initial menu creation
4b0ac50 ui/cocoa: Use the pixman image directly in switchSurface
c83f0eb ui/cocoa: Ensure we have the iothread lock when calling into QEMU
=== OUTPUT BEGIN ===
1/7 Checking commit c83f0eb31d04 (ui/cocoa: Ensure we have the iothread lock when calling into QEMU)
2/7 Checking commit 4b0ac5055867 (ui/cocoa: Use the pixman image directly in switchSurface)
3/7 Checking commit ae490d83dba7 (ui/cocoa: Factor out initial menu creation)
4/7 Checking commit 07e018072093 (ui/cocoa: Move console/device menu creation code up in file)
5/7 Checking commit 1da7a6a15059 (ui/cocoa: Don't call NSApp sendEvent directly from handleEvent)
ERROR: trailing whitespace
#41: FILE: ui/cocoa.m:152:
+ $
total: 1 errors, 0 warnings, 122 lines checked
Patch 5/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/7 Checking commit 3aee068a7c6a (ui/cocoa: Subclass NSApplication so we can implement sendEvent)
7/7 Checking commit 8d76c56d6c33 (ui/cocoa: Perform UI operations only on the main thread)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190214102816.3393-1-peter.maydell@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
2019-02-14 10:28 [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop Peter Maydell
` (10 preceding siblings ...)
2019-02-14 17:48 ` no-reply
@ 2019-02-14 17:52 ` no-reply
2019-02-15 1:01 ` no-reply
` (3 subsequent siblings)
15 siblings, 0 replies; 38+ messages in thread
From: no-reply @ 2019-02-14 17:52 UTC (permalink / raw)
To: peter.maydell
Cc: fam, qemu-devel, patches, programmingkidx, r.bolshakov, kraxel,
ben.hekster, berkus
Patchew URL: https://patchew.org/QEMU/20190214102816.3393-1-peter.maydell@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Message-id: 20190214102816.3393-1-peter.maydell@linaro.org
Subject: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
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
t [tag update] patchew/20190214102816.3393-1-peter.maydell@linaro.org -> patchew/20190214102816.3393-1-peter.maydell@linaro.org
Switched to a new branch 'test'
0538b64902 ui/cocoa: Perform UI operations only on the main thread
9c813776c5 ui/cocoa: Subclass NSApplication so we can implement sendEvent
ee910daa7f ui/cocoa: Don't call NSApp sendEvent directly from handleEvent
475c73329c ui/cocoa: Move console/device menu creation code up in file
84187b00a1 ui/cocoa: Factor out initial menu creation
ec9d2d35f1 ui/cocoa: Use the pixman image directly in switchSurface
31e3ce4546 ui/cocoa: Ensure we have the iothread lock when calling into QEMU
=== OUTPUT BEGIN ===
1/7 Checking commit 31e3ce4546b9 (ui/cocoa: Ensure we have the iothread lock when calling into QEMU)
2/7 Checking commit ec9d2d35f160 (ui/cocoa: Use the pixman image directly in switchSurface)
3/7 Checking commit 84187b00a139 (ui/cocoa: Factor out initial menu creation)
4/7 Checking commit 475c73329cdd (ui/cocoa: Move console/device menu creation code up in file)
5/7 Checking commit ee910daa7f20 (ui/cocoa: Don't call NSApp sendEvent directly from handleEvent)
ERROR: trailing whitespace
#42: FILE: ui/cocoa.m:152:
+ $
total: 1 errors, 0 warnings, 122 lines checked
Patch 5/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/7 Checking commit 9c813776c52a (ui/cocoa: Subclass NSApplication so we can implement sendEvent)
7/7 Checking commit 0538b64902ce (ui/cocoa: Perform UI operations only on the main thread)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190214102816.3393-1-peter.maydell@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/7] ui/cocoa: Move console/device menu creation code up in file
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 4/7] ui/cocoa: Move console/device menu creation code up in file Peter Maydell
@ 2019-02-15 0:34 ` BALATON Zoltan
2019-02-22 18:10 ` Roman Bolshakov
1 sibling, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2019-02-15 0:34 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, patches, John Arbuckle, Roman Bolshakov,
Berkus Decker, Gerd Hoffmann, Ben Hekster
On Thu, 14 Feb 2019, Peter Maydell wrote:
> Move the console/device menu creation code functions
> further up in the source file, next to the code which
> creates the initial menus. We're going to want to
> change the location we call these functions from in
> the next patch.
>
> This commit is a pure code move with no other changes.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Regards,
BALATON Zoltan
> ---
> ui/cocoa.m | 184 ++++++++++++++++++++++++++---------------------------
> 1 file changed, 92 insertions(+), 92 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 0b1cd31543..2d943b6e2a 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -1548,6 +1548,98 @@ static void create_initial_menus(void)
> [[NSApp mainMenu] addItem:menuItem];
> }
>
> +/* Returns a name for a given console */
> +static NSString * getConsoleName(QemuConsole * console)
> +{
> + return [NSString stringWithFormat: @"%s", qemu_console_get_label(console)];
> +}
> +
> +/* Add an entry to the View menu for each console */
> +static void add_console_menu_entries(void)
> +{
> + NSMenu *menu;
> + NSMenuItem *menuItem;
> + int index = 0;
> +
> + menu = [[[NSApp mainMenu] itemWithTitle:@"View"] submenu];
> +
> + [menu addItem:[NSMenuItem separatorItem]];
> +
> + while (qemu_console_lookup_by_index(index) != NULL) {
> + menuItem = [[[NSMenuItem alloc] initWithTitle: getConsoleName(qemu_console_lookup_by_index(index))
> + action: @selector(displayConsole:) keyEquivalent: @""] autorelease];
> + [menuItem setTag: index];
> + [menu addItem: menuItem];
> + index++;
> + }
> +}
> +
> +/* Make menu items for all removable devices.
> + * Each device is given an 'Eject' and 'Change' menu item.
> + */
> +static void addRemovableDevicesMenuItems(void)
> +{
> + NSMenu *menu;
> + NSMenuItem *menuItem;
> + BlockInfoList *currentDevice, *pointerToFree;
> + NSString *deviceName;
> +
> + currentDevice = qmp_query_block(NULL);
> + pointerToFree = currentDevice;
> + if(currentDevice == NULL) {
> + NSBeep();
> + QEMU_Alert(@"Failed to query for block devices!");
> + return;
> + }
> +
> + menu = [[[NSApp mainMenu] itemWithTitle:@"Machine"] submenu];
> +
> + // Add a separator between related groups of menu items
> + [menu addItem:[NSMenuItem separatorItem]];
> +
> + // Set the attributes to the "Removable Media" menu item
> + NSString *titleString = @"Removable Media";
> + NSMutableAttributedString *attString=[[NSMutableAttributedString alloc] initWithString:titleString];
> + NSColor *newColor = [NSColor blackColor];
> + NSFontManager *fontManager = [NSFontManager sharedFontManager];
> + NSFont *font = [fontManager fontWithFamily:@"Helvetica"
> + traits:NSBoldFontMask|NSItalicFontMask
> + weight:0
> + size:14];
> + [attString addAttribute:NSFontAttributeName value:font range:NSMakeRange(0, [titleString length])];
> + [attString addAttribute:NSForegroundColorAttributeName value:newColor range:NSMakeRange(0, [titleString length])];
> + [attString addAttribute:NSUnderlineStyleAttributeName value:[NSNumber numberWithInt: 1] range:NSMakeRange(0, [titleString length])];
> +
> + // Add the "Removable Media" menu item
> + menuItem = [NSMenuItem new];
> + [menuItem setAttributedTitle: attString];
> + [menuItem setEnabled: NO];
> + [menu addItem: menuItem];
> +
> + /* Loop through all the block devices in the emulator */
> + while (currentDevice) {
> + deviceName = [[NSString stringWithFormat: @"%s", currentDevice->value->device] retain];
> +
> + if(currentDevice->value->removable) {
> + menuItem = [[NSMenuItem alloc] initWithTitle: [NSString stringWithFormat: @"Change %s...", currentDevice->value->device]
> + action: @selector(changeDeviceMedia:)
> + keyEquivalent: @""];
> + [menu addItem: menuItem];
> + [menuItem setRepresentedObject: deviceName];
> + [menuItem autorelease];
> +
> + menuItem = [[NSMenuItem alloc] initWithTitle: [NSString stringWithFormat: @"Eject %s", currentDevice->value->device]
> + action: @selector(ejectDeviceMedia:)
> + keyEquivalent: @""];
> + [menu addItem: menuItem];
> + [menuItem setRepresentedObject: deviceName];
> + [menuItem autorelease];
> + }
> + currentDevice = currentDevice->next;
> + }
> + qapi_free_BlockInfoList(pointerToFree);
> +}
> +
> int main (int argc, const char * argv[]) {
>
> gArgc = argc;
> @@ -1676,98 +1768,6 @@ static void cocoa_cleanup(void)
> .dpy_refresh = cocoa_refresh,
> };
>
> -/* Returns a name for a given console */
> -static NSString * getConsoleName(QemuConsole * console)
> -{
> - return [NSString stringWithFormat: @"%s", qemu_console_get_label(console)];
> -}
> -
> -/* Add an entry to the View menu for each console */
> -static void add_console_menu_entries(void)
> -{
> - NSMenu *menu;
> - NSMenuItem *menuItem;
> - int index = 0;
> -
> - menu = [[[NSApp mainMenu] itemWithTitle:@"View"] submenu];
> -
> - [menu addItem:[NSMenuItem separatorItem]];
> -
> - while (qemu_console_lookup_by_index(index) != NULL) {
> - menuItem = [[[NSMenuItem alloc] initWithTitle: getConsoleName(qemu_console_lookup_by_index(index))
> - action: @selector(displayConsole:) keyEquivalent: @""] autorelease];
> - [menuItem setTag: index];
> - [menu addItem: menuItem];
> - index++;
> - }
> -}
> -
> -/* Make menu items for all removable devices.
> - * Each device is given an 'Eject' and 'Change' menu item.
> - */
> -static void addRemovableDevicesMenuItems(void)
> -{
> - NSMenu *menu;
> - NSMenuItem *menuItem;
> - BlockInfoList *currentDevice, *pointerToFree;
> - NSString *deviceName;
> -
> - currentDevice = qmp_query_block(NULL);
> - pointerToFree = currentDevice;
> - if(currentDevice == NULL) {
> - NSBeep();
> - QEMU_Alert(@"Failed to query for block devices!");
> - return;
> - }
> -
> - menu = [[[NSApp mainMenu] itemWithTitle:@"Machine"] submenu];
> -
> - // Add a separator between related groups of menu items
> - [menu addItem:[NSMenuItem separatorItem]];
> -
> - // Set the attributes to the "Removable Media" menu item
> - NSString *titleString = @"Removable Media";
> - NSMutableAttributedString *attString=[[NSMutableAttributedString alloc] initWithString:titleString];
> - NSColor *newColor = [NSColor blackColor];
> - NSFontManager *fontManager = [NSFontManager sharedFontManager];
> - NSFont *font = [fontManager fontWithFamily:@"Helvetica"
> - traits:NSBoldFontMask|NSItalicFontMask
> - weight:0
> - size:14];
> - [attString addAttribute:NSFontAttributeName value:font range:NSMakeRange(0, [titleString length])];
> - [attString addAttribute:NSForegroundColorAttributeName value:newColor range:NSMakeRange(0, [titleString length])];
> - [attString addAttribute:NSUnderlineStyleAttributeName value:[NSNumber numberWithInt: 1] range:NSMakeRange(0, [titleString length])];
> -
> - // Add the "Removable Media" menu item
> - menuItem = [NSMenuItem new];
> - [menuItem setAttributedTitle: attString];
> - [menuItem setEnabled: NO];
> - [menu addItem: menuItem];
> -
> - /* Loop through all the block devices in the emulator */
> - while (currentDevice) {
> - deviceName = [[NSString stringWithFormat: @"%s", currentDevice->value->device] retain];
> -
> - if(currentDevice->value->removable) {
> - menuItem = [[NSMenuItem alloc] initWithTitle: [NSString stringWithFormat: @"Change %s...", currentDevice->value->device]
> - action: @selector(changeDeviceMedia:)
> - keyEquivalent: @""];
> - [menu addItem: menuItem];
> - [menuItem setRepresentedObject: deviceName];
> - [menuItem autorelease];
> -
> - menuItem = [[NSMenuItem alloc] initWithTitle: [NSString stringWithFormat: @"Eject %s", currentDevice->value->device]
> - action: @selector(ejectDeviceMedia:)
> - keyEquivalent: @""];
> - [menu addItem: menuItem];
> - [menuItem setRepresentedObject: deviceName];
> - [menuItem autorelease];
> - }
> - currentDevice = currentDevice->next;
> - }
> - qapi_free_BlockInfoList(pointerToFree);
> -}
> -
> static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
> {
> COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n");
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/7] ui/cocoa: Don't call NSApp sendEvent directly from handleEvent
2019-02-14 17:41 ` Peter Maydell
@ 2019-02-15 0:42 ` BALATON Zoltan
0 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2019-02-15 0:42 UTC (permalink / raw)
To: Peter Maydell
Cc: QEMU Developers, patches, John Arbuckle, Roman Bolshakov,
Berkus Decker, Gerd Hoffmann, Ben Hekster
On Thu, 14 Feb 2019, Peter Maydell wrote:
> On Thu, 14 Feb 2019 at 17:04, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>> -- (void) handleEvent:(NSEvent *)event
>>> +- (bool) handleEvent:(NSEvent *)event
>>> {
>>> - with_iothread_lock(^{
>>> - [self handleEventLocked:event];
>>> + return bool_with_iothread_lock(^{
>>> + return [self handleEventLocked:event];
>>> });
>>> }
>>
>> If this is only ever used for this one method, wouldn't it be easier to
>> move locking to the method below (even with some goto after setting a ret
>> variable to unlock at the end of the method where now it returns in the
>> middle, but maybe it could even be done without goto as the whole code is
>> one big switch that can be exited with break and an if that can be skipped
>> by a flag)? That may be easier to follow than this method within block
>> within method and then you wouldn't need bool_with_iothread_lock and
>> neither handleEvent. Unless there's something I'm missing which makes this
>> convoluted way needed.
>
> The aim was to avoid having to do changes to handleEvent's code flow
> in order to do "run it with the lock held"; it also means that the
> invariant "we always unlock the lock" is easy to confirm, whereas
> if you do the lock/unlock inside a single handleEvent method you have
> to look for whether it was done right in early-exit cases. It's
> a bit less of a convincing argument than it was in v1 (where we were
> making no changes to handleEvent at all other than wrapping it in a
> lock), but I think it still makes sense this way.
I thought of something like below which to me is simpler than your
proposal and makes less changes to the original but feel free to choose
whichever you like, I don't mind either way.
But the comment about sendEvent near the end is now outdated after either
patch so you may want to update that even in your patch.
Regards,
BALATON Zoltan
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 4727f9c392..de6606e017 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -132,9 +132,8 @@
QemuSemaphore display_init_sem;
QemuSemaphore app_started_sem;
-// Utility functions to run specified code block with iothread lock held
+// Utility function to run specified code block with iothread lock held
typedef void (^CodeBlock)(void);
-typedef bool (^BoolCodeBlock)(void);
static void with_iothread_lock(CodeBlock block)
{
@@ -148,21 +147,6 @@ static void with_iothread_lock(CodeBlock block)
}
}
-static bool bool_with_iothread_lock(BoolCodeBlock block)
-{
- bool locked = qemu_mutex_iothread_locked();
- bool val;
-
- if (!locked) {
- qemu_mutex_lock_iothread();
- }
- val = block();
- if (!locked) {
- qemu_mutex_unlock_iothread();
- }
- return val;
-}
-
// Mac to QKeyCode conversion
const int mac_to_qkeycode_map[] = {
[kVK_ANSI_A] = Q_KEY_CODE_A,
@@ -341,7 +325,6 @@ - (void) ungrabMouse;
- (void) toggleFullScreen:(id)sender;
- (void) handleMonitorInput:(NSEvent *)event;
- (bool) handleEvent:(NSEvent *)event;
-- (bool) handleEventLocked:(NSEvent *)event;
- (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
/* The state surrounding mouse grabbing is potentially confusing.
* isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated
@@ -689,15 +672,15 @@ - (void) handleMonitorInput:(NSEvent *)event
- (bool) handleEvent:(NSEvent *)event
{
- return bool_with_iothread_lock(^{
- return [self handleEventLocked:event];
- });
-}
-
-- (bool) handleEventLocked:(NSEvent *)event
-{
- /* Return true if we handled the event, false if it should be given to OSX */
COCOA_DEBUG("QemuCocoaView: handleEvent\n");
+ /* Return true if we handled the event, false if it should be given to OSX */
+ bool handled = true;
+ bool locked = qemu_mutex_iothread_locked();
+ if (!locked) {
+ qemu_mutex_lock_iothread();
+ }
+ /* Make sure to release lock so don't return before the end */
+
int buttons = 0;
int keycode = 0;
bool mouse_event = false;
@@ -767,7 +750,8 @@ - (bool) handleEventLocked:(NSEvent *)event
if (keycode == Q_KEY_CODE_F) {
switched_to_fullscreen = true;
}
- return false;
+ handled = false;
+ goto unlock;
}
// default
@@ -782,12 +766,12 @@ - (bool) handleEventLocked:(NSEvent *)event
// enable graphic console
case '1' ... '9':
console_select(key - '0' - 1); /* ascii math */
- return true;
+ goto unlock;
// release the mouse grab
case 'g':
[self ungrabMouse];
- return true;
+ goto unlock;
}
}
}
@@ -804,7 +788,7 @@ - (bool) handleEventLocked:(NSEvent *)event
// don't pass the guest a spurious key-up if we treated this
// command-key combo as a host UI action
if (!isMouseGrabbed && ([event modifierFlags] & NSEventModifierFlagCommand)) {
- return true;
+ goto unlock;
}
if (qemu_console_is_graphic(NULL)) {
@@ -898,16 +882,16 @@ - (bool) handleEventLocked:(NSEvent *)event
mouse_event = false;
break;
default:
- return false;
+ handled = false;
+ goto unlock;
}
if (mouse_event) {
/* Don't send button events to the guest unless we've got a
* mouse grab or window focus. If we have neither then this event
* is the user clicking on the background window to activate and
- * bring us to the front, which will be done by the sendEvent
- * call below. We definitely don't want to pass that click through
- * to the guest.
+ * bring us to the front, which will be done by sendEvent.
+ * We definitely don't want to pass that click through to the guest.
*/
if ((isMouseGrabbed || [[self window] isKeyWindow]) &&
(last_buttons != buttons)) {
@@ -934,11 +918,16 @@ - (bool) handleEventLocked:(NSEvent *)event
qemu_input_queue_rel(dcl->con, INPUT_AXIS_Y, (int)[event deltaY]);
}
} else {
- return false;
+ handled = false;
+ goto unlock;
}
qemu_input_event_sync();
}
- return true;
+unlock:
+ if (!locked) {
+ qemu_mutex_unlock_iothread();
+ }
+ return handled;
}
- (void) grabMouse
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 7/7] ui/cocoa: Perform UI operations only on the main thread
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 7/7] ui/cocoa: Perform UI operations only on the main thread Peter Maydell
@ 2019-02-15 0:55 ` BALATON Zoltan
2019-02-15 1:28 ` BALATON Zoltan
2019-02-22 21:35 ` Roman Bolshakov
2 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2019-02-15 0:55 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, patches, John Arbuckle, Roman Bolshakov,
Berkus Decker, Gerd Hoffmann, Ben Hekster
On Thu, 14 Feb 2019, Peter Maydell wrote:
> The OSX Mojave release is more picky about enforcing the Cocoa API
> restriction that only the main thread may perform UI calls. To
> accommodate this we need to restructure the Cocoa code:
> * the special OSX main() creates a second thread and uses
> that to call the vl.c qemu_main(); the original main
> thread goes into the OSX event loop
> * the refresh, switch and update callbacks asynchronously
> tell the main thread to do the necessary work
> * the refresh callback no longer does the "get events from the
> UI event queue and handle them" loop, since we now use
> the stock OSX event loop. Instead our NSApplication sendEvent
> method will either deal with them or pass them on to OSX
>
> All these things have to be changed in one commit, to avoid
> breaking bisection.
>
> Note that since we use dispatch_get_main_queue(), this bumps
> our minimum version requirement to OSX 10.10 Yosemite (released
> in 2014, unsupported by Apple since 2017).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> v2 changes: call handleEvent from sendEvent
There's still something not right I'm afraid. After this menus stop
working for me (they don't react to mouse although keyboard short-cuts are
still working) but I don't know how to fix this. I could also reproduce a
stuck modifier key problem but not sure if that's preexisting and probably
obscure enough in itself to not hold back this series if the menu problem
is fixed. I'm just documenting it here for later debugging:
1. start qemu-system-ppc -M mac99 (or maybe other but that's what I've tried)
2. Switch to monitor with Ctrl-Alt-2
3. Switch to full screen with Cmd-F
4. type to verify it works
5. Still in full screen, switch to main screen with Ctrl-Alt-1
6. Now sometimes Alt seems to be stuck even after exiting full screen
I'm also sending another patch on top of this series that brings the app
to the front to avoid it starting behind the active Terminal window when
started from the command line that has annoyed me in the past.
Regards,
BALATON Zoltan
> ---
> ui/cocoa.m | 191 +++++++++++++++++++++++++++++++----------------------
> 1 file changed, 113 insertions(+), 78 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 184fbd877d..201a294ed4 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -129,6 +129,9 @@
> NSTextField *pauseLabel;
> NSArray * supportedImageFileTypes;
>
> +QemuSemaphore display_init_sem;
> +QemuSemaphore app_started_sem;
> +
> // Utility functions to run specified code block with iothread lock held
> typedef void (^CodeBlock)(void);
> typedef bool (^BoolCodeBlock)(void);
> @@ -325,6 +328,7 @@ @interface QemuCocoaView : NSView
> NSWindow *fullScreenWindow;
> float cx,cy,cw,ch,cdx,cdy;
> CGDataProviderRef dataProviderRef;
> + pixman_image_t *pixman_image;
> BOOL modifiers_state[256];
> BOOL isMouseGrabbed;
> BOOL isFullscreen;
> @@ -535,13 +539,16 @@ - (void) switchSurface:(pixman_image_t *)image
> }
>
> // update screenBuffer
> - if (dataProviderRef)
> + if (dataProviderRef) {
> CGDataProviderRelease(dataProviderRef);
> + pixman_image_unref(pixman_image);
> + }
>
> //sync host window color space with guests
> screen.bitsPerPixel = PIXMAN_FORMAT_BPP(image_format);
> screen.bitsPerComponent = DIV_ROUND_UP(screen.bitsPerPixel, 8) * 2;
>
> + pixman_image = image;
> dataProviderRef = CGDataProviderCreateWithData(NULL, pixman_image_get_data(image), w * 4 * h, NULL);
>
> // update windows
> @@ -1013,7 +1020,6 @@ @interface QemuCocoaAppController : NSObject
> #endif
> {
> }
> -- (void)startEmulationWithArgc:(int)argc argv:(char**)argv;
> - (void)doToggleFullScreen:(id)sender;
> - (void)toggleFullScreen:(id)sender;
> - (void)showQEMUDoc:(id)sender;
> @@ -1101,8 +1107,8 @@ - (void) dealloc
> - (void)applicationDidFinishLaunching: (NSNotification *) note
> {
> COCOA_DEBUG("QemuCocoaAppController: applicationDidFinishLaunching\n");
> - // launch QEMU, with the global args
> - [self startEmulationWithArgc:gArgc argv:(char **)gArgv];
> + /* Tell cocoa_display_init to proceed */
> + qemu_sem_post(&app_started_sem);
> }
>
> - (void)applicationWillTerminate:(NSNotification *)aNotification
> @@ -1145,15 +1151,6 @@ - (void) applicationWillResignActive: (NSNotification *)aNotification
> [cocoaView raiseAllKeys];
> }
>
> -- (void)startEmulationWithArgc:(int)argc argv:(char**)argv
> -{
> - COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
> -
> - int status;
> - status = qemu_main(argc, argv, *_NSGetEnviron());
> - exit(status);
> -}
> -
> /* We abstract the method called by the Enter Fullscreen menu item
> * because Mac OS 10.7 and higher disables it. This is because of the
> * menu item's old selector's name toggleFullScreen:
> @@ -1485,7 +1482,9 @@ @implementation QemuApplication
> - (void)sendEvent:(NSEvent *)event
> {
> COCOA_DEBUG("QemuApplication: sendEvent\n");
> - [super sendEvent: event];
> + if (!cocoaView || ![cocoaView handleEvent:event]) {
> + [super sendEvent: event];
> + }
> }
> @end
>
> @@ -1668,32 +1667,59 @@ static void addRemovableDevicesMenuItems(void)
> qapi_free_BlockInfoList(pointerToFree);
> }
>
> -int main (int argc, const char * argv[]) {
> +/*
> + * The startup process for the OSX/Cocoa UI is complicated, because
> + * OSX insists that the UI runs on the initial main thread, and so we
> + * need to start a second thread which runs the vl.c qemu_main():
> + *
> + * Initial thread: 2nd thread:
> + * in main():
> + * create qemu-main thread
> + * wait on display_init semaphore
> + * call qemu_main()
> + * ...
> + * in cocoa_display_init():
> + * post the display_init semaphore
> + * wait on app_started semaphore
> + * create application, menus, etc
> + * enter OSX run loop
> + * in applicationDidFinishLaunching:
> + * post app_started semaphore
> + * tell main thread to fullscreen if needed
> + * [...]
> + * run qemu main-loop
> + *
> + * We do this in two stages so that we don't do the creation of the
> + * GUI application menus and so on for command line options like --help
> + * where we want to just print text to stdout and exit immediately.
> + */
>
> +static void *call_qemu_main(void *opaque)
> +{
> + int status;
> +
> + COCOA_DEBUG("Second thread: calling qemu_main()\n");
> + status = qemu_main(gArgc, gArgv, *_NSGetEnviron());
> + COCOA_DEBUG("Second thread: qemu_main() returned, exiting\n");
> + exit(status);
> +}
> +
> +int main (int argc, const char * argv[]) {
> + QemuThread thread;
> +
> + COCOA_DEBUG("Entered main()\n");
> gArgc = argc;
> gArgv = (char **)argv;
> - int i;
>
> - /* In case we don't need to display a window, let's not do that */
> - for (i = 1; i < argc; i++) {
> - const char *opt = argv[i];
> + qemu_sem_init(&display_init_sem, 0);
> + qemu_sem_init(&app_started_sem, 0);
>
> - if (opt[0] == '-') {
> - /* Treat --foo the same as -foo. */
> - if (opt[1] == '-') {
> - opt++;
> - }
> - if (!strcmp(opt, "-h") || !strcmp(opt, "-help") ||
> - !strcmp(opt, "-vnc") ||
> - !strcmp(opt, "-nographic") ||
> - !strcmp(opt, "-version") ||
> - !strcmp(opt, "-curses") ||
> - !strcmp(opt, "-display") ||
> - !strcmp(opt, "-qtest")) {
> - return qemu_main(gArgc, gArgv, *_NSGetEnviron());
> - }
> - }
> - }
> + qemu_thread_create(&thread, "qemu_main", call_qemu_main,
> + NULL, QEMU_THREAD_DETACHED);
> +
> + COCOA_DEBUG("Main thread: waiting for display_init_sem\n");
> + qemu_sem_wait(&display_init_sem);
> + COCOA_DEBUG("Main thread: initializing app\n");
>
> NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
>
> @@ -1706,12 +1732,24 @@ int main (int argc, const char * argv[]) {
>
> create_initial_menus();
>
> + /*
> + * Create the menu entries which depend on QEMU state (for consoles
> + * and removeable devices). These make calls back into QEMU functions,
> + * which is OK because at this point we know that the second thread
> + * holds the iothread lock and is synchronously waiting for us to
> + * finish.
> + */
> + add_console_menu_entries();
> + addRemovableDevicesMenuItems();
> +
> // Create an Application controller
> QemuCocoaAppController *appController = [[QemuCocoaAppController alloc] init];
> [NSApp setDelegate:appController];
>
> // Start the main event loop
> + COCOA_DEBUG("Main thread: entering OSX run loop\n");
> [NSApp run];
> + COCOA_DEBUG("Main thread: left OSX run loop, exiting\n");
>
> [appController release];
> [pool release];
> @@ -1729,17 +1767,19 @@ static void cocoa_update(DisplayChangeListener *dcl,
>
> COCOA_DEBUG("qemu_cocoa: cocoa_update\n");
>
> - NSRect rect;
> - if ([cocoaView cdx] == 1.0) {
> - rect = NSMakeRect(x, [cocoaView gscreen].height - y - h, w, h);
> - } else {
> - rect = NSMakeRect(
> - x * [cocoaView cdx],
> - ([cocoaView gscreen].height - y - h) * [cocoaView cdy],
> - w * [cocoaView cdx],
> - h * [cocoaView cdy]);
> - }
> - [cocoaView setNeedsDisplayInRect:rect];
> + dispatch_async(dispatch_get_main_queue(), ^{
> + NSRect rect;
> + if ([cocoaView cdx] == 1.0) {
> + rect = NSMakeRect(x, [cocoaView gscreen].height - y - h, w, h);
> + } else {
> + rect = NSMakeRect(
> + x * [cocoaView cdx],
> + ([cocoaView gscreen].height - y - h) * [cocoaView cdy],
> + w * [cocoaView cdx],
> + h * [cocoaView cdy]);
> + }
> + [cocoaView setNeedsDisplayInRect:rect];
> + });
>
> [pool release];
> }
> @@ -1748,9 +1788,19 @@ static void cocoa_switch(DisplayChangeListener *dcl,
> DisplaySurface *surface)
> {
> NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
> + pixman_image_t *image = surface->image;
>
> COCOA_DEBUG("qemu_cocoa: cocoa_switch\n");
> - [cocoaView switchSurface:surface->image];
> +
> + // The DisplaySurface will be freed as soon as this callback returns.
> + // We take a reference to the underlying pixman image here so it does
> + // not disappear from under our feet; the switchSurface method will
> + // deref the old image when it is done with it.
> + pixman_image_ref(image);
> +
> + dispatch_async(dispatch_get_main_queue(), ^{
> + [cocoaView switchSurface:image];
> + });
> [pool release];
> }
>
> @@ -1762,26 +1812,15 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
> graphic_hw_update(NULL);
>
> if (qemu_input_is_absolute()) {
> - if (![cocoaView isAbsoluteEnabled]) {
> - if ([cocoaView isMouseGrabbed]) {
> - [cocoaView ungrabMouse];
> + dispatch_async(dispatch_get_main_queue(), ^{
> + if (![cocoaView isAbsoluteEnabled]) {
> + if ([cocoaView isMouseGrabbed]) {
> + [cocoaView ungrabMouse];
> + }
> }
> - }
> - [cocoaView setAbsoluteEnabled:YES];
> + [cocoaView setAbsoluteEnabled:YES];
> + });
> }
> -
> - NSDate *distantPast;
> - NSEvent *event;
> - distantPast = [NSDate distantPast];
> - do {
> - event = [NSApp nextEventMatchingMask:NSEventMaskAny untilDate:distantPast
> - inMode: NSDefaultRunLoopMode dequeue:YES];
> - if (event != nil) {
> - if (![cocoaView handleEvent:event]) {
> - [NSApp sendEvent:event];
> - }
> - }
> - } while(event != nil);
> [pool release];
> }
>
> @@ -1802,10 +1841,17 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
> {
> COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n");
>
> + /* Tell main thread to go ahead and create the app and enter the run loop */
> + qemu_sem_post(&display_init_sem);
> + qemu_sem_wait(&app_started_sem);
> + COCOA_DEBUG("cocoa_display_init: app start completed\n");
> +
> /* if fullscreen mode is to be used */
> if (opts->has_full_screen && opts->full_screen) {
> - [NSApp activateIgnoringOtherApps: YES];
> - [(QemuCocoaAppController *)[[NSApplication sharedApplication] delegate] toggleFullScreen: nil];
> + dispatch_async(dispatch_get_main_queue(), ^{
> + [NSApp activateIgnoringOtherApps: YES];
> + [(QemuCocoaAppController *)[[NSApplication sharedApplication] delegate] toggleFullScreen: nil];
> + });
> }
>
> dcl = g_malloc0(sizeof(DisplayChangeListener));
> @@ -1816,17 +1862,6 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
>
> // register cleanup function
> atexit(cocoa_cleanup);
> -
> - /* At this point QEMU has created all the consoles, so we can add View
> - * menu entries for them.
> - */
> - add_console_menu_entries();
> -
> - /* Give all removable devices a menu item.
> - * Has to be called after QEMU has started to
> - * find out what removable devices it has.
> - */
> - addRemovableDevicesMenuItems();
> }
>
> static QemuDisplay qemu_display_cocoa = {
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
2019-02-14 10:28 [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop Peter Maydell
` (11 preceding siblings ...)
2019-02-14 17:52 ` no-reply
@ 2019-02-15 1:01 ` no-reply
2019-02-27 15:24 ` no-reply
` (2 subsequent siblings)
15 siblings, 0 replies; 38+ messages in thread
From: no-reply @ 2019-02-15 1:01 UTC (permalink / raw)
To: peter.maydell
Cc: fam, qemu-devel, patches, programmingkidx, r.bolshakov, kraxel,
ben.hekster, berkus
Patchew URL: https://patchew.org/QEMU/20190214102816.3393-1-peter.maydell@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Message-id: 20190214102816.3393-1-peter.maydell@linaro.org
Subject: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
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
t [tag update] patchew/20190214102816.3393-1-peter.maydell@linaro.org -> patchew/20190214102816.3393-1-peter.maydell@linaro.org
t [tag update] patchew/20190214190603.25030-1-peter.maydell@linaro.org -> patchew/20190214190603.25030-1-peter.maydell@linaro.org
Switched to a new branch 'test'
9aa7dd27d3 ui/cocoa: Perform UI operations only on the main thread
417e53a95a ui/cocoa: Subclass NSApplication so we can implement sendEvent
211ca5f248 ui/cocoa: Don't call NSApp sendEvent directly from handleEvent
17de4a307c ui/cocoa: Move console/device menu creation code up in file
cf54a90e45 ui/cocoa: Factor out initial menu creation
5f7c287c74 ui/cocoa: Use the pixman image directly in switchSurface
53f929697b ui/cocoa: Ensure we have the iothread lock when calling into QEMU
=== OUTPUT BEGIN ===
1/7 Checking commit 53f929697be6 (ui/cocoa: Ensure we have the iothread lock when calling into QEMU)
2/7 Checking commit 5f7c287c7483 (ui/cocoa: Use the pixman image directly in switchSurface)
3/7 Checking commit cf54a90e45cd (ui/cocoa: Factor out initial menu creation)
4/7 Checking commit 17de4a307c5a (ui/cocoa: Move console/device menu creation code up in file)
5/7 Checking commit 211ca5f248f2 (ui/cocoa: Don't call NSApp sendEvent directly from handleEvent)
ERROR: trailing whitespace
#42: FILE: ui/cocoa.m:152:
+ $
total: 1 errors, 0 warnings, 122 lines checked
Patch 5/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/7 Checking commit 417e53a95ae0 (ui/cocoa: Subclass NSApplication so we can implement sendEvent)
7/7 Checking commit 9aa7dd27d3bb (ui/cocoa: Perform UI operations only on the main thread)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190214102816.3393-1-peter.maydell@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 7/7] ui/cocoa: Perform UI operations only on the main thread
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 7/7] ui/cocoa: Perform UI operations only on the main thread Peter Maydell
2019-02-15 0:55 ` BALATON Zoltan
@ 2019-02-15 1:28 ` BALATON Zoltan
2019-02-15 10:04 ` Peter Maydell
2019-02-22 21:35 ` Roman Bolshakov
2 siblings, 1 reply; 38+ messages in thread
From: BALATON Zoltan @ 2019-02-15 1:28 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, patches, John Arbuckle, Roman Bolshakov,
Berkus Decker, Gerd Hoffmann, Ben Hekster
On Thu, 14 Feb 2019, Peter Maydell wrote:
> The OSX Mojave release is more picky about enforcing the Cocoa API
> restriction that only the main thread may perform UI calls. To
> accommodate this we need to restructure the Cocoa code:
> * the special OSX main() creates a second thread and uses
> that to call the vl.c qemu_main(); the original main
> thread goes into the OSX event loop
> * the refresh, switch and update callbacks asynchronously
> tell the main thread to do the necessary work
> * the refresh callback no longer does the "get events from the
> UI event queue and handle them" loop, since we now use
> the stock OSX event loop. Instead our NSApplication sendEvent
> method will either deal with them or pass them on to OSX
>
> All these things have to be changed in one commit, to avoid
> breaking bisection.
>
> Note that since we use dispatch_get_main_queue(), this bumps
> our minimum version requirement to OSX 10.10 Yosemite (released
> in 2014, unsupported by Apple since 2017).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> v2 changes: call handleEvent from sendEvent
> ---
> ui/cocoa.m | 191 +++++++++++++++++++++++++++++++----------------------
> 1 file changed, 113 insertions(+), 78 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 184fbd877d..201a294ed4 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -129,6 +129,9 @@
> NSTextField *pauseLabel;
> NSArray * supportedImageFileTypes;
>
> +QemuSemaphore display_init_sem;
> +QemuSemaphore app_started_sem;
> +
> // Utility functions to run specified code block with iothread lock held
> typedef void (^CodeBlock)(void);
> typedef bool (^BoolCodeBlock)(void);
> @@ -325,6 +328,7 @@ @interface QemuCocoaView : NSView
> NSWindow *fullScreenWindow;
> float cx,cy,cw,ch,cdx,cdy;
> CGDataProviderRef dataProviderRef;
> + pixman_image_t *pixman_image;
> BOOL modifiers_state[256];
> BOOL isMouseGrabbed;
> BOOL isFullscreen;
> @@ -535,13 +539,16 @@ - (void) switchSurface:(pixman_image_t *)image
> }
>
> // update screenBuffer
> - if (dataProviderRef)
> + if (dataProviderRef) {
> CGDataProviderRelease(dataProviderRef);
> + pixman_image_unref(pixman_image);
> + }
>
> //sync host window color space with guests
> screen.bitsPerPixel = PIXMAN_FORMAT_BPP(image_format);
> screen.bitsPerComponent = DIV_ROUND_UP(screen.bitsPerPixel, 8) * 2;
>
> + pixman_image = image;
> dataProviderRef = CGDataProviderCreateWithData(NULL, pixman_image_get_data(image), w * 4 * h, NULL);
>
> // update windows
> @@ -1013,7 +1020,6 @@ @interface QemuCocoaAppController : NSObject
> #endif
> {
> }
> -- (void)startEmulationWithArgc:(int)argc argv:(char**)argv;
> - (void)doToggleFullScreen:(id)sender;
> - (void)toggleFullScreen:(id)sender;
> - (void)showQEMUDoc:(id)sender;
> @@ -1101,8 +1107,8 @@ - (void) dealloc
> - (void)applicationDidFinishLaunching: (NSNotification *) note
> {
> COCOA_DEBUG("QemuCocoaAppController: applicationDidFinishLaunching\n");
> - // launch QEMU, with the global args
> - [self startEmulationWithArgc:gArgc argv:(char **)gArgv];
> + /* Tell cocoa_display_init to proceed */
> + qemu_sem_post(&app_started_sem);
> }
>
> - (void)applicationWillTerminate:(NSNotification *)aNotification
> @@ -1145,15 +1151,6 @@ - (void) applicationWillResignActive: (NSNotification *)aNotification
> [cocoaView raiseAllKeys];
> }
>
> -- (void)startEmulationWithArgc:(int)argc argv:(char**)argv
> -{
> - COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
> -
> - int status;
> - status = qemu_main(argc, argv, *_NSGetEnviron());
> - exit(status);
> -}
> -
> /* We abstract the method called by the Enter Fullscreen menu item
> * because Mac OS 10.7 and higher disables it. This is because of the
> * menu item's old selector's name toggleFullScreen:
> @@ -1485,7 +1482,9 @@ @implementation QemuApplication
> - (void)sendEvent:(NSEvent *)event
> {
> COCOA_DEBUG("QemuApplication: sendEvent\n");
> - [super sendEvent: event];
> + if (!cocoaView || ![cocoaView handleEvent:event]) {
> + [super sendEvent: event];
> + }
I think the menu problem may be because now all mouse events go to our
handleEvent method and it may swallow those that should operate the menus
somehow while previously [super sendEvent] took care of those events and
only left the remaining mouse events in queue? Maybe we should test in
handleEvent if the mouse event is within our window and forward everything
else to super? But I'm not sure and could be wrong, only guessing.
Regards,
BALATON Zoltan
> }
> @end
>
> @@ -1668,32 +1667,59 @@ static void addRemovableDevicesMenuItems(void)
> qapi_free_BlockInfoList(pointerToFree);
> }
>
> -int main (int argc, const char * argv[]) {
> +/*
> + * The startup process for the OSX/Cocoa UI is complicated, because
> + * OSX insists that the UI runs on the initial main thread, and so we
> + * need to start a second thread which runs the vl.c qemu_main():
> + *
> + * Initial thread: 2nd thread:
> + * in main():
> + * create qemu-main thread
> + * wait on display_init semaphore
> + * call qemu_main()
> + * ...
> + * in cocoa_display_init():
> + * post the display_init semaphore
> + * wait on app_started semaphore
> + * create application, menus, etc
> + * enter OSX run loop
> + * in applicationDidFinishLaunching:
> + * post app_started semaphore
> + * tell main thread to fullscreen if needed
> + * [...]
> + * run qemu main-loop
> + *
> + * We do this in two stages so that we don't do the creation of the
> + * GUI application menus and so on for command line options like --help
> + * where we want to just print text to stdout and exit immediately.
> + */
>
> +static void *call_qemu_main(void *opaque)
> +{
> + int status;
> +
> + COCOA_DEBUG("Second thread: calling qemu_main()\n");
> + status = qemu_main(gArgc, gArgv, *_NSGetEnviron());
> + COCOA_DEBUG("Second thread: qemu_main() returned, exiting\n");
> + exit(status);
> +}
> +
> +int main (int argc, const char * argv[]) {
> + QemuThread thread;
> +
> + COCOA_DEBUG("Entered main()\n");
> gArgc = argc;
> gArgv = (char **)argv;
> - int i;
>
> - /* In case we don't need to display a window, let's not do that */
> - for (i = 1; i < argc; i++) {
> - const char *opt = argv[i];
> + qemu_sem_init(&display_init_sem, 0);
> + qemu_sem_init(&app_started_sem, 0);
>
> - if (opt[0] == '-') {
> - /* Treat --foo the same as -foo. */
> - if (opt[1] == '-') {
> - opt++;
> - }
> - if (!strcmp(opt, "-h") || !strcmp(opt, "-help") ||
> - !strcmp(opt, "-vnc") ||
> - !strcmp(opt, "-nographic") ||
> - !strcmp(opt, "-version") ||
> - !strcmp(opt, "-curses") ||
> - !strcmp(opt, "-display") ||
> - !strcmp(opt, "-qtest")) {
> - return qemu_main(gArgc, gArgv, *_NSGetEnviron());
> - }
> - }
> - }
> + qemu_thread_create(&thread, "qemu_main", call_qemu_main,
> + NULL, QEMU_THREAD_DETACHED);
> +
> + COCOA_DEBUG("Main thread: waiting for display_init_sem\n");
> + qemu_sem_wait(&display_init_sem);
> + COCOA_DEBUG("Main thread: initializing app\n");
>
> NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
>
> @@ -1706,12 +1732,24 @@ int main (int argc, const char * argv[]) {
>
> create_initial_menus();
>
> + /*
> + * Create the menu entries which depend on QEMU state (for consoles
> + * and removeable devices). These make calls back into QEMU functions,
> + * which is OK because at this point we know that the second thread
> + * holds the iothread lock and is synchronously waiting for us to
> + * finish.
> + */
> + add_console_menu_entries();
> + addRemovableDevicesMenuItems();
> +
> // Create an Application controller
> QemuCocoaAppController *appController = [[QemuCocoaAppController alloc] init];
> [NSApp setDelegate:appController];
>
> // Start the main event loop
> + COCOA_DEBUG("Main thread: entering OSX run loop\n");
> [NSApp run];
> + COCOA_DEBUG("Main thread: left OSX run loop, exiting\n");
>
> [appController release];
> [pool release];
> @@ -1729,17 +1767,19 @@ static void cocoa_update(DisplayChangeListener *dcl,
>
> COCOA_DEBUG("qemu_cocoa: cocoa_update\n");
>
> - NSRect rect;
> - if ([cocoaView cdx] == 1.0) {
> - rect = NSMakeRect(x, [cocoaView gscreen].height - y - h, w, h);
> - } else {
> - rect = NSMakeRect(
> - x * [cocoaView cdx],
> - ([cocoaView gscreen].height - y - h) * [cocoaView cdy],
> - w * [cocoaView cdx],
> - h * [cocoaView cdy]);
> - }
> - [cocoaView setNeedsDisplayInRect:rect];
> + dispatch_async(dispatch_get_main_queue(), ^{
> + NSRect rect;
> + if ([cocoaView cdx] == 1.0) {
> + rect = NSMakeRect(x, [cocoaView gscreen].height - y - h, w, h);
> + } else {
> + rect = NSMakeRect(
> + x * [cocoaView cdx],
> + ([cocoaView gscreen].height - y - h) * [cocoaView cdy],
> + w * [cocoaView cdx],
> + h * [cocoaView cdy]);
> + }
> + [cocoaView setNeedsDisplayInRect:rect];
> + });
>
> [pool release];
> }
> @@ -1748,9 +1788,19 @@ static void cocoa_switch(DisplayChangeListener *dcl,
> DisplaySurface *surface)
> {
> NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
> + pixman_image_t *image = surface->image;
>
> COCOA_DEBUG("qemu_cocoa: cocoa_switch\n");
> - [cocoaView switchSurface:surface->image];
> +
> + // The DisplaySurface will be freed as soon as this callback returns.
> + // We take a reference to the underlying pixman image here so it does
> + // not disappear from under our feet; the switchSurface method will
> + // deref the old image when it is done with it.
> + pixman_image_ref(image);
> +
> + dispatch_async(dispatch_get_main_queue(), ^{
> + [cocoaView switchSurface:image];
> + });
> [pool release];
> }
>
> @@ -1762,26 +1812,15 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
> graphic_hw_update(NULL);
>
> if (qemu_input_is_absolute()) {
> - if (![cocoaView isAbsoluteEnabled]) {
> - if ([cocoaView isMouseGrabbed]) {
> - [cocoaView ungrabMouse];
> + dispatch_async(dispatch_get_main_queue(), ^{
> + if (![cocoaView isAbsoluteEnabled]) {
> + if ([cocoaView isMouseGrabbed]) {
> + [cocoaView ungrabMouse];
> + }
> }
> - }
> - [cocoaView setAbsoluteEnabled:YES];
> + [cocoaView setAbsoluteEnabled:YES];
> + });
> }
> -
> - NSDate *distantPast;
> - NSEvent *event;
> - distantPast = [NSDate distantPast];
> - do {
> - event = [NSApp nextEventMatchingMask:NSEventMaskAny untilDate:distantPast
> - inMode: NSDefaultRunLoopMode dequeue:YES];
> - if (event != nil) {
> - if (![cocoaView handleEvent:event]) {
> - [NSApp sendEvent:event];
> - }
> - }
> - } while(event != nil);
> [pool release];
> }
>
> @@ -1802,10 +1841,17 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
> {
> COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n");
>
> + /* Tell main thread to go ahead and create the app and enter the run loop */
> + qemu_sem_post(&display_init_sem);
> + qemu_sem_wait(&app_started_sem);
> + COCOA_DEBUG("cocoa_display_init: app start completed\n");
> +
> /* if fullscreen mode is to be used */
> if (opts->has_full_screen && opts->full_screen) {
> - [NSApp activateIgnoringOtherApps: YES];
> - [(QemuCocoaAppController *)[[NSApplication sharedApplication] delegate] toggleFullScreen: nil];
> + dispatch_async(dispatch_get_main_queue(), ^{
> + [NSApp activateIgnoringOtherApps: YES];
> + [(QemuCocoaAppController *)[[NSApplication sharedApplication] delegate] toggleFullScreen: nil];
> + });
> }
>
> dcl = g_malloc0(sizeof(DisplayChangeListener));
> @@ -1816,17 +1862,6 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
>
> // register cleanup function
> atexit(cocoa_cleanup);
> -
> - /* At this point QEMU has created all the consoles, so we can add View
> - * menu entries for them.
> - */
> - add_console_menu_entries();
> -
> - /* Give all removable devices a menu item.
> - * Has to be called after QEMU has started to
> - * find out what removable devices it has.
> - */
> - addRemovableDevicesMenuItems();
> }
>
> static QemuDisplay qemu_display_cocoa = {
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 7/7] ui/cocoa: Perform UI operations only on the main thread
2019-02-15 1:28 ` BALATON Zoltan
@ 2019-02-15 10:04 ` Peter Maydell
2019-02-16 1:15 ` BALATON Zoltan
0 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2019-02-15 10:04 UTC (permalink / raw)
To: BALATON Zoltan
Cc: QEMU Developers, patches, John Arbuckle, Roman Bolshakov,
Berkus Decker, Gerd Hoffmann, Ben Hekster
On Fri, 15 Feb 2019 at 01:28, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Thu, 14 Feb 2019, Peter Maydell wrote:
> > - (void)sendEvent:(NSEvent *)event
> > {
> > COCOA_DEBUG("QemuApplication: sendEvent\n");
> > - [super sendEvent: event];
> > + if (!cocoaView || ![cocoaView handleEvent:event]) {
> > + [super sendEvent: event];
> > + }
>
> I think the menu problem may be because now all mouse events go to our
> handleEvent method and it may swallow those that should operate the menus
> somehow while previously [super sendEvent] took care of those events and
> only left the remaining mouse events in queue? Maybe we should test in
> handleEvent if the mouse event is within our window and forward everything
> else to super? But I'm not sure and could be wrong, only guessing.
Menus definitely did work for me (both mouse interaction and
keyboard shortcuts), so I'm not sure why you're seeing something
different.
My understanding is that the new code should eat exactly the
same set of events that the old code did:
* in the old code, only our custom "pull events off the queue"
code took events from OSX, and it passed them to handleEvent;
only events handleEvent called [NSApp sendEvent] on would
ever get to the OSX menu/etc handling
* in the new code, OSX's run loop takes events off the queue:
they go to our NSApplication sendEvent method, which passes
them to handleEvent. Only events that handleEvent returns false
for will ever get to the OSX menu/etc handling
and (absent bugs) handleEvent should now return false for exactly
the set of events that it previously called sendEvent on.
I know menu handling in OSX is a bit odd because the menu
drag stuff uses a 'private event loop' to handle events
while the menu is down. Maybe something is interacting
badly with that.
thanks
-- PMM
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 7/7] ui/cocoa: Perform UI operations only on the main thread
2019-02-15 10:04 ` Peter Maydell
@ 2019-02-16 1:15 ` BALATON Zoltan
0 siblings, 0 replies; 38+ messages in thread
From: BALATON Zoltan @ 2019-02-16 1:15 UTC (permalink / raw)
To: Peter Maydell
Cc: QEMU Developers, patches, John Arbuckle, Roman Bolshakov,
Berkus Decker, Gerd Hoffmann, Ben Hekster
On Fri, 15 Feb 2019, Peter Maydell wrote:
> On Fri, 15 Feb 2019 at 01:28, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>
>> On Thu, 14 Feb 2019, Peter Maydell wrote:
>>> - (void)sendEvent:(NSEvent *)event
>>> {
>>> COCOA_DEBUG("QemuApplication: sendEvent\n");
>>> - [super sendEvent: event];
>>> + if (!cocoaView || ![cocoaView handleEvent:event]) {
>>> + [super sendEvent: event];
>>> + }
>>
>> I think the menu problem may be because now all mouse events go to our
>> handleEvent method and it may swallow those that should operate the menus
>> somehow while previously [super sendEvent] took care of those events and
>> only left the remaining mouse events in queue? Maybe we should test in
>> handleEvent if the mouse event is within our window and forward everything
>> else to super? But I'm not sure and could be wrong, only guessing.
>
> Menus definitely did work for me (both mouse interaction and
> keyboard shortcuts), so I'm not sure why you're seeing something
> different.
>
> My understanding is that the new code should eat exactly the
> same set of events that the old code did:
> * in the old code, only our custom "pull events off the queue"
> code took events from OSX, and it passed them to handleEvent;
> only events handleEvent called [NSApp sendEvent] on would
> ever get to the OSX menu/etc handling
> * in the new code, OSX's run loop takes events off the queue:
> they go to our NSApplication sendEvent method, which passes
> them to handleEvent. Only events that handleEvent returns false
> for will ever get to the OSX menu/etc handling
> and (absent bugs) handleEvent should now return false for exactly
> the set of events that it previously called sendEvent on.
>
> I know menu handling in OSX is a bit odd because the menu
> drag stuff uses a 'private event loop' to handle events
> while the menu is down. Maybe something is interacting
> badly with that.
Sorry, looks like my proposed patch [ui/cocoa: Make sure app is not
starting in the background] is the one which breaks this. After some
recompiling menus work here as well with your series so forget about this
patch and sorry for the false alarm. (The habit of macOS to open apps
started from Terminal in the background is quite annoying but looks like
it's normal and can't be fixed that easily.)
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/7] ui/cocoa: Ensure we have the iothread lock when calling into QEMU
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 1/7] ui/cocoa: Ensure we have the iothread lock when calling into QEMU Peter Maydell
@ 2019-02-22 15:19 ` Roman Bolshakov
2019-02-22 15:41 ` Peter Maydell
0 siblings, 1 reply; 38+ messages in thread
From: Roman Bolshakov @ 2019-02-22 15:19 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, patches, John Arbuckle, Berkus Decker, Gerd Hoffmann,
Ben Hekster, BALATON Zoltan
On Thu, Feb 14, 2019 at 10:28:10AM +0000, Peter Maydell wrote:
> The Cocoa UI should run on the main thread; this is enforced
> in OSX Mojave. In order to be able to run on the main thread,
> we need to make sure we hold the iothread lock whenever we
> call into various QEMU UI midlayer functions.
>
Hi Peter,
I wonder if qmp_stop and qmp_cont calls should be made in locked context
within pauseQEMU and resumeQEMU methods, respectively?
I also think it's better to clarify that the reason of the commit is not
Mojave enforcing usage of event loop in main thread but an improvement
of event processing in Cocoa UI, because Cocoa UI works on Mojave.
As of now qemu main loop and Cocoa events processing is done in the same
thread. And you can see from below that invocation of the qemu_main blocks
Cocoa event loop [NSApp run] in applicationDidFinishLaunching as qemu_main
doesn't quit.
Here's a backtrace that shows lockup of the primary Cocoa event loop:
(lldb) b cocoa_refresh
Breakpoint 1: where = qemu-system-x86_64`cocoa_refresh + 12 at cocoa.m:1634, address = 0x0000000109a03a0c
(lldb) c
Process 46179 resuming
Process 46179 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
frame #0: 0x0000000109a03a0c qemu-system-x86_64`cocoa_refresh(dcl=0x00007f914eccefe0) at cocoa.m:1634
1631
1632 static void cocoa_refresh(DisplayChangeListener *dcl)
1633 {
-> 1634 NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
1635
1636 COCOA_DEBUG("qemu_cocoa: cocoa_refresh\n");
1637 graphic_hw_update(NULL);
Target 0: (qemu-system-x86_64) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
* frame #0: 0x0000000109a03a0c qemu-system-x86_64`cocoa_refresh(dcl=0x00007f914eccefe0) at cocoa.m:1634
frame #1: 0x00000001099f6888 qemu-system-x86_64`dpy_refresh(s=0x00007f915074cbd0) at console.c:1622
frame #2: 0x00000001099f66dd qemu-system-x86_64`gui_update(opaque=0x00007f915074cbd0) at console.c:205
frame #3: 0x0000000109ba6009 qemu-system-x86_64`timerlist_run_timers(timer_list=0x00007f915070c390) at qemu-timer.c:574
frame #4: 0x0000000109ba60e0 qemu-system-x86_64`qemu_clock_run_timers(type=QEMU_CLOCK_REALTIME) at qemu-timer.c:588
frame #5: 0x0000000109ba662a qemu-system-x86_64`qemu_clock_run_all_timers at qemu-timer.c:708
frame #6: 0x0000000109ba6b8f qemu-system-x86_64`main_loop_wait(nonblocking=0) at main-loop.c:521
frame #7: 0x0000000109760464 qemu-system-x86_64`main_loop at vl.c:1923
frame #8: 0x000000010975b624 qemu-system-x86_64`qemu_main(argc=5, argv=0x00007ffee66a0688, envp=0x00007ffee66a06b8) at vl.c:4578
frame #9: 0x00000001099ffef9 qemu-system-x86_64`-[QemuCocoaAppController startEmulationWithArgc:argv:](self=0x00007f914ee05e10, _cmd="startEmulationWithArgc:argv:", argc=5, argv=0x00007ffee66a0688) at cocoa.m:1135
frame #10: 0x00000001099ffd97 qemu-system-x86_64`-[QemuCocoaAppController applicationDidFinishLaunching:](self=0x00007f914ee05e10, _cmd="applicationDidFinishLaunching:", note=@"NSApplicationDidFinishLaunchingNotification") at cocoa.m:1087
frame #11: 0x00007fff46bd5632 CoreFoundation`__CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ + 12
frame #12: 0x00007fff46bd55ac CoreFoundation`___CFXRegistrationPost_block_invoke + 63
frame #13: 0x00007fff46bd54cd CoreFoundation`_CFXRegistrationPost + 398
frame #14: 0x00007fff46bdd929 CoreFoundation`___CFXNotificationPost_block_invoke + 87
frame #15: 0x00007fff46b450ca CoreFoundation`-[_CFXNotificationRegistrar find:object:observer:enumerator:] + 1633
frame #16: 0x00007fff46b4448d CoreFoundation`_CFXNotificationPost + 742
frame #17: 0x00007fff48ecca7b Foundation`-[NSNotificationCenter postNotificationName:object:userInfo:] + 66
frame #18: 0x00007fff440c964a AppKit`-[NSApplication _postDidFinishNotification] + 313
frame #19: 0x00007fff440c8f6e AppKit`-[NSApplication _sendFinishLaunchingNotification] + 209
frame #20: 0x00007fff440c68c8 AppKit`-[NSApplication(NSAppleEventHandling) _handleAEOpenEvent:] + 552
frame #21: 0x00007fff440c6517 AppKit`-[NSApplication(NSAppleEventHandling) _handleCoreEvent:withReplyEvent:] + 690
frame #22: 0x00007fff48f17144 Foundation`-[NSAppleEventManager dispatchRawAppleEvent:withRawReply:handlerRefCon:] + 287
frame #23: 0x00007fff48f16fc0 Foundation`_NSAppleEventManagerGenericHandler + 102
frame #24: 0x00007fff47dfcb93 AE`aeDispatchAppleEvent(AEDesc const*, AEDesc*, unsigned int, unsigned char*) + 1855
frame #25: 0x00007fff47dfc3fd AE`dispatchEventAndSendReply(AEDesc const*, AEDesc*) + 41
frame #26: 0x00007fff47dfc2d5 AE`aeProcessAppleEvent + 439
frame #27: 0x00007fff45e1110e HIToolbox`AEProcessAppleEvent + 55
frame #28: 0x00007fff440c2644 AppKit`_DPSNextEvent + 1734
frame #29: 0x00007fff440c1102 AppKit`-[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1362
frame #30: 0x00007fff440bb165 AppKit`-[NSApplication run] + 699
frame #31: 0x0000000109a03041 qemu-system-x86_64`main(argc=5, argv=0x00007ffee66a0688) at cocoa.m:1589
frame #32: 0x00007fff73dc2ed9 libdyld.dylib`start + 1
frame #33: 0x00007fff73dc2ed9 libdyld.dylib`start + 1
Each millisecond cocoa_refresh gets called by display handler. The function
extracts all available events from the cocoa event queue and dispatches them to
handleEvent:
event = [NSApp nextEventMatchingMask:NSEventMaskAny untilDate:distantPast
inMode: NSDefaultRunLoopMode dequeue:YES];
if (event != nil) {
[cocoaView handleEvent:event];
}
handleEvent enqueues keyboard and mouse events into ps2 queue and the
other events are enqueued to Cocoa event queue. I'm curious what happens
to the events that are enqueued with:
[NSApp sendEvent:event];
Perhaps the events are enqueued in handleEvent over and over and never
get out of the event queue. And there could be a chance of the event
queue overrun.
--
Thanks,
Roman
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/7] ui/cocoa: Ensure we have the iothread lock when calling into QEMU
2019-02-22 15:19 ` Roman Bolshakov
@ 2019-02-22 15:41 ` Peter Maydell
2019-02-22 21:48 ` Roman Bolshakov
0 siblings, 1 reply; 38+ messages in thread
From: Peter Maydell @ 2019-02-22 15:41 UTC (permalink / raw)
To: Roman Bolshakov
Cc: QEMU Developers, patches, John Arbuckle, Berkus Decker,
Gerd Hoffmann, Ben Hekster, BALATON Zoltan
On Fri, 22 Feb 2019 at 15:19, Roman Bolshakov <r.bolshakov@yadro.com> wrote:
>
> On Thu, Feb 14, 2019 at 10:28:10AM +0000, Peter Maydell wrote:
> > The Cocoa UI should run on the main thread; this is enforced
> > in OSX Mojave. In order to be able to run on the main thread,
> > we need to make sure we hold the iothread lock whenever we
> > call into various QEMU UI midlayer functions.
> >
>
> Hi Peter,
>
> I wonder if qmp_stop and qmp_cont calls should be made in locked context
> within pauseQEMU and resumeQEMU methods, respectively?
Yes, you're right, I missed these, but they should also be
called within a with_iothread_lock wrapper.
> I also think it's better to clarify that the reason of the commit is not
> Mojave enforcing usage of event loop in main thread but an improvement
> of event processing in Cocoa UI, because Cocoa UI works on Mojave.
Hmm? The point of this patchset is exactly that Mojave enforces
that things go on the main thread, where previous OSX versions
did not, and so in some situations QEMU will crash on Mojave
where it did not on older versions. So I'm not sure what you're
suggesting should be clarified here.
> As of now qemu main loop and Cocoa events processing is done in the same
> thread. And you can see from below that invocation of the qemu_main blocks
> Cocoa event loop [NSApp run] in applicationDidFinishLaunching as qemu_main
> doesn't quit.
Yes, indeed. This is how we've traditionally done the Cocoa
event handling. It's worked OK for everything up to Mojave...
> Each millisecond cocoa_refresh gets called by display handler. The function
> extracts all available events from the cocoa event queue and dispatches them to
> handleEvent:
>
> event = [NSApp nextEventMatchingMask:NSEventMaskAny untilDate:distantPast
> inMode: NSDefaultRunLoopMode dequeue:YES];
> if (event != nil) {
> [cocoaView handleEvent:event];
> }
>
> handleEvent enqueues keyboard and mouse events into ps2 queue and the
> other events are enqueued to Cocoa event queue. I'm curious what happens
> to the events that are enqueued with:
> [NSApp sendEvent:event];
As I understand it, this sendEvent method doesn't queue anything.
It just hands the event to the usual OSX event handling chain,
which will either use it (for example, if it's a menu-item
key accelerator event) or drop it on the floor. The logic is
basically "QEMU-specific code gets first chance to decide whether
to consume the event; the default (and a few other odd cases)
if it does not is that we let the OSX framework code do whatever
it wants to do with it".
thanks
-- PMM
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/7] ui/cocoa: Use the pixman image directly in switchSurface
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 2/7] ui/cocoa: Use the pixman image directly in switchSurface Peter Maydell
2019-02-14 17:19 ` BALATON Zoltan
@ 2019-02-22 17:27 ` Roman Bolshakov
1 sibling, 0 replies; 38+ messages in thread
From: Roman Bolshakov @ 2019-02-22 17:27 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, patches, John Arbuckle, Berkus Decker, Gerd Hoffmann,
Ben Hekster, BALATON Zoltan
On Thu, Feb 14, 2019 at 10:28:11AM +0000, Peter Maydell wrote:
> Currently the switchSurface method takes a DisplaySurface. We want
> to change our DisplayChangeListener's dpy_gfx_switch callback
> to do this work asynchronously on a different thread. The caller
> of the switch callback will free the old DisplaySurface
> immediately the callback returns, so to ensure that the
> other thread doesn't access freed data we need to switch
> to using the underlying pixman image instead. The pixman
> image is reference counted, so we will be able to take
> a reference to it to avoid it vanishing too early.
>
> In this commit we only change the switchSurface method
> to take a pixman image, and keep the flow of control
> synchronous for now.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> ui/cocoa.m | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>
Thanks,
Roman
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/7] ui/cocoa: Factor out initial menu creation
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 3/7] ui/cocoa: Factor out initial menu creation Peter Maydell
2019-02-14 17:11 ` BALATON Zoltan
@ 2019-02-22 17:47 ` Roman Bolshakov
1 sibling, 0 replies; 38+ messages in thread
From: Roman Bolshakov @ 2019-02-22 17:47 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, patches, John Arbuckle, Berkus Decker, Gerd Hoffmann,
Ben Hekster, BALATON Zoltan
On Thu, Feb 14, 2019 at 10:28:12AM +0000, Peter Maydell wrote:
> Factor out the long code sequence in main() which creates
> the initial set of menus. This will make later patches
> which move initialization code around a bit clearer.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> ui/cocoa.m | 78 ++++++++++++++++++++++++++++--------------------------
> 1 file changed, 41 insertions(+), 37 deletions(-)
>
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>
--
Roman
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/7] ui/cocoa: Move console/device menu creation code up in file
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 4/7] ui/cocoa: Move console/device menu creation code up in file Peter Maydell
2019-02-15 0:34 ` BALATON Zoltan
@ 2019-02-22 18:10 ` Roman Bolshakov
1 sibling, 0 replies; 38+ messages in thread
From: Roman Bolshakov @ 2019-02-22 18:10 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, patches, John Arbuckle, Berkus Decker, Gerd Hoffmann,
Ben Hekster, BALATON Zoltan
On Thu, Feb 14, 2019 at 10:28:13AM +0000, Peter Maydell wrote:
> Move the console/device menu creation code functions
> further up in the source file, next to the code which
> creates the initial menus. We're going to want to
> change the location we call these functions from in
> the next patch.
>
> This commit is a pure code move with no other changes.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> ui/cocoa.m | 184 ++++++++++++++++++++++++++---------------------------
> 1 file changed, 92 insertions(+), 92 deletions(-)
>
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>
Thanks,
Roman
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/7] ui/cocoa: Don't call NSApp sendEvent directly from handleEvent
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 5/7] ui/cocoa: Don't call NSApp sendEvent directly from handleEvent Peter Maydell
2019-02-14 17:04 ` BALATON Zoltan
@ 2019-02-22 19:20 ` Roman Bolshakov
1 sibling, 0 replies; 38+ messages in thread
From: Roman Bolshakov @ 2019-02-22 19:20 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, patches, John Arbuckle, Berkus Decker, Gerd Hoffmann,
Ben Hekster, BALATON Zoltan
On Thu, Feb 14, 2019 at 10:28:14AM +0000, Peter Maydell wrote:
> Currently the handleEvent method will directly call the NSApp
> sendEvent method for any events that we want to let OSX deal
> with. When we rearrange the event handling code, the way that
> we say "let OSX have this event" is going to change. Prepare
> for that by refactoring so that handleEvent returns a flag
> indicating whether it consumed the event.
>
> Suggested-by: BALATON Zoltan <balaton@eik.bme.hu>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> New patch in v2
> ---
> ui/cocoa.m | 49 ++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 34 insertions(+), 15 deletions(-)
>
> @@ -1749,7 +1766,9 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
> event = [NSApp nextEventMatchingMask:NSEventMaskAny untilDate:distantPast
> inMode: NSDefaultRunLoopMode dequeue:YES];
> if (event != nil) {
> - [cocoaView handleEvent:event];
> + if (![cocoaView handleEvent:event]) {
> + [NSApp sendEvent:event];
> + }
> }
> } while(event != nil);
> [pool release];
> --
> 2.17.2 (Apple Git-113)
>
I like the patch. It makes clear that cocoa_refresh performs the work
of [NSApp run].
Besides the trailing whitespace issue,
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>
Thanks,
Roman
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/7] ui/cocoa: Subclass NSApplication so we can implement sendEvent
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 6/7] ui/cocoa: Subclass NSApplication so we can implement sendEvent Peter Maydell
2019-02-14 17:21 ` BALATON Zoltan
@ 2019-02-22 20:18 ` Roman Bolshakov
1 sibling, 0 replies; 38+ messages in thread
From: Roman Bolshakov @ 2019-02-22 20:18 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, patches, John Arbuckle, Berkus Decker, Gerd Hoffmann,
Ben Hekster, BALATON Zoltan
On Thu, Feb 14, 2019 at 10:28:15AM +0000, Peter Maydell wrote:
> When we switch away from our custom event handling, we still want to
> be able to have first go at any events our application receives,
> because in full-screen mode we want to send key events to the guest,
> even if they would be menu item activation events. There are several
> ways we could do that, but one simple approach is to subclass
> NSApplication so we can implement a custom sendEvent method.
> Do that, but for the moment have our sendEvent just invoke the
> superclass method.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> New patch in v2
> ---
> ui/cocoa.m | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>
Thanks,
Roman
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 7/7] ui/cocoa: Perform UI operations only on the main thread
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 7/7] ui/cocoa: Perform UI operations only on the main thread Peter Maydell
2019-02-15 0:55 ` BALATON Zoltan
2019-02-15 1:28 ` BALATON Zoltan
@ 2019-02-22 21:35 ` Roman Bolshakov
2 siblings, 0 replies; 38+ messages in thread
From: Roman Bolshakov @ 2019-02-22 21:35 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, patches, John Arbuckle, Berkus Decker, Gerd Hoffmann,
Ben Hekster, BALATON Zoltan
On Thu, Feb 14, 2019 at 10:28:16AM +0000, Peter Maydell wrote:
> The OSX Mojave release is more picky about enforcing the Cocoa API
> restriction that only the main thread may perform UI calls. To
> accommodate this we need to restructure the Cocoa code:
> * the special OSX main() creates a second thread and uses
> that to call the vl.c qemu_main(); the original main
> thread goes into the OSX event loop
> * the refresh, switch and update callbacks asynchronously
> tell the main thread to do the necessary work
> * the refresh callback no longer does the "get events from the
> UI event queue and handle them" loop, since we now use
> the stock OSX event loop. Instead our NSApplication sendEvent
> method will either deal with them or pass them on to OSX
>
> All these things have to be changed in one commit, to avoid
> breaking bisection.
>
> Note that since we use dispatch_get_main_queue(), this bumps
> our minimum version requirement to OSX 10.10 Yosemite (released
> in 2014, unsupported by Apple since 2017).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> v2 changes: call handleEvent from sendEvent
> ---
> ui/cocoa.m | 191 +++++++++++++++++++++++++++++++----------------------
> 1 file changed, 113 insertions(+), 78 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 184fbd877d..201a294ed4 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -129,6 +129,9 @@
> NSTextField *pauseLabel;
> NSArray * supportedImageFileTypes;
>
> +QemuSemaphore display_init_sem;
> +QemuSemaphore app_started_sem;
> +
They might have file scope.
> @@ -535,13 +539,16 @@ - (void) switchSurface:(pixman_image_t *)image
> }
>
> // update screenBuffer
> - if (dataProviderRef)
> + if (dataProviderRef) {
> CGDataProviderRelease(dataProviderRef);
> + pixman_image_unref(pixman_image);
> + }
pixman_image also needs to be unreferenced in dealloc.
> @@ -1485,7 +1482,9 @@ @implementation QemuApplication
> - (void)sendEvent:(NSEvent *)event
> {
> COCOA_DEBUG("QemuApplication: sendEvent\n");
> - [super sendEvent: event];
> + if (!cocoaView || ![cocoaView handleEvent:event]) {
> + [super sendEvent: event];
> + }
> }
> @end
>
if (!cocoaView || ![cocoaView handleEvent:event]) {
can be written as
if (![cocoaView handleEvent:event]) {
It's valid to send a message to nil and it will return 0/false/NO.
Thank you for working on the patch series. It definitely improves UI event
handling.
Besides the pixman_image leak,
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>
Roman
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/7] ui/cocoa: Ensure we have the iothread lock when calling into QEMU
2019-02-22 15:41 ` Peter Maydell
@ 2019-02-22 21:48 ` Roman Bolshakov
2019-02-23 13:11 ` Peter Maydell
0 siblings, 1 reply; 38+ messages in thread
From: Roman Bolshakov @ 2019-02-22 21:48 UTC (permalink / raw)
To: Peter Maydell
Cc: QEMU Developers, patches, John Arbuckle, Berkus Decker,
Gerd Hoffmann, Ben Hekster, BALATON Zoltan
On Fri, Feb 22, 2019 at 03:41:05PM +0000, Peter Maydell wrote:
> On Fri, 22 Feb 2019 at 15:19, Roman Bolshakov <r.bolshakov@yadro.com> wrote:
> >
> > On Thu, Feb 14, 2019 at 10:28:10AM +0000, Peter Maydell wrote:
> > > The Cocoa UI should run on the main thread; this is enforced
> > > in OSX Mojave. In order to be able to run on the main thread,
> > > we need to make sure we hold the iothread lock whenever we
> > > call into various QEMU UI midlayer functions.
> > >
> >
> > I also think it's better to clarify that the reason of the commit is not
> > Mojave enforcing usage of event loop in main thread but an improvement
> > of event processing in Cocoa UI, because Cocoa UI works on Mojave.
>
> Hmm? The point of this patchset is exactly that Mojave enforces
> that things go on the main thread, where previous OSX versions
> did not, and so in some situations QEMU will crash on Mojave
> where it did not on older versions. So I'm not sure what you're
> suggesting should be clarified here.
>
I'm not exactly sure there's an issue with QEMU on Mojave. But I lean
towards the opinion because I haven't seen it :)
> > As of now qemu main loop and Cocoa events processing is done in the same
> > thread. And you can see from below that invocation of the qemu_main blocks
> > Cocoa event loop [NSApp run] in applicationDidFinishLaunching as qemu_main
> > doesn't quit.
>
> Yes, indeed. This is how we've traditionally done the Cocoa
> event handling. It's worked OK for everything up to Mojave...
>
Oh, you also mention that in the cover letter. I somehow skipped it.
> > Each millisecond cocoa_refresh gets called by display handler. The function
> > extracts all available events from the cocoa event queue and dispatches them to
> > handleEvent:
> >
> > event = [NSApp nextEventMatchingMask:NSEventMaskAny untilDate:distantPast
> > inMode: NSDefaultRunLoopMode dequeue:YES];
> > if (event != nil) {
> > [cocoaView handleEvent:event];
> > }
> >
> > handleEvent enqueues keyboard and mouse events into ps2 queue and the
> > other events are enqueued to Cocoa event queue. I'm curious what happens
> > to the events that are enqueued with:
> > [NSApp sendEvent:event];
>
> As I understand it, this sendEvent method doesn't queue anything.
> It just hands the event to the usual OSX event handling chain,
> which will either use it (for example, if it's a menu-item
> key accelerator event) or drop it on the floor. The logic is
> basically "QEMU-specific code gets first chance to decide whether
> to consume the event; the default (and a few other odd cases)
> if it does not is that we let the OSX framework code do whatever
> it wants to do with it".
>
Thanks, I see! I should have consulted apple reference before raising
the issue. FWIW there's a method to post an event to the event queue but
sendEvent is used to dispatch an event within an app and the event that
is already taken from the queue.
Besides qmp_stop and qmp_cont not being called under iothread lock,
Reviewed-by: Roman Bolshakov <r.bolshakv@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>
Thanks,
Roman
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/7] ui/cocoa: Ensure we have the iothread lock when calling into QEMU
2019-02-22 21:48 ` Roman Bolshakov
@ 2019-02-23 13:11 ` Peter Maydell
0 siblings, 0 replies; 38+ messages in thread
From: Peter Maydell @ 2019-02-23 13:11 UTC (permalink / raw)
To: Roman Bolshakov
Cc: QEMU Developers, patches, John Arbuckle, Berkus Decker,
Gerd Hoffmann, Ben Hekster, BALATON Zoltan
On Fri, 22 Feb 2019 at 21:48, Roman Bolshakov <r.bolshakov@yadro.com> wrote:
>
> On Fri, Feb 22, 2019 at 03:41:05PM +0000, Peter Maydell wrote:
> > On Fri, 22 Feb 2019 at 15:19, Roman Bolshakov <r.bolshakov@yadro.com> wrote:
> > >
> > > On Thu, Feb 14, 2019 at 10:28:10AM +0000, Peter Maydell wrote:
> > > > The Cocoa UI should run on the main thread; this is enforced
> > > > in OSX Mojave. In order to be able to run on the main thread,
> > > > we need to make sure we hold the iothread lock whenever we
> > > > call into various QEMU UI midlayer functions.
> > > >
> > >
> > > I also think it's better to clarify that the reason of the commit is not
> > > Mojave enforcing usage of event loop in main thread but an improvement
> > > of event processing in Cocoa UI, because Cocoa UI works on Mojave.
> >
> > Hmm? The point of this patchset is exactly that Mojave enforces
> > that things go on the main thread, where previous OSX versions
> > did not, and so in some situations QEMU will crash on Mojave
> > where it did not on older versions. So I'm not sure what you're
> > suggesting should be clarified here.
> >
>
> I'm not exactly sure there's an issue with QEMU on Mojave. But I lean
> towards the opinion because I haven't seen it :)
It only happens for some guest workloads. The "usual" case
is that the cocoa_refresh callback is called from the QEMU
main loop, which happens to be on the OSX main thread, which
means OSX is still happy. But in some cases cocoa_refresh can
be called from a guest vCPU thread -- I think we've seen this
when a guest initiates a screen resolution change: the call
from the guest vCPU thread goes into the model of the graphics
device, which makes a call into the UI midlayer to say
"resolution changed", which immediately triggers a
refresh callback to the UI frontend layer from that thread.
In Mojave this causes OSX to terminate QEMU. I think in
older OSX versions it would probably be a race condition,
so it's technically a bug but not one that usually has
any visible bad effects; it's only surfaced as a problem
now that Mojave actively checks for this condition and
kills the process.
thanks
-- PMM
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
2019-02-14 10:28 [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop Peter Maydell
` (12 preceding siblings ...)
2019-02-15 1:01 ` no-reply
@ 2019-02-27 15:24 ` no-reply
2019-02-27 15:29 ` no-reply
2019-02-27 17:41 ` no-reply
15 siblings, 0 replies; 38+ messages in thread
From: no-reply @ 2019-02-27 15:24 UTC (permalink / raw)
To: peter.maydell
Cc: fam, qemu-devel, patches, programmingkidx, r.bolshakov, kraxel,
ben.hekster, berkus
Patchew URL: https://patchew.org/QEMU/20190214102816.3393-1-peter.maydell@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Message-id: 20190214102816.3393-1-peter.maydell@linaro.org
Subject: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
Type: series
=== 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
t [tag update] patchew/20190215174548.2630-1-yury-kotov@yandex-team.ru -> patchew/20190215174548.2630-1-yury-kotov@yandex-team.ru
Switched to a new branch 'test'
93a3cf9c42 ui/cocoa: Perform UI operations only on the main thread
7eb130375a ui/cocoa: Subclass NSApplication so we can implement sendEvent
556e313bf5 ui/cocoa: Don't call NSApp sendEvent directly from handleEvent
5e20ccf63c ui/cocoa: Move console/device menu creation code up in file
017353b840 ui/cocoa: Factor out initial menu creation
455ce903fa ui/cocoa: Use the pixman image directly in switchSurface
02b675ede0 ui/cocoa: Ensure we have the iothread lock when calling into QEMU
=== OUTPUT BEGIN ===
1/7 Checking commit 02b675ede0a2 (ui/cocoa: Ensure we have the iothread lock when calling into QEMU)
2/7 Checking commit 455ce903fa09 (ui/cocoa: Use the pixman image directly in switchSurface)
3/7 Checking commit 017353b840b4 (ui/cocoa: Factor out initial menu creation)
4/7 Checking commit 5e20ccf63c62 (ui/cocoa: Move console/device menu creation code up in file)
5/7 Checking commit 556e313bf5e1 (ui/cocoa: Don't call NSApp sendEvent directly from handleEvent)
ERROR: trailing whitespace
#44: FILE: ui/cocoa.m:152:
+ $
total: 1 errors, 0 warnings, 122 lines checked
Patch 5/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/7 Checking commit 7eb130375ad0 (ui/cocoa: Subclass NSApplication so we can implement sendEvent)
7/7 Checking commit 93a3cf9c4268 (ui/cocoa: Perform UI operations only on the main thread)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190214102816.3393-1-peter.maydell@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
2019-02-14 10:28 [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop Peter Maydell
` (13 preceding siblings ...)
2019-02-27 15:24 ` no-reply
@ 2019-02-27 15:29 ` no-reply
2019-02-27 17:41 ` no-reply
15 siblings, 0 replies; 38+ messages in thread
From: no-reply @ 2019-02-27 15:29 UTC (permalink / raw)
To: peter.maydell
Cc: fam, qemu-devel, patches, programmingkidx, r.bolshakov, kraxel,
ben.hekster, berkus
Patchew URL: https://patchew.org/QEMU/20190214102816.3393-1-peter.maydell@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Message-id: 20190214102816.3393-1-peter.maydell@linaro.org
Type: series
Subject: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
=== 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
2e68b86..86c7e2f master -> master
- [tag update] patchew/1549562254-41157-1-git-send-email-pbonzini@redhat.com -> patchew/1549562254-41157-1-git-send-email-pbonzini@redhat.com
- [tag update] patchew/1550076626-7202-1-git-send-email-sandra@codesourcery.com -> patchew/1550076626-7202-1-git-send-email-sandra@codesourcery.com
- [tag update] patchew/20190114111744.113188-1-anton.nefedov@virtuozzo.com -> patchew/20190114111744.113188-1-anton.nefedov@virtuozzo.com
- [tag update] patchew/20190129175403.18017-1-philmd@redhat.com -> patchew/20190129175403.18017-1-philmd@redhat.com
- [tag update] patchew/20190211144421.106232-1-vsementsov@virtuozzo.com -> patchew/20190211144421.106232-1-vsementsov@virtuozzo.com
- [tag update] patchew/20190212193855.13223-1-ccarrara@redhat.com -> patchew/20190212193855.13223-1-ccarrara@redhat.com
- [tag update] patchew/20190214043916.22128-1-david@gibson.dropbear.id.au -> patchew/20190214043916.22128-1-david@gibson.dropbear.id.au
- [tag update] patchew/20190214102816.3393-1-peter.maydell@linaro.org -> patchew/20190214102816.3393-1-peter.maydell@linaro.org
- [tag update] patchew/20190215174548.2630-1-yury-kotov@yandex-team.ru -> patchew/20190215174548.2630-1-yury-kotov@yandex-team.ru
Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for path 'capstone'
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) registered for path 'roms/QemuMacDrivers'
Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 'roms/SLOF'
Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 'roms/ipxe'
Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered for path 'roms/openbios'
Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) registered for path 'roms/openhackware'
Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) registered for path 'roms/qemu-palcode'
Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for path 'roms/seabios'
Submodule 'roms/seabios-hppa' (https://github.com/hdeller/seabios-hppa.git) registered for path 'roms/seabios-hppa'
Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for path 'roms/sgabios'
Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for path 'roms/skiboot'
Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for path 'roms/u-boot'
Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) registered for path 'roms/u-boot-sam460ex'
Submodule 'tests/fp/berkeley-softfloat-3' (https://github.com/cota/berkeley-softfloat-3) registered for path 'tests/fp/berkeley-softfloat-3'
Submodule 'tests/fp/berkeley-testfloat-3' (https://github.com/cota/berkeley-testfloat-3) registered for path 'tests/fp/berkeley-testfloat-3'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) registered for path 'ui/keycodemapdb'
Cloning into 'capstone'...
Submodule path 'capstone': checked out '22ead3e0bfdb87516656453336160e0a37b066bf'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Cloning into 'roms/QemuMacDrivers'...
Submodule path 'roms/QemuMacDrivers': checked out '90c488d5f4a407342247b9ea869df1c2d9c8e266'
Cloning into 'roms/SLOF'...
Submodule path 'roms/SLOF': checked out 'a5b428e1c1eae703bdd62a3f527223c291ee3fdc'
Cloning into 'roms/ipxe'...
Submodule path 'roms/ipxe': checked out 'de4565cbe76ea9f7913a01f331be3ee901bb6e17'
Cloning into 'roms/openbios'...
Submodule path 'roms/openbios': checked out '3464681b2b5983df80086a40179d324102347da3'
Cloning into 'roms/openhackware'...
Submodule path 'roms/openhackware': checked out 'c559da7c8eec5e45ef1f67978827af6f0b9546f5'
Cloning into 'roms/qemu-palcode'...
Submodule path 'roms/qemu-palcode': checked out '51c237d7e20d05100eacadee2f61abc17e6bc097'
Cloning into 'roms/seabios'...
Submodule path 'roms/seabios': checked out 'a698c8995ffb2838296ec284fe3c4ad33dfca307'
Cloning into 'roms/seabios-hppa'...
Submodule path 'roms/seabios-hppa': checked out '1ef99a01572c2581c30e16e6fe69e9ea2ef92ce0'
Cloning into 'roms/sgabios'...
Submodule path 'roms/sgabios': checked out 'cbaee52287e5f32373181cff50a00b6c4ac9015a'
Cloning into 'roms/skiboot'...
Submodule path 'roms/skiboot': checked out 'e0ee24c27a172bcf482f6f2bc905e6211c134bcc'
Cloning into 'roms/u-boot'...
Submodule path 'roms/u-boot': checked out 'd85ca029f257b53a96da6c2fb421e78a003a9943'
Cloning into 'roms/u-boot-sam460ex'...
Submodule path 'roms/u-boot-sam460ex': checked out '60b3916f33e617a815973c5a6df77055b2e3a588'
Cloning into 'tests/fp/berkeley-softfloat-3'...
Submodule path 'tests/fp/berkeley-softfloat-3': checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'tests/fp/berkeley-testfloat-3'...
Submodule path 'tests/fp/berkeley-testfloat-3': checked out '5a59dcec19327396a011a17fd924aed4fec416b3'
Cloning into 'ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce'
Switched to a new branch 'test'
93a3cf9 ui/cocoa: Perform UI operations only on the main thread
7eb1303 ui/cocoa: Subclass NSApplication so we can implement sendEvent
556e313 ui/cocoa: Don't call NSApp sendEvent directly from handleEvent
5e20ccf ui/cocoa: Move console/device menu creation code up in file
017353b ui/cocoa: Factor out initial menu creation
455ce90 ui/cocoa: Use the pixman image directly in switchSurface
02b675e ui/cocoa: Ensure we have the iothread lock when calling into QEMU
=== OUTPUT BEGIN ===
1/7 Checking commit 02b675ede0a2 (ui/cocoa: Ensure we have the iothread lock when calling into QEMU)
2/7 Checking commit 455ce903fa09 (ui/cocoa: Use the pixman image directly in switchSurface)
3/7 Checking commit 017353b840b4 (ui/cocoa: Factor out initial menu creation)
4/7 Checking commit 5e20ccf63c62 (ui/cocoa: Move console/device menu creation code up in file)
5/7 Checking commit 556e313bf5e1 (ui/cocoa: Don't call NSApp sendEvent directly from handleEvent)
ERROR: trailing whitespace
#44: FILE: ui/cocoa.m:152:
+ $
total: 1 errors, 0 warnings, 122 lines checked
Patch 5/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/7 Checking commit 7eb130375ad0 (ui/cocoa: Subclass NSApplication so we can implement sendEvent)
7/7 Checking commit 93a3cf9c4268 (ui/cocoa: Perform UI operations only on the main thread)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190214102816.3393-1-peter.maydell@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
2019-02-14 10:28 [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop Peter Maydell
` (14 preceding siblings ...)
2019-02-27 15:29 ` no-reply
@ 2019-02-27 17:41 ` no-reply
15 siblings, 0 replies; 38+ messages in thread
From: no-reply @ 2019-02-27 17:41 UTC (permalink / raw)
To: peter.maydell
Cc: fam, qemu-devel, patches, programmingkidx, r.bolshakov, kraxel,
ben.hekster, berkus
Patchew URL: https://patchew.org/QEMU/20190214102816.3393-1-peter.maydell@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Message-id: 20190214102816.3393-1-peter.maydell@linaro.org
Subject: [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop
Type: series
=== 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
t [tag update] patchew/1550603704-22474-1-git-send-email-aleksandar.markovic@rt-rk.com -> patchew/1550603704-22474-1-git-send-email-aleksandar.markovic@rt-rk.com
t [tag update] patchew/20190214102816.3393-1-peter.maydell@linaro.org -> patchew/20190214102816.3393-1-peter.maydell@linaro.org
t [tag update] patchew/20190218125615.18970-1-armbru@redhat.com -> patchew/20190218125615.18970-1-armbru@redhat.com
t [tag update] patchew/20190218181349.23885-1-ppandit@redhat.com -> patchew/20190218181349.23885-1-ppandit@redhat.com
Switched to a new branch 'test'
58e0e41bba ui/cocoa: Perform UI operations only on the main thread
6dbdd9bfcc ui/cocoa: Subclass NSApplication so we can implement sendEvent
fba41f39fb ui/cocoa: Don't call NSApp sendEvent directly from handleEvent
84b10e148f ui/cocoa: Move console/device menu creation code up in file
920aaaacee ui/cocoa: Factor out initial menu creation
a45b2c5d67 ui/cocoa: Use the pixman image directly in switchSurface
7bf643a8be ui/cocoa: Ensure we have the iothread lock when calling into QEMU
=== OUTPUT BEGIN ===
1/7 Checking commit 7bf643a8be6d (ui/cocoa: Ensure we have the iothread lock when calling into QEMU)
2/7 Checking commit a45b2c5d67c9 (ui/cocoa: Use the pixman image directly in switchSurface)
3/7 Checking commit 920aaaacee07 (ui/cocoa: Factor out initial menu creation)
4/7 Checking commit 84b10e148f28 (ui/cocoa: Move console/device menu creation code up in file)
5/7 Checking commit fba41f39fbb3 (ui/cocoa: Don't call NSApp sendEvent directly from handleEvent)
ERROR: trailing whitespace
#44: FILE: ui/cocoa.m:152:
+ $
total: 1 errors, 0 warnings, 122 lines checked
Patch 5/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/7 Checking commit 6dbdd9bfcc26 (ui/cocoa: Subclass NSApplication so we can implement sendEvent)
7/7 Checking commit 58e0e41bba98 (ui/cocoa: Perform UI operations only on the main thread)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190214102816.3393-1-peter.maydell@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2019-02-27 17:42 UTC | newest]
Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-14 10:28 [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop Peter Maydell
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 1/7] ui/cocoa: Ensure we have the iothread lock when calling into QEMU Peter Maydell
2019-02-22 15:19 ` Roman Bolshakov
2019-02-22 15:41 ` Peter Maydell
2019-02-22 21:48 ` Roman Bolshakov
2019-02-23 13:11 ` Peter Maydell
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 2/7] ui/cocoa: Use the pixman image directly in switchSurface Peter Maydell
2019-02-14 17:19 ` BALATON Zoltan
2019-02-22 17:27 ` Roman Bolshakov
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 3/7] ui/cocoa: Factor out initial menu creation Peter Maydell
2019-02-14 17:11 ` BALATON Zoltan
2019-02-22 17:47 ` Roman Bolshakov
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 4/7] ui/cocoa: Move console/device menu creation code up in file Peter Maydell
2019-02-15 0:34 ` BALATON Zoltan
2019-02-22 18:10 ` Roman Bolshakov
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 5/7] ui/cocoa: Don't call NSApp sendEvent directly from handleEvent Peter Maydell
2019-02-14 17:04 ` BALATON Zoltan
2019-02-14 17:41 ` Peter Maydell
2019-02-15 0:42 ` BALATON Zoltan
2019-02-22 19:20 ` Roman Bolshakov
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 6/7] ui/cocoa: Subclass NSApplication so we can implement sendEvent Peter Maydell
2019-02-14 17:21 ` BALATON Zoltan
2019-02-22 20:18 ` Roman Bolshakov
2019-02-14 10:28 ` [Qemu-devel] [PATCH v2 7/7] ui/cocoa: Perform UI operations only on the main thread Peter Maydell
2019-02-15 0:55 ` BALATON Zoltan
2019-02-15 1:28 ` BALATON Zoltan
2019-02-15 10:04 ` Peter Maydell
2019-02-16 1:15 ` BALATON Zoltan
2019-02-22 21:35 ` Roman Bolshakov
2019-02-14 11:07 ` [Qemu-devel] [PATCH v2 0/7] ui/cocoa: Use OSX's main loop no-reply
2019-02-14 11:11 ` no-reply
2019-02-14 17:44 ` no-reply
2019-02-14 17:48 ` no-reply
2019-02-14 17:52 ` no-reply
2019-02-15 1:01 ` no-reply
2019-02-27 15:24 ` no-reply
2019-02-27 15:29 ` no-reply
2019-02-27 17:41 ` no-reply
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.