[librsvg] document: rework acquire()
- From: Paolo Borelli <pborelli src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [librsvg] document: rework acquire()
- Date: Wed, 8 Apr 2020 06:29:33 +0000 (UTC)
commit 42b621465dfecbff28131bcd62b593ff74cdfce4
Author: Paolo Borelli <pborelli gnome org>
Date: Mon Apr 6 11:08:01 2020 +0200
document: rework acquire()
Do not take the element types as param, instead handle in every
caller whether the type of the acquired node is the expected one.
This also allows to simplify a couple of places, since for instance
for gradients we were already matching by element type in the
caller.
rsvg_internals/src/document.rs | 57 ++++-------
rsvg_internals/src/drawing_ctx.rs | 195 +++++++++++++++++++-----------------
rsvg_internals/src/filters/image.rs | 2 +-
rsvg_internals/src/gradient.rs | 31 ++----
rsvg_internals/src/marker.rs | 31 +++---
rsvg_internals/src/pattern.rs | 22 ++--
rsvg_internals/src/text.rs | 2 +-
7 files changed, 165 insertions(+), 175 deletions(-)
---
diff --git a/rsvg_internals/src/document.rs b/rsvg_internals/src/document.rs
index 55c32ba2..b4ea99a1 100644
--- a/rsvg_internals/src/document.rs
+++ b/rsvg_internals/src/document.rs
@@ -289,50 +289,13 @@ impl<'i> AcquiredNodes<'i> {
}
}
- fn lookup_node(
- &self,
- fragment: &Fragment,
- element_types: &[ElementType],
- ) -> Result<Node, AcquireError> {
- let node = self.document.lookup(fragment).map_err(|_| {
- // FIXME: callers shouldn't have to know that get_node() can initiate a file load.
- // Maybe we should have the following stages:
- // - load main SVG XML
- //
- // - load secondary SVG XML and other files like images; all document::Resources and
- // document::Images loaded
- //
- // - Now that all files are loaded, resolve URL references
- AcquireError::LinkNotFound(fragment.clone())
- })?;
-
- if element_types.is_empty() {
- Ok(node)
- } else if node.is_element() {
- let element_type = node.borrow_element().get_type();
- if element_types.iter().find(|&&t| t == element_type).is_some() {
- Ok(node)
- } else {
- Err(AcquireError::InvalidLinkType(fragment.clone()))
- }
- } else {
- Err(AcquireError::InvalidLinkType(fragment.clone()))
- }
- }
-
pub fn lookup_image(&self, href: &str) -> Result<SharedImageSurface, LoadingError> {
self.document.lookup_image(href)
}
/// Acquires a node.
- /// Specify `element_types` when expecting the node to be of a particular type,
- /// or use an empty slice for `element_types` if you want a node of any type.
/// Nodes acquired by this function must be released in reverse acquiring order.
- pub fn acquire(
- &mut self,
- fragment: &Fragment,
- element_types: &[ElementType],
- ) -> Result<AcquiredNode, AcquireError> {
+ pub fn acquire(&mut self, fragment: &Fragment) -> Result<AcquiredNode, AcquireError> {
self.num_elements_acquired += 1;
// This is a mitigation for SVG files that try to instance a huge number of
@@ -341,9 +304,23 @@ impl<'i> AcquiredNodes<'i> {
return Err(AcquireError::MaxReferencesExceeded);
}
- let node = self.lookup_node(fragment, element_types)?;
+ let node = self.document.lookup(fragment).map_err(|_| {
+ // FIXME: callers shouldn't have to know that get_node() can initiate a file load.
+ // Maybe we should have the following stages:
+ // - load main SVG XML
+ //
+ // - load secondary SVG XML and other files like images; all document::Resources and
+ // document::Images loaded
+ //
+ // - Now that all files are loaded, resolve URL references
+ AcquireError::LinkNotFound(fragment.clone())
+ })?;
+
+ if !node.is_element() {
+ return Err(AcquireError::InvalidLinkType(fragment.clone()));
+ }
- if node.is_element() && node.borrow_element().is_accessed_by_reference() {
+ if node.borrow_element().is_accessed_by_reference() {
self.acquire_ref(&node)
} else {
Ok(AcquiredNode {
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index 4470c469..2ea3965a 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -12,7 +12,7 @@ use crate::aspect_ratio::AspectRatio;
use crate::bbox::BoundingBox;
use crate::coord_units::CoordUnits;
use crate::dasharray::Dasharray;
-use crate::document::{AcquiredNode, AcquiredNodes};
+use crate::document::AcquiredNodes;
use crate::dpi::Dpi;
use crate::element::ElementType;
use crate::error::{AcquireError, RenderingError};
@@ -577,27 +577,37 @@ impl DrawingCtx {
// Mask
if let Some(fragment) = mask {
- if let Ok(acquired) = acquired_nodes.acquire(fragment, &[ElementType::Mask])
- {
+ if let Ok(acquired) = acquired_nodes.acquire(fragment) {
let mask_node = acquired.get();
- res = res.and_then(|bbox| {
- dc.generate_cairo_mask(
- &mask_node.borrow_element().get_impl::<Mask>(),
- &mask_node,
- affines.for_temporary_surface,
- &bbox,
- acquired_nodes,
- )
- .and_then(|mask_surf| {
- if let Some(surf) = mask_surf {
- dc.cr.set_matrix(affines.compositing.into());
- dc.cr.mask_surface(&surf, 0.0, 0.0);
- }
- Ok(())
- })
- .map(|_: ()| bbox)
- });
+ match mask_node.borrow_element().get_type() {
+ ElementType::Mask => {
+ res = res.and_then(|bbox| {
+ dc.generate_cairo_mask(
+ &mask_node.borrow_element().get_impl::<Mask>(),
+ &mask_node,
+ affines.for_temporary_surface,
+ &bbox,
+ acquired_nodes,
+ )
+ .and_then(|mask_surf| {
+ if let Some(surf) = mask_surf {
+ dc.cr.set_matrix(affines.compositing.into());
+ dc.cr.mask_surface(&surf, 0.0, 0.0);
+ }
+ Ok(())
+ })
+ .map(|_: ()| bbox)
+ });
+ }
+ _ => {
+ rsvg_log!(
+ "element {} references \"{}\" which is not a mask",
+ node,
+ fragment
+ );
+ }
+ }
} else {
rsvg_log!(
"element {} references nonexistent mask \"{}\"",
@@ -745,41 +755,51 @@ impl DrawingCtx {
child_surface: SharedImageSurface,
node_bbox: BoundingBox,
) -> Result<SharedImageSurface, RenderingError> {
- match acquired_nodes.acquire(filter_uri, &[ElementType::Filter]) {
+ match acquired_nodes.acquire(filter_uri) {
Ok(acquired) => {
let filter_node = acquired.get();
+ let filter_elt = filter_node.borrow_element();
- if !filter_node.borrow_element().is_in_error() {
- // FIXME: deal with out of memory here
- filters::render(
- &filter_node,
- values,
- child_surface,
- acquired_nodes,
- self,
- node_bbox,
- )
- } else {
- Ok(child_surface)
+ match filter_elt.get_type() {
+ ElementType::Filter => {
+ if filter_elt.is_in_error() {
+ return Ok(child_surface);
+ }
+
+ // FIXME: deal with out of memory here
+ return filters::render(
+ &filter_node,
+ values,
+ child_surface,
+ acquired_nodes,
+ self,
+ node_bbox,
+ );
+ }
+ _ => {
+ rsvg_log!(
+ "element {} will not be rendered since \"{}\" is not a filter",
+ node,
+ filter_uri,
+ );
+ }
}
}
-
- Err(_) => {
+ _ => {
rsvg_log!(
"element {} will not be rendered since its filter \"{}\" was not found",
node,
filter_uri,
);
-
- // Non-existing filters must act as null filters (that is, an
- // empty surface is returned).
- Ok(SharedImageSurface::empty(
- child_surface.width(),
- child_surface.height(),
- child_surface.surface_type(),
- )?)
}
}
+
+ // Non-existing filters must act as null filters (an empty surface is returned).
+ Ok(SharedImageSurface::empty(
+ child_surface.width(),
+ child_surface.height(),
+ child_surface.surface_type(),
+ )?)
}
fn set_color(
@@ -817,7 +837,7 @@ impl DrawingCtx {
} => {
let mut had_paint_server = false;
- match acquire_paint_server(acquired_nodes, iri) {
+ match acquired_nodes.acquire(iri) {
Ok(acquired) => {
let node = acquired.get();
@@ -854,7 +874,7 @@ impl DrawingCtx {
opacity,
bbox,
)?,
- _ => unreachable!(),
+ _ => false,
}
}
@@ -1115,9 +1135,6 @@ impl DrawingCtx {
cascaded: &CascadedValues<'_>,
clipping: bool,
) -> Result<BoundingBox, RenderingError> {
- let elt = node.borrow_element();
- let use_ = elt.get_impl::<Use>();
-
// <use> is an element that is used directly, unlike
// <pattern>, which is used through a fill="url(#...)"
// reference. However, <use> will always reference another
@@ -1134,12 +1151,15 @@ impl DrawingCtx {
}
})?;
+ let elt = node.borrow_element();
+ let use_ = elt.get_impl::<Use>();
let link = use_.get_link();
+
if link.is_none() {
return Ok(self.empty_bbox());
}
- let acquired = match acquired_nodes.acquire(link.unwrap(), &[]) {
+ let acquired = match acquired_nodes.acquire(link.unwrap()) {
Ok(acquired) => acquired,
Err(AcquireError::CircularReference(node)) => {
@@ -1171,6 +1191,7 @@ impl DrawingCtx {
let child = acquired.get();
+ // if it is a symbol
if child.is_element() && child.borrow_element().get_type() == ElementType::Symbol {
let elt = child.borrow_element();
let symbol = elt.get_impl::<Symbol>();
@@ -1184,34 +1205,41 @@ impl DrawingCtx {
None
};
- self.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |an, dc| {
- let _params = dc.push_new_viewport(
- symbol.get_viewbox(),
- use_rect,
- symbol.get_preserve_aspect_ratio(),
- clip_mode,
- );
+ return self.with_discrete_layer(
+ node,
+ acquired_nodes,
+ values,
+ clipping,
+ &mut |an, dc| {
+ let _params = dc.push_new_viewport(
+ symbol.get_viewbox(),
+ use_rect,
+ symbol.get_preserve_aspect_ratio(),
+ clip_mode,
+ );
- child.draw_children(
- an,
- &CascadedValues::new_from_values(&child, values),
- dc,
- clipping,
- )
- })
- } else {
- let cr = self.get_cairo_context();
- cr.translate(use_rect.x0, use_rect.y0);
-
- self.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |an, dc| {
- dc.draw_node_from_stack(
- &child,
- an,
- &CascadedValues::new_from_values(&child, values),
- clipping,
- )
- })
+ child.draw_children(
+ an,
+ &CascadedValues::new_from_values(&child, values),
+ dc,
+ clipping,
+ )
+ },
+ );
}
+
+ // all other nodes
+ let cr = self.get_cairo_context();
+ cr.translate(use_rect.x0, use_rect.y0);
+
+ self.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |an, dc| {
+ dc.draw_node_from_stack(
+ &child,
+ an,
+ &CascadedValues::new_from_values(&child, values),
+ clipping,
+ )
+ })
}
}
@@ -1272,8 +1300,9 @@ fn get_clip_in_user_and_object_space(
clip_uri
.and_then(|fragment| {
acquired_nodes
- .acquire(fragment, &[ElementType::ClipPath])
+ .acquire(fragment)
.ok()
+ .filter(|a| a.get().borrow_element().get_type() == ElementType::ClipPath)
})
.and_then(|acquired| {
let clip_node = acquired.get().clone();
@@ -1291,20 +1320,6 @@ fn get_clip_in_user_and_object_space(
.unwrap_or((None, None))
}
-fn acquire_paint_server(
- acquired_nodes: &mut AcquiredNodes,
- fragment: &Fragment,
-) -> Result<AcquiredNode, AcquireError> {
- acquired_nodes.acquire(
- fragment,
- &[
- ElementType::LinearGradient,
- ElementType::RadialGradient,
- ElementType::Pattern,
- ],
- )
-}
-
fn compute_stroke_and_fill_box(cr: &cairo::Context, values: &ComputedValues) -> BoundingBox {
let affine = Transform::from(cr.get_matrix());
diff --git a/rsvg_internals/src/filters/image.rs b/rsvg_internals/src/filters/image.rs
index b03cfa71..4bb4d960 100644
--- a/rsvg_internals/src/filters/image.rs
+++ b/rsvg_internals/src/filters/image.rs
@@ -45,7 +45,7 @@ impl FeImage {
fragment: &Fragment,
) -> Result<FilterResult, FilterError> {
let acquired_drawable = acquired_nodes
- .acquire(fragment, &[])
+ .acquire(fragment)
.map_err(|_| FilterError::InvalidInput)?;
let drawable = acquired_drawable.get();
diff --git a/rsvg_internals/src/gradient.rs b/rsvg_internals/src/gradient.rs
index 1d176e34..42f3728f 100644
--- a/rsvg_internals/src/gradient.rs
+++ b/rsvg_internals/src/gradient.rs
@@ -9,7 +9,7 @@ use std::cell::RefCell;
use crate::allowed_url::Fragment;
use crate::bbox::*;
use crate::coord_units::CoordUnits;
-use crate::document::{AcquiredNode, AcquiredNodes, NodeStack};
+use crate::document::{AcquiredNodes, NodeStack};
use crate::drawing_ctx::{DrawingCtx, ViewParams};
use crate::element::{ElementResult, ElementTrait, ElementType};
use crate::error::*;
@@ -687,24 +687,22 @@ macro_rules! impl_paint_source {
while !gradient.is_resolved() {
if let Some(fragment) = fallback {
- let acquired = acquire_gradient(acquired_nodes, &fragment)?;
+ let acquired = acquired_nodes.acquire(&fragment)?;
let acquired_node = acquired.get();
if stack.contains(acquired_node) {
return Err(AcquireError::CircularReference(acquired_node.clone()));
}
- let borrowed_node = acquired_node.borrow_element();
- let unresolved = match borrowed_node.get_type() {
- ElementType::$gradient_type => {
- let a_gradient = borrowed_node.get_impl::<$gradient_type>();
- a_gradient.get_unresolved(&acquired_node)
- }
+ let elt = acquired_node.borrow_element();
+ let unresolved = match acquired_node.borrow_element().get_type() {
+ ElementType::$gradient_type => elt
+ .get_impl::<$gradient_type>()
+ .get_unresolved(&acquired_node),
ElementType::$other_type => {
- let a_gradient = borrowed_node.get_impl::<$other_type>();
- a_gradient.get_unresolved(&acquired_node)
+ elt.get_impl::<$other_type>().get_unresolved(&acquired_node)
}
- _ => unreachable!(),
+ _ => return Err(AcquireError::InvalidLinkType(fragment.clone())),
};
gradient = gradient.resolve_from_fallback(&unresolved.gradient);
@@ -816,17 +814,6 @@ impl Gradient {
}
}
-/// Acquires a node of linearGradient or radialGradient type
-fn acquire_gradient(
- acquired_nodes: &mut AcquiredNodes,
- fragment: &Fragment,
-) -> Result<AcquiredNode, AcquireError> {
- acquired_nodes.acquire(
- fragment,
- &[ElementType::LinearGradient, ElementType::RadialGradient],
- )
-}
-
#[cfg(test)]
mod tests {
use super::*;
diff --git a/rsvg_internals/src/marker.rs b/rsvg_internals/src/marker.rs
index 0ed54e0a..6d641b3d 100644
--- a/rsvg_internals/src/marker.rs
+++ b/rsvg_internals/src/marker.rs
@@ -569,19 +569,26 @@ fn emit_marker_by_name(
line_width: f64,
clipping: bool,
) -> Result<BoundingBox, RenderingError> {
- if let Ok(acquired) = acquired_nodes.acquire(name, &[ElementType::Marker]) {
+ if let Ok(acquired) = acquired_nodes.acquire(name) {
let node = acquired.get();
-
- node.borrow_element().get_impl::<Marker>().render(
- &node,
- acquired_nodes,
- draw_ctx,
- xpos,
- ypos,
- computed_angle,
- line_width,
- clipping,
- )
+ let elt = node.borrow_element();
+
+ match elt.get_type() {
+ ElementType::Marker => elt.get_impl::<Marker>().render(
+ &node,
+ acquired_nodes,
+ draw_ctx,
+ xpos,
+ ypos,
+ computed_angle,
+ line_width,
+ clipping,
+ ),
+ _ => {
+ rsvg_log!("\"{}\" is not marker", name);
+ Ok(draw_ctx.empty_bbox())
+ }
+ }
} else {
rsvg_log!("marker \"{}\" not found", name);
Ok(draw_ctx.empty_bbox())
diff --git a/rsvg_internals/src/pattern.rs b/rsvg_internals/src/pattern.rs
index 9f521224..9e878aa6 100644
--- a/rsvg_internals/src/pattern.rs
+++ b/rsvg_internals/src/pattern.rs
@@ -177,7 +177,7 @@ impl PaintSource for Pattern {
while !pattern.is_resolved() {
if let Some(ref fragment) = fallback {
- match acquired_nodes.acquire(&fragment, &[ElementType::Pattern]) {
+ match acquired_nodes.acquire(&fragment) {
Ok(acquired) => {
let acquired_node = acquired.get();
@@ -185,14 +185,18 @@ impl PaintSource for Pattern {
return Err(AcquireError::CircularReference(acquired_node.clone()));
}
- let borrowed_node = acquired_node.borrow_element();
- let borrowed_pattern = borrowed_node.get_impl::<Pattern>();
- let unresolved = borrowed_pattern.get_unresolved(&acquired_node);
-
- pattern = pattern.resolve_from_fallback(&unresolved.pattern);
- fallback = unresolved.fallback;
-
- stack.push(acquired_node);
+ let elt = acquired_node.borrow_element();
+ match elt.get_type() {
+ ElementType::Pattern => {
+ let unresolved =
+ elt.get_impl::<Pattern>().get_unresolved(&acquired_node);
+ pattern = pattern.resolve_from_fallback(&unresolved.pattern);
+ fallback = unresolved.fallback;
+
+ stack.push(acquired_node);
+ }
+ _ => return Err(AcquireError::InvalidLinkType(fragment.clone())),
+ }
}
Err(AcquireError::MaxReferencesExceeded) => {
diff --git a/rsvg_internals/src/text.rs b/rsvg_internals/src/text.rs
index 9122f88d..5fb9fe4f 100644
--- a/rsvg_internals/src/text.rs
+++ b/rsvg_internals/src/text.rs
@@ -693,7 +693,7 @@ impl TRef {
let link = self.link.as_ref().unwrap();
let values = cascaded.get();
- if let Ok(acquired) = acquired_nodes.acquire(link, &[]) {
+ if let Ok(acquired) = acquired_nodes.acquire(link) {
let c = acquired.get();
extract_chars_children_to_chunks_recursively(chunks, &c, values, depth);
} else {
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
