[librsvg/librsvg-2.44] AcquiredNode.get(): Return a reference, not a cloned Rc<>

[Federico Mena Quintero <federico src gnome org>]

commit ec12d042155a63e5d08e300329c6a0e1ca866d29
Author: Federico Mena Quintero <federico gnome org>
Date:   Wed Sep 19 13:16:44 2018 -0500

    AcquiredNode.get(): Return a reference, not a cloned Rc<>
    
    This does two things:
    
    * Makes it more clear that the AcquiredNode must be kept alive while
      the contained node is being used.  The node's Rc could still be
      cloned and used after the AcquiredNode is dropped, but we'll try not
      to do that.
    
    * It breaks the test for 761175-recursive-masks.svg; to fix later.

 rsvg_internals/src/drawing_ctx.rs |  50 +++++--------
 rsvg_internals/src/gradient.rs    |  18 ++---
 rsvg_internals/src/length.rs      |   4 +-
 rsvg_internals/src/structure.rs   | 152 +++++++++++++++++++-------------------
 4 files changed, 104 insertions(+), 120 deletions(-)
---
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index 0b22d6c7..5d831c78 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -286,31 +286,24 @@ impl<'a> DrawingCtx<'a> {
 
             let affine = original_cr.get_matrix();
 
-            let (clip_node, clip_units) = {
-                let clip_node = self
-                    .get_acquired_node_of_type(clip_uri, NodeType::ClipPath)
-                    .and_then(|acquired| Some(acquired.get()));
-
-                let mut clip_units = Default::default();
-
-                if let Some(ref clip_node) = clip_node {
-                    clip_node.with_impl(|clip_path: &NodeClipPath| {
-                        let ClipPathUnits(u) = clip_path.get_units();
-                        clip_units = Some(u);
-                    });
+            let (acquired_clip, clip_units) = {
+                if let Some(acquired) = self.get_acquired_node_of_type(clip_uri, NodeType::ClipPath)
+                {
+                    let ClipPathUnits(units) = acquired
+                        .get()
+                        .with_impl(|clip_path: &NodeClipPath| clip_path.get_units());
+
+                    (Some(acquired), Some(units))
+                } else {
+                    (None, None)
                 }
-
-                (clip_node, clip_units)
             };
 
             if clip_units == Some(CoordUnits::UserSpaceOnUse) {
-                let res = if let Some(ref clip_node) = clip_node {
-                    clip_node.with_impl(|clip_path: &NodeClipPath| {
-                        clip_path.to_cairo_context(clip_node, &affine, self)
-                    })
-                } else {
-                    Ok(())
-                };
+                let clip_node = acquired_clip.as_ref().unwrap().get();
+                let res = clip_node.with_impl(|clip_path: &NodeClipPath| {
+                    clip_path.to_cairo_context(clip_node, &affine, self)
+                });
 
                 if let Err(e) = res {
                     original_cr.restore();
@@ -400,13 +393,10 @@ impl<'a> DrawingCtx<'a> {
                 original_cr.set_source_surface(&filter_result_surface, xofs, yofs);
 
                 if clip_units == Some(CoordUnits::ObjectBoundingBox) {
-                    let res = if let Some(ref clip_node) = clip_node {
-                        clip_node.with_impl(|clip_path: &NodeClipPath| {
-                            clip_path.to_cairo_context(clip_node, &affine, self)
-                        })
-                    } else {
-                        Ok(())
-                    };
+                    let clip_node = acquired_clip.as_ref().unwrap().get();
+                    let res = clip_node.with_impl(|clip_path: &NodeClipPath| {
+                        clip_path.to_cairo_context(clip_node, &affine, self)
+                    });
 
                     if let Err(e) = res {
                         original_cr.restore();
@@ -1037,8 +1027,8 @@ impl Drop for AcquiredNode {
 }
 
 impl AcquiredNode {
-    pub fn get(&self) -> RsvgNode {
-        self.1.clone()
+    pub fn get(&self) -> &RsvgNode {
+        &self.1
     }
 }
 
diff --git a/rsvg_internals/src/gradient.rs b/rsvg_internals/src/gradient.rs
index 114882b2..bcc94473 100644
--- a/rsvg_internals/src/gradient.rs
+++ b/rsvg_internals/src/gradient.rs
@@ -416,17 +416,15 @@ impl Gradient {
 }
 
 fn acquire_gradient<'a>(draw_ctx: &'a mut DrawingCtx, name: &str) -> Option<AcquiredNode> {
-    draw_ctx.get_acquired_node(name).and_then(|acquired| {
-        // FIXME: replace with .filter() once Option.filter() becomes stable
-        let node = acquired.get();
-        if node.get_type() == NodeType::LinearGradient
-            || node.get_type() == NodeType::RadialGradient
-        {
-            Some(acquired)
-        } else {
-            None
+    if let Some(acquired) = draw_ctx.get_acquired_node(name) {
+        let node_type = acquired.get().get_type();
+
+        if node_type == NodeType::LinearGradient || node_type == NodeType::RadialGradient {
+            return Some(acquired);
         }
-    })
+    }
+
+    None
 }
 
 fn resolve_gradient(gradient: &Gradient, draw_ctx: &mut DrawingCtx) -> Gradient {
diff --git a/rsvg_internals/src/length.rs b/rsvg_internals/src/length.rs
index 306f9d8b..fdec8dd3 100644
--- a/rsvg_internals/src/length.rs
+++ b/rsvg_internals/src/length.rs
@@ -243,9 +243,7 @@ fn font_size_from_values(values: &ComputedValues, draw_ctx: &DrawingCtx) -> f64
 
         LengthUnit::Inch => font_size_from_inch(v.length, v.dir, draw_ctx),
 
-        LengthUnit::Percent => {
-            unreachable!("ComputedValues can't have a relative font size")
-        }
+        LengthUnit::Percent => unreachable!("ComputedValues can't have a relative font size"),
 
         LengthUnit::FontEm | LengthUnit::FontEx => {
             // This is the same default as used in rsvg_node_svg_get_size()
diff --git a/rsvg_internals/src/structure.rs b/rsvg_internals/src/structure.rs
index 5f79f3bc..a8e9c63c 100644
--- a/rsvg_internals/src/structure.rs
+++ b/rsvg_internals/src/structure.rs
@@ -286,87 +286,85 @@ impl NodeTrait for NodeUse {
             return Ok(());
         }
 
-        let acquired = draw_ctx.get_acquired_node(link.as_ref().unwrap());
+        if let Some(acquired) = draw_ctx.get_acquired_node(link.as_ref().unwrap()) {
+            let child = acquired.get();
 
-        let child = if let Some(acquired) = acquired {
-            acquired.get()
-        } else {
-            return Ok(());
-        };
-
-        if Node::is_ancestor(node.clone(), child.clone()) {
-            // or, if we're <use>'ing ourselves
-            return Err(RenderingError::CircularReference);
-        }
-
-        draw_ctx.increase_num_elements_rendered_through_use(1);
-
-        let nx = self.x.get().normalize(values, draw_ctx);
-        let ny = self.y.get().normalize(values, draw_ctx);
+            if Node::is_ancestor(node.clone(), child.clone()) {
+                // or, if we're <use>'ing ourselves
+                return Err(RenderingError::CircularReference);
+            }
 
-        // If attributes ‘width’ and/or ‘height’ are not specified,
-        // [...] use values of '100%' for these attributes.
-        // From https://www.w3.org/TR/SVG/struct.html#UseElement in
-        // "If the ‘use’ element references a ‘symbol’ element"
-
-        let nw = self
-            .w
-            .get()
-            .unwrap_or_else(|| Length::parse_str("100%", LengthDir::Horizontal).unwrap())
-            .normalize(values, draw_ctx);
-        let nh = self
-            .h
-            .get()
-            .unwrap_or_else(|| Length::parse_str("100%", LengthDir::Vertical).unwrap())
-            .normalize(values, draw_ctx);
-
-        // width or height set to 0 disables rendering of the element
-        // https://www.w3.org/TR/SVG/struct.html#UseElementWidthAttribute
-        if nw.approx_eq_cairo(&0.0) || nh.approx_eq_cairo(&0.0) {
-            return Ok(());
-        }
+            draw_ctx.increase_num_elements_rendered_through_use(1);
+
+            let nx = self.x.get().normalize(values, draw_ctx);
+            let ny = self.y.get().normalize(values, draw_ctx);
+
+            // If attributes ‘width’ and/or ‘height’ are not specified,
+            // [...] use values of '100%' for these attributes.
+            // From https://www.w3.org/TR/SVG/struct.html#UseElement in
+            // "If the ‘use’ element references a ‘symbol’ element"
+
+            let nw = self
+                .w
+                .get()
+                .unwrap_or_else(|| Length::parse_str("100%", LengthDir::Horizontal).unwrap())
+                .normalize(values, draw_ctx);
+            let nh = self
+                .h
+                .get()
+                .unwrap_or_else(|| Length::parse_str("100%", LengthDir::Vertical).unwrap())
+                .normalize(values, draw_ctx);
+
+            // width or height set to 0 disables rendering of the element
+            // https://www.w3.org/TR/SVG/struct.html#UseElementWidthAttribute
+            if nw.approx_eq_cairo(&0.0) || nh.approx_eq_cairo(&0.0) {
+                return Ok(());
+            }
 
-        if child.get_type() != NodeType::Symbol {
-            let cr = draw_ctx.get_cairo_context();
-            cr.translate(nx, ny);
-
-            draw_ctx.with_discrete_layer(node, values, clipping, &mut |dc| {
-                dc.draw_node_from_stack(
-                    &CascadedValues::new_from_values(&child, values),
-                    &child,
-                    clipping,
-                )
-            })
+            if child.get_type() != NodeType::Symbol {
+                let cr = draw_ctx.get_cairo_context();
+                cr.translate(nx, ny);
+
+                draw_ctx.with_discrete_layer(node, values, clipping, &mut |dc| {
+                    dc.draw_node_from_stack(
+                        &CascadedValues::new_from_values(&child, values),
+                        &child,
+                        clipping,
+                    )
+                })
+            } else {
+                child.with_impl(|symbol: &NodeSymbol| {
+                    let do_clip = !values.is_overflow()
+                        || (values.overflow == Overflow::Visible
+                            && child.get_specified_values().is_overflow());
+
+                    draw_in_viewport(
+                        nx,
+                        ny,
+                        nw,
+                        nh,
+                        ClipMode::ClipToVbox,
+                        do_clip,
+                        symbol.vbox.get(),
+                        symbol.preserve_aspect_ratio.get(),
+                        node,
+                        values,
+                        draw_ctx.get_cairo_context().get_matrix(),
+                        draw_ctx,
+                        clipping,
+                        &mut |dc| {
+                            // We don't push a layer because draw_in_viewport() already does it
+                            child.draw_children(
+                                &CascadedValues::new_from_values(&child, values),
+                                dc,
+                                clipping,
+                            )
+                        },
+                    )
+                })
+            }
         } else {
-            child.with_impl(|symbol: &NodeSymbol| {
-                let do_clip = !values.is_overflow()
-                    || (values.overflow == Overflow::Visible
-                        && child.get_specified_values().is_overflow());
-
-                draw_in_viewport(
-                    nx,
-                    ny,
-                    nw,
-                    nh,
-                    ClipMode::ClipToVbox,
-                    do_clip,
-                    symbol.vbox.get(),
-                    symbol.preserve_aspect_ratio.get(),
-                    node,
-                    values,
-                    draw_ctx.get_cairo_context().get_matrix(),
-                    draw_ctx,
-                    clipping,
-                    &mut |dc| {
-                        // We don't push a layer because draw_in_viewport() already does it
-                        child.draw_children(
-                            &CascadedValues::new_from_values(&child, values),
-                            dc,
-                            clipping,
-                        )
-                    },
-                )
-            })
+            Ok(())
         }
     }
 }