| Message ID | EE48B61D-32F7-4BD7-8F53-092105607731@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 21 January 2015 at 19:25, Programmingkid <programmingkidx@gmail.com> wrote: > This patch makes several changes: > - Minimizes distorted full screen display by respecting aspect > ratios. > - Makes full screen mode available on Mac OS 10.7 and higher. > - Allows user to decide if video should be stretched to fill the > screen, using a menu item called "Zoom To Fit". > - Hides the normalWindow so it won't show up in full screen mode. > - Allows user to exit full screen mode. > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > > --- > Changes in version 2: > - Completely rewritten. > - Eliminated depreciated API's. > - Does not change host monitor resolution. > > Change in version 3: > - Fixed full screen window not receiving mouse moved events. Thanks; this patch seems to make fullscreen work pretty well for me. Some oddities I noticed: (1) This doesn't enable the green "fullscreen" button in OSX 10.9.5, so you have to fullscreen via the menu. That seems a shame, but if it's too hard to implement I can live without it. (2) Maybe zoom-to-fit should be the default? I think that's how we handle fullscreen on other UIs, and it seems like the most intuitive thing for it to do. (3) If you are fullscreened then the only way to get out of it seems to be to first hit ctrl+alt to get out of the mouse grab, and then command-F to un-fullscreen. This isn't very obvious, but I can't think of a better thing right now so I guess it's OK. Otherwise the patch itself looks good. A non-fullscreen related bug I noticed: (4) if you have the mouse grabbed and use ctrl-up to enter Expose (or whatever Apple call that "show all the virtual desktops" mode) we don't un-grab the mouse. We should either release the grab or not let OSX steal ctrl-up when grabbed. This isn't a fullscreen related thing, though, so nothing to do with this patch; I just happened to spot it while testing this. thanks -- PMM
On Feb 11, 2015, at 10:06 PM, Peter Maydell wrote: > On 21 January 2015 at 19:25, Programmingkid <programmingkidx@gmail.com> wrote: >> This patch makes several changes: >> - Minimizes distorted full screen display by respecting aspect >> ratios. >> - Makes full screen mode available on Mac OS 10.7 and higher. >> - Allows user to decide if video should be stretched to fill the >> screen, using a menu item called "Zoom To Fit". >> - Hides the normalWindow so it won't show up in full screen mode. >> - Allows user to exit full screen mode. >> >> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> >> >> --- >> Changes in version 2: >> - Completely rewritten. >> - Eliminated depreciated API's. >> - Does not change host monitor resolution. >> >> Change in version 3: >> - Fixed full screen window not receiving mouse moved events. > > Thanks; this patch seems to make fullscreen work pretty well > for me. Some oddities I noticed: > > (1) This doesn't enable the green "fullscreen" button in > OSX 10.9.5, so you have to fullscreen via the menu. That > seems a shame, but if it's too hard to implement I can > live without it. I don't use Mac OS 10.9, so I don't know how to implement it right now. But it could be implemented in its own patch later. > > (2) Maybe zoom-to-fit should be the default? I think that's > how we handle fullscreen on other UIs, and it seems like the > most intuitive thing for it to do. Zoom To Fit makes the screen look weird and text unreadable. I would prefer to have the full screen look good in full screen. > > (3) If you are fullscreened then the only way to get out of it > seems to be to first hit ctrl+alt to get out of the mouse > grab, and then command-F to un-fullscreen. This isn't very > obvious, but I can't think of a better thing right now so > I guess it's OK. That is the way to do it. If we designate one of the keys on the keyboard as the "Get out of full screen" button, we would not be able to use it in the guest. > > Otherwise the patch itself looks good. Thank you. > > A non-fullscreen related bug I noticed: > > (4) if you have the mouse grabbed and use ctrl-up to enter Expose > (or whatever Apple call that "show all the virtual desktops" > mode) we don't un-grab the mouse. We should either release the > grab or not let OSX steal ctrl-up when grabbed. This isn't > a fullscreen related thing, though, so nothing to do with this > patch; I just happened to spot it while testing this. I haven't used control-up to use expose. It was always one of the function keys for me.
On 12 February 2015 at 04:42, Programmingkid <programmingkidx@gmail.com> wrote: >> (2) Maybe zoom-to-fit should be the default? I think that's >> how we handle fullscreen on other UIs, and it seems like the >> most intuitive thing for it to do. > > Zoom To Fit makes the screen look weird and text unreadable. > I would prefer to have the full screen look good in full screen. I only tested briefly with x86 (so the stock VGA resolution) on a 1440x900 macbook air. That looked fine and entirely readable to me. What's the test case you have that doesn't come out looking ok? -- PMM
On Feb 12, 2015, at 12:24 AM, Peter Maydell wrote: > On 12 February 2015 at 04:42, Programmingkid <programmingkidx@gmail.com> wrote: >>> (2) Maybe zoom-to-fit should be the default? I think that's >>> how we handle fullscreen on other UIs, and it seems like the >>> most intuitive thing for it to do. >> >> Zoom To Fit makes the screen look weird and text unreadable. >> I would prefer to have the full screen look good in full screen. > > I only tested briefly with x86 (so the stock VGA resolution) > on a 1440x900 macbook air. That looked fine and entirely > readable to me. What's the test case you have that doesn't > come out looking ok? Open up a text editor in the guest and type anything. Then try to actually read what was typed. Pictures with and without zoom to fit enabled are attached. Host resolution 1280x800 Guest resolution 800x600 No "Zoom To Fit" looks good. With "Zoom To Fit" the text does not look good. When moving the mouse, the redraw looks fuzzy.
On 21 January 2015 at 19:25, Programmingkid <programmingkidx@gmail.com> wrote: > This patch makes several changes: > - Minimizes distorted full screen display by respecting aspect > ratios. > - Makes full screen mode available on Mac OS 10.7 and higher. > - Allows user to decide if video should be stretched to fill the > screen, using a menu item called "Zoom To Fit". > - Hides the normalWindow so it won't show up in full screen mode. > - Allows user to exit full screen mode. > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > > --- > Changes in version 2: > - Completely rewritten. > - Eliminated depreciated API's. > - Does not change host monitor resolution. > > Change in version 3: > - Fixed full screen window not receiving mouse moved events. > @@ -1005,7 +1043,8 @@ int main (int argc, const char * argv[]) { > > // View menu > menu = [[NSMenu alloc] initWithTitle:@"View"]; > - [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Enter Fullscreen" action:@selector(toggleFullScreen:) keyEquivalent:@"f"] autorelease]]; // Fullscreen > + [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:@"f"] autorelease]]; This creates two menu items with the same keyEquivalent, which looks like a cut and paste error? It doesn't seem to me like we need an accelerator for zoom-to-fit, so we can just make that @"" instead I think. Unless you'd rather do something else, I'm going to apply this patch to my cocoa queue with that change and a couple of other minor whitespace-formatting tweaks. (Looking again at whether zoom-to-fit should be default, I think I must have been deceived by the 1400x900 builtin MBA screen being a nice multiple of the VGA screen size; doing full-screen-zoomed on my other monitor looks much worse. So I'm leaving that as you wrote it.) Sorry it's taken me so long to get back to this. thanks -- PMM
On May 10, 2015, at 3:13 PM, Peter Maydell wrote: > On 21 January 2015 at 19:25, Programmingkid <programmingkidx@gmail.com> wrote: >> This patch makes several changes: >> - Minimizes distorted full screen display by respecting aspect >> ratios. >> - Makes full screen mode available on Mac OS 10.7 and higher. >> - Allows user to decide if video should be stretched to fill the >> screen, using a menu item called "Zoom To Fit". >> - Hides the normalWindow so it won't show up in full screen mode. >> - Allows user to exit full screen mode. >> >> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> >> >> --- >> Changes in version 2: >> - Completely rewritten. >> - Eliminated depreciated API's. >> - Does not change host monitor resolution. >> >> Change in version 3: >> - Fixed full screen window not receiving mouse moved events. >> @@ -1005,7 +1043,8 @@ int main (int argc, const char * argv[]) { >> >> // View menu >> menu = [[NSMenu alloc] initWithTitle:@"View"]; >> - [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Enter Fullscreen" action:@selector(toggleFullScreen:) keyEquivalent:@"f"] autorelease]]; // Fullscreen >> + [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:@"f"] autorelease]]; > > This creates two menu items with the same keyEquivalent, which > looks like a cut and paste error? It doesn't seem to me like > we need an accelerator for zoom-to-fit, so we can just make > that @"" instead I think. > > Unless you'd rather do something else, I'm going to apply this > patch to my cocoa queue with that change and a couple of other > minor whitespace-formatting tweaks. > > (Looking again at whether zoom-to-fit should be default, > I think I must have been deceived by the 1400x900 builtin > MBA screen being a nice multiple of the VGA screen size; > doing full-screen-zoomed on my other monitor looks much > worse. So I'm leaving that as you wrote it.) > > Sorry it's taken me so long to get back to this. > > thanks > -- PMM Thank you for reviewing my patch and correcting the double command-f shortcut mistake.
diff --git a/ui/cocoa.m b/ui/cocoa.m index d37c29b..35ab195 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -64,6 +64,7 @@ static int last_buttons; int gArgc; char **gArgv; +bool stretch_video; // keymap conversion int keymap[] = @@ -414,10 +415,21 @@ QemuCocoaView *cocoaView; - (void) setContentDimensions { COCOA_DEBUG("QemuCocoaView: setContentDimensions\n"); - if (isFullscreen) { cdx = [[NSScreen mainScreen] frame].size.width / (float)screen.width; cdy = [[NSScreen mainScreen] frame].size.height / (float)screen.height; + + /* stretches video, but keeps same aspect ratio */ + if(stretch_video == true) { + /* use smallest stretch value - prevents clipping on sides */ + if (MIN(cdx, cdy) == cdx) { + cdy = cdx; + } else { + cdx = cdy; + } + } else { /* No stretching */ + cdx = cdy = 1; + } cw = screen.width * cdx; ch = screen.height * cdy; cx = ([[NSScreen mainScreen] frame].size.width - cw) / 2.0; @@ -502,6 +514,7 @@ QemuCocoaView *cocoaView; #endif } else { // switch from desktop to fullscreen isFullscreen = TRUE; + [normalWindow orderOut: nil]; /* Hide the window */ [self grabMouse]; [self setContentDimensions]; // test if host supports "enterFullScreenMode:withOptions" at compile time @@ -518,8 +531,11 @@ QemuCocoaView *cocoaView; styleMask:NSBorderlessWindowMask backing:NSBackingStoreBuffered defer:NO]; + [fullScreenWindow setAcceptsMouseMovedEvents: YES]; [fullScreenWindow setHasShadow:NO]; - [fullScreenWindow setContentView:self]; + [fullScreenWindow setBackgroundColor: [NSColor blackColor]]; + [self setFrame:NSMakeRect(cx, cy, cw, ch)]; + [[fullScreenWindow contentView] addSubview: self]; [fullScreenWindow makeKeyAndOrderFront:self]; #if (MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_5) } @@ -561,7 +577,7 @@ QemuCocoaView *cocoaView; } // release Mouse grab when pressing ctrl+alt - if (!isFullscreen && ([event modifierFlags] & NSControlKeyMask) && ([event modifierFlags] & NSAlternateKeyMask)) { + if (([event modifierFlags] & NSControlKeyMask) && ([event modifierFlags] & NSAlternateKeyMask)) { [self ungrabMouse]; } break; @@ -798,9 +814,11 @@ QemuCocoaView *cocoaView; } - (void)startEmulationWithArgc:(int)argc argv:(char**)argv; - (void)openPanelDidEnd:(NSOpenPanel *)sheet returnCode:(int)returnCode contextInfo:(void *)contextInfo; +- (void)doToggleFullScreen:(id)sender; - (void)toggleFullScreen:(id)sender; - (void)showQEMUDoc:(id)sender; - (void)showQEMUTec:(id)sender; +- (void)zoomToFit:(id) sender; @end @implementation QemuCocoaAppController @@ -832,7 +850,7 @@ QemuCocoaView *cocoaView; [normalWindow useOptimizedDrawing:YES]; [normalWindow makeKeyAndOrderFront:self]; [normalWindow center]; - + stretch_video = false; } return self; } @@ -921,6 +939,15 @@ QemuCocoaView *cocoaView; [self startEmulationWithArgc:3 argv:(char**)argv]; } } + +/* 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: */ +- (void) doToggleFullScreen:(id)sender +{ + [self toggleFullScreen:(id)sender]; +} + - (void)toggleFullScreen:(id)sender { COCOA_DEBUG("QemuCocoaAppController: toggleFullScreen\n"); @@ -943,6 +970,17 @@ QemuCocoaView *cocoaView; [[NSWorkspace sharedWorkspace] openFile:[NSString stringWithFormat:@"%@/../doc/qemu/qemu-tech.html", [[NSBundle mainBundle] resourcePath]] withApplication:@"Help Viewer"]; } + +/* Stretches video to fit host monitor size */ +- (void)zoomToFit:(id) sender +{ + stretch_video = !stretch_video; + if (stretch_video == true) { + [sender setState: NSOnState]; + } else { + [sender setState: NSOffState]; + } +} @end @@ -1005,7 +1043,8 @@ int main (int argc, const char * argv[]) { // View menu menu = [[NSMenu alloc] initWithTitle:@"View"]; - [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Enter Fullscreen" action:@selector(toggleFullScreen:) keyEquivalent:@"f"] autorelease]]; // Fullscreen + [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:@"f"] autorelease]]; menuItem = [[[NSMenuItem alloc] initWithTitle:@"View" action:nil keyEquivalent:@""] autorelease]; [menuItem setSubmenu:menu]; [[NSApp mainMenu] addItem:menuItem];
This patch makes several changes: - Minimizes distorted full screen display by respecting aspect ratios. - Makes full screen mode available on Mac OS 10.7 and higher. - Allows user to decide if video should be stretched to fill the screen, using a menu item called "Zoom To Fit". - Hides the normalWindow so it won't show up in full screen mode. - Allows user to exit full screen mode. Signed-off-by: John Arbuckle <programmingkidx@gmail.com> --- Changes in version 2: - Completely rewritten. - Eliminated depreciated API's. - Does not change host monitor resolution. Change in version 3: - Fixed full screen window not receiving mouse moved events. ui/cocoa.m | 49 ++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 44 insertions(+), 5 deletions(-)