Skip to content

Commit

Permalink
Expose view and taffy node from ViewId (#454)
Browse files Browse the repository at this point in the history
* Expose view and taffy node from ViewId

Rationale: Solving two problems here:

1. It is impossible to implement the same code as the default implementation of `View::layout()` outside of the `floem` crate,
which limits what components in other crates can do to lay themselves out.

2. Most of the components in `floem::views` actually implement layout by manipulating the taffy tree (e.g, in `Label`,
`self.id.taffy().borrow_mut().set_style(text_node, style);`.  It is possible to implement at least the things I am trying
to do right now simply by having access to the existing taffy NodeId for a View and using `ViewId::set_taffy_style`, so rather
that the more extreme approach of exposing the whole taffy tree, I opted simply to add a call to get the node id - otherwise,
the only access to it is from within a `layout` method which (prior to this patch at least) was inaccessible unless you could
afford to give up the default layout behavior.

I'm not entirely sure that just exposing the `view()` method is ideal here (though *something* is needed) - in my case I was
debugging a panic from the `borrow_mut()` call in `View::layout()` which turned out to be that I was misusing the `ViewId`
that gets passed in to the closure in `add_overlay()` as the id of the overlay component, which was already in use by the
container for the overlay - so I wanted to reimplement the default behavior with logging to see exactly where things were
going wrong.  Attempting to use `try_borrow_mut()` instead to detect the source of the problem runs into lifetime problems
between the erased lifetimes of `RefMut<'_, Box<dyn View>>` and `&mut LayoutCx<'_>`, which had to be resolved with some
compiler trickery like this monstrosity (again, this is just debug code):

```rust
fn layout_if_posible<'l, V: View + ?Sized + 'l>(
    view_cell: &Rc<RefCell<Box<V>>>,
    cx: &mut LayoutCx<'_>,
) -> Option<NodeId> {
    if let Ok(view) = view_cell.try_borrow_mut() {
        return Some(layout_view(view, cx));
    }
    None
}

fn layout_view<'l, V: View + ?Sized + 'l>(
    mut v: RefMut<'l, Box<V>>,
    cx: &mut LayoutCx<'_>,
) -> NodeId {
    v.layout(cx)
}
```

So it might actually be better to expose a method on `ViewId` that takes a closure with a signature like `with_view_id(&mut self,
mut f : impl FnMut(&mut dyn View))` so users of the API are insulated from the erased lifetimes involved, and (perhaps?) use
`try_borrow_mut` there and return a boolean (is there *ever* a **legitimate** case where there could be contention for the view,
or was that only because I was abusing one id for two views?).

At any rate, this patch is likely to be harmless as-is.

* Don't expose `ViewId.view()` after all

Since the problem I'm attempting to solve is to be able to override `layout()` in
a `View` and still be able to use the default functionality, the ability to simply
call that functionality as a standalone `pub fn` is probably enough to solve it
(TBD, but very likely).

* Export view::recursively_layout_view() to external callers
  • Loading branch information
timboudreau authored May 24, 2024
1 parent 408b825 commit 58ef62f
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 10 deletions.
4 changes: 4 additions & 0 deletions src/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ impl ViewId {
self.taffy().borrow().layout(node).cloned().ok()
}

pub fn taffy_node(&self) -> NodeId {
self.state().borrow().node
}

pub(crate) fn state(&self) -> Rc<RefCell<ViewState>> {
VIEW_STORAGE.with_borrow_mut(|s| {
if !s.view_ids.contains_key(*self) {
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,5 +191,5 @@ pub use id::ViewId;
pub use kurbo;
pub use peniko;
pub use taffy;
pub use view::{AnyView, IntoView, View};
pub use view::{recursively_layout_view, AnyView, IntoView, View};
pub use window::{close_window, new_window};
25 changes: 16 additions & 9 deletions src/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,21 @@ impl<IV: IntoView + 'static> IntoView for Vec<IV> {
}
}

/// Default implementation of `View::layout()` which can be used by
/// view implementations that need the default behavior and also need
/// to implement that method to do additional work.
pub fn recursively_layout_view(id: ViewId, cx: &mut LayoutCx) -> NodeId {
cx.layout_node(id, true, |cx| {
let mut nodes = Vec::new();
for child in id.children() {
let view = child.view();
let mut view = view.borrow_mut();
nodes.push(view.layout(cx));
}
nodes
})
}

/// The Widget trait contains the methods for implementing updates, styling, layout, events, and painting.
///
/// The [view_data](Widget::view_data) and [view_data_mut](Widget::view_data_mut) methods must be implemented. If the widget contains a child then the [for_each_child](Widget::for_each_child), [for_each_child_mut](Widget::for_each_child_mut), and [for_each_child_rev_mut](Widget::for_each_child_rev_mut) methods must also be implemented.
Expand Down Expand Up @@ -227,15 +242,7 @@ pub trait View {
/// If the layout changes needs other passes to run you're expected to call
/// `cx.app_state_mut().request_changes`.
fn layout(&mut self, cx: &mut LayoutCx) -> NodeId {
cx.layout_node(self.id(), true, |cx| {
let mut nodes = Vec::new();
for child in self.id().children() {
let view = child.view();
let mut view = view.borrow_mut();
nodes.push(view.layout(cx));
}
nodes
})
recursively_layout_view(self.id(), cx)
}

/// Responsible for computing the layout of the view's children.
Expand Down

0 comments on commit 58ef62f

Please sign in to comment.