[librsvg: 2/3] node: Use a RefCell for the State
- From: Federico Mena Quintero <federico src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [librsvg: 2/3] node: Use a RefCell for the State
- Date: Sun, 2 Sep 2018 23:51:40 +0000 (UTC)
commit 569137ed1fcda1271ecec6ebf84ef404a60eefb6
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 8327ea6c..43e0bcdc 100644
--- a/rsvg_internals/src/load.rs
+++ b/rsvg_internals/src/load.rs
@@ -34,7 +34,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};
@@ -350,7 +349,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();
@@ -369,6 +368,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 f12d5080..8dfe8e0e 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 5f79f3bc..9d5efde3 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);
}
}
}
@@ -340,8 +340,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]
