[librsvg: 1/3] drawing_ctx: split AcquiredNodes in its own type

[Federico Mena Quintero <federico src gnome org>]

commit e098d94b72be58541a7413c3548b7dcf2e4970d0
Author: Paolo Borelli <pborelli gnome org>
Date:   Thu May 9 11:59:16 2019 +0200

    drawing_ctx: split AcquiredNodes in its own type

 rsvg_internals/src/drawing_ctx.rs   | 157 +++++++++++++++++++++---------------
 rsvg_internals/src/filters/image.rs |   3 +-
 rsvg_internals/src/gradient.rs      |   2 +-
 rsvg_internals/src/marker.rs        |   5 +-
 rsvg_internals/src/pattern.rs       |   5 +-
 rsvg_internals/src/structure.rs     |   2 +-
 rsvg_internals/src/text.rs          |   2 +-
 7 files changed, 104 insertions(+), 72 deletions(-)
---
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index 2d4cd545..1288a7db 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -105,7 +105,7 @@ pub struct DrawingCtx {
 
     drawsub_stack: Vec<RsvgNode>,
 
-    acquired_nodes: Rc<RefCell<Vec<RsvgNode>>>,
+    acquired_nodes: AcquiredNodes,
 
     measuring: bool,
     testing: bool,
@@ -159,6 +159,8 @@ impl DrawingCtx {
         let mut view_box_stack = Vec::new();
         view_box_stack.push(vbox);
 
+        let acquired_nodes = AcquiredNodes::new(svg.clone());
+
         let mut draw_ctx = DrawingCtx {
             svg,
             initial_affine,
@@ -170,7 +172,7 @@ impl DrawingCtx {
             view_box_stack: Rc::new(RefCell::new(view_box_stack)),
             bbox: BoundingBox::new(&initial_affine),
             drawsub_stack: Vec::new(),
-            acquired_nodes: Rc::new(RefCell::new(Vec::new())),
+            acquired_nodes,
             measuring,
             testing,
         };
@@ -330,63 +332,8 @@ impl DrawingCtx {
         &self.bbox
     }
 
-    // Use this function when looking up urls to other nodes. This function
-    // does proper recursion checking and thereby avoids infinite loops.
-    //
-    // Nodes acquired by this function must be released in reverse
-    // acquiring order.
-    //
-    // Note that if you acquire a node, you have to release it before trying to
-    // acquire it again.  If you acquire a node "#foo" and don't release it before
-    // trying to acquire "foo" again, you will obtain a %NULL the second time.
-    pub fn get_acquired_node(&mut self, fragment: &Fragment) -> Option<AcquiredNode> {
-        if let Ok(node) = self.svg.lookup(fragment) {
-            if !self.acquired_nodes_contains(&node) {
-                self.acquired_nodes.borrow_mut().push(node.clone());
-                let acq = AcquiredNode(self.acquired_nodes.clone(), node.clone());
-                return Some(acq);
-            }
-        }
-
-        None
-    }
-
-    fn acquired_nodes_contains(&self, node: &RsvgNode) -> bool {
-        self.acquired_nodes
-            .borrow()
-            .iter()
-            .find(|n| Rc::ptr_eq(n, node))
-            .is_some()
-    }
-
-    // Use this function when looking up urls to other nodes, and when you expect
-    // the node to be of a particular type. This function does proper recursion
-    // checking and thereby avoids infinite loops.
-    //
-    // Malformed SVGs, for example, may reference a marker by its IRI, but
-    // the object referenced by the IRI is not a marker.
-    //
-    // Note that if you acquire a node, you have to release it before trying to
-    // acquire it again.  If you acquire a node "#foo" and don't release it before
-    // trying to acquire "foo" again, you will obtain a None the second time.
-    //
-    // For convenience, this function will return None if url is None.
-
-    // FIXME: return a Result<AcquiredNode, RenderingError::InvalidReference>
-    pub fn get_acquired_node_of_type(
-        &mut self,
-        fragment: Option<&Fragment>,
-        node_type: NodeType,
-    ) -> Option<AcquiredNode> {
-        fragment
-            .and_then(move |fragment| self.get_acquired_node(fragment))
-            .and_then(|acquired| {
-                if acquired.get().get_type() == node_type {
-                    Some(acquired)
-                } else {
-                    None
-                }
-            })
+    pub fn acquired_nodes(&self) -> &AcquiredNodes {
+        &self.acquired_nodes
     }
 
     // Returns (clip_in_user_space, clip_in_object_space), both Option<RsvgNode>
@@ -394,7 +341,10 @@ impl DrawingCtx {
         &mut self,
         clip_uri: Option<&Fragment>,
     ) -> (Option<RsvgNode>, Option<RsvgNode>) {
-        if let Some(clip_node) = self.get_acquired_node_of_type(clip_uri, NodeType::ClipPath) {
+        if let Some(clip_node) = self
+            .acquired_nodes
+            .get_node_of_type(clip_uri, NodeType::ClipPath)
+        {
             let clip_node = clip_node.get().clone();
 
             let ClipPathUnits(units) =
@@ -528,8 +478,9 @@ impl DrawingCtx {
                     // Mask
 
                     if let Some(mask) = mask {
-                        if let Some(acquired) =
-                            dc.get_acquired_node_of_type(Some(mask), NodeType::Mask)
+                        if let Some(acquired) = dc
+                            .acquired_nodes
+                            .get_node_of_type(Some(mask), NodeType::Mask)
                         {
                             let mask_node = acquired.get();
 
@@ -613,7 +564,10 @@ impl DrawingCtx {
         child_surface: &cairo::ImageSurface,
         node_bbox: BoundingBox,
     ) -> Result<cairo::ImageSurface, RenderingError> {
-        match self.get_acquired_node_of_type(Some(filter_uri), NodeType::Filter) {
+        match self
+            .acquired_nodes
+            .get_node_of_type(Some(filter_uri), NodeType::Filter)
+        {
             Some(acquired) => {
                 let filter_node = acquired.get();
 
@@ -679,7 +633,7 @@ impl DrawingCtx {
             } => {
                 had_paint_server = false;
 
-                if let Some(acquired) = self.get_acquired_node(iri) {
+                if let Some(acquired) = self.acquired_nodes.get_node(iri) {
                     let node = acquired.get();
 
                     if node.get_type() == NodeType::LinearGradient
@@ -1123,10 +1077,83 @@ impl AcquiredNode {
     }
 }
 
+pub struct AcquiredNodes {
+    svg: Rc<Svg>,
+    nodes: Rc<RefCell<Vec<RsvgNode>>>,
+}
+
+impl AcquiredNodes {
+    pub fn new(svg: Rc<Svg>) -> AcquiredNodes {
+        AcquiredNodes {
+            svg,
+            nodes: Rc::new(RefCell::new(Vec::new())),
+        }
+    }
+
+    fn contains(&self, node: &RsvgNode) -> bool {
+        self.nodes
+            .borrow()
+            .iter()
+            .find(|n| Rc::ptr_eq(n, node))
+            .is_some()
+    }
+
+    // Use this function when looking up urls to other nodes. This function
+    // does proper recursion checking and thereby avoids infinite loops.
+    //
+    // Nodes acquired by this function must be released in reverse
+    // acquiring order.
+    //
+    // Note that if you acquire a node, you have to release it before trying to
+    // acquire it again.  If you acquire a node "#foo" and don't release it before
+    // trying to acquire "foo" again, you will obtain a %NULL the second time.
+    pub fn get_node(&self, fragment: &Fragment) -> Option<AcquiredNode> {
+        if let Ok(node) = self.svg.lookup(fragment) {
+            if !self.contains(&node) {
+                self.nodes.borrow_mut().push(node.clone());
+                let acq = AcquiredNode(self.nodes.clone(), node.clone());
+                return Some(acq);
+            }
+        }
+
+        None
+    }
+
+    // Use this function when looking up urls to other nodes, and when you expect
+    // the node to be of a particular type. This function does proper recursion
+    // checking and thereby avoids infinite loops.
+    //
+    // Malformed SVGs, for example, may reference a marker by its IRI, but
+    // the object referenced by the IRI is not a marker.
+    //
+    // Note that if you acquire a node, you have to release it before trying to
+    // acquire it again.  If you acquire a node "#foo" and don't release it before
+    // trying to acquire "foo" again, you will obtain a None the second time.
+    //
+    // For convenience, this function will return None if url is None.
+
+    // FIXME: return a Result<AcquiredNode, RenderingError::InvalidReference>
+    pub fn get_node_of_type(
+        &self,
+        fragment: Option<&Fragment>,
+        node_type: NodeType,
+    ) -> Option<AcquiredNode> {
+        fragment
+            .and_then(move |fragment| self.get_node(fragment))
+            .and_then(|acquired| {
+                if acquired.get().get_type() == node_type {
+                    Some(acquired)
+                } else {
+                    None
+                }
+            })
+    }
+}
+
 /// Keeps a stack of nodes and can check if a certain node is contained in the stack
 ///
 /// Sometimes parts of the code cannot plainly use the implicit stack of acquired
-/// nodes as maintained by DrawingCtx::get_acquired_node(), and they must keep their
+/// nodes as maintained by DrawingCtx::acquired_nodes, and they must keep their
 /// own stack of nodes to test for reference cycles.  NodeStack can be used to do that.
 pub struct NodeStack(Vec<RsvgNode>);
 
diff --git a/rsvg_internals/src/filters/image.rs b/rsvg_internals/src/filters/image.rs
index b736a7ec..b09d39b0 100644
--- a/rsvg_internals/src/filters/image.rs
+++ b/rsvg_internals/src/filters/image.rs
@@ -45,7 +45,8 @@ impl Image {
         fragment: &Fragment,
     ) -> Result<ImageSurface, FilterError> {
         let acquired_drawable = draw_ctx
-            .get_acquired_node(fragment)
+            .acquired_nodes()
+            .get_node(fragment)
             .ok_or(FilterError::InvalidInput)?;
         let drawable = acquired_drawable.get();
 
diff --git a/rsvg_internals/src/gradient.rs b/rsvg_internals/src/gradient.rs
index 4867f519..e3d3cdd1 100644
--- a/rsvg_internals/src/gradient.rs
+++ b/rsvg_internals/src/gradient.rs
@@ -447,7 +447,7 @@ fn acquire_gradient<'a>(
     draw_ctx: &'a mut DrawingCtx,
     name: Option<&Fragment>,
 ) -> Option<AcquiredNode> {
-    name.and_then(move |fragment| draw_ctx.get_acquired_node(fragment))
+    name.and_then(move |fragment| draw_ctx.acquired_nodes().get_node(fragment))
         .and_then(|acquired| {
             let node_type = acquired.get().get_type();
 
diff --git a/rsvg_internals/src/marker.rs b/rsvg_internals/src/marker.rs
index b6ace005..364702a7 100644
--- a/rsvg_internals/src/marker.rs
+++ b/rsvg_internals/src/marker.rs
@@ -595,7 +595,10 @@ fn emit_marker_by_name(
     line_width: f64,
     clipping: bool,
 ) -> Result<(), RenderingError> {
-    if let Some(acquired) = draw_ctx.get_acquired_node_of_type(Some(name), NodeType::Marker) {
+    if let Some(acquired) = draw_ctx
+        .acquired_nodes()
+        .get_node_of_type(Some(name), NodeType::Marker)
+    {
         let node = acquired.get();
 
         node.with_impl(|marker: &NodeMarker| {
diff --git a/rsvg_internals/src/pattern.rs b/rsvg_internals/src/pattern.rs
index ebcdffec..90077096 100644
--- a/rsvg_internals/src/pattern.rs
+++ b/rsvg_internals/src/pattern.rs
@@ -236,8 +236,9 @@ impl PaintSource for NodePattern {
         let mut stack = NodeStack::new();
 
         while !result.is_resolved() {
-            if let Some(acquired) =
-                draw_ctx.get_acquired_node_of_type(result.fallback.as_ref(), NodeType::Pattern)
+            if let Some(acquired) = draw_ctx
+                .acquired_nodes()
+                .get_node_of_type(result.fallback.as_ref(), NodeType::Pattern)
             {
                 let a_node = acquired.get();
 
diff --git a/rsvg_internals/src/structure.rs b/rsvg_internals/src/structure.rs
index 0994fc9e..0b6ad689 100644
--- a/rsvg_internals/src/structure.rs
+++ b/rsvg_internals/src/structure.rs
@@ -370,7 +370,7 @@ impl NodeTrait for NodeUse {
 
         let link = link.as_ref().unwrap();
 
-        let child = if let Some(acquired) = draw_ctx.get_acquired_node(link) {
+        let child = if let Some(acquired) = draw_ctx.acquired_nodes().get_node(link) {
             // Here we clone the acquired child, so that we can drop the AcquiredNode as
             // early as possible.  This is so that the child's drawing method will be able
             // to re-acquire the child for other purposes.
diff --git a/rsvg_internals/src/text.rs b/rsvg_internals/src/text.rs
index 1262f728..c539cad5 100644
--- a/rsvg_internals/src/text.rs
+++ b/rsvg_internals/src/text.rs
@@ -675,7 +675,7 @@ impl NodeTRef {
 
         let values = cascaded.get();
 
-        if let Some(acquired) = draw_ctx.get_acquired_node(link) {
+        if let Some(acquired) = draw_ctx.acquired_nodes().get_node(link) {
             let c = acquired.get();
             extract_chars_children_to_chunks_recursively(chunks, &c, values, depth);
         } else {