Skip to content

Commit aa83726

Browse files
madsmtmRJ
andauthored
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>
1 parent 1800fa1 commit aa83726

File tree

3 files changed

+29
-9
lines changed

3 files changed

+29
-9
lines changed

src/changelog/unreleased.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,3 +256,4 @@ changelog entry.
256256
- On Wayland, apply fractional scaling to custom cursors.
257257
- On macOS, fixed `VideoMode::refresh_rate_millihertz` for fractional refresh rates.
258258
- On macOS, store monitor handle to avoid panics after going in/out of sleep.
259+
- On macOS, allow certain invalid monitor handles and return `None` instead of panicking.

src/platform_impl/apple/appkit/monitor.rs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,18 @@ impl MonitorHandle {
141141
let refresh_rate_millihertz = self.refresh_rate_millihertz();
142142
let monitor = self.clone();
143143

144-
let array = unsafe { CGDisplayCopyAllDisplayModes(self.display_id(), None) }
145-
.expect("failed to get list of display modes");
146-
// SAFETY: `CGDisplayCopyAllDisplayModes` is documented to return an array of display modes.
147-
let modes = unsafe { CFRetained::cast_unchecked::<CFArray<CGDisplayMode>>(array) };
144+
let array = unsafe { CGDisplayCopyAllDisplayModes(self.display_id(), None) };
145+
let modes = if let Some(array) = array {
146+
// SAFETY: `CGDisplayCopyAllDisplayModes` is documented to return an array of
147+
// display modes.
148+
unsafe { CFRetained::cast_unchecked::<CFArray<CGDisplayMode>>(array) }
149+
} else {
150+
// Occasionally, certain CalDigit Thunderbolt Hubs report a spurious monitor during
151+
// sleep/wake/cycling monitors. It tends to have null or 1 video mode only.
152+
// See <https://github.com/bevyengine/bevy/issues/17827>.
153+
warn!(monitor = ?self, "failed to get a list of display modes");
154+
CFArray::empty()
155+
};
148156

149157
modes.into_iter().map(move |mode| {
150158
let cg_refresh_rate_hertz = unsafe { CGDisplayMode::refresh_rate(Some(&mode)) };
@@ -165,9 +173,14 @@ impl MonitorHandle {
165173
let uuid = self.uuid();
166174
NSScreen::screens(mtm).into_iter().find(|screen| {
167175
let other_native_id = get_display_id(screen);
168-
// Display ID just fetched from live NSScreen, should be fine to unwrap.
169-
let other = MonitorHandle::new(other_native_id).expect("invalid display ID");
170-
uuid == other.uuid()
176+
if let Some(other) = MonitorHandle::new(other_native_id) {
177+
uuid == other.uuid()
178+
} else {
179+
// Display ID was just fetched from live NSScreen, but can still result in `None`
180+
// with certain Thunderbolt docked monitors.
181+
warn!(other_native_id, "comparing against screen with invalid display ID");
182+
false
183+
}
171184
})
172185
}
173186
}

src/platform_impl/apple/appkit/window_delegate.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1756,8 +1756,14 @@ impl WindowDelegate {
17561756
// Allow directly accessing the current monitor internally without unwrapping.
17571757
pub(crate) fn current_monitor_inner(&self) -> Option<MonitorHandle> {
17581758
let display_id = get_display_id(&*self.window().screen()?);
1759-
// Display ID just fetched from live NSScreen, should be fine to unwrap.
1760-
Some(MonitorHandle::new(display_id).expect("invalid display ID"))
1759+
if let Some(monitor) = MonitorHandle::new(display_id) {
1760+
Some(monitor)
1761+
} else {
1762+
// NOTE: Display ID was just fetched from live NSScreen, but can still result in `None`
1763+
// with certain Thunderbolt docked monitors.
1764+
warn!(display_id, "got screen with invalid display ID");
1765+
None
1766+
}
17611767
}
17621768

17631769
#[inline]

0 commit comments

Comments
 (0)