From 19f76975a8800bc048e4337e38570bceb0379f1f Mon Sep 17 00:00:00 2001 From: jrmoulton Date: Tue, 11 Nov 2025 22:40:41 -0700 Subject: [PATCH] Add better ViewId debugging with debug name access and make the ViewId not thread safe. --- src/id.rs | 65 ++++++++++++++++++++++++++++++++++++++---- src/window_tracking.rs | 45 +++++++++-------------------- 2 files changed, 73 insertions(+), 37 deletions(-) diff --git a/src/id.rs b/src/id.rs index 5455e414b..860743f54 100644 --- a/src/id.rs +++ b/src/id.rs @@ -7,7 +7,6 @@ use std::{any::Any, cell::RefCell, rc::Rc}; use peniko::kurbo::{Insets, Point, Rect, Size}; -use slotmap::new_key_type; use taffy::{Display, Layout, NodeId, TaffyTree}; use winit::window::WindowId; @@ -26,11 +25,40 @@ use crate::{ window_tracking::{is_known_root, window_id_for_root}, }; -new_key_type! { - /// A small unique identifier for an instance of a [View](crate::View). - /// - /// This id is how you can access and modify a view, including accessing children views and updating state. - pub struct ViewId; +#[allow(unused)] +pub struct NotThreadSafe(*const ()); + +/// A small unique identifier, and handle, for an instance of a [View](crate::View). +/// +/// Through this handle, you can access the associated view [ViewState](crate::view_state::ViewState). +/// You can also use this handle to access the children ViewId's which allows you access to their states. +/// +/// This type is not thread safe and can only be used from the main thread. +#[derive(Copy, Clone, Default, Eq, PartialEq, Ord, PartialOrd, Hash)] +#[repr(transparent)] +pub struct ViewId(slotmap::KeyData, std::marker::PhantomData); +impl std::fmt::Debug for ViewId { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let debug_name = self.debug_name(); + let mut start = f.debug_struct("ViewId"); + + if !debug_name.is_empty() { + start.field("id", &self.0).field("debug_name", &debug_name) + } else { + start.field("id", &self.0) + } + .finish() + } +} +impl slotmap::__impl::From for ViewId { + fn from(k: slotmap::KeyData) -> Self { + ViewId(k, std::marker::PhantomData) + } +} +unsafe impl slotmap::Key for ViewId { + fn data(&self) -> slotmap::KeyData { + self.0 + } } impl ViewId { @@ -226,6 +254,31 @@ impl ViewId { }) } + /// Get the chain of debug names that have been applied to this view. + /// + /// This uses try_borrow on the view state so if the view state has already been borrowed when using this method, it won't crash and it will just return an empty string. + pub fn debug_name(&self) -> String { + let state_names = self + .state() + .try_borrow() + .ok() + .map(|state| state.debug_name.iter().rev().cloned().collect::>()) + .unwrap_or_default(); + + let view_name = self + .view() + .try_borrow() + .ok() + .map(|view| View::debug_name(view.as_ref()).to_string()) + .unwrap_or_default(); + + state_names + .into_iter() + .chain(std::iter::once(view_name)) + .collect::>() + .join(" - ") + } + /// Get the computed rectangle that covers the area of this View pub fn layout_rect(&self) -> Rect { self.state().borrow().layout_rect diff --git a/src/window_tracking.rs b/src/window_tracking.rs index 4daa4dbe6..3f723432a 100644 --- a/src/window_tracking.rs +++ b/src/window_tracking.rs @@ -5,17 +5,18 @@ //! such as screen position. use crate::ViewId; use peniko::kurbo::{Point, Rect}; -use std::{ - collections::HashMap, - sync::{Arc, OnceLock, RwLock}, -}; +use std::{collections::HashMap, sync::Arc}; use winit::{ dpi::{PhysicalPosition, PhysicalSize}, monitor::MonitorHandle, window::{Window, WindowId}, }; -static WINDOW_FOR_WINDOW_AND_ROOT_IDS: OnceLock> = OnceLock::new(); +use std::cell::RefCell; + +thread_local! { + static WINDOW_FOR_WINDOW_AND_ROOT_IDS: RefCell = RefCell::new(WindowMapping::default()); +} /// Add a mapping from `root_id` -> `window_id` -> `window` for the given triple. pub fn store_window_id_mapping( @@ -96,38 +97,26 @@ pub fn with_window_id_and_window) -> T, T> ) -> 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) } pub fn is_known_root(id: &ViewId) -> bool { - with_window_map(|map| map.window_id_for_root_view_id.contains_key(id)).unwrap_or(false) + with_window_map(|map| map.window_id_for_root_view_id.contains_key(id)) } -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_mut T, T>(f: F) -> T { + WINDOW_FOR_WINDOW_AND_ROOT_IDS.with(|map| f(&mut map.borrow_mut())) } -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 - } +fn with_window_map T, T>(f: F) -> T { + WINDOW_FOR_WINDOW_AND_ROOT_IDS.with(|map| f(&map.borrow())) } pub fn with_window) -> T, T>(window: &WindowId, f: F) -> Option { - with_window_map(|m| m.with_window(window, |w| f(w))).unwrap_or(None) + with_window_map(|m| m.with_window(window, |w| f(w))) } pub fn root_view_id(window: &WindowId) -> Option { - with_window_map(|m| m.root_view_id_for(window)).unwrap_or(None) + with_window_map(|m| m.root_view_id_for(window)) } /// Force a single window to repaint - this is necessary in cases where the @@ -138,11 +127,10 @@ pub fn force_window_repaint(id: &WindowId) -> bool { 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) + with_window_map(|map| map.window_id_for_root(&root_id)) } pub fn monitor_bounds(id: &WindowId) -> Option { @@ -154,7 +142,6 @@ pub fn monitor_bounds(id: &WindowId) -> Option { }) .unwrap_or(None) }) - .unwrap_or(None) } pub fn monitor_bounds_for_monitor(window: &Arc, monitor: &MonitorHandle) -> Rect { @@ -197,7 +184,6 @@ pub fn window_inner_screen_position(id: &WindowId) -> Option { scale_point(window, Point::new(pos.x as f64, pos.y as f64)) }) }) - .unwrap_or(None) } pub fn window_inner_screen_bounds(id: &WindowId) -> Option { @@ -207,7 +193,6 @@ pub fn window_inner_screen_bounds(id: &WindowId) -> Option { rect_from_physical_bounds_for_window(window, pos, window.surface_size()) }) }) - .unwrap_or(None) } pub fn rect_from_physical_bounds_for_window( @@ -236,7 +221,6 @@ pub fn window_outer_screen_position(id: &WindowId) -> Option { }) .unwrap_or(None) }) - .unwrap_or(None) } pub fn window_outer_screen_bounds(id: &WindowId) -> Option { @@ -255,5 +239,4 @@ pub fn window_outer_screen_bounds(id: &WindowId) -> Option { }) .unwrap_or(None) }) - .unwrap_or(None) }