X11: Fix get_current_monitor (#515)

* X11: Fix get_current_monitor

Fixes #64

* impl Debug for MonitorId on all platforms

CHANGELOG.md

@@ -4,6 +4,10 @@
 - Wayland: improve diagnostics if initialization fails
 - Fix some system event key doesn't work when focused, do not block keyevent forward to system on macOS
 - On X11, the scroll wheel position is now correctly reset on i3 and other WMs that have the same quirk.
+- On X11, `Window::get_current_monitor` now reliably returns the correct monitor.
+- On X11, `Window::hidpi_factor` returns values from XRandR rather than the inaccurate values previously queried from the core protocol.
+- On X11, the primary monitor is detected correctly even when using versions of XRandR less than 1.5.
+- `MonitorId` now implements `Debug`.
 
 # Version 0.14.0 (2018-05-09)
 

src/platform/android/mod.rs

@@ -164,7 +164,7 @@ pub struct Window {
     native_window: *const c_void,
 }
 
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub struct MonitorId;
 
 mod ffi;

src/platform/emscripten/mod.rs

@@ -24,7 +24,7 @@ pub struct DeviceId;
 #[derive(Clone, Default)]
 pub struct PlatformSpecificHeadlessBuilderAttributes;
 
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub struct MonitorId;
 
 impl MonitorId {

src/platform/ios/mod.rs

@@ -96,7 +96,7 @@ use self::ffi::{
 
 static mut jmpbuf: [c_int;27] = [0;27];
 
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub struct MonitorId;
 
 pub struct Window {

src/platform/linux/mod.rs

@@ -69,7 +69,7 @@ pub enum DeviceId {
     Wayland(wayland::DeviceId)
 }
 
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub enum MonitorId {
     X(x11::MonitorId),
     Wayland(wayland::MonitorId),
@@ -340,6 +340,8 @@ unsafe extern "C" fn x_error_callback(
             minor_code: (*event).minor_code,
         };
 
+        eprintln!("[winit X11 error] {:#?}", error);
+
         *xconn.latest_error.lock() = Some(error);
     }
     // Fun fact: this return value is completely ignored.

src/platform/linux/wayland/event_loop.rs

@@ -1,5 +1,6 @@
 use std::cell::RefCell;
 use std::collections::VecDeque;
+use std::fmt;
 use std::sync::{Arc, Mutex, Weak};
 use std::sync::atomic::{AtomicBool, Ordering};
 
@@ -427,6 +428,29 @@ impl Clone for MonitorId {
     }
 }
 
+impl fmt::Debug for MonitorId {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        #[derive(Debug)]
+        struct MonitorId {
+            name: Option<String>,
+            native_identifier: u32,
+            dimensions: (u32, u32),
+            position: (i32, i32),
+            hidpi_factor: f32,
+        }
+
+        let monitor_id_proxy = MonitorId {
+            name: self.get_name(),
+            native_identifier: self.get_native_identifier(),
+            dimensions: self.get_dimensions(),
+            position: self.get_position(),
+            hidpi_factor: self.get_hidpi_factor(),
+        };
+
+        monitor_id_proxy.fmt(f)
+    }
+}
+
 impl MonitorId {
     pub fn get_name(&self) -> Option<String> {
         self.mgr.with_info(&self.proxy, |_, info| {

src/platform/linux/x11/mod.rs

@@ -9,7 +9,12 @@ mod dnd;
 mod ime;
 mod util;
 
-pub use self::monitor::{MonitorId, get_available_monitors, get_primary_monitor};
+pub use self::monitor::{
+    MonitorId,
+    get_available_monitors,
+    get_monitor_for_window,
+    get_primary_monitor,
+};
 pub use self::window::{Window2, XWindow};
 pub use self::xdisplay::{XConnection, XNotSupported, XError};
 
@@ -46,6 +51,7 @@ pub struct EventsLoop {
     ime_receiver: ImeReceiver,
     ime_sender: ImeSender,
     ime: RefCell<Ime>,
+    randr_event_offset: c_int,
     windows: Arc<Mutex<HashMap<WindowId, WindowData>>>,
     // Please don't laugh at this type signature
     shared_state: RefCell<HashMap<WindowId, Weak<Mutex<window::SharedState>>>>,
@@ -67,6 +73,8 @@ pub struct EventsLoopProxy {
 
 impl EventsLoop {
     pub fn new(display: Arc<XConnection>) -> EventsLoop {
+        let root = unsafe { (display.xlib.XDefaultRootWindow)(display.display) };
+
         let wm_delete_window = unsafe { util::get_atom(&display, b"WM_DELETE_WINDOW\0") }
             .expect("Failed to call XInternAtom (WM_DELETE_WINDOW)");
 
@@ -85,6 +93,9 @@ impl EventsLoop {
             result.expect("Failed to set input method destruction callback")
         });
 
+        let randr_event_offset = monitor::select_input(&display, root)
+            .expect("Failed to query XRandR extension");
+
         let xi2ext = unsafe {
             let mut result = XExtension {
                 opcode: mem::uninitialized(),
@@ -119,7 +130,6 @@ impl EventsLoop {
             }
         }
 
-        let root = unsafe { (display.xlib.XDefaultRootWindow)(display.display) };
         util::update_cached_wm_info(&display, root);
 
         let wakeup_dummy_window = unsafe {
@@ -146,6 +156,7 @@ impl EventsLoop {
             ime_receiver,
             ime_sender,
             ime,
+            randr_event_offset,
             windows: Arc::new(Mutex::new(HashMap::new())),
             shared_state: RefCell::new(HashMap::new()),
             devices: RefCell::new(HashMap::new()),
@@ -1041,7 +1052,12 @@ impl EventsLoop {
                     _ => {}
                 }
             },
-            _ => (),
+            _ => {
+                if event_type == self.randr_event_offset {
+                    // In the future, it would be quite easy to emit monitor hotplug events.
+                    monitor::invalidate_cached_monitor_list();
+                }
+            },
         }
 
         match self.ime_receiver.try_recv() {

src/platform/linux/x11/monitor.rs

@@ -1,9 +1,33 @@
+use std::os::raw::*;
 use std::sync::Arc;
-use std::slice;
 
-use super::XConnection;
+use parking_lot::Mutex;
 
-#[derive(Clone)]
+use super::ffi::{
+    RRCrtcChangeNotifyMask,
+    RROutputPropertyNotifyMask,
+    RRScreenChangeNotifyMask,
+    True,
+    Window,
+    XRRScreenResources,
+};
+use super::{util, XConnection, XError};
+
+// Used to test XRandR < 1.5 code path. This should always be committed as false.
+const FORCE_RANDR_COMPAT: bool = false;
+// Also used for testing. This should always be committed as false.
+const DISABLE_MONITOR_LIST_CACHING: bool = false;
+
+lazy_static! {
+    static ref MONITORS: Mutex<Option<Vec<MonitorId>>> = Mutex::new(None);
+}
+
+pub fn invalidate_cached_monitor_list() -> Option<Vec<MonitorId>> {
+    // We update this lazily.
+    (*MONITORS.lock()).take()
+}
+
+#[derive(Debug, Clone)]
 pub struct MonitorId {
     /// The actual id
     id: u32,
@@ -15,96 +39,34 @@ pub struct MonitorId {
     position: (i32, i32),
     /// If the monitor is the primary one
     primary: bool,
-    /// The DPI scaling factor
+    /// The DPI scale factor
-    hidpi_factor: f32,
+    pub(crate) hidpi_factor: f32,
-}
+    /// Used to determine which windows are on this monitor
-
+    pub(crate) rect: util::Rect,
-pub fn get_available_monitors(x: &Arc<XConnection>) -> Vec<MonitorId> {
-    let mut available = Vec::new();
-    unsafe {
-        let root = (x.xlib.XDefaultRootWindow)(x.display);
-        let resources = (x.xrandr.XRRGetScreenResources)(x.display, root);
-
-        if let Some(ref xrandr_1_5) = x.xrandr_1_5 {
-            // We're in XRandR >= 1.5, enumerate Monitors to handle things like MST and videowalls
-            let mut nmonitors = 0;
-            let monitors = (xrandr_1_5.XRRGetMonitors)(x.display, root, 1, &mut nmonitors);
-            for i in 0..nmonitors {
-                let monitor = *(monitors.offset(i as isize));
-                let output = (xrandr_1_5.XRRGetOutputInfo)(x.display, resources, *(monitor.outputs.offset(0)));
-                let nameslice = slice::from_raw_parts((*output).name as *mut u8, (*output).nameLen as usize);
-                let name = String::from_utf8_lossy(nameslice).into_owned();
-                let hidpi_factor = {
-                    let x_mm = (*output).mm_width as f32;
-                    let y_mm = (*output).mm_height as f32;
-                    let x_px = monitor.width as f32;
-                    let y_px = monitor.height as f32;
-                    let ppmm = ((x_px * y_px) / (x_mm * y_mm)).sqrt();
-
-                    // Quantize 1/12 step size
-                    ((ppmm * (12.0 * 25.4 / 96.0)).round() / 12.0).max(1.0)
-                };
-                (xrandr_1_5.XRRFreeOutputInfo)(output);
-                available.push(MonitorId{
-                    id: i as u32,
-                    name,
-                    hidpi_factor,
-                    dimensions: (monitor.width as u32, monitor.height as u32),
-                    position: (monitor.x as i32, monitor.y as i32),
-                    primary: (monitor.primary != 0),
-                });
-            }
-            (xrandr_1_5.XRRFreeMonitors)(monitors);
-        } else {
-            // We're in XRandR < 1.5, enumerate CRTCs. Everything will work but MST and
-            // videowall setups will show more monitors than the logical groups the user
-            // cares about
-            for i in 0..(*resources).ncrtc {
-                let crtcid = *((*resources).crtcs.offset(i as isize));
-                let crtc = (x.xrandr.XRRGetCrtcInfo)(x.display, resources, crtcid);
-                if (*crtc).width > 0 && (*crtc).height > 0 && (*crtc).noutput > 0 {
-                    let output = (x.xrandr.XRRGetOutputInfo)(x.display, resources, *((*crtc).outputs.offset(0)));
-                    let nameslice = slice::from_raw_parts((*output).name as *mut u8, (*output).nameLen as usize);
-                    let name = String::from_utf8_lossy(nameslice).into_owned();
-
-                    let hidpi_factor = {
-                        let x_mm = (*output).mm_width as f32;
-                        let y_mm = (*output).mm_height as f32;
-                        let x_px = (*crtc).width as f32;
-                        let y_px = (*crtc).height as f32;
-                        let ppmm = ((x_px * y_px) / (x_mm * y_mm)).sqrt();
-
-                        // Quantize 1/12 step size
-                        ((ppmm * (12.0 * 25.4 / 96.0)).round() / 12.0).max(1.0)
-                    };
-
-                    (x.xrandr.XRRFreeOutputInfo)(output);
-                    available.push(MonitorId{
-                        id: crtcid as u32,
-                        name,
-                        hidpi_factor,
-                        dimensions: ((*crtc).width as u32, (*crtc).height as u32),
-                        position: ((*crtc).x as i32, (*crtc).y as i32),
-                        primary: true,
-                    });
-                }
-                (x.xrandr.XRRFreeCrtcInfo)(crtc);
-            }
-        }
-        (x.xrandr.XRRFreeScreenResources)(resources);
-    }
-    available
-}
-
-#[inline]
-pub fn get_primary_monitor(x: &Arc<XConnection>) -> MonitorId {
-    get_available_monitors(x).into_iter().find(|m| m.primary)
-        // 'no primary' case is better handled picking some existing monitor
-        .or_else(|| get_available_monitors(x).into_iter().next())
-        .expect("[winit] Failed to find any x11 monitor")
 }
 
 impl MonitorId {
+    fn from_repr(
+        xconn: &Arc<XConnection>,
+        resources: *mut XRRScreenResources,
+        id: u32,
+        repr: util::MonitorRepr,
+        primary: bool,
+    ) -> Self {
+        let (name, hidpi_factor) = unsafe { util::get_output_info(xconn, resources, &repr) };
+        let (dimensions, position) = unsafe { (repr.get_dimensions(), repr.get_position()) };
+        let rect = util::Rect::new(position, dimensions);
+        MonitorId {
+            id,
+            name,
+            hidpi_factor,
+            dimensions,
+            position,
+            primary,
+            rect,
+        }
+    }
+
     pub fn get_name(&self) -> Option<String> {
         Some(self.name.clone())
     }
@@ -127,3 +89,153 @@ impl MonitorId {
         self.hidpi_factor
     }
 }
+
+pub fn get_monitor_for_window(
+    xconn: &Arc<XConnection>,
+    window_rect: Option<util::Rect>,
+) -> MonitorId {
+    let monitors = get_available_monitors(xconn);
+    let default = monitors
+        .get(0)
+        .expect("[winit] Failed to find any monitors using XRandR.");
+
+    let window_rect = match window_rect {
+        Some(rect) => rect,
+        None => return default.to_owned(),
+    };
+
+    let mut largest_overlap = 0;
+    let mut matched_monitor = default;
+    for monitor in &monitors {
+        let overlapping_area = window_rect.get_overlapping_area(&monitor.rect);
+        if overlapping_area > largest_overlap {
+            largest_overlap = overlapping_area;
+            matched_monitor = &monitor;
+        }
+    }
+
+    matched_monitor.to_owned()
+}
+
+fn query_monitor_list(xconn: &Arc<XConnection>) -> Vec<MonitorId> {
+    unsafe {
+        let root = (xconn.xlib.XDefaultRootWindow)(xconn.display);
+        // WARNING: this function is supposedly very slow, on the order of hundreds of ms.
+        // Upon failure, `resources` will be null.
+        let resources = (xconn.xrandr.XRRGetScreenResources)(xconn.display, root);
+        if resources.is_null() {
+            panic!("[winit] `XRRGetScreenResources` returned NULL. That should only happen if the root window doesn't exist.");
+        }
+
+        let mut available;
+
+        if xconn.xrandr_1_5.is_some() && !FORCE_RANDR_COMPAT {
+            // We're in XRandR >= 1.5, enumerate monitors. This supports things like MST and
+            // videowalls.
+            let xrandr_1_5 = xconn.xrandr_1_5.as_ref().unwrap();
+            let mut monitor_count = 0;
+            let monitors = (xrandr_1_5.XRRGetMonitors)(xconn.display, root, 1, &mut monitor_count);
+            available = Vec::with_capacity(monitor_count as usize);
+            for monitor_index in 0..monitor_count {
+                let monitor = monitors.offset(monitor_index as isize);
+                let is_primary = (*monitor).primary != 0;
+                available.push(MonitorId::from_repr(
+                    xconn,
+                    resources,
+                    monitor_index as u32,
+                    monitor.into(),
+                    is_primary,
+                ));
+            }
+            (xrandr_1_5.XRRFreeMonitors)(monitors);
+        } else {
+            // We're in XRandR < 1.5, enumerate CRTCs. Everything will work except MST and
+            // videowall setups will also show monitors that aren't in the logical groups the user
+            // cares about.
+            let primary = (xconn.xrandr.XRRGetOutputPrimary)(xconn.display, root);
+            available = Vec::with_capacity((*resources).ncrtc as usize);
+            for crtc_index in 0..(*resources).ncrtc {
+                let crtc_id = *((*resources).crtcs.offset(crtc_index as isize));
+                let crtc = (xconn.xrandr.XRRGetCrtcInfo)(xconn.display, resources, crtc_id);
+                let is_active = (*crtc).width > 0 && (*crtc).height > 0 && (*crtc).noutput > 0;
+                if is_active {
+                    let crtc = util::MonitorRepr::from(crtc);
+                    let is_primary = crtc.get_output() == primary;
+                    available.push(MonitorId::from_repr(
+                        xconn,
+                        resources,
+                        crtc_id as u32,
+                        crtc,
+                        is_primary,
+                    ));
+                }
+                (xconn.xrandr.XRRFreeCrtcInfo)(crtc);
+            }
+        }
+
+        (xconn.xrandr.XRRFreeScreenResources)(resources);
+        available
+    }
+}
+
+pub fn get_available_monitors(xconn: &Arc<XConnection>) -> Vec<MonitorId> {
+    let mut monitors_lock = MONITORS.lock();
+    (*monitors_lock)
+        .as_ref()
+        .cloned()
+        .or_else(|| {
+            let monitors = Some(query_monitor_list(xconn));
+            if !DISABLE_MONITOR_LIST_CACHING {
+                (*monitors_lock) = monitors.clone();
+            }
+            monitors
+        })
+        .unwrap()
+}
+
+#[inline]
+pub fn get_primary_monitor(x: &Arc<XConnection>) -> MonitorId {
+    let mut available_monitors = get_available_monitors(x).into_iter();
+    available_monitors
+        .find(|m| m.primary)
+        // If no monitors were detected as being primary, we just pick one ourselves!
+        .or_else(|| available_monitors.next())
+        .expect("[winit] Failed to find any monitors using XRandR.")
+}
+
+pub fn select_input(xconn: &Arc<XConnection>, root: Window) -> Result<c_int, XError> {
+    let mut major = 0;
+    let mut minor = 0;
+    let has_extension = unsafe {
+        (xconn.xrandr.XRRQueryExtension)(
+            xconn.display,
+            &mut major,
+            &mut minor,
+        )
+    };
+    if has_extension != True {
+        panic!("[winit] XRandR extension not available.");
+    }
+
+    let mut event_offset = 0;
+    let mut error_offset = 0;
+    let status = unsafe {
+        (xconn.xrandr.XRRQueryExtension)(
+            xconn.display,
+            &mut event_offset,
+            &mut error_offset,
+        )
+    };
+
+    if status != True {
+        xconn.check_errors()?;
+        unreachable!("[winit] `XRRQueryExtension` failed but no error was received.");
+    }
+
+    let mask = RRCrtcChangeNotifyMask
+        | RROutputPropertyNotifyMask
+        | RRScreenChangeNotifyMask;
+    unsafe { (xconn.xrandr.XRRSelectInput)(xconn.display, root, mask) };
+
+    Ok(event_offset)
+}

src/platform/linux/x11/util/geometry.rs

@@ -1,5 +1,40 @@
+use std::cmp;
+
 use super::*;
 
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct Rect {
+    left: i64,
+    right: i64,
+    top: i64,
+    bottom: i64,
+}
+
+impl Rect {
+    pub fn new((x, y): (i32, i32), (width, height): (u32, u32)) -> Self {
+        let (x, y) = (x as i64, y as i64);
+        let (width, height) = (width as i64, height as i64);
+        Rect {
+            left: x,
+            right: x + width,
+            top: y,
+            bottom: y + height,
+        }
+    }
+
+    pub fn get_overlapping_area(&self, other: &Self) -> i64 {
+        let x_overlap = cmp::max(
+            0,
+            cmp::min(self.right, other.right) - cmp::max(self.left, other.left),
+        );
+        let y_overlap = cmp::max(
+            0,
+            cmp::min(self.bottom, other.bottom) - cmp::max(self.top, other.top),
+        );
+        x_overlap * y_overlap
+    }
+}
+
 #[derive(Debug)]
 pub struct TranslatedCoords {
     pub x_rel_root: c_int,

src/platform/linux/x11/util/mod.rs

@@ -6,6 +6,7 @@ mod geometry;
 mod hint;
 mod icon;
 mod input;
+mod randr;
 mod window_property;
 mod wm;
 
@@ -14,6 +15,7 @@ pub use self::geometry::*;
 pub use self::hint::*;
 pub use self::icon::*;
 pub use self::input::*;
+pub use self::randr::*;
 pub use self::window_property::*;
 pub use self::wm::*;
 

src/platform/linux/x11/util/randr.rs

@@ -0,0 +1,84 @@
+use std::slice;
+
+use super::*;
+use super::ffi::{
+    RROutput,
+    XRRCrtcInfo,
+    XRRMonitorInfo,
+    XRRScreenResources,
+};
+
+pub enum MonitorRepr {
+    Monitor(*mut XRRMonitorInfo),
+    Crtc(*mut XRRCrtcInfo),
+}
+
+impl MonitorRepr {
+    pub unsafe fn get_output(&self) -> RROutput {
+        match *self {
+            // Same member names, but different locations within the struct...
+            MonitorRepr::Monitor(monitor) => *((*monitor).outputs.offset(0)),
+            MonitorRepr::Crtc(crtc) => *((*crtc).outputs.offset(0)),
+        }
+    }
+
+    pub unsafe fn get_dimensions(&self) -> (u32, u32) {
+        match *self {
+            MonitorRepr::Monitor(monitor) => ((*monitor).width as u32, (*monitor).height as u32),
+            MonitorRepr::Crtc(crtc) => ((*crtc).width as u32, (*crtc).height as u32),
+        }
+    }
+
+    pub unsafe fn get_position(&self) -> (i32, i32) {
+        match *self {
+            MonitorRepr::Monitor(monitor) => ((*monitor).x as i32, (*monitor).y as i32),
+            MonitorRepr::Crtc(crtc) => ((*crtc).x as i32, (*crtc).y as i32),
+        }
+    }
+}
+
+impl From<*mut XRRMonitorInfo> for MonitorRepr {
+    fn from(monitor: *mut XRRMonitorInfo) -> Self {
+        MonitorRepr::Monitor(monitor)
+    }
+}
+
+impl From<*mut XRRCrtcInfo> for MonitorRepr {
+    fn from(crtc: *mut XRRCrtcInfo) -> Self {
+        MonitorRepr::Crtc(crtc)
+    }
+}
+
+pub fn calc_dpi_factor(
+    (width_px, height_px): (u32, u32),
+    (width_mm, height_mm): (u64, u64),
+) -> f64 {
+    let ppmm = (
+        (width_px as f64 * height_px as f64) / (width_mm as f64 * height_mm as f64)
+    ).sqrt();
+    // Quantize 1/12 step size
+    ((ppmm * (12.0 * 25.4 / 96.0)).round() / 12.0).max(1.0)
+}
+
+pub unsafe fn get_output_info(
+    xconn: &Arc<XConnection>,
+    resources: *mut XRRScreenResources,
+    repr: &MonitorRepr,
+) -> (String, f32) {
+    let output_info = (xconn.xrandr.XRRGetOutputInfo)(
+        xconn.display,
+        resources,
+        repr.get_output(),
+    );
+    let name_slice = slice::from_raw_parts(
+        (*output_info).name as *mut u8,
+        (*output_info).nameLen as usize,
+    );
+    let name = String::from_utf8_lossy(name_slice).into();
+    let hidpi_factor = calc_dpi_factor(
+        repr.get_dimensions(),
+        ((*output_info).mm_width as u64, (*output_info).mm_height as u64),
+    ) as f32;
+    (xconn.xrandr.XRRFreeOutputInfo)(output_info);
+    (name, hidpi_factor)
+}

src/platform/linux/x11/window.rs

@@ -12,7 +12,7 @@ use CreationError::{self, OsError};
 use platform::MonitorId as PlatformMonitorId;
 use platform::PlatformSpecificWindowBuilderAttributes;
 use platform::x11::MonitorId as X11MonitorId;
-use platform::x11::monitor::get_available_monitors;
+use platform::x11::monitor::get_monitor_for_window;
 use window::MonitorId as RootMonitorId;
 
 use super::{ffi, util, XConnection, XError, WindowId, EventsLoop};
@@ -395,41 +395,17 @@ impl Window2 {
         self.invalidate_cached_frame_extents();
     }
 
+    fn get_rect(&self) -> Option<util::Rect> {
+        // TODO: This might round-trip more times than needed.
+        if let (Some(position), Some(size)) = (self.get_position(), self.get_outer_size()) {
+            Some(util::Rect::new(position, size))
+        } else {
+            None
+        }
+    }
+
     pub fn get_current_monitor(&self) -> X11MonitorId {
-        let monitors = get_available_monitors(&self.x.display);
+        get_monitor_for_window(&self.x.display, self.get_rect()).to_owned()
-        let default = monitors[0].clone();
-
-        let (wx,wy) = match self.get_position() {
-            Some(val) => (cmp::max(0,val.0) as u32, cmp::max(0,val.1) as u32),
-            None=> return default,
-        };
-        let (ww,wh) = match self.get_outer_size() {
-            Some(val) => val,
-            None=> return default,
-        };
-        // Opposite corner coordinates
-        let (wxo, wyo) = (wx+ww-1, wy+wh-1);
-
-        // Find the monitor with the biggest overlap with the window
-        let mut overlap = 0;
-        let mut find = default;
-        for monitor in monitors {
-            let (mx, my) = monitor.get_position();
-            let mx = mx as u32;
-            let my = my as u32;
-            let (mw, mh) = monitor.get_dimensions();
-            let (mxo, myo) = (mx+mw-1, my+mh-1);
-            let (ox, oy) = (cmp::max(wx, mx), cmp::max(wy, my));
-            let (oxo, oyo) = (cmp::min(wxo, mxo), cmp::min(wyo, myo));
-            let osize = if ox <= oxo || oy <= oyo { 0 } else { (oxo-ox)*(oyo-oy) };
-
-            if osize > overlap {
-                overlap = osize;
-                find = monitor;
-            }
-        }
-
-        find
     }
 
     fn set_maximized_inner(&self, maximized: bool) -> util::Flusher {
@@ -663,13 +639,13 @@ impl Window2 {
     }
 
     #[inline]
-    pub fn set_inner_size(&self, x: u32, y: u32) {
+    pub fn set_inner_size(&self, width: u32, height: u32) {
         unsafe {
             (self.x.display.xlib.XResizeWindow)(
                 self.x.display.display,
                 self.x.window,
-                x as c_uint,
+                width as c_uint,
-                y as c_uint,
+                height as c_uint,
             );
             util::flush_requests(&self.x.display)
         }.expect("Failed to call XResizeWindow");
@@ -972,14 +948,7 @@ impl Window2 {
     }
 
     pub fn hidpi_factor(&self) -> f32 {
-        unsafe {
+        self.get_current_monitor().hidpi_factor
-            let x_px = (self.x.display.xlib.XDisplayWidth)(self.x.display.display, self.x.screen_id);
-            let y_px = (self.x.display.xlib.XDisplayHeight)(self.x.display.display, self.x.screen_id);
-            let x_mm = (self.x.display.xlib.XDisplayWidthMM)(self.x.display.display, self.x.screen_id);
-            let y_mm = (self.x.display.xlib.XDisplayHeightMM)(self.x.display.display, self.x.screen_id);
-            let ppmm = ((x_px as f32 * y_px as f32) / (x_mm as f32 * y_mm as f32)).sqrt();
-            ((ppmm * (12.0 * 25.4 / 96.0)).round() / 12.0).max(1.0) // quantize with 1/12 step size.
-        }
     }
 
     pub fn set_cursor_position(&self, x: i32, y: i32) -> Result<(), ()> {

src/platform/macos/monitor.rs

@@ -3,6 +3,7 @@ use cocoa::base::{id, nil};
 use cocoa::foundation::{NSString, NSUInteger};
 use core_graphics::display::{CGDirectDisplayID, CGDisplay};
 use std::collections::VecDeque;
+use std::fmt;
 use super::EventsLoop;
 use super::window::IdRef;
 
@@ -32,6 +33,29 @@ impl EventsLoop {
     }
 }
 
+impl fmt::Debug for MonitorId {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        #[derive(Debug)]
+        struct MonitorId {
+            name: Option<String>,
+            native_identifier: u32,
+            dimensions: (u32, u32),
+            position: &'static str,
+            hidpi_factor: f32,
+        }
+
+        let monitor_id_proxy = MonitorId {
+            name: self.get_name(),
+            native_identifier: self.get_native_identifier(),
+            dimensions: self.get_dimensions(),
+            position: "WARNING: `MonitorId::get_position` is unimplemented on macOS!",
+            hidpi_factor: self.get_hidpi_factor(),
+        };
+
+        monitor_id_proxy.fmt(f)
+    }
+}
+
 impl MonitorId {
     pub fn get_name(&self) -> Option<String> {
         let MonitorId(display_id) = *self;

src/platform/windows/monitor.rs

@@ -9,7 +9,7 @@ use std::{mem, ptr};
 use super::{EventsLoop, util};
 
 /// Win32 implementation of the main `MonitorId` object.
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub struct MonitorId {
     /// The system name of the adapter.
     adapter_name: [wchar_t; 32],
@@ -39,7 +39,7 @@ pub struct MonitorId {
 // For more info see:
 // https://github.com/retep998/winapi-rs/issues/360
 // https://github.com/retep998/winapi-rs/issues/396
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 struct HMonitor(HMONITOR);
 
 unsafe impl Send for HMonitor {}

src/window.rs

@@ -409,7 +409,7 @@ impl Iterator for AvailableMonitorsIter {
 }
 
 /// Identifier for a monitor.
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub struct MonitorId {
     pub(crate) inner: platform::MonitorId
 }