X11+Windows: Guess initial DPI factor (#583)

* X11: Guess initial DPI factor

* Windows: Guess initial DPI factor

.gitignore

@@ -1,5 +1,6 @@
 Cargo.lock
 target/
 rls/
+.vscode/
 *~
 #*#

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Unreleased
 
+- Added logging through `log`. Logging will become more extensive over time.
+- On X11 and Windows, the window's DPI factor is guessed before creating the window. This *greatly* cuts back on unsightly auto-resizing that would occur immediately after window creation.
+
 # Version 0.16.0 (2018-06-25)
 
 - Windows additionally has `WindowBuilderExt::with_no_redirection_bitmap`.

Cargo.toml

@@ -19,6 +19,7 @@ icon_loading = ["image"]
 [dependencies]
 lazy_static = "1"
 libc = "0.2"
+log = "0.4"
 image = { version = "0.19", optional = true }
 
 [target.'cfg(target_os = "android")'.dependencies.android_glue]

src/lib.rs

@@ -88,6 +88,8 @@
 #[macro_use]
 extern crate lazy_static;
 extern crate libc;
+#[macro_use]
+extern crate log;
 #[cfg(feature = "icon_loading")]
 extern crate image;
 

src/platform/linux/x11/mod.rs

@@ -482,21 +482,21 @@ impl EventsLoop {
                         shared_state_lock.position.unwrap()
                     };
 
+                    if is_synthetic {
                         // If we don't use the existing adjusted value when available, then the user can screw up the
                         // resizing by dragging across monitors *without* dropping the window.
                         let (width, height) = shared_state_lock.dpi_adjusted
                             .unwrap_or_else(|| (xev.width as f64, xev.height as f64));
-                    let last_hidpi_factor = if shared_state_lock.is_new_window {
+                        let last_hidpi_factor = shared_state_lock.guessed_dpi
-                        shared_state_lock.is_new_window = false;
+                            .take()
-                        1.0
+                            .unwrap_or_else(|| {
-                    } else {
                                 shared_state_lock.last_monitor
                                     .as_ref()
                                     .map(|last_monitor| last_monitor.hidpi_factor)
                                     .unwrap_or(1.0)
-                    };
+                            });
                         let new_hidpi_factor = {
-                        let window_rect = util::Rect::new(new_outer_position, new_inner_size);
+                            let window_rect = util::AaRect::new(new_outer_position, new_inner_size);
                             let monitor = self.xconn.get_monitor_for_window(Some(window_rect));
                             let new_hidpi_factor = monitor.hidpi_factor;
                             shared_state_lock.last_monitor = Some(monitor);
@@ -513,6 +513,7 @@ impl EventsLoop {
                             flusher.queue();
                             shared_state_lock.dpi_adjusted = Some((new_width, new_height));
                         }
+                    }
 
                     events
                 });
@@ -592,7 +593,7 @@ impl EventsLoop {
 
                 // Standard virtual core keyboard ID. XInput2 needs to be used to get a reliable
                 // value, though this should only be an issue under multiseat configurations.
-                let device = 3;
+                let device = util::VIRTUAL_CORE_KEYBOARD;
                 let device_id = mkdid(device);
 
                 // When a compose sequence or IME pre-edit is finished, it ends in a KeyPress with

src/platform/linux/x11/monitor.rs

@@ -55,7 +55,7 @@ pub struct MonitorId {
     /// The DPI scale factor
     pub(crate) hidpi_factor: f64,
     /// Used to determine which windows are on this monitor
-    pub(crate) rect: util::Rect,
+    pub(crate) rect: util::AaRect,
 }
 
 impl MonitorId {
@@ -68,7 +68,7 @@ impl MonitorId {
     ) -> Self {
         let (name, hidpi_factor) = unsafe { xconn.get_output_info(resources, &repr) };
         let (dimensions, position) = unsafe { (repr.get_dimensions(), repr.get_position()) };
-        let rect = util::Rect::new(position, dimensions);
+        let rect = util::AaRect::new(position, dimensions);
         MonitorId {
             id,
             name,
@@ -104,7 +104,7 @@ impl MonitorId {
 }
 
 impl XConnection {
-    pub fn get_monitor_for_window(&self, window_rect: Option<util::Rect>) -> MonitorId {
+    pub fn get_monitor_for_window(&self, window_rect: Option<util::AaRect>) -> MonitorId {
         let monitors = self.get_available_monitors();
         let default = monitors
             .get(0)

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

@@ -3,34 +3,34 @@ use std::cmp;
 use super::*;
 use {LogicalPosition, LogicalSize};
 
+// Friendly neighborhood axis-aligned rectangle
 #[derive(Debug, Clone, PartialEq, Eq)]
-pub struct Rect {
+pub struct AaRect {
-    left: i64,
+    x: i64,
-    right: i64,
+    y: i64,
-    top: i64,
+    width: i64,
-    bottom: i64,
+    height: i64,
 }
 
-impl Rect {
+impl AaRect {
     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 {
+        AaRect { x, y, width, height }
-            left: x,
-            right: x + width,
-            top: y,
-            bottom: y + height,
     }
+
+    pub fn contains_point(&self, x: i64, y: i64) -> bool {
+        x >= self.x && x <= self.x + self.width && y >= self.y && y <= self.y + self.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),
+            cmp::min(self.x + self.width, other.x + other.width) - cmp::max(self.x, other.x),
         );
         let y_overlap = cmp::max(
             0,
-            cmp::min(self.bottom, other.bottom) - cmp::max(self.top, other.top),
+            cmp::min(self.y + self.height, other.y + other.height) - cmp::max(self.y, other.y),
         );
         x_overlap * y_overlap
     }

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

@@ -3,6 +3,9 @@ use std::str;
 use super::*;
 use events::ModifiersState;
 
+pub const VIRTUAL_CORE_POINTER: c_int = 2;
+pub const VIRTUAL_CORE_KEYBOARD: c_int = 3;
+
 // A base buffer size of 1kB uses a negligible amount of RAM while preventing us from having to
 // re-allocate (and make another round-trip) in the *vast* majority of cases.
 // To test if `lookup_utf8` works correctly, set this to 1.
@@ -24,8 +27,8 @@ pub struct PointerState<'a> {
     xconn: &'a XConnection,
     root: ffi::Window,
     child: ffi::Window,
-    root_x: c_double,
+    pub root_x: c_double,
-    root_y: c_double,
+    pub root_y: c_double,
     win_x: c_double,
     win_y: c_double,
     buttons: ffi::XIButtonState,

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

@@ -24,6 +24,7 @@ pub fn calc_dpi_factor(
 
     // See http://xpra.org/trac/ticket/728 for more information.
     if width_mm == 0 || width_mm == 0 {
+        warn!("XRandR reported that the display's 0mm in size, which is certifiably insane");
         return 1.0;
     }
 

src/platform/linux/x11/window.rs

@@ -29,13 +29,12 @@ unsafe extern "C" fn visibility_predicate(
 
 #[derive(Debug, Default)]
 pub struct SharedState {
-    // Window creation assumes a DPI factor of 1.0, so we use this flag to handle that special case.
-    pub is_new_window: bool,
     pub cursor_pos: Option<(f64, f64)>,
     pub size: Option<(u32, u32)>,
     pub position: Option<(i32, i32)>,
     pub inner_position: Option<(i32, i32)>,
     pub inner_position_rel_parent: Option<(i32, i32)>,
+    pub guessed_dpi: Option<f64>,
     pub last_monitor: Option<X11MonitorId>,
     pub dpi_adjusted: Option<(f64, f64)>,
     // Used to restore position after exiting fullscreen.
@@ -46,9 +45,9 @@ pub struct SharedState {
 }
 
 impl SharedState {
-    fn new() -> Mutex<Self> {
+    fn new(dpi_factor: f64) -> Mutex<Self> {
         let mut shared_state = SharedState::default();
-        shared_state.is_new_window = true;
+        shared_state.guessed_dpi = Some(dpi_factor);
         Mutex::new(shared_state)
     }
 }
@@ -78,15 +77,51 @@ impl UnownedWindow {
         let xconn = &event_loop.xconn;
         let root = event_loop.root;
 
-        let max_dimensions: Option<(u32, u32)> = window_attrs.max_dimensions.map(Into::into);
+        let monitors = xconn.get_available_monitors();
-        let min_dimensions: Option<(u32, u32)> = window_attrs.min_dimensions.map(Into::into);
+        let dpi_factor = if !monitors.is_empty() {
+            let mut dpi_factor = Some(monitors[0].get_hidpi_factor());
+            for monitor in &monitors {
+                if Some(monitor.get_hidpi_factor()) != dpi_factor {
+                    dpi_factor = None;
+                }
+            }
+            dpi_factor.unwrap_or_else(|| {
+                xconn.query_pointer(root, util::VIRTUAL_CORE_POINTER)
+                    .ok()
+                    .and_then(|pointer_state| {
+                        let (x, y) = (pointer_state.root_x as i64, pointer_state.root_y as i64);
+                        let mut dpi_factor = None;
+                        for monitor in &monitors {
+                            if monitor.rect.contains_point(x, y) {
+                                dpi_factor = Some(monitor.get_hidpi_factor());
+                                break;
+                            }
+                        }
+                        dpi_factor
+                    })
+                    .unwrap_or(1.0)
+            })
+        } else {
+            unreachable!("There are no detected monitors, which should've already caused a panic.");
+        };
+
+        info!("Guessed window DPI factor: {}", dpi_factor);
+
+        let max_dimensions: Option<(u32, u32)> = window_attrs.max_dimensions.map(|size| {
+            size.to_physical(dpi_factor).into()
+        });
+        let min_dimensions: Option<(u32, u32)> = window_attrs.min_dimensions.map(|size| {
+            size.to_physical(dpi_factor).into()
+        });
 
         let dimensions = {
             // x11 only applies constraints when the window is actively resized
             // by the user, so we have to manually apply the initial constraints
-            let mut dimensions = window_attrs.dimensions
+            let mut dimensions: (u32, u32) = window_attrs.dimensions
+                .or_else(|| Some((800, 600).into()))
+                .map(|size| size.to_physical(dpi_factor))
                 .map(Into::into)
-                .unwrap_or((800, 600));
+                .unwrap();
             if let Some(max) = max_dimensions {
                 dimensions.0 = cmp::min(dimensions.0, max.0);
                 dimensions.1 = cmp::min(dimensions.1, max.1);
@@ -95,6 +130,7 @@ impl UnownedWindow {
                 dimensions.0 = cmp::max(dimensions.0, min.0);
                 dimensions.1 = cmp::max(dimensions.1, min.1);
             }
+            debug!("Calculated physical dimensions: {}x{}", dimensions.0, dimensions.1);
             dimensions
         };
 
@@ -166,7 +202,7 @@ impl UnownedWindow {
             cursor_hidden: Default::default(),
             ime_sender: Mutex::new(event_loop.ime_sender.clone()),
             multitouch: window_attrs.multitouch,
-            shared_state: SharedState::new(),
+            shared_state: SharedState::new(dpi_factor),
         };
 
         // Title must be set before mapping. Some tiling window managers (i.e. i3) use the window
@@ -240,7 +276,10 @@ impl UnownedWindow {
             {
                 let mut min_dimensions = window_attrs.min_dimensions;
                 let mut max_dimensions = window_attrs.max_dimensions;
-                if !window_attrs.resizable && !util::wm_name_is_one_of(&["Xfwm4"]) {
+                if !window_attrs.resizable {
+                    if util::wm_name_is_one_of(&["Xfwm4"]) {
+                        warn!("To avoid a WM bug, disabling resizing has no effect on Xfwm4");
+                    } else {
                         max_dimensions = Some(dimensions.into());
                         min_dimensions = Some(dimensions.into());
 
@@ -248,6 +287,7 @@ impl UnownedWindow {
                         shared_state_lock.min_dimensions = window_attrs.min_dimensions;
                         shared_state_lock.max_dimensions = window_attrs.max_dimensions;
                     }
+                }
 
                 let mut normal_hints = util::NormalHints::new(xconn);
                 normal_hints.set_size(Some(dimensions));
@@ -482,10 +522,10 @@ impl UnownedWindow {
         self.invalidate_cached_frame_extents();
     }
 
-    fn get_rect(&self) -> Option<util::Rect> {
+    fn get_rect(&self) -> Option<util::AaRect> {
         // TODO: This might round-trip more times than needed.
         if let (Some(position), Some(size)) = (self.get_position_physical(), self.get_outer_size_physical()) {
-            Some(util::Rect::new(position, size))
+            Some(util::AaRect::new(position, size))
         } else {
             None
         }
@@ -853,6 +893,7 @@ impl UnownedWindow {
             // Making the window unresizable on Xfwm prevents further changes to `WM_NORMAL_HINTS` from being detected.
             // This makes it impossible for resizing to be re-enabled, and also breaks DPI scaling. As such, we choose
             // the lesser of two evils and do nothing.
+            warn!("To avoid a WM bug, disabling resizing has no effect on Xfwm4");
             return;
         }
 

src/platform/windows/monitor.rs

@@ -1,5 +1,6 @@
 use winapi::shared::minwindef::{BOOL, DWORD, LPARAM, TRUE};
 use winapi::shared::windef::{HDC, HMONITOR, HWND, LPRECT, POINT};
+use winapi::um::winnt::LONG;
 use winapi::um::winuser;
 
 use std::{mem, ptr};
@@ -49,7 +50,7 @@ unsafe extern "system" fn monitor_enum_proc(
     TRUE // continue enumeration
 }
 
-fn get_available_monitors() -> VecDeque<MonitorId> {
+pub fn get_available_monitors() -> VecDeque<MonitorId> {
     let mut monitors: VecDeque<MonitorId> = VecDeque::new();
     unsafe {
         winuser::EnumDisplayMonitors(
@@ -62,7 +63,7 @@ fn get_available_monitors() -> VecDeque<MonitorId> {
     monitors
 }
 
-fn get_primary_monitor() -> MonitorId {
+pub fn get_primary_monitor() -> MonitorId {
     const ORIGIN: POINT = POINT { x: 0, y: 0 };
     let hmonitor = unsafe {
         winuser::MonitorFromPoint(ORIGIN, winuser::MONITOR_DEFAULTTOPRIMARY)
@@ -132,6 +133,14 @@ impl MonitorId {
         }
     }
 
+    pub(crate) fn contains_point(&self, point: &POINT) -> bool {
+        let left = self.position.0 as LONG;
+        let right = left + self.dimensions.0 as LONG;
+        let top = self.position.1 as LONG;
+        let bottom = top + self.dimensions.1 as LONG;
+        point.x >= left && point.x <= right && point.y >= top && point.y <= bottom
+    }
+
     #[inline]
     pub fn get_name(&self) -> Option<String> {
         Some(self.monitor_name.clone())

src/platform/windows/util.rs

@@ -2,8 +2,8 @@ use std::{self, mem, ptr, slice};
 use std::ops::BitAnd;
 
 use winapi::ctypes::wchar_t;
-use winapi::shared::minwindef::DWORD;
+use winapi::shared::minwindef::{BOOL, DWORD};
-use winapi::shared::windef::{HWND, RECT};
+use winapi::shared::windef::{HWND, POINT, RECT};
 use winapi::um::errhandlingapi::GetLastError;
 use winapi::um::winbase::{
     FormatMessageW,
@@ -38,15 +38,23 @@ pub fn wchar_ptr_to_string(wchar: *const wchar_t) -> String {
     wchar_to_string(wchar_slice)
 }
 
-pub fn get_window_rect(hwnd: HWND) -> Option<RECT> {
+pub unsafe fn status_map<T, F: FnMut(&mut T) -> BOOL>(mut fun: F) -> Option<T> {
-    let mut rect: RECT = unsafe { mem::uninitialized() };
+    let mut data: T = mem::uninitialized();
-    if unsafe { winuser::GetWindowRect(hwnd, &mut rect) } != 0 {
+    if fun(&mut data) != 0 {
-        Some(rect)
+        Some(data)
     } else {
         None
     }
 }
 
+pub fn get_cursor_pos() -> Option<POINT> {
+    unsafe { status_map(|cursor_pos| winuser::GetCursorPos(cursor_pos)) }
+}
+
+pub fn get_window_rect(hwnd: HWND) -> Option<RECT> {
+    unsafe { status_map(|rect| winuser::GetWindowRect(hwnd, rect)) }
+}
+
 // This won't be needed anymore if we just add a derive to winapi.
 pub fn rect_eq(a: &RECT, b: &RECT) -> bool {
     let left_eq = a.left == b.left;

src/platform/windows/window.rs

@@ -26,9 +26,10 @@ use {
     WindowAttributes,
 };
 use platform::platform::{Cursor, PlatformSpecificWindowBuilderAttributes, WindowId};
-use platform::platform::dpi::{BASE_DPI, dpi_to_scale_factor, get_window_dpi, get_window_scale_factor};
+use platform::platform::dpi::{dpi_to_scale_factor, get_window_dpi, get_window_scale_factor};
 use platform::platform::events_loop::{self, DESTROY_MSG_ID, EventsLoop, INITIAL_DPI_MSG_ID};
 use platform::platform::icon::{self, IconType, WinIcon};
+use platform::platform::monitor::get_available_monitors;
 use platform::platform::raw_input::register_all_mice_and_keyboards_for_raw_input;
 use platform::platform::util;
 
@@ -858,9 +859,38 @@ unsafe fn init(
     // registering the window class
     let class_name = register_window_class(&window_icon, &taskbar_icon);
 
-    let (width, height) = attributes.dimensions
+    let guessed_dpi_factor = {
-        .map(Into::into)
+        let monitors = get_available_monitors();
-        .unwrap_or((1024, 768));
+        let dpi_factor = if !monitors.is_empty() {
+            let mut dpi_factor = Some(monitors[0].get_hidpi_factor());
+            for monitor in &monitors {
+                if Some(monitor.get_hidpi_factor()) != dpi_factor {
+                    dpi_factor = None;
+                }
+            }
+            dpi_factor
+        } else {
+            unreachable!("There are no detected monitors, which should've already caused a panic.");
+        };
+        dpi_factor.unwrap_or_else(|| {
+            util::get_cursor_pos()
+                .and_then(|cursor_pos| {
+                    let mut dpi_factor = None;
+                    for monitor in &monitors {
+                        if monitor.contains_point(&cursor_pos) {
+                            dpi_factor = Some(monitor.get_hidpi_factor());
+                            break;
+                        }
+                    }
+                    dpi_factor
+                })
+                .unwrap_or(1.0)
+        })
+    };
+    info!("Guessed window DPI factor: {}", guessed_dpi_factor);
+
+    let dimensions = attributes.dimensions.unwrap_or_else(|| (1024, 768).into());
+    let (width, height): (u32, u32) = dimensions.to_physical(guessed_dpi_factor).into();
     // building a RECT object with coordinates
     let mut rect = RECT {
         left: 0,
@@ -899,11 +929,11 @@ unsafe fn init(
     let real_window = {
         let (adjusted_width, adjusted_height) = if attributes.dimensions.is_some() {
             let min_dimensions = attributes.min_dimensions
-                .map(|logical_size| PhysicalSize::from_logical(logical_size, 1.0))
+                .map(|logical_size| PhysicalSize::from_logical(logical_size, guessed_dpi_factor))
                 .map(|physical_size| adjust_size(physical_size, style, ex_style))
                 .unwrap_or((0, 0));
             let max_dimensions = attributes.max_dimensions
-                .map(|logical_size| PhysicalSize::from_logical(logical_size, 1.0))
+                .map(|logical_size| PhysicalSize::from_logical(logical_size, guessed_dpi_factor))
                 .map(|physical_size| adjust_size(physical_size, style, ex_style))
                 .unwrap_or((c_int::max_value(), c_int::max_value()));
             (
@@ -968,7 +998,8 @@ unsafe fn init(
 
     let dpi = get_window_dpi(real_window.0, real_window.1);
     let dpi_factor = dpi_to_scale_factor(dpi);
-    if dpi != BASE_DPI {
+    if dpi_factor != guessed_dpi_factor {
+        let (width, height): (u32, u32) = dimensions.into();
         let mut packed_dimensions = 0;
         // MAKELPARAM isn't provided by winapi yet.
         let ptr = &mut packed_dimensions as *mut LPARAM as *mut WORD;