[librsvg] (#438): Only create a temporary ImageSurface if we'll need filtering



commit 4c678968e151763c75a5b798bc4161c50c010407
Author: Federico Mena Quintero <federico gnome org>
Date:   Thu Mar 14 08:46:00 2019 -0600

    (#438): Only create a temporary ImageSurface if we'll need filtering
    
    When with_discrete_layer() needs a temporary surface, the original C
    code distinguished between the case where filters will run (so it
    needs an ImageSurface), and the "easy" case, where it is sufficient to
    have a surface of similar type to the cr's current target.
    
    If we always create an ImageSurface, it causes the following problem.
    When rendering to a Cairo PDF surface, if some SVG elements have
    opacity or clip (but no filters), then those elements will be rendered
    to a raster surface unnecessarily, while PDF is perfectly capable of
    representing them with vector primitives.
    
    Also, check the status of the temporary surface by hand, since
    cairo-rs doesn't.
    
    Thanks to Julian Sparber, Jordan Petridis, and Zeeshan Ali for doing
    the huge bisect needed to find the culprit
    a9a5b65ee098919e8aa408b7d59a23fb3fc17b55 from last year!
    
    Fixes https://gitlab.gnome.org/GNOME/librsvg/issues/438

 rsvg_internals/src/drawing_ctx.rs       | 45 +++++++++++++++++++++++----------
 rsvg_internals/src/surface_utils/mod.rs | 20 +++++++++++++++
 2 files changed, 52 insertions(+), 13 deletions(-)
---
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index 93c52b87..27a06148 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -30,7 +30,7 @@ use crate::properties::{
     StrokeLinejoin,
 };
 use crate::rect::RectangleExt;
-use crate::surface_utils::shared_surface::SharedImageSurface;
+use crate::surface_utils::{shared_surface::SharedImageSurface, SurfaceExt};
 use crate::svg::Svg;
 use crate::unit_interval::UnitInterval;
 use crate::viewbox::ViewBox;
@@ -433,13 +433,31 @@ impl DrawingCtx {
                 && enable_background == EnableBackground::Accumulate);
 
             if needs_temporary_surface {
-                let surface = cairo::ImageSurface::create(
-                    cairo::Format::ARgb32,
-                    self.rect.width as i32,
-                    self.rect.height as i32,
-                )?;
+                let cr = if filter.is_some() {
+                    cairo::Context::new(&cairo::ImageSurface::create(
+                        cairo::Format::ARgb32,
+                        self.rect.width as i32,
+                        self.rect.height as i32,
+                    )?)
+                } else {
+                    // FIXME: cairo-rs should return a Result from create_similar()!
+                    // Since it doesn't, we need to check its status by hand...
+
+                    let surface = cairo::Surface::create_similar(
+                        &self.cr.get_target(),
+                        cairo::Content::ColorAlpha,
+                        self.rect.width as i32,
+                        self.rect.height as i32,
+                    );
+
+                    let status = surface.status();
+                    if status != cairo::Status::Success {
+                        return Err(RenderingError::Cairo(status));
+                    }
+
+                    cairo::Context::new(&surface)
+                };
 
-                let cr = cairo::Context::new(&surface);
                 cr.set_matrix(affine);
 
                 self.cr_stack.push(self.cr.clone());
@@ -452,12 +470,13 @@ impl DrawingCtx {
             let mut res = draw_fn(self);
 
             if needs_temporary_surface {
-                let child_surface = cairo::ImageSurface::from(self.cr.get_target()).unwrap();
-
-                let filter_result_surface = if let Some(filter_uri) = filter {
-                    self.run_filter(filter_uri, node, values, &child_surface)?
+                let source_surface = if let Some(filter_uri) = filter {
+                    let child_surface = cairo::ImageSurface::from(self.cr.get_target()).unwrap();
+                    let img_surface = self.run_filter(filter_uri, node, values, &child_surface)?;
+                    // turn into a Surface
+                    img_surface.as_ref().clone()
                 } else {
-                    child_surface
+                    self.cr.get_target()
                 };
 
                 self.cr = self.cr_stack.pop().unwrap();
@@ -465,7 +484,7 @@ impl DrawingCtx {
                 let (xofs, yofs) = self.get_offset();
 
                 original_cr.identity_matrix();
-                original_cr.set_source_surface(&filter_result_surface, xofs, yofs);
+                original_cr.set_source_surface(&source_surface, xofs, yofs);
 
                 if let Some(clip_node) = clip_in_object_space {
                     clip_node
diff --git a/rsvg_internals/src/surface_utils/mod.rs b/rsvg_internals/src/surface_utils/mod.rs
index efc28542..3b1ec267 100644
--- a/rsvg_internals/src/surface_utils/mod.rs
+++ b/rsvg_internals/src/surface_utils/mod.rs
@@ -2,6 +2,8 @@
 use std::ops::DerefMut;
 
 use cairo;
+use cairo_sys;
+use glib::translate::*;
 
 pub mod iterators;
 pub mod shared_surface;
@@ -149,3 +151,21 @@ impl Pixel {
 
 impl<'a> ImageSurfaceDataExt for cairo::ImageSurfaceData<'a> {}
 impl<'a> ImageSurfaceDataExt for &'a mut [u8] {}
+
+// FIXME: cairo-rs forgot to export its own SurfaceExt with the status() method
+// and others: https://github.com/gtk-rs/cairo/issues/252
+//
+// Remove the following when cairo-rs gets fixed.
+pub trait SurfaceExt {
+    fn status(&self) -> cairo::Status;
+}
+
+impl SurfaceExt for cairo::Surface {
+    fn status(&self) -> cairo::Status {
+        unsafe {
+            let raw_surface = self.to_glib_none();
+
+            cairo_sys::cairo_surface_status(raw_surface.0).into()
+        }
+    }
+}


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]