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

[Federico Mena Quintero <federico src gnome org>]

commit 325b7360ce24ab3c34971b7f2cc9cfb7bfbdbc4a
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       | 52 ++++++++++++++++++++++++++-------
 rsvg_internals/src/surface_utils/mod.rs | 20 +++++++++++++
 2 files changed, 61 insertions(+), 11 deletions(-)
---
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index 70b68cec..01986569 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;
@@ -221,6 +221,30 @@ impl DrawingCtx {
         )?)
     }
 
+    fn create_similar_surface_for_toplevel_viewport(
+        &self,
+        surface: &cairo::Surface,
+    ) -> Result<cairo::Surface, RenderingError> {
+        // This truncation may mean that we clip off the rightmost/bottommost row of pixels.
+        // See https://gitlab.gnome.org/GNOME/librsvg/issues/295
+
+        let width = self.rect.width as i32;
+        let height = self.rect.height as i32;
+
+        let surface =
+            cairo::Surface::create_similar(surface, cairo::Content::ColorAlpha, width, height);
+
+        // FIXME: cairo-rs should return a Result from create_similar()!
+        // Since it doesn't, we need to check its status by hand...
+
+        let status = surface.status();
+        if status == cairo::Status::Success {
+            Ok(surface)
+        } else {
+            Err(RenderingError::Cairo(status))
+        }
+    }
+
     /// Gets the viewport that was last pushed with `push_view_box()`.
     pub fn get_view_params(&self) -> ViewParams {
         let view_box_stack = self.view_box_stack.borrow();
@@ -440,9 +464,14 @@ impl DrawingCtx {
                     && enable_background == EnableBackground::Accumulate);
 
                 if needs_temporary_surface {
-                    let surface = dc.create_surface_for_toplevel_viewport()?;
+                    let cr = if filter.is_some() {
+                        cairo::Context::new(&dc.create_surface_for_toplevel_viewport()?)
+                    } else {
+                        cairo::Context::new(
+                            &dc.create_similar_surface_for_toplevel_viewport(&dc.cr.get_target())?,
+                        )
+                    };
 
-                    let cr = cairo::Context::new(&surface);
                     cr.set_matrix(affine);
 
                     dc.cr_stack.push(dc.cr.clone());
@@ -453,20 +482,21 @@ impl DrawingCtx {
 
                     let mut res = draw_fn(dc);
 
-                    let filter_result_surface = if let Some(filter_uri) = filter {
-                        dc.run_filter(filter_uri, node, values, &surface, dc.bbox)
-                            .map_err(|e| {
-                                dc.cr = dc.cr_stack.pop().unwrap();
-                                e
-                            })?
+                    let source_surface = if let Some(filter_uri) = filter {
+                        let child_surface =
+                            cairo::ImageSurface::from(dc.cr.get_target()).unwrap();
+                        let img_surface =
+                            dc.run_filter(filter_uri, node, values, &child_surface, dc.bbox)?;
+                        // turn into a Surface
+                        img_surface.as_ref().clone()
                     } else {
-                        surface
+                        dc.cr.get_target()
                     };
 
                     dc.cr = dc.cr_stack.pop().unwrap();
 
                     dc.cr.identity_matrix();
-                    dc.cr.set_source_surface(&filter_result_surface, 0.0, 0.0);
+                    dc.cr.set_source_surface(&source_surface, 0.0, 0.0);
 
                     dc.clip_to_node(&affine, clip_in_object_space)?;
 
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()
+        }
+    }
+}