From d9ef987b049a97e649e40284dc44b37cdb25f27a Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 17 Aug 2023 15:23:28 -0700 Subject: [PATCH] Fix AppKit screen coordinate conversion leading to wrong window bounds --- crates/collab_ui/src/collab_ui.rs | 38 ++++++++++++- .../src/incoming_call_notification.rs | 31 +++-------- .../src/project_shared_notification.rs | 27 +++------- crates/gpui/src/platform.rs | 1 + crates/gpui/src/platform/mac/geometry.rs | 15 ++---- crates/gpui/src/platform/mac/screen.rs | 53 +++++++++++++++---- crates/gpui/src/platform/mac/window.rs | 43 ++++----------- crates/gpui/src/platform/test.rs | 4 ++ 8 files changed, 114 insertions(+), 98 deletions(-) diff --git a/crates/collab_ui/src/collab_ui.rs b/crates/collab_ui/src/collab_ui.rs index f2ba35967f..0fed2e0ef9 100644 --- a/crates/collab_ui/src/collab_ui.rs +++ b/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 std::sync::Arc; +use gpui::{ + 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, + 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), + } +} diff --git a/crates/collab_ui/src/incoming_call_notification.rs b/crates/collab_ui/src/incoming_call_notification.rs index 410adbf862..c614a814ca 100644 --- a/crates/collab_ui/src/incoming_call_notification.rs +++ b/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}, - platform::{CursorStyle, MouseButton, WindowBounds, WindowKind, WindowOptions}, + geometry::vector::vec2f, + 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, 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.add_window( - 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), - }, - |_| IncomingCallNotification::new(incoming_call.clone(), app_state.clone()), - ); + let window = cx + .add_window(notification_window_options(screen, window_size), |_| { + IncomingCallNotification::new(incoming_call.clone(), app_state.clone()) + }); notification_windows.push(window); } diff --git a/crates/collab_ui/src/project_shared_notification.rs b/crates/collab_ui/src/project_shared_notification.rs index 500599db59..21fa7d4ee6 100644 --- a/crates/collab_ui/src/project_shared_notification.rs +++ b/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}, - platform::{CursorStyle, MouseButton, WindowBounds, WindowKind, WindowOptions}, + geometry::vector::vec2f, + platform::{CursorStyle, MouseButton}, AppContext, Entity, View, ViewContext, }; use std::sync::{Arc, Weak}; @@ -20,35 +21,19 @@ pub fn init(app_state: &Arc, 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 = cx.add_window( - 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), - }, - |_| { + let window = + cx.add_window(notification_window_options(screen, window_size), |_| { ProjectSharedNotification::new( owner.clone(), *project_id, worktree_root_names.clone(), app_state.clone(), ) - }, - ); + }); notification_windows .entry(*project_id) .or_insert(Vec::new()) diff --git a/crates/gpui/src/platform.rs b/crates/gpui/src/platform.rs index 9f6e303cb7..c994b65573 100644 --- a/crates/gpui/src/platform.rs +++ b/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; } diff --git a/crates/gpui/src/platform/mac/geometry.rs b/crates/gpui/src/platform/mac/geometry.rs index 3ff6c1d8cb..d4ca665549 100644 --- a/crates/gpui/src/platform/mac/geometry.rs +++ b/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::{ - rect::RectF, - vector::{vec2f, Vector2F}, -}; +use pathfinder_geometry::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 { - RectF::new( - vec2f(self.origin.x as f32, self.origin.y as f32), - vec2f(self.size.width as f32, self.size.height as f32), - ) + fn size_vec(&self) -> Vector2F { + vec2f(self.size.width as f32, self.size.height as f32) } fn intersects(&self, other: Self) -> bool { diff --git a/crates/gpui/src/platform/mac/screen.rs b/crates/gpui/src/platform/mac/screen.rs index 3766a13de3..e44ea35480 100644 --- a/crates/gpui/src/platform/mac/screen.rs +++ b/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 { - let frame = self.native_screen.frame(); - frame.to_rectf() - } + unsafe { Self::screen_rect_from_native(self.native_screen.frame()) } + } + + fn content_bounds(&self) -> RectF { + unsafe { Self::screen_rect_from_native(self.native_screen.visibleFrame()) } } } diff --git a/crates/gpui/src/platform/mac/window.rs b/crates/gpui/src/platform/mac/window.rs index f62c33e9f5..3be425d312 100644 --- a/crates/gpui/src/platform/mac/window.rs +++ b/crates/gpui/src/platform/mac/window.rs @@ -368,32 +368,20 @@ impl WindowState { return WindowBounds::Fullscreen; } - let window_frame = self.frame(); - let screen_frame = self.native_window.screen().visibleFrame().to_rectf(); - if window_frame.size() == screen_frame.size() { + let frame = self.frame(); + let screen_size = self.native_window.screen().visibleFrame().size_vec(); + 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 window_frame = NSWindow::frame(self.native_window); - 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, - ), - ) + let frame = NSWindow::frame(self.native_window); + Screen::screen_rect_from_native(frame) } } @@ -480,21 +468,12 @@ impl MacWindow { native_window.setFrame_display_(screen.visibleFrame(), YES); } WindowBounds::Fixed(rect) => { - let screen_frame = screen.visibleFrame(); - let ns_rect = NSRect::new( - NSPoint::new( - rect.origin_x() as f64, - 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); + let bounds = Screen::screen_rect_to_native(rect); + let screen_bounds = screen.visibleFrame(); + if bounds.intersects(screen_bounds) { + native_window.setFrame_display_(bounds, YES); } else { - native_window.setFrame_display_(screen_frame, YES); + native_window.setFrame_display_(screen_bounds, YES); } } } diff --git a/crates/gpui/src/platform/test.rs b/crates/gpui/src/platform/test.rs index 6c11817b5c..ee3a26c6fd 100644 --- a/crates/gpui/src/platform/test.rs +++ b/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 { Some(uuid::Uuid::new_v4()) }