gpui: Retain maximized and fullscreen state for new windows derived from previous windows (#44605)

Lukas Wirth created

Release Notes:

- Fixed new windows underflowing the taskbar on windows
- Improved new windows spawned from maximized or fullscreened windows by
copying the maximized and fullscreened states

Change summary

crates/gpui/src/platform.rs                 |   7 
crates/gpui/src/platform/mac/display.rs     |  53 +++++++
crates/gpui/src/platform/windows/display.rs |  32 ++++
crates/gpui/src/window.rs                   | 162 ++++++++++------------
4 files changed, 161 insertions(+), 93 deletions(-)

Detailed changes

crates/gpui/src/platform.rs 🔗

@@ -289,6 +289,13 @@ pub trait PlatformDisplay: Send + Sync + Debug {
     /// Get the bounds for this display
     fn bounds(&self) -> Bounds<Pixels>;
 
+    /// Get the visible bounds for this display, excluding taskbar/dock areas.
+    /// This is the usable area where windows can be placed without being obscured.
+    /// Defaults to the full display bounds if not overridden.
+    fn visible_bounds(&self) -> Bounds<Pixels> {
+        self.bounds()
+    }
+
     /// Get the default bounds for this display to place a window
     fn default_bounds(&self) -> Bounds<Pixels> {
         let bounds = self.bounds();

crates/gpui/src/platform/mac/display.rs 🔗

@@ -1,9 +1,9 @@
-use crate::{Bounds, DisplayId, Pixels, PlatformDisplay, px, size};
+use crate::{Bounds, DisplayId, Pixels, PlatformDisplay, point, px, size};
 use anyhow::Result;
 use cocoa::{
     appkit::NSScreen,
     base::{id, nil},
-    foundation::{NSDictionary, NSString},
+    foundation::{NSArray, NSDictionary, NSString},
 };
 use core_foundation::uuid::{CFUUIDGetUUIDBytes, CFUUIDRef};
 use core_graphics::display::{CGDirectDisplayID, CGDisplayBounds, CGGetActiveDisplayList};
@@ -114,4 +114,53 @@ impl PlatformDisplay for MacDisplay {
             }
         }
     }
+
+    fn visible_bounds(&self) -> Bounds<Pixels> {
+        unsafe {
+            let dominated_screen = self.get_nsscreen();
+
+            if dominated_screen == nil {
+                return self.bounds();
+            }
+
+            let screen_frame = NSScreen::frame(dominated_screen);
+            let visible_frame = NSScreen::visibleFrame(dominated_screen);
+
+            // Convert from bottom-left origin (AppKit) to top-left origin
+            let origin_y =
+                screen_frame.size.height - visible_frame.origin.y - visible_frame.size.height
+                    + screen_frame.origin.y;
+
+            Bounds {
+                origin: point(
+                    px(visible_frame.origin.x as f32 - screen_frame.origin.x as f32),
+                    px(origin_y as f32),
+                ),
+                size: size(
+                    px(visible_frame.size.width as f32),
+                    px(visible_frame.size.height as f32),
+                ),
+            }
+        }
+    }
+}
+
+impl MacDisplay {
+    /// Find the NSScreen corresponding to this display
+    unsafe fn get_nsscreen(&self) -> id {
+        let screens = unsafe { NSScreen::screens(nil) };
+        let count = unsafe { NSArray::count(screens) };
+        let screen_number_key: id = unsafe { NSString::alloc(nil).init_str("NSScreenNumber") };
+
+        for i in 0..count {
+            let screen = unsafe { NSArray::objectAtIndex(screens, i) };
+            let device_description = unsafe { NSScreen::deviceDescription(screen) };
+            let screen_number = unsafe { device_description.objectForKey_(screen_number_key) };
+            let screen_id: CGDirectDisplayID = msg_send![screen_number, unsignedIntegerValue];
+            if screen_id == self.0 {
+                return screen;
+            }
+        }
+        nil
+    }
 }

crates/gpui/src/platform/windows/display.rs 🔗

@@ -23,6 +23,7 @@ pub(crate) struct WindowsDisplay {
     pub display_id: DisplayId,
     scale_factor: f32,
     bounds: Bounds<Pixels>,
+    visible_bounds: Bounds<Pixels>,
     physical_bounds: Bounds<DevicePixels>,
     uuid: Uuid,
 }
@@ -36,6 +37,7 @@ impl WindowsDisplay {
         let screen = available_monitors().into_iter().nth(display_id.0 as _)?;
         let info = get_monitor_info(screen).log_err()?;
         let monitor_size = info.monitorInfo.rcMonitor;
+        let work_area = info.monitorInfo.rcWork;
         let uuid = generate_uuid(&info.szDevice);
         let scale_factor = get_scale_factor_for_monitor(screen).log_err()?;
         let physical_size = size(
@@ -55,6 +57,14 @@ impl WindowsDisplay {
                 ),
                 size: physical_size.to_pixels(scale_factor),
             },
+            visible_bounds: Bounds {
+                origin: logical_point(work_area.left as f32, work_area.top as f32, scale_factor),
+                size: size(
+                    (work_area.right - work_area.left) as f32 / scale_factor,
+                    (work_area.bottom - work_area.top) as f32 / scale_factor,
+                )
+                .map(crate::px),
+            },
             physical_bounds: Bounds {
                 origin: point(monitor_size.left.into(), monitor_size.top.into()),
                 size: physical_size,
@@ -66,6 +76,7 @@ impl WindowsDisplay {
     pub fn new_with_handle(monitor: HMONITOR) -> anyhow::Result<Self> {
         let info = get_monitor_info(monitor)?;
         let monitor_size = info.monitorInfo.rcMonitor;
+        let work_area = info.monitorInfo.rcWork;
         let uuid = generate_uuid(&info.szDevice);
         let display_id = available_monitors()
             .iter()
@@ -89,6 +100,14 @@ impl WindowsDisplay {
                 ),
                 size: physical_size.to_pixels(scale_factor),
             },
+            visible_bounds: Bounds {
+                origin: logical_point(work_area.left as f32, work_area.top as f32, scale_factor),
+                size: size(
+                    (work_area.right - work_area.left) as f32 / scale_factor,
+                    (work_area.bottom - work_area.top) as f32 / scale_factor,
+                )
+                .map(crate::px),
+            },
             physical_bounds: Bounds {
                 origin: point(monitor_size.left.into(), monitor_size.top.into()),
                 size: physical_size,
@@ -100,6 +119,7 @@ impl WindowsDisplay {
     fn new_with_handle_and_id(handle: HMONITOR, display_id: DisplayId) -> anyhow::Result<Self> {
         let info = get_monitor_info(handle)?;
         let monitor_size = info.monitorInfo.rcMonitor;
+        let work_area = info.monitorInfo.rcWork;
         let uuid = generate_uuid(&info.szDevice);
         let scale_factor = get_scale_factor_for_monitor(handle)?;
         let physical_size = size(
@@ -119,6 +139,14 @@ impl WindowsDisplay {
                 ),
                 size: physical_size.to_pixels(scale_factor),
             },
+            visible_bounds: Bounds {
+                origin: logical_point(work_area.left as f32, work_area.top as f32, scale_factor),
+                size: size(
+                    (work_area.right - work_area.left) as f32 / scale_factor,
+                    (work_area.bottom - work_area.top) as f32 / scale_factor,
+                )
+                .map(crate::px),
+            },
             physical_bounds: Bounds {
                 origin: point(monitor_size.left.into(), monitor_size.top.into()),
                 size: physical_size,
@@ -193,6 +221,10 @@ impl PlatformDisplay for WindowsDisplay {
     fn bounds(&self) -> Bounds<Pixels> {
         self.bounds
     }
+
+    fn visible_bounds(&self) -> Bounds<Pixels> {
+        self.visible_bounds
+    }
 }
 
 fn available_monitors() -> SmallVec<[HMONITOR; 4]> {

crates/gpui/src/window.rs 🔗

@@ -918,86 +918,69 @@ pub(crate) struct ElementStateBox {
     pub(crate) type_name: &'static str,
 }
 
-fn default_bounds(display_id: Option<DisplayId>, cx: &mut App) -> Bounds<Pixels> {
-    #[cfg(target_os = "macos")]
-    {
-        const CASCADE_OFFSET: f32 = 25.0;
-
-        let display = display_id
-            .map(|id| cx.find_display(id))
-            .unwrap_or_else(|| cx.primary_display());
-
-        let display_bounds = display
-            .as_ref()
-            .map(|d| d.bounds())
-            .unwrap_or_else(|| Bounds::new(point(px(0.), px(0.)), DEFAULT_WINDOW_SIZE));
-
-        // TODO, BUG: if you open a window with the currently active window
-        // on the stack, this will erroneously select the 'unwrap_or_else'
-        // code path
-        let (base_origin, base_size) = cx
-            .active_window()
-            .and_then(|w| {
-                w.update(cx, |_, window, _| {
-                    let bounds = window.bounds();
-                    (bounds.origin, bounds.size)
-                })
-                .ok()
-            })
-            .unwrap_or_else(|| {
-                let default_bounds = display
-                    .as_ref()
-                    .map(|d| d.default_bounds())
-                    .unwrap_or_else(|| Bounds::new(point(px(0.), px(0.)), DEFAULT_WINDOW_SIZE));
-                (default_bounds.origin, default_bounds.size)
-            });
-
-        let cascade_offset = point(px(CASCADE_OFFSET), px(CASCADE_OFFSET));
-        let proposed_origin = base_origin + cascade_offset;
-        let proposed_bounds = Bounds::new(proposed_origin, base_size);
-
-        let display_right = display_bounds.origin.x + display_bounds.size.width;
-        let display_bottom = display_bounds.origin.y + display_bounds.size.height;
-        let window_right = proposed_bounds.origin.x + proposed_bounds.size.width;
-        let window_bottom = proposed_bounds.origin.y + proposed_bounds.size.height;
-
-        let fits_horizontally = window_right <= display_right;
-        let fits_vertically = window_bottom <= display_bottom;
-
-        let final_origin = match (fits_horizontally, fits_vertically) {
-            (true, true) => proposed_origin,
-            (false, true) => point(display_bounds.origin.x, base_origin.y),
-            (true, false) => point(base_origin.x, display_bounds.origin.y),
-            (false, false) => display_bounds.origin,
-        };
-
-        Bounds::new(final_origin, base_size)
-    }
-
-    #[cfg(not(target_os = "macos"))]
-    {
-        const DEFAULT_WINDOW_OFFSET: Point<Pixels> = point(px(0.), px(35.));
-
-        // TODO, BUG: if you open a window with the currently active window
-        // on the stack, this will erroneously select the 'unwrap_or_else'
-        // code path
-        cx.active_window()
-            .and_then(|w| w.update(cx, |_, window, _| window.bounds()).ok())
-            .map(|mut bounds| {
-                bounds.origin += DEFAULT_WINDOW_OFFSET;
-                bounds
-            })
-            .unwrap_or_else(|| {
-                let display = display_id
-                    .map(|id| cx.find_display(id))
-                    .unwrap_or_else(|| cx.primary_display());
-
-                display
-                    .as_ref()
-                    .map(|display| display.default_bounds())
-                    .unwrap_or_else(|| Bounds::new(point(px(0.), px(0.)), DEFAULT_WINDOW_SIZE))
-            })
-    }
+fn default_bounds(display_id: Option<DisplayId>, cx: &mut App) -> WindowBounds {
+    // TODO, BUG: if you open a window with the currently active window
+    // on the stack, this will erroneously fallback to `None`
+    //
+    // TODO these should be the initial window bounds not considering maximized/fullscreen
+    let active_window_bounds = cx
+        .active_window()
+        .and_then(|w| w.update(cx, |_, window, _| window.window_bounds()).ok());
+
+    const CASCADE_OFFSET: f32 = 25.0;
+
+    let display = display_id
+        .map(|id| cx.find_display(id))
+        .unwrap_or_else(|| cx.primary_display());
+
+    let default_placement = || Bounds::new(point(px(0.), px(0.)), DEFAULT_WINDOW_SIZE);
+
+    // Use visible_bounds to exclude taskbar/dock areas
+    let display_bounds = display
+        .as_ref()
+        .map(|d| d.visible_bounds())
+        .unwrap_or_else(default_placement);
+
+    let (
+        Bounds {
+            origin: base_origin,
+            size: base_size,
+        },
+        window_bounds_ctor,
+    ): (_, fn(Bounds<Pixels>) -> WindowBounds) = match active_window_bounds {
+        Some(bounds) => match bounds {
+            WindowBounds::Windowed(bounds) => (bounds, WindowBounds::Windowed),
+            WindowBounds::Maximized(bounds) => (bounds, WindowBounds::Maximized),
+            WindowBounds::Fullscreen(bounds) => (bounds, WindowBounds::Fullscreen),
+        },
+        None => (
+            display
+                .as_ref()
+                .map(|d| d.default_bounds())
+                .unwrap_or_else(default_placement),
+            WindowBounds::Windowed,
+        ),
+    };
+
+    let cascade_offset = point(px(CASCADE_OFFSET), px(CASCADE_OFFSET));
+    let proposed_origin = base_origin + cascade_offset;
+    let proposed_bounds = Bounds::new(proposed_origin, base_size);
+
+    let display_right = display_bounds.origin.x + display_bounds.size.width;
+    let display_bottom = display_bounds.origin.y + display_bounds.size.height;
+    let window_right = proposed_bounds.origin.x + proposed_bounds.size.width;
+    let window_bottom = proposed_bounds.origin.y + proposed_bounds.size.height;
+
+    let fits_horizontally = window_right <= display_right;
+    let fits_vertically = window_bottom <= display_bottom;
+
+    let final_origin = match (fits_horizontally, fits_vertically) {
+        (true, true) => proposed_origin,
+        (false, true) => point(display_bounds.origin.x, base_origin.y),
+        (true, false) => point(base_origin.x, display_bounds.origin.y),
+        (false, false) => display_bounds.origin,
+    };
+    window_bounds_ctor(Bounds::new(final_origin, base_size))
 }
 
 impl Window {
@@ -1024,13 +1007,11 @@ impl Window {
             tabbing_identifier,
         } = options;
 
-        let bounds = window_bounds
-            .map(|bounds| bounds.get_bounds())
-            .unwrap_or_else(|| default_bounds(display_id, cx));
+        let window_bounds = window_bounds.unwrap_or_else(|| default_bounds(display_id, cx));
         let mut platform_window = cx.platform.open_window(
             handle,
             WindowParams {
-                bounds,
+                bounds: window_bounds.get_bounds(),
                 titlebar,
                 kind,
                 is_movable,
@@ -1071,12 +1052,10 @@ impl Window {
             .request_decorations(window_decorations.unwrap_or(WindowDecorations::Server));
         platform_window.set_background_appearance(window_background);
 
-        if let Some(ref window_open_state) = window_bounds {
-            match window_open_state {
-                WindowBounds::Fullscreen(_) => platform_window.toggle_fullscreen(),
-                WindowBounds::Maximized(_) => platform_window.zoom(),
-                WindowBounds::Windowed(_) => {}
-            }
+        match window_bounds {
+            WindowBounds::Fullscreen(_) => platform_window.toggle_fullscreen(),
+            WindowBounds::Maximized(_) => platform_window.zoom(),
+            WindowBounds::Windowed(_) => {}
         }
 
         platform_window.on_close(Box::new({
@@ -1518,7 +1497,8 @@ impl Window {
         style
     }
 
-    /// Check if the platform window is maximized
+    /// Check if the platform window is maximized.
+    ///
     /// On some platforms (namely Windows) this is different than the bounds being the size of the display
     pub fn is_maximized(&self) -> bool {
         self.platform_window.is_maximized()