Fix AppKit screen coordinate conversion leading to wrong window bounds

crates/collab_ui/src/collab_ui.rs

@@ -9,8 +9,16 @@ mod sharing_status_indicator;
 
 use call::{ActiveCall, Room};
 pub use collab_titlebar_item::CollabTitlebarItem;
-use gpui::{actions, AppContext, Task};
+use gpui::{
-use std::sync::Arc;
+    actions,
+    geometry::{
+        rect::RectF,
+        vector::{vec2f, Vector2F},
+    },
+    platform::{Screen, WindowBounds, WindowKind, WindowOptions},
+    AppContext, Task,
+};
+use std::{rc::Rc, sync::Arc};
 use util::ResultExt;
 use workspace::AppState;
 
@@ -88,3 +96,29 @@ pub fn toggle_deafen(_: &ToggleDeafen, cx: &mut AppContext) {
             .log_err();
     }
 }
+
+fn notification_window_options(
+    screen: Rc<dyn Screen>,
+    window_size: Vector2F,
+) -> WindowOptions<'static> {
+    const NOTIFICATION_PADDING: f32 = 16.;
+
+    let screen_bounds = screen.content_bounds();
+    WindowOptions {
+        bounds: WindowBounds::Fixed(RectF::new(
+            screen_bounds.upper_right()
+                + vec2f(
+                    -NOTIFICATION_PADDING - window_size.x(),
+                    NOTIFICATION_PADDING,
+                ),
+            window_size,
+        )),
+        titlebar: None,
+        center: false,
+        focus: false,
+        show: true,
+        kind: WindowKind::PopUp,
+        is_movable: false,
+        screen: Some(screen),
+    }
+}

crates/collab_ui/src/incoming_call_notification.rs

@@ -1,14 +1,14 @@
-use std::sync::{Arc, Weak};
+use crate::notification_window_options;
-
 use call::{ActiveCall, IncomingCall};
 use client::proto;
 use futures::StreamExt;
 use gpui::{
     elements::*,
-    geometry::{rect::RectF, vector::vec2f},
+    geometry::vector::vec2f,
-    platform::{CursorStyle, MouseButton, WindowBounds, WindowKind, WindowOptions},
+    platform::{CursorStyle, MouseButton},
     AnyElement, AppContext, Entity, View, ViewContext, WindowHandle,
 };
+use std::sync::{Arc, Weak};
 use util::ResultExt;
 use workspace::AppState;
 
@@ -23,31 +23,16 @@ pub fn init(app_state: &Arc<AppState>, cx: &mut AppContext) {
             }
 
             if let Some(incoming_call) = incoming_call {
-                const PADDING: f32 = 16.;
                 let window_size = cx.read(|cx| {
                     let theme = &theme::current(cx).incoming_call_notification;
                     vec2f(theme.window_width, theme.window_height)
                 });
 
                 for screen in cx.platform().screens() {
-                    let screen_bounds = screen.bounds();
+                    let window = cx
-                    let window = cx.add_window(
+                        .add_window(notification_window_options(screen, window_size), |_| {
-                        WindowOptions {
+                            IncomingCallNotification::new(incoming_call.clone(), app_state.clone())
-                            bounds: WindowBounds::Fixed(RectF::new(
+                        });
-                                screen_bounds.upper_right()
-                                    - vec2f(PADDING + window_size.x(), PADDING),
-                                window_size,
-                            )),
-                            titlebar: None,
-                            center: false,
-                            focus: false,
-                            show: true,
-                            kind: WindowKind::PopUp,
-                            is_movable: false,
-                            screen: Some(screen),
-                        },
-                        |_| IncomingCallNotification::new(incoming_call.clone(), app_state.clone()),
-                    );
 
                     notification_windows.push(window);
                 }

crates/collab_ui/src/project_shared_notification.rs

@@ -1,10 +1,11 @@
+use crate::notification_window_options;
 use call::{room, ActiveCall};
 use client::User;
 use collections::HashMap;
 use gpui::{
     elements::*,
-    geometry::{rect::RectF, vector::vec2f},
+    geometry::vector::vec2f,
-    platform::{CursorStyle, MouseButton, WindowBounds, WindowKind, WindowOptions},
+    platform::{CursorStyle, MouseButton},
     AppContext, Entity, View, ViewContext,
 };
 use std::sync::{Arc, Weak};
@@ -20,35 +21,19 @@ pub fn init(app_state: &Arc<AppState>, cx: &mut AppContext) {
             project_id,
             worktree_root_names,
         } => {
-            const PADDING: f32 = 16.;
             let theme = &theme::current(cx).project_shared_notification;
             let window_size = vec2f(theme.window_width, theme.window_height);
 
             for screen in cx.platform().screens() {
-                let screen_bounds = screen.bounds();
+                let window =
-                let window = cx.add_window(
+                    cx.add_window(notification_window_options(screen, window_size), |_| {
-                    WindowOptions {
-                        bounds: WindowBounds::Fixed(RectF::new(
-                            screen_bounds.upper_right() - vec2f(PADDING + window_size.x(), PADDING),
-                            window_size,
-                        )),
-                        titlebar: None,
-                        center: false,
-                        focus: false,
-                        show: true,
-                        kind: WindowKind::PopUp,
-                        is_movable: false,
-                        screen: Some(screen),
-                    },
-                    |_| {
                         ProjectSharedNotification::new(
                             owner.clone(),
                             *project_id,
                             worktree_root_names.clone(),
                             app_state.clone(),
                         )
-                    },
+                    });
-                );
                 notification_windows
                     .entry(*project_id)
                     .or_insert(Vec::new())

crates/gpui/src/platform.rs

@@ -135,6 +135,7 @@ pub trait InputHandler {
 pub trait Screen: Debug {
     fn as_any(&self) -> &dyn Any;
     fn bounds(&self) -> RectF;
+    fn content_bounds(&self) -> RectF;
     fn display_uuid(&self) -> Option<Uuid>;
 }
 

crates/gpui/src/platform/mac/geometry.rs

@@ -3,10 +3,7 @@ use cocoa::{
     foundation::{NSPoint, NSRect},
 };
 use objc::{msg_send, sel, sel_impl};
-use pathfinder_geometry::{
+use pathfinder_geometry::vector::{vec2f, Vector2F};
-    rect::RectF,
-    vector::{vec2f, Vector2F},
-};
 
 ///! Macos screen have a y axis that goings up from the bottom of the screen and
 ///! an origin at the bottom left of the main display.
@@ -15,6 +12,7 @@ pub trait Vector2FExt {
     /// Converts self to an NSPoint with y axis pointing up.
     fn to_screen_ns_point(&self, native_window: id, window_height: f64) -> NSPoint;
 }
+
 impl Vector2FExt for Vector2F {
     fn to_screen_ns_point(&self, native_window: id, window_height: f64) -> NSPoint {
         unsafe {
@@ -25,16 +23,13 @@ impl Vector2FExt for Vector2F {
 }
 
 pub trait NSRectExt {
-    fn to_rectf(&self) -> RectF;
+    fn size_vec(&self) -> Vector2F;
     fn intersects(&self, other: Self) -> bool;
 }
 
 impl NSRectExt for NSRect {
-    fn to_rectf(&self) -> RectF {
+    fn size_vec(&self) -> Vector2F {
-        RectF::new(
+        vec2f(self.size.width as f32, self.size.height as f32)
-            vec2f(self.origin.x as f32, self.origin.y as f32),
-            vec2f(self.size.width as f32, self.size.height as f32),
-        )
     }
 
     fn intersects(&self, other: Self) -> bool {

crates/gpui/src/platform/mac/screen.rs

@@ -1,21 +1,19 @@
-use std::{any::Any, ffi::c_void};
+use super::ns_string;
-
 use crate::platform;
 use cocoa::{
     appkit::NSScreen,
     base::{id, nil},
-    foundation::{NSArray, NSDictionary},
+    foundation::{NSArray, NSDictionary, NSPoint, NSRect, NSSize},
 };
 use core_foundation::{
     number::{kCFNumberIntType, CFNumberGetValue, CFNumberRef},
     uuid::{CFUUIDGetUUIDBytes, CFUUIDRef},
 };
 use core_graphics::display::CGDirectDisplayID;
-use pathfinder_geometry::rect::RectF;
+use pathfinder_geometry::{rect::RectF, vector::vec2f};
+use std::{any::Any, ffi::c_void};
 use uuid::Uuid;
 
-use super::{geometry::NSRectExt, ns_string};
-
 #[link(name = "ApplicationServices", kind = "framework")]
 extern "C" {
     pub fn CGDisplayCreateUUIDFromDisplayID(display: CGDirectDisplayID) -> CFUUIDRef;
@@ -51,6 +49,40 @@ impl Screen {
         }
         screens
     }
+
+    /// Convert the given rectangle in screen coordinates from GPUI's
+    /// coordinate system to the AppKit coordinate system.
+    ///
+    /// In GPUI's coordinates, the origin is at the top left of the main screen, with
+    /// the Y axis pointing downward. In the AppKit coordindate system, the origin is at the
+    /// bottom left of the main screen, with the Y axis pointing upward.
+    pub(crate) fn screen_rect_to_native(rect: RectF) -> NSRect {
+        let main_screen_height = unsafe { NSScreen::mainScreen(nil).frame().size.height };
+        NSRect::new(
+            NSPoint::new(
+                rect.origin_x() as f64,
+                main_screen_height - rect.origin_y() as f64 - rect.height() as f64,
+            ),
+            NSSize::new(rect.width() as f64, rect.height() as f64),
+        )
+    }
+
+    /// Convert the given rectangle in screen coordinates from the AppKit
+    /// coordinate system to GPUI's coordinate system.
+    ///
+    /// In GPUI's coordinates, the origin is at the top left of the main screen, with
+    /// the Y axis pointing downward. In the AppKit coordindate system, the origin is at the
+    /// bottom left of the main screen, with the Y axis pointing upward.
+    pub(crate) fn screen_rect_from_native(rect: NSRect) -> RectF {
+        let main_screen_height = unsafe { NSScreen::mainScreen(nil).frame().size.height };
+        RectF::new(
+            vec2f(
+                rect.origin.x as f32,
+                (main_screen_height - rect.origin.y - rect.size.height) as f32,
+            ),
+            vec2f(rect.size.width as f32, rect.size.height as f32),
+        )
+    }
 }
 
 impl platform::Screen for Screen {
@@ -108,9 +140,10 @@ impl platform::Screen for Screen {
     }
 
     fn bounds(&self) -> RectF {
-        unsafe {
+        unsafe { Self::screen_rect_from_native(self.native_screen.frame()) }
-            let frame = self.native_screen.frame();
+    }
-            frame.to_rectf()
+
-        }
+    fn content_bounds(&self) -> RectF {
+        unsafe { Self::screen_rect_from_native(self.native_screen.visibleFrame()) }
     }
 }

crates/gpui/src/platform/mac/window.rs

@@ -368,32 +368,20 @@ impl WindowState {
                 return WindowBounds::Fullscreen;
             }
 
-            let window_frame = self.frame();
+            let frame = self.frame();
-            let screen_frame = self.native_window.screen().visibleFrame().to_rectf();
+            let screen_size = self.native_window.screen().visibleFrame().size_vec();
-            if window_frame.size() == screen_frame.size() {
+            if frame.size() == screen_size {
                 WindowBounds::Maximized
             } else {
-                WindowBounds::Fixed(window_frame)
+                WindowBounds::Fixed(frame)
             }
         }
     }
 
-    // Returns the window bounds in window coordinates
     fn frame(&self) -> RectF {
         unsafe {
-            let screen_frame = self.native_window.screen().visibleFrame();
+            let frame = NSWindow::frame(self.native_window);
-            let window_frame = NSWindow::frame(self.native_window);
+            Screen::screen_rect_from_native(frame)
-            RectF::new(
-                vec2f(
-                    window_frame.origin.x as f32,
-                    (screen_frame.size.height - window_frame.origin.y - window_frame.size.height)
-                        as f32,
-                ),
-                vec2f(
-                    window_frame.size.width as f32,
-                    window_frame.size.height as f32,
-                ),
-            )
         }
     }
 
@@ -480,21 +468,12 @@ impl MacWindow {
                     native_window.setFrame_display_(screen.visibleFrame(), YES);
                 }
                 WindowBounds::Fixed(rect) => {
-                    let screen_frame = screen.visibleFrame();
+                    let bounds = Screen::screen_rect_to_native(rect);
-                    let ns_rect = NSRect::new(
+                    let screen_bounds = screen.visibleFrame();
-                        NSPoint::new(
+                    if bounds.intersects(screen_bounds) {
-                            rect.origin_x() as f64,
+                        native_window.setFrame_display_(bounds, YES);
-                            screen_frame.size.height
-                                - rect.origin_y() as f64
-                                - rect.height() as f64,
-                        ),
-                        NSSize::new(rect.width() as f64, rect.height() as f64),
-                    );
-
-                    if ns_rect.intersects(screen_frame) {
-                        native_window.setFrame_display_(ns_rect, YES);
                     } else {
-                        native_window.setFrame_display_(screen_frame, YES);
+                        native_window.setFrame_display_(screen_bounds, YES);
                     }
                 }
             }

crates/gpui/src/platform/test.rs

@@ -250,6 +250,10 @@ impl super::Screen for Screen {
         RectF::new(Vector2F::zero(), Vector2F::new(1920., 1080.))
     }
 
+    fn content_bounds(&self) -> RectF {
+        RectF::new(Vector2F::zero(), Vector2F::new(1920., 1080.))
+    }
+
     fn display_uuid(&self) -> Option<uuid::Uuid> {
         Some(uuid::Uuid::new_v4())
     }