diff --git a/core/src/pipe.rs b/core/src/pipe.rs index d175ef291..a3d3c57f7 100644 --- a/core/src/pipe.rs +++ b/core/src/pipe.rs @@ -4,7 +4,6 @@ use rxrust::{ prelude::{BoxIt, ObservableExt, ObservableItem}, subscription::Subscription, }; -use smallvec::SmallVec; use std::{ cell::{Cell, RefCell, UnsafeCell}, convert::Infallible, @@ -109,31 +108,27 @@ pub(crate) trait InnerPipe: Pipe { Self: Sized, Self::Value: 'static, { - let id_share = Sc::new(Cell::new(ctx.tree.borrow().root())); - let id_share2 = id_share.clone(); + let dyn_info = Sc::new(Cell::new(ctx.tree.borrow().root())); + let dyn_info2 = dyn_info.clone(); let (w, modifies) = self.tick_unzip( - Box::new(move |wnd| wnd.mark_widgets_regenerating(id_share2.get(), None)), + Box::new(move |wnd| wnd.mark_widgets_regenerating(dyn_info2.get(), None)), ctx, ); let w = build(w, ctx); - id_share.set(w.id()); + dyn_info.set(w.id()); - let pipe_node = PipeNode::share_capture(w.id(), Box::new(id_share.clone()), ctx); + let pipe_node = PipeNode::share_capture(w.id(), Box::new(dyn_info.clone()), ctx); let c_pipe_node = pipe_node.clone(); let handle = ctx.handle(); - let u = modifies.subscribe(move |(_, w)| { handle.with_ctx(|ctx| { - let id = id_share.get(); + let id = dyn_info.get(); if !ctx.window().is_in_another_regenerating(id) { let new_id = build(w, ctx).consume(); - pipe_node.remove_logic_child_infos(&id_share); - pipe_node - .infos_iter() - .for_each(|i| i.single_replace(id, new_id)); + replace_outside_info(id, &dyn_info, ctx, |info| info.single_replace(id, new_id)); pipe_node.primary_transplant(id, new_id, ctx); update_key_status_single(id, new_id, ctx); @@ -172,11 +167,11 @@ pub(crate) trait InnerPipe: Pipe { widgets }; - let ids_share = Sc::new(RefCell::new(vec![])); - let id_share2 = ids_share.clone(); + let dyn_info = Sc::new(RefCell::new(vec![])); + let dyn_info2 = dyn_info.clone(); let (m, modifies) = self.tick_unzip( Box::new(move |wnd| { - for id in id_share2.borrow().iter() { + for id in dyn_info2.borrow().iter() { wnd.mark_widgets_regenerating(*id, None) } }), @@ -188,24 +183,26 @@ pub(crate) trait InnerPipe: Pipe { // Only need to wrap `PipeNode` for primary widget, because others have no way // to attach information to widgets generated by `build_multi`, except the - // first widget -- if parent is also a multi pipe, it will attach a - // `DynWidgetInfo` to the first widget. - let pipe_node = PipeNode::share_capture(first, Box::new(ids_share.clone()), ctx); + // first widget -- `ComposeChild` only support explicitly pipe multi widget, + // `MultiChild` can't apply logic to its children, only if parent is also a + // multi pipe, it will attach a `DynWidgetInfo` to the first widget. + let pipe_node = PipeNode::share_capture(first, Box::new(dyn_info.clone()), ctx); for w in widgets.iter().skip(1) { - let arena = &ctx.tree.borrow().arena; - w.id() - .assert_get(arena) - .query_most_outside(|node: &PipeNode| node.push_info(ids_share.dyn_clone())); + let arena = &mut ctx.tree.borrow_mut().arena; + let id = w.id(); + if id.assert_get(arena).contain_type::() { + id.attach_data(Box::new(dyn_info.clone()) as DynInfo, arena); + }; } - *ids_share.borrow_mut() = widgets.iter().map(|w| w.id()).collect(); + *dyn_info.borrow_mut() = widgets.iter().map(|w| w.id()).collect(); vec.extend(widgets); let c_pipe_node = pipe_node.clone(); let handle = ctx.handle(); let u = modifies.subscribe(move |(_, m)| { handle.with_ctx(|ctx| { - let old = ids_share.borrow().clone(); + let old = dyn_info.borrow().clone(); if !ctx.window().is_in_another_regenerating(old[0]) { let new = build_multi(m, ctx) @@ -213,13 +210,14 @@ pub(crate) trait InnerPipe: Pipe { .map(Widget::consume) .collect::>(); - pipe_node.remove_logic_child_infos(&ids_share); - pipe_node - .infos_iter() - .for_each(|i| i.multi_replace(&old, &new)); - old[1..].iter().for_each(|w| { - w.assert_get(&ctx.tree.borrow().arena) - .query_most_outside(|node: &PipeNode| node.extend_infos(pipe_node.infos_iter())); + replace_outside_info(old[0], &dyn_info, ctx, |info| { + info.multi_replace(&old, &new) + }); + new[1..].iter().for_each(|w| { + let arena = &mut ctx.tree.borrow_mut().arena; + if w.assert_get(arena).contain_type::() { + w.attach_data(Box::new(dyn_info.clone()) as DynInfo, arena); + }; }); pipe_node.primary_transplant(old[0], new[0], ctx); @@ -254,59 +252,54 @@ pub(crate) trait InnerPipe: Pipe { where Self: Sized, { - fn on_pipe_without_primary( - range: &RangeInclusive, - ctx: &BuildCtx, - f: impl Fn(&PipeNode), - ) { - let arena = &ctx.tree.borrow().arena; - for w in range.end().ancestors(arena) { - if &w == range.start() { - break; + fn attach_pipe_without_primary(ctx: &BuildCtx, info: &Sc>>) { + let range = info.borrow().clone(); + let arena = &mut ctx.tree.borrow_mut().arena; + let mut f = *range.end(); + while &f != range.start() { + if f.assert_get(arena).contain_type::() { + f.attach_data(Box::new(info.clone()) as DynInfo, arena) } - w.assert_get(arena).query_most_outside(&f); + f = f.parent(arena).unwrap(); } } let root = ctx.tree.borrow().root(); - let id_share = Sc::new(RefCell::new(root..=root)); - let id_share2 = id_share.clone(); + let dyn_info = Sc::new(RefCell::new(root..=root)); + let dyn_info2 = dyn_info.clone(); let (v, modifies) = self.tick_unzip( Box::new(move |wnd| { - let rg = id_share2.borrow().clone(); + let rg = dyn_info2.borrow().clone(); wnd.mark_widgets_regenerating(*rg.start(), Some(*rg.end())) }), ctx, ); let (p, child) = compose_child(v); - let pipe_node = PipeNode::share_capture(p.id(), Box::new(id_share.clone()), ctx); + let pipe_node = PipeNode::share_capture(p.id(), Box::new(dyn_info.clone()), ctx); let range = half_to_close_interval(p.id()..child, ctx); - on_pipe_without_primary(&range, ctx, |node| node.push_info(id_share.dyn_clone())); + *dyn_info.borrow_mut() = range; + attach_pipe_without_primary(ctx, &dyn_info); let c_pipe_node = pipe_node.clone(); - *id_share.borrow_mut() = range; let handle = ctx.handle(); let u = modifies.subscribe(move |(_, w)| { handle.with_ctx(|ctx| { - let (top, bottom) = id_share.borrow().clone().into_inner(); + let (top, bottom) = dyn_info.borrow().clone().into_inner(); if !ctx.window().is_in_another_regenerating(top) { let first_child = bottom.first_child(&ctx.tree.borrow().arena).unwrap(); let p = transplant(w, bottom, ctx); let new_rg = half_to_close_interval(p..first_child, ctx); - pipe_node.remove_logic_child_infos(&id_share); - pipe_node - .infos_iter() - .for_each(|i| i.single_range_replace(&(top..=bottom), &new_rg)); - - on_pipe_without_primary(&new_rg, ctx, |node| { - node.extend_infos(pipe_node.infos_iter()) + replace_outside_info(top, &dyn_info, ctx, |info| { + info.single_range_replace(&(top..=bottom), &new_rg); }); + attach_pipe_without_primary(ctx, &dyn_info); pipe_node.primary_transplant(top, p, ctx); + update_key_status_single(top, p, ctx); ctx.insert_after(top, p); @@ -342,11 +335,8 @@ fn disconnect_when_dropped( // if the subscription is closed, we can cancel and unwrap the `PipeNode` // immediately. if u.is_closed() { - node.pop_info(); - if !node.is_pipe_node() { - let v = std::mem::replace(&mut node.as_mut().data, Box::new(Void)); - *id.get_node_mut(tree).unwrap() = v; - } + let v = std::mem::replace(&mut node.as_mut().data, Box::new(Void)); + *id.get_node_mut(tree).unwrap() = v; } else { id.attach_anonymous_data(u.unsubscribe_when_dropped(), tree) } @@ -919,31 +909,16 @@ struct PipeNode(Sc>); struct InnerPipeNode { data: Box, - infos: SmallVec<[Box; 1]>, + dyn_info: Box, } trait DynWidgetInfo: Any { fn single_replace(&self, old: WidgetId, new: WidgetId); fn single_range_replace(&self, old: &RangeInclusive, new: &RangeInclusive); fn multi_replace(&self, old: &[WidgetId], new: &[WidgetId]); - fn ptr_eq(&self, other: &dyn DynWidgetInfo) -> bool; - fn dyn_clone(&self) -> Box; fn as_any(&self) -> &dyn Any; } -macro_rules! impl_trivial_methods { - () => { - fn ptr_eq(&self, other: &dyn DynWidgetInfo) -> bool { - other - .as_any() - .downcast_ref::() - .map_or(false, |other| Sc::ptr_eq(self, other)) - } - - fn dyn_clone(&self) -> Box { Box::new(self.clone()) } - - fn as_any(&self) -> &dyn Any { self } - }; -} +type DynInfo = Box; impl DynWidgetInfo for Sc> { fn single_replace(&self, old: WidgetId, new: WidgetId) { @@ -965,7 +940,7 @@ impl DynWidgetInfo for Sc> { unreachable!("Single pipe node never have multi pipe child."); } - impl_trivial_methods!(); + fn as_any(&self) -> &dyn Any { self } } impl DynWidgetInfo for Sc>> { @@ -993,7 +968,7 @@ impl DynWidgetInfo for Sc>> { unreachable!("Single parent node never have multi pipe child."); } - impl_trivial_methods!(); + fn as_any(&self) -> &dyn Any { self } } impl DynWidgetInfo for Sc>> { @@ -1023,76 +998,33 @@ impl DynWidgetInfo for Sc>> { } } - impl_trivial_methods!(); + fn as_any(&self) -> &dyn Any { self } } impl PipeNode { - fn share_capture(id: WidgetId, info: Box, ctx: &BuildCtx) -> Self { + fn share_capture(id: WidgetId, dyn_info: Box, ctx: &BuildCtx) -> Self { let tree = &mut ctx.tree.borrow_mut().arena; - let mut pipe_node = id - .assert_get(tree) - .query_most_outside(|node: &PipeNode| node.clone()); - if pipe_node.is_none() { - id.wrap_node(tree, |r| { - let inner_node = InnerPipeNode { data: r, infos: SmallVec::new() }; - let p = Self(Sc::new(UnsafeCell::new(inner_node))); - pipe_node = Some(p.clone()); - Box::new(RenderProxy::new(p)) - }); - } + let mut pipe_node = None; + + id.wrap_node(tree, |r| { + let inner_node = InnerPipeNode { data: r, dyn_info }; + let p = Self(Sc::new(UnsafeCell::new(inner_node))); + pipe_node = Some(p.clone()); + Box::new(RenderProxy::new(p)) + }); + // Safety: init before. - let node = unsafe { pipe_node.unwrap_unchecked() }; - node.push_info(info); - node + unsafe { pipe_node.unwrap_unchecked() } } // update the primary `PipeNode`. fn primary_transplant(&self, old: WidgetId, new: WidgetId, ctx: &BuildCtx) { - let mut t = ctx.tree.borrow_mut(); - new.assert_get(&t.arena).query_most_outside(|n: &PipeNode| { - n.degenerate().for_each(|info| self.front_push_info(info)); - }); - - let [old_node, new_node] = t.get_many_mut(&[old, new]); + let mut tree = ctx.tree.borrow_mut(); + let [old_node, new_node] = tree.get_many_mut(&[old, new]); std::mem::swap(&mut self.as_mut().data, new_node); std::mem::swap(old_node, new_node); } - /// remove dynamic widget information until to `to`, and return the rest. - fn remove_logic_child_infos(&self, to: &dyn DynWidgetInfo) { - let infos = &mut self.as_mut().infos; - if infos.len() > 1 { - let idx = infos.iter().position(|info| info.ptr_eq(to)).unwrap(); - infos.drain(..idx); - } else { - // if the `infos` only have one element, that must be the `to` itself. - assert!(infos[0].ptr_eq(to)) - } - } - - fn infos_iter(&self) -> impl Iterator> { - self.as_ref().infos.iter() - } - - fn front_push_info(&self, info: Box) { self.as_mut().infos.insert(0, info); } - - fn extend_infos<'i>(&self, iter: impl Iterator>) { - self - .as_mut() - .infos - .extend(iter.map(|info| info.dyn_clone())); - } - - fn push_info(&self, info: Box) { self.as_mut().infos.push(info); } - - fn pop_info(&self) { self.as_mut().infos.pop(); } - - // let's the `PipeNode` as a normal `Render` widget and can't be query, return - // all `DynWidgetInfo`. - fn degenerate(&self) -> impl Iterator> + '_ { - self.as_mut().infos.drain(..) - } - fn as_ref(&self) -> &InnerPipeNode { // safety: see the `PipeNode` document. unsafe { &*self.0.get() } @@ -1103,10 +1035,18 @@ impl PipeNode { // safety: see the `PipeNode` document. unsafe { &mut *self.0.get() } } +} + +fn replace_outside_info(id: WidgetId, from: &Sc, ctx: &BuildCtx, cb: impl Fn(&DynInfo)) { + id.assert_get(&ctx.tree.borrow().arena) + .query_type_outside_first(|info: &DynInfo| { + cb(info); - /// `PipeNode` without `DynWidgetInfo` is a normal `Render` widget. For - /// example, called `degenerate`. - fn is_pipe_node(&self) -> bool { !self.as_ref().infos.is_empty() } + info + .as_any() + .downcast_ref::>() + .map_or(true, |info| !Sc::ptr_eq(info, from)) + }); } impl Query for PipeNode { @@ -1115,14 +1055,8 @@ impl Query for PipeNode { type_id: TypeId, callback: &mut dyn FnMut(&dyn Any) -> bool, ) -> bool { - if self.as_ref().data.query_inside_first(type_id, callback) - && type_id == TypeId::of::() - && self.is_pipe_node() - { - callback(self) - } else { - true - } + let p = self.as_ref(); + p.data.query_inside_first(type_id, callback) && p.dyn_info.query_inside_first(type_id, callback) } fn query_outside_first( @@ -1130,14 +1064,16 @@ impl Query for PipeNode { type_id: TypeId, callback: &mut dyn FnMut(&dyn Any) -> bool, ) -> bool { - if type_id == TypeId::of::() && self.is_pipe_node() && !callback(self) { - return false; - } - - self.as_ref().data.query_outside_first(type_id, callback) + let p = self.as_ref(); + p.dyn_info.query_outside_first(type_id, callback) + && p.data.query_outside_first(type_id, callback) } } +impl Query for Box { + crate::widget::impl_query_self_only!(); +} + impl RenderTarget for PipeNode { type Target = dyn Render; fn proxy(&self, f: impl FnOnce(&Self::Target) -> V) -> V { f(&*self.as_ref().data) } @@ -1836,4 +1772,38 @@ mod tests { wnd.draw_frame(); assert_layout_result_by_path!(wnd, { path = [0], size == Size::new(1., 1.), }); } + + #[test] + fn fix_pipe_gen_pipe_widget_leak() { + reset_test_env!(); + + let parent = State::value(true); + let child = State::value(true); + let hit_count = State::value(0); + let c_parent = parent.clone_writer(); + let c_child = child.clone_writer(); + let c_hit_count = hit_count.clone_writer(); + + let w = fn_widget! { + pipe!($parent;).map(move |_| { + pipe!($child;).map(move |_| { + *$hit_count.write() += 1; + Void + }) + }) + }; + + let mut wnd = TestWindow::new(w); + wnd.draw_frame(); + assert_eq!(*c_hit_count.read(), 1); + + *c_parent.write() = false; + wnd.draw_frame(); + assert_eq!(*c_hit_count.read(), 2); + + *c_child.write() = false; + wnd.draw_frame(); + // if the child pipe not reset, the hit count will be 4. + assert_eq!(*c_hit_count.read(), 3); + } } diff --git a/core/src/state.rs b/core/src/state.rs index 4911eff5c..62aef7ee7 100644 --- a/core/src/state.rs +++ b/core/src/state.rs @@ -526,28 +526,31 @@ mod tests { let track_split = Sc::new(Cell::new(0)); let c_origin = track_origin.clone(); - origin.modifies().subscribe(move |_| { - c_origin.set(c_origin.get() + 1); + origin.modifies().subscribe(move |s| { + c_origin.set(c_origin.get() + s.bits()); }); let c_split = track_split.clone(); - split.modifies().subscribe(move |_| { - c_split.set(c_split.get() + 1); + split.modifies().subscribe(move |s| { + c_split.set(c_split.get() + s.bits()); }); - *split.write() = 1; + *split.write() = 0; Timer::wake_timeout_futures(); AppCtx::run_until_stalled(); - assert_eq!(track_origin.get(), 0); - assert_eq!(track_split.get(), 1); + assert_eq!(track_origin.get(), ModifyScope::DATA.bits()); + assert_eq!(track_split.get(), ModifyScope::BOTH.bits()); - origin.write().b = 1; + origin.write().b = 0; Timer::wake_timeout_futures(); AppCtx::run_until_stalled(); - assert_eq!(track_origin.get(), 1); + assert_eq!( + track_origin.get(), + ModifyScope::DATA.bits() + ModifyScope::BOTH.bits() + ); // splitted downstream will not be notified. - assert_eq!(track_split.get(), 1); + assert_eq!(track_split.get(), ModifyScope::BOTH.bits()); } #[test] diff --git a/core/src/state/splitted_state.rs b/core/src/state/splitted_state.rs index d3b43f4e0..585970cda 100644 --- a/core/src/state/splitted_state.rs +++ b/core/src/state/splitted_state.rs @@ -183,6 +183,15 @@ where self.create_at > self.origin.time_stamp(), "A splitted writer is invalid because its origin state is modified after it created." ); + + let modify_scope = orig.modify_scope; + + // the origin mark as a silent write, because split writer not effect the origin + // state in ribir framework level. But keep notify in the data level. + assert!(!orig.modified); + orig.modify_scope.remove(ModifyScope::FRAMEWORK); + orig.modified = true; + let value = orig .value .take() @@ -191,7 +200,7 @@ where WriteRef { value, modified: false, - modify_scope: orig.modify_scope, + modify_scope, control: self, } } diff --git a/examples/todos/src/ui.rs b/examples/todos/src/ui.rs index 9176309e0..4bb6ab21b 100644 --- a/examples/todos/src/ui.rs +++ b/examples/todos/src/ui.rs @@ -53,34 +53,36 @@ fn task_lists(this: &impl StateWriter, cond: fn(&Task) -> bool) - move |todos| todos.get_task(id).unwrap(), move |todos| todos.get_task_mut(id).unwrap(), ); - let item = pipe!(*$editing == Some($task.id())).map(move |b|{ - if b { - @Container { - size: Size::new(f32::INFINITY, 64.), - @{ - let input = input(Some($task.label.clone()), move |text|{ - $task.write().label = text.to_string(); - *$editing.write() = None; - }); - @$input { - v_align: VAlign::Center, - on_key_down: move |e| { - if e.key_code() == &PhysicalKey::Code(KeyCode::Escape) { - *$editing.write() = None; + let item = pipe!(*$editing == Some($task.id())) + .value_chain(|s| s.distinct_until_changed().box_it()) + .map(move |b|{ + if b { + @Container { + size: Size::new(f32::INFINITY, 64.), + @{ + let input = input(Some($task.label.clone()), move |text|{ + $task.write().label = text.to_string(); + *$editing.write() = None; + }); + @$input { + v_align: VAlign::Center, + on_key_down: move |e| { + if e.key_code() == &PhysicalKey::Code(KeyCode::Escape) { + *$editing.write() = None; + } } } } - } - }.widget_build(ctx!()) + }.widget_build(ctx!()) - } else { - let _hint = || $stagger.write(); - let item = task_item_widget(task.clone_writer(), stagger.clone_writer()); - @$item { - on_double_tap: move |_| *$editing.write() = Some(id) - }.widget_build(ctx!()) - } - }); + } else { + let _hint = || $stagger.write(); + let item = task_item_widget(task.clone_writer(), stagger.clone_writer()); + @$item { + on_double_tap: move |_| *$editing.write() = Some(id) + }.widget_build(ctx!()) + } + }); widgets.push(item); } diff --git a/macros/src/pipe_macro.rs b/macros/src/pipe_macro.rs index 18cb41193..42ea535db 100644 --- a/macros/src/pipe_macro.rs +++ b/macros/src/pipe_macro.rs @@ -49,7 +49,7 @@ impl ToTokens for PipeMacro { #refs let _ctx_handle_ಠ_ಠ = ctx!().handle(); MapPipe::new( - ModifiesPipe::new(#upstream.box_it()), + ModifiesPipe::new(#upstream.filter(|s| s.contains(ModifyScope::FRAMEWORK)).box_it()), move |_: ModifyScope| { _ctx_handle_ಠ_ಠ .with_ctx(|ctx!(): &BuildCtx<'_>| { #(#expr)* })