[librsvg: 1/2] (#759) - Replace SavedCr again - use a with_saved_cr() function
- From: Marge Bot <marge-bot src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [librsvg: 1/2] (#759) - Replace SavedCr again - use a with_saved_cr() function
- Date: Thu, 1 Jul 2021 18:20:39 +0000 (UTC)
commit 2df1d968388e04b1b8bcabda30cc4967f972b63c
Author: Federico Mena Quintero <federico gnome org>
Date: Tue Jun 29 20:54:46 2021 -0500
(#759) - Replace SavedCr again - use a with_saved_cr() function
Sigh, it was so nice to cr.restore() on Drop of the SavedCr, but since
restore() can now return an error, this is no longer practicable. So,
we go back to the scheme of using a helper function - just so we can
use the ? operator easily in the callbacks.
Fixes https://gitlab.gnome.org/GNOME/librsvg/-/issues/759
Part-of: <https://gitlab.gnome.org/GNOME/librsvg/-/merge_requests/559>
src/drawing_ctx.rs | 440 ++++++++++++++++++++++++++---------------------------
src/handle.rs | 70 +++++----
2 files changed, 254 insertions(+), 256 deletions(-)
---
diff --git a/src/drawing_ctx.rs b/src/drawing_ctx.rs
index 97c63fc5..9e7fa2f6 100644
--- a/src/drawing_ctx.rs
+++ b/src/drawing_ctx.rs
@@ -247,20 +247,18 @@ pub fn draw_tree(
Ok(user_bbox)
}
-/// Does `cairo_save()` on creation, and will `cairo_restore()` on `Drop`.
-pub struct SavedCr(cairo::Context);
-
-impl SavedCr {
- pub fn new(cr: &cairo::Context) -> Result<SavedCr, cairo::Error> {
- cr.save()?;
- Ok(SavedCr(cr.clone()))
- }
-}
+pub fn with_saved_cr<O, F>(cr: &cairo::Context, f: F) -> Result<O, RenderingError>
+where
+ F: FnOnce() -> Result<O, RenderingError>,
+{
+ cr.save()?;
+ match f() {
+ Ok(o) => {
+ cr.restore()?;
+ Ok(o)
+ }
-impl Drop for SavedCr {
- fn drop(&mut self) {
- // Ignore cairo errors :(
- let _ = self.0.restore();
+ Err(e) => Err(e),
}
}
@@ -459,7 +457,7 @@ impl DrawingCtx {
/// Note that this actually changes the `draw_ctx.cr`'s transformation to match
/// the new coordinate space, but the old one is not restored after the
/// result's `ViewParams` is dropped. Thus, this function must be called
- /// inside `SavedCr` scope or `draw_ctx.with_discrete_layer`.
+ /// inside `with_saved_cr` or `draw_ctx.with_discrete_layer`.
pub fn push_new_viewport(
&self,
vbox: Option<ViewBox>,
@@ -677,185 +675,187 @@ impl DrawingCtx {
let res = if clipping {
draw_fn(acquired_nodes, self)
} else {
- let _saved_cr = SavedCr::new(&self.cr)?;
+ with_saved_cr(&self.cr.clone(), || {
+ let Opacity(UnitInterval(opacity)) = stacking_ctx.opacity;
- let Opacity(UnitInterval(opacity)) = stacking_ctx.opacity;
+ let affine_at_start = self.get_transform();
- let affine_at_start = self.get_transform();
-
- if let Some(rect) = clip_rect {
- clip_to_rectangle(&self.cr, &rect);
- }
-
- // Here we are clipping in user space, so the bbox doesn't matter
- self.clip_to_node(
- &stacking_ctx.clip_in_user_space,
- acquired_nodes,
- &self.empty_bbox(),
- )?;
-
- let is_opaque = approx_eq!(f64, opacity, 1.0);
- let needs_temporary_surface = !(is_opaque
- && stacking_ctx.filter == Filter::None
- && stacking_ctx.mask.is_none()
- && stacking_ctx.mix_blend_mode == MixBlendMode::Normal
- && stacking_ctx.clip_in_object_space.is_none());
+ if let Some(rect) = clip_rect {
+ clip_to_rectangle(&self.cr, &rect);
+ }
- if needs_temporary_surface {
- // Compute our assortment of affines
+ // Here we are clipping in user space, so the bbox doesn't matter
+ self.clip_to_node(
+ &stacking_ctx.clip_in_user_space,
+ acquired_nodes,
+ &self.empty_bbox(),
+ )?;
- let affines = CompositingAffines::new(
- affine_at_start,
- self.initial_transform_with_offset(),
- self.cr_stack.borrow().len(),
- );
+ let is_opaque = approx_eq!(f64, opacity, 1.0);
+ let needs_temporary_surface = !(is_opaque
+ && stacking_ctx.filter == Filter::None
+ && stacking_ctx.mask.is_none()
+ && stacking_ctx.mix_blend_mode == MixBlendMode::Normal
+ && stacking_ctx.clip_in_object_space.is_none());
- // Create temporary surface and its cr
+ if needs_temporary_surface {
+ // Compute our assortment of affines
- let cr = match stacking_ctx.filter {
- Filter::None => cairo::Context::new(
- &self.create_similar_surface_for_toplevel_viewport(&self.cr.target())?,
- )?,
- Filter::List(_) => {
- cairo::Context::new(&*self.create_surface_for_toplevel_viewport()?)?
- }
- };
+ let affines = CompositingAffines::new(
+ affine_at_start,
+ self.initial_transform_with_offset(),
+ self.cr_stack.borrow().len(),
+ );
- cr.set_matrix(affines.for_temporary_surface.into());
+ // Create temporary surface and its cr
- let (source_surface, mut res, bbox) = {
- let mut temporary_draw_ctx = self.nested(cr);
+ let cr = match stacking_ctx.filter {
+ Filter::None => cairo::Context::new(
+ &self
+ .create_similar_surface_for_toplevel_viewport(&self.cr.target())?,
+ )?,
+ Filter::List(_) => {
+ cairo::Context::new(&*self.create_surface_for_toplevel_viewport()?)?
+ }
+ };
- // Draw!
+ cr.set_matrix(affines.for_temporary_surface.into());
+
+ let (source_surface, mut res, bbox) = {
+ let mut temporary_draw_ctx = self.nested(cr);
+
+ // Draw!
+
+ let res = draw_fn(acquired_nodes, &mut temporary_draw_ctx);
+
+ let bbox = if let Ok(ref bbox) = res {
+ *bbox
+ } else {
+ BoundingBox::new().with_transform(affines.for_temporary_surface)
+ };
+
+ if let Filter::List(ref filter_list) = stacking_ctx.filter {
+ let surface_to_filter = SharedImageSurface::copy_from_surface(
+ &cairo::ImageSurface::try_from(temporary_draw_ctx.cr.target())
+ .unwrap(),
+ )?;
+
+ let current_color = values.color().0;
+
+ let params = temporary_draw_ctx.get_view_params();
+
+ // TODO: the stroke/fill paint are already resolved for shapes. Outside of
shapes,
+ // they are also needed for filters in all elements. Maybe we should make them
part
+ // of the StackingContext instead of Shape?
+ let stroke_paint_source = Rc::new(
+ values
+ .stroke()
+ .0
+ .resolve(
+ acquired_nodes,
+ values.stroke_opacity().0,
+ current_color,
+ None,
+ None,
+ )
+ .to_user_space(&bbox, ¶ms, values),
+ );
+
+ let fill_paint_source = Rc::new(
+ values
+ .fill()
+ .0
+ .resolve(
+ acquired_nodes,
+ values.fill_opacity().0,
+ current_color,
+ None,
+ None,
+ )
+ .to_user_space(&bbox, ¶ms, values),
+ );
+
+ // Filter functions (like "blend()", not the <filter> element) require
+ // being resolved in userSpaceonUse units, since that is the default
+ // for primitive_units. So, get the corresponding NormalizeParams
+ // here and pass them down.
+ let user_space_params = NormalizeParams::new(
+ values,
+ ¶ms.with_units(CoordUnits::UserSpaceOnUse),
+ );
+
+ let filtered_surface = temporary_draw_ctx
+ .run_filters(
+ surface_to_filter,
+ filter_list,
+ acquired_nodes,
+ &stacking_ctx.element_name,
+ &user_space_params,
+ stroke_paint_source,
+ fill_paint_source,
+ current_color,
+ bbox,
+ )?
+ .into_image_surface()?;
- let res = draw_fn(acquired_nodes, &mut temporary_draw_ctx);
+ let generic_surface: &cairo::Surface = &filtered_surface; // deref to Surface
- let bbox = if let Ok(ref bbox) = res {
- *bbox
- } else {
- BoundingBox::new().with_transform(affines.for_temporary_surface)
+ (generic_surface.clone(), res, bbox)
+ } else {
+ (temporary_draw_ctx.cr.target(), res, bbox)
+ }
};
- if let Filter::List(ref filter_list) = stacking_ctx.filter {
- let surface_to_filter = SharedImageSurface::copy_from_surface(
- &cairo::ImageSurface::try_from(temporary_draw_ctx.cr.target()).unwrap(),
- )?;
+ // Set temporary surface as source
- let current_color = values.color().0;
+ self.cr.set_matrix(affines.compositing.into());
+ self.cr.set_source_surface(&source_surface, 0.0, 0.0)?;
- let params = temporary_draw_ctx.get_view_params();
+ // Clip
- // TODO: the stroke/fill paint are already resolved for shapes. Outside of shapes,
- // they are also needed for filters in all elements. Maybe we should make them part
- // of the StackingContext instead of Shape?
- let stroke_paint_source = Rc::new(
- values
- .stroke()
- .0
- .resolve(
- acquired_nodes,
- values.stroke_opacity().0,
- current_color,
- None,
- None,
- )
- .to_user_space(&bbox, ¶ms, values),
- );
-
- let fill_paint_source = Rc::new(
- values
- .fill()
- .0
- .resolve(
- acquired_nodes,
- values.fill_opacity().0,
- current_color,
- None,
- None,
- )
- .to_user_space(&bbox, ¶ms, values),
- );
+ self.cr.set_matrix(affines.outside_temporary_surface.into());
+ self.clip_to_node(&stacking_ctx.clip_in_object_space, acquired_nodes, &bbox)?;
- // Filter functions (like "blend()", not the <filter> element) require
- // being resolved in userSpaceonUse units, since that is the default
- // for primitive_units. So, get the corresponding NormalizeParams
- // here and pass them down.
- let user_space_params = NormalizeParams::new(
- values,
- ¶ms.with_units(CoordUnits::UserSpaceOnUse),
- );
+ // Mask
- let filtered_surface = temporary_draw_ctx
- .run_filters(
- surface_to_filter,
- filter_list,
+ if let Some(ref mask_node) = stacking_ctx.mask {
+ res = res.and_then(|bbox| {
+ self.generate_cairo_mask(
+ mask_node,
+ affines.for_temporary_surface,
+ &bbox,
acquired_nodes,
- &stacking_ctx.element_name,
- &user_space_params,
- stroke_paint_source,
- fill_paint_source,
- current_color,
- bbox,
- )?
- .into_image_surface()?;
-
- let generic_surface: &cairo::Surface = &filtered_surface; // deref to Surface
-
- (generic_surface.clone(), res, bbox)
+ )
+ .and_then(|mask_surf| {
+ if let Some(surf) = mask_surf {
+ self.cr.set_matrix(affines.compositing.into());
+ Ok(self.cr.mask_surface(&surf, 0.0, 0.0)?)
+ } else {
+ Ok(())
+ }
+ })
+ .map(|_: ()| bbox)
+ });
} else {
- (temporary_draw_ctx.cr.target(), res, bbox)
- }
- };
-
- // Set temporary surface as source
-
- self.cr.set_matrix(affines.compositing.into());
- self.cr.set_source_surface(&source_surface, 0.0, 0.0)?;
-
- // Clip
-
- self.cr.set_matrix(affines.outside_temporary_surface.into());
- self.clip_to_node(&stacking_ctx.clip_in_object_space, acquired_nodes, &bbox)?;
-
- // Mask
-
- if let Some(ref mask_node) = stacking_ctx.mask {
- res = res.and_then(|bbox| {
- self.generate_cairo_mask(
- mask_node,
- affines.for_temporary_surface,
- &bbox,
- acquired_nodes,
- )
- .and_then(|mask_surf| {
- if let Some(surf) = mask_surf {
- self.cr.set_matrix(affines.compositing.into());
- Ok(self.cr.mask_surface(&surf, 0.0, 0.0)?)
- } else {
- Ok(())
- }
- })
- .map(|_: ()| bbox)
- });
- } else {
- // No mask, so composite the temporary surface
+ // No mask, so composite the temporary surface
- self.cr.set_matrix(affines.compositing.into());
- self.cr.set_operator(stacking_ctx.mix_blend_mode.into());
+ self.cr.set_matrix(affines.compositing.into());
+ self.cr.set_operator(stacking_ctx.mix_blend_mode.into());
- if opacity < 1.0 {
- self.cr.paint_with_alpha(opacity)?;
- } else {
- self.cr.paint()?;
+ if opacity < 1.0 {
+ self.cr.paint_with_alpha(opacity)?;
+ } else {
+ self.cr.paint()?;
+ }
}
- }
- self.cr.set_matrix(affine_at_start.into());
+ self.cr.set_matrix(affine_at_start.into());
- res
- } else {
- draw_fn(acquired_nodes, self)
- }
+ res
+ } else {
+ draw_fn(acquired_nodes, self)
+ }
+ })
};
self.cr.set_matrix(orig_transform.into());
@@ -1324,15 +1324,15 @@ impl DrawingCtx {
clipping,
None,
&mut |_an, dc| {
- let _saved_cr = SavedCr::new(&dc.cr)?;
-
- if let Some(_params) =
- dc.push_new_viewport(Some(vbox), image.rect, image.aspect, clip_mode)
- {
- dc.paint_surface(&image.surface, image_width, image_height)?;
- }
+ with_saved_cr(&dc.cr.clone(), || {
+ if let Some(_params) =
+ dc.push_new_viewport(Some(vbox), image.rect, image.aspect, clip_mode)
+ {
+ dc.paint_surface(&image.surface, image_width, image_height)?;
+ }
- Ok(bounds)
+ Ok(bounds)
+ })
},
)
} else {
@@ -1359,62 +1359,62 @@ impl DrawingCtx {
let mut bbox = bbox.unwrap();
- let _saved_cr = SavedCr::new(&self.cr)?;
-
- self.cr
- .set_antialias(cairo::Antialias::from(span.text_rendering));
+ with_saved_cr(&self.cr.clone(), || {
+ self.cr
+ .set_antialias(cairo::Antialias::from(span.text_rendering));
- setup_cr_for_stroke(&self.cr, &span.stroke);
+ setup_cr_for_stroke(&self.cr, &span.stroke);
- self.cr.move_to(span.x, span.y);
+ self.cr.move_to(span.x, span.y);
- let rotation = gravity.to_rotation();
- if !rotation.approx_eq_cairo(0.0) {
- self.cr.rotate(-rotation);
- }
-
- if clipping {
- pangocairo::functions::update_layout(&self.cr, &span.layout);
- pangocairo::functions::layout_path(&self.cr, &span.layout);
- return Ok(self.empty_bbox());
- }
+ let rotation = gravity.to_rotation();
+ if !rotation.approx_eq_cairo(0.0) {
+ self.cr.rotate(-rotation);
+ }
- // Fill
+ if clipping {
+ pangocairo::functions::update_layout(&self.cr, &span.layout);
+ pangocairo::functions::layout_path(&self.cr, &span.layout);
+ return Ok(self.empty_bbox());
+ }
- if span.is_visible {
- let fill_paint = span.fill_paint.to_user_space(&bbox, &view_params, values);
+ // Fill
- self.set_paint_source(&fill_paint, acquired_nodes)
- .map(|had_paint_server| {
- if had_paint_server {
- pangocairo::functions::update_layout(&self.cr, &span.layout);
- pangocairo::functions::show_layout(&self.cr, &span.layout);
- };
- })?;
- }
+ if span.is_visible {
+ let fill_paint = span.fill_paint.to_user_space(&bbox, &view_params, values);
+
+ self.set_paint_source(&fill_paint, acquired_nodes)
+ .map(|had_paint_server| {
+ if had_paint_server {
+ pangocairo::functions::update_layout(&self.cr, &span.layout);
+ pangocairo::functions::show_layout(&self.cr, &span.layout);
+ };
+ })?;
+ }
- // Stroke
+ // Stroke
- let stroke_paint = span.stroke_paint.to_user_space(&bbox, &view_params, values);
+ let stroke_paint = span.stroke_paint.to_user_space(&bbox, &view_params, values);
- let had_paint_server = self.set_paint_source(&stroke_paint, acquired_nodes)?;
+ let had_paint_server = self.set_paint_source(&stroke_paint, acquired_nodes)?;
- if had_paint_server {
- pangocairo::functions::update_layout(&self.cr, &span.layout);
- pangocairo::functions::layout_path(&self.cr, &span.layout);
-
- let (x0, y0, x1, y1) = self.cr.stroke_extents()?;
- let r = Rect::new(x0, y0, x1, y1);
- let ib = BoundingBox::new()
- .with_transform(transform)
- .with_ink_rect(r);
- bbox.insert(&ib);
- if span.is_visible {
- self.cr.stroke()?;
+ if had_paint_server {
+ pangocairo::functions::update_layout(&self.cr, &span.layout);
+ pangocairo::functions::layout_path(&self.cr, &span.layout);
+
+ let (x0, y0, x1, y1) = self.cr.stroke_extents()?;
+ let r = Rect::new(x0, y0, x1, y1);
+ let ib = BoundingBox::new()
+ .with_transform(transform)
+ .with_ink_rect(r);
+ bbox.insert(&ib);
+ if span.is_visible {
+ self.cr.stroke()?;
+ }
}
- }
- Ok(bbox)
+ Ok(bbox)
+ })
}
pub fn get_snapshot(
diff --git a/src/handle.rs b/src/handle.rs
index db15281c..7cfb23da 100644
--- a/src/handle.rs
+++ b/src/handle.rs
@@ -7,7 +7,7 @@ use crate::bbox::BoundingBox;
use crate::css::{Origin, Stylesheet};
use crate::document::{AcquiredNodes, Document, NodeId};
use crate::dpi::Dpi;
-use crate::drawing_ctx::{draw_tree, DrawingMode, SavedCr, ViewParams};
+use crate::drawing_ctx::{draw_tree, with_saved_cr, DrawingMode, ViewParams};
use crate::error::{DefsLookupErrorKind, LoadingError, RenderingError};
use crate::length::*;
use crate::node::{CascadedValues, Node, NodeBorrow};
@@ -253,20 +253,19 @@ impl Handle {
let viewport = Rect::from(*viewport);
- let _saved_cr = SavedCr::new(&cr)?;
-
- let res = draw_tree(
- DrawingMode::LimitToStack { node, root },
- cr,
- viewport,
- user_language,
- dpi,
- false,
- is_testing,
- &mut AcquiredNodes::new(&self.document),
- );
-
- res.map(|_bbox| ())
+ with_saved_cr(cr, || {
+ draw_tree(
+ DrawingMode::LimitToStack { node, root },
+ cr,
+ viewport,
+ user_language,
+ dpi,
+ false,
+ is_testing,
+ &mut AcquiredNodes::new(&self.document),
+ )
+ .map(|_bbox| ())
+ })
}
fn get_bbox_for_element(
@@ -345,27 +344,26 @@ impl Handle {
// Render, transforming so element is at the new viewport's origin
- let _saved_cr = SavedCr::new(&cr)?;
-
- let factor =
- (element_viewport.width / ink_r.width()).min(element_viewport.height / ink_r.height());
-
- cr.translate(element_viewport.x, element_viewport.y);
- cr.scale(factor, factor);
- cr.translate(-ink_r.x0, -ink_r.y0);
-
- let res = draw_tree(
- DrawingMode::OnlyNode(node),
- &cr,
- unit_rectangle(),
- user_language,
- dpi,
- false,
- is_testing,
- &mut AcquiredNodes::new(&self.document),
- );
-
- res.map(|_bbox| ())
+ with_saved_cr(cr, || {
+ let factor = (element_viewport.width / ink_r.width())
+ .min(element_viewport.height / ink_r.height());
+
+ cr.translate(element_viewport.x, element_viewport.y);
+ cr.scale(factor, factor);
+ cr.translate(-ink_r.x0, -ink_r.y0);
+
+ draw_tree(
+ DrawingMode::OnlyNode(node),
+ &cr,
+ unit_rectangle(),
+ user_language,
+ dpi,
+ false,
+ is_testing,
+ &mut AcquiredNodes::new(&self.document),
+ )
+ .map(|_bbox| ())
+ })
}
pub fn get_intrinsic_dimensions(&self) -> IntrinsicDimensions {
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
