[librsvg: 4/6] Switch to the rctree crate



commit 89cd4161989d8a8cf9e8088122f642203ea0918a
Author: Paolo Borelli <pborelli gnome org>
Date:   Sat Jun 29 18:08:16 2019 +0200

    Switch to the rctree crate
    
    Switch from our tree implementation to the rctree dependency.
    This requires adapting some of the code since rctree returns Node
    in its API, not a ref.
    
    In particular, rctree::Node has a RefCell<T> itself, and its
    Node.borrow() gives back a Ref<T>, so we let it handle all the
    mutability tracking insted of having a cell for each field of
    NodeData.
    
    We also need to tell the compiler that the NodeData we borrow
    from a node lives long enough, so in some code paths we have
    to extract a node_data variable. This is evident in particular
    in component_transfer.rs because we had to expand the
    func_or_default utility.

 Cargo.lock                                       |   7 +
 Makefile.am                                      |   1 -
 rsvg_internals/Cargo.toml                        |   1 +
 rsvg_internals/src/create_node.rs                |  36 +--
 rsvg_internals/src/drawing_ctx.rs                |   3 +-
 rsvg_internals/src/filters/component_transfer.rs |  87 +++---
 rsvg_internals/src/filters/context.rs            |   6 +-
 rsvg_internals/src/lib.rs                        |   1 -
 rsvg_internals/src/node.rs                       | 136 ++++-----
 rsvg_internals/src/pattern.rs                    |   5 +-
 rsvg_internals/src/structure.rs                  |   6 +-
 rsvg_internals/src/svg.rs                        |  10 +-
 rsvg_internals/src/tree_utils/mod.rs             | 351 -----------------------
 rsvg_internals/src/xml.rs                        |  76 ++---
 14 files changed, 180 insertions(+), 546 deletions(-)
---
diff --git a/Cargo.lock b/Cargo.lock
index f7d00f6e..0a310196 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -885,6 +885,11 @@ dependencies = [
  "num_cpus 1.10.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
+[[package]]
+name = "rctree"
+version = "0.3.3"
+source = "registry+https://github.com/rust-lang/crates.io-index";
+
 [[package]]
 name = "rdrand"
 version = "0.4.0"
@@ -979,6 +984,7 @@ dependencies = [
  "pangocairo 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "phf 0.7.24 (registry+https://github.com/rust-lang/crates.io-index)",
  "rayon 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
+ "rctree 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "regex 1.1.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "url 1.7.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "xml-rs 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1333,6 +1339,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index";
 "checksum rawpointer 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = 
"ebac11a9d2e11f2af219b8b8d833b76b1ea0e054aa0e8d8e9e4cbde353bdf019"
 "checksum rayon 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)" = 
"373814f27745b2686b350dd261bfd24576a6fb0e2c5919b3a2b6005f820b0473"
 "checksum rayon-core 1.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = 
"b055d1e92aba6877574d8fe604a63c8b5df60f60e5982bf7ccbb1338ea527356"
+"checksum rctree 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = 
"be9e29cb19c8fe84169fcb07f8f11e66bc9e6e0280efd4715c54818296f8a4a8"
 "checksum rdrand 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = 
"678054eb77286b51581ba43620cc911abf02758c91f93f479767aed0f90458b2"
 "checksum redox_syscall 0.1.54 (registry+https://github.com/rust-lang/crates.io-index)" = 
"12229c14a0f65c4f1cb046a3b52047cdd9da1f4b30f8a39c5063c8bae515e252"
 "checksum redox_termios 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = 
"7e891cfe48e9100a70a3b6eb652fef28920c117d366339687bd5576160db0f76"
diff --git a/Makefile.am b/Makefile.am
index 3d69ea6a..a2b88382 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -104,7 +104,6 @@ RUST_SRC =                                                  \
        rsvg_internals/src/svg.rs                               \
        rsvg_internals/src/text.rs                              \
        rsvg_internals/src/transform.rs                         \
-       rsvg_internals/src/tree_utils/mod.rs                    \
        rsvg_internals/src/unit_interval.rs                     \
        rsvg_internals/src/util.rs                              \
        rsvg_internals/src/viewbox.rs                           \
diff --git a/rsvg_internals/Cargo.toml b/rsvg_internals/Cargo.toml
index 2d11c2e8..6c6f045f 100644
--- a/rsvg_internals/Cargo.toml
+++ b/rsvg_internals/Cargo.toml
@@ -46,6 +46,7 @@ pango-sys = "0.8.0"
 pangocairo = "0.7.0"
 phf = "0.7.21"
 rayon = "1"
+rctree = "0.3.3"
 regex = "1"
 url = "1.7.2"
 xml-rs = "0.8.0"
diff --git a/rsvg_internals/src/create_node.rs b/rsvg_internals/src/create_node.rs
index 9546f463..0c21995e 100644
--- a/rsvg_internals/src/create_node.rs
+++ b/rsvg_internals/src/create_node.rs
@@ -37,22 +37,14 @@ use crate::text::{NodeTRef, NodeTSpan, NodeText};
 
 macro_rules! n {
     ($name:ident, $node_type:ident, $new_fn:expr) => {
-        pub fn $name(
-            element_name: LocalName,
-            id: Option<&str>,
-            class: Option<&str>,
-            parent: Option<&RsvgNode>,
-        ) -> RsvgNode {
-            RsvgNode::new(
-                NodeData::new(
-                    NodeType::$node_type,
-                    element_name,
-                    id,
-                    class,
-                    Box::new($new_fn()),
-                ),
-                parent,
-            )
+        pub fn $name(element_name: LocalName, id: Option<&str>, class: Option<&str>) -> RsvgNode {
+            RsvgNode::new(NodeData::new(
+                NodeType::$node_type,
+                element_name,
+                id,
+                class,
+                Box::new($new_fn()),
+            ))
         }
     };
 }
@@ -125,17 +117,12 @@ mod creators {
 
 use creators::*;
 
-type NodeCreateFn = fn(
-    element_name: LocalName,
-    id: Option<&str>,
-    class: Option<&str>,
-    parent: Option<&RsvgNode>,
-) -> RsvgNode;
+type NodeCreateFn = fn(element_name: LocalName, id: Option<&str>, class: Option<&str>) -> RsvgNode;
 
 macro_rules! c {
     ($hashset:expr, $str_name:expr, $supports_class:expr, $fn_name:ident) => {
         $hashset.insert($str_name, ($supports_class, $fn_name as NodeCreateFn));
-    }
+    };
 }
 
 lazy_static! {
@@ -234,7 +221,6 @@ lazy_static! {
 
 pub fn create_node_and_register_id(
     name: &str,
-    parent: Option<&RsvgNode>,
     pbag: &PropertyBag,
     ids: &mut HashMap<String, RsvgNode>,
 ) -> RsvgNode {
@@ -264,7 +250,7 @@ pub fn create_node_and_register_id(
         class = None;
     };
 
-    let node = create_fn(element_name, id, class, parent);
+    let node = create_fn(element_name, id, class);
 
     if let Some(id) = id {
         // This is so we don't overwrite an existing id
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index 0bb1b2ca..5a2afa43 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -375,7 +375,8 @@ impl DrawingCtx {
         if let Some(node) = clip_node {
             let orig_bbox = self.bbox;
 
-            let clip_path = node.borrow().get_impl::<NodeClipPath>();
+            let node_data = node.borrow();
+            let clip_path = node_data.get_impl::<NodeClipPath>();
             let res = clip_path.to_cairo_context(&node, self, &orig_bbox);
 
             // FIXME: this is an EPIC HACK to keep the clipping context from
diff --git a/rsvg_internals/src/filters/component_transfer.rs 
b/rsvg_internals/src/filters/component_transfer.rs
index 342b2102..c909dfab 100644
--- a/rsvg_internals/src/filters/component_transfer.rs
+++ b/rsvg_internals/src/filters/component_transfer.rs
@@ -282,30 +282,26 @@ impl Filter for ComponentTransfer {
             ctx.source_graphic().height(),
         )?;
 
-        // Enumerate all child <feFuncX> nodes.
-        let functions = node
-            .children()
-            .rev()
-            .filter(|c| match c.borrow().get_type() {
-                NodeType::ComponentTransferFunctionA
-                | NodeType::ComponentTransferFunctionB
-                | NodeType::ComponentTransferFunctionG
-                | NodeType::ComponentTransferFunctionR => true,
-                _ => false,
-            });
-
         // Get a node for every pixel component.
         let get_node = |channel| {
-            functions
-                .clone()
+            node.children()
+                .rev()
+                .filter(|c| match c.borrow().get_type() {
+                    NodeType::ComponentTransferFunctionA
+                    | NodeType::ComponentTransferFunctionB
+                    | NodeType::ComponentTransferFunctionG
+                    | NodeType::ComponentTransferFunctionR => true,
+                    _ => false,
+                })
                 .find(|c| c.borrow().get_impl::<FuncX>().channel == channel)
         };
-        let func_r = get_node(Channel::R);
-        let func_g = get_node(Channel::G);
-        let func_b = get_node(Channel::B);
-        let func_a = get_node(Channel::A);
 
-        for node in [&func_r, &func_g, &func_b, &func_a]
+        let func_r_node = get_node(Channel::R);
+        let func_g_node = get_node(Channel::G);
+        let func_b_node = get_node(Channel::B);
+        let func_a_node = get_node(Channel::A);
+
+        for node in [&func_r_node, &func_g_node, &func_b_node, &func_a_node]
             .iter()
             .filter_map(|x| x.as_ref())
         {
@@ -314,18 +310,46 @@ impl Filter for ComponentTransfer {
             }
         }
 
+        // We need to tell the borrow checker that these live long enough
+        let func_r_data;
+        let func_g_data;
+        let func_b_data;
+        let func_a_data;
+
         // This is the default node that performs an identity transformation.
         let func_default = FuncX::default();
 
-        // Retrieve the compute function and parameters for each pixel component.
-        // Can't make this a closure without hacks since it's not currently possible to
-        // cleanly describe |&'a T| -> &'a U to the type system.
-        #[inline]
-        fn func_or_default<'a>(func: &'a Option<RsvgNode>, default: &'a FuncX) -> &'a FuncX {
-            func.as_ref()
-                .map(|c| c.borrow().get_impl::<FuncX>())
-                .unwrap_or(default)
-        }
+        let func_r = match func_r_node {
+            Some(ref f) => {
+                func_r_data = f.borrow();
+                func_r_data.get_impl::<FuncX>()
+            }
+            _ => &func_default,
+        };
+
+        let func_g = match func_g_node {
+            Some(ref f) => {
+                func_g_data = f.borrow();
+                func_g_data.get_impl::<FuncX>()
+            }
+            _ => &func_default,
+        };
+
+        let func_b = match func_b_node {
+            Some(ref f) => {
+                func_b_data = f.borrow();
+                func_b_data.get_impl::<FuncX>()
+            }
+            _ => &func_default,
+        };
+
+        let func_a = match func_a_node {
+            Some(ref f) => {
+                func_a_data = f.borrow();
+                func_a_data.get_impl::<FuncX>()
+            }
+            _ => &func_default,
+        };
 
         #[inline]
         fn compute_func<'a>(func: &'a FuncX) -> impl Fn(u8, f64, f64) -> u8 + 'a {
@@ -344,12 +368,11 @@ impl Filter for ComponentTransfer {
             }
         }
 
-        let compute_r = compute_func(func_or_default(&func_r, &func_default));
-        let compute_g = compute_func(func_or_default(&func_g, &func_default));
-        let compute_b = compute_func(func_or_default(&func_b, &func_default));
+        let compute_r = compute_func(&func_r);
+        let compute_g = compute_func(&func_g);
+        let compute_b = compute_func(&func_b);
 
         // Alpha gets special handling since everything else depends on it.
-        let func_a = func_or_default(&func_a, &func_default);
         let compute_a = func_a.function();
         let params_a = func_a.function_parameters();
         let compute_a = |alpha| compute_a(&params_a, alpha);
diff --git a/rsvg_internals/src/filters/context.rs b/rsvg_internals/src/filters/context.rs
index edd2f736..477611be 100644
--- a/rsvg_internals/src/filters/context.rs
+++ b/rsvg_internals/src/filters/context.rs
@@ -114,7 +114,8 @@ impl FilterContext {
             height: 0.0,
         });
 
-        let filter = filter_node.borrow().get_impl::<NodeFilter>();
+        let node_data = filter_node.borrow();
+        let filter = node_data.get_impl::<NodeFilter>();
 
         let affine = match filter.filterunits.get() {
             CoordUnits::UserSpaceOnUse => cr_affine,
@@ -305,7 +306,8 @@ impl FilterContext {
 
     /// Pushes the viewport size based on the value of `primitiveUnits`.
     pub fn get_view_params(&self, draw_ctx: &mut DrawingCtx) -> ViewParams {
-        let filter = self.node.borrow().get_impl::<NodeFilter>();
+        let node_data = self.node.borrow();
+        let filter = node_data.get_impl::<NodeFilter>();
 
         // See comments in compute_effects_region() for how this works.
         if filter.primitiveunits.get() == CoordUnits::ObjectBoundingBox {
diff --git a/rsvg_internals/src/lib.rs b/rsvg_internals/src/lib.rs
index a504c15a..4452e378 100644
--- a/rsvg_internals/src/lib.rs
+++ b/rsvg_internals/src/lib.rs
@@ -122,7 +122,6 @@ pub mod surface_utils;
 mod svg;
 mod text;
 mod transform;
-pub mod tree_utils;
 mod unit_interval;
 mod util;
 mod viewbox;
diff --git a/rsvg_internals/src/node.rs b/rsvg_internals/src/node.rs
index b4fa92c7..4d9b06b3 100644
--- a/rsvg_internals/src/node.rs
+++ b/rsvg_internals/src/node.rs
@@ -1,7 +1,7 @@
 use cairo::{Matrix, MatrixTrait};
 use downcast_rs::*;
 use markup5ever::{local_name, LocalName};
-use std::cell::{Cell, Ref, RefCell};
+use std::cell::Ref;
 use std::collections::HashSet;
 use std::fmt;
 
@@ -14,12 +14,12 @@ use crate::parsers::Parse;
 use crate::properties::{ComputedValues, SpecifiedValue, SpecifiedValues};
 use crate::property_bag::PropertyBag;
 use crate::property_defs::Overflow;
-use crate::tree_utils::{NodeRef, NodeWeakRef};
 use locale_config::Locale;
+use rctree;
 
 /// Tree node with specific data
-pub type RsvgNode = NodeRef<NodeData>;
-pub type RsvgWeakNode = NodeWeakRef<NodeData>;
+pub type RsvgNode = rctree::Node<NodeData>;
+pub type RsvgWeakNode = rctree::WeakNode<NodeData>;
 
 /// Contents of a tree node
 pub struct NodeData {
@@ -27,14 +27,14 @@ pub struct NodeData {
     element_name: LocalName,
     id: Option<String>,    // id attribute from XML element
     class: Option<String>, // class attribute from XML element
-    specified_values: RefCell<SpecifiedValues>,
-    important_styles: RefCell<HashSet<LocalName>>,
-    result: RefCell<NodeResult>,
-    transform: Cell<Matrix>,
-    values: RefCell<ComputedValues>,
-    cond: Cell<bool>,
+    specified_values: SpecifiedValues,
+    important_styles: HashSet<LocalName>,
+    result: NodeResult,
+    transform: Matrix,
+    values: ComputedValues,
+    cond: bool,
+    style_attr: String,
     node_impl: Box<NodeTrait>,
-    style_attr: RefCell<String>,
 }
 
 impl NodeData {
@@ -50,14 +50,14 @@ impl NodeData {
             element_name,
             id: id.map(str::to_string),
             class: class.map(str::to_string),
-            specified_values: RefCell::new(Default::default()),
+            specified_values: Default::default(),
             important_styles: Default::default(),
-            transform: Cell::new(Matrix::identity()),
-            result: RefCell::new(Ok(())),
-            values: RefCell::new(ComputedValues::default()),
-            cond: Cell::new(true),
+            transform: Matrix::identity(),
+            result: Ok(()),
+            values: ComputedValues::default(),
+            cond: true,
+            style_attr: String::new(),
             node_impl,
-            style_attr: RefCell::new(String::new()),
         }
     }
 
@@ -90,21 +90,20 @@ impl NodeData {
     }
 
     pub fn get_cond(&self) -> bool {
-        self.cond.get()
+        self.cond
     }
 
     pub fn get_transform(&self) -> Matrix {
-        self.transform.get()
+        self.transform
     }
 
     pub fn is_overflow(&self) -> bool {
-        self.specified_values.borrow().is_overflow()
+        self.specified_values.is_overflow()
     }
 
-    pub fn set_atts(&self, parent: Option<&RsvgNode>, pbag: &PropertyBag<'_>, locale: &Locale) {
+    pub fn set_atts(&mut self, parent: Option<&RsvgNode>, pbag: &PropertyBag<'_>, locale: &Locale) {
         if self.node_impl.overflow_hidden() {
-            let mut specified_values = self.specified_values.borrow_mut();
-            specified_values.overflow = SpecifiedValue::Specified(Overflow::Hidden);
+            self.specified_values.overflow = SpecifiedValue::Specified(Overflow::Hidden);
         }
 
         self.save_style_attribute(pbag);
@@ -118,30 +117,26 @@ impl NodeData {
             self.set_error(e);
         }
 
-        let mut specified_values = self.specified_values.borrow_mut();
         self.node_impl
-            .set_overridden_properties(&mut specified_values);
+            .set_overridden_properties(&mut self.specified_values);
     }
 
-    fn save_style_attribute(&self, pbag: &PropertyBag<'_>) {
-        let mut style_attr = self.style_attr.borrow_mut();
-
+    fn save_style_attribute(&mut self, pbag: &PropertyBag<'_>) {
         for (attr, value) in pbag.iter() {
             match attr {
-                local_name!("style") => style_attr.push_str(value),
-
+                local_name!("style") => self.style_attr.push_str(value),
                 _ => (),
             }
         }
     }
 
-    fn set_transform_attribute(&self, pbag: &PropertyBag<'_>) -> Result<(), NodeError> {
+    fn set_transform_attribute(&mut self, pbag: &PropertyBag<'_>) -> Result<(), NodeError> {
         for (attr, value) in pbag.iter() {
             match attr {
                 local_name!("transform") => {
                     return Matrix::parse_str(value)
                         .attribute(attr)
-                        .and_then(|affine| Ok(self.transform.set(affine)));
+                        .and_then(|affine| Ok(self.transform = affine));
                 }
 
                 _ => (),
@@ -152,11 +147,11 @@ impl NodeData {
     }
 
     fn set_conditional_processing_attributes(
-        &self,
+        &mut self,
         pbag: &PropertyBag<'_>,
         locale: &Locale,
     ) -> Result<(), NodeError> {
-        let mut cond = self.cond.get();
+        let mut cond = self.cond;
 
         for (attr, value) in pbag.iter() {
             // FIXME: move this to "try {}" when we can bump the rustc version dependency
@@ -184,7 +179,7 @@ impl NodeData {
             };
 
             parse()
-                .map(|c| self.cond.set(c))
+                .map(|c| self.cond = c)
                 .map_err(|e| NodeError::attribute_error(attr, e))?;
         }
 
@@ -192,12 +187,8 @@ impl NodeData {
     }
 
     /// Hands the pbag to the node's state, to apply the presentation attributes
-    fn set_presentation_attributes(&self, pbag: &PropertyBag<'_>) -> Result<(), NodeError> {
-        match self
-            .specified_values
-            .borrow_mut()
-            .parse_presentation_attributes(pbag)
-        {
+    fn set_presentation_attributes(&mut self, pbag: &PropertyBag<'_>) -> Result<(), NodeError> {
+        match self.specified_values.parse_presentation_attributes(pbag) {
             Ok(_) => Ok(()),
             Err(e) => {
                 // FIXME: we'll ignore errors here for now.
@@ -218,53 +209,45 @@ impl NodeData {
     }
 
     /// Applies the CSS rules that match into the node's specified_values
-    fn set_css_styles(&self, css_rules: &CssRules) {
-        let mut specified_values = self.specified_values.borrow_mut();
-        let mut important_styles = self.important_styles.borrow_mut();
-
+    fn set_css_styles(&mut self, css_rules: &CssRules) {
         for selector in &css_rules.get_matches(self) {
             if let Some(decl_list) = css_rules.get_declarations(selector) {
                 for declaration in decl_list.iter() {
-                    specified_values
-                        .set_property_from_declaration(declaration, &mut important_styles);
+                    self.specified_values
+                        .set_property_from_declaration(declaration, &mut self.important_styles);
                 }
             }
         }
     }
 
     /// Applies CSS styles from the saved value of the "style" attribute
-    fn set_style_attribute(&self) {
-        let mut style_attr = self.style_attr.borrow_mut();
-
-        if !style_attr.is_empty() {
-            let mut important_styles = self.important_styles.borrow_mut();
-
+    fn set_style_attribute(&mut self) {
+        if !self.style_attr.is_empty() {
             if let Err(e) = self
                 .specified_values
-                .borrow_mut()
-                .parse_style_declarations(style_attr.as_str(), &mut important_styles)
+                .parse_style_declarations(self.style_attr.as_str(), &mut self.important_styles)
             {
                 self.set_error(e);
             }
 
-            style_attr.clear();
-            style_attr.shrink_to_fit();
+            self.style_attr.clear();
+            self.style_attr.shrink_to_fit();
         }
     }
 
     // Sets the node's specified values from the style-related attributes in the pbag.
     // Also applies CSS rules in our limited way based on the node's tag/class/id.
-    pub fn set_style(&self, css_rules: &CssRules) {
+    pub fn set_style(&mut self, css_rules: &CssRules) {
         self.set_css_styles(css_rules);
         self.set_style_attribute();
     }
 
-    fn set_error(&self, error: NodeError) {
-        *self.result.borrow_mut() = Err(error);
+    fn set_error(&mut self, error: NodeError) {
+        self.result = Err(error);
     }
 
     pub fn is_in_error(&self) -> bool {
-        self.result.borrow().is_err()
+        self.result.is_err()
     }
 }
 
@@ -294,7 +277,7 @@ pub struct CascadedValues<'a> {
 }
 
 enum CascadedInner<'a> {
-    FromNode(Ref<'a, ComputedValues>),
+    FromNode(Ref<'a, NodeData>),
     FromValues(ComputedValues),
 }
 
@@ -307,7 +290,7 @@ impl<'a> CascadedValues<'a> {
     pub fn new(&self, node: &'a RsvgNode) -> CascadedValues<'a> {
         match self.inner {
             CascadedInner::FromNode(_) => CascadedValues {
-                inner: CascadedInner::FromNode(node.borrow().values.borrow()),
+                inner: CascadedInner::FromNode(node.borrow()),
             },
 
             CascadedInner::FromValues(ref v) => CascadedValues::new_from_values(node, v),
@@ -321,7 +304,7 @@ impl<'a> CascadedValues<'a> {
     /// `new()` to derive the cascade from an existing one.
     pub fn new_from_node(node: &RsvgNode) -> CascadedValues<'_> {
         CascadedValues {
-            inner: CascadedInner::FromNode(node.borrow().values.borrow()),
+            inner: CascadedInner::FromNode(node.borrow()),
         }
     }
 
@@ -332,10 +315,7 @@ impl<'a> CascadedValues<'a> {
     /// `<use>`'s own cascade, not wih the element's original cascade.
     pub fn new_from_values(node: &'a RsvgNode, values: &ComputedValues) -> CascadedValues<'a> {
         let mut v = values.clone();
-        node.borrow()
-            .specified_values
-            .borrow()
-            .to_computed_values(&mut v);
+        node.borrow().specified_values.to_computed_values(&mut v);
 
         CascadedValues {
             inner: CascadedInner::FromValues(v),
@@ -348,7 +328,7 @@ impl<'a> CascadedValues<'a> {
     /// `ComputedValues` from the `CascadedValues` that got passed to `draw()`.
     pub fn get(&'a self) -> &'a ComputedValues {
         match self.inner {
-            CascadedInner::FromNode(ref r) => &*r,
+            CascadedInner::FromNode(ref node) => &node.values,
             CascadedInner::FromValues(ref v) => v,
         }
     }
@@ -473,19 +453,21 @@ pub enum NodeType {
 
 /// Helper trait for cascading recursively
 pub trait NodeCascade {
-    fn cascade(&self, values: &ComputedValues);
+    fn cascade(&mut self, values: &ComputedValues);
 }
 
 impl NodeCascade for RsvgNode {
-    fn cascade(&self, values: &ComputedValues) {
+    fn cascade(&mut self, values: &ComputedValues) {
         let mut values = values.clone();
-        self.borrow()
-            .specified_values
-            .borrow()
-            .to_computed_values(&mut values);
-        *self.borrow().values.borrow_mut() = values.clone();
 
-        for child in self.children() {
+        {
+            let mut node_mut = self.borrow_mut();
+
+            node_mut.specified_values.to_computed_values(&mut values);
+            node_mut.values = values.clone();
+        }
+
+        for mut child in self.children() {
             child.cascade(&values);
         }
     }
diff --git a/rsvg_internals/src/pattern.rs b/rsvg_internals/src/pattern.rs
index 076cd0bd..5859da91 100644
--- a/rsvg_internals/src/pattern.rs
+++ b/rsvg_internals/src/pattern.rs
@@ -210,7 +210,8 @@ impl PaintSource for NodePattern {
                     return Err(RenderingError::CircularReference);
                 }
 
-                let fallback = a_node.borrow().get_impl::<NodePattern>().pattern.borrow();
+                let node_data = a_node.borrow();
+                let fallback = node_data.get_impl::<NodePattern>().pattern.borrow();
                 result.resolve_from_fallback(&fallback);
 
                 stack.push(a_node);
@@ -381,7 +382,7 @@ impl PaintSource for NodePattern {
         // Set up transformations to be determined by the contents units
 
         // Draw everything
-        let pattern_node = RsvgNode::upgrade(pattern.node.as_ref().unwrap()).unwrap();
+        let pattern_node = pattern.node.as_ref().unwrap().upgrade().unwrap();
         let pattern_cascaded = CascadedValues::new_from_node(&pattern_node);
         let pattern_values = pattern_cascaded.get();
 
diff --git a/rsvg_internals/src/structure.rs b/rsvg_internals/src/structure.rs
index 6f4eef03..e35a325f 100644
--- a/rsvg_internals/src/structure.rs
+++ b/rsvg_internals/src/structure.rs
@@ -17,7 +17,6 @@ use crate::properties::ComputedValues;
 use crate::property_bag::PropertyBag;
 use crate::property_defs::Overflow;
 use crate::rect::RectangleExt;
-use crate::tree_utils::Node;
 use crate::viewbox::*;
 
 #[derive(Default)]
@@ -347,7 +346,7 @@ impl NodeTrait for NodeUse {
             return Ok(());
         };
 
-        if Node::is_ancestor(child.clone(), node.clone()) {
+        if node.ancestors().any(|ancestor| ancestor == child) {
             // or, if we're <use>'ing ourselves
             return Err(RenderingError::CircularReference);
         }
@@ -395,7 +394,8 @@ impl NodeTrait for NodeUse {
                 )
             })
         } else {
-            let symbol = child.borrow().get_impl::<NodeSymbol>();
+            let node_data = child.borrow();
+            let symbol = node_data.get_impl::<NodeSymbol>();
 
             let clip_mode = if !values.is_overflow()
                 || (values.overflow == Overflow::Visible && child.borrow().is_overflow())
diff --git a/rsvg_internals/src/svg.rs b/rsvg_internals/src/svg.rs
index be3e0ec5..7aabdd32 100644
--- a/rsvg_internals/src/svg.rs
+++ b/rsvg_internals/src/svg.rs
@@ -36,7 +36,7 @@ pub struct Svg {
 }
 
 impl Svg {
-    pub fn new(tree: RsvgNode, ids: HashMap<String, RsvgNode>, load_options: LoadOptions) -> Svg {
+    pub fn new(mut tree: RsvgNode, ids: HashMap<String, RsvgNode>, load_options: LoadOptions) -> Svg {
         let values = ComputedValues::default();
         tree.cascade(&values);
 
@@ -86,12 +86,10 @@ impl Svg {
 
     pub fn get_intrinsic_dimensions(&self) -> IntrinsicDimensions {
         let root = self.root();
+        let node_data = root.borrow();
 
-        assert!(root.borrow().get_type() == NodeType::Svg);
-
-        root.borrow()
-            .get_impl::<NodeSvg>()
-            .get_intrinsic_dimensions()
+        assert!(node_data.get_type() == NodeType::Svg);
+        node_data.get_impl::<NodeSvg>().get_intrinsic_dimensions()
     }
 }
 
diff --git a/rsvg_internals/src/xml.rs b/rsvg_internals/src/xml.rs
index 53618e9c..e2f24c6b 100644
--- a/rsvg_internals/src/xml.rs
+++ b/rsvg_internals/src/xml.rs
@@ -114,9 +114,9 @@ impl XmlState {
         match self.tree_root {
             None => Err(LoadingError::SvgHasNoElements),
             Some(ref root) if root.borrow().get_type() == NodeType::Svg => {
-                let root = self.tree_root.take().unwrap();
+                let mut root = self.tree_root.take().unwrap();
 
-                set_styles_recursively(&root, self.css_rules.as_ref().unwrap());
+                set_styles_recursively(&mut root, self.css_rules.as_ref().unwrap());
 
                 Ok(Svg::new(
                     root,
@@ -257,12 +257,19 @@ impl XmlState {
         match name {
             "include" => self.xinclude_start_element(name, pbag),
             _ => {
+                let ids = self.ids.as_mut().unwrap();
+                let mut node = create_node_and_register_id(name, pbag, ids);
+
                 let parent = self.current_node.clone();
-                let node = self.create_node(parent.as_ref(), name, pbag);
+                node.borrow_mut()
+                    .set_atts(parent.as_ref(), pbag, self.load_options.locale());
 
-                if self.current_node.is_none() {
+                if let Some(mut parent) = parent {
+                    parent.append(node.clone());
+                } else {
                     self.set_root(&node);
                 }
+
                 self.current_node = Some(node);
 
                 Context::ElementCreation
@@ -274,9 +281,8 @@ impl XmlState {
         let node = self.current_node.take().unwrap();
 
         if node.borrow().get_type() == NodeType::Style {
-            let css_data = node.borrow().get_impl::<NodeStyle>().get_css(&node);
-
             let css_rules = self.css_rules.as_mut().unwrap();
+            let css_data = node.borrow().get_impl::<NodeStyle>().get_css(&node);
 
             css_rules.parse(self.load_options.base_url.as_ref(), &css_data);
         }
@@ -285,28 +291,29 @@ impl XmlState {
     }
 
     fn element_creation_characters(&self, text: &str) {
-        let node = self.current_node.as_ref().unwrap();
-
         if text.len() != 0 {
             // When the last child is a Chars node we can coalesce
             // the text and avoid screwing up the Pango layouts
-            let chars_node = if let Some(child) = node
+            let chars_node = if let Some(child) = self
+                .current_node
+                .as_ref()
+                .unwrap()
                 .last_child()
                 .filter(|c| c.borrow().get_type() == NodeType::Chars)
             {
                 child
             } else {
-                let child = RsvgNode::new(
-                    NodeData::new(
-                        NodeType::Chars,
-                        LocalName::from("rsvg-chars"),
-                        None,
-                        None,
-                        Box::new(NodeChars::new()),
-                    ),
-                    Some(node),
-                );
-                node.append(&child);
+                let child = RsvgNode::new(NodeData::new(
+                    NodeType::Chars,
+                    LocalName::from("rsvg-chars"),
+                    None,
+                    None,
+                    Box::new(NodeChars::new()),
+                ));
+
+                let mut node = self.current_node.as_ref().unwrap().clone();
+                node.append(child.clone());
+
                 child
             };
 
@@ -314,27 +321,6 @@ impl XmlState {
         }
     }
 
-    fn create_node(
-        &mut self,
-        parent: Option<&RsvgNode>,
-        name: &str,
-        pbag: &PropertyBag,
-    ) -> RsvgNode {
-        let ids = self.ids.as_mut().unwrap();
-
-        let new_node = create_node_and_register_id(name, parent, pbag, ids);
-
-        if let Some(parent) = parent {
-            parent.append(&new_node);
-        }
-
-        new_node
-            .borrow()
-            .set_atts(parent, pbag, self.load_options.locale());
-
-        new_node
-    }
-
     fn xinclude_start_element(&mut self, _name: &str, pbag: &PropertyBag) -> Context {
         let mut href = None;
         let mut parse = None;
@@ -534,11 +520,11 @@ fn skip_namespace(s: &str) -> &str {
     s.find(':').map_or(s, |pos| &s[pos + 1..])
 }
 
-fn set_styles_recursively(node: &RsvgNode, css_rules: &CssRules) {
-    node.borrow().set_style(css_rules);
+fn set_styles_recursively(node: &mut RsvgNode, css_rules: &CssRules) {
+    node.borrow_mut().set_style(css_rules);
 
-    for child in node.children() {
-        set_styles_recursively(&child, css_rules);
+    for mut child in node.children() {
+        set_styles_recursively(&mut child, css_rules);
     }
 }
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]