| Message ID | 20210212000540.28486-1-akihiko.odaki@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | ui/cocoa: Remove the uses of full screen APIs | expand |
On Fri, Feb 12, 2021 at 09:05:40AM +0900, Akihiko Odaki wrote: > The detections of full screen APIs were wrong. A detection is coded as: > [NSView respondsToSelector:@selector(exitFullScreenModeWithOptions:)] > but it should be: > [NSView instancesRespondToSelector:@selector(exitFullScreenModeWithOptions:)] > > The uses of full screen APIs were also incorrect, and if you fix the > detections, the full screen view stretches the video, changing the > aspect ratio, even if zooming is disabled. > > Remove the code as it does nothing good. So, it's broken right now (and probably for quite a while without anyone complaining). And the attempt to fix it didn't work out very well. Correct? Just dropping the code makes sense to me then. Any objections or better suggestions from the macos camp? If not I'll go queue it for the next UI pull request in a day or two. thanks, Gerd
2021年2月17日(水) 22:09 Gerd Hoffmann <kraxel@redhat.com>: > > On Fri, Feb 12, 2021 at 09:05:40AM +0900, Akihiko Odaki wrote: > > The detections of full screen APIs were wrong. A detection is coded as: > > [NSView respondsToSelector:@selector(exitFullScreenModeWithOptions:)] > > but it should be: > > [NSView instancesRespondToSelector:@selector(exitFullScreenModeWithOptions:)] > > > > The uses of full screen APIs were also incorrect, and if you fix the > > detections, the full screen view stretches the video, changing the > > aspect ratio, even if zooming is disabled. > > > > Remove the code as it does nothing good. > > So, it's broken right now (and probably for quite a while without anyone > complaining). And the attempt to fix it didn't work out very well. > Correct? Because the detections of APIs are wrong, the code using those APIs were never executed and nobody realized it was broken. I did not seriously attempt to fix it because the APIs are no longer the best ways to implement fullscreen. ([NSWindow -toggleFullScreen:] is more favorable today.) There is not much to reuse even if implementing fullscreen with [NSView -enterFullScreenModeWithOptions:] since the code is so small. > > Just dropping the code makes sense to me then. > > Any objections or better suggestions from the macos camp? > If not I'll go queue it for the next UI pull request in a day or two. > > thanks, > Gerd > Thank you for responding to my patches. Akihiko Odaki
On Fri, 19 Feb 2021, Akihiko Odaki wrote: > 2021年2月17日(水) 22:09 Gerd Hoffmann <kraxel@redhat.com>: >> >> On Fri, Feb 12, 2021 at 09:05:40AM +0900, Akihiko Odaki wrote: >>> The detections of full screen APIs were wrong. A detection is coded as: >>> [NSView respondsToSelector:@selector(exitFullScreenModeWithOptions:)] >>> but it should be: >>> [NSView instancesRespondToSelector:@selector(exitFullScreenModeWithOptions:)] >>> >>> The uses of full screen APIs were also incorrect, and if you fix the >>> detections, the full screen view stretches the video, changing the >>> aspect ratio, even if zooming is disabled. >>> >>> Remove the code as it does nothing good. >> >> So, it's broken right now (and probably for quite a while without anyone >> complaining). And the attempt to fix it didn't work out very well. >> Correct? > > Because the detections of APIs are wrong, the code using those APIs > were never executed and nobody realized it was broken. Full screen on MacOS X worked when I've last tried but that was 2 years ago. > I did not seriously attempt to fix it because the APIs are no longer > the best ways to implement fullscreen. ([NSWindow -toggleFullScreen:] > is more favorable today.) There is not much to reuse even if > implementing fullscreen with [NSView -enterFullScreenModeWithOptions:] > since the code is so small. I think there are people using QEMU to run old MacOS versions on MacOS X/macOS and may not follow this mailing list but I'm sure they'll complain once you break it. Regards, BALATON Zoltan >> Just dropping the code makes sense to me then. >> >> Any objections or better suggestions from the macos camp? >> If not I'll go queue it for the next UI pull request in a day or two. >> >> thanks, >> Gerd >> > > Thank you for responding to my patches. > > Akihiko Odaki > >
2021年2月19日(金) 19:24 BALATON Zoltan <balaton@eik.bme.hu>: > > On Fri, 19 Feb 2021, Akihiko Odaki wrote: > > 2021年2月17日(水) 22:09 Gerd Hoffmann <kraxel@redhat.com>: > >> > >> On Fri, Feb 12, 2021 at 09:05:40AM +0900, Akihiko Odaki wrote: > >>> The detections of full screen APIs were wrong. A detection is coded as: > >>> [NSView respondsToSelector:@selector(exitFullScreenModeWithOptions:)] > >>> but it should be: > >>> [NSView instancesRespondToSelector:@selector(exitFullScreenModeWithOptions:)] > >>> > >>> The uses of full screen APIs were also incorrect, and if you fix the > >>> detections, the full screen view stretches the video, changing the > >>> aspect ratio, even if zooming is disabled. > >>> > >>> Remove the code as it does nothing good. > >> > >> So, it's broken right now (and probably for quite a while without anyone > >> complaining). And the attempt to fix it didn't work out very well. > >> Correct? > > > > Because the detections of APIs are wrong, the code using those APIs > > were never executed and nobody realized it was broken. > > Full screen on MacOS X worked when I've last tried but that was 2 years > ago. > > > I did not seriously attempt to fix it because the APIs are no longer > > the best ways to implement fullscreen. ([NSWindow -toggleFullScreen:] > > is more favorable today.) There is not much to reuse even if > > implementing fullscreen with [NSView -enterFullScreenModeWithOptions:] > > since the code is so small. > > I think there are people using QEMU to run old MacOS versions on MacOS > X/macOS and may not follow this mailing list but I'm sure they'll complain > once you break it. It was not clear what "full screen APIs" refer to in my patch. Today macOS have three different methods to enter fullscreen: - [NSWindow -toggleFullscreen:] (the latest one) - [NSView -enterFullScreenModeWithOptions:] - Make a borderless window whose frame matches with the screen ui/cocoa checks if [NSView -enterFullScreenModeWithOptions:] exists and uses it in this case. Otherwise, it chooses the last method. However, the detection of [NSView -enterFullScreenModeWithOptions:] was broken and I fixed it to find the use of [NSView -enterFullScreenModeWithOptions:] was wrong. This patch deletes references to [NSView -enterFullScreenModeWithOptions:] but the code implementing the last method is still intact and properly functions. Akihiko Odaki > > Regards, > BALATON Zoltan > > >> Just dropping the code makes sense to me then. > >> > >> Any objections or better suggestions from the macos camp? > >> If not I'll go queue it for the next UI pull request in a day or two. > >> > >> thanks, > >> Gerd > >> > > > > Thank you for responding to my patches. > > > > Akihiko Odaki > > > >
> > I think there are people using QEMU to run old MacOS versions on MacOS > > X/macOS and may not follow this mailing list but I'm sure they'll complain > > once you break it. > > It was not clear what "full screen APIs" refer to in my patch. Today > macOS have three different methods to enter fullscreen: > - [NSWindow -toggleFullscreen:] (the latest one) > - [NSView -enterFullScreenModeWithOptions:] > - Make a borderless window whose frame matches with the screen > > ui/cocoa checks if [NSView -enterFullScreenModeWithOptions:] exists > and uses it in this case. Otherwise, it chooses the last method. > However, the detection of [NSView -enterFullScreenModeWithOptions:] > was broken and I fixed it to find the use of [NSView > -enterFullScreenModeWithOptions:] was wrong. This patch deletes > references to [NSView -enterFullScreenModeWithOptions:] but the code > implementing the last method is still intact and properly functions. Ah, that explains why fullscreen worked just fine when I tried it yesterday in my macos test vm. Can you update the commit message explaining this please? The text above should do it for the most part. thanks, Gerd
diff --git a/ui/cocoa.m b/ui/cocoa.m index 13fba8103e1..36e45cd98b4 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -564,37 +564,26 @@ - (void) toggleFullScreen:(id)sender isFullscreen = FALSE; [self ungrabMouse]; [self setContentDimensions]; - if ([NSView respondsToSelector:@selector(exitFullScreenModeWithOptions:)]) { // test if "exitFullScreenModeWithOptions" is supported on host at runtime - [self exitFullScreenModeWithOptions:nil]; - } else { - [fullScreenWindow close]; - [normalWindow setContentView: self]; - [normalWindow makeKeyAndOrderFront: self]; - [NSMenu setMenuBarVisible:YES]; - } + [fullScreenWindow close]; + [normalWindow setContentView: self]; + [normalWindow makeKeyAndOrderFront: self]; + [NSMenu setMenuBarVisible:YES]; } else { // switch from desktop to fullscreen isFullscreen = TRUE; [normalWindow orderOut: nil]; /* Hide the window */ [self grabMouse]; [self setContentDimensions]; - if ([NSView respondsToSelector:@selector(enterFullScreenMode:withOptions:)]) { // test if "enterFullScreenMode:withOptions" is supported on host at runtime - [self enterFullScreenMode:[NSScreen mainScreen] withOptions:[NSDictionary dictionaryWithObjectsAndKeys: - [NSNumber numberWithBool:NO], NSFullScreenModeAllScreens, - [NSDictionary dictionaryWithObjectsAndKeys:[NSNumber numberWithBool:NO], kCGDisplayModeIsStretched, nil], NSFullScreenModeSetting, - nil]]; - } else { - [NSMenu setMenuBarVisible:NO]; - fullScreenWindow = [[NSWindow alloc] initWithContentRect:[[NSScreen mainScreen] frame] - styleMask:NSWindowStyleMaskBorderless - backing:NSBackingStoreBuffered - defer:NO]; - [fullScreenWindow setAcceptsMouseMovedEvents: YES]; - [fullScreenWindow setHasShadow:NO]; - [fullScreenWindow setBackgroundColor: [NSColor blackColor]]; - [self setFrame:NSMakeRect(cx, cy, cw, ch)]; - [[fullScreenWindow contentView] addSubview: self]; - [fullScreenWindow makeKeyAndOrderFront:self]; - } + [NSMenu setMenuBarVisible:NO]; + fullScreenWindow = [[NSWindow alloc] initWithContentRect:[[NSScreen mainScreen] frame] + styleMask:NSWindowStyleMaskBorderless + backing:NSBackingStoreBuffered + defer:NO]; + [fullScreenWindow setAcceptsMouseMovedEvents: YES]; + [fullScreenWindow setHasShadow:NO]; + [fullScreenWindow setBackgroundColor: [NSColor blackColor]]; + [self setFrame:NSMakeRect(cx, cy, cw, ch)]; + [[fullScreenWindow contentView] addSubview: self]; + [fullScreenWindow makeKeyAndOrderFront:self]; } }
The detections of full screen APIs were wrong. A detection is coded as: [NSView respondsToSelector:@selector(exitFullScreenModeWithOptions:)] but it should be: [NSView instancesRespondToSelector:@selector(exitFullScreenModeWithOptions:)] The uses of full screen APIs were also incorrect, and if you fix the detections, the full screen view stretches the video, changing the aspect ratio, even if zooming is disabled. Remove the code as it does nothing good. Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> --- ui/cocoa.m | 41 +++++++++++++++-------------------------- 1 file changed, 15 insertions(+), 26 deletions(-)