razrfalcon / rctree Goto Github PK

The resvg crate used to use ego_tree which had a value() function but rctree does not.

Is there something similar?

#[test]
fn stack_overflow_siblings() {
    let mut prev_sibling = Node::new(1);
    for _ in 0..200_000 {
        let node = Node::new(1);
        prev_sibling.insert_before(node.clone());
        prev_sibling = node;
    }
}

This is very similar to stack_overflow() test, but this ends up in stack overflow.

Current Drop impl of NodeData<T> detaches descendants before they are dropped, but do nothing special to following siblings.
If they have parent, they would be detached by parent's NodeData<T>::drop(), so this does not matter.
However, rctree allows root node to have siblings, and in this case siblings are not detached beforehand and dropped recursively.

I have an rctree tree of the type TypeA, which is a type that can be easily converted to type TypeB (in my case, TypeB is just a value stored in an enum variant of TypeA). I would like to convert the whole tree over (by cloning it). I couldn't see any easy way to do this based on the documentation, unless I missed something. However this seems like a commonly needed action. If it's not a built-in feature, what code should I write to make a function that performs this conversion?

The methods make_copy and make_deep_copy require a mutable reference to self. But looking at the code I don't see an obvious reason for this. But this requirement makes usage of that method more complicated.
Is there a reason behind that requirement I just don't see? Or can mut be dropped? (I changed it on a local copy and at least all tests still compiled and ran green)

See https://github.com/kuchiki-rs/kuchiki/blob/616690fbfce1d77ac0cc4025f5b9fc5d48d71264/src/tree.rs#L134

I noticed this while looking through how to use this library (could not find examples), so i looked at the source and noticed that a macro named try_opt! exists, which basically is like using ? operator from what i know (can also be used after converting between Result and Option)

example (from what i know):
currently there is

which could be replaced with

pub fn parent(&self) -> Option<Node<T>> {
    Some(Node(self.0.borrow().parent.as_ref()?.upgrade()?))
}

How about adding an optional feature which makes the tree derive Serialize and Deserialize?

These four functions will have a lot of redundant memory operations that push values into the stack and pop the values out of the stack when they are called. If we add a #[inline(always)] tag to these functions, these memory operations will be eliminated.

According to my test with the svg example of printpdf library (https://github.com/fschutt/printpdf), the average execution time decreased from 5.8s to 5.19s, which is a 11.8% speedup.

Hope this information helps!

#[test]
fn keep_subtree() {
    fn create_tree() -> Node<&'static str> {
        let mut n1 = Node::new("1");
        let n1_1 = Node::new("1-1");
        n1.append(n1_1);
        let mut n1_2 = Node::new("1-2");
        n1.append(n1_2.clone());
        let n1_2_1 = Node::new("1-2-1");
        n1_2.append(n1_2_1);
        let n1_3 = Node::new("1-3");
        n1.append(n1_3);
        // 1
        // |-- 1-1
        // |-- 1-2
        // |   `-- 1-2-1
        // `-- 1-3

        // And, return subtree 1-2.
        n1_2
    }

    let n1_2 = create_tree();
    // `n1_2` should have a child `n1_2_1`.
    assert!(n1_2.first_child().is_some());
}

This test passes without custom impl Drop, but fails with current implementation.

This is because the current custom Drop detaches all descendants without checking if they are shared.
In contrast, without custom Drop, NodeData will be modified (destroyed) only if Rc's strong refcount is 1.

Is this behavior intended?
I've expected default (implicit) drop behavior and I'm surprised when noticed this difference.

A node that already has children will not have the root updated on its children when it is inserted into a tree.

For example, the below test will fail on the last assertion. Is this intentional (maybe to save time not having to walk descendants), or is this an oversight?

#[test]
fn root_5() {
    let mut node1 = Node::new("node1");
    let mut node2 = Node::new("node2");
    let node3 = Node::new("node3");
    node2.append(node3.clone());
    node1.append(node2.clone());
    assert_eq!(node1.root(), node1);
    assert_eq!(node2.root(), node1);
    assert_eq!(node3.root(), node1);
}