[librsvg: 1/14] PropertyBag: don't copy the resulting strings during lookup



commit 94bbc26613af2189b9b83e37a0ab1b10af8a6577
Author: Federico Mena Quintero <federico gnome org>
Date:   Wed Jan 24 10:23:14 2018 -0600

    PropertyBag: don't copy the resulting strings during lookup
    
    String copies in property_bag::lookup() were hurting performance a
    bit.  We want some things:
    
    * lookup() -> Option<&str> instead of lookup() -> Option<String>
      i.e. avoid copies for strings which will be short-lived anyway.
    
    * Don't expose property_bag::free() to callers; have automatic
      resource management.
    
    * Support property_bag::dup() for NodeSvg's special use case.
    
    The strategy is as follows.  We introduce an opaque PropertyBag
    struct, which is just a wrapper for the (RsvgPropertyBag *) that comes
    from the C side (typedef GHashTable RsvgPropertyBag).
    
    PropertyBag has dup() and lookup() methods, and an ffi() method to get
    the underlying pointer for special cases.
    
    We change all the users of the old RsvgPropertyBag pointers to use
    PropertyBag now.  NodeSvg no longer needs to impl Drop, since all it
    was doing was to call property_bag::free().
    
    The few places where the result of lookup() was meant to be
    long-lived, will actually copy the string now for their own purposes.

 rust/src/chars.rs        |  4 +--
 rust/src/clip_path.rs    |  4 +--
 rust/src/cnode.rs        |  8 ++---
 rust/src/gradient.rs     |  7 ++---
 rust/src/image.rs        |  9 +++---
 rust/src/marker.rs       |  5 ++--
 rust/src/mask.rs         |  4 +--
 rust/src/node.rs         | 17 ++++++-----
 rust/src/pattern.rs      |  7 ++---
 rust/src/property_bag.rs | 77 +++++++++++++++++++++++++++++++++---------------
 rust/src/shapes.rs       | 19 ++++++------
 rust/src/stop.rs         | 11 ++++---
 rust/src/structure.rs    | 46 ++++++++++-------------------
 13 files changed, 115 insertions(+), 103 deletions(-)
---
diff --git a/rust/src/chars.rs b/rust/src/chars.rs
index 3faa558..ef0dcb8 100644
--- a/rust/src/chars.rs
+++ b/rust/src/chars.rs
@@ -5,7 +5,7 @@ use std::cell::RefCell;
 use drawing_ctx::{self, RsvgDrawingCtx};
 use handle::RsvgHandle;
 use node::{NodeResult, NodeTrait, NodeType, RsvgCNodeImpl, RsvgNode, boxed_node_new, rsvg_node_get_state};
-use property_bag::RsvgPropertyBag;
+use property_bag::PropertyBag;
 
 /// Container for XML character data.
 ///
@@ -50,7 +50,7 @@ impl NodeChars {
 }
 
 impl NodeTrait for NodeChars {
-    fn set_atts(&self, _: &RsvgNode, _: *const RsvgHandle, _: *const RsvgPropertyBag) -> NodeResult {
+    fn set_atts(&self, _: &RsvgNode, _: *const RsvgHandle, _: &PropertyBag) -> NodeResult {
         Ok(())
     }
 
diff --git a/rust/src/clip_path.rs b/rust/src/clip_path.rs
index 0a3eab0..a2993d4 100644
--- a/rust/src/clip_path.rs
+++ b/rust/src/clip_path.rs
@@ -5,7 +5,7 @@ use drawing_ctx::RsvgDrawingCtx;
 use handle::RsvgHandle;
 use node::{NodeResult, NodeTrait, NodeType, RsvgCNodeImpl, RsvgNode, boxed_node_new};
 use coord_units::CoordUnits;
-use property_bag::{self, RsvgPropertyBag};
+use property_bag::{self, PropertyBag};
 
 coord_units!(ClipPathUnits, CoordUnits::UserSpaceOnUse);
 
@@ -22,7 +22,7 @@ impl NodeClipPath {
 }
 
 impl NodeTrait for NodeClipPath {
-    fn set_atts(&self, _: &RsvgNode, _: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> NodeResult {
+    fn set_atts(&self, _: &RsvgNode, _: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult {
         self.units.set(property_bag::parse_or_default(pbag, "clipPathUnits", (), None)?);
 
         Ok(())
diff --git a/rust/src/cnode.rs b/rust/src/cnode.rs
index bfba391..b8afece 100644
--- a/rust/src/cnode.rs
+++ b/rust/src/cnode.rs
@@ -1,12 +1,12 @@
 use drawing_ctx::RsvgDrawingCtx;
 use handle::*;
 use node::*;
-use property_bag::RsvgPropertyBag;
+use property_bag::{FfiRsvgPropertyBag, PropertyBag};
 use state::RsvgState;
 
 use std::rc::*;
 
-type CNodeSetAtts = unsafe extern "C" fn (node: *const RsvgNode, node_impl: *const RsvgCNodeImpl, handle: 
*const RsvgHandle, pbag: *const RsvgPropertyBag);
+type CNodeSetAtts = unsafe extern "C" fn (node: *const RsvgNode, node_impl: *const RsvgCNodeImpl, handle: 
*const RsvgHandle, pbag: FfiRsvgPropertyBag);
 type CNodeDraw = unsafe extern "C" fn (node: *const RsvgNode, node_impl: *const RsvgCNodeImpl, draw_ctx: 
*const RsvgDrawingCtx, dominate: i32);
 type CNodeFree = unsafe extern "C" fn (node_impl: *const RsvgCNodeImpl);
 
@@ -19,8 +19,8 @@ struct CNode {
 }
 
 impl NodeTrait for CNode {
-    fn set_atts (&self, node: &RsvgNode, handle: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> 
NodeResult {
-        unsafe { (self.set_atts_fn) (node as *const RsvgNode, self.c_node_impl, handle, pbag); }
+    fn set_atts (&self, node: &RsvgNode, handle: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult {
+        unsafe { (self.set_atts_fn) (node as *const RsvgNode, self.c_node_impl, handle, pbag.ffi()); }
 
         Ok (())
     }
diff --git a/rust/src/gradient.rs b/rust/src/gradient.rs
index 32243c6..08d0c68 100644
--- a/rust/src/gradient.rs
+++ b/rust/src/gradient.rs
@@ -16,8 +16,7 @@ use length::*;
 use node::*;
 use paint_server::*;
 use parsers::Parse;
-use property_bag;
-use property_bag::*;
+use property_bag::{self, PropertyBag};
 use stop::*;
 use util::*;
 
@@ -557,7 +556,7 @@ impl NodeGradient {
 }
 
 impl NodeTrait for NodeGradient {
-    fn set_atts (&self, node: &RsvgNode, _: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> NodeResult {
+    fn set_atts (&self, node: &RsvgNode, _: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult {
         let mut g = self.gradient.borrow_mut ();
 
         // Attributes common to linear and radial gradients
@@ -565,7 +564,7 @@ impl NodeTrait for NodeGradient {
         g.common.units    = property_bag::parse_or_none (pbag, "gradientUnits", (), None)?;
         g.common.affine   = property_bag::parse_or_none (pbag, "gradientTransform", (), None)?;
         g.common.spread   = property_bag::parse_or_none (pbag, "spreadMethod", (), None)?;
-        g.common.fallback = property_bag::lookup (pbag, "xlink:href");
+        g.common.fallback = pbag.lookup("xlink:href").map(|s| s.to_owned());
 
         // Attributes specific to each gradient type.  The defaults mandated by the spec
         // are in GradientVariant::resolve_from_defaults()
diff --git a/rust/src/image.rs b/rust/src/image.rs
index 514c7fa..0f29ac0 100644
--- a/rust/src/image.rs
+++ b/rust/src/image.rs
@@ -14,8 +14,7 @@ use aspect_ratio::*;
 use handle::RsvgHandle;
 use length::*;
 use node::*;
-use property_bag;
-use property_bag::RsvgPropertyBag;
+use property_bag::{self, PropertyBag};
 
 struct NodeImage {
     aspect:  Cell<AspectRatio>,
@@ -40,7 +39,7 @@ impl NodeImage {
 }
 
 impl NodeTrait for NodeImage {
-    fn set_atts (&self, _: &RsvgNode, handle: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> NodeResult 
{
+    fn set_atts (&self, _: &RsvgNode, handle: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult {
         self.x.set (property_bag::parse_or_default (pbag, "x", LengthDir::Horizontal, None)?);
         self.y.set (property_bag::parse_or_default (pbag, "y", LengthDir::Vertical, None)?);
         self.w.set (property_bag::parse_or_default (pbag, "width", LengthDir::Horizontal,
@@ -49,11 +48,11 @@ impl NodeTrait for NodeImage {
                                                     Some(RsvgLength::check_nonnegative))?);
         self.aspect.set (property_bag::parse_or_default (pbag, "preserveAspectRatio", (), None)?);
 
-        let mut href = property_bag::lookup (pbag, "xlink:href");
+        let mut href = pbag.lookup("xlink:href");
 
         if href.is_none() {
             // "path" is used by some older adobe illustrator versions
-            href = property_bag::lookup (pbag, "path");
+            href = pbag.lookup("path");
         }
 
         if let Some(href) = href {
diff --git a/rust/src/marker.rs b/rust/src/marker.rs
index 3d1b509..7a1e1b4 100644
--- a/rust/src/marker.rs
+++ b/rust/src/marker.rs
@@ -17,8 +17,7 @@ use path_builder::*;
 use parsers;
 use parsers::Parse;
 use parsers::ParseError;
-use property_bag;
-use property_bag::*;
+use property_bag::{self, PropertyBag};
 use util::*;
 use viewbox::*;
 
@@ -187,7 +186,7 @@ impl NodeMarker {
 }
 
 impl NodeTrait for NodeMarker {
-    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> NodeResult {
+    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult {
         self.units.set (property_bag::parse_or_default (pbag, "markerUnits", (), None)?);
 
         self.ref_x.set (property_bag::parse_or_default (pbag, "refX", LengthDir::Horizontal, None)?);
diff --git a/rust/src/mask.rs b/rust/src/mask.rs
index 492d0c8..54a525a 100644
--- a/rust/src/mask.rs
+++ b/rust/src/mask.rs
@@ -7,7 +7,7 @@ use handle::RsvgHandle;
 use length::{RsvgLength, LengthDir};
 use node::{NodeResult, NodeTrait, NodeType, RsvgCNodeImpl, RsvgNode, boxed_node_new};
 use parsers::Parse;
-use property_bag::{self, RsvgPropertyBag};
+use property_bag::{self, PropertyBag};
 
 coord_units!(MaskUnits, CoordUnits::ObjectBoundingBox);
 coord_units!(MaskContentUnits, CoordUnits::UserSpaceOnUse);
@@ -46,7 +46,7 @@ impl NodeMask {
 }
 
 impl NodeTrait for NodeMask {
-    fn set_atts(&self, _: &RsvgNode, _: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> NodeResult {
+    fn set_atts(&self, _: &RsvgNode, _: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult {
         self.x.set(property_bag::parse_or_value(pbag, "x",
                                                 LengthDir::Horizontal,
                                                 NodeMask::get_default_pos(LengthDir::Horizontal),
diff --git a/rust/src/node.rs b/rust/src/node.rs
index 2aadfd9..be76e51 100644
--- a/rust/src/node.rs
+++ b/rust/src/node.rs
@@ -12,7 +12,7 @@ use drawing_ctx;
 use error::*;
 use handle::RsvgHandle;
 use parsers::ParseError;
-use property_bag::RsvgPropertyBag;
+use property_bag::{FfiRsvgPropertyBag, PropertyBag};
 use state::RsvgState;
 
 /* A *const RsvgNode is just a pointer for the C code's benefit: it
@@ -27,7 +27,7 @@ pub type RsvgNode = Rc<Node>;
 pub enum RsvgCNodeImpl {}
 
 pub trait NodeTrait: Downcast {
-    fn set_atts (&self, node: &RsvgNode, handle: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> 
NodeResult;
+    fn set_atts (&self, node: &RsvgNode, handle: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult;
     fn draw (&self, node: &RsvgNode, draw_ctx: *const RsvgDrawingCtx, dominate: i32);
     fn get_c_impl (&self) -> *const RsvgCNodeImpl;
 }
@@ -170,7 +170,7 @@ impl Node {
         self.children.borrow_mut ().push (child.clone ());
     }
 
-    pub fn set_atts (&self, node: &RsvgNode, handle: *const RsvgHandle, pbag: *const RsvgPropertyBag) {
+    pub fn set_atts (&self, node: &RsvgNode, handle: *const RsvgHandle, pbag: &PropertyBag) {
         *self.result.borrow_mut () = self.node_impl.set_atts (node, handle, pbag);
     }
 
@@ -359,11 +359,15 @@ pub extern fn rsvg_node_add_child (raw_node: *mut RsvgNode, raw_child: *const Rs
 }
 
 #[no_mangle]
-pub extern fn rsvg_node_set_atts (raw_node: *mut RsvgNode, handle: *const RsvgHandle, pbag: *const 
RsvgPropertyBag) {
+pub extern fn rsvg_node_set_atts (raw_node: *mut RsvgNode,
+                                  handle: *const RsvgHandle,
+                                  ffi_pbag: FfiRsvgPropertyBag) {
     assert! (!raw_node.is_null ());
     let node: &RsvgNode = unsafe { & *raw_node };
 
-    node.set_atts (node, handle, pbag);
+    let pbag = PropertyBag::new(ffi_pbag);
+
+    node.set_atts (node, handle, &pbag);
 }
 
 #[no_mangle]
@@ -423,14 +427,13 @@ mod tests {
     use std::rc::Rc;
     use drawing_ctx::RsvgDrawingCtx;
     use handle::RsvgHandle;
-    use property_bag::RsvgPropertyBag;
     use super::*;
     use std::ptr;
 
     struct TestNodeImpl {}
 
     impl NodeTrait for TestNodeImpl {
-        fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, _: *const RsvgPropertyBag) -> NodeResult {
+        fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, _: &PropertyBag) -> NodeResult {
             Ok (())
         }
 
diff --git a/rust/src/pattern.rs b/rust/src/pattern.rs
index 03bac23..76b32c1 100644
--- a/rust/src/pattern.rs
+++ b/rust/src/pattern.rs
@@ -17,8 +17,7 @@ use drawing_ctx::RsvgDrawingCtx;
 use handle::RsvgHandle;
 use length::*;
 use node::*;
-use property_bag;
-use property_bag::*;
+use property_bag::{self, PropertyBag};
 use util::*;
 use viewbox::*;
 
@@ -165,7 +164,7 @@ impl NodePattern {
 }
 
 impl NodeTrait for NodePattern {
-    fn set_atts (&self, node: &RsvgNode, _: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> NodeResult {
+    fn set_atts (&self, node: &RsvgNode, _: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult {
         let mut p = self.pattern.borrow_mut ();
 
         p.node = Some (Rc::downgrade (node));
@@ -178,7 +177,7 @@ impl NodeTrait for NodePattern {
 
         p.affine = property_bag::parse_or_none (pbag, "patternTransform", (), None)?;
 
-        p.fallback = property_bag::lookup (pbag, "xlink:href");
+        p.fallback = pbag.lookup("xlink:href").map(|s| s.to_owned());
 
         p.x      = property_bag::parse_or_none (pbag, "x", LengthDir::Horizontal, None)?;
         p.y      = property_bag::parse_or_none (pbag, "y", LengthDir::Vertical, None)?;
diff --git a/rust/src/property_bag.rs b/rust/src/property_bag.rs
index 59b615a..b05e84d 100644
--- a/rust/src/property_bag.rs
+++ b/rust/src/property_bag.rs
@@ -1,43 +1,72 @@
-use ::glib::translate::*;
-use ::libc;
+use glib::translate::*;
+use libc;
+use std::ffi::CStr;
 
 use error::*;
 use parsers::Parse;
 
-pub enum RsvgPropertyBag {}
+pub type FfiRsvgPropertyBag = *mut libc::c_void;
+
+pub enum PropertyBag {
+    Borrowed(FfiRsvgPropertyBag),
+    Owned(FfiRsvgPropertyBag)
+}
 
 extern "C" {
-    fn rsvg_property_bag_lookup (pbag: *const RsvgPropertyBag, key: *const libc::c_char) -> *const 
libc::c_char;
-    fn rsvg_property_bag_dup (pbag: *const RsvgPropertyBag) -> *mut RsvgPropertyBag;
-    fn rsvg_property_bag_free (pbag: *mut RsvgPropertyBag);
+    fn rsvg_property_bag_lookup (pbag: FfiRsvgPropertyBag, key: *const libc::c_char) -> *const libc::c_char;
+    fn rsvg_property_bag_dup (pbag: FfiRsvgPropertyBag) -> FfiRsvgPropertyBag;
+    fn rsvg_property_bag_free (pbag: FfiRsvgPropertyBag);
 }
 
-pub fn lookup (pbag: *const RsvgPropertyBag, key: &str) -> Option<String> {
-    unsafe {
-        let c_value = rsvg_property_bag_lookup (pbag, key.to_glib_none ().0);
-        from_glib_none (c_value)
+impl PropertyBag {
+    pub fn new(ffi: FfiRsvgPropertyBag) -> PropertyBag {
+        PropertyBag::Borrowed(ffi)
     }
-}
 
-pub fn dup (pbag: *const RsvgPropertyBag) -> *mut RsvgPropertyBag {
-    unsafe {
-        rsvg_property_bag_dup (pbag)
+    pub fn ffi(&self) -> FfiRsvgPropertyBag {
+        match self {
+            &PropertyBag::Borrowed(ffi) => ffi,
+            &PropertyBag::Owned(ffi) => ffi
+        }
+    }
+
+    pub fn dup(&self) -> PropertyBag {
+        unsafe {
+            PropertyBag::Owned(rsvg_property_bag_dup(self.ffi()))
+        }
+    }
+
+    pub fn lookup(&self, key: &str) -> Option<&str> {
+        let ffi = self.ffi();
+
+        unsafe {
+            let c_value = rsvg_property_bag_lookup (ffi, key.to_glib_none ().0);
+            if c_value.is_null() {
+                None
+            } else {
+                // we can unwrap because libxml2 already validated this for UTF-8
+                Some(CStr::from_ptr(c_value).to_str().unwrap())
+            }
+        }
     }
 }
 
-pub fn free (pbag: *mut RsvgPropertyBag) {
-    unsafe {
-        rsvg_property_bag_free (pbag);
+impl Drop for PropertyBag {
+    fn drop(&mut self) {
+        match *self {
+            PropertyBag::Borrowed(_) => (),
+            PropertyBag::Owned(ffi) => unsafe { rsvg_property_bag_free(ffi) }
+        }
     }
 }
 
-pub fn parse_or_none<T> (pbag: *const RsvgPropertyBag,
-                         key: &'static str,
+pub fn parse_or_none<T> (pbag: &PropertyBag,
+                         key: &str,
                          data: <T as Parse>::Data,
                          validate: Option<fn(T) -> Result<T, AttributeError>>) -> Result <Option<T>, 
NodeError>
     where T: Parse<Err = AttributeError> + Copy
 {
-    let value = lookup (pbag, key);
+    let value = pbag.lookup(key);
 
     match value {
         Some (v) => {
@@ -56,8 +85,8 @@ pub fn parse_or_none<T> (pbag: *const RsvgPropertyBag,
     }
 }
 
-pub fn parse_or_default<T> (pbag: *const RsvgPropertyBag,
-                            key: &'static str,
+pub fn parse_or_default<T> (pbag: &PropertyBag,
+                            key: &str,
                             data: <T as Parse>::Data,
                             validate: Option<fn(T) -> Result<T, AttributeError>>) -> Result <T, NodeError>
     where T: Default + Parse<Err = AttributeError> + Copy
@@ -65,8 +94,8 @@ pub fn parse_or_default<T> (pbag: *const RsvgPropertyBag,
     parse_or_value (pbag, key, data, T::default (), validate)
 }
 
-pub fn parse_or_value<T> (pbag: *const RsvgPropertyBag,
-                          key: &'static str,
+pub fn parse_or_value<T> (pbag: &PropertyBag,
+                          key: &str,
                           data: <T as Parse>::Data,
                           value: T,
                           validate: Option<fn(T) -> Result<T, AttributeError>>) -> Result <T, NodeError>
diff --git a/rust/src/shapes.rs b/rust/src/shapes.rs
index c1cc421..a1a48c1 100644
--- a/rust/src/shapes.rs
+++ b/rust/src/shapes.rs
@@ -13,8 +13,7 @@ use node::*;
 use parsers;
 use path_builder::*;
 use path_parser;
-use property_bag;
-use property_bag::*;
+use property_bag::{self, PropertyBag};
 use state::RsvgState;
 
 fn render_path_builder (builder:  &RsvgPathBuilder,
@@ -88,8 +87,8 @@ impl NodePath {
 }
 
 impl NodeTrait for NodePath {
-    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> NodeResult {
-        if let Some (value) = property_bag::lookup (pbag, "d") {
+    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult {
+        if let Some (value) = pbag.lookup("d") {
             let mut builder = RsvgPathBuilder::new ();
 
             if path_parser::parse_path_into_builder (&value, &mut builder).is_err() {
@@ -137,11 +136,11 @@ impl NodePoly {
 }
 
 impl NodeTrait for NodePoly {
-    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> NodeResult {
+    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult {
         // support for svg < 1.0 which used verts
 
         for name in &["verts", "points"] {
-            if let Some (value) = property_bag::lookup (pbag, name) {
+            if let Some (value) = pbag.lookup(name) {
                 let result = parsers::list_of_points (value.trim ());
 
                 match result {
@@ -204,7 +203,7 @@ impl NodeLine {
 }
 
 impl NodeTrait for NodeLine {
-    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> NodeResult {
+    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult {
         self.x1.set (property_bag::parse_or_default (pbag, "x1", LengthDir::Horizontal, None)?);
         self.y1.set (property_bag::parse_or_default (pbag, "y1", LengthDir::Vertical, None)?);
         self.x2.set (property_bag::parse_or_default (pbag, "x2", LengthDir::Horizontal, None)?);
@@ -261,7 +260,7 @@ impl NodeRect {
 }
 
 impl NodeTrait for NodeRect {
-    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> NodeResult {
+    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult {
         self.x.set (property_bag::parse_or_default (pbag, "x", LengthDir::Horizontal, None)?);
         self.y.set (property_bag::parse_or_default (pbag, "y", LengthDir::Vertical, None)?);
         self.w.set (property_bag::parse_or_default (pbag, "width", LengthDir::Horizontal, None)?);
@@ -436,7 +435,7 @@ impl NodeCircle {
 }
 
 impl NodeTrait for NodeCircle {
-    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> NodeResult {
+    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult {
         self.cx.set (property_bag::parse_or_default (pbag, "cx", LengthDir::Horizontal, None)?);
         self.cy.set (property_bag::parse_or_default (pbag, "cy", LengthDir::Vertical, None)?);
 
@@ -480,7 +479,7 @@ impl NodeEllipse {
 }
 
 impl NodeTrait for NodeEllipse {
-    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> NodeResult {
+    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult {
         self.cx.set (property_bag::parse_or_default (pbag, "cx", LengthDir::Horizontal, None)?);
         self.cy.set (property_bag::parse_or_default (pbag, "cy", LengthDir::Vertical, None)?);
 
diff --git a/rust/src/stop.rs b/rust/src/stop.rs
index 8defca7..b8c2374 100644
--- a/rust/src/stop.rs
+++ b/rust/src/stop.rs
@@ -12,8 +12,7 @@ use handle::RsvgHandle;
 use length::*;
 use node::*;
 use opacity::*;
-use property_bag;
-use property_bag::*;
+use property_bag::{self, FfiRsvgPropertyBag, PropertyBag};
 use state::RsvgState;
 
 pub struct NodeStop {
@@ -60,7 +59,7 @@ fn validate_offset(length: RsvgLength) -> Result<RsvgLength, AttributeError> {
 }
 
 impl NodeTrait for NodeStop {
-    fn set_atts (&self, node: &RsvgNode, handle: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> 
NodeResult {
+    fn set_atts (&self, node: &RsvgNode, handle: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult {
         let length = property_bag::parse_or_default (pbag, "offset", LengthDir::Both,
                                                      Some(validate_offset))?;
         assert! (length.unit == LengthUnit::Default || length.unit == LengthUnit::Percent);
@@ -76,14 +75,14 @@ impl NodeTrait for NodeStop {
         // Should we resolve the stop-color / stop-opacity at
         // rendering time?
 
-        if let Some (v) = property_bag::lookup (pbag, "style") {
+        if let Some (v) = pbag.lookup("style") {
             unsafe {
                 rsvg_parse_style (handle, state, v.to_glib_none ().0);
             }
         }
 
         unsafe {
-            rsvg_parse_style_pairs (state, pbag);
+            rsvg_parse_style_pairs (state, pbag.ffi());
         }
 
         let inherited_state = drawing_ctx::state_new ();
@@ -173,7 +172,7 @@ fn u32_from_rgba (rgba: cssparser::RGBA) -> u32 {
 }
 
 extern "C" {
-    fn rsvg_parse_style_pairs (state: *mut RsvgState, pbag: *const RsvgPropertyBag);
+    fn rsvg_parse_style_pairs (state: *mut RsvgState, pbag: FfiRsvgPropertyBag);
     fn rsvg_parse_style (handle: *const RsvgHandle, state: *mut RsvgState, string: *const libc::c_char);
 }
 
diff --git a/rust/src/structure.rs b/rust/src/structure.rs
index 2377192..2122c41 100644
--- a/rust/src/structure.rs
+++ b/rust/src/structure.rs
@@ -3,7 +3,6 @@ use ::libc;
 
 use std::cell::RefCell;
 use std::cell::Cell;
-use std::ptr;
 
 use cairo::MatrixTrait;
 
@@ -14,8 +13,7 @@ use handle::RsvgHandle;
 use length::*;
 use node::*;
 use parsers::Parse;
-use property_bag;
-use property_bag::*;
+use property_bag::{self, FfiRsvgPropertyBag, PropertyBag};
 use util::*;
 use viewbox::*;
 use viewport::{ClipMode,draw_in_viewport};
@@ -31,7 +29,7 @@ impl NodeGroup {
 }
 
 impl NodeTrait for NodeGroup {
-    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, _: *const RsvgPropertyBag) -> NodeResult {
+    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, _: &PropertyBag) -> NodeResult {
         Ok (())
     }
 
@@ -55,7 +53,7 @@ impl NodeDefs {
 }
 
 impl NodeTrait for NodeDefs {
-    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, _: *const RsvgPropertyBag) -> NodeResult {
+    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, _: &PropertyBag) -> NodeResult {
         Ok (())
     }
 
@@ -79,7 +77,7 @@ impl NodeSwitch {
 }
 
 impl NodeTrait for NodeSwitch {
-    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, _: *const RsvgPropertyBag) -> NodeResult {
+    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, _: &PropertyBag) -> NodeResult {
         Ok (())
     }
 
@@ -119,7 +117,7 @@ struct NodeSvg {
     w:                     Cell<RsvgLength>,
     h:                     Cell<RsvgLength>,
     vbox:                  Cell<Option<ViewBox>>,
-    atts:                  Cell<*mut RsvgPropertyBag>
+    atts:                  RefCell<Option<PropertyBag>>
 }
 
 impl NodeSvg {
@@ -131,13 +129,13 @@ impl NodeSvg {
             w:                     Cell::new (RsvgLength::parse ("100%", LengthDir::Horizontal).unwrap ()),
             h:                     Cell::new (RsvgLength::parse ("100%", LengthDir::Vertical).unwrap ()),
             vbox:                  Cell::new (None),
-            atts:                  Cell::new (ptr::null_mut ())
+            atts:                  RefCell::new(None)
         }
     }
 }
 
 impl NodeTrait for NodeSvg {
-    fn set_atts (&self, node: &RsvgNode, _: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> NodeResult {
+    fn set_atts (&self, node: &RsvgNode, _: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult {
         self.preserve_aspect_ratio.set (property_bag::parse_or_default (pbag, "preserveAspectRatio", (), 
None)?);
 
         // x & y attributes have no effect on outermost svg
@@ -163,7 +161,7 @@ impl NodeTrait for NodeSvg {
 
         // The "style" sub-element is not loaded yet here, so we need
         // to store other attributes to be applied later.
-        self.atts.set (property_bag::dup (pbag));
+        *self.atts.borrow_mut() = Some(pbag.dup());
 
         Ok (())
     }
@@ -198,16 +196,6 @@ impl NodeTrait for NodeSvg {
     }
 }
 
-impl Drop for NodeSvg {
-    fn drop (&mut self) {
-        let pbag = self.atts.get ();
-
-        if !pbag.is_null () {
-            property_bag::free (pbag);
-        }
-    }
-}
-
 /***** NodeUse *****/
 
 struct NodeUse {
@@ -231,8 +219,8 @@ impl NodeUse {
 }
 
 impl NodeTrait for NodeUse {
-    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> NodeResult {
-        *self.link.borrow_mut () = property_bag::lookup (pbag, "xlink:href");
+    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult {
+        *self.link.borrow_mut () = pbag.lookup("xlink:href").map(|s| s.to_owned());
 
         self.x.set (property_bag::parse_or_default (pbag, "x", LengthDir::Horizontal, None)?);
         self.y.set (property_bag::parse_or_default (pbag, "y", LengthDir::Vertical, None)?);
@@ -348,7 +336,7 @@ impl NodeSymbol {
 }
 
 impl NodeTrait for NodeSymbol {
-    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: *const RsvgPropertyBag) -> NodeResult {
+    fn set_atts (&self, _: &RsvgNode, _: *const RsvgHandle, pbag: &PropertyBag) -> NodeResult {
         self.preserve_aspect_ratio.set (property_bag::parse_or_default (pbag, "preserveAspectRatio", (), 
None)?);
         self.vbox.set (property_bag::parse_or_none (pbag, "viewBox", (), None)?);
 
@@ -444,7 +432,7 @@ extern "C" {
                                tag:    *const libc::c_char,
                                class:  *const libc::c_char,
                                id:     *const libc::c_char,
-                               pbag:   *const RsvgPropertyBag);
+                               pbag:   FfiRsvgPropertyBag);
 }
 
 #[no_mangle]
@@ -453,11 +441,9 @@ pub extern fn rsvg_node_svg_apply_atts (raw_node: *const RsvgNode, handle: *cons
     let node: &RsvgNode = unsafe { & *raw_node };
 
     node.with_impl (|svg: &NodeSvg| {
-        let pbag = svg.atts.get ();
-
-        if !pbag.is_null () {
-            let class = property_bag::lookup (pbag, "class");
-            let id = property_bag::lookup (pbag, "id");
+        if let Some(pbag) = svg.atts.borrow().as_ref() {
+            let class = pbag.lookup("class");
+            let id = pbag.lookup("id");
 
             let c_class = class.to_glib_none ();
             let c_id = id.to_glib_none ();
@@ -467,7 +453,7 @@ pub extern fn rsvg_node_svg_apply_atts (raw_node: *const RsvgNode, handle: *cons
                                              str::to_glib_none ("svg").0,
                                              c_class.0,
                                              c_id.0,
-                                             pbag); }
+                                             pbag.ffi()); }
         }
     });
 }


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