-
Notifications
You must be signed in to change notification settings - Fork 2k
[Mac] Fix for DeviceDisplay.MainDisplayInfoChanged event not getting raised #31786
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
401e2c3
f7787ce
7e127db
24578c5
8022ab0
7eb388b
a5edd2f
2acd1bf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,42 @@ | ||
| #nullable enable | ||
| using System; | ||
| using System.Runtime.InteropServices; | ||
| using Foundation; | ||
| using UIKit; | ||
| using ObjCRuntime; | ||
|
|
||
| namespace Microsoft.Maui.Devices | ||
| { | ||
| partial class DeviceDisplayImplementation : IDeviceDisplay | ||
| { | ||
| NSObject? observer; | ||
|
|
||
| #if MACCATALYST | ||
| static readonly NSString ScreenParametersChangedNotification = | ||
| new NSString("NSApplicationDidChangeScreenParametersNotification"); | ||
|
|
||
| // Core Graphics P/Invoke declarations for Mac Catalyst | ||
| [DllImport(Constants.CoreGraphicsLibrary)] | ||
| static extern uint CGMainDisplayID(); | ||
|
|
||
| [DllImport(Constants.CoreGraphicsLibrary)] | ||
| static extern IntPtr CGDisplayCopyDisplayMode(uint display); | ||
|
|
||
| [DllImport(Constants.CoreGraphicsLibrary)] | ||
| static extern void CGDisplayModeRelease(IntPtr mode); | ||
|
|
||
| [DllImport(Constants.CoreGraphicsLibrary)] | ||
| static extern nuint CGDisplayModeGetWidth(IntPtr mode); | ||
|
|
||
| [DllImport(Constants.CoreGraphicsLibrary)] | ||
| static extern nuint CGDisplayModeGetHeight(IntPtr mode); | ||
|
|
||
| [DllImport(Constants.CoreGraphicsLibrary)] | ||
| static extern double CGDisplayModeGetRefreshRate(IntPtr mode); | ||
|
|
||
| [DllImport(Constants.CoreGraphicsLibrary)] | ||
| static extern double CGDisplayRotation(uint display); | ||
|
|
||
| readonly object locker = new object(); | ||
| NSObject? keepScreenOnActivity; | ||
|
|
||
|
|
@@ -50,6 +78,65 @@ protected override void SetKeepScreenOn(bool keepScreenOn) | |
|
|
||
| protected override DisplayInfo GetMainDisplayInfo() | ||
| { | ||
| #if MACCATALYST | ||
| // On Mac Catalyst, bypass UIScreen entirely and use Core Graphics APIs | ||
| // This gets fresh, non-cached screen information directly from the system | ||
| // Note: CGMainDisplayID returns the primary display (with menu bar). | ||
| // In multi-monitor setups, this may not be the display the app window is on. | ||
| var displayId = CGMainDisplayID(); | ||
| var mode = CGDisplayCopyDisplayMode(displayId); | ||
|
|
||
| if (mode == IntPtr.Zero) | ||
| { | ||
| return GetFallbackDisplayInfo(); | ||
| } | ||
|
|
||
| try | ||
| { | ||
| var width = (double)CGDisplayModeGetWidth(mode); | ||
| var height = (double)CGDisplayModeGetHeight(mode); | ||
| var refreshRate = CGDisplayModeGetRefreshRate(mode); | ||
|
|
||
| // Get rotation from Core Graphics | ||
| var rotationDegrees = CGDisplayRotation(displayId); | ||
| var rotation = ConvertRotationDegreesToDisplayRotation(rotationDegrees); | ||
|
|
||
| // Get scale factor from UIScreen as a fallback (this is usually stable) | ||
| var scale = UIScreen.MainScreen.Scale; | ||
|
|
||
| return new DisplayInfo( | ||
| width: width, | ||
| height: height, | ||
| density: scale, | ||
| // Orientation is intentionally hardcoded to Portrait to match Xamarin's Mac Catalyst | ||
| // behavior. Deriving orientation from dimensions/rotation breaks existing tests and | ||
| // Mac desktop apps don't have a meaningful orientation concept. | ||
| orientation: DisplayOrientation.Portrait, | ||
| rotation: rotation, | ||
| rate: (float)refreshRate); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [major] Logic and Correctness - This PR reports the new Mac Catalyst refresh rate, but
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In one of the previous reviews it was suggested to remove the from as this might be a breaking change, so removed that.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [major] Logic and Correctness - This now reports the CoreGraphics refresh rate on Mac Catalyst, but |
||
| } | ||
| finally | ||
| { | ||
| CGDisplayModeRelease(mode); | ||
| } | ||
| #else | ||
| // iOS implementation | ||
| return GetFallbackDisplayInfo(); | ||
| #endif | ||
| } | ||
|
|
||
| static DisplayRotation ConvertRotationDegreesToDisplayRotation(double degrees) => | ||
| degrees switch | ||
| { | ||
| 0 => DisplayRotation.Rotation0, | ||
| 90 => DisplayRotation.Rotation90, | ||
| 180 => DisplayRotation.Rotation180, | ||
| 270 => DisplayRotation.Rotation270, | ||
| _ => DisplayRotation.Rotation0 | ||
| }; | ||
|
|
||
| DisplayInfo GetFallbackDisplayInfo() | ||
| { | ||
| var bounds = UIScreen.MainScreen.Bounds; | ||
| var scale = UIScreen.MainScreen.Scale; | ||
|
|
||
|
|
@@ -70,8 +157,16 @@ protected override DisplayInfo GetMainDisplayInfo() | |
| protected override void StartScreenMetricsListeners() | ||
| { | ||
| var notificationCenter = NSNotificationCenter.DefaultCenter; | ||
|
|
||
| #if MACCATALYST | ||
| // On Mac Catalyst, use multiple notifications to cover all display changes | ||
| // NSApplicationDidChangeScreenParametersNotification - for resolution/refresh rate changes | ||
| observer = notificationCenter.AddObserver(ScreenParametersChangedNotification, OnMainDisplayInfoChanged); | ||
| #else | ||
| // On iOS, use status bar orientation changes (deprecated but still works) | ||
| var notification = UIApplication.DidChangeStatusBarOrientationNotification; | ||
| observer = notificationCenter.AddObserver(notification, OnMainDisplayInfoChanged); | ||
| #endif | ||
| } | ||
|
|
||
| protected override void StopScreenMetricsListeners() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major] Logic and Correctness -
CGDisplayModeGetWidth/Heightreturn the display mode logical size on HiDPI Mac displays, butDisplayInfo.Width/Heightare documented and previously returned as physical pixels (UIScreen.Bounds * Scale). On Retina/HiDPI modes this reports values that are too small while still reporting a Retina density. UseCGDisplayModeGetPixelWidth/PixelHeightfor the pixel dimensions.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a valid concern — we took the reference from Xamarin.Essentials' macOS implementation, which also returns logical units paired with BackingScaleFactor as density (not physical pixel dimensions): xamarin