[librsvg] AcquiredNode.get(): Return a reference, not a cloned Rc<>
- From: Federico Mena Quintero <federico src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [librsvg] AcquiredNode.get(): Return a reference, not a cloned Rc<>
- Date: Wed, 19 Sep 2018 16:42:40 +0000 (UTC)
commit 0e3cfd8d9ddb80ee3da161b3c43b7e3dfab66366
Author: Federico Mena Quintero <federico gnome org>
Date: Wed Sep 19 10:37:47 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 | 154 +++++++++++++++++++-------------------
4 files changed, 105 insertions(+), 121 deletions(-)
---
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index 0b4ab218..805baa08 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -363,31 +363,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();
@@ -477,13 +470,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();
@@ -1118,8 +1108,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 37a60749..a42fe4fe 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 1b89a59c..4bdd27e5 100644
--- a/rsvg_internals/src/length.rs
+++ b/rsvg_internals/src/length.rs
@@ -243,9 +243,7 @@ fn font_size_from_values(values: &ComputedValues, params: &ViewParams) -> f64 {
LengthUnit::Inch => font_size_from_inch(v.length, v.dir, params),
- 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 430b8473..5fd36201 100644
--- a/rsvg_internals/src/structure.rs
+++ b/rsvg_internals/src/structure.rs
@@ -293,88 +293,86 @@ 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 params = draw_ctx.get_view_params();
-
- let nx = self.x.get().normalize(values, ¶ms);
- let ny = self.y.get().normalize(values, ¶ms);
+ 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, ¶ms);
- let nh = self
- .h
- .get()
- .unwrap_or_else(|| Length::parse_str("100%", LengthDir::Vertical).unwrap())
- .normalize(values, ¶ms);
-
- // 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 params = draw_ctx.get_view_params();
+
+ let nx = self.x.get().normalize(values, ¶ms);
+ let ny = self.y.get().normalize(values, ¶ms);
+
+ // 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, ¶ms);
+ let nh = self
+ .h
+ .get()
+ .unwrap_or_else(|| Length::parse_str("100%", LengthDir::Vertical).unwrap())
+ .normalize(values, ¶ms);
+
+ // 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.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.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(())
}
}
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
