Skip to content

Commit

Permalink
Fix #463 - broken inter-window update messages
Browse files Browse the repository at this point in the history
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<Window>` 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.
  • Loading branch information
timboudreau committed May 30, 2024
1 parent 28c78fa commit 942442a
Show file tree
Hide file tree
Showing 5 changed files with 564 additions and 1 deletion.
100 changes: 100 additions & 0 deletions src/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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! {
Expand Down Expand Up @@ -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<WindowId> {
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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -430,4 +479,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<ScreenLayout> {
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<Rect>;
/// Get the bounds of the content of this window, excluding
/// titlebar and native window borders.
fn bounds_of_content_on_screen(&self) -> Option<Rect>;
/// Get the location of the window including any OS titlebar.
fn position_on_screen_including_frame(&self) -> Option<Point>;
/// Get the location of the window **excluding** any OS titlebar.
fn position_of_content_on_screen(&self) -> Option<Point>;
/// Get the logical bounds of the monitor this window is on
fn monitor_bounds(&self) -> Option<Rect>;
}

impl WindowIdExt for WindowId {
fn bounds_on_screen_including_frame(&self) -> Option<Rect> {
window_outer_screen_bounds(self)
}

fn bounds_of_content_on_screen(&self) -> Option<Rect> {
window_inner_screen_bounds(self)
}

fn position_on_screen_including_frame(&self) -> Option<Point> {
window_outer_screen_position(self)
}

fn position_of_content_on_screen(&self) -> Option<Point> {
window_inner_screen_position(self)
}

fn monitor_bounds(&self) -> Option<Rect> {
monitor_bounds(self)
}
}
5 changes: 4 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -180,16 +181,18 @@ 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;
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};
200 changes: 200 additions & 0 deletions src/screen_layout.rs
Original file line number Diff line number Diff line change
@@ -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<ScreenLayout> {
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<Point>,
target_size: Option<Size>,
) -> 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
}
3 changes: 3 additions & 0 deletions src/window_handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -1127,6 +1129,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")]
Expand Down
Loading

0 comments on commit 942442a

Please sign in to comment.