[librsvg: 7/8] Turn RsvgNode into a newtype on Rc<Node<NodeData>>
- From: Federico Mena Quintero <federico src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [librsvg: 7/8] Turn RsvgNode into a newtype on Rc<Node<NodeData>>
- Date: Sat, 11 May 2019 00:00:22 +0000 (UTC)
commit 10321f2a223267c4adcfdb1d26e0977bb469b192
Author: Federico Mena Quintero <federico gnome org>
Date: Fri May 10 18:58:07 2019 -0500
Turn RsvgNode into a newtype on Rc<Node<NodeData>>
Previously this was a type alias,
pub type RsvgNode = Rc<Node>;
But now we have a newtype
pub type RsvgNode = NodeRef<NodeData>; // in node.rs
pub struct NodeRef<T>(pub Rc<Node<T>>); // in tree_utils/mod.rs
The newtype similar to "NodeRef" in the Kuchiki crate. Later we will
implement the selectors::Element trait on that RsvgNode newtype;
whereas beforehand, we wouldn't be able to implement a trait on a
plain Rc<> because of the orphan rule.
rsvg_internals/src/create_node.rs | 2 +-
rsvg_internals/src/drawing_ctx.rs | 6 +--
rsvg_internals/src/filters/mod.rs | 34 +++++++++------
rsvg_internals/src/handle.rs | 2 +-
rsvg_internals/src/node.rs | 82 +++++++++++++++++++++---------------
rsvg_internals/src/pattern.rs | 7 ++-
rsvg_internals/src/structure.rs | 1 +
rsvg_internals/src/tree_utils/mod.rs | 68 ++++++++++++++++++++++--------
rsvg_internals/src/xml.rs | 15 +++----
9 files changed, 136 insertions(+), 81 deletions(-)
---
diff --git a/rsvg_internals/src/create_node.rs b/rsvg_internals/src/create_node.rs
index cfcb1e0d..10a1724a 100644
--- a/rsvg_internals/src/create_node.rs
+++ b/rsvg_internals/src/create_node.rs
@@ -37,7 +37,7 @@ use crate::text::{NodeTRef, NodeTSpan, NodeText};
macro_rules! node_create_fn {
($name:ident, $node_type:ident, $new_fn:expr) => {
fn $name(id: Option<&str>, class: Option<&str>, parent: Option<&RsvgNode>) -> RsvgNode {
- node_new(NodeType::$node_type, parent, id, class, Box::new($new_fn()))
+ RsvgNode::new(NodeType::$node_type, parent, id, class, Box::new($new_fn()))
}
};
}
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index 41dbc274..35f53fd9 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -824,7 +824,7 @@ impl DrawingCtx {
let stack_top = self.drawsub_stack.pop();
let draw = if let Some(ref top) = stack_top {
- Rc::ptr_eq(top, node)
+ top == node
} else {
true
};
@@ -1067,7 +1067,7 @@ impl Drop for AcquiredNode {
fn drop(&mut self) {
let mut stack = self.0.borrow_mut();
let last = stack.pop().unwrap();
- assert!(Rc::ptr_eq(&last, &self.1));
+ assert!(last == self.1);
}
}
@@ -1163,6 +1163,6 @@ impl NodeStack {
}
pub fn contains(&self, node: &RsvgNode) -> bool {
- self.0.iter().find(|n| Rc::ptr_eq(n, node)).is_some()
+ self.0.iter().find(|n| **n == *node).is_some()
}
}
diff --git a/rsvg_internals/src/filters/mod.rs b/rsvg_internals/src/filters/mod.rs
index 8613c599..ce903ec2 100644
--- a/rsvg_internals/src/filters/mod.rs
+++ b/rsvg_internals/src/filters/mod.rs
@@ -11,12 +11,13 @@ use crate::coord_units::CoordUnits;
use crate::drawing_ctx::DrawingCtx;
use crate::error::{RenderingError, ValueErrorKind};
use crate::length::{LengthHorizontal, LengthUnit, LengthVertical};
-use crate::node::{NodeResult, NodeTrait, NodeType, RsvgNode};
+use crate::node::{NodeData, NodeResult, NodeTrait, NodeType, RsvgNode};
use crate::parsers::{ParseError, ParseValue};
-use crate::property_defs::ColorInterpolationFilters;
-use crate::property_bag::PropertyBag;
use crate::properties::ComputedValues;
+use crate::property_bag::PropertyBag;
+use crate::property_defs::ColorInterpolationFilters;
use crate::surface_utils::shared_surface::{SharedImageSurface, SurfaceType};
+use crate::tree_utils::{Node, NodeRef};
mod bounds;
use self::bounds::BoundsBuilder;
@@ -303,24 +304,27 @@ pub fn render(
(c, linear_rgb)
})
+ // Keep only filter primitives (those that implement the Filter trait)
.filter_map(|(c, linear_rgb)| {
- let rr = RcRef::new(c)
- .try_map(|c| {
+ let rr = RcRef::new(c.0)
+ .try_map(|c: &Node<NodeData>| {
// Go through the filter primitives and see if the node is one of them.
#[inline]
fn as_filter<T: Filter>(x: &T) -> &Filter {
x
}
- // Unfortunately it's not possible to downcast to a trait object. If we could
- // attach arbitrary data to nodes it would really help here. Previously that
- // arbitrary data was in form of RsvgCNodeImpl, but that was heavily tuned for
- // storing C pointers with all subsequent downsides.
+ // Unfortunately it's not possible to downcast to a trait object. So, we
+ // try downcasting to each filter type individually, and use
+ // owning_ref::RcRef to reduce the children to a list of
+ // OwningRef<Rc<Node<NodeData>>, dyn Filter> -- which is essentially the
+ // NodeRef, and something inside the NodeRef that implements the Filter
+ // trait.
macro_rules! try_downcasting_to_filter {
($c:expr; $($t:ty),+$(,)*) => ({
let mut filter = None;
$(
- filter = filter.or_else(|| $c.get_impl::<$t>().map(as_filter));
+ filter = filter.or_else(|| $c.borrow().get_impl::<$t>().map(as_filter));
)+
filter
})
@@ -351,14 +355,18 @@ pub fn render(
});
for (rr, linear_rgb) in primitives {
+ // rr: OwningRef<Rc<Node<NodeData>>, dyn Filter>
+
+ let rr_node = NodeRef(rr.as_owner().clone());
+
let mut render = |filter_ctx: &mut FilterContext| {
if let Err(err) = rr
- .render(rr.as_owner(), filter_ctx, draw_ctx)
+ .render(&rr_node, filter_ctx, draw_ctx)
.and_then(|result| filter_ctx.store_result(result))
{
rsvg_log!(
"(filter primitive {} returned an error: {})",
- rr.as_owner().get_human_readable_name(),
+ rr_node.get_human_readable_name(),
err
);
@@ -382,7 +390,7 @@ pub fn render(
let elapsed = start.elapsed();
rsvg_log!(
"(rendered filter primitive {} in\n {} seconds)",
- rr.as_owner().get_human_readable_name(),
+ rr_node.get_human_readable_name(),
elapsed.as_secs() as f64 + f64::from(elapsed.subsec_nanos()) / 1e9
);
}
diff --git a/rsvg_internals/src/handle.rs b/rsvg_internals/src/handle.rs
index c32c2a9a..36b8dc01 100644
--- a/rsvg_internals/src/handle.rs
+++ b/rsvg_internals/src/handle.rs
@@ -233,7 +233,7 @@ impl Handle {
let node = self.get_node_or_root(id)?;
let root = self.svg.root();
- let is_root = Rc::ptr_eq(&node, &root);
+ let is_root = node == root;
if is_root {
let cascaded = node.get_cascaded_values();
diff --git a/rsvg_internals/src/node.rs b/rsvg_internals/src/node.rs
index 38f666e8..04e98e60 100644
--- a/rsvg_internals/src/node.rs
+++ b/rsvg_internals/src/node.rs
@@ -13,12 +13,14 @@ use crate::parsers::Parse;
use crate::properties::{ComputedValues, SpecifiedValue, SpecifiedValues};
use crate::property_bag::PropertyBag;
use crate::property_defs::Overflow;
-use crate::tree_utils;
+use crate::tree_utils::{self, NodeRef, NodeWeakRef};
use locale_config::Locale;
-/// A strong reference to a node
-pub type RsvgNode = Rc<Node>;
+/// Tree node with specific data
+pub type RsvgNode = NodeRef<NodeData>;
+pub type RsvgWeakNode = NodeWeakRef<NodeData>;
+/// Contents of a tree node
pub struct NodeData {
node_type: NodeType,
id: Option<String>, // id attribute from XML element
@@ -33,7 +35,45 @@ pub struct NodeData {
style_attr: RefCell<String>,
}
-pub type Node = tree_utils::Node<NodeData>;
+impl NodeRef<NodeData> {
+ pub fn new(
+ node_type: NodeType,
+ parent: Option<&NodeRef<NodeData>>,
+ id: Option<&str>,
+ class: Option<&str>,
+ node_impl: Box<NodeTrait>,
+ ) -> NodeRef<NodeData> {
+ let data = NodeData {
+ node_type,
+ id: id.map(str::to_string),
+ class: class.map(str::to_string),
+ specified_values: RefCell::new(Default::default()),
+ important_styles: Default::default(),
+ transform: Cell::new(Matrix::identity()),
+ result: RefCell::new(Ok(())),
+ values: RefCell::new(ComputedValues::default()),
+ cond: Cell::new(true),
+ node_impl,
+ style_attr: RefCell::new(String::new()),
+ };
+
+ NodeRef(Rc::new(tree_utils::Node::new(data, parent)))
+ }
+
+ pub fn downgrade(&self) -> RsvgWeakNode {
+ Rc::downgrade(&self.0)
+ }
+
+ pub fn upgrade(weak: &RsvgWeakNode) -> Option<NodeRef<NodeData>> {
+ weak.upgrade().map(NodeRef)
+ }
+}
+
+impl NodeData {
+ pub fn get_impl<T: NodeTrait>(&self) -> Option<&T> {
+ (&self.node_impl).downcast_ref::<T>()
+ }
+}
/// Can obtain computed values from a node
///
@@ -60,7 +100,7 @@ impl<'a> CascadedValues<'a> {
/// This is what nodes should normally use to draw their children from their `draw()` method.
/// Nodes that need to override the cascade for their children can use `new_from_values()`
/// instead.
- pub fn new(&self, node: &'a Node) -> CascadedValues<'a> {
+ pub fn new(&self, node: &'a RsvgNode) -> CascadedValues<'a> {
match self.inner {
CascadedInner::FromNode(_) => CascadedValues {
inner: CascadedInner::FromNode(node.borrow().values.borrow()),
@@ -75,7 +115,7 @@ impl<'a> CascadedValues<'a> {
/// This is to be used only in the toplevel drawing function, or in elements like `<marker>`
/// that don't propagate their parent's cascade to their children. All others should use
/// `new()` to derive the cascade from an existing one.
- fn new_from_node(node: &Node) -> CascadedValues<'_> {
+ fn new_from_node(node: &RsvgNode) -> CascadedValues<'_> {
CascadedValues {
inner: CascadedInner::FromNode(node.borrow().values.borrow()),
}
@@ -86,7 +126,7 @@ 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> {
+ pub fn new_from_values(node: &'a RsvgNode, values: &ComputedValues) -> CascadedValues<'a> {
let mut v = values.clone();
node.borrow()
.specified_values
@@ -274,7 +314,7 @@ impl NodeType {
}
}
-impl Node {
+impl RsvgNode {
pub fn get_type(&self) -> NodeType {
self.borrow().node_type
}
@@ -537,7 +577,7 @@ impl Node {
}
pub fn get_impl<T: NodeTrait>(&self) -> Option<&T> {
- (&self.borrow().node_impl).downcast_ref::<T>()
+ self.borrow().get_impl()
}
pub fn draw_children(
@@ -566,27 +606,3 @@ impl Node {
specified_values.overflow = SpecifiedValue::Specified(Overflow::Hidden);
}
}
-
-pub fn node_new(
- node_type: NodeType,
- parent: Option<&RsvgNode>,
- id: Option<&str>,
- class: Option<&str>,
- node_impl: Box<NodeTrait>,
-) -> RsvgNode {
- let data = NodeData {
- node_type,
- id: id.map(str::to_string),
- class: class.map(str::to_string),
- specified_values: RefCell::new(Default::default()),
- important_styles: Default::default(),
- transform: Cell::new(Matrix::identity()),
- result: RefCell::new(Ok(())),
- values: RefCell::new(ComputedValues::default()),
- cond: Cell::new(true),
- node_impl,
- style_attr: RefCell::new(String::new()),
- };
-
- Rc::new(tree_utils::Node::<NodeData>::new(data, parent))
-}
diff --git a/rsvg_internals/src/pattern.rs b/rsvg_internals/src/pattern.rs
index 90077096..54dc9934 100644
--- a/rsvg_internals/src/pattern.rs
+++ b/rsvg_internals/src/pattern.rs
@@ -2,7 +2,6 @@ use cairo;
use cairo::{MatrixTrait, PatternTrait};
use std::cell::RefCell;
use std::f64;
-use std::rc::*;
use crate::allowed_url::Fragment;
use crate::aspect_ratio::*;
@@ -45,7 +44,7 @@ pub struct Pattern {
// Point back to our corresponding node, or to the fallback node which has children.
// If the value is None, it means we are fully resolved and didn't find any children
// among the fallbacks.
- pub node: Option<Weak<Node>>,
+ pub node: Option<RsvgWeakNode>,
}
impl Default for Pattern {
@@ -179,7 +178,7 @@ impl NodeTrait for NodePattern {
let mut p = self.pattern.borrow_mut();
- p.node = Some(Rc::downgrade(node));
+ p.node = Some(node.downgrade());
for (attr, value) in pbag.iter() {
match attr {
@@ -422,7 +421,7 @@ impl PaintSource for NodePattern {
// Set up transformations to be determined by the contents units
// Draw everything
- let pattern_node = pattern.node.clone().unwrap().upgrade().unwrap();
+ let pattern_node = RsvgNode::upgrade(pattern.node.as_ref().unwrap()).unwrap();
let pattern_cascaded = pattern_node.get_cascaded_values();
let pattern_values = pattern_cascaded.get();
diff --git a/rsvg_internals/src/structure.rs b/rsvg_internals/src/structure.rs
index ce45d124..76998774 100644
--- a/rsvg_internals/src/structure.rs
+++ b/rsvg_internals/src/structure.rs
@@ -17,6 +17,7 @@ use crate::properties::ComputedValues;
use crate::property_bag::PropertyBag;
use crate::property_defs::Overflow;
use crate::rect::RectangleExt;
+use crate::tree_utils::Node;
use crate::viewbox::*;
pub struct NodeGroup();
diff --git a/rsvg_internals/src/tree_utils/mod.rs b/rsvg_internals/src/tree_utils/mod.rs
index 66fde4ea..885d11dc 100644
--- a/rsvg_internals/src/tree_utils/mod.rs
+++ b/rsvg_internals/src/tree_utils/mod.rs
@@ -1,9 +1,39 @@
use std::cell::RefCell;
+use std::ops::Deref;
use std::rc::{Rc, Weak};
-pub type NodeRef<T> = Rc<Node<T>>;
+/// A strong reference to a node
+///
+/// These strong references can be compared with `==` as they
+/// implement `PartialEq` and `Eq`; comparison means whether they
+/// point to the same node struct.
+pub struct NodeRef<T>(pub Rc<Node<T>>);
+
pub type NodeWeakRef<T> = Weak<Node<T>>;
+impl<T> Clone for NodeRef<T> {
+ fn clone(&self) -> NodeRef<T> {
+ NodeRef(self.0.clone())
+ }
+}
+
+impl<T> Deref for NodeRef<T> {
+ type Target = Node<T>;
+
+ #[inline]
+ fn deref(&self) -> &Node<T> {
+ &*self.0
+ }
+}
+
+impl<T> Eq for NodeRef<T> {}
+impl<T> PartialEq for NodeRef<T> {
+ #[inline]
+ fn eq(&self, other: &NodeRef<T>) -> bool {
+ Rc::ptr_eq(&self.0, &other.0)
+ }
+}
+
pub struct Node<T> {
parent: Option<NodeWeakRef<T>>,
first_child: RefCell<Option<NodeRef<T>>>,
@@ -16,7 +46,7 @@ pub struct Node<T> {
impl<T> Node<T> {
pub fn new(data: T, parent: Option<&NodeRef<T>>) -> Node<T> {
Node {
- parent: parent.map(Rc::downgrade),
+ parent: parent.map(|n| Rc::downgrade(&n.0)),
first_child: RefCell::new(None),
last_child: RefCell::new(None),
next_sibling: RefCell::new(None),
@@ -28,7 +58,7 @@ impl<T> Node<T> {
pub fn parent(&self) -> Option<NodeRef<T>> {
match self.parent {
None => None,
- Some(ref weak_node) => Some(weak_node.upgrade().unwrap()),
+ Some(ref weak_node) => Some(NodeRef(weak_node.upgrade().unwrap())),
}
}
@@ -42,7 +72,7 @@ impl<T> Node<T> {
pub fn last_child(&self) -> Option<NodeRef<T>> {
match *self.last_child.borrow() {
None => None,
- Some(ref weak_node) => Some(weak_node.upgrade().unwrap()),
+ Some(ref weak_node) => Some(NodeRef(weak_node.upgrade().unwrap())),
}
}
@@ -56,7 +86,7 @@ impl<T> Node<T> {
pub fn previous_sibling(&self) -> Option<NodeRef<T>> {
match *self.previous_sibling.borrow() {
None => None,
- Some(ref weak_node) => Some(weak_node.upgrade().unwrap()),
+ Some(ref weak_node) => Some(NodeRef(weak_node.upgrade().unwrap())),
}
}
@@ -64,7 +94,7 @@ impl<T> Node<T> {
let mut desc = Some(descendant.clone());
while let Some(ref d) = desc.clone() {
- if Rc::ptr_eq(&ancestor, d) {
+ if ancestor == *d {
return true;
}
@@ -78,7 +108,7 @@ impl<T> Node<T> {
assert!(child.next_sibling.borrow().is_none());
assert!(child.previous_sibling.borrow().is_none());
- if let Some(last_child_weak) = self.last_child.replace(Some(Rc::downgrade(child))) {
+ if let Some(last_child_weak) = self.last_child.replace(Some(Rc::downgrade(&child.0))) {
if let Some(last_child) = last_child_weak.upgrade() {
child.previous_sibling.replace(Some(last_child_weak));
last_child.next_sibling.replace(Some(child.clone()));
@@ -93,7 +123,8 @@ impl<T> Node<T> {
.last_child
.borrow()
.as_ref()
- .and_then(|child_weak| child_weak.upgrade());
+ .and_then(|child_weak| child_weak.upgrade())
+ .map(NodeRef);
Children::new(self.first_child.borrow().clone(), last_child)
}
@@ -168,7 +199,7 @@ fn take_if_unique_strong<T>(r: &RefCell<Option<NodeRef<T>>>) -> Option<NodeRef<T
let has_single_ref = match *r {
None => false,
- Some(ref rc) if Rc::strong_count(rc) > 1 => false,
+ Some(ref rc) if Rc::strong_count(&rc.0) > 1 => false,
Some(_) => true,
};
@@ -241,7 +272,8 @@ impl<T> DoubleEndedIterator for Children<T> {
.previous_sibling
.borrow()
.as_ref()
- .and_then(|sib_weak| sib_weak.upgrade());
+ .and_then(|sib_weak| sib_weak.upgrade())
+ .map(NodeRef);
Some(next_back)
})
}
@@ -256,14 +288,14 @@ mod tests {
#[test]
fn node_is_its_own_ancestor() {
- let node = Rc::new(N::new((), None));;
+ let node = NodeRef(Rc::new(N::new((), None)));
assert!(Node::is_ancestor(node.clone(), node.clone()));
}
#[test]
fn node_is_ancestor_of_child() {
- let node = Rc::new(N::new((), None));;
- let child = Rc::new(N::new((), Some(&node)));
+ let node = NodeRef(Rc::new(N::new((), None)));
+ let child = NodeRef(Rc::new(N::new((), Some(&node))));
node.append(&child);
@@ -273,9 +305,9 @@ mod tests {
#[test]
fn node_children_iterator() {
- let node = Rc::new(N::new((), None));;
- let child = Rc::new(N::new((), Some(&node)));
- let second_child = Rc::new(N::new((), Some(&node)));
+ let node = NodeRef(Rc::new(N::new((), None)));
+ let child = NodeRef(Rc::new(N::new((), Some(&node))));
+ let second_child = NodeRef(Rc::new(N::new((), Some(&node))));
node.append(&child);
node.append(&second_child);
@@ -285,12 +317,12 @@ mod tests {
let c = children.next();
assert!(c.is_some());
let c = c.unwrap();
- assert!(Rc::ptr_eq(&c, &child));
+ assert!(c == child);
let c = children.next_back();
assert!(c.is_some());
let c = c.unwrap();
- assert!(Rc::ptr_eq(&c, &second_child));
+ assert!(c == second_child);
assert!(children.next().is_none());
assert!(children.next_back().is_none());
diff --git a/rsvg_internals/src/xml.rs b/rsvg_internals/src/xml.rs
index 927745e1..9a459b0a 100644
--- a/rsvg_internals/src/xml.rs
+++ b/rsvg_internals/src/xml.rs
@@ -4,7 +4,6 @@ use encoding::DecoderTrap;
use glib::IsA;
use libc;
use std::collections::HashMap;
-use std::rc::Rc;
use std::str;
use crate::allowed_url::AllowedUrl;
@@ -14,7 +13,7 @@ use crate::css::CssRules;
use crate::error::LoadingError;
use crate::handle::LoadOptions;
use crate::io::{self, get_input_stream_for_loading};
-use crate::node::{node_new, Node, NodeType, RsvgNode};
+use crate::node::{NodeType, RsvgNode};
use crate::property_bag::PropertyBag;
use crate::style::NodeStyle;
use crate::svg::Svg;
@@ -67,11 +66,11 @@ extern "C" {
/// trait objects. Normally the context refers to a `NodeCreationContext` implementation which is
/// what creates normal graphical elements.
pub struct XmlState {
- tree_root: Option<Rc<Node>>,
+ tree_root: Option<RsvgNode>,
ids: Option<HashMap<String, RsvgNode>>,
css_rules: Option<CssRules>,
context_stack: Vec<Context>,
- current_node: Option<Rc<Node>>,
+ current_node: Option<RsvgNode>,
entities: HashMap<String, XmlEntityPtr>,
@@ -103,7 +102,7 @@ impl XmlState {
}
}
- fn set_root(&mut self, root: &Rc<Node>) {
+ fn set_root(&mut self, root: &RsvgNode) {
if self.tree_root.is_some() {
panic!("The tree root has already been set");
}
@@ -294,7 +293,7 @@ impl XmlState {
{
child
} else {
- let child = node_new(
+ let child = RsvgNode::new(
NodeType::Chars,
Some(node),
None,
@@ -313,10 +312,10 @@ impl XmlState {
fn create_node(
&mut self,
- parent: Option<&Rc<Node>>,
+ parent: Option<&RsvgNode>,
name: &str,
pbag: &PropertyBag,
- ) -> Rc<Node> {
+ ) -> RsvgNode {
let ids = self.ids.as_mut().unwrap();
let new_node = create_node_and_register_id(name, parent, pbag, ids);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]