[librsvg/librsvg-2.50] (#745): Fix mismatched cairo_save/restore when running in inside the Cairo test suite
- From: Federico Mena Quintero <federico src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [librsvg/librsvg-2.50] (#745): Fix mismatched cairo_save/restore when running in inside the Cairo test suite
- Date: Fri, 4 Jun 2021 23:06:02 +0000 (UTC)
commit 13423e8d2ac6c156a3bf14af2c690eecea0a7f71
Author: Federico Mena Quintero <federico gnome org>
Date: Fri Jun 4 17:42:02 2021 -0500
(#745): Fix mismatched cairo_save/restore when running in inside the Cairo test suite
This is a backport of 5281167a - replace DrawingCtx::with_saved_cr()
by a SavedCr object.
Fixes https://gitlab.gnome.org/GNOME/librsvg/-/issues/745
rsvg_internals/src/drawing_ctx.rs | 391 +++++++++++++++++++-------------------
1 file changed, 197 insertions(+), 194 deletions(-)
---
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index e176e691..47a5c3a2 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -194,6 +194,22 @@ pub fn draw_tree(
Ok(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) -> SavedCr {
+ cr.save();
+ SavedCr(cr.clone())
+ }
+}
+
+impl Drop for SavedCr {
+ fn drop(&mut self) {
+ self.0.restore();
+ }
+}
+
impl DrawingCtx {
fn new(
cr: &cairo::Context,
@@ -375,7 +391,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 `draw_ctx.with_saved_cr` or `draw_ctx.with_discrete_layer`.
+ /// inside `SavedCr` scope or `draw_ctx.with_discrete_layer`.
pub fn push_new_viewport(
&self,
vbox: Option<ViewBox>,
@@ -567,149 +583,149 @@ impl DrawingCtx {
if clipping {
draw_fn(acquired_nodes, self)
} else {
- self.with_saved_cr(&mut |dc| {
- let clip_path_value = values.clip_path();
- let mask_value = values.mask();
+ let _saved_cr = SavedCr::new(&self.cr);
- let clip_uri = clip_path_value.0.get();
- let mask = mask_value.0.get();
+ let clip_path_value = values.clip_path();
+ let mask_value = values.mask();
- let filters = if node.is_element() {
- match *node.borrow_element() {
- Element::Mask(_) => Filter::None,
- _ => values.filter(),
- }
- } else {
- values.filter()
- };
+ let clip_uri = clip_path_value.0.get();
+ let mask = mask_value.0.get();
- let UnitInterval(opacity) = values.opacity().0;
+ let filters = if node.is_element() {
+ match *node.borrow_element() {
+ Element::Mask(_) => Filter::None,
+ _ => values.filter(),
+ }
+ } else {
+ values.filter()
+ };
- let affine_at_start = dc.get_transform();
+ let UnitInterval(opacity) = values.opacity().0;
- let (clip_in_user_space, clip_in_object_space) =
- get_clip_in_user_and_object_space(acquired_nodes, clip_uri);
+ let affine_at_start = self.get_transform();
- // Here we are clipping in user space, so the bbox doesn't matter
- dc.clip_to_node(&clip_in_user_space, acquired_nodes, &dc.empty_bbox())?;
+ let (clip_in_user_space, clip_in_object_space) =
+ get_clip_in_user_and_object_space(acquired_nodes, clip_uri);
- let is_opaque = approx_eq!(f64, opacity, 1.0);
- let needs_temporary_surface = !(is_opaque
- && filters == Filter::None
- && mask.is_none()
- && values.mix_blend_mode() == MixBlendMode::Normal
- && clip_in_object_space.is_none());
+ // Here we are clipping in user space, so the bbox doesn't matter
+ self.clip_to_node(&clip_in_user_space, acquired_nodes, &self.empty_bbox())?;
- if needs_temporary_surface {
- // Compute our assortment of affines
+ let is_opaque = approx_eq!(f64, opacity, 1.0);
+ let needs_temporary_surface = !(is_opaque
+ && filters == Filter::None
+ && mask.is_none()
+ && values.mix_blend_mode() == MixBlendMode::Normal
+ && clip_in_object_space.is_none());
- let affines = CompositingAffines::new(
- affine_at_start,
- dc.initial_transform_with_offset(),
- dc.cr_stack.len(),
- );
+ if needs_temporary_surface {
+ // Compute our assortment of affines
- // Create temporary surface and its cr
+ let affines = CompositingAffines::new(
+ affine_at_start,
+ self.initial_transform_with_offset(),
+ self.cr_stack.len(),
+ );
- let cr = match filters {
- Filter::None => cairo::Context::new(
- &dc.create_similar_surface_for_toplevel_viewport(&dc.cr.get_target())?,
- ),
- Filter::List(_) => {
- cairo::Context::new(&*dc.create_surface_for_toplevel_viewport()?)
- }
- };
+ // Create temporary surface and its cr
- cr.set_matrix(affines.for_temporary_surface.into());
+ let cr = match filters {
+ Filter::None => cairo::Context::new(
+ &self.create_similar_surface_for_toplevel_viewport(&self.cr.get_target())?,
+ ),
+ Filter::List(_) => {
+ cairo::Context::new(&*self.create_surface_for_toplevel_viewport()?)
+ }
+ };
- dc.push_cairo_context(cr);
+ cr.set_matrix(affines.for_temporary_surface.into());
- // Draw!
+ self.push_cairo_context(cr);
- let mut res = draw_fn(acquired_nodes, dc);
+ // Draw!
- let bbox = if let Ok(ref bbox) = res {
- *bbox
- } else {
- BoundingBox::new().with_transform(affines.for_temporary_surface)
- };
+ let mut res = draw_fn(acquired_nodes, self);
- // Filter
- let source_surface =
- dc.run_filters(&filters, acquired_nodes, node, values, bbox)?;
+ let bbox = if let Ok(ref bbox) = res {
+ *bbox
+ } else {
+ BoundingBox::new().with_transform(affines.for_temporary_surface)
+ };
+
+ // Filter
+ let source_surface =
+ self.run_filters(&filters, acquired_nodes, node, values, bbox)?;
- dc.pop_cairo_context();
+ self.pop_cairo_context();
- // Set temporary surface as source
+ // Set temporary surface as source
- dc.cr.set_matrix(affines.compositing.into());
- dc.cr.set_source_surface(&source_surface, 0.0, 0.0);
+ self.cr.set_matrix(affines.compositing.into());
+ self.cr.set_source_surface(&source_surface, 0.0, 0.0);
- // Clip
+ // Clip
- dc.cr.set_matrix(affines.outside_temporary_surface.into());
- dc.clip_to_node(&clip_in_object_space, acquired_nodes, &bbox)?;
+ self.cr.set_matrix(affines.outside_temporary_surface.into());
+ self.clip_to_node(&clip_in_object_space, acquired_nodes, &bbox)?;
- // Mask
+ // Mask
- if let Some(fragment) = mask {
- if let Ok(acquired) = acquired_nodes.acquire(fragment) {
- let mask_node = acquired.get();
+ if let Some(fragment) = mask {
+ if let Ok(acquired) = acquired_nodes.acquire(fragment) {
+ let mask_node = acquired.get();
- match *mask_node.borrow_element() {
- Element::Mask(ref m) => {
- res = res.and_then(|bbox| {
- dc.generate_cairo_mask(
- &m,
- &mask_node,
- affines.for_temporary_surface,
- &bbox,
- acquired_nodes,
- )
+ match *mask_node.borrow_element() {
+ Element::Mask(ref m) => {
+ res = res.and_then(|bbox| {
+ self.generate_cairo_mask(
+ &m,
+ &mask_node,
+ affines.for_temporary_surface,
+ &bbox,
+ acquired_nodes,
+ )
.map(|mask_surf| {
if let Some(surf) = mask_surf {
- dc.cr.set_matrix(affines.compositing.into());
- dc.cr.mask_surface(&surf, 0.0, 0.0);
+ self.cr.set_matrix(affines.compositing.into());
+ self.cr.mask_surface(&surf, 0.0, 0.0);
}
})
.map(|_: ()| bbox)
- });
- }
- _ => {
- rsvg_log!(
- "element {} references \"{}\" which is not a mask",
- node,
- fragment
- );
- }
+ });
+ }
+ _ => {
+ rsvg_log!(
+ "element {} references \"{}\" which is not a mask",
+ node,
+ fragment
+ );
}
- } else {
- rsvg_log!(
- "element {} references nonexistent mask \"{}\"",
- node,
- fragment
- );
}
} else {
- // No mask, so composite the temporary surface
+ rsvg_log!(
+ "element {} references nonexistent mask \"{}\"",
+ node,
+ fragment
+ );
+ }
+ } else {
+ // No mask, so composite the temporary surface
- dc.cr.set_matrix(affines.compositing.into());
- dc.cr.set_operator(values.mix_blend_mode().into());
+ self.cr.set_matrix(affines.compositing.into());
+ self.cr.set_operator(values.mix_blend_mode().into());
- if opacity < 1.0 {
- dc.cr.paint_with_alpha(opacity);
- } else {
- dc.cr.paint();
- }
+ if opacity < 1.0 {
+ self.cr.paint_with_alpha(opacity);
+ } else {
+ self.cr.paint();
}
+ }
- dc.cr.set_matrix(affine_at_start.into());
+ self.cr.set_matrix(affine_at_start.into());
- res
- } else {
- draw_fn(acquired_nodes, dc)
- }
- })
+ res
+ } else {
+ draw_fn(acquired_nodes, self)
+ }
}
}
@@ -792,17 +808,6 @@ impl DrawingCtx {
res
}
- /// Saves the current Cairo context, runs the draw_fn, and restores the context
- fn with_saved_cr(
- &mut self,
- draw_fn: &mut dyn FnMut(&mut DrawingCtx) -> Result<BoundingBox, RenderingError>,
- ) -> Result<BoundingBox, RenderingError> {
- self.cr.save();
- let res = draw_fn(self);
- self.cr.restore();
- res
- }
-
/// Wraps the draw_fn in a link to the given target
pub fn with_link_tag(
&mut self,
@@ -1409,31 +1414,31 @@ impl DrawingCtx {
};
self.with_discrete_layer(node, acquired_nodes, values, clipping, &mut |_an, dc| {
- dc.with_saved_cr(&mut |dc| {
- if let Some(_params) = dc.push_new_viewport(Some(vbox), rect, aspect, clip_mode) {
- let cr = dc.cr.clone();
-
- // We need to set extend appropriately, so can't use cr.set_source_surface().
- //
- // If extend is left at its default value (None), then bilinear scaling uses
- // transparency outside of the image producing incorrect results.
- // For example, in svg1.1/filters-blend-01-b.svgthere's a completely
- // opaque 100×1 image of a gradient scaled to 100×98 which ends up
- // transparent almost everywhere without this fix (which it shouldn't).
- let ptn = surface.to_cairo_pattern();
- ptn.set_extend(cairo::Extend::Pad);
- cr.set_source(&ptn);
-
- // Clip is needed due to extend being set to pad.
- clip_to_rectangle(&cr, &Rect::from_size(image_width, image_height));
-
- cr.paint();
- }
+ let _saved_cr = SavedCr::new(&dc.cr);
- // The bounding box for <image> is decided by the values of x, y, w, h
- // and not by the final computed image bounds.
- Ok(dc.empty_bbox().with_rect(rect))
- })
+ if let Some(_params) = dc.push_new_viewport(Some(vbox), rect, aspect, clip_mode) {
+ let cr = dc.cr.clone();
+
+ // We need to set extend appropriately, so can't use cr.set_source_surface().
+ //
+ // If extend is left at its default value (None), then bilinear scaling uses
+ // transparency outside of the image producing incorrect results.
+ // For example, in svg1.1/filters-blend-01-b.svgthere's a completely
+ // opaque 100×1 image of a gradient scaled to 100×98 which ends up
+ // transparent almost everywhere without this fix (which it shouldn't).
+ let ptn = surface.to_cairo_pattern();
+ ptn.set_extend(cairo::Extend::Pad);
+ cr.set_source(&ptn);
+
+ // Clip is needed due to extend being set to pad.
+ clip_to_rectangle(&cr, &Rect::from_size(image_width, image_height));
+
+ cr.paint();
+ }
+
+ // The bounding box for <image> is decided by the values of x, y, w, h
+ // and not by the final computed image bounds.
+ Ok(dc.empty_bbox().with_rect(rect))
})
}
@@ -1461,78 +1466,76 @@ impl DrawingCtx {
bbox.unwrap()
};
- self.with_saved_cr(&mut |dc| {
- let cr = dc.cr.clone();
+ let _saved_cr = SavedCr::new(&self.cr);
- cr.set_antialias(cairo::Antialias::from(values.text_rendering()));
- dc.setup_cr_for_stroke(&cr, &values);
- cr.move_to(x, y);
+ self.cr.set_antialias(cairo::Antialias::from(values.text_rendering()));
+ self.setup_cr_for_stroke(&self.cr, &values);
+ self.cr.move_to(x, y);
- let rotation = gravity.to_rotation();
- if !rotation.approx_eq_cairo(0.0) {
- cr.rotate(-rotation);
- }
+ let rotation = gravity.to_rotation();
+ if !rotation.approx_eq_cairo(0.0) {
+ self.cr.rotate(-rotation);
+ }
- let current_color = values.color().0;
+ let current_color = values.color().0;
+
+ let res = if !clipping {
+ self.set_source_paint_server(
+ acquired_nodes,
+ &values.fill().0,
+ values.fill_opacity().0,
+ &bbox,
+ current_color,
+ values,
+ )
+ .map(|had_paint_server| {
+ if had_paint_server {
+ pangocairo::functions::update_layout(&self.cr, &layout);
+ pangocairo::functions::show_layout(&self.cr, &layout);
+ };
+ })
+ } else {
+ Ok(())
+ };
+
+ if res.is_ok() {
+ let mut need_layout_path = clipping;
let res = if !clipping {
- dc.set_source_paint_server(
+ self.set_source_paint_server(
acquired_nodes,
- &values.fill().0,
- values.fill_opacity().0,
+ &values.stroke().0,
+ values.stroke_opacity().0,
&bbox,
current_color,
values,
)
- .map(|had_paint_server| {
- if had_paint_server {
- pangocairo::functions::update_layout(&cr, &layout);
- pangocairo::functions::show_layout(&cr, &layout);
- };
- })
- } else {
- Ok(())
- };
-
- if res.is_ok() {
- let mut need_layout_path = clipping;
-
- let res = if !clipping {
- dc.set_source_paint_server(
- acquired_nodes,
- &values.stroke().0,
- values.stroke_opacity().0,
- &bbox,
- current_color,
- values,
- )
.map(|had_paint_server| {
if had_paint_server {
need_layout_path = true;
}
})
- } else {
- Ok(())
- };
+ } else {
+ Ok(())
+ };
- if res.is_ok() && need_layout_path {
- pangocairo::functions::update_layout(&cr, &layout);
- pangocairo::functions::layout_path(&cr, &layout);
-
- if !clipping {
- let (x0, y0, x1, y1) = cr.stroke_extents();
- let r = Rect::new(x0, y0, x1, y1);
- let ib = BoundingBox::new()
- .with_transform(transform)
- .with_ink_rect(r);
- cr.stroke();
- bbox.insert(&ib);
- }
+ if res.is_ok() && need_layout_path {
+ pangocairo::functions::update_layout(&self.cr, &layout);
+ pangocairo::functions::layout_path(&self.cr, &layout);
+
+ if !clipping {
+ 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);
+ self.cr.stroke();
+ bbox.insert(&ib);
}
}
+ }
- res.map(|_: ()| bbox)
- })
+ res.map(|_: ()| bbox)
}
pub fn get_snapshot(
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
