diff --git a/Cargo.lock b/Cargo.lock index 030584c..c792a68 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -639,6 +639,7 @@ dependencies = [ "kludgine", "nominals", "palette", + "parking_lot", "plotters", "png", "pollster", diff --git a/Cargo.toml b/Cargo.toml index cb903cb..5bf62c7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,6 +45,7 @@ png = "0.17.10" image = { version = "0.25.0", features = ["png"] } plotters = { version = "0.3.5", default-features = false, optional = true } nominals = "0.3.0" +parking_lot = "0.12.1" # [patch.crates-io] diff --git a/examples/custom-widgets.rs b/examples/custom-widgets.rs index 14173ef..0310dfa 100644 --- a/examples/custom-widgets.rs +++ b/examples/custom-widgets.rs @@ -14,11 +14,7 @@ fn main() -> cushy::Result { "Inline Widgets" .and(callback_widget()) .into_rows() - .and( - "impl MakeWidget" - .and(ToggleMakeWidget::default()) - .into_rows(), - ) + .and("impl MakeWidget".and(ToggleMakeWidget).into_rows()) .and("impl Widget".and(impl_widget()).into_rows()) .into_columns() .centered() @@ -59,7 +55,7 @@ fn callback_widget() -> impl MakeWidgetWithTag { /// widget or any of its children aren't focusable, implementing [`MakeWidget`] /// directly will make more sense. #[derive(Default)] -struct ToggleMakeWidget(Toggle); +struct ToggleMakeWidget; impl MakeWidgetWithTag for ToggleMakeWidget { fn make_with_tag(self, id: WidgetTag) -> WidgetInstance { diff --git a/src/animation.rs b/src/animation.rs index 536b92f..a2d6491 100644 --- a/src/animation.rs +++ b/src/animation.rs @@ -43,7 +43,7 @@ use std::cmp::Ordering; use std::fmt::{Debug, Display}; use std::ops::{ControlFlow, Deref, Div, DivAssign, Mul, MulAssign, Sub}; use std::str::FromStr; -use std::sync::{Arc, Condvar, Mutex, MutexGuard, OnceLock}; +use std::sync::{Arc, OnceLock}; use std::thread; use std::time::{Duration, Instant}; @@ -53,10 +53,11 @@ use figures::{Angle, Point, Ranged, Rect, Size, UnscaledUnit, Zero}; use intentional::Cast; use kempt::Set; use kludgine::Color; +use parking_lot::{Condvar, Mutex, MutexGuard}; use crate::animation::easings::Linear; use crate::styles::{Component, RequireInvalidation}; -use crate::utils::{run_in_bg, IgnorePoison}; +use crate::utils::run_in_bg; use crate::value::{Destination, Dynamic, Source}; static ANIMATIONS: Mutex = Mutex::new(Animating::new()); @@ -67,7 +68,7 @@ fn thread_state() -> MutexGuard<'static, Animating> { THREAD.get_or_init(|| { thread::spawn(animation_thread); }); - ANIMATIONS.lock().ignore_poison() + ANIMATIONS.lock() } fn animation_thread() { @@ -75,7 +76,7 @@ fn animation_thread() { loop { if state.running.is_empty() { state.last_updated = None; - state = NEW_ANIMATIONS.wait(state).ignore_poison(); + NEW_ANIMATIONS.wait(&mut state); } else { let start = Instant::now(); let last_tick = state.last_updated.unwrap_or(start); diff --git a/src/app.rs b/src/app.rs index c383d0f..8807e2f 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1,10 +1,10 @@ -use std::sync::{Arc, Mutex, MutexGuard}; +use std::sync::Arc; use arboard::Clipboard; use kludgine::app::{AppEvent, AsApplication}; +use parking_lot::{Mutex, MutexGuard}; use crate::fonts::FontCollection; -use crate::utils::IgnorePoison; use crate::window::sealed::WindowCommand; use crate::window::WindowHandle; @@ -64,9 +64,7 @@ impl Cushy { /// initialized when the window opened. #[must_use] pub fn clipboard_guard(&self) -> Option> { - self.clipboard - .as_ref() - .map(|mutex| mutex.lock().ignore_poison()) + self.clipboard.as_ref().map(|mutex| mutex.lock()) } /// Returns the font collection that will be loaded in all Cushy windows. diff --git a/src/context.rs b/src/context.rs index 6af6dbe..c3d58e9 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1414,11 +1414,11 @@ impl Trackable for T where T: sealed::Trackable {} pub(crate) mod sealed { use std::sync::atomic::{AtomicBool, Ordering}; - use std::sync::{Arc, Mutex, MutexGuard}; + use std::sync::Arc; use kempt::Set; + use parking_lot::{Mutex, MutexGuard}; - use crate::utils::IgnorePoison; use crate::widget::WidgetId; use crate::window::WindowHandle; @@ -1445,12 +1445,12 @@ pub(crate) mod sealed { } pub fn invalidate(&self, widget: WidgetId) -> bool { - let mut invalidated = self.invalidated.lock().ignore_poison(); + let mut invalidated = self.invalidated.lock(); invalidated.insert(widget) } pub fn invalidations(&self) -> MutexGuard<'_, Set> { - self.invalidated.lock().ignore_poison() + self.invalidated.lock() } } diff --git a/src/tick.rs b/src/tick.rs index 6120618..a884f56 100644 --- a/src/tick.rs +++ b/src/tick.rs @@ -1,5 +1,5 @@ use std::sync::atomic::{AtomicUsize, Ordering}; -use std::sync::{Arc, Condvar, Mutex, MutexGuard}; +use std::sync::Arc; use std::time::{Duration, Instant}; use ahash::AHashSet; @@ -8,9 +8,9 @@ use figures::Point; use intentional::Assert; use kludgine::app::winit::event::{ElementState, MouseButton}; use kludgine::app::winit::keyboard::Key; +use parking_lot::{Condvar, Mutex, MutexGuard}; use crate::context::WidgetContext; -use crate::utils::IgnorePoison; use crate::value::{Destination, Dynamic}; use crate::widget::{EventHandling, HANDLED, IGNORED}; use crate::window::KeyEvent; @@ -170,7 +170,7 @@ struct TickData { impl TickData { fn state(&self) -> MutexGuard<'_, TickState> { - self.state.lock().ignore_poison() + self.state.lock() } } @@ -218,7 +218,7 @@ where while state.keep_running { let current_frame = data.rendered_frame.load(Ordering::Acquire); if state.frame == current_frame { - state = data.sync.wait(state).ignore_poison(); + data.sync.wait(&mut state); } else { break; } diff --git a/src/tree.rs b/src/tree.rs index 5f4aa78..9de8bb0 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -1,13 +1,13 @@ use std::mem; -use std::sync::{Arc, Mutex, Weak}; +use std::sync::{Arc, Weak}; use ahash::AHashMap; use alot::{LotId, Lots}; use figures::units::{Px, UPx}; use figures::{Point, Rect, Size}; +use parking_lot::Mutex; use crate::styles::{Styles, ThemePair, VisualOrder}; -use crate::utils::IgnorePoison; use crate::value::Value; use crate::widget::{MountedWidget, WidgetId, WidgetInstance}; use crate::window::{ThemeMode, WindowHandle}; @@ -24,7 +24,7 @@ impl Tree { widget: WidgetInstance, parent: Option<&MountedWidget>, ) -> MountedWidget { - let mut data = self.data.lock().ignore_poison(); + let mut data = self.data.lock(); let id = widget.id(); let (effective_styles, parent_id) = if let Some(parent) = parent { ( @@ -72,7 +72,7 @@ impl Tree { parent: &MountedWidget, children_to_unmount: &mut Vec, ) { - let mut data = self.data.lock().ignore_poison(); + let mut data = self.data.lock(); data.remove_child(child.node_id, parent.node_id, children_to_unmount); if child.widget.is_default() { @@ -84,7 +84,7 @@ impl Tree { } pub(crate) fn set_layout(&self, widget: LotId, rect: Rect) { - let mut data = self.data.lock().ignore_poison(); + let mut data = self.data.lock(); let node = &mut data.nodes[widget]; node.layout = Some(rect); @@ -104,12 +104,12 @@ impl Tree { } pub(crate) fn layout(&self, widget: LotId) -> Option> { - let data = self.data.lock().ignore_poison(); + let data = self.data.lock(); data.nodes.get(widget).and_then(|widget| widget.layout) } pub(crate) fn new_frame(&self, invalidations: impl IntoIterator) { - let mut data = self.data.lock().ignore_poison(); + let mut data = self.data.lock(); data.render_info.clear(); for id in invalidations { @@ -122,7 +122,7 @@ impl Tree { } pub(crate) fn note_widget_rendered(&self, widget: LotId) { - let mut data = self.data.lock().ignore_poison(); + let mut data = self.data.lock(); let Some(layout) = data.nodes.get(widget).and_then(|node| node.layout) else { return; }; @@ -134,7 +134,7 @@ impl Tree { parent: LotId, constraints: Size, ) -> Option> { - let mut data = self.data.lock().ignore_poison(); + let mut data = self.data.lock(); let node = &mut data.nodes[parent]; if let Some(cached_layout) = &node.last_layout_query { @@ -161,7 +161,7 @@ impl Tree { constraints: Size, size: Size, ) { - let mut data = self.data.lock().ignore_poison(); + let mut data = self.data.lock(); data.nodes[id].last_layout_query = Some(CachedLayoutQuery { constraints, size }); } @@ -170,7 +170,7 @@ impl Tree { parent: LotId, order: VisualOrder, ) -> Vec { - let data = self.data.lock().ignore_poison(); + let data = self.data.lock(); let node = &data.nodes[parent]; let mut unordered = node.children.clone(); let mut ordered = Vec::::with_capacity(unordered.len()); @@ -230,12 +230,12 @@ impl Tree { } pub(crate) fn effective_styles(&self, id: LotId) -> Styles { - let data = self.data.lock().ignore_poison(); + let data = self.data.lock(); data.nodes[id].effective_styles.clone() } pub(crate) fn hover(&self, new_hover: Option<&MountedWidget>) -> HoverResults { - let mut data = self.data.lock().ignore_poison(); + let mut data = self.data.lock(); let hovered = new_hover .map(|new_hover| data.widget_hierarchy(new_hover.node_id, self)) .unwrap_or_default(); @@ -260,12 +260,12 @@ impl Tree { } pub fn focus(&self, new_focus: Option) -> Result, ()> { - let mut data = self.data.lock().ignore_poison(); + let mut data = self.data.lock(); data.update_tracked_widget(new_focus, self, |data| &mut data.focus) } pub fn previous_focus(&self, focus: WidgetId) -> Option { - let data = self.data.lock().ignore_poison(); + let data = self.data.lock(); let previous = *data.previous_focuses.get(&focus)?; data.widget_from_id(previous, self) } @@ -274,24 +274,24 @@ impl Tree { &self, new_active: Option<&MountedWidget>, ) -> Result, ()> { - let mut data = self.data.lock().ignore_poison(); + let mut data = self.data.lock(); data.update_tracked_widget(new_active.map(MountedWidget::id), self, |data| { &mut data.active }) } pub fn widget(&self, id: WidgetId) -> Option { - let data = self.data.lock().ignore_poison(); + let data = self.data.lock(); data.widget_from_id(id, self) } pub(crate) fn widget_from_node(&self, id: LotId) -> Option { - let data = self.data.lock().ignore_poison(); + let data = self.data.lock(); data.widget_from_node(id, self) } pub(crate) fn is_enabled(&self, mut id: LotId, context: &WindowHandle) -> bool { - let data = self.data.lock().ignore_poison(); + let data = self.data.lock(); loop { let Some(node) = data.nodes.get(id) else { return false; @@ -310,23 +310,23 @@ impl Tree { } pub(crate) fn active_widget(&self) -> Option { - self.data.lock().ignore_poison().active + self.data.lock().active } pub(crate) fn hovered_widget(&self) -> Option { - self.data.lock().ignore_poison().hover + self.data.lock().hover } pub(crate) fn default_widget(&self) -> Option { - self.data.lock().ignore_poison().defaults.last().copied() + self.data.lock().defaults.last().copied() } pub(crate) fn escape_widget(&self) -> Option { - self.data.lock().ignore_poison().escapes.last().copied() + self.data.lock().escapes.last().copied() } pub(crate) fn is_hovered(&self, id: LotId) -> bool { - let data = self.data.lock().ignore_poison(); + let data = self.data.lock(); let mut search = data.hover; while let Some(hovered) = search { if hovered == id { @@ -339,21 +339,21 @@ impl Tree { } pub(crate) fn focused_widget(&self) -> Option { - self.data.lock().ignore_poison().focus + self.data.lock().focus } pub(crate) fn widgets_under_point(&self, point: Point) -> Vec { - let data = self.data.lock().ignore_poison(); + let data = self.data.lock(); data.render_info.widgets_under_point(point, &data, self) } pub(crate) fn parent(&self, id: LotId) -> Option { - let data = self.data.lock().ignore_poison(); + let data = self.data.lock(); data.nodes.get(id).expect("missing widget").parent } pub(crate) fn is_child(&self, mut id: LotId, possible_parent: &WidgetInstance) -> bool { - let data = self.data.lock().ignore_poison(); + let data = self.data.lock(); while let Some(node) = data.nodes.get(id) { if &node.widget == possible_parent { return true; @@ -371,17 +371,17 @@ impl Tree { } pub(crate) fn attach_styles(&self, id: LotId, styles: Value) { - let mut data = self.data.lock().ignore_poison(); + let mut data = self.data.lock(); data.attach_styles(id, styles); } pub(crate) fn attach_theme(&self, id: LotId, theme: Value) { - let mut data = self.data.lock().ignore_poison(); + let mut data = self.data.lock(); data.nodes.get_mut(id).expect("missing widget").theme = Some(theme); } pub(crate) fn attach_theme_mode(&self, id: LotId, theme: Value) { - let mut data = self.data.lock().ignore_poison(); + let mut data = self.data.lock(); data.nodes.get_mut(id).expect("missing widget").theme_mode = Some(theme); } @@ -389,7 +389,7 @@ impl Tree { &self, id: LotId, ) -> (Styles, Option>, Option>) { - let data = self.data.lock().ignore_poison(); + let data = self.data.lock(); let node = data.nodes.get(id).expect("missing widget"); ( node.effective_styles.clone(), @@ -399,10 +399,7 @@ impl Tree { } pub fn invalidate(&self, id: LotId, include_hierarchy: bool) { - self.data - .lock() - .ignore_poison() - .invalidate(id, include_hierarchy); + self.data.lock().invalidate(id, include_hierarchy); } } diff --git a/src/utils.rs b/src/utils.rs index ad34001..b244c0c 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,6 +1,6 @@ use std::ops::Deref; use std::sync::mpsc::{self, SyncSender}; -use std::sync::{OnceLock, PoisonError}; +use std::sync::OnceLock; use intentional::Assert; use kludgine::app::winit::event::Modifiers; @@ -225,19 +225,6 @@ impl Deref for Lazy { } } -pub trait IgnorePoison { - type Unwrapped; - fn ignore_poison(self) -> Self::Unwrapped; -} - -impl IgnorePoison for Result> { - type Unwrapped = T; - - fn ignore_poison(self) -> Self::Unwrapped { - self.map_or_else(PoisonError::into_inner, |g| g) - } -} - pub trait BgFunction: FnOnce() + Send + 'static {} pub fn run_in_bg(f: F) diff --git a/src/value.rs b/src/value.rs index 3ab8f47..4d2a714 100644 --- a/src/value.rs +++ b/src/value.rs @@ -7,7 +7,7 @@ use std::future::Future; use std::hash::{BuildHasher, Hash}; use std::ops::{Add, AddAssign, Deref, DerefMut, Not}; use std::str::FromStr; -use std::sync::{Arc, Condvar, Mutex, MutexGuard, TryLockError, Weak}; +use std::sync::{Arc, Weak}; use std::task::{Poll, Waker}; use std::thread::{self, ThreadId}; use std::time::{Duration, Instant}; @@ -16,10 +16,11 @@ use ahash::AHashSet; use alot::{LotId, Lots}; use intentional::Assert; use kempt::{Map, Sort}; +use parking_lot::{Condvar, Mutex, MutexGuard}; use crate::animation::{AnimationHandle, DynamicTransition, IntoAnimate, LinearInterpolate, Spawn}; use crate::context::{self, Trackable, WidgetContext}; -use crate::utils::{run_in_bg, IgnorePoison, WithClone}; +use crate::utils::WithClone; use crate::widget::{ MakeWidget, MakeWidgetWithTag, OnceCallback, WidgetId, WidgetInstance, WidgetList, }; @@ -32,7 +33,7 @@ pub trait Source { /// [`Generation`]. fn try_map_generational( &self, - map: impl FnOnce(&GenerationalValue) -> R, + map: impl FnOnce(DynamicGuard<'_, T, true>) -> R, ) -> Result; /// Maps the contents with read-only access, providing access to the value's @@ -42,7 +43,7 @@ pub trait Source { /// /// This function panics if this value is already locked by the current /// thread. - fn map_generational(&self, map: impl FnOnce(&GenerationalValue) -> R) -> R { + fn map_generational(&self, map: impl FnOnce(DynamicGuard<'_, T, true>) -> R) -> R { self.try_map_generational(map).expect("deadlocked") } @@ -54,7 +55,7 @@ pub trait Source { /// thread. #[must_use] fn generation(&self) -> Generation { - self.map_generational(GenerationalValue::generation) + self.map_generational(|g| g.generation()) } /// Maps the contents with read-only access. @@ -64,7 +65,7 @@ pub trait Source { /// This function panics if this value is already locked by the current /// thread. fn map_ref(&self, map: impl FnOnce(&T) -> R) -> R { - self.map_generational(|gen| map(&gen.value)) + self.map_generational(|gen| map(&*gen)) } /// Returns a clone of the currently contained value. @@ -83,7 +84,7 @@ pub trait Source { /// Maps the contents with read-only access. fn try_map_ref(&self, map: impl FnOnce(&T) -> R) -> Result { - self.try_map_generational(|gen| map(&gen.value)) + self.try_map_generational(|gen| map(&*gen)) } /// Returns a clone of the currently contained value. @@ -91,7 +92,7 @@ pub trait Source { where T: Clone, { - self.try_map_generational(|gen| gen.value.clone()) + self.try_map_generational(|gen| gen.clone()) } /// Returns a clone of the currently contained value. @@ -138,7 +139,7 @@ pub trait Source { fn for_each_generational_try(&self, for_each: F) -> CallbackHandle where T: Send + 'static, - F: for<'a> FnMut(&'a GenerationalValue) -> Result<(), CallbackDisconnected> + F: for<'a> FnMut(DynamicGuard<'_, T, true>) -> Result<(), CallbackDisconnected> + Send + 'static; @@ -147,7 +148,7 @@ pub trait Source { fn for_each_generational(&self, mut for_each: F) -> CallbackHandle where T: Send + 'static, - F: for<'a> FnMut(&'a GenerationalValue) + Send + 'static, + F: for<'a> FnMut(DynamicGuard<'_, T, true>) + Send + 'static, { self.for_each_generational_try(move |value| { for_each(value); @@ -165,7 +166,7 @@ pub trait Source { T: Send + 'static, F: for<'a> FnMut(&'a T) -> Result<(), CallbackDisconnected> + Send + 'static, { - self.for_each_generational_try(move |gen| for_each(&gen.value)) + self.for_each_generational_try(move |gen| for_each(&*gen)) } /// Attaches `for_each` to this value so that it is invoked each time the @@ -248,7 +249,7 @@ pub trait Source { fn map_each_generational(&self, mut map: F) -> Dynamic where T: Send + 'static, - F: for<'a> FnMut(&'a GenerationalValue) -> R + Send + 'static, + F: for<'a> FnMut(DynamicGuard<'a, T, true>) -> R + Send + 'static, R: PartialEq + Send + 'static, { let mapped = Dynamic::new(self.map_generational(&mut map)); @@ -269,7 +270,7 @@ pub trait Source { F: for<'a> FnMut(&'a T) -> R + Send + 'static, R: PartialEq + Send + 'static, { - self.map_each_generational(move |gen| map(&gen.value)) + self.map_each_generational(move |gen| map(&*gen)) } /// Creates a new dynamic value that contains the result of invoking `map` @@ -520,16 +521,20 @@ pub trait Destination { impl Source for Arc> { fn try_map_generational( &self, - map: impl FnOnce(&GenerationalValue) -> R, + map: impl FnOnce(DynamicGuard<'_, T, true>) -> R, ) -> Result { let state = self.state()?; - Ok(map(&state.wrapped)) + Ok(map(DynamicGuard { + guard: DynamicOrOwnedGuard::Dynamic(state), + accessed_mut: false, + prevent_notifications: false, + })) } fn for_each_generational_try(&self, mut for_each: F) -> CallbackHandle where T: Send + 'static, - F: for<'a> FnMut(&'a GenerationalValue) -> Result<(), CallbackDisconnected> + F: for<'a> FnMut(DynamicGuard<'a, T, true>) -> Result<(), CallbackDisconnected> + Send + 'static, { @@ -550,7 +555,7 @@ impl Source for Arc> { dynamic_for_each(self, move || { let this = this.upgrade().ok_or(CallbackDisconnected)?; - if let Ok(value) = this.try_map_generational(GenerationalValue::clone) { + if let Ok(value) = this.try_map_generational(|g| g.guard.clone()) { for_each(value)?; } @@ -562,7 +567,7 @@ impl Source for Arc> { impl Source for Dynamic { fn try_map_generational( &self, - map: impl FnOnce(&GenerationalValue) -> R, + map: impl FnOnce(DynamicGuard<'_, T, true>) -> R, ) -> Result { self.0.try_map_generational(map) } @@ -570,7 +575,7 @@ impl Source for Dynamic { fn for_each_generational_try(&self, for_each: F) -> CallbackHandle where T: Send + 'static, - F: for<'a> FnMut(&'a GenerationalValue) -> Result<(), CallbackDisconnected> + F: for<'a> FnMut(DynamicGuard<'_, T, true>) -> Result<(), CallbackDisconnected> + Send + 'static, { @@ -589,10 +594,10 @@ impl Source for Dynamic { impl Source for DynamicReader { fn try_map_generational( &self, - map: impl FnOnce(&GenerationalValue) -> R, + map: impl FnOnce(DynamicGuard<'_, T, true>) -> R, ) -> Result { self.source.try_map_generational(|generational| { - *self.read_generation.lock().ignore_poison() = generational.generation; + *self.read_generation.lock() = generational.generation(); map(generational) }) } @@ -600,7 +605,7 @@ impl Source for DynamicReader { fn for_each_generational_try(&self, for_each: F) -> CallbackHandle where T: Send + 'static, - F: for<'a> FnMut(&'a GenerationalValue) -> Result<(), CallbackDisconnected> + F: for<'a> FnMut(DynamicGuard<'_, T, true>) -> Result<(), CallbackDisconnected> + Send + 'static, { @@ -735,19 +740,23 @@ impl Owned { impl Source for Owned { fn try_map_generational( &self, - map: impl FnOnce(&GenerationalValue) -> R, + map: impl FnOnce(DynamicGuard<'_, T, true>) -> R, ) -> Result { - Ok(map(&self.wrapped.borrow())) + Ok(map(DynamicGuard { + guard: DynamicOrOwnedGuard::Owned(self.wrapped.borrow_mut()), + accessed_mut: false, + prevent_notifications: false, + })) } fn for_each_generational_try(&self, for_each: F) -> CallbackHandle where T: Send + 'static, - F: for<'a> FnMut(&'a GenerationalValue) -> Result<(), CallbackDisconnected> + F: for<'a> FnMut(DynamicGuard<'a, T, true>) -> Result<(), CallbackDisconnected> + Send + 'static, { - let mut callbacks = self.callbacks.active.lock().ignore_poison(); + let mut callbacks = self.callbacks.active.lock(); CallbackHandle(CallbackHandleInner::Single(CallbackHandleData { id: Some(callbacks.push(Box::new(for_each))), owner: None, @@ -760,7 +769,7 @@ impl Source for Owned { T: Clone + Send + 'static, F: FnMut(GenerationalValue) -> Result<(), CallbackDisconnected> + Send + 'static, { - self.for_each_generational_try(move |value| for_each(value.clone())) + self.for_each_generational_try(move |gen| for_each(gen.guard.clone())) } } @@ -775,7 +784,9 @@ where &mut updated, )); if updated { - self.callbacks.invoke(&self.wrapped.borrow()); + self.callbacks.invoke(&mut &self.wrapped, |wrapped| { + DynamicOrOwnedGuard::Owned(wrapped.borrow_mut()) + }); } Ok(result) } @@ -829,7 +840,9 @@ where { fn drop(&mut self) { if self.accessed_mut { - self.owned.callbacks.invoke(&self.borrowed); + self.owned.callbacks.invoke(&mut self.borrowed, |borrowed| { + DynamicOrOwnedGuard::OwnedRef(&mut *borrowed) + }); } } } @@ -850,9 +863,21 @@ impl OwnedCallbacks where T: 'static, { - pub fn invoke(&self, value: &GenerationalValue) { - let mut callbacks = self.active.lock().ignore_poison(); - callbacks.drain_filter(|callback| callback.updated(value).is_err()); + pub fn invoke<'a, U>( + &self, + user: &'a mut U, + value: impl for<'b> Fn(&'b mut U) -> DynamicOrOwnedGuard<'b, T>, + ) { + let mut callbacks = self.active.lock(); + callbacks.drain_filter(|callback| { + callback + .updated(DynamicGuard { + guard: value(user), + accessed_mut: false, + prevent_notifications: false, + }) + .is_err() + }); } } @@ -861,19 +886,21 @@ where T: 'static, { fn remove(&self, id: LotId) { - self.active.lock().ignore_poison().remove(id); + self.active.lock().remove(id); } } trait OwnedCallbackFn: Send + 'static { - fn updated(&mut self, value: &GenerationalValue) -> Result<(), CallbackDisconnected>; + fn updated(&mut self, value: DynamicGuard<'_, T, true>) -> Result<(), CallbackDisconnected>; } impl OwnedCallbackFn for F where - F: for<'a> FnMut(&'a GenerationalValue) -> Result<(), CallbackDisconnected> + Send + 'static, + F: for<'a> FnMut(DynamicGuard<'a, T, true>) -> Result<(), CallbackDisconnected> + + Send + + 'static, { - fn updated(&mut self, value: &GenerationalValue) -> Result<(), CallbackDisconnected> { + fn updated(&mut self, value: DynamicGuard<'_, T, true>) -> Result<(), CallbackDisconnected> { self(value) } } @@ -1104,7 +1131,7 @@ impl Dynamic { /// dynamic. pub fn try_lock(&self) -> Result, DeadlockError> { Ok(DynamicGuard { - guard: self.0.state()?, + guard: DynamicOrOwnedGuard::Dynamic(self.0.state()?), accessed_mut: false, prevent_notifications: false, }) @@ -1112,7 +1139,7 @@ impl Dynamic { fn lock_inner(&self) -> DynamicGuard<'_, T, READONLY> { DynamicGuard { - guard: self.0.state().expect("deadlocked"), + guard: DynamicOrOwnedGuard::Dynamic(self.0.state().expect("deadlocked")), accessed_mut: false, prevent_notifications: false, } @@ -1344,9 +1371,19 @@ where } } +impl<'a, T> DynamicMutexGuard<'a, T> { + fn unlocked(&mut self, while_unlocked: impl FnOnce()) { + MutexGuard::unlocked(&mut self.guard, || { + let current_state = self.dynamic.during_callback_state.lock().take(); + while_unlocked(); + *self.dynamic.during_callback_state.lock() = current_state; + }); + } +} + impl<'a, T> Drop for DynamicMutexGuard<'a, T> { fn drop(&mut self) { - let mut during_state = self.dynamic.during_callback_state.lock().ignore_poison(); + let mut during_state = self.dynamic.during_callback_state.lock(); *during_state = None; drop(during_state); self.dynamic.sync.notify_all(); @@ -1379,20 +1416,19 @@ struct DynamicData { impl DynamicData { fn state(&self) -> Result, DeadlockError> { - let mut during_sync = self.during_callback_state.lock().ignore_poison(); + let mut during_sync = self.during_callback_state.lock(); let current_thread_id = std::thread::current().id(); let guard = loop { match self.state.try_lock() { - Ok(g) => break g, - Err(TryLockError::Poisoned(poision)) => break poision.into_inner(), - Err(TryLockError::WouldBlock) => loop { + Some(g) => break g, + None => loop { match &*during_sync { Some(state) if state.locked_thread == current_thread_id => { return Err(DeadlockError) } Some(_) => { - during_sync = self.sync.wait(during_sync).ignore_poison(); + self.sync.wait(&mut during_sync); } None => break, } @@ -1443,7 +1479,7 @@ where T: Send + 'static, { let state = this.state().expect("deadlocked"); - let mut data = state.callbacks.callbacks.lock().ignore_poison(); + let mut data = state.callbacks.callbacks.lock(); CallbackHandle(CallbackHandleInner::Single(CallbackHandleData { id: Some(data.callbacks.push(Box::new(map))), owner: Some(this.clone()), @@ -1816,7 +1852,7 @@ struct ChangeCallbacksData { impl CallbackCollection for ChangeCallbacksData { fn remove(&self, id: LotId) { - let mut data = self.callbacks.lock().ignore_poison(); + let mut data = self.callbacks.lock(); data.callbacks.remove(id); } } @@ -1842,7 +1878,7 @@ struct ChangeCallbacks { impl Drop for ChangeCallbacks { fn drop(&mut self) { - let mut currently_executing = self.data.currently_executing.lock().expect("lock poisoned"); + let mut currently_executing = self.data.currently_executing.lock(); let current_thread = thread::current().id(); loop { match &*currently_executing { @@ -1854,7 +1890,7 @@ impl Drop for ChangeCallbacks { drop(currently_executing); // Invoke the callbacks - let mut state = self.data.callbacks.lock().ignore_poison(); + let mut state = self.data.callbacks.lock(); // If the callbacks have already been invoked by another // thread such that the callbacks observed the value our // thread wrote, we can skip the callbacks. @@ -1870,8 +1906,7 @@ impl Drop for ChangeCallbacks { // Remove ourselves as the current executor, notifying any // other threads that are waiting. - currently_executing = - self.data.currently_executing.lock().expect("lock poisoned"); + currently_executing = self.data.currently_executing.lock(); *currently_executing = None; drop(currently_executing); self.data.sync.notify_all(); @@ -1886,11 +1921,7 @@ impl Drop for ChangeCallbacks { return; } Some(_) => { - currently_executing = self - .data - .sync - .wait(currently_executing) - .expect("lock poisoned"); + self.data.sync.wait(&mut currently_executing); } } } @@ -1964,13 +1995,57 @@ impl DerefMut for GenerationalValue { } } +#[derive(Debug)] +enum DynamicOrOwnedGuard<'a, T> { + Dynamic(DynamicMutexGuard<'a, T>), + Owned(RefMut<'a, GenerationalValue>), + OwnedRef(&'a mut GenerationalValue), +} +impl<'a, T> DynamicOrOwnedGuard<'a, T> { + fn note_changed(&mut self) -> Option { + match self { + Self::Dynamic(guard) => Some(guard.note_changed()), + Self::Owned(_) | Self::OwnedRef(_) => None, + } + } + + fn unlocked(&mut self, while_unlocked: impl FnOnce()) { + match self { + Self::Dynamic(guard) => guard.unlocked(while_unlocked), + Self::Owned(_) | Self::OwnedRef(_) => while_unlocked(), + } + } +} + +impl<'a, T> Deref for DynamicOrOwnedGuard<'a, T> { + type Target = GenerationalValue; + + fn deref(&self) -> &Self::Target { + match self { + Self::Dynamic(guard) => &guard.wrapped, + Self::Owned(r) => r, + Self::OwnedRef(r) => r, + } + } +} + +impl<'a, T> DerefMut for DynamicOrOwnedGuard<'a, T> { + fn deref_mut(&mut self) -> &mut Self::Target { + match self { + Self::Dynamic(guard) => &mut guard.wrapped, + Self::Owned(r) => r, + Self::OwnedRef(r) => r, + } + } +} + /// An exclusive reference to the contents of a [`Dynamic`]. /// /// If the contents are accessed through [`DerefMut`], all obververs will be /// notified of a change when this guard is dropped. #[derive(Debug)] pub struct DynamicGuard<'a, T, const READONLY: bool = false> { - guard: DynamicMutexGuard<'a, T>, + guard: DynamicOrOwnedGuard<'a, T>, accessed_mut: bool, prevent_notifications: bool, } @@ -1982,7 +2057,7 @@ impl DynamicGuard<'_, T, READONLY> { /// will remain unchanged while the guard is held. #[must_use] pub fn generation(&self) -> Generation { - self.guard.wrapped.generation + self.guard.generation } /// Prevent any access through [`DerefMut`] from triggering change @@ -1996,22 +2071,22 @@ impl<'a, T, const READONLY: bool> Deref for DynamicGuard<'a, T, READONLY> { type Target = T; fn deref(&self) -> &Self::Target { - &self.guard.wrapped.value + &self.guard.value } } impl<'a, T> DerefMut for DynamicGuard<'a, T, false> { fn deref_mut(&mut self) -> &mut Self::Target { self.accessed_mut = true; - &mut self.guard.wrapped.value + &mut self.guard.value } } impl Drop for DynamicGuard<'_, T, READONLY> { fn drop(&mut self) { if self.accessed_mut && !self.prevent_notifications { - let mut callbacks = Some(self.guard.note_changed()); - run_in_bg(move || drop(callbacks.take())); + let callbacks = self.guard.note_changed(); + self.guard.unlocked(|| drop(callbacks)); } } } @@ -2103,7 +2178,7 @@ impl DynamicReader { #[must_use] pub fn lock(&self) -> DynamicGuard<'_, T, true> { DynamicGuard { - guard: self.source.state().expect("deadlocked"), + guard: DynamicOrOwnedGuard::Dynamic(self.source.state().expect("deadlocked")), accessed_mut: false, prevent_notifications: false, } @@ -2113,7 +2188,7 @@ impl DynamicReader { /// reader. #[must_use] pub fn read_generation(&self) -> Generation { - *self.read_generation.lock().ignore_poison() + *self.read_generation.lock() } /// Returns true if the dynamic has been modified since the last time the @@ -2142,13 +2217,12 @@ impl DynamicReader { self.source .during_callback_state .lock() - .ignore_poison() .as_ref() .map_or(true, |state| state.locked_thread != std::thread::current().id()), "deadlocked" ); - let mut state = self.source.state.lock().ignore_poison(); + let mut state = self.source.state.lock(); loop { if state.wrapped.generation != self.read_generation() { return true; @@ -2159,14 +2233,14 @@ impl DynamicReader { } // Wait for a notification of a change, which is synch - state = self.source.sync.wait(state).ignore_poison(); + self.source.sync.wait(&mut state); } } /// Returns true if this reader still has any writers connected to it. #[must_use] pub fn connected(&self) -> bool { - let state = self.source.state.lock().ignore_poison(); + let state = self.source.state.lock(); state.readers < Arc::strong_count(&self.source) && state.on_disconnect.is_some() } @@ -2936,7 +3010,7 @@ macro_rules! impl_tuple_for_each { move |$var: &$type| { $(let $rvar = $rvar.read();)+ let mut for_each = - for_each.lock().ignore_poison(); + for_each.lock(); (for_each)(($(&$avar,)+)); } })); @@ -3108,7 +3182,7 @@ macro_rules! impl_tuple_for_each_cloned { $handles += $var.for_each_cloned((&$for_each, $(&$rvar,)+).with_clone(|(for_each, $($rvar,)+)| { move |$var: $type| { $(let $rvar = $rvar.get();)+ - if let Ok(mut for_each) = + if let Some(mut for_each) = for_each.try_lock() { (for_each)(($($avar,)+)); } @@ -3259,7 +3333,7 @@ impl Validations { { let validation = Dynamic::new(Validation::None); let mut message_mapping = Self::map_to_message(move |value| check(value)); - let error_message = dynamic.map_each_generational(move |value| message_mapping(value)); + let error_message = dynamic.map_each_generational(move |gen| message_mapping(&gen.guard)); validation.set_source((&self.state, &error_message).for_each_cloned({ let mut f = self.generate_validation(dynamic); @@ -3453,7 +3527,7 @@ impl WhenValidation<'_> { let validation = Dynamic::new(Validation::None); let mut map_to_message = Validations::map_to_message(move |value| check(value)); let error_message = - dynamic.map_each_generational(move |generational| map_to_message(generational)); + dynamic.map_each_generational(move |generational| map_to_message(&generational.guard)); let mut f = self.validations.generate_validation(dynamic); let not = self.not; diff --git a/src/widget.rs b/src/widget.rs index 3b9f362..49a5a6d 100644 --- a/src/widget.rs +++ b/src/widget.rs @@ -5,7 +5,7 @@ use std::clone::Clone; use std::fmt::{self, Debug}; use std::ops::{ControlFlow, Deref, DerefMut}; use std::sync::atomic::{self, AtomicU64}; -use std::sync::{Arc, Mutex, MutexGuard}; +use std::sync::Arc; use std::{slice, vec}; use alot::LotId; @@ -15,6 +15,7 @@ use intentional::Assert; use kludgine::app::winit::event::{Ime, MouseButton, MouseScrollDelta, TouchPhase}; use kludgine::app::winit::window::CursorIcon; use kludgine::Color; +use parking_lot::{Mutex, MutexGuard}; use crate::app::{Application, Open, PendingApp, Run}; use crate::context::sealed::Trackable as _; @@ -35,7 +36,6 @@ use crate::styles::{ IntoDynamicComponentValue, Styles, ThemePair, VisualOrder, }; use crate::tree::{Tree, WeakTree}; -use crate::utils::IgnorePoison; use crate::value::{Dynamic, Generation, IntoDynamic, IntoValue, Validation, Value}; use crate::widgets::checkbox::{Checkable, CheckboxState}; use crate::widgets::layers::{OverlayLayer, Tooltipped}; @@ -1404,8 +1404,8 @@ pub struct WidgetInstance { impl Debug for WidgetInstance { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self.data.widget.try_lock() { - Ok(widget) => widget.summarize(f), - Err(_) => f.debug_struct("WidgetInstance").finish_non_exhaustive(), + Some(widget) => widget.summarize(f), + None => f.debug_struct("WidgetInstance").finish_non_exhaustive(), } } } @@ -1539,7 +1539,7 @@ impl WidgetInstance { /// occur due to other widget locks being held. #[must_use] pub fn lock(&self) -> WidgetGuard<'_> { - WidgetGuard(self.data.widget.lock().ignore_poison()) + WidgetGuard(self.data.widget.lock()) } /// Returns the id of the widget that should receive focus after this diff --git a/src/widgets/layers.rs b/src/widgets/layers.rs index 684b937..97b9dd9 100644 --- a/src/widgets/layers.rs +++ b/src/widgets/layers.rs @@ -1,7 +1,7 @@ //! Widgets that stack in the Z-direction. use std::fmt::{self, Debug}; -use std::sync::{Arc, Mutex}; +use std::sync::Arc; use std::time::Duration; use alot::{LotId, OrderedLots}; @@ -9,11 +9,11 @@ use cushy::widget::{RootBehavior, WidgetInstance}; use figures::units::{Lp, Px, UPx}; use figures::{IntoSigned, IntoUnsigned, Point, Rect, Size, Zero}; use intentional::Assert; +use parking_lot::Mutex; use crate::animation::easings::EaseOutQuadradic; use crate::animation::{AnimationHandle, AnimationTarget, IntoAnimate, Spawn, ZeroToOne}; use crate::context::{AsEventContext, EventContext, GraphicsContext, LayoutContext, Trackable}; -use crate::utils::IgnorePoison; use crate::value::{Destination, Dynamic, DynamicGuard, IntoValue, Source, Value}; use crate::widget::{ Callback, MakeWidget, MountedChildren, MountedWidget, Widget, WidgetId, WidgetList, WidgetRef, @@ -641,7 +641,7 @@ struct OverlayLayout { impl Drop for OverlayLayout { fn drop(&mut self) { if let Some(on_dismiss) = &self.on_dismiss { - let mut on_dismiss = on_dismiss.lock().ignore_poison(); + let mut on_dismiss = on_dismiss.lock(); on_dismiss.invoke(()); } } diff --git a/src/widgets/tilemap.rs b/src/widgets/tilemap.rs index 80430e5..69205a4 100644 --- a/src/widgets/tilemap.rs +++ b/src/widgets/tilemap.rs @@ -144,9 +144,7 @@ where fn hover(&mut self, local: Point, context: &mut EventContext<'_>) -> Option { if let Some(tick) = &self.tick { - let Some(size) = context.last_layout().map(|rect| rect.size) else { - return None; - }; + let size = context.last_layout().map(|rect| rect.size)?; let world = tilemap::translate_coordinates(local, context.kludgine.scale(), self.zoom, size); diff --git a/src/window.rs b/src/window.rs index 02c04f5..51e8fc6 100644 --- a/src/window.rs +++ b/src/window.rs @@ -11,7 +11,7 @@ use std::num::{NonZeroU32, TryFromIntError}; use std::ops::{Deref, DerefMut, Not}; use std::path::Path; use std::string::ToString; -use std::sync::{mpsc, Arc, Mutex, MutexGuard, OnceLock}; +use std::sync::{mpsc, Arc, OnceLock}; use std::time::{Duration, Instant}; use ahash::AHashMap; @@ -37,6 +37,7 @@ use kludgine::drawing::Drawing; use kludgine::shapes::Shape; use kludgine::wgpu::{self, CompositeAlphaMode, COPY_BYTES_PER_ROW_ALIGNMENT}; use kludgine::{Color, DrawableExt, Kludgine, KludgineId, Origin, Texture}; +use parking_lot::{Mutex, MutexGuard}; use tracing::Level; use unicode_segmentation::UnicodeSegmentation; @@ -2216,11 +2217,7 @@ impl InnerWindowHandle { if let Some(handle) = pending.handle.get() { let _result = handle.send(message); } else { - pending - .commands - .lock() - .expect("lock poisoned") - .push(message); + pending.commands.lock().push(message); } } InnerWindowHandle::Known(handle) => { @@ -2287,7 +2284,7 @@ impl PendingWindow { let initialized = pending.handle.set(handle.clone()); assert!(initialized.is_ok()); - for command in pending.commands.lock().expect("poisoned").drain(..) { + for command in pending.commands.lock().drain(..) { let _result = handle.send(command); } @@ -3170,7 +3167,7 @@ impl Capture { slice.map_async(wgpu::MapMode::Read, { let map_result = map_result.clone(); move |result| { - *map_result.lock().assert("thread panicked") = Some(result); + *map_result.lock() = Some(result); } }); @@ -3179,7 +3176,7 @@ impl Capture { loop { device.poll(wgpu::Maintain::Poll); - let mut result = map_result.lock().assert("thread panicked"); + let mut result = map_result.lock(); if let Some(result) = result.take() { result?; break;