From 9828f368d6b75b5928274e5f72d909b99469cad0 Mon Sep 17 00:00:00 2001 From: Murarth Date: Sun, 10 Nov 2019 13:55:29 -0700 Subject: [PATCH] X11: Fix misreporting DPI factor at startup (#1252) * X11: Fix misreporting DPI factor at startup * Add a CHANGELOG entry --- CHANGELOG.md | 1 + .../linux/x11/event_processor.rs | 12 +- src/platform_impl/linux/x11/monitor.rs | 2 +- src/platform_impl/linux/x11/window.rs | 103 +++++++----------- 4 files changed, 46 insertions(+), 72 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c55d004..68850199 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Unreleased - On macOS, fix application termination on `ControlFlow::Exit` +- On X11, fix misreporting DPI factor at startup. - On X11, fix events not being reported when using `run_return`. - On X11, fix key modifiers being incorrectly reported. diff --git a/src/platform_impl/linux/x11/event_processor.rs b/src/platform_impl/linux/x11/event_processor.rs index bce77e8e..cf613744 100644 --- a/src/platform_impl/linux/x11/event_processor.rs +++ b/src/platform_impl/linux/x11/event_processor.rs @@ -382,14 +382,8 @@ impl EventProcessor { let (width, height) = shared_state_lock .dpi_adjusted .unwrap_or_else(|| (xev.width as f64, xev.height as f64)); - let last_hidpi_factor = - shared_state_lock.guessed_dpi.take().unwrap_or_else(|| { - shared_state_lock - .last_monitor - .as_ref() - .map(|last_monitor| last_monitor.hidpi_factor) - .unwrap_or(1.0) - }); + + let last_hidpi_factor = shared_state_lock.last_monitor.hidpi_factor; let new_hidpi_factor = { let window_rect = util::AaRect::new(new_outer_position, new_inner_size); monitor = wt.xconn.get_monitor_for_window(Some(window_rect)); @@ -397,7 +391,7 @@ impl EventProcessor { // Avoid caching an invalid dummy monitor handle if monitor.id != 0 { - shared_state_lock.last_monitor = Some(monitor.clone()); + shared_state_lock.last_monitor = monitor.clone(); } new_hidpi_factor }; diff --git a/src/platform_impl/linux/x11/monitor.rs b/src/platform_impl/linux/x11/monitor.rs index bb75e806..1fe2fd20 100644 --- a/src/platform_impl/linux/x11/monitor.rs +++ b/src/platform_impl/linux/x11/monitor.rs @@ -130,7 +130,7 @@ impl MonitorHandle { }) } - fn dummy() -> Self { + pub fn dummy() -> Self { MonitorHandle { id: 0, name: "".into(), diff --git a/src/platform_impl/linux/x11/window.rs b/src/platform_impl/linux/x11/window.rs index 305414f1..8df107bc 100644 --- a/src/platform_impl/linux/x11/window.rs +++ b/src/platform_impl/linux/x11/window.rs @@ -38,15 +38,14 @@ unsafe extern "C" fn visibility_predicate( (event.window == window && event.type_ == ffi::VisibilityNotify) as _ } -#[derive(Debug, Default)] +#[derive(Debug)] pub struct SharedState { 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, - pub last_monitor: Option, + pub last_monitor: X11MonitorHandle, pub dpi_adjusted: Option<(f64, f64)>, pub fullscreen: Option, // Used to restore position after exiting fullscreen @@ -60,11 +59,24 @@ pub struct SharedState { } impl SharedState { - fn new(dpi_factor: f64, is_visible: bool) -> Mutex { - let mut shared_state = SharedState::default(); - shared_state.guessed_dpi = Some(dpi_factor); - shared_state.is_visible = is_visible; - Mutex::new(shared_state) + fn new(last_monitor: X11MonitorHandle, is_visible: bool) -> Mutex { + Mutex::new(SharedState { + last_monitor, + is_visible, + + cursor_pos: None, + size: None, + position: None, + inner_position: None, + inner_position_rel_parent: None, + dpi_adjusted: None, + fullscreen: None, + restore_position: None, + desktop_video_mode: None, + frame_extents: None, + min_inner_size: None, + max_inner_size: None, + }) } } @@ -93,34 +105,27 @@ impl UnownedWindow { let xconn = &event_loop.xconn; let root = event_loop.root; - let monitors = xconn.available_monitors(); - let dpi_factor = if !monitors.is_empty() { - let mut dpi_factor = Some(monitors[0].hidpi_factor()); - for monitor in &monitors { - if Some(monitor.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.hidpi_factor()); - break; - } - } - dpi_factor - }) - .unwrap_or(1.0) - }) + let mut monitors = xconn.available_monitors(); + let guessed_monitor = if monitors.is_empty() { + X11MonitorHandle::dummy() } else { - 1.0 + 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); + + for i in 0..monitors.len() { + if monitors[i].rect.contains_point(x, y) { + return Some(monitors.swap_remove(i)); + } + } + + None + }) + .unwrap_or_else(|| monitors.swap_remove(0)) }; + let dpi_factor = guessed_monitor.hidpi_factor(); info!("Guessed window DPI factor: {}", dpi_factor); @@ -232,7 +237,7 @@ impl UnownedWindow { cursor_grabbed: Mutex::new(false), cursor_visible: Mutex::new(true), ime_sender: Mutex::new(event_loop.ime_sender.clone()), - shared_state: SharedState::new(dpi_factor, window_attrs.visible), + shared_state: SharedState::new(guessed_monitor, window_attrs.visible), pending_redraws: event_loop.pending_redraws.clone(), }; @@ -698,24 +703,9 @@ impl UnownedWindow { } } - fn get_rect(&self) -> util::AaRect { - // TODO: This might round-trip more times than needed. - let position = self.outer_position_physical(); - let size = self.outer_size_physical(); - util::AaRect::new(position, size) - } - #[inline] pub fn current_monitor(&self) -> X11MonitorHandle { - let monitor = self.shared_state.lock().last_monitor.as_ref().cloned(); - monitor.unwrap_or_else(|| { - let monitor = self.xconn.get_monitor_for_window(Some(self.get_rect())); - // Avoid caching an invalid dummy monitor handle - if monitor.id != 0 { - self.shared_state.lock().last_monitor = Some(monitor.clone()); - } - monitor - }) + self.shared_state.lock().last_monitor.clone() } pub fn available_monitors(&self) -> Vec { @@ -982,17 +972,6 @@ impl UnownedWindow { self.logicalize_size(self.inner_size_physical()) } - pub(crate) fn outer_size_physical(&self) -> (u32, u32) { - let extents = self.shared_state.lock().frame_extents.clone(); - if let Some(extents) = extents { - let (w, h) = self.inner_size_physical(); - extents.inner_size_to_outer(w, h) - } else { - self.update_cached_frame_extents(); - self.outer_size_physical() - } - } - #[inline] pub fn outer_size(&self) -> LogicalSize { let extents = self.shared_state.lock().frame_extents.clone();