Fix positioning of windows on secondary displays (#4169)

Conrad Irwin created 2 years ago

CGDisplayBounds returns data in "global display coordinates" (which are
the same as Zed's coordinates), different from the NS APIs which use
"screen coordinates" (which have the Y axis inverted)

Also remove some transmutes while we're at it

Release Notes:

- Fixed position of notifications on secondary displays

Change summary

crates/gpui/src/platform/mac/display.rs | 45 ++++++++++++++++----------
crates/gpui/src/platform/mac/window.rs  | 16 ++++-----
2 files changed, 34 insertions(+), 27 deletions(-)

Detailed changes

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

@@ -3,13 +3,10 @@ use anyhow::Result;
 use cocoa::{
     appkit::NSScreen,
     base::{id, nil},
-    foundation::{NSDictionary, NSString},
+    foundation::{NSDictionary, NSPoint, NSRect, NSSize, NSString},
 };
 use core_foundation::uuid::{CFUUIDGetUUIDBytes, CFUUIDRef};
-use core_graphics::{
-    display::{CGDirectDisplayID, CGDisplayBounds, CGGetActiveDisplayList},
-    geometry::{CGPoint, CGRect, CGSize},
-};
+use core_graphics::display::{CGDirectDisplayID, CGDisplayBounds, CGGetActiveDisplayList};
 use objc::{msg_send, sel, sel_impl};
 use uuid::Uuid;
 
@@ -77,14 +74,14 @@ extern "C" {
     fn CGDisplayCreateUUIDFromDisplayID(display: CGDirectDisplayID) -> CFUUIDRef;
 }
 
-/// Convert the given rectangle from CoreGraphics' native coordinate space to GPUI's coordinate space.
+/// Convert the given rectangle from Cocoa's coordinate space to GPUI's coordinate space.
 ///
-/// CoreGraphics' coordinate space has its origin at the bottom left of the primary screen,
+/// Cocoa's coordinate space has its origin at the bottom left of the primary screen,
 /// with the Y axis pointing upwards.
 ///
 /// Conversely, in GPUI's coordinate system, the origin is placed at the top left of the primary
-/// screen, with the Y axis pointing downwards.
-pub(crate) fn display_bounds_from_native(rect: CGRect) -> Bounds<GlobalPixels> {
+/// screen, with the Y axis pointing downwards (matching CoreGraphics)
+pub(crate) fn global_bounds_from_ns_rect(rect: NSRect) -> Bounds<GlobalPixels> {
     let primary_screen_size = unsafe { CGDisplayBounds(MacDisplay::primary().id().0) }.size;
 
     Bounds {
@@ -101,22 +98,22 @@ pub(crate) fn display_bounds_from_native(rect: CGRect) -> Bounds<GlobalPixels> {
     }
 }
 
-/// Convert the given rectangle from GPUI's coordinate system to CoreGraphics' native coordinate space.
+/// Convert the given rectangle from GPUI's coordinate system to Cocoa's native coordinate space.
 ///
-/// CoreGraphics' coordinate space has its origin at the bottom left of the primary screen,
+/// Cocoa's coordinate space has its origin at the bottom left of the primary screen,
 /// with the Y axis pointing upwards.
 ///
 /// Conversely, in GPUI's coordinate system, the origin is placed at the top left of the primary
-/// screen, with the Y axis pointing downwards.
-pub(crate) fn display_bounds_to_native(bounds: Bounds<GlobalPixels>) -> CGRect {
+/// screen, with the Y axis pointing downwards (matching CoreGraphics)
+pub(crate) fn global_bounds_to_ns_rect(bounds: Bounds<GlobalPixels>) -> NSRect {
     let primary_screen_height = MacDisplay::primary().bounds().size.height;
 
-    CGRect::new(
-        &CGPoint::new(
+    NSRect::new(
+        NSPoint::new(
             bounds.origin.x.into(),
             (primary_screen_height - bounds.origin.y - bounds.size.height).into(),
         ),
-        &CGSize::new(bounds.size.width.into(), bounds.size.height.into()),
+        NSSize::new(bounds.size.width.into(), bounds.size.height.into()),
     )
 }
 
@@ -155,8 +152,20 @@ impl PlatformDisplay for MacDisplay {
 
     fn bounds(&self) -> Bounds<GlobalPixels> {
         unsafe {
-            let native_bounds = CGDisplayBounds(self.0);
-            display_bounds_from_native(native_bounds)
+            // CGDisplayBounds is in "global display" coordinates, where 0 is
+            // the top left of the primary display.
+            let bounds = CGDisplayBounds(self.0);
+
+            Bounds {
+                origin: point(
+                    GlobalPixels(bounds.origin.x as f32),
+                    GlobalPixels(bounds.origin.y as f32),
+                ),
+                size: size(
+                    GlobalPixels(bounds.size.width as f32),
+                    GlobalPixels(bounds.size.height as f32),
+                ),
+            }
         }
     }
 }

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

@@ -1,6 +1,6 @@
-use super::{display_bounds_from_native, ns_string, MacDisplay, MetalRenderer, NSRange};
+use super::{global_bounds_from_ns_rect, ns_string, MacDisplay, MetalRenderer, NSRange};
 use crate::{
-    display_bounds_to_native, point, px, size, AnyWindowHandle, Bounds, ExternalPaths,
+    global_bounds_to_ns_rect, point, px, size, AnyWindowHandle, Bounds, ExternalPaths,
     FileDropEvent, ForegroundExecutor, GlobalPixels, KeyDownEvent, Keystroke, Modifiers,
     ModifiersChangedEvent, MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent, Pixels,
     PlatformAtlas, PlatformDisplay, PlatformInput, PlatformInputHandler, PlatformWindow, Point,
@@ -411,10 +411,8 @@ impl MacWindowState {
     }
 
     fn frame(&self) -> Bounds<GlobalPixels> {
-        unsafe {
-            let frame = NSWindow::frame(self.native_window);
-            display_bounds_from_native(mem::transmute::<NSRect, CGRect>(frame))
-        }
+        let frame = unsafe { NSWindow::frame(self.native_window) };
+        global_bounds_from_ns_rect(frame)
     }
 
     fn content_size(&self) -> Size<Pixels> {
@@ -650,11 +648,11 @@ impl MacWindow {
                 WindowBounds::Fixed(bounds) => {
                     let display_bounds = display.bounds();
                     let frame = if bounds.intersects(&display_bounds) {
-                        display_bounds_to_native(bounds)
+                        global_bounds_to_ns_rect(bounds)
                     } else {
-                        display_bounds_to_native(display_bounds)
+                        global_bounds_to_ns_rect(display_bounds)
                     };
-                    native_window.setFrame_display_(mem::transmute::<CGRect, NSRect>(frame), YES);
+                    native_window.setFrame_display_(frame, YES);
                 }
             }