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



commit 692c0b6569e2fd8caf38c6a9fdb16433ca2413e8
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.
    
    Thanks to Julian Sparber, Jordan Petridis, and Zeeshan Ali for doing
    the huge bisect needed to find the culprit
    a9a5b65ee098919e8aa408b7d59a23fb3fc17b55 from last year!
    
    Also, check the status of the temporary surface by hand, since
    cairo-rs doesn't.
    
    Fixes https://gitlab.gnome.org/GNOME/librsvg/issues/438

 rsvg_internals/src/drawing_ctx.rs       | 44 ++++++++++++++++++++++++---------
 rsvg_internals/src/surface_utils/mod.rs | 20 +++++++++++++++
 2 files changed, 52 insertions(+), 12 deletions(-)
---
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index 4524661a..ac330677 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -35,6 +35,7 @@ use state::{
     StrokeLinejoin,
     TextRendering,
 };
+use surface_utils::SurfaceExt;
 use tree::Tree;
 use unitinterval::UnitInterval;
 use viewbox::ViewBox;
@@ -393,13 +394,31 @@ impl<'a> DrawingCtx<'a> {
                 && 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());
@@ -412,12 +431,13 @@ impl<'a> DrawingCtx<'a> {
             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();
@@ -425,7 +445,7 @@ impl<'a> DrawingCtx<'a> {
                 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 clip_units == Some(CoordUnits::ObjectBoundingBox) {
                     let clip_node = acquired_clip.as_ref().unwrap().get();
diff --git a/rsvg_internals/src/surface_utils/mod.rs b/rsvg_internals/src/surface_utils/mod.rs
index 06d54a47..7c5a5836 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;
@@ -96,3 +98,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]