From 6ab4d1f83035eeca825f68aee1c5782fbe5e0529 Mon Sep 17 00:00:00 2001 From: Tim Boudreau Date: Sun, 26 May 2024 00:22:06 -0400 Subject: [PATCH] Fix #463 - broken inter-window update messages This addresses a couple of issues: * Messages (including repaints) not getting processed when delivered a `View` in a window which is inactive * Ability to retrieve the window id of a view without the application having to pass that information down to whatever code needs it and addresses a related issue - obtaining a `WindowId` for a realized `View` and its bounds on screen, in order to convert view-relative locations into screen locations for purposes such as positioning windows relative to a `View`. This patch adds a tracking data structure, and a convenience data structure which contains sufficient information to convert view-relative positions to screen-positions and back. * A static mapping of `root-id:WindowId`:`Arc` which is populated on window creation and cleared on destruction, which makes it possible to access `Window.request_redraw()` to force a redraw. It is guarded by a read-write lock (but written to only during window creation and destruction). * `ScreenLayout` contains a snapshot of the window id, the window's content and framed bounds, the view's origin within the window and the monitor scaling. So, a ViewId that wants its window-id looks up its root view's id, and looks up the WindowId associated with that root. We extend `WindowId` with methods to get the window bounds on screen with and without decorations and similar - these methods could be added to `ViewId` instead, but it seems more intuitive that you ask a window id for the window's bounds the same way you ask a view id for the view's bounds - that seems like what users will expect. Given the usage patterns (the mapping will only be locked for writing when a window is being created or destroyed, and will not be read-locked except in the case of forcing a repaint or looking up window coordinates), it is not touched in the critical path of rendering - unless the application and destroying many windows at the same time on different threads, I would not expect the read-write lock to be contended at all in practice (famous last words, I know). A thread-local for the mapping, as is done in `update.rs` might work (or look like it works), but we would be assuming no platform floem will ever support uses different event threads for different windows' event loops. I know of two that do - BeOS and Java AWT (with its stack of event threads to allow modal dialogs that work when their parent is disabled or blocked, which was a source of "interesting" deadlocks). If there was some path that I missed that would let a `ViewId` reach down to the `WindowHandle` that owns it without doing this sort of tracking, I would love to know about it - `PaintCx` has this information, but it is the only thing that does, and the `ViewId` has no access to it. --- src/id.rs | 100 ++++++++++++++++ src/lib.rs | 5 +- src/screen_layout.rs | 200 ++++++++++++++++++++++++++++++++ src/window_handle.rs | 3 + src/window_tracking.rs | 257 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 564 insertions(+), 1 deletion(-) create mode 100644 src/screen_layout.rs create mode 100644 src/window_tracking.rs diff --git a/src/id.rs b/src/id.rs index a3010eed..6b1f80ba 100644 --- a/src/id.rs +++ b/src/id.rs @@ -6,6 +6,7 @@ use std::{any::Any, cell::RefCell, rc::Rc}; +use floem_winit::window::WindowId; use peniko::kurbo::{Insets, Point, Rect, Size}; use slotmap::new_key_type; use taffy::{Display, Layout, NodeId, TaffyTree}; @@ -21,6 +22,11 @@ use crate::{ view::{IntoView, View}, view_state::{ChangeFlags, StackOffset, ViewState}, view_storage::VIEW_STORAGE, + window_tracking::{ + monitor_bounds, window_id_for_root, window_inner_screen_bounds, + window_inner_screen_position, window_outer_screen_bounds, window_outer_screen_position, + }, + ScreenLayout, }; new_key_type! { @@ -252,8 +258,34 @@ impl ViewId { self.request_changes(ChangeFlags::LAYOUT) } + fn is_in_active_window(&self) -> bool { + if let Some(root) = self.root() { + return crate::window_handle::get_current_view() == root; + } + false + } + + fn force_repaint(&self) -> bool { + if let Some(window_id) = self.window_id() { + // This should not fail unless, say, the view was never + // added to a parent view that is realized + if crate::window_tracking::force_window_repaint(&window_id) { + return true; + } + } + crate::window_tracking::force_all_repaint() + } + + pub fn window_id(&self) -> Option { + self.root().and_then(window_id_for_root) + } + pub fn request_paint(&self) { + let active = self.is_in_active_window(); self.add_update_message(UpdateMessage::RequestPaint); + if !active { + self.force_repaint(); + } } /// request that this node be styled again @@ -320,6 +352,23 @@ impl ViewId { id: *self, state: Box::new(state), }); + // A state update sent from the active window to a view + // in another window will be enqueued, but will not actually + // be retrieved and processed some external OS-level event + // triggers a repaint; in order to get the event processed, + // we need to force a repaint via the winit window, bypassing + // the usual mechanism to trigger a repaint (repaint messages + // too, will just be enqueued but not run until ... a repaint + // happens for some external reason). + // + // Without this, it is impossible for popup windows and similar + // to deliver messages to their parent that affect their display + // (or close the popup window), because the inactive window is + // completely quiescent unless jogged by, say the mouse passing + // over it. + if !self.is_in_active_window() { + self.force_repaint(); + } } /// `viewport` is relative to the `id` view. @@ -424,4 +473,55 @@ impl ViewId { msgs.push((*self, Box::new(state))); }); } + + /// Get a layout in screen-coordinates for this view, if possible. + pub fn screen_layout(&self) -> Option { + crate::screen_layout::try_create_screen_layout(self) + } +} + +/// Ensures `WindowIdExt` cannot be implemented on arbitrary types. +trait WindowIdExtSealed {} +impl WindowIdExtSealed for WindowId {} + +/// Extends WindowId to give instances methods to retrieve properties of the associated window, +/// much as ViewId does. Methods may return None if the view is not realized on-screen, or +/// if information needed to compute the result is not available on the current platform or +/// available on the current platform but not from the calling thread. +#[allow(private_bounds)] +pub trait WindowIdExt: WindowIdExtSealed { + /// Get the bounds of the content of this window, including + /// titlebar and native window borders. + fn bounds_on_screen_including_frame(&self) -> Option; + /// Get the bounds of the content of this window, excluding + /// titlebar and native window borders. + fn bounds_of_content_on_screen(&self) -> Option; + /// Get the location of the window including any OS titlebar. + fn position_on_screen_including_frame(&self) -> Option; + /// Get the location of the window **excluding** any OS titlebar. + fn position_of_content_on_screen(&self) -> Option; + /// Get the logical bounds of the monitor this window is on + fn monitor_bounds(&self) -> Option; +} + +impl WindowIdExt for WindowId { + fn bounds_on_screen_including_frame(&self) -> Option { + window_outer_screen_bounds(self) + } + + fn bounds_of_content_on_screen(&self) -> Option { + window_inner_screen_bounds(self) + } + + fn position_on_screen_including_frame(&self) -> Option { + window_outer_screen_position(self) + } + + fn position_of_content_on_screen(&self) -> Option { + window_inner_screen_position(self) + } + + fn monitor_bounds(&self) -> Option { + monitor_bounds(self) + } } diff --git a/src/lib.rs b/src/lib.rs index 2f00a1ab..8701830a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -169,6 +169,7 @@ pub mod pointer; mod profiler; pub mod renderer; pub mod responsive; +mod screen_layout; pub mod style; pub(crate) mod theme; pub mod unit; @@ -180,6 +181,7 @@ pub mod view_tuple; pub mod views; pub mod window; mod window_handle; +mod window_tracking; pub use app::{launch, quit_app, AppEvent, Application}; pub use app_state::AppState; @@ -187,9 +189,10 @@ pub use clipboard::{Clipboard, ClipboardError}; pub use floem_reactive as reactive; pub use floem_renderer::cosmic_text; pub use floem_renderer::Renderer; -pub use id::ViewId; +pub use id::{ViewId, WindowIdExt}; pub use peniko; pub use peniko::kurbo; +pub use screen_layout::ScreenLayout; pub use taffy; pub use view::{recursively_layout_view, AnyView, IntoView, View}; pub use window::{close_window, new_window}; diff --git a/src/screen_layout.rs b/src/screen_layout.rs new file mode 100644 index 00000000..dfa282e2 --- /dev/null +++ b/src/screen_layout.rs @@ -0,0 +1,200 @@ +//! Tools for computing screen locations from locations within a View and +//! vice-versa. +use crate::ViewId; +use floem_winit::window::WindowId; +use peniko::kurbo::{Point, Rect, Size}; + +use crate::window_tracking::{ + monitor_bounds_for_monitor, rect_from_physical_bounds_for_window, with_window_id_and_window, +}; + +/// Create a ScreenLayout for a view. This can fail if the view or an +/// ancestor of it has no parent and is not realized on-screen. ScreenLayout +/// is useful when needing to convert locations within a view into absolute +/// positions on-screen, such as for creating a window at position relative +/// to that view. +pub fn try_create_screen_layout(view: &ViewId) -> Option { + with_window_id_and_window(view, |window_id, window| { + window + .current_monitor() + .map(|monitor| { + window + .inner_position() + .map(|inner_position| { + window + .outer_position() + .map(|outer_position| { + let monitor_bounds = monitor_bounds_for_monitor(window, &monitor); + let inner_size = window.inner_size(); + let outer_size = window.outer_size(); + + let window_bounds = rect_from_physical_bounds_for_window( + window, + outer_position, + outer_size, + ); + + let window_content_bounds = rect_from_physical_bounds_for_window( + window, + inner_position, + inner_size, + ); + + let view_origin_in_window = find_window_origin(view); + let monitor_scale = window.scale_factor(); + + ScreenLayout { + monitor_scale, + monitor_bounds, + window_content_bounds, + window_bounds, + view_origin_in_window, + window_id: *window_id, + } + }) + .ok() + }) + .ok() + }) + .unwrap_or(None) + .unwrap_or(None) + }) + .unwrap_or(None) +} + +/// Relates a realized `View` to the bounds of the window that contains it, +/// and the window to the bounds of the monitor that contains it. All fields +/// are in logical coordinates (if the OS scales physical coordinates for high +/// DPI displays, the scaling is already applied). +/// +/// Instances are a snapshot in time of the location of the view and window +/// at the time of creation, and are not updated if view or window or monitor +/// in use is. +pub struct ScreenLayout { + /// The window id + pub window_id: WindowId, + /// The scaling of the monitor, if any + pub monitor_scale: f64, + /// The logical bounds of the monitor + pub monitor_bounds: Rect, + /// The bounds of the view content within the monitor's bounds + pub window_content_bounds: Rect, + /// The bounds of the window within the monitor's bounds + pub window_bounds: Rect, + /// The origin of the view within the window + pub view_origin_in_window: Point, +} + +impl ScreenLayout { + /// Unscales this Screen to physical device coordinates, less any DPI + /// scaling done by hardware. + pub fn to_physical_scale(&self) -> Self { + // The bounds we have are pre-scaled by 1.0/window.scale(), so + // inverting them is multiplication, not division. + Self { + monitor_scale: 1.0, + monitor_bounds: scale_rect(self.monitor_scale, self.monitor_bounds), + window_bounds: scale_rect(self.monitor_scale, self.window_bounds), + window_content_bounds: scale_rect(self.monitor_scale, self.window_content_bounds), + view_origin_in_window: scale_point(self.monitor_scale, self.view_origin_in_window), + window_id: self.window_id, + } + } + + /// If true, this instance has scaling applied. + pub fn is_scaled(&self) -> bool { + self.monitor_scale != 0_f64 + } + + /// Convert a screen position to a position within the view that created + /// this one. + pub fn view_location_from_screen(&self, screen_point: Point) -> Point { + let mut result = screen_point; + result.x -= self.view_origin_in_window.x + self.window_content_bounds.x0; + result.y -= self.view_origin_in_window.y + self.window_content_bounds.y0; + result + } + + /// Compute a position, in screen coordinates, relative to the view this layout + /// was created from. If a target size is passed, the implementation will attempt + /// to adjust the resulting point so that a rectangle of the required size fits + /// entirely on-screen. + pub fn screen_location_from_view( + &self, + relative_position: Option, + target_size: Option, + ) -> Point { + let mut result = Point::new(self.window_content_bounds.x0, self.window_content_bounds.y0); + if let Some(offset) = relative_position { + result.x += offset.x; + result.y += offset.y; + } + + result.x += self.view_origin_in_window.x; + result.y += self.view_origin_in_window.y; + + // If we have a size, adjust the resulting point to ensure the resulting + // bounds will fit on screen (if it is possible) + if let Some(size) = target_size { + let mut target_bounds = Rect::new( + result.x, + result.y, + result.x + size.width, + result.y + size.height, + ); + if target_bounds.x1 > self.monitor_bounds.x1 { + let offset = target_bounds.x1 - self.monitor_bounds.x1; + target_bounds.x0 -= offset; + target_bounds.x1 -= offset; + } + if target_bounds.y1 > self.monitor_bounds.y1 { + let offset = target_bounds.y1 - self.monitor_bounds.y1; + target_bounds.y0 -= offset; + target_bounds.y1 -= offset; + } + if target_bounds.x0 < self.monitor_bounds.x0 { + let offset = self.monitor_bounds.x0 - target_bounds.x0; + target_bounds.x0 += offset; + target_bounds.x1 += offset; + } + if target_bounds.y0 < self.monitor_bounds.y0 { + let offset = self.monitor_bounds.y0 - target_bounds.y0; + target_bounds.y0 += offset; + target_bounds.y1 += offset + } + result.x = target_bounds.x0; + result.y = target_bounds.y0; + } + result + } +} + +fn find_window_origin(view: &ViewId) -> Point { + let mut pt = Point::ZERO; + recursively_find_window_origin(*view, &mut pt); + pt +} + +fn recursively_find_window_origin(view: ViewId, point: &mut Point) { + if let Some(layout) = view.get_layout() { + point.x += layout.location.x as f64; + point.y += layout.location.y as f64; + if let Some(parent) = view.parent() { + recursively_find_window_origin(parent, point); + } + } +} + +fn scale_point(by: f64, mut pt: Point) -> Point { + pt.x *= by; + pt.y *= by; + pt +} + +fn scale_rect(by: f64, mut bds: Rect) -> Rect { + bds.x0 *= by; + bds.y0 *= by; + bds.x1 *= by; + bds.y1 *= by; + bds +} diff --git a/src/window_handle.rs b/src/window_handle.rs index 9a452dc1..75d7526c 100644 --- a/src/window_handle.rs +++ b/src/window_handle.rs @@ -45,6 +45,7 @@ use crate::{ view::{default_compute_layout, view_tab_navigation, IntoView, View}, view_state::ChangeFlags, views::Decorators, + window_tracking::{remove_window_id_mapping, store_window_id_mapping}, }; /// The top-level window handle that owns the winit Window. @@ -120,6 +121,7 @@ impl WindowHandle { id.set_view(view.into_any()); let window = Arc::new(window); + store_window_id_mapping(id, window_id, &window); let paint_state = PaintState::new(window.clone(), scale, size.get_untracked() * scale); let mut window_handle = Self { window: Some(window), @@ -1118,6 +1120,7 @@ impl WindowHandle { pub(crate) fn destroy(&mut self) { self.event(Event::WindowClosed); self.scope.dispose(); + remove_window_id_mapping(&self.id, &self.window_id); } #[cfg(target_os = "macos")] diff --git a/src/window_tracking.rs b/src/window_tracking.rs new file mode 100644 index 00000000..b8c53e1f --- /dev/null +++ b/src/window_tracking.rs @@ -0,0 +1,257 @@ +//! Provides tracking to map window ids to windows in order to force repaints +//! on inactive windows (which would otherwise not receive messages), and so +//! that views can retrieve the `WindowId` of the window that contains them +//! and use the methods that look up the `Window` for that id to retrieve information +//! such as screen position. +use crate::ViewId; +use floem_winit::{ + dpi::{PhysicalPosition, PhysicalSize}, + monitor::MonitorHandle, + window::{Window, WindowId}, +}; +use peniko::kurbo::{Point, Rect}; +use std::{ + collections::HashMap, + sync::{Arc, OnceLock, RwLock}, +}; + +static WINDOW_FOR_WINDOW_AND_ROOT_IDS: OnceLock> = OnceLock::new(); + +/// Add a mapping from root_id -> window_id -> window for the given triple. +pub fn store_window_id_mapping( + root_id: ViewId, + window_id: WindowId, + window: &Arc, +) { + with_window_map_mut(move |m| m.add(root_id, window_id, window.clone())); +} + +/// Remove the mapping from root_id -> window_id -> window for the given triple. +pub fn remove_window_id_mapping(root_id: &ViewId, window_id: &WindowId) { + with_window_map_mut(move |m| m.remove(root_id, window_id)); +} + +/// Maps root-id:window-id:window triples, so a view can get its root and +/// from that locate the window-id (if any) that it belongs to. +#[derive(Default, Debug)] +struct WindowMapping { + window_for_window_id: HashMap>, + window_id_for_root_view_id: HashMap, +} + +impl WindowMapping { + fn add(&mut self, root: ViewId, window_id: WindowId, window: Arc) { + self.window_for_window_id.insert(window_id, window); + self.window_id_for_root_view_id.insert(root, window_id); + } + + fn remove(&mut self, root: &ViewId, window_id: &WindowId) { + let root_found = self.window_id_for_root_view_id.remove(root).is_some(); + let window_found = self.window_for_window_id.remove(window_id).is_some(); + debug_assert!(root_found == window_found, + "Window mapping state inconsistent. Remove root {:?} success was {} but remove {:?} success was {}", + root, root_found, window_id, window_found); + } + + fn with_window_id_and_window T, T>( + &self, + root_view_id: ViewId, + f: F, + ) -> Option { + self.window_id_for_root_view_id + .get(&root_view_id) + .and_then(|window_id| { + self.window_for_window_id + .get(window_id) + .map(|window| f(window_id, window)) + }) + } + + fn with_window) -> T, T>(&self, window: &WindowId, f: F) -> Option { + self.window_for_window_id.get(window).map(f) + } + + fn each_window)>(&self, f: F) { + for (_, window) in self.window_for_window_id.iter() { + f(window) + } + } + + fn window_id_for_root(&self, id: &ViewId) -> Option { + self.window_id_for_root_view_id.get(id).copied() + } +} + +pub fn with_window_id_and_window T, T>( + view: &ViewId, + f: F, +) -> Option { + view.root() + .and_then(|root_view_id| with_window_map(|m| m.with_window_id_and_window(root_view_id, f))) + .unwrap_or(None) +} + +fn with_window_map_mut(mut f: F) -> bool { + let map = WINDOW_FOR_WINDOW_AND_ROOT_IDS.get_or_init(|| RwLock::new(Default::default())); + if let Ok(mut map) = map.write() { + f(&mut map); + true + } else { + false + } +} + +fn with_window_map T, T>(f: F) -> Option { + let map = WINDOW_FOR_WINDOW_AND_ROOT_IDS.get_or_init(|| RwLock::new(Default::default())); + if let Ok(map) = map.read() { + Some(f(&map)) + } else { + None + } +} + +/// Force a single window to repaint - this is necessary in cases where the +/// window is not the active window and otherwise would not process update +/// messages sent to it. +pub fn force_window_repaint(id: &WindowId) -> bool { + with_window_map(|m| { + m.with_window(id, |window| window.request_redraw()) + .is_some() + }) + .unwrap_or(false) +} + +pub fn window_id_for_root(root_id: ViewId) -> Option { + with_window_map(|map| map.window_id_for_root(&root_id)).unwrap_or(None) +} + +pub fn force_all_repaint() -> bool { + with_window_map(|m| { + println!("Force repaint of {} windows", m.window_for_window_id.len()); + m.each_window(|window| window.request_redraw()); + !m.window_for_window_id.is_empty() + }) + .unwrap_or(false) +} + +pub fn monitor_bounds(id: &WindowId) -> Option { + with_window_map(|m| { + m.with_window(id, |window| { + window + .current_monitor() + .map(|monitor| monitor_bounds_for_monitor(window, &monitor)) + }) + .unwrap_or(None) + }) + .unwrap_or(None) +} + +pub fn monitor_bounds_for_monitor(window: &Window, monitor: &MonitorHandle) -> Rect { + let scale = 1.0 / window.scale_factor(); + let pos = monitor.position(); + let sz = monitor.size(); + let x = pos.x as f64 * scale; + let y = pos.y as f64 * scale; + Rect::new( + x, + y, + x + sz.width as f64 * scale, + y + sz.height as f64 * scale, + ) +} + +fn scale_rect(window: &Window, mut rect: Rect) -> Rect { + let scale = 1.0 / window.scale_factor(); + rect.x0 *= scale; + rect.y0 *= scale; + rect.x1 *= scale; + rect.y1 *= scale; + rect +} + +fn scale_point(window: &Window, mut rect: Point) -> Point { + let scale = 1.0 / window.scale_factor(); + rect.x *= scale; + rect.y *= scale; + rect +} + +pub fn window_inner_screen_position(id: &WindowId) -> Option { + with_window_map(|m| { + m.with_window(id, |window| { + window + .inner_position() + .map(|pos| Some(scale_point(window, Point::new(pos.x as f64, pos.y as f64)))) + .unwrap_or(None) + }) + .unwrap_or(None) + }) + .unwrap_or(None) +} + +pub fn window_inner_screen_bounds(id: &WindowId) -> Option { + with_window_map(|m| { + m.with_window(id, |window| { + window + .inner_position() + .map(|pos| { + Some(rect_from_physical_bounds_for_window( + window, + pos, + window.inner_size(), + )) + }) + .unwrap_or(None) + }) + .unwrap_or(None) + }) + .unwrap_or(None) +} + +pub fn rect_from_physical_bounds_for_window( + window: &Window, + pos: PhysicalPosition, + sz: PhysicalSize, +) -> Rect { + scale_rect( + window, + Rect::new( + pos.x as f64, + pos.y as f64, + pos.x as f64 + sz.width.max(0) as f64, + pos.y as f64 + sz.height.max(0) as f64, + ), + ) +} + +pub fn window_outer_screen_position(id: &WindowId) -> Option { + with_window_map(|m| { + m.with_window(id, |window| { + window + .outer_position() + .map(|pos| Some(scale_point(window, Point::new(pos.x as f64, pos.y as f64)))) + .unwrap_or(None) + }) + .unwrap_or(None) + }) + .unwrap_or(None) +} + +pub fn window_outer_screen_bounds(id: &WindowId) -> Option { + with_window_map(|m| { + m.with_window(id, |window| { + window + .outer_position() + .map(|pos| { + Some(rect_from_physical_bounds_for_window( + window, + pos, + window.outer_size(), + )) + }) + .unwrap_or(None) + }) + .unwrap_or(None) + }) + .unwrap_or(None) +}