[librsvg/librsvg-2.44] node: Use a RefCell for the State



commit 4dd287759822978038ec728c966b8009385940e0
Author: Paolo Borelli <pborelli gnome org>
Date:   Sun Sep 2 11:55:09 2018 +0200

    node: Use a RefCell for the State
    
    We can remove the use of a raw pointer and use RefCell instead.
    Since the state is now only mutable internally, we need to move
    the parse_style_attributes method on the node itself.
    This also allows to remove the manual Drop implementation of Node.

 rsvg_internals/src/load.rs      |   5 +-
 rsvg_internals/src/node.rs      | 189 ++++++++++++++++++++++++++++------------
 rsvg_internals/src/state.rs     | 168 ++---------------------------------
 rsvg_internals/src/structure.rs |   9 +-
 4 files changed, 145 insertions(+), 226 deletions(-)
---
diff --git a/rsvg_internals/src/load.rs b/rsvg_internals/src/load.rs
index 248dd313..88e89ffa 100644
--- a/rsvg_internals/src/load.rs
+++ b/rsvg_internals/src/load.rs
@@ -32,7 +32,6 @@ use node::*;
 use pattern::NodePattern;
 use property_bag::PropertyBag;
 use shapes::{NodeCircle, NodeEllipse, NodeLine, NodePath, NodePoly, NodeRect};
-use state::parse_style_attrs;
 use stop::NodeStop;
 use structure::{NodeDefs, NodeGroup, NodeSvg, NodeSwitch, NodeSymbol, NodeUse};
 use text::{NodeTRef, NodeTSpan, NodeText};
@@ -333,7 +332,7 @@ pub extern "C" fn rsvg_load_set_node_atts(
     // attributes until the end, when sax_end_element_cb() calls
     // rsvg_node_svg_apply_atts()
     if node.get_type() != NodeType::Svg {
-        parse_style_attrs(handle, node, tag, pbag);
+        node.parse_style_attributes(handle, tag, pbag);
     }
 
     node.set_overridden_properties();
@@ -352,6 +351,6 @@ pub extern "C" fn rsvg_load_set_svg_node_atts(
     }
 
     node.with_impl(|svg: &NodeSvg| {
-        svg.parse_style_attrs(node, handle);
+        svg.parse_style_attributes(node, handle);
     });
 }
diff --git a/rsvg_internals/src/node.rs b/rsvg_internals/src/node.rs
index 323395ba..c5182e2d 100644
--- a/rsvg_internals/src/node.rs
+++ b/rsvg_internals/src/node.rs
@@ -3,7 +3,7 @@ use downcast_rs::*;
 use glib;
 use glib::translate::*;
 use glib_sys;
-
+use libc;
 use std::cell::{Cell, Ref, RefCell};
 use std::ptr;
 use std::rc::{Rc, Weak};
@@ -15,16 +15,15 @@ use error::*;
 use handle::RsvgHandle;
 use parsers::Parse;
 use property_bag::PropertyBag;
-use state::{
-    self,
-    rsvg_state_new,
-    ComputedValues,
-    Overflow,
-    RsvgState,
-    SpecifiedValue,
-    SpecifiedValues,
-    State,
-};
+use state::{ComputedValues, Overflow, RsvgState, SpecifiedValue, State};
+
+extern "C" {
+    fn rsvg_lookup_apply_css_style(
+        handle: *const RsvgHandle,
+        target: *const libc::c_char,
+        state: *mut RsvgState,
+    ) -> glib_sys::gboolean;
+}
 
 // A *const RsvgNode is just a pointer for the C code's benefit: it
 // points to an  Rc<Node>, which is our refcounted Rust representation
@@ -83,8 +82,9 @@ impl<'a> CascadedValues<'a> {
     /// This is for the `<use>` element, which draws the element which it references with the
     /// `<use>`'s own cascade, not wih the element's original cascade.
     pub fn new_from_values(node: &'a Node, values: &ComputedValues) -> CascadedValues<'a> {
+        let state = node.state.borrow();
         let mut v = values.clone();
-        node.get_specified_values().to_computed_values(&mut v);
+        state.get_specified_values().to_computed_values(&mut v);
 
         CascadedValues {
             inner: CascadedInner::FromValues(v),
@@ -168,7 +168,7 @@ pub struct Node {
     last_child: RefCell<Option<Weak<Node>>>,
     next_sib: RefCell<Option<Rc<Node>>>, // next sibling; strong ref
     prev_sib: RefCell<Option<Weak<Node>>>, // previous sibling; weak ref
-    state: *mut RsvgState,
+    state: RefCell<State>,
     result: RefCell<NodeResult>,
     transform: Cell<Matrix>,
     values: RefCell<ComputedValues>,
@@ -245,7 +245,6 @@ impl Node {
         parent: Option<Weak<Node>>,
         id: Option<&str>,
         class: Option<&str>,
-        state: *mut RsvgState,
         node_impl: Box<NodeTrait>,
     ) -> Node {
         Node {
@@ -257,7 +256,7 @@ impl Node {
             last_child: RefCell::new(None),
             next_sib: RefCell::new(None),
             prev_sib: RefCell::new(None),
-            state,
+            state: RefCell::new(State::new()),
             transform: Cell::new(Matrix::identity()),
             result: RefCell::new(Ok(())),
             values: RefCell::new(ComputedValues::default()),
@@ -290,21 +289,14 @@ impl Node {
         self.transform.get()
     }
 
-    pub fn get_state_mut(&self) -> &mut State {
-        state::from_c_mut(self.state)
-    }
-
-    pub fn get_specified_values(&self) -> &SpecifiedValues {
-        state::from_c(self.state).get_specified_values()
-    }
-
     pub fn get_cascaded_values(&self) -> CascadedValues {
         CascadedValues::new_from_node(self)
     }
 
     pub fn cascade(&self, values: &ComputedValues) {
+        let state = self.state.borrow();
         let mut values = values.clone();
-        self.get_specified_values().to_computed_values(&mut values);
+        state.get_specified_values().to_computed_values(&mut values);
         *self.values.borrow_mut() = values.clone();
 
         for child in self.children() {
@@ -419,8 +411,118 @@ impl Node {
         Ok(())
     }
 
+    // Sets the node's state from the attributes in the pbag.  Also applies
+    // CSS rules in our limited way based on the node's tag/class/id.
+    pub fn parse_style_attributes(&self, handle: *const RsvgHandle, tag: &str, pbag: &PropertyBag) {
+        {
+            let mut state = self.state.borrow_mut();
+            match state.parse_presentation_attributes(pbag) {
+                Ok(_) => (),
+                Err(e) => {
+                    // FIXME: we'll ignore errors here for now.
+                    // If we return, we expose buggy handling of the enable-background
+                    // property; we are not parsing it correctly. This causes
+                    // tests/fixtures/reftests/bugs/587721-text-transform.svg to fail
+                    // because it has enable-background="new 0 0 1179.75118 687.74173"
+                    // in the toplevel svg element.
+                    //
+                    //   self.set_error(e);
+                    //   return;
+
+                    rsvg_log!("(attribute error: {})", e);
+                }
+            }
+        }
+
+        // Try to properly support all of the following, including inheritance:
+        // *
+        // #id
+        // tag
+        // tag#id
+        // tag.class
+        // tag.class#id
+        //
+        // This is basically a semi-compliant CSS2 selection engine
+
+        unsafe {
+            let state_ptr = self.state.as_ptr() as *mut RsvgState;
+
+            // *
+            rsvg_lookup_apply_css_style(handle, "*".to_glib_none().0, state_ptr);
+
+            // tag
+            rsvg_lookup_apply_css_style(handle, tag.to_glib_none().0, state_ptr);
+
+            if let Some(klazz) = self.get_class() {
+                for cls in klazz.split_whitespace() {
+                    let mut found = false;
+
+                    if !cls.is_empty() {
+                        // tag.class#id
+                        if let Some(id) = self.get_id() {
+                            let target = format!("{}.{}#{}", tag, cls, id);
+                            found = found || from_glib(rsvg_lookup_apply_css_style(
+                                handle,
+                                target.to_glib_none().0,
+                                state_ptr,
+                            ));
+                        }
+
+                        // .class#id
+                        if let Some(id) = self.get_id() {
+                            let target = format!(".{}#{}", cls, id);
+                            found = found || from_glib(rsvg_lookup_apply_css_style(
+                                handle,
+                                target.to_glib_none().0,
+                                state_ptr,
+                            ));
+                        }
+
+                        // tag.class
+                        let target = format!("{}.{}", tag, cls);
+                        found = found || from_glib(rsvg_lookup_apply_css_style(
+                            handle,
+                            target.to_glib_none().0,
+                            state_ptr,
+                        ));
+
+                        if !found {
+                            // didn't find anything more specific, just apply the class style
+                            let target = format!(".{}", cls);
+                            rsvg_lookup_apply_css_style(handle, target.to_glib_none().0, state_ptr);
+                        }
+                    }
+                }
+            }
+
+            if let Some(id) = self.get_id() {
+                // id
+                let target = format!("#{}", id);
+                rsvg_lookup_apply_css_style(handle, target.to_glib_none().0, state_ptr);
+
+                // tag#id
+                let target = format!("{}#{}", tag, id);
+                rsvg_lookup_apply_css_style(handle, target.to_glib_none().0, state_ptr);
+            }
+
+            for (_key, attr, value) in pbag.iter() {
+                match attr {
+                    Attribute::Style => {
+                        let mut state = self.state.borrow_mut();
+                        if let Err(e) = state.parse_style_declarations(value) {
+                            self.set_error(e);
+                            break;
+                        }
+                    }
+
+                    _ => (),
+                }
+            }
+        }
+    }
+
     pub fn set_overridden_properties(&self) {
-        let mut state = self.get_state_mut();
+        let mut state = self.state.borrow_mut();
         self.node_impl.set_overridden_properties(&mut state);
     }
 
@@ -512,8 +614,13 @@ impl Node {
         self.first_child.borrow().is_some()
     }
 
+    pub fn is_overflow(&self) -> bool {
+        let state = self.state.borrow();
+        state.get_specified_values().is_overflow()
+    }
+
     pub fn set_overflow_hidden(&self) {
-        let state = self.get_state_mut();
+        let mut state = self.state.borrow_mut();
         state.values.overflow = SpecifiedValue::Specified(Overflow::Hidden);
     }
 
@@ -542,22 +649,6 @@ impl Node {
     }
 }
 
-// Sigh, rsvg_state_free() is only available if we are being linked into
-// librsvg.so.  In testing mode, we run standalone, so we omit this.
-// Fortunately, in testing mode we don't create "real" nodes with
-// states; we only create stub nodes with ptr::null() for state.
-#[cfg(not(test))]
-impl Drop for Node {
-    fn drop(&mut self) {
-        extern "C" {
-            fn rsvg_state_free(state: *mut RsvgState);
-        }
-        unsafe {
-            rsvg_state_free(self.state);
-        }
-    }
-}
-
 pub fn node_ptr_to_weak(raw_parent: *const RsvgNode) -> Option<Weak<Node>> {
     if raw_parent.is_null() {
         None
@@ -579,7 +670,6 @@ pub fn boxed_node_new(
         node_ptr_to_weak(raw_parent),
         id,
         class,
-        rsvg_state_new(),
         node_impl,
     )))
 }
@@ -743,8 +833,8 @@ pub extern "C" fn rsvg_node_children_iter_next(
 mod tests {
     use super::*;
     use handle::RsvgHandle;
+    use std::mem;
     use std::rc::Rc;
-    use std::{mem, ptr};
 
     struct TestNodeImpl {}
 
@@ -761,7 +851,6 @@ mod tests {
             None,
             None,
             None,
-            ptr::null_mut(),
             Box::new(TestNodeImpl {}),
         ));
 
@@ -787,7 +876,6 @@ mod tests {
             None,
             None,
             None,
-            ptr::null_mut(),
             Box::new(TestNodeImpl {}),
         ));
 
@@ -810,7 +898,6 @@ mod tests {
             None,
             None,
             None,
-            ptr::null_mut(),
             Box::new(TestNodeImpl {}),
         ));
 
@@ -824,7 +911,6 @@ mod tests {
             None,
             None,
             None,
-            ptr::null_mut(),
             Box::new(TestNodeImpl {}),
         ));
 
@@ -833,7 +919,6 @@ mod tests {
             Some(Rc::downgrade(&node)),
             None,
             None,
-            ptr::null_mut(),
             Box::new(TestNodeImpl {}),
         ));
 
@@ -850,7 +935,6 @@ mod tests {
             None,
             None,
             None,
-            ptr::null_mut(),
             Box::new(TestNodeImpl {}),
         ));
 
@@ -859,7 +943,6 @@ mod tests {
             Some(Rc::downgrade(&node)),
             None,
             None,
-            ptr::null_mut(),
             Box::new(TestNodeImpl {}),
         ));
 
@@ -868,7 +951,6 @@ mod tests {
             Some(Rc::downgrade(&node)),
             None,
             None,
-            ptr::null_mut(),
             Box::new(TestNodeImpl {}),
         ));
 
@@ -898,7 +980,6 @@ mod tests {
             None,
             None,
             None,
-            ptr::null_mut(),
             Box::new(TestNodeImpl {}),
         ));
 
@@ -907,7 +988,6 @@ mod tests {
             Some(Rc::downgrade(&node)),
             None,
             None,
-            ptr::null_mut(),
             Box::new(TestNodeImpl {}),
         ));
 
@@ -916,7 +996,6 @@ mod tests {
             Some(Rc::downgrade(&node)),
             None,
             None,
-            ptr::null_mut(),
             Box::new(TestNodeImpl {}),
         ));
 
diff --git a/rsvg_internals/src/state.rs b/rsvg_internals/src/state.rs
index e48055f8..93071e7c 100644
--- a/rsvg_internals/src/state.rs
+++ b/rsvg_internals/src/state.rs
@@ -9,10 +9,8 @@ use std::str::FromStr;
 use attributes::Attribute;
 use error::*;
 use font_props::{FontSizeSpec, FontWeightSpec, LetterSpacingSpec, SingleFontFamily};
-use handle::RsvgHandle;
 use iri::IRI;
 use length::{Dasharray, Length, LengthDir, LengthUnit};
-use node::RsvgNode;
 use paint_server::PaintServer;
 use parsers::{Parse, ParseError};
 use property_bag::PropertyBag;
@@ -346,7 +344,7 @@ impl SpecifiedValues {
 }
 
 impl State {
-    fn new() -> State {
+    pub fn new() -> State {
         State {
             values: Default::default(),
             important_styles: Default::default(),
@@ -606,7 +604,7 @@ impl State {
         Ok(())
     }
 
-    fn parse_presentation_attributes(&mut self, pbag: &PropertyBag) -> Result<(), NodeError> {
+    pub fn parse_presentation_attributes(&mut self, pbag: &PropertyBag) -> Result<(), NodeError> {
         for (_key, attr, value) in pbag.iter() {
             self.parse_style_pair(attr, value, false, false)?;
         }
@@ -614,7 +612,7 @@ impl State {
         Ok(())
     }
 
-    fn parse_style_declarations(&mut self, declarations: &str) -> Result<(), NodeError> {
+    pub fn parse_style_declarations(&mut self, declarations: &str) -> Result<(), NodeError> {
         // Split an attribute value like style="foo: bar; baz: beep;" into
         // individual CSS declarations ("foo: bar" and "baz: beep") and
         // set them onto the state struct.
@@ -1434,38 +1432,6 @@ make_property!(
     "preserve" => Preserve,
 );
 
-pub fn from_c<'a>(state: *const RsvgState) -> &'a State {
-    assert!(!state.is_null());
-
-    unsafe { &*(state as *const State) }
-}
-
-pub fn from_c_mut<'a>(state: *mut RsvgState) -> &'a mut State {
-    assert!(!state.is_null());
-
-    unsafe { &mut *(state as *mut State) }
-}
-
-pub fn to_c_mut(state: &mut State) -> *mut RsvgState {
-    state as *mut State as *mut RsvgState
-}
-
-// Rust State API for consumption from C ----------------------------------------
-
-#[no_mangle]
-pub extern "C" fn rsvg_state_new() -> *mut RsvgState {
-    Box::into_raw(Box::new(State::new())) as *mut RsvgState
-}
-
-#[no_mangle]
-pub extern "C" fn rsvg_state_free(state: *mut RsvgState) {
-    let state = from_c_mut(state);
-
-    unsafe {
-        Box::from_raw(state);
-    }
-}
-
 #[no_mangle]
 pub extern "C" fn rsvg_state_parse_style_pair(
     state: *mut RsvgState,
@@ -1474,10 +1440,10 @@ pub extern "C" fn rsvg_state_parse_style_pair(
     important: glib_sys::gboolean,
     accept_shorthands: glib_sys::gboolean,
 ) -> glib_sys::gboolean {
-    let state = from_c_mut(state);
+    assert!(!state.is_null());
+    let state = unsafe { &mut *(state as *mut State) };
 
     assert!(!value.is_null());
-
     let value = unsafe { utf8_cstr(value) };
 
     match state.parse_style_pair(
@@ -1490,127 +1456,3 @@ pub extern "C" fn rsvg_state_parse_style_pair(
         Err(_) => false.to_glib(),
     }
 }
-
-extern "C" {
-    fn rsvg_lookup_apply_css_style(
-        handle: *const RsvgHandle,
-        target: *const libc::c_char,
-        state: *mut RsvgState,
-    ) -> glib_sys::gboolean;
-}
-
-// Sets the node's state from the attributes in the pbag.  Also
-// applies CSS rules in our limited way based on the node's
-// tag/klazz/id.
-pub fn parse_style_attrs(
-    handle: *const RsvgHandle,
-    node: &RsvgNode,
-    tag: &str,
-    pbag: &PropertyBag,
-) {
-    let state = node.get_state_mut();
-
-    match state.parse_presentation_attributes(pbag) {
-        Ok(_) => (),
-        Err(e) => {
-            // FIXME: we'll ignore errors here for now.  If we return, we expose
-            // buggy handling of the enable-background property; we are not parsing it correctly.
-            // This causes tests/fixtures/reftests/bugs/587721-text-transform.svg to fail
-            // because it has enable-background="new 0 0 1179.75118 687.74173" in the toplevel svg
-            // element.
-            //        {
-            //            node.set_error(e);
-            //            return;
-            //        }
-
-            rsvg_log!("(attribute error: {})", e);
-        }
-    }
-
-    // Try to properly support all of the following, including inheritance:
-    // *
-    // #id
-    // tag
-    // tag#id
-    // tag.class
-    // tag.class#id
-    //
-    // This is basically a semi-compliant CSS2 selection engine
-
-    unsafe {
-        // *
-        rsvg_lookup_apply_css_style(handle, "*".to_glib_none().0, to_c_mut(state));
-
-        // tag
-        rsvg_lookup_apply_css_style(handle, tag.to_glib_none().0, to_c_mut(state));
-
-        if let Some(klazz) = node.get_class() {
-            for cls in klazz.split_whitespace() {
-                let mut found = false;
-
-                if !cls.is_empty() {
-                    // tag.class#id
-                    if let Some(id) = node.get_id() {
-                        let target = format!("{}.{}#{}", tag, cls, id);
-                        found = found || from_glib(rsvg_lookup_apply_css_style(
-                            handle,
-                            target.to_glib_none().0,
-                            to_c_mut(state),
-                        ));
-                    }
-
-                    // .class#id
-                    if let Some(id) = node.get_id() {
-                        let target = format!(".{}#{}", cls, id);
-                        found = found || from_glib(rsvg_lookup_apply_css_style(
-                            handle,
-                            target.to_glib_none().0,
-                            to_c_mut(state),
-                        ));
-                    }
-
-                    // tag.class
-                    let target = format!("{}.{}", tag, cls);
-                    found = found || from_glib(rsvg_lookup_apply_css_style(
-                        handle,
-                        target.to_glib_none().0,
-                        to_c_mut(state),
-                    ));
-
-                    if !found {
-                        // didn't find anything more specific, just apply the class style
-                        let target = format!(".{}", cls);
-                        rsvg_lookup_apply_css_style(
-                            handle,
-                            target.to_glib_none().0,
-                            to_c_mut(state),
-                        );
-                    }
-                }
-            }
-        }
-
-        if let Some(id) = node.get_id() {
-            // id
-            let target = format!("#{}", id);
-            rsvg_lookup_apply_css_style(handle, target.to_glib_none().0, to_c_mut(state));
-
-            // tag#id
-            let target = format!("{}#{}", tag, id);
-            rsvg_lookup_apply_css_style(handle, target.to_glib_none().0, to_c_mut(state));
-        }
-
-        for (_key, attr, value) in pbag.iter() {
-            match attr {
-                Attribute::Style => {
-                    if let Err(e) = state.parse_style_declarations(value) {
-                        node.set_error(e);
-                        break;
-                    }
-                }
-
-                _ => (),
-            }
-        }
-    }
-}
diff --git a/rsvg_internals/src/structure.rs b/rsvg_internals/src/structure.rs
index 64935e0a..4f04f753 100644
--- a/rsvg_internals/src/structure.rs
+++ b/rsvg_internals/src/structure.rs
@@ -15,7 +15,7 @@ use libc;
 use node::*;
 use parsers::{parse, parse_and_validate, Parse};
 use property_bag::{OwnedPropertyBag, PropertyBag};
-use state::{self, Overflow};
+use state::Overflow;
 use viewbox::*;
 use viewport::{draw_in_viewport, ClipMode};
 
@@ -116,10 +116,10 @@ impl NodeSvg {
         }
     }
 
-    pub fn parse_style_attrs(&self, node: &RsvgNode, handle: *const RsvgHandle) {
+    pub fn parse_style_attributes(&self, node: &RsvgNode, handle: *const RsvgHandle) {
         if let Some(owned_pbag) = self.pbag.borrow().as_ref() {
             let pbag = PropertyBag::from_owned(owned_pbag);
-            state::parse_style_attrs(handle, node, "svg", &pbag);
+            node.parse_style_attributes(handle, "svg", &pbag);
         }
     }
 }
@@ -341,8 +341,7 @@ impl NodeTrait for NodeUse {
         } else {
             child.with_impl(|symbol: &NodeSymbol| {
                 let do_clip = !values.is_overflow()
-                    || (values.overflow == Overflow::Visible
-                        && child.get_specified_values().is_overflow());
+                    || (values.overflow == Overflow::Visible && child.is_overflow());
 
                 draw_in_viewport(
                     nx,


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