collab ui: Fix notification windows on external monitors (#9817)
Sharing a project displays a notification (window) on every screen. Previously there was an issue with the positioning of windows on all screens except the primary screen. As you can see here:  Now:  @mikayla-maki and I also decided to refactor the `WindowOptions` a bit. Previously you could specify bounds which controlled the positioning and size of the window in the global coordinate space, while also providing a display id (which screen to show the window on). This can lead to unusual behavior because you could theoretically specify a global bound which does not even belong to the display id which was provided. Therefore we changed the api to this: ```rust struct WindowOptions { /// The bounds of the window in screen coordinates /// None -> inherit, Some(bounds) -> set bounds. pub bounds: Option<Bounds<DevicePixels>>, /// The display to create the window on, if this is None, /// the window will be created on the main display pub display_id: Option<DisplayId>, } ``` This lets you specify a display id, which maps to the screen where the window should be created and bounds relative to the upper left of the screen. Release Notes: - Fixed positioning of popup windows (e.g. when sharing a project) when using multiple external displays. --------- Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>
This commit is contained in:
co-authored by
Conrad Irwin
parent
ffd698be14
commit
e272acd1bc
@@ -1,9 +1,9 @@
|
||||
use crate::{point, size, Bounds, DisplayId, GlobalPixels, PlatformDisplay};
|
||||
use crate::{point, size, Bounds, DevicePixels, DisplayId, PlatformDisplay};
|
||||
use anyhow::Result;
|
||||
use cocoa::{
|
||||
appkit::NSScreen,
|
||||
base::{id, nil},
|
||||
foundation::{NSDictionary, NSPoint, NSRect, NSSize, NSString},
|
||||
foundation::{NSDictionary, NSString},
|
||||
};
|
||||
use core_foundation::uuid::{CFUUIDGetUUIDBytes, CFUUIDRef};
|
||||
use core_graphics::display::{CGDirectDisplayID, CGDisplayBounds, CGGetActiveDisplayList};
|
||||
@@ -69,49 +69,6 @@ extern "C" {
|
||||
fn CGDisplayCreateUUIDFromDisplayID(display: CGDirectDisplayID) -> CFUUIDRef;
|
||||
}
|
||||
|
||||
/// Convert the given rectangle from Cocoa's coordinate space to GPUI's coordinate space.
|
||||
///
|
||||
/// 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 (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 {
|
||||
origin: point(
|
||||
GlobalPixels(rect.origin.x as f32),
|
||||
GlobalPixels(
|
||||
primary_screen_size.height as f32 - rect.origin.y as f32 - rect.size.height as f32,
|
||||
),
|
||||
),
|
||||
size: size(
|
||||
GlobalPixels(rect.size.width as f32),
|
||||
GlobalPixels(rect.size.height as f32),
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
/// Convert the given rectangle from GPUI's coordinate system to Cocoa's native coordinate space.
|
||||
///
|
||||
/// 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 (matching CoreGraphics)
|
||||
pub(crate) fn global_bounds_to_ns_rect(bounds: Bounds<GlobalPixels>) -> NSRect {
|
||||
let primary_screen_height = MacDisplay::primary().bounds().size.height;
|
||||
|
||||
NSRect::new(
|
||||
NSPoint::new(
|
||||
bounds.origin.x.into(),
|
||||
(primary_screen_height - bounds.origin.y - bounds.size.height).into(),
|
||||
),
|
||||
NSSize::new(bounds.size.width.into(), bounds.size.height.into()),
|
||||
)
|
||||
}
|
||||
|
||||
impl PlatformDisplay for MacDisplay {
|
||||
fn id(&self) -> DisplayId {
|
||||
DisplayId(self.0)
|
||||
@@ -145,20 +102,17 @@ impl PlatformDisplay for MacDisplay {
|
||||
]))
|
||||
}
|
||||
|
||||
fn bounds(&self) -> Bounds<GlobalPixels> {
|
||||
fn bounds(&self) -> Bounds<DevicePixels> {
|
||||
unsafe {
|
||||
// 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),
|
||||
),
|
||||
origin: point(DevicePixels(0), DevicePixels(0)),
|
||||
size: size(
|
||||
GlobalPixels(bounds.size.width as f32),
|
||||
GlobalPixels(bounds.size.height as f32),
|
||||
DevicePixels(bounds.size.width as i32),
|
||||
DevicePixels(bounds.size.height as i32),
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,10 +1,10 @@
|
||||
use super::{global_bounds_from_ns_rect, ns_string, renderer, MacDisplay, NSRange};
|
||||
use super::{ns_string, renderer, MacDisplay, NSRange};
|
||||
use crate::{
|
||||
global_bounds_to_ns_rect, platform::PlatformInputHandler, point, px, size, AnyWindowHandle,
|
||||
Bounds, DisplayLink, ExternalPaths, FileDropEvent, ForegroundExecutor, GlobalPixels,
|
||||
KeyDownEvent, Keystroke, Modifiers, ModifiersChangedEvent, MouseButton, MouseDownEvent,
|
||||
MouseMoveEvent, MouseUpEvent, Pixels, PlatformAtlas, PlatformDisplay, PlatformInput,
|
||||
PlatformWindow, Point, PromptLevel, Size, Timer, WindowAppearance, WindowKind, WindowParams,
|
||||
platform::PlatformInputHandler, point, px, size, AnyWindowHandle, Bounds, DevicePixels,
|
||||
DisplayLink, ExternalPaths, FileDropEvent, ForegroundExecutor, KeyDownEvent, Keystroke,
|
||||
Modifiers, ModifiersChangedEvent, MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent,
|
||||
Pixels, PlatformAtlas, PlatformDisplay, PlatformInput, PlatformWindow, Point, PromptLevel,
|
||||
Size, Timer, WindowAppearance, WindowKind, WindowParams,
|
||||
};
|
||||
use block::ConcreteBlock;
|
||||
use cocoa::{
|
||||
@@ -441,9 +441,28 @@ impl MacWindowState {
|
||||
}
|
||||
}
|
||||
|
||||
fn bounds(&self) -> Bounds<GlobalPixels> {
|
||||
let frame = unsafe { NSWindow::frame(self.native_window) };
|
||||
global_bounds_from_ns_rect(frame)
|
||||
fn bounds(&self) -> Bounds<DevicePixels> {
|
||||
let mut window_frame = unsafe { NSWindow::frame(self.native_window) };
|
||||
let screen_frame = unsafe {
|
||||
let screen = NSWindow::screen(self.native_window);
|
||||
NSScreen::frame(screen)
|
||||
};
|
||||
|
||||
// Flip the y coordinate to be top-left origin
|
||||
window_frame.origin.y =
|
||||
screen_frame.size.height - window_frame.origin.y - window_frame.size.height;
|
||||
|
||||
let bounds = Bounds::new(
|
||||
point(
|
||||
((window_frame.origin.x - screen_frame.origin.x) as i32).into(),
|
||||
((window_frame.origin.y - screen_frame.origin.y) as i32).into(),
|
||||
),
|
||||
size(
|
||||
(window_frame.size.width as i32).into(),
|
||||
(window_frame.size.height as i32).into(),
|
||||
),
|
||||
);
|
||||
bounds
|
||||
}
|
||||
|
||||
fn content_size(&self) -> Size<Pixels> {
|
||||
@@ -494,9 +513,9 @@ impl MacWindow {
|
||||
titlebar,
|
||||
kind,
|
||||
is_movable,
|
||||
display_id,
|
||||
focus,
|
||||
show,
|
||||
display_id,
|
||||
}: WindowParams,
|
||||
executor: ForegroundExecutor,
|
||||
renderer_context: renderer::Context,
|
||||
@@ -529,28 +548,37 @@ impl MacWindow {
|
||||
|
||||
let display = display_id
|
||||
.and_then(MacDisplay::find_by_id)
|
||||
.unwrap_or_else(MacDisplay::primary);
|
||||
.unwrap_or_else(|| MacDisplay::primary());
|
||||
|
||||
let mut target_screen = nil;
|
||||
let mut screen_frame = None;
|
||||
|
||||
let screens = NSScreen::screens(nil);
|
||||
let count: u64 = cocoa::foundation::NSArray::count(screens);
|
||||
for i in 0..count {
|
||||
let screen = cocoa::foundation::NSArray::objectAtIndex(screens, i);
|
||||
let frame = NSScreen::visibleFrame(screen);
|
||||
let display_id = display_id_for_screen(screen);
|
||||
if display_id == display.id().0 {
|
||||
if display_id == display.0 {
|
||||
screen_frame = Some(frame);
|
||||
target_screen = screen;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
let window_rect = {
|
||||
let display_bounds = display.bounds();
|
||||
if bounds.intersects(&display_bounds) {
|
||||
global_bounds_to_ns_rect(bounds)
|
||||
} else {
|
||||
global_bounds_to_ns_rect(display_bounds)
|
||||
}
|
||||
};
|
||||
let screen_frame = screen_frame.unwrap_or_else(|| {
|
||||
let screen = NSScreen::mainScreen(nil);
|
||||
target_screen = screen;
|
||||
NSScreen::visibleFrame(screen)
|
||||
});
|
||||
|
||||
let window_rect = NSRect::new(
|
||||
NSPoint::new(
|
||||
screen_frame.origin.x + bounds.origin.x.0 as f64,
|
||||
screen_frame.origin.y
|
||||
+ (display.bounds().size.height - bounds.origin.y).0 as f64,
|
||||
),
|
||||
NSSize::new(bounds.size.width.0 as f64, bounds.size.height.0 as f64),
|
||||
);
|
||||
|
||||
let native_window = native_window.initWithContentRect_styleMask_backing_defer_screen_(
|
||||
window_rect,
|
||||
@@ -572,7 +600,10 @@ impl MacWindow {
|
||||
|
||||
let window_size = {
|
||||
let scale = get_scale_factor(native_window);
|
||||
size(bounds.size.width.0 * scale, bounds.size.height.0 * scale)
|
||||
size(
|
||||
bounds.size.width.0 as f32 * scale,
|
||||
bounds.size.height.0 as f32 * scale,
|
||||
)
|
||||
};
|
||||
|
||||
let window = Self(Arc::new(Mutex::new(MacWindowState {
|
||||
@@ -692,6 +723,11 @@ impl MacWindow {
|
||||
native_window.orderFront_(nil);
|
||||
}
|
||||
|
||||
// Set the initial position of the window to the specified origin.
|
||||
// Although we already specified the position using `initWithContentRect_styleMask_backing_defer_screen_`,
|
||||
// the window position might be incorrect if the main screen (the screen that contains the window that has focus)
|
||||
// is different from the primary screen.
|
||||
NSWindow::setFrameTopLeftPoint_(native_window, window_rect.origin);
|
||||
window.0.lock().move_traffic_light();
|
||||
|
||||
pool.drain();
|
||||
@@ -737,7 +773,7 @@ impl Drop for MacWindow {
|
||||
}
|
||||
|
||||
impl PlatformWindow for MacWindow {
|
||||
fn bounds(&self) -> Bounds<GlobalPixels> {
|
||||
fn bounds(&self) -> Bounds<DevicePixels> {
|
||||
self.0.as_ref().lock().bounds()
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user