Skip to content

Commit

Permalink
fix(core): 🐛 MixBuiltin does not currently support merging.
Browse files Browse the repository at this point in the history
This limitation exists because even though multiple `MixBuiltin` can be attached to the same widget, they may have different lifetimes. For instance, in the case of MixBuiltin(Pipe(MixBuiltin)), the outer `MixBuiltin` should outlive the inner one.
  • Loading branch information
M-Adoo committed Oct 22, 2024
1 parent c5c3904 commit eac6382
Show file tree
Hide file tree
Showing 41 changed files with 395 additions and 318 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ Please only add new entries below the [Unreleased](#unreleased---releasedate) he
- **core**: Setting the theme before running the app results in the tree being constructed twice. (#637, @M-Adoo)
- **core**: Resolve a crash occurring in a class implementation with multiple children. (#637 @M-Adoo)
- **core**: Nodes created by a class implementation may not be disposed of when switching to another class. (#637 @M-Adoo)
- **core**: When merge multiple `MixBuiltin` widgets, there may be a premature dropping of the outer `MixBuiltin` before it should occur. (#639 @M-Adoo)

### Breaking

- **macros**: Using `@ $w { ... }` will no longer automatically wrap a `FatObj` for `w`. (#639 @M-Adoo)

## [0.4.0-alpha.12] - 2024-10-09

Expand Down
20 changes: 10 additions & 10 deletions core/src/builtin_widgets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,25 +110,25 @@ pub struct FatObj<T> {
host: T,
host_id: LazyWidgetId,
id: LazyWidgetId,
mix_builtin: Option<MixBuiltin>,
request_focus: Option<State<RequestFocus>>,
padding: Option<State<Padding>>,
fitted_box: Option<State<FittedBox>>,
constrained_box: Option<State<ConstrainedBox>>,
box_decoration: Option<State<BoxDecoration>>,
foreground: Option<State<Foreground>>,
padding: Option<State<Padding>>,
scrollable: Option<State<ScrollableWidget>>,
layout_box: Option<State<LayoutBox>>,
mix_builtin: Option<MixBuiltin>,
request_focus: Option<State<RequestFocus>>,
cursor: Option<State<Cursor>>,
margin: Option<State<Margin>>,
scrollable: Option<State<ScrollableWidget>>,
constrained_box: Option<State<ConstrainedBox>>,
transform: Option<State<TransformWidget>>,
opacity: Option<State<Opacity>>,
visibility: Option<State<Visibility>>,
class: Option<State<Class>>,
h_align: Option<State<HAlignWidget>>,
v_align: Option<State<VAlignWidget>>,
relative_anchor: Option<State<RelativeAnchor>>,
global_anchor: Option<State<GlobalAnchor>>,
visibility: Option<State<Visibility>>,
opacity: Option<State<Opacity>>,
class: Option<State<Class>>,
painting_style: Option<State<PaintingStyleWidget>>,
text_style: Option<State<TextStyleWidget>>,
keep_alive: Option<State<KeepAlive>>,
Expand Down Expand Up @@ -170,7 +170,6 @@ impl<T> FatObj<T> {

/// Maps an `FatObj<T>` to `FatObj<V>` by applying a function to the host
/// object.
#[track_caller]
pub fn map<V>(self, f: impl FnOnce(T) -> V) -> FatObj<V> {
FatObj {
host: f(self.host),
Expand Down Expand Up @@ -216,6 +215,7 @@ impl<T> FatObj<T> {
&& self.cursor.is_none()
&& self.margin.is_none()
&& self.scrollable.is_none()
&& self.constrained_box.is_none()
&& self.transform.is_none()
&& self.h_align.is_none()
&& self.v_align.is_none()
Expand Down Expand Up @@ -941,8 +941,8 @@ impl<'a> FatObj<Widget<'a>> {
cursor,
margin,
transform,
visibility,
opacity,
visibility,
class,
h_align,
v_align,
Expand Down
2 changes: 1 addition & 1 deletion core/src/builtin_widgets/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ mod tests {
let mut theme = Theme::default();
theme
.classes
.insert(MARGIN, |w| fn_widget! { @ $w{ margin: EdgeInsets::all(10.) } }.into_widget());
.insert(MARGIN, style_class!(margin: EdgeInsets::all(10.)));
theme.classes.insert(SCALE_2X, |w| {
fn_widget! {
@MockBox {
Expand Down
3 changes: 2 additions & 1 deletion core/src/builtin_widgets/cursor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ impl<'c> ComposeChild<'c> for Cursor {
fn compose_child(this: impl StateWriter<Value = Self>, child: Self::Child) -> Widget<'c> {
fn_widget! {
let save_cursor: Stateful<Option<CursorIcon>> = Stateful::new(None);
@$child {
let child = FatObj::new(child);
@ $child {
on_pointer_enter: move |e: &mut PointerEvent| {
if e.point_type == PointerType::Mouse
&& e.mouse_buttons() == MouseButtons::empty()
Expand Down
21 changes: 8 additions & 13 deletions core/src/builtin_widgets/focus_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ impl<'c> ComposeChild<'c> for RequestFocus {
type Child = Widget<'c>;
fn compose_child(this: impl StateWriter<Value = Self>, child: Self::Child) -> Widget<'c> {
fn_widget! {
@$child {
let child = FatObj::new(child);
@ $child {
on_mounted: move |e| {
let handle = e.window().focus_mgr.borrow().focus_handle(e.id);
$this.silent().handle = Some(handle);
Expand Down Expand Up @@ -51,18 +52,12 @@ mod tests {
reset_test_env!();

let widget = fn_widget! {
@MixBuiltin {
tab_index: 0i16, auto_focus: false,
@MixBuiltin {
tab_index: 0i16, auto_focus: false,
@MixBuiltin {
tab_index: 0i16, auto_focus: false,
@MockBox {
size: Size::default(),
}
}
}
}
let m = @MockBox {
tab_index: 0i16,
size: Size::default(),
};
let m = @ $m { tab_index: 0i16, };
@ $m { tab_index: 0i16 }
};

let wnd = TestWindow::new(widget);
Expand Down
1 change: 1 addition & 0 deletions core/src/builtin_widgets/focus_scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ impl<'c> ComposeChild<'c> for FocusScope {
type Child = Widget<'c>;
fn compose_child(this: impl StateWriter<Value = Self>, child: Self::Child) -> Widget<'c> {
fn_widget! {
let child = FatObj::new(child);
@ $child {
on_mounted: move |e| e.window().add_focus_node(e.id, false, FocusType::Scope),
on_disposed: move|e| e.window().remove_focus_node(e.id, FocusType::Scope),
Expand Down
2 changes: 1 addition & 1 deletion core/src/builtin_widgets/global_anchor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ impl<'c> ComposeChild<'c> for GlobalAnchor {
.frame_tick_stream()
.filter(|msg| matches!(msg, FrameMsg::LayoutReady(_)));

let mut child = @$child {};
let mut child = FatObj::new(child);
let wid = child.lazy_id();
let u = watch!(($this.get_global_anchor(), $child.layout_size()))
.sample(tick_of_layout_ready)
Expand Down
12 changes: 5 additions & 7 deletions core/src/builtin_widgets/layout_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,12 @@ impl<'c> ComposeChild<'c> for LayoutBox {
type Child = Widget<'c>;
fn compose_child(this: impl StateWriter<Value = Self>, child: Self::Child) -> Widget<'c> {
fn_widget! {
@ $child {
on_performed_layout: move |e| {
let new_rect = e.box_rect().unwrap();
if $this.rect != new_rect {
$this.silent().rect = new_rect;
}
FatObj::new(child).on_performed_layout(move |e| {
let new_rect = e.box_rect().unwrap();
if $this.rect != new_rect {
$this.silent().rect = new_rect;
}
}
})
}
.into_widget()
}
Expand Down
104 changes: 46 additions & 58 deletions core/src/builtin_widgets/mix_builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,11 @@ impl MixFlags {
}
}

pub fn tab_index(&self) -> i16 { (self.bits() >> 48) as i16 }
pub fn tab_index(&self) -> Option<i16> {
self
.contains(MixFlags::Focus)
.then(|| (self.bits() >> 48) as i16)
}

pub fn set_tab_index(&mut self, tab_idx: i16) {
self.insert(MixFlags::Focus);
Expand Down Expand Up @@ -354,45 +358,12 @@ impl MixBuiltin {
}
}

fn merge(&self, other: Self) {
let tab_index = self.flags.read().tab_index();
let other_tab_index = other.flags.read().tab_index();
let mut this_flags = self.flags.write();
this_flags.insert(*other.flags.read());
if other_tab_index != 0 {
this_flags.set_tab_index(other_tab_index);
} else if tab_index != 0 {
this_flags.set_tab_index(tab_index);
}
drop(this_flags);
let Self { flags, subject } = other;

// if the other `MixFlags` is a writer, we need to subscribe it.
if let Err(other) = flags.into_reader() {
let this_flags = self.flags.clone_writer();
let u = other
.modifies()
.distinct_until_changed()
.subscribe(move |_| {
let flags = *other.read();
let mut this = this_flags.write();
this.insert(flags);
this.set_tab_index(flags.tab_index());
});
self.on_disposed(|_| u.unsubscribe());
}

fn subscribe_fn(subject: EventSubject) -> impl FnMut(&mut Event) {
move |e: &mut Event| subject.clone().next(e)
}
self.subject().subscribe(subscribe_fn(subject));
}

fn callbacks_for_focus_node(&self) {
self
.on_mounted(move |e| {
if let Some(mix) = e.query::<MixBuiltin>() {
let auto_focus = mix.flags.read().is_auto_focus();
let mut all_mix = e.query_all_iter::<MixBuiltin>().peekable();
if all_mix.peek().is_some() {
let auto_focus = all_mix.any(|mix| mix.flags.read().is_auto_focus());
e.window()
.add_focus_node(e.id, auto_focus, FocusType::Node)
}
Expand Down Expand Up @@ -428,27 +399,10 @@ fn life_fn_once_to_fn_mut(
impl<'c> ComposeChild<'c> for MixBuiltin {
type Child = Widget<'c>;
fn compose_child(this: impl StateWriter<Value = Self>, child: Self::Child) -> Widget<'c> {
child.on_build(move |id, ctx| match this.try_into_value() {
Ok(this) => {
if let Some(m) = id.query_ref::<MixBuiltin>(ctx.tree()) {
if !m.contain_flag(MixFlags::Focus) && this.contain_flag(MixFlags::Focus) {
this.callbacks_for_focus_node();
}
m.merge(this);
} else {
if this.contain_flag(MixFlags::Focus) {
this.callbacks_for_focus_node();
}
id.attach_data(Box::new(Queryable(this)), ctx.tree_mut());
}
}
Err(this) => {
if this.read().contain_flag(MixFlags::Focus) {
this.read().callbacks_for_focus_node();
}
id.attach_data(Box::new(this), ctx.tree_mut())
}
})
if this.read().contain_flag(MixFlags::Focus) {
this.read().callbacks_for_focus_node();
}
child.try_unwrap_state_and_attach(this)
}
}

Expand Down Expand Up @@ -507,3 +461,37 @@ impl Clone for MixBuiltin {
Self { flags, subject: self.subject.clone() }
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::{reset_test_env, test_helper::*};

#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
#[test]
fn mix_should_not_merge() {
reset_test_env!();

let (trigger, w_trigger) = split_value(0);
let (outer_layout, w_outer_layout) = split_value(0);
let mix_keep = fn_widget! {
let pipe_w = FatObj::new( pipe! {
$trigger;
@Void { on_performed_layout: move |_| {} }
});

@ $pipe_w {
on_performed_layout: move |_| *$w_outer_layout.write() +=1 ,
}
};

let mut wnd = TestWindow::new(mix_keep);
wnd.draw_frame();
assert_eq!(*outer_layout.read(), 1);

*w_trigger.write() = 1;

wnd.draw_frame();
assert_eq!(*outer_layout.read(), 2);
}
}
1 change: 1 addition & 0 deletions core/src/builtin_widgets/scrollable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ impl<'c> ComposeChild<'c> for ScrollableWidget {
clamp_dim: ClampDim::MAX_SIZE,
};

let child = FatObj::new(child);
let mut child = @ $child {
anchor: pipe!{
let this = $this;
Expand Down
8 changes: 7 additions & 1 deletion core/src/context/widget_ctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ pub trait WidgetCtx {
/// Translates the widget pos from the coordinate system of `w` to this widget
/// system.
fn map_from(&self, pos: Point, w: WidgetId) -> Point;
/// Query all references to the `T` if it is shared within the widget
/// represented by this context.
fn query_all_iter<T: 'static>(&self) -> impl DoubleEndedIterator<Item = QueryRef<T>>;
/// Query a reference to the `T` if it is shared within the widget
/// represented by this context.
///
Expand Down Expand Up @@ -182,7 +185,10 @@ impl<T: WidgetCtxImpl> WidgetCtx for T {
self.map_from_global(global)
}

#[inline]
fn query_all_iter<Q: 'static>(&self) -> impl DoubleEndedIterator<Item = QueryRef<Q>> {
self.id().query_all_iter(self.tree())
}

fn query<Q: 'static>(&self) -> Option<QueryRef<Q>> { self.query_of_widget::<Q>(self.id()) }

#[inline]
Expand Down
4 changes: 2 additions & 2 deletions core/src/events/dispatcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@ impl Dispatcher {

let nearest_focus = self.pointer_down_uid.and_then(|wid| {
wid.ancestors(tree).find(|id| {
id.query_ref::<MixBuiltin>(tree)
.map_or(false, |m| m.contain_flag(MixFlags::Focus))
id.query_all_iter::<MixBuiltin>(tree)
.any(|m| m.contain_flag(MixFlags::Focus))
})
});
if let Some(focus_id) = nearest_focus {
Expand Down
4 changes: 2 additions & 2 deletions core/src/events/focus_mgr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,8 @@ impl FocusManager {
.filter(|wid| !wid.is_dropped(tree))
.and_then(|wid| {
wid
.query_ref::<MixBuiltin>(tree)
.map(|m| m.mix_flags().read().tab_index())
.query_all_iter::<MixBuiltin>(tree)
.find_map(|m| m.mix_flags().read().tab_index())
})
.unwrap_or_default()
}
Expand Down
9 changes: 4 additions & 5 deletions core/src/overlay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ impl Overlay {
/// overlay.show_map(move |w| {
/// let wid = wid.clone();
/// fn_widget! {
/// let mut w = @$w {};
/// let mut w = FatObj::new(w);
/// w.left_align_to(&wid, 0., ctx!());
/// w
/// }.into_widget()
Expand Down Expand Up @@ -157,10 +157,9 @@ impl Overlay {
}
self.show_map(
move |w| {
fn_widget! {
@$w { anchor: Anchor::from_point(pos) }
}
.into_widget()
FatObj::new(w)
.anchor(Anchor::from_point(pos))
.into_widget()
},
wnd,
);
Expand Down
6 changes: 3 additions & 3 deletions core/src/pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1548,12 +1548,12 @@ mod tests {
wnd.draw_frame();
wnd.assert_root_size(Size::new(0., 0.));

println!("Inner pipe update:");
// Inner pipe update
*w_inner.write() += 1;
wnd.draw_frame();
wnd.assert_root_size(Size::new(0., 1.));

println!("Outter pipe update:");
// Outer pipe update
*w_outer.write() += 1;
wnd.draw_frame();
wnd.assert_root_size(Size::new(1., 1.));
Expand Down Expand Up @@ -1731,7 +1731,7 @@ mod tests {
pipe!($m_watcher;).map(move |_| {
// margin is static, but its child MockBox is a pipe.
let p = pipe!($son_watcher;).map(|_| MockBox { size: Size::zero() });
@$p { margin: EdgeInsets::all(1.) }
FatObj::new(p).margin(EdgeInsets::all(1.))
})
};
@ $p {
Expand Down
Loading

0 comments on commit eac6382

Please sign in to comment.