Re: [Qemu-devel] [PATCH v2] ui/cocoa.m: Make a better about dialog

[Programmingkid]

On Jul 28, 2016, at 11:12 AM, Peter Maydell wrote:

> On 28 July 2016 at 16:01, Programmingkid <address@hidden> wrote:
>> The about dialog in QEMU on Mac OS X is very plain and unhelpful. This patch
>> makes the about dialog look a lot better and have some descriptive 
>> information
>> on what version of QEMU the user is running.
>> 
>> Signed-off-by: John Arbuckle <address@hidden>
>> ---
>> version 2 changes:
>> Added QEMU version to the version label
>> 
>> ui/cocoa.m | 111 
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 108 insertions(+), 3 deletions(-)
>> 
>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>> index 36c6bf0..c6d8824 100644
>> --- a/ui/cocoa.m
>> +++ b/ui/cocoa.m
>> @@ -34,6 +34,7 @@
>> #include "qmp-commands.h"
>> #include "sysemu/blockdev.h"
>> #include <Carbon/Carbon.h>
>> +#include "qemu-version.h"
>> 
>> #ifndef MAC_OS_X_VERSION_10_5
>> #define MAC_OS_X_VERSION_10_5 1050
>> @@ -63,7 +64,7 @@ typedef struct {
>>     int bitsPerPixel;
>> } QEMUScreen;
>> 
>> -NSWindow *normalWindow;
>> +NSWindow *normalWindow, *about_window;
>> static DisplayChangeListener *dcl;
>> static int last_buttons;
>> 
>> @@ -670,7 +671,9 @@ QemuCocoaView *cocoaView;
>>         case NSLeftMouseUp:
>>             mouse_event = true;
>>             if (!isMouseGrabbed && [self screenContainsPoint:p]) {
>> -                [self grabMouse];
>> +                if([[self window] isKeyWindow]) {
>> +                    [self grabMouse];
>> +                }
> 
> What is this doing in this patch?

When a click would take place in the About window, QEMU would just grab the 
mouse when it shouldn't. Only when the user clicks on the main window is when 
QEMU should grab the mouse.

> 
>>             }
>>             break;
>>         case NSRightMouseUp:
>> @@ -824,6 +827,8 @@ QemuCocoaView *cocoaView;
>> - (void)changeDeviceMedia:(id)sender;
>> - (BOOL)verifyQuit;
>> - (void)openDocumentation:(NSString *)filename;
>> +- (IBAction) do_about_menu_item: (id) sender;
>> +- (void)make_about_window;
>> @end
>> 
>> @implementation QemuCocoaAppController
>> @@ -876,6 +881,7 @@ QemuCocoaView *cocoaView;
>>         supportedImageFileTypes = [NSArray arrayWithObjects: @"img", @"iso", 
>> @"dmg",
>>                                  @"qcow", @"qcow2", @"cloop", @"vmdk", 
>> @"cdr",
>>                                   nil];
>> +        [self make_about_window];
>>     }
>>     return self;
>> }
>> @@ -1138,6 +1144,105 @@ QemuCocoaView *cocoaView;
>>     }
>> }
>> 
>> +/* The action method for the About menu item */
>> +- (IBAction) do_about_menu_item: (id) sender
>> +{
>> +    [about_window makeKeyAndOrderFront: nil];
>> +}
>> +
>> +/* Create and display the about dialog */
>> +- (void)make_about_window
>> +{
>> +    /* Make the window */
>> +    int x = 0, y = 0, about_width = 400, about_height = 200;
>> +    NSRect window_rect = NSMakeRect(x, y, about_width, about_height);
>> +    about_window = [[NSWindow alloc] initWithContentRect:window_rect
>> +                    styleMask:NSTitledWindowMask | NSClosableWindowMask |
>> +                    NSMiniaturizableWindowMask
>> +                    backing:NSBackingStoreBuffered
>> +                    defer:NO];
>> +    [about_window setTitle: @"About"];
>> +    [about_window setReleasedWhenClosed: NO];
>> +    [about_window center];
>> +    NSView *superView = [about_window contentView];
>> +
>> +    /* Create the dimensions of the picture */
>> +    int picture_width = 80, picture_height = 80;
>> +    x = (about_width - picture_width)/2;
>> +    y = about_height - picture_height - 10;
>> +    NSRect picture_rect = NSMakeRect(x, y, picture_width, picture_height);
>> +
>> +    /* Get the path to the QEMU binary */
>> +    NSString *binary_name = [NSString stringWithCString: gArgv[0]
>> +                                      encoding: NSASCIIStringEncoding];
>> +    binary_name = [binary_name lastPathComponent];
>> +    NSString *program_path = [[NSString alloc] initWithFormat: @"%@/%@",
>> +    [[NSBundle mainBundle] bundlePath], binary_name];
>> +
>> +    /* Make the picture of QEMU */
>> +    NSImageView *picture_view = [[NSImageView alloc] initWithFrame:
>> +                                                     picture_rect];
>> +    NSImage *qemu_image = [[NSWorkspace sharedWorkspace] iconForFile:
>> +                                                         program_path];
>> +    [picture_view setImage: qemu_image];
>> +    [picture_view setImageScaling: NSScaleToFit];
>> +    [superView addSubview: picture_view];
>> +
>> +    /* Make the name label */
>> +    x = 0;
>> +    y = y - 25;
>> +    int name_width = about_width, name_height = 20;
>> +    NSRect name_rect = NSMakeRect(x, y, name_width, name_height);
>> +    NSTextField *name_label = [[NSTextField alloc] initWithFrame: 
>> name_rect];
>> +    [name_label setEditable: NO];
>> +    [name_label setBezeled: NO];
>> +    [name_label setDrawsBackground: NO];
>> +    [name_label setAlignment: NSCenterTextAlignment];
>> +    NSString *qemu_name = [[NSString alloc] initWithCString: gArgv[0]
>> +                                            encoding: 
>> NSASCIIStringEncoding];
>> +    qemu_name = [qemu_name lastPathComponent];
>> +    [name_label setStringValue: qemu_name];
>> +    [superView addSubview: name_label];
>> +
>> +    /* Set the version label's attributes */
>> +    x = 0;
>> +    y = 50;
>> +    int version_width = about_width, version_height = 20;
>> +    NSRect version_rect = NSMakeRect(x, y, version_width, version_height);
>> +    NSTextField *version_label = [[NSTextField alloc] initWithFrame:
>> +                                                      version_rect];
>> +    [version_label setEditable: NO];
>> +    [version_label setBezeled: NO];
>> +    [version_label setAlignment: NSCenterTextAlignment];
>> +    [version_label setDrawsBackground: NO];
>> +
>> +    /* Format the version output */
>> +    char buffer[100];
>> +    sprintf(buffer, "%s %s", QEMU_VERSION, QEMU_PKGVERSION);
> 
> Please don't use fixed sized buffers for string manipulation,
> they are never a good idea.

I had a feeling there was an issue with this part of the patch.

> 
>> +
>> +    /* Create the version string*/
>> +    NSString *version_string;
>> +    version_string = [[NSString alloc] initWithFormat:
>> +                                       @"QEMU emulator version %s", buffer];
> 
> ...you could have just used C concatenation on this string,
> or if that doesn't work for some reason, use multiple %s
> interpolations here rather than the sprintf above.

I like that idea.

> 
>> +    [version_label setStringValue: version_string];
>> +    [superView addSubview: version_label];
>> +
>> +    /* Make copyright label */
>> +    x = 0;
>> +    y = 35;
>> +    int copyright_width = about_width, copyright_height = 20;
>> +    NSRect copyright_rect = NSMakeRect(x, y, copyright_width, 
>> copyright_height);
>> +    NSTextField *copyright_label = [[NSTextField alloc] initWithFrame:
>> +                                                        copyright_rect];
>> +    [copyright_label setEditable: NO];
>> +    [copyright_label setBezeled: NO];
>> +    [copyright_label setDrawsBackground: NO];
>> +    [copyright_label setAlignment: NSCenterTextAlignment];
>> +    [copyright_label setStringValue:
>> +                     @"Copyright (c) 2003-2008 Fabrice Bellard"];
>> +    [superView addSubview: copyright_label];
>> +}
>> +
>> @end
>> 
>> 
>> @@ -1185,7 +1290,7 @@ int main (int argc, const char * argv[]) {
>> 
>>     // Application menu
>>     menu = [[NSMenu alloc] initWithTitle:@""];
>> -    [menu addItemWithTitle:@"About QEMU" 
>> action:@selector(orderFrontStandardAboutPanel:) keyEquivalent:@""]; // About 
>> QEMU
>> +    [menu addItemWithTitle:@"About QEMU" 
>> action:@selector(do_about_menu_item:) keyEquivalent:@""]; // About QEMU
> 
> Are we doing anything in our custom window that we couldn't
> do with orderFrontStandardAboutPanelWithOptions (which lets
> you provide a dictionary to specify a copyright string,
> version string, app icon, app name, etc) ?

I tried using the orderFrontStandardAboutPanelWithOptions method, but the 
result was not satisfactory. The icon for QEMU was really small and 
underdeveloped looking. My about window's icon looks really bold and powerful. 
Also the window doesn't resize to fit the contents. The version text was too 
long and was cropped by the window. I had to remove text to make it fit.

This code was used to make this window. If you have my patch applied, just 
replace the do_about_menu_item method with this one to try it out.


/* The action method for the About menu item */
- (IBAction) do_about_menu_item: (id) sender
{
    /* Get the path to the QEMU binary */
    NSString *binary_name = [NSString stringWithCString: gArgv[0]
                                      encoding: NSASCIIStringEncoding];
    binary_name = [binary_name lastPathComponent];
    NSString *program_path = [[NSString alloc] initWithFormat: @"%@/%@",
    [[NSBundle mainBundle] bundlePath], binary_name];
    
    NSImage *qemu_image = [[NSWorkspace sharedWorkspace] iconForFile:
                                                         program_path];


    NSString *version_string = [NSString stringWithFormat: @"%s %s",
    QEMU_VERSION, QEMU_PKGVERSION];
    NSArray *values_array = [NSArray arrayWithObjects: version_string,
    @"Copyright (c) 2003-2008 Fabrice Bellard", qemu_image, nil];
    NSArray *keys_array = [NSArray arrayWithObjects: @"Version", @"Copyright",
    @"ApplicationIcon", nil];
    NSDictionary *dict = [NSDictionary dictionaryWithObjects: values_array
    forKeys: keys_array];
    
    [NSApp orderFrontStandardAboutPanelWithOptions: dict];
    //[about_window makeKeyAndOrderFront: nil];
}