From 0cc42f2d4fa86f8e97913a37d83914585247eef2 Mon Sep 17 00:00:00 2001 From: Adoo Date: Mon, 11 Sep 2023 11:02:45 +0800 Subject: [PATCH] =?UTF-8?q?fix(core):=20=F0=9F=90=9B=20keep=20static=20dat?= =?UTF-8?q?a=20is=20attached=20to=20pipe=20widget?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/src/builtin_widgets/focus_node.rs | 5 +- core/src/builtin_widgets/key.rs | 2 - core/src/data_widget.rs | 19 +--- core/src/pipe.rs | 121 ++++++++++++++++++++++--- core/src/widget.rs | 4 +- core/src/widget_tree.rs | 22 ++++- core/src/widget_tree/widget_id.rs | 43 +++++---- 7 files changed, 158 insertions(+), 58 deletions(-) diff --git a/core/src/builtin_widgets/focus_node.rs b/core/src/builtin_widgets/focus_node.rs index ba96f0543..f636d1120 100644 --- a/core/src/builtin_widgets/focus_node.rs +++ b/core/src/builtin_widgets/focus_node.rs @@ -1,5 +1,4 @@ use crate::{ - data_widget::attach_to_id, events::focus_mgr::{FocusHandle, FocusType}, impl_query_self_only, prelude::*, @@ -53,7 +52,7 @@ impl ComposeChild for FocusNode { let subject = subject.unwrap_or_else(|| { let listener = LifecycleListener::default(); let subject = listener.lifecycle_stream(); - attach_to_id(id, &mut *ctx.tree.borrow_mut(), |child| { + id.wrap_node(&mut ctx.tree.borrow_mut().arena, |child| { Box::new(DataWidget::new(child, listener)) }); subject @@ -73,7 +72,7 @@ impl ComposeChild for FocusNode { .subscribe(subscribe_fn(this.clone_reader())) .unsubscribe_when_dropped(); - attach_to_id(id, &mut *ctx.tree.borrow_mut(), |child| { + id.wrap_node(&mut ctx.tree.borrow_mut().arena, |child| { let d = DataWidget::new(child, this); Box::new(DataWidget::new( Box::new(d), diff --git a/core/src/builtin_widgets/key.rs b/core/src/builtin_widgets/key.rs index 5813d5eb3..e275d2336 100644 --- a/core/src/builtin_widgets/key.rs +++ b/core/src/builtin_widgets/key.rs @@ -121,8 +121,6 @@ where fn record_before_value(&mut self, value: V) { self.before_value = Some(value); } } -impl_query_self_only!(Key); - macro from_key_impl($($ty: ty : $name: ident)*) { $( impl From<$ty> for Key { diff --git a/core/src/data_widget.rs b/core/src/data_widget.rs index e2449ce6a..523ed8625 100644 --- a/core/src/data_widget.rs +++ b/core/src/data_widget.rs @@ -2,9 +2,7 @@ //! is same as origin widget. use crate::{ - impl_proxy_query, impl_proxy_render, impl_query_self_only, - prelude::*, - widget::{FnWidget, WidgetTree}, + impl_proxy_query, impl_proxy_render, impl_query_self_only, prelude::*, widget::FnWidget, }; pub struct DataWidget { @@ -18,7 +16,7 @@ impl DataWidget { pub fn attach(widget: Widget, data: D) -> Widget { FnWidget::new(move |ctx| { let id = widget.build(ctx); - attach_to_id(id, &mut *ctx.tree.borrow_mut(), |child| { + id.wrap_node(&mut ctx.tree.borrow_mut().arena, |child| { Box::new(Self::new(child, data)) }); id @@ -37,19 +35,6 @@ impl DataWidget { } } -pub(crate) fn attach_to_id( - id: WidgetId, - tree: &mut WidgetTree, - attach_data: impl FnOnce(Box) -> Box, -) { - let mut tmp: Box = Box::new(Void); - let node = id.assert_get_mut(&mut tree.arena); - std::mem::swap(&mut tmp, &mut *node); - - let mut attached = attach_data(tmp); - std::mem::swap(&mut *node, &mut attached); -} - impl_proxy_query!(paths [data, render], DataWidget, , where D: Query + 'static); impl_proxy_render!(proxy render, DataWidget, , where D: Query + 'static); diff --git a/core/src/pipe.rs b/core/src/pipe.rs index 641b66a3e..8f09d83b9 100644 --- a/core/src/pipe.rs +++ b/core/src/pipe.rs @@ -5,7 +5,7 @@ use rxrust::{ subscription::Subscription, }; use std::{ - cell::{Cell, RefCell}, + cell::{Cell, RefCell, UnsafeCell}, convert::Infallible, ops::{Deref, Range}, }; @@ -13,13 +13,10 @@ use std::{ use crate::{ builtin_widgets::{key::AnyKey, Void}, context::{AppCtx, BuildCtx}, - data_widget::attach_to_id, - prelude::{ - AnonymousData, BoxedSingleParent, DataWidget, Multi, MultiChild, MultiParent, SingleChild, - SingleParent, - }, + impl_proxy_render, + prelude::*, ticker::FrameMsg, - widget::{QueryOrder, Widget, WidgetBuilder, WidgetId, WidgetTree}, + widget::{Query, QueryOrder, Render, Widget, WidgetBuilder, WidgetId, WidgetTree}, window::WindowId, }; @@ -88,10 +85,18 @@ impl Pipe { } impl + 'static> WidgetBuilder for Pipe { - fn build(self, ctx: &crate::context::BuildCtx) -> WidgetId { + fn build(self, ctx: &BuildCtx) -> WidgetId { let (v, modifies) = self.unzip(); let id = v.into().build(ctx); let id_share = Sc::new(Cell::new(id)); + + let mut pipe_node = None; + id.wrap_node(&mut ctx.tree.borrow_mut().arena, |r| { + let p = PipeNode::new(r); + pipe_node = Some(p.clone()); + Box::new(p) + }); + let id_share2 = id_share.clone(); let handle = ctx.handle(); let wnd_id = ctx.window().id(); @@ -119,6 +124,28 @@ impl + 'static> WidgetBuilder for Pipe { if !ctx.window().is_in_another_regenerating(id) { let new_id = v.into().build(ctx); + // We use a `PipeNode` wrap the initial widget node, so if the widget node is + // not a `PipeNode` means the node is attached some data, we need to keep the + // data attached when it replaced by the new widget, because the data is the + // static stuff. + // + // Only pipe widget as a normal widget need to do this, because compose child + // widget not support pipe object as its child if it's not a normal widget. Only + // compose child widget has the ability to apply new logic on a widget that + // built. + if let Some(pn) = pipe_node.as_mut() { + let mut tree = ctx.tree.borrow_mut(); + if !id.assert_get(&tree.arena).is::() { + let [old_node, new_node] = tree.get_many_mut(&[id, new_id]); + + std::mem::swap(pn.as_mut(), new_node.as_widget_mut()); + std::mem::swap(old_node, new_node); + } else { + // we know widget node not attached data, we can not care about it now. + pipe_node.take(); + } + } + update_key_status_single(id, new_id, ctx); ctx.insert_after(id, new_id); @@ -343,7 +370,7 @@ fn attach_unsubscribe_guard(id: WidgetId, wnd: WindowId, unsub: impl Subscriptio // auto unsubscribe when the widget is not a root and its parent is None. if let Some(p) = id.parent(&tree.arena) { let guard = AnonymousData::new(Box::new(guard)); - attach_to_id(p, &mut *tree, |d| Box::new(DataWidget::new(d, guard))) + p.wrap_node(&mut tree.arena, |d| Box::new(DataWidget::new(d, guard))) } } }) @@ -461,6 +488,39 @@ fn update_key_states( impl SingleChild for Pipe {} impl MultiChild for Pipe {} +/// `PipeNode` just use to wrap a `Box`, and provide a choice to +/// change the inner `Box` by `UnsafeCell` at a safe time. It's +/// transparent except the `Pipe` widget. +#[derive(Clone)] +struct PipeNode(Sc>>); + +impl PipeNode { + fn new(value: Box) -> Self { Self(Sc::new(UnsafeCell::new(value))) } + + fn as_ref(&self) -> &dyn Render { + // safety: see the `PipeNode` document. + unsafe { &**self.0.get() } + } + + fn as_mut(&mut self) -> &mut Box { + // safety: see the `PipeNode` document. + unsafe { &mut *self.0.get() } + } +} + +impl Query for PipeNode { + fn query_all( + &self, + type_id: std::any::TypeId, + callback: &mut dyn FnMut(&dyn std::any::Any) -> bool, + order: QueryOrder, + ) { + self.as_ref().query_all(type_id, callback, order) + } +} + +impl_proxy_render!(proxy as_ref(), PipeNode); + #[cfg(test)] mod tests { use std::{ @@ -469,8 +529,12 @@ mod tests { }; use crate::{ - builtin_widgets::key::KeyChange, impl_query_self_only, prelude::*, reset_test_env, - test_helper::*, widget::TreeArena, + builtin_widgets::key::{AnyKey, KeyChange}, + impl_query_self_only, + prelude::*, + reset_test_env, + test_helper::*, + widget::TreeArena, }; #[test] @@ -529,6 +593,41 @@ mod tests { assert_eq!(ids[2], new_ids[2]); } + #[test] + fn attach_data_to_pipe_widget() { + reset_test_env!(); + let trigger = Stateful::new(false); + let c_trigger = trigger.clone_reader(); + let w = fn_widget! { + let p = pipe! { + // just use to force update the widget, when trigger modified. + $c_trigger; + MockBox { size: Size::zero() } + }; + @KeyWidget { + key: 0, + value: (), + @ { p } + } + }; + + let mut wnd = TestWindow::new(w); + wnd.draw_frame(); + { + *trigger.write() = true; + } + wnd.draw_frame(); + let tree = wnd.widget_tree.borrow(); + + // the key should still in the root widget after pipe widget updated. + assert!( + tree + .root() + .assert_get(&tree.arena) + .contain_type::>() + ); + } + #[test] fn pipe_widget_mounted_new() { reset_test_env!(); diff --git a/core/src/widget.rs b/core/src/widget.rs index 3cb1b6fa1..0bfc63387 100644 --- a/core/src/widget.rs +++ b/core/src/widget.rs @@ -56,7 +56,7 @@ pub struct Widget(Box WidgetId>); /// A trait to query dynamic type and its inner type on runtime, use this trait /// to provide type information you want framework know. -pub trait Query { +pub trait Query: QueryFiler { /// A type can composed by others, this method query all type(include self) /// match the type id, and call the callback one by one. The callback accept /// an `& dyn Any` of the target type, and return if it want to continue. @@ -137,6 +137,8 @@ impl<'a> dyn Render + 'a { ); hit } + + pub fn is(&self) -> bool { self.query_filter(TypeId::of::()).is_some() } } pub struct FnWidget(F); diff --git a/core/src/widget_tree.rs b/core/src/widget_tree.rs index ccda24c81..74abdd978 100644 --- a/core/src/widget_tree.rs +++ b/core/src/widget_tree.rs @@ -2,6 +2,7 @@ use std::{ cell::RefCell, cmp::Reverse, collections::HashSet, + mem::MaybeUninit, rc::{Rc, Weak}, }; @@ -12,7 +13,7 @@ mod layout_info; use crate::prelude::*; pub use layout_info::*; -use self::widget_id::empty_node; +use self::widget_id::{empty_node, RenderNode}; pub(crate) type DirtySet = Rc>>; @@ -142,7 +143,7 @@ impl WidgetTree { info.size.take(); } - let r = self.arena.get(p.0).unwrap().get(); + let r = p.assert_get(&self.arena); if r.only_sized_by_parent() { break; } @@ -181,6 +182,23 @@ impl WidgetTree { }); id.0.remove_subtree(&mut self.arena); } + + pub(crate) fn get_many_mut( + &mut self, + ids: &[WidgetId; N], + ) -> [&mut RenderNode; N] { + unsafe { + let mut outs: MaybeUninit<[&mut RenderNode; N]> = MaybeUninit::uninit(); + let outs_ptr = outs.as_mut_ptr(); + for (idx, wid) in ids.iter().enumerate() { + let arena = &mut *(&mut self.arena as *mut TreeArena); + let cur = wid.get_node_mut(arena).expect("Invalid widget id."); + + *(*outs_ptr).get_unchecked_mut(idx) = cur; + } + outs.assume_init() + } + } } impl Default for WidgetTree { diff --git a/core/src/widget_tree/widget_id.rs b/core/src/widget_tree/widget_id.rs index 2f94c91be..a3997c05d 100644 --- a/core/src/widget_tree/widget_id.rs +++ b/core/src/widget_tree/widget_id.rs @@ -16,23 +16,12 @@ pub struct WidgetId(pub(crate) NodeId); pub(crate) type TreeArena = Arena; -pub(crate) struct RenderNode { - data: Box, -} +pub(crate) struct RenderNode(Box); impl WidgetId { /// Returns a reference to the node data. pub(crate) fn get<'a, 'b>(self, tree: &'a TreeArena) -> Option<&'a (dyn Render + 'b)> { - self.get_node(tree).map(|n| n.data.as_ref()) - } - - /// Returns a mutable reference to the node data. - pub(crate) fn get_mut(self, tree: &mut TreeArena) -> Option<&mut Box> { - self.get_node_mut(tree).map(|n| &mut n.data) - } - - pub(crate) fn get_node(self, tree: &TreeArena) -> Option<&RenderNode> { - tree.get(self.0).map(|n| n.get()) + tree.get(self.0).map(|n| &*n.get().0) } pub(crate) fn get_node_mut(self, tree: &mut TreeArena) -> Option<&mut RenderNode> { @@ -166,8 +155,19 @@ impl WidgetId { self.get(tree).expect("Widget not exists in the `tree`") } - pub(crate) fn assert_get_mut(self, tree: &mut TreeArena) -> &mut Box { - self.get_mut(tree).expect("Widget not exists in the `tree`") + /// We assume the `f` wrap the widget into a new widget, and keep the old + /// widget as part of the new widget, otherwise, undefined behavior. + pub(crate) fn wrap_node( + self, + tree: &mut TreeArena, + f: impl FnOnce(Box) -> Box, + ) { + let node = self.get_node_mut(tree).unwrap(); + unsafe { + let data = Box::from_raw(&mut *node.0 as *mut _); + let copied = node.replace(f(data)); + std::mem::forget(copied) + } } pub(crate) fn paint_subtree(self, ctx: &mut PaintingCtx) { @@ -213,16 +213,15 @@ impl WidgetId { } pub(crate) fn new_node(arena: &mut TreeArena, node: Box) -> WidgetId { - WidgetId(arena.new_node(RenderNode { data: node })) + WidgetId(arena.new_node(RenderNode(node))) } pub(crate) fn empty_node(arena: &mut TreeArena) -> WidgetId { new_node(arena, Box::new(Void)) } -impl std::ops::Deref for RenderNode { - type Target = dyn Render; - fn deref(&self) -> &Self::Target { self.data.as_ref() } -} +impl RenderNode { + pub(crate) fn as_widget_mut(&mut self) -> &mut Box { &mut self.0 } -impl std::ops::DerefMut for RenderNode { - fn deref_mut(&mut self) -> &mut Self::Target { self.data.as_mut() } + pub(crate) fn replace(&mut self, value: Box) -> Box { + std::mem::replace(&mut self.0, value) + } }