LayoutPartialTree low level api is restrictive

[bogzbonny]

What problem does this solve or what need does it fill?

It would be nice if there was a less restrictive low-level api, within my UIs node structure my nodes are not owned but are subject to interior mutability (my trees own something akin to Rc<RefCell<Vec>>), hence unless I'm missing something (which I very well may be) its impossible for me to return the references (&Style, &mut Cache) as required by LayoutPartialTree as it breaks rust ownership guarantees. For reference here is the current trait:

pub trait LayoutPartialTree: TraversePartialTree {
    fn get_style(&self, node_id: NodeId) -> &Style;
    fn set_unrounded_layout(&mut self, node_id: NodeId, layout: &Layout);
    fn get_cache_mut(&mut self, node_id: NodeId) -> &mut Cache;
    fn compute_child_layout(&mut self, node_id: NodeId, inputs: LayoutInput) -> LayoutOutput;
}

What solution would you like?

I'm not 100% sure on what the ideal solution really would be. In my exact scenario a API which returned an owned Style (this would be a performance cost) and Rc<RefCell<Cache>> would work, however returning the Rc<RefCell<Cache>> really doesn't seem ideal to me either as this doesn't leave room for alternative mutability patterns.

MAYBE creating a Cache Trait would actually be the more versatile solution. Something like:

pub trait LayoutPartialTree: TraversePartialTree + CacheForNode {
    fn get_style(&self, node_id: NodeId) -> Style;
    fn set_unrounded_layout(&mut self, node_id: NodeId, layout: &Layout);
    fn compute_child_layout(&mut self, node_id: NodeId, inputs: LayoutInput) -> LayoutOutput;
}

pub trait CacheForNode {
    fn get(&self, node_id, ...) -> Option<LayoutOutput>;
    fn store(&mut self, node_id, ...)
    fn clear(&mut self, node_id)
    fn is_empty(&self, node_id)
}

Ideally it would be cool to have a solution which doesn't introduce any significant performance cost (&Style -> Style might, I'm not sure); and also isn't just a second low level api for maintainability purposes. Curious if anyone else has other thoughts, maybe folks like I just need to use my workaround (below), however I bet there is a good solution

What alternative(s) have you considered?

Right now, I figured I can just use the high-level api as a workaround and rebuild the tree for each time a recomputation is required. It's just not the nicest solution but it will work.

[coderedart]

Did you look at the current master branch code? It might already fix your issue.

https://github.com/DioxusLabs/taffy/blob/main/src/tree/traits.rs#L169

LayoutPartialTree has an associated type bound by the CoreStyle trait. Its already implemented for Style, so you can return it or make your own custom struct to return that.

[nicoburns]

It's funny that you have this issue now, as I have just been dealing with the exact same issue with the html5ever crate. This definitely seems like a use case we ought to cater for.

I think the new trait-based API for Style on the master branch ought to sort your problem with the style method(s) (it's now multiple methods). You'll need to implement FlexContainerStyle, FlexItemStyle, etc as well as CoreStyle which will be a bit of boilerplate, but it should be quite straightforward. My recommendation would be to implement the traits for a newtype wrapper around std::cell::Ref<'a, Style>. We could also consider including implementations for std::cell::Ref<'a, Style> in taffy.

For the get/set layout methods I think we can make them work by value if need be.

I think you might be right that we might need another trait for the Cache. It's really not ideal that everything needs to be so generic, but I guess it's better to make things possible than not. We could also potentially just add get and store methods to the main trait (I don't think Taffy's layout algorithms ever actually clear the cache atm - there is a method on the Taffy tree to do so but that isn't behind a trait).

We may also be able to use the Deref trait to abstract over types of pointers.

[bogzbonny]

Alright! wow yeah great timing. looking through the new LayoutPartialTree trait - I think everything may be already solved. wrt the Cache because I do use a refcell guard it seems as though the new trait should accommodate my situation. - there may however be a better system entirely, maybe adding a Cache Trait functions to LayoutPartialTree - maybe moving the Cache entirely into Taffy and removing the implementation burden from the user of the low-level-api, for now it seems as though it should be sufficient if not ideal.

wrt to the set layout methods - they seem to be fine as references, references can always be cloned when they're incoming, its just returning references which can be problematic.

Well stuff is looking good I'm going to give an implementation based on the master branch and see how that works out, or if there are additional issues unresolved by the new interface.