Skip to content

Commit

Permalink
fix(core): 🐛 keep static data is attached to pipe widget
Browse files Browse the repository at this point in the history
  • Loading branch information
M-Adoo committed Sep 11, 2023
1 parent d921d9e commit 6d38da8
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 58 deletions.
5 changes: 2 additions & 3 deletions core/src/builtin_widgets/focus_node.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::{
data_widget::attach_to_id,
events::focus_mgr::{FocusHandle, FocusType},
impl_query_self_only,
prelude::*,
Expand Down Expand Up @@ -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
Expand All @@ -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),
Expand Down
2 changes: 0 additions & 2 deletions core/src/builtin_widgets/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
19 changes: 2 additions & 17 deletions core/src/data_widget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<D> {
Expand All @@ -18,7 +16,7 @@ impl<D: Query + 'static> DataWidget<D> {
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
Expand All @@ -37,19 +35,6 @@ impl<D: Query + 'static> DataWidget<D> {
}
}

pub(crate) fn attach_to_id(
id: WidgetId,
tree: &mut WidgetTree,
attach_data: impl FnOnce(Box<dyn Render>) -> Box<dyn Render>,
) {
let mut tmp: Box<dyn Render> = 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<D>, <D>, where D: Query + 'static);
impl_proxy_render!(proxy render, DataWidget<D>, <D>, where D: Query + 'static);

Expand Down
121 changes: 110 additions & 11 deletions core/src/pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,18 @@ use rxrust::{
subscription::Subscription,
};
use std::{
cell::{Cell, RefCell},
cell::{Cell, RefCell, UnsafeCell},
convert::Infallible,
ops::{Deref, Range},
};

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,
};

Expand Down Expand Up @@ -83,10 +80,18 @@ impl<V> Pipe<V> {
}

impl<W: Into<Widget> + 'static> WidgetBuilder for Pipe<W> {
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();
Expand All @@ -113,6 +118,28 @@ impl<W: Into<Widget> + 'static> WidgetBuilder for Pipe<W> {
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::<PipeNode>() {
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);
Expand Down Expand Up @@ -335,7 +362,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)))
}
}
})
Expand Down Expand Up @@ -453,6 +480,39 @@ fn update_key_states(
impl<W: SingleChild> SingleChild for Pipe<W> {}
impl<W: MultiChild> MultiChild for Pipe<W> {}

/// `PipeNode` just use to wrap a `Box<dyn Render>`, and provide a choice to
/// change the inner `Box<dyn Render>` by `UnsafeCell` at a safe time. It's
/// transparent except the `Pipe` widget.
#[derive(Clone)]
struct PipeNode(Sc<UnsafeCell<Box<dyn Render>>>);

impl PipeNode {
fn new(value: Box<dyn Render>) -> 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<dyn Render> {
// 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::{
Expand All @@ -461,8 +521,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]
Expand Down Expand Up @@ -521,6 +585,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::<Box<dyn AnyKey>>()
);
}

#[test]
fn pipe_widget_mounted_new() {
reset_test_env!();
Expand Down
4 changes: 3 additions & 1 deletion core/src/widget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub struct Widget(Box<dyn FnOnce(&BuildCtx) -> 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.
Expand Down Expand Up @@ -137,6 +137,8 @@ impl<'a> dyn Render + 'a {
);
hit
}

pub fn is<T: Any>(&self) -> bool { self.query_filter(TypeId::of::<T>()).is_some() }
}

pub struct FnWidget<F>(F);
Expand Down
22 changes: 20 additions & 2 deletions core/src/widget_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::{
cell::RefCell,
cmp::Reverse,
collections::HashSet,
mem::MaybeUninit,
rc::{Rc, Weak},
};

Expand All @@ -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<RefCell<HashSet<WidgetId, ahash::RandomState>>>;

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -181,6 +182,23 @@ impl WidgetTree {
});
id.0.remove_subtree(&mut self.arena);
}

pub(crate) fn get_many_mut<const N: usize>(
&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 {
Expand Down
43 changes: 21 additions & 22 deletions core/src/widget_tree/widget_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,12 @@ pub struct WidgetId(pub(crate) NodeId);

pub(crate) type TreeArena = Arena<RenderNode>;

pub(crate) struct RenderNode {
data: Box<dyn Render>,
}
pub(crate) struct RenderNode(Box<dyn Render>);

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<dyn Render>> {
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> {
Expand Down Expand Up @@ -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<dyn Render> {
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<dyn Render>) -> Box<dyn Render>,
) {
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) {
Expand Down Expand Up @@ -213,16 +213,15 @@ impl WidgetId {
}

pub(crate) fn new_node(arena: &mut TreeArena, node: Box<dyn Render>) -> 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<dyn Render> { &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<dyn Render>) -> Box<dyn Render> {
std::mem::replace(&mut self.0, value)
}
}

0 comments on commit 6d38da8

Please sign in to comment.