From 9b3e6c13f4d196c2e184f10519dd37a469da1314 Mon Sep 17 00:00:00 2001 From: Jonathan Johnson Date: Tue, 20 Aug 2024 10:31:47 -0700 Subject: [PATCH] Fixed remounting WidgetInstances --- CHANGELOG.md | 7 +++++ src/context.rs | 4 +-- src/tree.rs | 60 ++++++++++++++++++++--------------------- src/widget.rs | 27 +++++++++++++++++++ src/widgets/grid.rs | 8 ++++++ src/widgets/stack.rs | 6 +++++ src/widgets/switcher.rs | 2 +- 7 files changed, 81 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e13656..f844f45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 measurements to ensure accurate rendering. Fixes [#158][158]. - `Input` selection handling when dragging below or above the text field is now handled correctly. +- Nested hierarchies of widgets stored in a reused `WidgetInstance` are now + properly unmounted and remounted. For widgets that store `MountedWidget`s, in + their `mounted` events the widgets should remount their children if needed. + + This fix not only fixes underlying issues with how unmounting was occuring, + but also fixes `Stack`, `Grid`, and `WidgetRef` to automatically remount as + needed. [158]: https://github.com/khonsulabs/cushy/issues/158 diff --git a/src/context.rs b/src/context.rs index 9d13bee..376037e 100644 --- a/src/context.rs +++ b/src/context.rs @@ -852,10 +852,10 @@ pub trait AsEventContext { let Some(mut unmount_context) = context.for_other(&to_unmount) else { continue; }; + let child = unmount_context.widget.widget().clone(); child.lock().as_widget().unmounted(&mut unmount_context); unmount_context.widget.tree.remove_child( - child, - &unmount_context.widget.current_node, + &child, &mut unmount_context.widget.pending_state.unmount_queue, ); } diff --git a/src/tree.rs b/src/tree.rs index 1fdfa20..895dbf3 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -66,21 +66,9 @@ impl Tree { } } - pub fn remove_child( - &self, - child: &MountedWidget, - parent: &MountedWidget, - children_to_unmount: &mut Vec, - ) { + pub fn remove_child(&self, child: &MountedWidget, children_to_unmount: &mut Vec) { let mut data = self.data.lock(); - data.remove_child(child.node_id, parent.node_id, children_to_unmount); - - if child.widget.is_default() { - data.defaults.retain(|id| *id != child.node_id); - } - if child.widget.is_escape() { - data.escapes.retain(|id| *id != child.node_id); - } + data.remove_child(child.node_id, children_to_unmount); } pub(crate) fn set_layout(&self, widget: LotId, rect: Rect) { @@ -285,6 +273,11 @@ impl Tree { data.widget_from_id(id, self) } + pub(crate) fn widget_is_valid(&self, id: LotId) -> bool { + let data = self.data.lock(); + data.nodes.get(id).is_some() + } + pub(crate) fn widget_from_node(&self, id: LotId) -> Option { let data = self.data.lock(); data.widget_from_node(id, self) @@ -390,7 +383,9 @@ impl Tree { id: LotId, ) -> (Styles, Option>, Option>) { let data = self.data.lock(); - let node = data.nodes.get(id).expect("missing widget"); + let Some(node) = data.nodes.get(id) else { + return Default::default(); + }; ( node.effective_styles.clone(), node.theme.clone(), @@ -481,37 +476,42 @@ impl TreeData { } } - fn remove_child( - &mut self, - child: LotId, - parent: LotId, - children_to_unmount: &mut Vec, - ) { + fn remove_child(&mut self, child: LotId, children_to_unmount: &mut Vec) { let Some(removed_node) = self.nodes.remove(child) else { return; }; self.nodes_by_id.remove(&removed_node.widget.id()); - if let Some(parent) = self.nodes.get_mut(parent) { - let index = parent - .children - .iter() - .enumerate() - .find_map(|(index, c)| (*c == child).then_some(index)) - .expect("child not found in parent"); - parent.children.remove(index); + if let Some(parent) = removed_node.parent { + if let Some(parent_node) = self.nodes.get_mut(parent) { + if let Some(index) = parent_node + .children + .iter() + .enumerate() + .find_map(|(index, c)| (*c == child).then_some(index)) + { + parent_node.children.remove(index); + } + } } children_to_unmount.extend( removed_node .children .into_iter() - .filter_map(|id| self.nodes.get(id).map(|node| node.widget.id())), + .filter_map(|id| dbg!(self.nodes.get(id).map(|node| node.widget.id()))), ); if let Some(next_focus) = removed_node.widget.next_focus() { self.previous_focuses.remove(&next_focus); } + self.render_info.order.retain(|info| info.node != child); + if removed_node.widget.is_default() { + self.defaults.retain(|id| *id != child); + } + if removed_node.widget.is_escape() { + self.escapes.retain(|id| *id != child); + } } pub(crate) fn widget_hierarchy(&self, mut widget: LotId, tree: &Tree) -> Vec { diff --git a/src/widget.rs b/src/widget.rs index 3f5fa12..4f2cbcd 100644 --- a/src/widget.rs +++ b/src/widget.rs @@ -306,6 +306,9 @@ pub trait Widget: Send + Debug + 'static { } /// The widget has been mounted into a parent widget. + /// + /// Widgets that contain [`MountedWidget`] references should call + /// [`MountedWidget::remount_if_needed`] in this function. #[allow(unused_variables)] fn mounted(&mut self, context: &mut EventContext<'_>) {} @@ -1718,6 +1721,11 @@ where } /// A [`Widget`] that has been attached to a widget hierarchy. +/// +/// Because [`WidgetInstance`]s can be reused, a mounted widget can be unmounted +/// and eventually remounted. To ensure the widget is in a consistent state, all +/// types that own `MountedWidget`s should call +/// [`MountedWidget::remount_if_needed`] during their `mount()` functions. #[derive(Clone)] pub struct MountedWidget { pub(crate) node_id: LotId, @@ -1736,6 +1744,22 @@ impl MountedWidget { self.tree.upgrade().expect("tree missing") } + /// Remounts this widget, if it was previously unmounted. + pub fn remount_if_needed(&mut self, context: &mut EventContext<'_>) { + if !self.is_mounted() { + *self = context.push_child(self.widget.clone()); + } + } + + /// Returns true if this widget is still mounted in a window. + #[must_use] + pub fn is_mounted(&self) -> bool { + let Some(tree) = self.tree.upgrade() else { + return false; + }; + tree.widget_is_valid(self.node_id) + } + /// Locks the widget for exclusive access. Locking widgets should only be /// done for brief moments of time when you are certain no deadlocks can /// occur due to other widget locks being held. @@ -2414,6 +2438,9 @@ impl WidgetRef { let mut context = context.as_event_context(); self.mounted .entry(&context) + .and_modify(|w| { + w.remount_if_needed(&mut context.as_event_context()); + }) .or_insert_with(|| context.push_child(self.instance.clone())) } diff --git a/src/widgets/grid.rs b/src/widgets/grid.rs index 5a2a3ed..0271c6d 100644 --- a/src/widgets/grid.rs +++ b/src/widgets/grid.rs @@ -144,6 +144,14 @@ impl Widget for Grid { } } + fn mounted(&mut self, context: &mut EventContext<'_>) { + for row in &mut self.live_rows { + for col in row { + col.remount_if_needed(context); + } + } + } + fn layout( &mut self, available_space: Size, diff --git a/src/widgets/stack.rs b/src/widgets/stack.rs index 41b1260..90bc83f 100644 --- a/src/widgets/stack.rs +++ b/src/widgets/stack.rs @@ -126,6 +126,12 @@ impl Widget for Stack { } } + fn mounted(&mut self, context: &mut EventContext<'_>) { + for child in &mut self.synced_children { + child.remount_if_needed(context); + } + } + fn layout( &mut self, available_space: Size, diff --git a/src/widgets/switcher.rs b/src/widgets/switcher.rs index d23f755..766ba83 100644 --- a/src/widgets/switcher.rs +++ b/src/widgets/switcher.rs @@ -57,7 +57,7 @@ impl WrapperWidget for Switcher { let current_source = self.source.get_tracking_invalidate(context); if ¤t_source != self.child.widget() { self.child.unmount_in(context); - self.child = WidgetRef::new(self.source.get()); + self.child = WidgetRef::new(current_source); } context.invalidate_when_changed(&self.source); available_space