From 03bfe3ef80eb2f2af6932f6f0f9117a430328ecd Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Tue, 16 Jan 2024 12:01:54 +0100 Subject: [PATCH 1/2] Call CGGetActiveDisplayList once to avoid panic Previously we called CGGetActiveDisplayList twice: once to get the number of displays and then to get the displays. We saw a panic due to no displays being returned here. As a first attempt to fix the panic, we're reducing the amount of calls to CGGetActiveDisplayList and just do one with the trade-off being that we pre-allocate 32 pointers in a Vec. --- crates/gpui/src/platform/mac/display.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/gpui/src/platform/mac/display.rs b/crates/gpui/src/platform/mac/display.rs index 25e0921fee28efd25c0c1ddf88d6df84318a19aa..ba15fbd90d8c05284049023f515f2539a5f8d8db 100644 --- a/crates/gpui/src/platform/mac/display.rs +++ b/crates/gpui/src/platform/mac/display.rs @@ -33,17 +33,20 @@ impl MacDisplay { /// Obtains an iterator over all currently active system displays. pub fn all() -> impl Iterator { unsafe { - let mut display_count: u32 = 0; - let result = CGGetActiveDisplayList(0, std::ptr::null_mut(), &mut display_count); + // We're assuming there aren't more than 32 displays connected to the system. + let mut displays = Vec::with_capacity(32); + let mut display_count = 0; + let result = CGGetActiveDisplayList( + displays.capacity() as u32, + displays.as_mut_ptr(), + &mut display_count, + ); if result == 0 { - let mut displays = Vec::with_capacity(display_count as usize); - CGGetActiveDisplayList(display_count, displays.as_mut_ptr(), &mut display_count); displays.set_len(display_count as usize); - displays.into_iter().map(MacDisplay) } else { - panic!("Failed to get active display list"); + panic!("Failed to get active display list. Result: {result}"); } } } From f938bae0a2122b5244a19bfef5d3b3b4c1a5335b Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Tue, 16 Jan 2024 14:23:10 +0100 Subject: [PATCH 2/2] Use NSScreen to fetch primary display According to Chromium source, `NSScreen::screens` should always get us one display. We made this change because we ran into panics caused by the previous `unwrap()` when `CGGetActiveDisplayList` might return an empty list. --- crates/gpui/src/platform/mac/display.rs | 23 ++++++++++++++++++++++- crates/gpui/src/platform/mac/window.rs | 2 +- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/crates/gpui/src/platform/mac/display.rs b/crates/gpui/src/platform/mac/display.rs index ba15fbd90d8c05284049023f515f2539a5f8d8db..2b72c335c80e80f700d6caa25fd64d5c8bf4f414 100644 --- a/crates/gpui/src/platform/mac/display.rs +++ b/crates/gpui/src/platform/mac/display.rs @@ -1,10 +1,16 @@ use crate::{point, size, Bounds, DisplayId, GlobalPixels, PlatformDisplay}; use anyhow::Result; +use cocoa::{ + appkit::NSScreen, + base::{id, nil}, + foundation::{NSDictionary, NSString}, +}; use core_foundation::uuid::{CFUUIDGetUUIDBytes, CFUUIDRef}; use core_graphics::{ display::{CGDirectDisplayID, CGDisplayBounds, CGGetActiveDisplayList}, geometry::{CGPoint, CGRect, CGSize}, }; +use objc::{msg_send, sel, sel_impl}; use std::any::Any; use uuid::Uuid; @@ -27,7 +33,22 @@ impl MacDisplay { /// Get the primary screen - the one with the menu bar, and whose bottom left /// corner is at the origin of the AppKit coordinate system. pub fn primary() -> Self { - Self::all().next().unwrap() + // Instead of iterating through all active systems displays via `all()` we use the first + // NSScreen and gets its CGDirectDisplayID, because we can't be sure that `CGGetActiveDisplayList` + // will always return a list of active displays (machine might be sleeping). + // + // The following is what Chromium does too: + // + // https://chromium.googlesource.com/chromium/src/+/66.0.3359.158/ui/display/mac/screen_mac.mm#56 + unsafe { + let screens = NSScreen::screens(nil); + let screen = cocoa::foundation::NSArray::objectAtIndex(screens, 0); + let device_description = NSScreen::deviceDescription(screen); + let screen_number_key: id = NSString::alloc(nil).init_str("NSScreenNumber"); + let screen_number = device_description.objectForKey_(screen_number_key); + let screen_number: CGDirectDisplayID = msg_send![screen_number, unsignedIntegerValue]; + Self(screen_number) + } } /// Obtains an iterator over all currently active system displays. diff --git a/crates/gpui/src/platform/mac/window.rs b/crates/gpui/src/platform/mac/window.rs index 4c71141cdce41b1492b48bd00048255d5763e535..c364021281a3690635713bd756c520b1d5f3558b 100644 --- a/crates/gpui/src/platform/mac/window.rs +++ b/crates/gpui/src/platform/mac/window.rs @@ -484,7 +484,7 @@ impl MacWindow { let display = options .display_id - .and_then(|display_id| MacDisplay::all().find(|display| display.id() == display_id)) + .and_then(MacDisplay::find_by_id) .unwrap_or_else(MacDisplay::primary); let mut target_screen = nil;