From aa83726c728f9cdc319e347968e77d4c73d52f18 Mon Sep 17 00:00:00 2001
From: Mads Marquart <mads@marquart.dk>
Date: Tue, 29 Apr 2025 13:25:46 +0200
Subject: [PATCH] macOS: Fix monitors connected via certain Thunderbolt hubs
 (#4207)

Instead of panicking, raise a warning and return `None` or similar.

Co-authored-by: RJ <rj@metabrew.com>
---
 src/changelog/unreleased.md                   |  1 +
 src/platform_impl/apple/appkit/monitor.rs     | 27 ++++++++++++++-----
 .../apple/appkit/window_delegate.rs           | 10 +++++--
 3 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/src/changelog/unreleased.md b/src/changelog/unreleased.md
index 7fb17247..a50817b5 100644
--- a/src/changelog/unreleased.md
+++ b/src/changelog/unreleased.md
@@ -256,3 +256,4 @@ changelog entry.
 - On Wayland, apply fractional scaling to custom cursors.
 - On macOS, fixed `VideoMode::refresh_rate_millihertz` for fractional refresh rates.
 - On macOS, store monitor handle to avoid panics after going in/out of sleep.
+- On macOS, allow certain invalid monitor handles and return `None` instead of panicking.
diff --git a/src/platform_impl/apple/appkit/monitor.rs b/src/platform_impl/apple/appkit/monitor.rs
index 4d005761..09c6931c 100644
--- a/src/platform_impl/apple/appkit/monitor.rs
+++ b/src/platform_impl/apple/appkit/monitor.rs
@@ -141,10 +141,18 @@ impl MonitorHandle {
         let refresh_rate_millihertz = self.refresh_rate_millihertz();
         let monitor = self.clone();
 
-        let array = unsafe { CGDisplayCopyAllDisplayModes(self.display_id(), None) }
-            .expect("failed to get list of display modes");
-        // SAFETY: `CGDisplayCopyAllDisplayModes` is documented to return an array of display modes.
-        let modes = unsafe { CFRetained::cast_unchecked::<CFArray<CGDisplayMode>>(array) };
+        let array = unsafe { CGDisplayCopyAllDisplayModes(self.display_id(), None) };
+        let modes = if let Some(array) = array {
+            // SAFETY: `CGDisplayCopyAllDisplayModes` is documented to return an array of
+            // display modes.
+            unsafe { CFRetained::cast_unchecked::<CFArray<CGDisplayMode>>(array) }
+        } else {
+            // Occasionally, certain CalDigit Thunderbolt Hubs report a spurious monitor during
+            // sleep/wake/cycling monitors. It tends to have null or 1 video mode only.
+            // See <https://github.com/bevyengine/bevy/issues/17827>.
+            warn!(monitor = ?self, "failed to get a list of display modes");
+            CFArray::empty()
+        };
 
         modes.into_iter().map(move |mode| {
             let cg_refresh_rate_hertz = unsafe { CGDisplayMode::refresh_rate(Some(&mode)) };
@@ -165,9 +173,14 @@ impl MonitorHandle {
         let uuid = self.uuid();
         NSScreen::screens(mtm).into_iter().find(|screen| {
             let other_native_id = get_display_id(screen);
-            // Display ID just fetched from live NSScreen, should be fine to unwrap.
-            let other = MonitorHandle::new(other_native_id).expect("invalid display ID");
-            uuid == other.uuid()
+            if let Some(other) = MonitorHandle::new(other_native_id) {
+                uuid == other.uuid()
+            } else {
+                // Display ID was just fetched from live NSScreen, but can still result in `None`
+                // with certain Thunderbolt docked monitors.
+                warn!(other_native_id, "comparing against screen with invalid display ID");
+                false
+            }
         })
     }
 }
diff --git a/src/platform_impl/apple/appkit/window_delegate.rs b/src/platform_impl/apple/appkit/window_delegate.rs
index 71d4bde0..62555bc9 100644
--- a/src/platform_impl/apple/appkit/window_delegate.rs
+++ b/src/platform_impl/apple/appkit/window_delegate.rs
@@ -1756,8 +1756,14 @@ impl WindowDelegate {
     // Allow directly accessing the current monitor internally without unwrapping.
     pub(crate) fn current_monitor_inner(&self) -> Option<MonitorHandle> {
         let display_id = get_display_id(&*self.window().screen()?);
-        // Display ID just fetched from live NSScreen, should be fine to unwrap.
-        Some(MonitorHandle::new(display_id).expect("invalid display ID"))
+        if let Some(monitor) = MonitorHandle::new(display_id) {
+            Some(monitor)
+        } else {
+            // NOTE: Display ID was just fetched from live NSScreen, but can still result in `None`
+            // with certain Thunderbolt docked monitors.
+            warn!(display_id, "got screen with invalid display ID");
+            None
+        }
     }
 
     #[inline]