| Message ID | 20231025140443.68520-2-carwynellis@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | ui/cocoa: add full-screen-scaling display option | expand |
On 2023/10/25 23:04, carwynellis@gmail.com wrote: > From: Carwyn Ellis <carwynellis@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-trivial@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. > --- > 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)". > # 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. > + [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. > [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.
> On 26 Oct 2023, at 03:51, Akihiko Odaki <akihiko.odaki@daynix.com> wrote: > > On 2023/10/25 23:04, carwynellis@gmail.com wrote: >> From: Carwyn Ellis <carwynellis@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-trivial@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
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". +# # 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]; + [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]; [NSApp setDelegate:controller]; @@ -2062,6 +2052,7 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts) [NSApp activateIgnoringOtherApps: YES]; [controller toggleFullScreen: nil]; } + if (opts->u.cocoa.has_full_grab && opts->u.cocoa.full_grab) { [controller setFullGrab: nil]; } @@ -2077,6 +2068,22 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts) left_command_key_enabled = 0; } + if (opts->u.cocoa.has_full_screen_scaling && opts->u.cocoa.full_screen_scaling) { + stretch_video = true; + } + + 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(); + + // register vga output callbacks register_displaychangelistener(&dcl);
From: Carwyn Ellis <carwynellis@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. --- qapi/ui.json | 6 +++++- ui/cocoa.m | 33 ++++++++++++++++++++------------- 2 files changed, 25 insertions(+), 14 deletions(-)