> On 26 Oct 2023, at 03:51, Akihiko Odaki <akihiko.od...@daynix.com> wrote:
>
> On 2023/10/25 23:04, carwynel...@gmail.com wrote:
>> From: Carwyn Ellis <carwynel...@gmail.com>
>> Provides a display option, full-screen-scaling, that enables scaling of
>> the display when full-screen mode is enabled.
>> Also ensures that the corresponding menu item is marked as enabled when
>> the option is set to on.
>
> Hi,
>
> Thank you for your contribution.
>
> Please drop qemu-triv...@nongnu.org. This change is not that trivial.
> Instead add "Graphics maintainers" listed in MAINTAINERS file to CC.
>
> Please also add Signed-off-by tag. See docs/devel/submitting-a-patch.rst to
> know what the tag means.
Hi,
No problem at all. Thanks for getting back to me so quickly! :)
I guess I missed this on the email body so I’ll make sure this is present when
I resubmit.
Thanks
>
>> ---
>> qapi/ui.json | 6 +++++-
>> ui/cocoa.m | 33 ++++++++++++++++++++-------------
>> 2 files changed, 25 insertions(+), 14 deletions(-)
>> diff --git a/qapi/ui.json b/qapi/ui.json
>> index 006616aa77..9035b230ce 100644
>> --- a/qapi/ui.json
>> +++ b/qapi/ui.json
>> @@ -1409,13 +1409,17 @@
>> # codes match their position on non-Mac keyboards and you can use
>> # Meta/Super and Alt where you expect them. (default: off)
>> #
>> +# @full-screen-scaling: Scale display to fit when full-screen enabled.
>> +# Defaults to "off".
>> +#
>
> I think it's better to name zoom-to-fit to align with DisplayGTK.
> It should also have "(Since 8.2)".
I did look for an existing param but missed this one. Thanks!
>
>> # Since: 7.0
>> ##
>> { 'struct': 'DisplayCocoa',
>> 'data': {
>> '*left-command-key': 'bool',
>> '*full-grab': 'bool',
>> - '*swap-opt-cmd': 'bool'
>> + '*swap-opt-cmd': 'bool',
>> + '*full-screen-scaling': 'bool'
>> } }
>> ##
>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>> index d95276013c..7ddc4de174 100644
>> --- a/ui/cocoa.m
>> +++ b/ui/cocoa.m
>> @@ -1671,7 +1671,9 @@ static void create_initial_menus(void)
>> // View menu
>> menu = [[NSMenu alloc] initWithTitle:@"View"];
>> [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Enter Fullscreen"
>> action:@selector(doToggleFullScreen:) keyEquivalent:@"f"] autorelease]]; //
>> Fullscreen
>> - [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Zoom To Fit"
>> action:@selector(zoomToFit:) keyEquivalent:@""] autorelease]];
>> + menuItem = [[[NSMenuItem alloc] initWithTitle:@"Zoom To Fit"
>> action:@selector(zoomToFit:) keyEquivalent:@""] autorelease];
>> + [menuItem setState: (stretch_video) ? NSControlStateValueOn :
>> NSControlStateValueOff];
>
> nit: You don't need parentheses around strech_video.
No problem - will change this.
>
>> + [menu addItem: menuItem];
>> menuItem = [[[NSMenuItem alloc] initWithTitle:@"View" action:nil
>> keyEquivalent:@""] autorelease];
>> [menuItem setSubmenu:menu];
>> [[NSApp mainMenu] addItem:menuItem];
>> @@ -2041,18 +2043,6 @@ static void cocoa_display_init(DisplayState *ds,
>> DisplayOptions *opts)
>> [QemuApplication sharedApplication];
>> - create_initial_menus();
>> -
>> - /*
>> - * Create the menu entries which depend on QEMU state (for consoles
>> - * and removable 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 *controller = [[QemuCocoaAppController alloc]
>> init];
>
> QemuCocoaAppController also has code to initialize stretch_video; it's not OK
> to have code to initialize a same variable in two different places.
Ok, I’ll take a look and tidy this up.
>
>> [NSApp setDelegate:controller];
>> @@ -2062,6 +2052,7 @@ static void cocoa_display_init(DisplayState *ds,
>> DisplayOptions *opts)
>> [NSApp activateIgnoringOtherApps: YES];
>> [controller toggleFullScreen: nil];
>> }
>> +
>
> Don't add a blank line here. It's irrelevant from this change.
Ok.
I’ll aim to get this resubmitted as a V2 later today with the appropriate CCs.
Thanks again,
Carwyn
