macOS: Clean up coordinate system calculations (#3302)

* Clean up macOS and iOS monitor code a bit

* Clean up window size methods

Use `setContentSize`, `setContentMinSize`, `setContentMaxSize` and `contentRectForFrameRect` to let the windowing system figure out the required scaling, instead of us doing it manually.

* Use a flipped NSView coordinate system

* Clean up window position methods

src/platform_impl/macos/monitor.rs

@@ -11,17 +11,17 @@ use core_graphics::display::{
     CGDirectDisplayID, CGDisplay, CGDisplayBounds, CGDisplayCopyDisplayMode,
 };
 use icrate::AppKit::NSScreen;
-use icrate::Foundation::{ns_string, MainThreadMarker, NSNumber};
+use icrate::Foundation::{ns_string, MainThreadMarker, NSNumber, NSPoint, NSRect};
 use objc2::{rc::Id, runtime::AnyObject};
 
 use super::ffi;
-use crate::dpi::{PhysicalPosition, PhysicalSize};
+use crate::dpi::{LogicalPosition, PhysicalPosition, PhysicalSize};
 
 #[derive(Clone)]
 pub struct VideoMode {
-    pub(crate) size: (u32, u32),
-    pub(crate) bit_depth: u16,
-    pub(crate) refresh_rate_millihertz: u32,
+    size: PhysicalSize<u32>,
+    bit_depth: u16,
+    refresh_rate_millihertz: u32,
     pub(crate) monitor: MonitorHandle,
     pub(crate) native_mode: NativeDisplayMode,
 }
@@ -80,7 +80,7 @@ impl Clone for NativeDisplayMode {
 
 impl VideoMode {
     pub fn size(&self) -> PhysicalSize<u32> {
-        self.size.into()
+        self.size
     }
 
     pub fn bit_depth(&self) -> u16 {
@@ -154,26 +154,14 @@ pub fn primary_monitor() -> MonitorHandle {
 
 impl fmt::Debug for MonitorHandle {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        // TODO: Do this using the proper fmt API
-        #[derive(Debug)]
-        #[allow(dead_code)]
-        struct MonitorHandle {
-            name: Option<String>,
-            native_identifier: u32,
-            size: PhysicalSize<u32>,
-            position: PhysicalPosition<i32>,
-            scale_factor: f64,
-        }
-
-        let monitor_id_proxy = MonitorHandle {
-            name: self.name(),
-            native_identifier: self.native_identifier(),
-            size: self.size(),
-            position: self.position(),
-            scale_factor: self.scale_factor(),
-        };
-
-        monitor_id_proxy.fmt(f)
+        f.debug_struct("MonitorHandle")
+            .field("name", &self.name())
+            .field("native_identifier", &self.native_identifier())
+            .field("size", &self.size())
+            .field("position", &self.position())
+            .field("scale_factor", &self.scale_factor())
+            .field("refresh_rate_millihertz", &self.refresh_rate_millihertz())
+            .finish_non_exhaustive()
     }
 }
 
@@ -182,6 +170,8 @@ impl MonitorHandle {
         MonitorHandle(id)
     }
 
+    // TODO: Be smarter about this:
+    // <https://github.com/glfw/glfw/blob/57cbded0760a50b9039ee0cb3f3c14f60145567c/src/cocoa_monitor.m#L44-L126>
     pub fn name(&self) -> Option<String> {
         let MonitorHandle(display_id) = *self;
         let screen_num = CGDisplay::new(display_id).model_number();
@@ -203,11 +193,12 @@ impl MonitorHandle {
 
     #[inline]
     pub fn position(&self) -> PhysicalPosition<i32> {
+        // This is already in screen coordinates. If we were using `NSScreen`,
+        // then a conversion would've been needed:
+        // flip_window_screen_coordinates(self.ns_screen(mtm)?.frame())
         let bounds = unsafe { CGDisplayBounds(self.native_identifier()) };
-        PhysicalPosition::from_logical::<_, f64>(
-            (bounds.origin.x as f64, bounds.origin.y as f64),
-            self.scale_factor(),
-        )
+        let position = LogicalPosition::new(bounds.origin.x, bounds.origin.y);
+        position.to_physical(self.scale_factor())
     }
 
     pub fn scale_factor(&self) -> f64 {
@@ -268,13 +259,12 @@ impl MonitorHandle {
             };
 
             modes.into_iter().map(move |mode| {
-                let cg_refresh_rate_millihertz =
-                    ffi::CGDisplayModeGetRefreshRate(mode).round() as i64;
+                let cg_refresh_rate_hertz = ffi::CGDisplayModeGetRefreshRate(mode).round() as i64;
 
                 // CGDisplayModeGetRefreshRate returns 0.0 for any display that
                 // isn't a CRT
-                let refresh_rate_millihertz = if cg_refresh_rate_millihertz > 0 {
-                    (cg_refresh_rate_millihertz * 1000) as u32
+                let refresh_rate_millihertz = if cg_refresh_rate_hertz > 0 {
+                    (cg_refresh_rate_hertz * 1000) as u32
                 } else {
                     refresh_rate_millihertz
                 };
@@ -293,7 +283,7 @@ impl MonitorHandle {
                 };
 
                 VideoMode {
-                    size: (
+                    size: PhysicalSize::new(
                         ffi::CGDisplayModeGetPixelWidth(mode) as u32,
                         ffi::CGDisplayModeGetPixelHeight(mode) as u32,
                     ),
@@ -339,3 +329,24 @@ pub(crate) fn get_display_id(screen: &NSScreen) -> u32 {
         obj.as_u32()
     })
 }
+
+/// Core graphics screen coordinates are relative to the top-left corner of
+/// the so-called "main" display, with y increasing downwards - which is
+/// exactly what we want in Winit.
+///
+/// However, `NSWindow` and `NSScreen` changes these coordinates to:
+/// 1. Be relative to the bottom-left corner of the "main" screen.
+/// 2. Be relative to the bottom-left corner of the window/screen itself.
+/// 3. Have y increasing upwards.
+///
+/// This conversion happens to be symmetric, so we only need this one function
+/// to convert between the two coordinate systems.
+pub(crate) fn flip_window_screen_coordinates(frame: NSRect) -> NSPoint {
+    // It is intentional that we use `CGMainDisplayID` (as opposed to
+    // `NSScreen::mainScreen`), because that's what the screen coordinates
+    // are relative to, no matter which display the window is currently on.
+    let main_screen_height = CGDisplay::main().bounds().size.height;
+
+    let y = main_screen_height - frame.size.height - frame.origin.y;
+    NSPoint::new(frame.origin.x, y)
+}